BIND 10 trac2472, updated. 2adb0227e8ad12a657fc80462928a51a3ceffae3 [2472] Miscellaneous fixes after review

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Nov 13 15:44:50 UTC 2012


The branch, trac2472 has been updated
       via  2adb0227e8ad12a657fc80462928a51a3ceffae3 (commit)
      from  7c8b1108b5df315e79aaa5b6ab21a655b24168cd (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 2adb0227e8ad12a657fc80462928a51a3ceffae3
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Nov 13 15:43:42 2012 +0000

    [2472] Miscellaneous fixes after review

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dhcp/memfile_lease_mgr.h                 |   17 +++++++-------
 src/lib/dhcp/mysql_lease_mgr.h                   |   10 ---------
 src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc |   13 ++++-------
 src/lib/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 2f66e8c..9120c5c 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<uint32_t, uint32_t> 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 e3f1a7a..bdd99cc 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<uint32_t, uint32_t> 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 f25ceab..53f277a 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<Memfile_LeaseMgr> 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<Memfile_LeaseMgr> 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 fc43bb5..103534f 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<uint32_t, uint32_t> 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_);



More information about the bind10-changes mailing list