From: Marcin Siodelski Date: Mon, 1 Sep 2014 11:01:17 +0000 (+0200) Subject: [3534] Implemented (in)equality operators for CfgIface. X-Git-Tag: trac3595_base~7^2~9 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9c8079972f761b7ad8aa4aaa2e59db5374f71308;p=thirdparty%2Fkea.git [3534] Implemented (in)equality operators for CfgIface. --- diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc index e7a630d875..76efd0c6fd 100644 --- a/src/lib/dhcpsrv/cfg_iface.cc +++ b/src/lib/dhcpsrv/cfg_iface.cc @@ -34,6 +34,13 @@ CfgIface::closeSockets() const { IfaceMgr::instance().closeSockets(); } +bool +CfgIface::equals(const CfgIface& other) const { + return (iface_set_ == other.iface_set_ && + unicast_map_ == other.unicast_map_ && + wildcard_used_ == other.wildcard_used_); +} + void CfgIface::openSockets(const Family& family, const uint16_t port, const bool use_bcast) const { diff --git a/src/lib/dhcpsrv/cfg_iface.h b/src/lib/dhcpsrv/cfg_iface.h index 14c5236936..c60a2f6006 100644 --- a/src/lib/dhcpsrv/cfg_iface.h +++ b/src/lib/dhcpsrv/cfg_iface.h @@ -86,6 +86,13 @@ public: /// @brief Convenience function which closes all open sockets. void closeSockets() const; + /// @brief Compares two @c CfgIface objects for equality. + /// + /// @param other An object to be compared with this object. + /// + /// @return true if objects are equal, false otherwise. + bool equals(const CfgIface& other) const; + /// @brief Tries to open sockets on selected interfaces. /// /// This function opens sockets bound to link-local address as well as @@ -139,6 +146,24 @@ public: /// @c CfgIface::use has been already called for this interface. void use(const Family& family, const std::string& iface_name); + /// @brief Equality operator. + /// + /// @param other Object to be compared with this object. + /// + /// @return true if objects are equal, false otherwise. + bool operator==(const CfgIface& other) const { + return (equals(other)); + } + + /// @brief Inequality operator. + /// + /// @param other Object to be compared with this object. + /// + /// @return true if objects are not equal, false otherwise. + bool operator!=(const CfgIface& other) const { + return (!equals(other)); + } + private: /// @brief Selects or deselects interfaces. @@ -165,9 +190,6 @@ private: /// @param errmsg Error message being logged by the function. static void socketOpenErrorHandler(const std::string& errmsg); - /// @brief Protocol family. - Family family_; - /// @brief Represents a set of interface names. typedef std::set IfaceSet; diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 588da3c84c..0176fc0e50 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -25,6 +25,8 @@ using namespace isc::util; namespace isc { namespace dhcp { +const size_t CfgMgr::CONFIG_LIST_SIZE = 10; + CfgMgr& CfgMgr::instance() { static CfgMgr cfg_mgr; @@ -376,6 +378,13 @@ CfgMgr::commit() { ensureCurrentAllocated(); if (!configs_.back()->sequenceEquals(*configuration_)) { configuration_ = configs_.back(); + // Keep track of the maximum size of the configs history. Before adding + // new element, we have to remove the oldest one. + if (configs_.size() > CONFIG_LIST_SIZE) { + ConfigurationList::iterator it = configs_.begin(); + std::advance(it, configs_.size() - CONFIG_LIST_SIZE); + configs_.erase(configs_.begin(), it); + } } } diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 83140f2cb1..1baaeab843 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -95,6 +95,11 @@ public: class CfgMgr : public boost::noncopyable { public: + /// @brief A number of configurations held by @c CfgMgr. + /// + /// @todo Make it configurable. + static const size_t CONFIG_LIST_SIZE; + /// @brief returns a single instance of Configuration Manager /// /// CfgMgr is a singleton and this method is the only way of @@ -385,6 +390,8 @@ public: /// @todo Migrate all configuration parameters to use the model supported /// by these functions. /// + /// @todo Make the size of the configurations history configurable. + /// //@{ /// @brief Removes current, staging and all previous configurations. @@ -399,7 +406,9 @@ public: /// @brief Commits the staging configuration. /// /// The staging configuration becomes current configuration when this - /// function is called. + /// function is called. It removes the oldest configurations held in the + /// history so as the size of the list of configuration does not excide + /// the @c CONFIG_LIST_SIZE. /// /// This function is exception safe. void commit(); diff --git a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc index 1884d3b358..4df8bbecf4 100644 --- a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc @@ -218,9 +218,60 @@ TEST_F(CfgIfaceTest, invalidValues) { ASSERT_THROW(cfg.use(CfgIface::V6, "eth0/fe80::3a60:77ff:fed5:cdef"), InvalidIfaceName); ASSERT_THROW(cfg.use(CfgIface::V6, "eth0/2001:db8:1::2"), NoSuchAddress); - ASSERT_NO_THROW(cfg.use(CfgIface::V6, "*")); ASSERT_THROW(cfg.use(CfgIface::V6, "*"), DuplicateIfaceName); } +// Test that the equality and inequality operators work fine for CfgIface. +TEST_F(CfgIfaceTest, equality) { + CfgIface cfg1; + CfgIface cfg2; + + // Initially objects must be equal. + EXPECT_TRUE(cfg1 == cfg2); + EXPECT_FALSE(cfg1 != cfg2); + + // Differ by one interface. + cfg1.use(CfgIface::V4, "eth0"); + EXPECT_FALSE(cfg1 == cfg2); + EXPECT_TRUE(cfg1 != cfg2); + + // Now interfaces should be equal. + cfg2.use(CfgIface::V4, "eth0"); + EXPECT_TRUE(cfg1 == cfg2); + EXPECT_FALSE(cfg1 != cfg2); + + // Differ by unicast address. + cfg1.use(CfgIface::V6, "eth0/2001:db8:1::1"); + EXPECT_FALSE(cfg1 == cfg2); + EXPECT_TRUE(cfg1 != cfg2); + + // Differ by unicast address and one interface. + cfg2.use(CfgIface::V6, "eth1"); + EXPECT_FALSE(cfg1 == cfg2); + EXPECT_TRUE(cfg1 != cfg2); + + // Now, the unicast addresses are equal but still differ by one interface. + cfg2.use(CfgIface::V6, "eth0/2001:db8:1::1"); + EXPECT_FALSE(cfg1 == cfg2); + EXPECT_TRUE(cfg1 != cfg2); + + // They should be now back to equal. + cfg1.use(CfgIface::V6, "eth1"); + EXPECT_TRUE(cfg1 == cfg2); + EXPECT_FALSE(cfg1 != cfg2); + + // Even though the wildcard doesn't change anything because all interfaces + // are already in use, the fact that the wildcard is specified should + // cause them to be not equal. + cfg1.use(CfgIface::V6, "*"); + EXPECT_FALSE(cfg1 == cfg2); + EXPECT_TRUE(cfg1 != cfg2); + + // Finally, both are equal as they use wildacard. + cfg2.use(CfgIface::V4, "*"); + EXPECT_TRUE(cfg1 == cfg2); + EXPECT_FALSE(cfg1 != cfg2); +} + } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 8be72d935f..d3579f7e4f 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -1125,7 +1125,7 @@ TEST_F(CfgMgrTest, subnet6Duplication) { } -// This test verifies that the configuration staging and commit works +// This test verifies that the configuration staging, commit and rollback works // as expected. TEST_F(CfgMgrTest, staging) { CfgMgr& cfg_mgr = CfgMgr::instance(); @@ -1159,7 +1159,7 @@ TEST_F(CfgMgrTest, staging) { // This should change the staging configuration so as it becomes a current // one. - CfgMgr::instance().commit(); + cfg_mgr.commit(); const_config = cfg_mgr.getCurrentCfg(); ASSERT_TRUE(const_config); // Sequence id equal to 1 indicates that the current configuration points @@ -1176,7 +1176,7 @@ TEST_F(CfgMgrTest, staging) { // changes the configuration having sequence 2 to current configuration. // Other commits are no-op. for (int i = 0; i < 5; ++i) { - CfgMgr::instance().commit(); + cfg_mgr.commit(); } // The current configuration now have sequence number 2. @@ -1185,15 +1185,29 @@ TEST_F(CfgMgrTest, staging) { EXPECT_EQ(2, const_config->getSequence()); // Clear configuration along with a history. - CfgMgr::instance().clear(); + cfg_mgr.clear(); // After clearing configuration we should successfully get the // new staging configuration. - const_config = cfg_mgr.getStagingCfg(); - ASSERT_TRUE(const_config); - EXPECT_EQ(1, const_config->getSequence()); -} + config = cfg_mgr.getStagingCfg(); + ASSERT_TRUE(config); + EXPECT_EQ(1, config->getSequence()); + // Modify the staging configuration. + config->addLoggingInfo(LoggingInfo()); + ASSERT_TRUE(config); + // The modified staging configuration should have one logger configured. + ASSERT_EQ(1, config->getLoggingInfo().size()); + + // Rollback should remove a staging configuration, including the logger. + ASSERT_NO_THROW(cfg_mgr.rollback()); + + // Make sure that the logger is not set. This is an indication that the + // rollback worked. + config = cfg_mgr.getStagingCfg(); + ASSERT_TRUE(config); + EXPECT_EQ(0, config->getLoggingInfo().size()); +} /// @todo Add unit-tests for testing: /// - addActiveIface() with invalid interface name