From 2adb0227e8ad12a657fc80462928a51a3ceffae3 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Tue, 13 Nov 2012 15:43:42 +0000 Subject: [PATCH] [2472] Miscellaneous fixes after review --- src/lib/dhcp/memfile_lease_mgr.h | 17 ++++++------ src/lib/dhcp/mysql_lease_mgr.h | 10 ------- .../dhcp/tests/memfile_lease_mgr_unittest.cc | 13 +++------- .../dhcp/tests/mysql_lease_mgr_unittest.cc | 26 +++++++++++++++---- 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/lib/dhcp/memfile_lease_mgr.h b/src/lib/dhcp/memfile_lease_mgr.h index 2f66e8c02e..9120c5cd6f 100644 --- a/src/lib/dhcp/memfile_lease_mgr.h +++ b/src/lib/dhcp/memfile_lease_mgr.h @@ -133,7 +133,7 @@ public: /// @param addr address of the searched lease /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const; + virtual Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const; /// @brief Returns existing IPv6 lease for a given DUID+IA combination /// @@ -143,7 +143,7 @@ public: /// @param iaid IA identifier /// /// @return collection of IPv6 leases - Lease6Collection getLease6(const DUID& duid, uint32_t iaid) const; + virtual Lease6Collection getLease6(const DUID& duid, uint32_t iaid) const; /// @brief Returns existing IPv6 lease for a given DUID+IA combination /// @@ -154,7 +154,7 @@ public: /// @param subnet_id identifier of the subnet the lease must belong to /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; + virtual Lease6Ptr getLease6(const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; /// @brief Updates IPv4 lease. /// @@ -163,7 +163,7 @@ public: /// @param lease4 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease4(const Lease4Ptr& lease4); + virtual void updateLease4(const Lease4Ptr& lease4); /// @brief Updates IPv4 lease. /// @@ -172,7 +172,7 @@ public: /// @param lease6 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease6(const Lease6Ptr& lease6); + virtual void updateLease6(const Lease6Ptr& lease6); /// @brief Deletes a lease. /// @@ -186,7 +186,7 @@ public: /// @param addr IPv4 address of the lease to be deleted. /// /// @return true if deletion was successful, false if no such lease exists - bool deleteLease6(const isc::asiolink::IOAddress& addr); + virtual bool deleteLease6(const isc::asiolink::IOAddress& addr); /// @brief Return backend type /// @@ -215,6 +215,9 @@ public: virtual std::string getDescription() const; /// @brief Returns backend version. + /// + /// @return Version number as a pair of unsigned integers. "first" is the + /// major version number, "second" the minor number. virtual std::pair getVersion() const { return (std::make_pair(1, 0)); } @@ -231,8 +234,6 @@ public: /// support transactions, this is a no-op. virtual void rollback(); - using LeaseMgr::getParameter; - protected: typedef boost::multi_index_container< // this is a multi-index container... diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h index e3f1a7a1ea..bdd99cc1df 100644 --- a/src/lib/dhcp/mysql_lease_mgr.h +++ b/src/lib/dhcp/mysql_lease_mgr.h @@ -267,16 +267,6 @@ public: /// @return Version number as a pair of unsigned integers. "first" is the /// major version number, "second" the minor number. /// - /// @todo: We will need to implement 3 version functions eventually: - /// A. abstract API version - /// B. backend version - /// C. database version (stored in the database scheme) - /// - /// and then check that: - /// B>=A and B=C (it is ok to have newer backend, as it should be backward - /// compatible) - /// Also if B>C, some database upgrade procedure may be triggered - /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. virtual std::pair getVersion() const; diff --git a/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc index f25ceabb9a..53f277add5 100644 --- a/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc @@ -45,7 +45,8 @@ TEST_F(MemfileLeaseMgrTest, constructor) { ASSERT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap))); } -TEST_F(MemfileLeaseMgrTest, GetTypeAndName) { +// Checks if the getType() and getName() methods both return "memfile". +TEST_F(MemfileLeaseMgrTest, getTypeAndName) { const LeaseMgr::ParameterMap pmap; // Empty parameter map boost::scoped_ptr lease_mgr(new Memfile_LeaseMgr(pmap)); @@ -53,14 +54,8 @@ TEST_F(MemfileLeaseMgrTest, GetTypeAndName) { EXPECT_EQ(std::string("memfile"), lease_mgr->getName()); } -// There's no point in calling any other methods in LeaseMgr, as they -// are purely virtual, so we would only call Memfile_LeaseMgr methods. -// Those methods are just stubs that does not return anything. -// It seems likely that we will need to extend the memfile code for -// allocation engine tests, so we may implement tests that call -// Memfile_LeaseMgr methods then. - -TEST_F(MemfileLeaseMgrTest, addGetDelete) { +// Checks that adding/getting/deleting a Lease6 object works. +TEST_F(MemfileLeaseMgrTest, addGetDeletei6) { const LeaseMgr::ParameterMap pmap; // Empty parameter map boost::scoped_ptr lease_mgr(new Memfile_LeaseMgr(pmap)); diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc index fc43bb56f4..103534f35f 100644 --- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc @@ -470,11 +470,25 @@ TEST(MySqlOpenTest, OpenDatabase) { destroySchema(); } -TEST_F(MySqlLeaseMgrTest, GetType) { +// @brief Check the getType() method +// +// getType() returns a string giving the type of the backend, which should +// always be "mysql". +TEST_F(MySqlLeaseMgrTest, getType) { EXPECT_EQ(std::string("mysql"), lmptr_->getType()); } // @brief Check conversion functions +// +// The server works using cltt and valid_filetime. In the database, the +// information is stored as expire_time and valid-lifetime, which are +// related by +// +// expire_time = cltt + valid_lifetime +// +// This test checks that the conversion is correct. It does not check that the +// data is entered into the database correctly, only that the MYSQL_TIME +// structure used for the entry is correctly set up. TEST_F(MySqlLeaseMgrTest, CheckTimeConversion) { const time_t cltt = time(NULL); const uint32_t valid_lft = 86400; // 1 day @@ -513,7 +527,7 @@ TEST_F(MySqlLeaseMgrTest, getName) { // @TODO: check for the negative } -// @brief Check that getVersion() works +// @brief Check that getVersion() returns the expected version TEST_F(MySqlLeaseMgrTest, CheckVersion) { // Check version pair version; @@ -522,13 +536,15 @@ TEST_F(MySqlLeaseMgrTest, CheckVersion) { EXPECT_EQ(CURRENT_VERSION_MINOR, version.second); } +// @brief Compare two Lease6 structures for equality void detailCompareLease6(const Lease6Ptr& first, const Lease6Ptr& second) { EXPECT_EQ(first->type_, second->type_); - // Compare address strings - odd things happen when they are different - // as the EXPECT_EQ appears to call the operator uint32_t() function, - // which causes an exception to be thrown for IPv6 addresses. + // Compare address strings. Comparison of address objects is not used, as + // odd things happen when they are different: the EXPECT_EQ macro appears to + // call the operator uint32_t() function, which causes an exception to be + // thrown for IPv6 addresses. EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); EXPECT_EQ(first->prefixlen_, second->prefixlen_); EXPECT_EQ(first->iaid_, second->iaid_); -- GitLab