[3083] Return the old lease instance when new lease is acquired.

src/lib/dhcpsrv/alloc_engine.cc

@@ -329,7 +329,10 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                               const HWAddrPtr& hwaddr,
                               const IOAddress& hint,
                               bool fake_allocation,
-                              const isc::hooks::CalloutHandlePtr& callout_handle) {
+                              const isc::hooks::CalloutHandlePtr& callout_handle,
+                              Lease4Ptr& old_lease) {
+
+    old_lease.reset();
 
     try {
         // Allocator is always created in AllocEngine constructor and there is
@@ -349,6 +352,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
         // Check if there's existing lease for that subnet/clientid/hwaddr combination.
         Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet->getID());
         if (existing) {
+            old_lease.reset(new Lease4(*existing));
             // We have a lease already. This is a returning client, probably after
             // its reboot.
             existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
@@ -363,6 +367,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
         if (clientid) {
             existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
             if (existing) {
+                old_lease.reset(new Lease4(*existing));
                 // we have a lease already. This is a returning client, probably after
                 // its reboot.
                 existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
@@ -393,6 +398,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                 }
             } else {
                 if (existing->expired()) {
+                    old_lease.reset(new Lease4(*existing));
                     return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
                                               callout_handle, fake_allocation));
                 }

src/lib/dhcpsrv/alloc_engine.h

@@ -196,6 +196,10 @@ protected:
     ///        an address for DISCOVER that is not really allocated (true)
     /// @param callout_handle a callout handle (used in hooks). A lease callouts
     ///        will be executed if this parameter is passed.
+    /// @param [out] old_lease Holds the pointer to a previous instance of a
+    /// lease. The NULL pointer indicates that lease didn't exist prior
+    /// to calling this function (e.g. new lease has been allocated).
+    ///
     /// @return Allocated IPv4 lease (or NULL if allocation failed)
     Lease4Ptr
     allocateAddress4(const SubnetPtr& subnet,
@@ -203,7 +207,8 @@ protected:
                      const HWAddrPtr& hwaddr,
                      const isc::asiolink::IOAddress& hint,
                      bool fake_allocation,
-                     const isc::hooks::CalloutHandlePtr& callout_handle);
+                     const isc::hooks::CalloutHandlePtr& callout_handle,
+                     Lease4Ptr& old_lease);
 
     /// @brief Renews a IPv4 lease
     ///

src/lib/dhcpsrv/lease_mgr.cc

@@ -38,6 +38,52 @@ Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2,
      subnet_id_(subnet_id), fixed_(false), fqdn_fwd_(false), fqdn_rev_(false) {
 }
 
+Lease4::Lease4(const Lease4& other)
+    : Lease(other.addr_, other.t1_, other.t2_, other.valid_lft_,
+            other.subnet_id_, other.cltt_), ext_(other.ext_),
+      hwaddr_(other.hwaddr_) {
+
+    fixed_ = other.fixed_;
+    fqdn_fwd_ = other.fqdn_fwd_;
+    fqdn_rev_ = other.fqdn_rev_;
+    hostname_ = other.hostname_;
+    comments_ = other.comments_;
+
+    if (other.client_id_) {
+        client_id_.reset(new ClientId(other.client_id_->getClientId()));
+
+    } else {
+        client_id_.reset();
+
+    }
+}
+
+Lease4&
+Lease4::operator=(const Lease4& other) {
+    if (this != &other) {
+        addr_ = other.addr_;
+        t1_ = other.t1_;
+        t2_ = other.t2_;
+        valid_lft_ = other.valid_lft_;
+        cltt_ = other.cltt_;
+        subnet_id_ = other.subnet_id_;
+        fixed_ = other.fixed_;
+        hostname_ = other.hostname_;
+        fqdn_fwd_ = other.fqdn_fwd_;
+        fqdn_rev_ = other.fqdn_rev_;
+        comments_ = other.comments_;
+        ext_ = other.ext_;
+        hwaddr_ = other.hwaddr_;
+
+        if (other.client_id_) {
+            client_id_.reset(new ClientId(other.client_id_->getClientId()));
+        } else {
+            client_id_.reset();
+        }
+    }
+    return (*this);
+}
+
 Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr,
                DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid,
                uint32_t t1, uint32_t t2, SubnetID subnet_id, uint8_t prefixlen)

src/lib/dhcpsrv/lease_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -259,6 +259,16 @@ struct Lease4 : public Lease {
     Lease4() : Lease(0, 0, 0, 0, 0, 0) {
     }
 
+    /// @brief Copy constructor
+    ///
+    /// @param other the @c Lease4 object to be copied.
+    Lease4(const Lease4& other);
+
+    /// @brief Assignment operator.
+    ///
+    /// @param other the @c Lease4 object to be assigned.
+    Lease4& operator=(const Lease4& other);
+
     /// @brief Compare two leases for equality
     ///
     /// @param other lease6 object with which to compare

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -188,6 +188,7 @@ public:
     Subnet4Ptr subnet_;       ///< Subnet4 (used in tests)
     Pool4Ptr pool_;           ///< Pool belonging to subnet_
     LeaseMgrFactory factory_; ///< Pointer to LeaseMgr factory
+    Lease4Ptr old_lease_;     ///< Holds previous instance of the lease.
 };
 
 // This test checks if the Allocation Engine can be instantiated and that it
@@ -613,7 +614,8 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("0.0.0.0"), false,
-                                               CalloutHandlePtr());
+                                               CalloutHandlePtr(),
+                                               old_lease_);
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -637,7 +639,8 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("0.0.0.0"), true,
-                                               CalloutHandlePtr());
+                                               CalloutHandlePtr(),
+                                               old_lease_);
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -660,7 +663,8 @@ TEST_F(AllocEngine4Test, allocWithValidHint4) {
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("192.0.2.105"),
-                                               false, CalloutHandlePtr());
+                                               false, CalloutHandlePtr(),
+                                               old_lease_);
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -700,7 +704,8 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     // twice.
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("192.0.2.106"),
-                                               false, CalloutHandlePtr());
+                                               false, CalloutHandlePtr(),
+                                               old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -734,7 +739,8 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
     // with the normal allocation
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("10.1.1.1"),
-                                               false, CalloutHandlePtr());
+                                               false, CalloutHandlePtr(),
+                                               old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -762,18 +768,22 @@ TEST_F(AllocEngine4Test, allocateAddress4Nulls) {
     // Allocations without subnet are not allowed
     Lease4Ptr lease = engine->allocateAddress4(SubnetPtr(), clientid_, hwaddr_,
                                                IOAddress("0.0.0.0"), false,
-                                               CalloutHandlePtr());
+                                               CalloutHandlePtr(), old_lease_);
     EXPECT_FALSE(lease);
 
     // Allocations without HW address are not allowed
     lease = engine->allocateAddress4(subnet_, clientid_, HWAddrPtr(),
-                                     IOAddress("0.0.0.0"), false, CalloutHandlePtr());
+                                     IOAddress("0.0.0.0"), false,
+                                     CalloutHandlePtr(),
+                                     old_lease_);
     EXPECT_FALSE(lease);
 
     // Allocations without client-id are allowed
     clientid_ = ClientIdPtr();
     lease = engine->allocateAddress4(subnet_, ClientIdPtr(), hwaddr_,
-                                     IOAddress("0.0.0.0"), false, CalloutHandlePtr());
+                                     IOAddress("0.0.0.0"), false,
+                                     CalloutHandlePtr(),
+                                     old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -877,7 +887,8 @@ TEST_F(AllocEngine4Test, smallPool4) {
     cfg_mgr.addSubnet4(subnet_);
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
-                                               false, CalloutHandlePtr());
+                                               false, CalloutHandlePtr(),
+                                               old_lease_);
 
     // Check that we got that single lease
     ASSERT_TRUE(lease);
@@ -926,7 +937,8 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
 
     Lease4Ptr lease2 = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                 IOAddress("0.0.0.0"), false,
-                                                CalloutHandlePtr());
+                                                CalloutHandlePtr(),
+                                                old_lease_);
     EXPECT_FALSE(lease2);
 }
 
@@ -959,7 +971,8 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
 
     // CASE 1: Asking for any address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
-                                     true, CalloutHandlePtr());
+                                     true, CalloutHandlePtr(),
+                                     old_lease_);
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr.toText(), lease->addr_.toText());
@@ -969,7 +982,8 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
 
     // CASE 2: Asking specifically for this address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress(addr.toText()),
-                                     true, CalloutHandlePtr());
+                                     true, CalloutHandlePtr(),
+                                     old_lease_);
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr.toText(), lease->addr_.toText());
@@ -997,7 +1011,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     // A client comes along, asking specifically for this address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                      IOAddress(addr.toText()), false,
-                                     CalloutHandlePtr());
+                                     CalloutHandlePtr(),
+                                     old_lease_);
 
     // Check that he got that single lease
     ASSERT_TRUE(lease);
@@ -1399,7 +1414,8 @@ TEST_F(HookAllocEngine4Test, lease4_select) {
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("0.0.0.0"),
-                                               false, callout_handle);
+                                               false, callout_handle,
+                                               old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -1462,7 +1478,8 @@ TEST_F(HookAllocEngine4Test, change_lease4_select) {
 
     // Call allocateAddress4. Callouts should be triggered here.
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
-                                               false, callout_handle);
+                                               false, callout_handle,
+                                               old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 

src/lib/dhcpsrv/tests/lease_mgr_unittest.cc

@@ -245,7 +245,7 @@ TEST(LeaseMgr, getParameter) {
 ///
 /// Lease4 is also defined in lease_mgr.h, so is tested in this file as well.
 // This test checks if the Lease4 structure can be instantiated correctly
-TEST(Lease4, Lease4Constructor) {
+TEST(Lease4, constructor) {
 
     // Random values for the tests
     const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
@@ -292,12 +292,79 @@ TEST(Lease4, Lease4Constructor) {
     }
 }
 
+// This test verfies that copy constructor copies Lease4 fields correctly.
+TEST(Lease4, copyConstructor) {
+
+    // Random values for the tests
+    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
+    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
+
+    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
+    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
+    ClientId clientid(clientid_vec);
+
+    // ...and a time
+    const time_t current_time = time(NULL);
+
+    // Other random constants.
+    const uint32_t SUBNET_ID = 42;
+    const uint32_t VALID_LIFETIME = 500;
+
+    // Create the lease
+    Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR),
+                 CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
+                 SUBNET_ID);
+
+    // Use copy constructor to copy the lease.
+    Lease4 copied_lease(lease);
+
+    // Both leases should be now equal. When doing this check we assume that
+    // the equality operator works correctly.
+    EXPECT_TRUE(lease == copied_lease);
+    // Client IDs are equal, but they should be in two distinct pointers.
+    EXPECT_FALSE(lease.client_id_ == copied_lease.client_id_);
+}
+
+// This test verfies that the assignment operator copies all Lease4 fields
+// correctly.
+TEST(Lease4, operatorAssign) {
+
+    // Random values for the tests
+    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
+    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
+
+    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
+    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
+    ClientId clientid(clientid_vec);
+
+    // ...and a time
+    const time_t current_time = time(NULL);
+
+    // Other random constants.
+    const uint32_t SUBNET_ID = 42;
+    const uint32_t VALID_LIFETIME = 500;
+
+    // Create the lease
+    Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR),
+                 CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
+                 SUBNET_ID);
+
+    // Use assignment operator to assign the lease.
+    Lease4 copied_lease = lease;
+
+    // Both leases should be now equal. When doing this check we assume that
+    // the equality operator works correctly.
+    EXPECT_TRUE(lease == copied_lease);
+    // Client IDs are equal, but they should be in two distinct pointers.
+    EXPECT_FALSE(lease.client_id_ == copied_lease.client_id_);
+}
+
 /// @brief Lease4 Equality Test
 ///
 /// Checks that the operator==() correctly compares two leases for equality.
 /// As operator!=() is also defined for this class, every check on operator==()
 /// is followed by the reverse check on operator!=().
-TEST(Lease4, OperatorEquals) {
+TEST(Lease4, operatorEquals) {
 
     // Random values for the tests
     const uint32_t ADDRESS = 0x01020304;