[2546] Unify deleteLease4() and deleteLease6() into a single deleteLease()

src/lib/dhcpsrv/lease_mgr.h

@@ -555,17 +555,11 @@ public:
 
     /// @brief Deletes a lease.
     ///
-    /// @param addr IPv4 address of the lease to be deleted.
+    /// @param addr Address of the lease to be deleted. (This can be IPv4 or
+    ///        IPv6.)
     ///
     /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease4(const isc::asiolink::IOAddress& addr) = 0;
-
-    /// @brief Deletes a lease.
-    ///
-    /// @param addr IPv6 address of the lease to be deleted.
-    ///
-    /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease6(const isc::asiolink::IOAddress& addr) = 0;
+    virtual bool deleteLease(const isc::asiolink::IOAddress& addr) = 0;
 
     /// @brief Return backend type
     ///

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -97,19 +97,22 @@ void Memfile_LeaseMgr::updateLease6(const Lease6Ptr& ) {
 
 }
 
-bool Memfile_LeaseMgr::deleteLease4(const isc::asiolink::IOAddress&) {
+bool Memfile_LeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
+    if (addr.isV4()) {
+        // V4 not implemented yet
         return (false);
-}
 
-bool Memfile_LeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) {
+    } else {
+        // V6 lease
         Lease6Storage::iterator l = storage6_.find(addr);
         if (l == storage6_.end()) {
-        // no such lease
+            // No such lease
             return (false);
         } else {
             storage6_.erase(l);
             return (true);
         }
+    }
 }
 
 std::string Memfile_LeaseMgr::getDescription() const {

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -166,17 +166,11 @@ public:
 
     /// @brief Deletes a lease.
     ///
-    /// @param addr IPv4 address of the lease to be deleted.
+    /// @param addr Address of the lease to be deleted. (This can be IPv4 or
+    ///        IPv6.)
     ///
     /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease4(const isc::asiolink::IOAddress& addr);
-
-    /// @brief Deletes a lease.
-    ///
-    /// @param addr IPv4 address of the lease to be deleted.
-    ///
-    /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease6(const isc::asiolink::IOAddress& addr);
+    virtual bool deleteLease(const isc::asiolink::IOAddress& addr);
 
     /// @brief Return backend type
     ///

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -1555,12 +1555,13 @@ MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) {
     updateLeaseCommon(stindex, &bind[0], lease);
 }
 
-// Delete lease methods.  As with other groups of methods, these comprise
-// a per-type method that sets up the relevant MYSQL_BIND array and a
-// common method than handles the common processing.
+// Delete lease methods.  Similar to other groups of methods, these comprise
+// a per-type method that sets up the relevant MYSQL_BIND array (in this
+// case, a single method for both V4 and V6 addresses) and a common method that
+// handles the common processing.
 
 bool
-MySqlLeaseMgr::deleteLease(StatementIndex stindex, MYSQL_BIND* bind) {
+MySqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind) {
 
     // Bind the input parameters to the statement
     int status = mysql_stmt_bind_param(statements_[stindex], bind);
@@ -1577,31 +1578,22 @@ MySqlLeaseMgr::deleteLease(StatementIndex stindex, MYSQL_BIND* bind) {
 
 
 bool
-MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& addr) {
+MySqlLeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
 
     // Set up the WHERE clause value
     MYSQL_BIND inbind[1];
     memset(inbind, 0, sizeof(inbind));
 
+    if (addr.isV4()) {
         uint32_t addr4 = static_cast<uint32_t>(addr);
 
-    // See the earlier description of the use of "const_cast" when accessing
-    // the address for an explanation of the reason.
         inbind[0].buffer_type = MYSQL_TYPE_LONG;
         inbind[0].buffer = reinterpret_cast<char*>(&addr4);
         inbind[0].is_unsigned = MLM_TRUE;
 
-    return (deleteLease(DELETE_LEASE4, inbind));
-}
-
-
-bool
-MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) {
-
-    // Set up the WHERE clause value
-    MYSQL_BIND inbind[1];
-    memset(inbind, 0, sizeof(inbind));
+        return (deleteLeaseCommon(DELETE_LEASE4, inbind));
 
+    } else {
         std::string addr6 = addr.toText();
         unsigned long addr6_length = addr6.size();
 
@@ -1612,7 +1604,8 @@ MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) {
         inbind[0].buffer_length = addr6_length;
         inbind[0].length = &addr6_length;
 
-    return (deleteLease(DELETE_LEASE6, inbind));
+        return (deleteLeaseCommon(DELETE_LEASE6, inbind));
+    }
 }
 
 // Miscellaneous database methods.

src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -273,33 +273,16 @@ public:
     ///        failed.
     virtual void updateLease6(const Lease6Ptr& lease6);
 
-    /// @brief Deletes an IPv4 lease.
+    /// @brief Deletes a lease.
     ///
-    /// @todo Merge with deleteLease6: it is possible to determine whether
-    ///       an address is V4 or V6 from the IOAddress argument, so there
-    ///       is no need for separate V4 or V6 methods.
-    ///
-    /// @param addr IPv4 address of the lease to be deleted.
-    ///
-    /// @return true if deletion was successful, false if no such lease exists
-    ///
-    /// @throw isc::dhcp::DbOperationError An operation on the open database has
-    ///        failed.
-    virtual bool deleteLease4(const isc::asiolink::IOAddress& addr);
-
-    /// @brief Deletes an IPv6 lease.
-    ///
-    /// @todo Merge with deleteLease4: it is possible to determine whether
-    ///       an address is V4 or V6 from the IOAddress argument, so there
-    ///       is no need for separate V4 or V6 methods.
-    ///
-    /// @param addr IPv6 address of the lease to be deleted.
+    /// @param addr Address of the lease to be deleted.  This can be an IPv4
+    ///             address or an IPv6 address.
     ///
     /// @return true if deletion was successful, false if no such lease exists
     ///
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
-    virtual bool deleteLease6(const isc::asiolink::IOAddress& addr);
+    virtual bool deleteLease(const isc::asiolink::IOAddress& addr);
 
     /// @brief Return backend type
     ///
@@ -600,7 +583,7 @@ private:
     ///
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
-    bool deleteLease(StatementIndex stindex, MYSQL_BIND* bind);
+    bool deleteLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind);
 
     /// @brief Check Error and Throw Exception
     ///

src/lib/dhcpsrv/tests/lease_mgr_unittest.cc

@@ -169,19 +169,11 @@ public:
 
     /// @brief Deletes a lease.
     ///
-    /// @param addr IPv4 address of the lease to be deleted.
+    /// @param addr Address of the lease to be deleted. (This can be either
+    ///        a V4 address or a V6 address.)
     ///
     /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease4(const isc::asiolink::IOAddress&) {
-        return (false);
-    }
-
-    /// @brief Deletes a lease.
-    ///
-    /// @param addr IPv4 address of the lease to be deleted.
-    ///
-    /// @return true if deletion was successful, false if no such lease exists
-    virtual bool deleteLease6(const isc::asiolink::IOAddress&) {
+    virtual bool deleteLease(const isc::asiolink::IOAddress&) {
         return (false);
     }
 

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -120,10 +120,10 @@ TEST_F(MemfileLeaseMgrTest, addGetDelete6) {
     EXPECT_FALSE(y);
 
     // should return false - there's no such address
-    EXPECT_FALSE(lease_mgr->deleteLease6(IOAddress("2001:db8:1::789")));
+    EXPECT_FALSE(lease_mgr->deleteLease(IOAddress("2001:db8:1::789")));
 
     // this one should succeed
-    EXPECT_TRUE(lease_mgr->deleteLease6(IOAddress("2001:db8:1::456")));
+    EXPECT_TRUE(lease_mgr->deleteLease(IOAddress("2001:db8:1::456")));
 
     // after the lease is deleted, it should really be gone
     x = lease_mgr->getLease6(IOAddress("2001:db8:1::456"));

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -708,7 +708,8 @@ TEST_F(MySqlLeaseMgrTest, checkVersion) {
 
 /// @brief Basic Lease4 Checks
 ///
-/// Checks that the addLease, getLease4 (by address) and deleteLease4 works.
+/// Checks that the addLease, getLease4 (by address) and deleteLease (with an
+/// IPv4 address) works.
 TEST_F(MySqlLeaseMgrTest, basicLease4) {
     // Get the leases to be used for the test.
     vector<Lease4Ptr> leases = createLeases4();
@@ -740,10 +741,10 @@ TEST_F(MySqlLeaseMgrTest, basicLease4) {
 
     // Delete a lease, check that it's gone, and that we can't delete it
     // a second time.
-    EXPECT_TRUE(lmptr_->deleteLease4(ioaddress4_[1]));
+    EXPECT_TRUE(lmptr_->deleteLease(ioaddress4_[1]));
     l_returned = lmptr_->getLease4(ioaddress4_[1]);
     EXPECT_FALSE(l_returned);
-    EXPECT_FALSE(lmptr_->deleteLease4(ioaddress4_[1]));
+    EXPECT_FALSE(lmptr_->deleteLease(ioaddress4_[1]));
 
     // Check that the second address is still there.
     l_returned = lmptr_->getLease4(ioaddress4_[2]);
@@ -753,7 +754,8 @@ TEST_F(MySqlLeaseMgrTest, basicLease4) {
 
 /// @brief Basic Lease6 Checks
 ///
-/// Checks that the addLease, getLease6 (by address) and deleteLease6 works.
+/// Checks that the addLease, getLease6 (by address) and deleteLease (with an
+/// IPv6 address) works.
 TEST_F(MySqlLeaseMgrTest, basicLease6) {
     // Get the leases to be used for the test.
     vector<Lease6Ptr> leases = createLeases6();
@@ -785,10 +787,10 @@ TEST_F(MySqlLeaseMgrTest, basicLease6) {
 
     // Delete a lease, check that it's gone, and that we can't delete it
     // a second time.
-    EXPECT_TRUE(lmptr_->deleteLease6(ioaddress6_[1]));
+    EXPECT_TRUE(lmptr_->deleteLease(ioaddress6_[1]));
     l_returned = lmptr_->getLease6(ioaddress6_[1]);
     EXPECT_FALSE(l_returned);
-    EXPECT_FALSE(lmptr_->deleteLease6(ioaddress6_[1]));
+    EXPECT_FALSE(lmptr_->deleteLease(ioaddress6_[1]));
 
     // Check that the second address is still there.
     l_returned = lmptr_->getLease6(ioaddress6_[2]);
@@ -857,7 +859,7 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSize) {
         Lease4Collection returned = lmptr_->getLease4(leases[1]->hwaddr_);
         ASSERT_EQ(1, returned.size());
         detailCompareLease(leases[1], *returned.begin());
-        (void) lmptr_->deleteLease4(leases[1]->addr_);
+        (void) lmptr_->deleteLease(leases[1]->addr_);
     }
 
     // Expect some problem when accessing a lease that had too long a hardware
@@ -915,7 +917,7 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
     // "multiple records" exception. (We expect there to be only one record
     // with that combination, so getting them via getLeaseX() (as opposed
     // to getLeaseXCollection() should throw an exception.)
-    EXPECT_TRUE(lmptr_->deleteLease4(leases[2]->addr_));
+    EXPECT_TRUE(lmptr_->deleteLease(leases[2]->addr_));
     leases[1]->addr_ = leases[2]->addr_;
     EXPECT_TRUE(lmptr_->addLease(leases[1]));
     EXPECT_THROW(returned = lmptr_->getLease4(leases[1]->hwaddr_,
@@ -925,7 +927,7 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
     // Delete all leases in the database
     for (int i = 0; ADDRESS4[i] != NULL; ++i) {
         IOAddress addr(ADDRESS4[i]);
-        (void) lmptr_->deleteLease4(addr);
+        (void) lmptr_->deleteLease(addr);
     }
 }
 
@@ -947,7 +949,7 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetIdSize) {
                                                leases[1]->subnet_id_);
         ASSERT_TRUE(returned);
         detailCompareLease(leases[1], returned);
-        (void) lmptr_->deleteLease4(leases[1]->addr_);
+        (void) lmptr_->deleteLease(leases[1]->addr_);
     }
 
     // Expect some error when getting a lease with too long a hardware
@@ -1030,7 +1032,7 @@ TEST_F(MySqlLeaseMgrTest, getLease4ClientIdSize) {
         Lease4Collection returned = lmptr_->getLease4(*leases[1]->client_id_);
         ASSERT_TRUE(returned.size() == 1);
         detailCompareLease(leases[1], *returned.begin());
-        (void) lmptr_->deleteLease4(leases[1]->addr_);
+        (void) lmptr_->deleteLease(leases[1]->addr_);
     }
 
     // Don't bother to check client IDs longer than the maximum -
@@ -1140,7 +1142,7 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaidSize) {
                                                       leases[1]->iaid_);
         EXPECT_EQ(1, returned.size());
         detailCompareLease(leases[1], *returned.begin());
-        (void) lmptr_->deleteLease6(leases[1]->addr_);
+        (void) lmptr_->deleteLease(leases[1]->addr_);
     }
 
     // Don't bother to check DUIDs longer than the maximum - these cannot be
@@ -1206,7 +1208,7 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaidSubnetIdSize) {
                                                leases[1]->subnet_id_);
         ASSERT_TRUE(returned);
         detailCompareLease(leases[1], returned);
-        (void) lmptr_->deleteLease6(leases[1]->addr_);
+        (void) lmptr_->deleteLease(leases[1]->addr_);
     }
 
     // Don't bother to check DUIDs longer than the maximum - these cannot be
@@ -1254,7 +1256,7 @@ TEST_F(MySqlLeaseMgrTest, updateLease4) {
     detailCompareLease(leases[1], l_returned);
 
     // Try updating a lease not in the database.
-    lmptr_->deleteLease4(ioaddress4_[2]);
+    lmptr_->deleteLease(ioaddress4_[2]);
     EXPECT_THROW(lmptr_->updateLease4(leases[2]), isc::dhcp::NoSuchLease);
 }