Browse Source

[master] Merge branch 'trac2940'

Conflicts:
	src/lib/dhcpsrv/lease.cc
	src/lib/dhcpsrv/lease.h
Marcin Siodelski 11 years ago
parent
commit
a232f3d7d9

+ 1 - 0
src/bin/d2/Makefile.am

@@ -76,6 +76,7 @@ b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/asiodns/libb10-asiodns.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libb10-dhcp_ddns.la
+b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/util/libb10-util.la

+ 1 - 0
src/bin/d2/tests/Makefile.am

@@ -94,6 +94,7 @@ d2_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libb10-dhcp_ddns.la
+d2_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la

+ 3 - 3
src/lib/dhcp/duid.cc

@@ -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
@@ -46,7 +46,7 @@ DUID::DUID(const uint8_t* data, size_t len) {
     duid_ = std::vector<uint8_t>(data, data + len);
 }
 
-std::vector<uint8_t> DUID::getDuid() const {
+const std::vector<uint8_t>& DUID::getDuid() const {
     return (duid_);
 }
 
@@ -104,7 +104,7 @@ ClientId::ClientId(const uint8_t *clientid, size_t len)
 }
 
 // Returns a copy of client-id data
-std::vector<uint8_t> ClientId::getClientId() const {
+const std::vector<uint8_t>& ClientId::getClientId() const {
     return (duid_);
 }
 

+ 17 - 9
src/lib/dhcp/duid.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
@@ -58,12 +58,13 @@ class DUID {
 
     /// @brief Returns a const reference to the actual DUID value
     ///
-    /// Note: For safety reasons, this method returns a copy of data as
-    /// otherwise the reference would be only valid as long as the object that
-    /// returned it. In any case, this method should be used only sporadically.
-    /// If there are frequent uses, we must implement some other method
-    /// (e.g. storeSelf()) that will avoid data copying.
-    std::vector<uint8_t> getDuid() const;
+    /// @warning Since this function returns a reference to the vector (not a
+    /// copy) the returned object must be used with caution because it remains
+    /// valid only for the time period when the object which returned it is
+    /// valid.
+    ///
+    /// @return A reference to a vector holding a DUID.
+    const std::vector<uint8_t>& getDuid() const;
 
     /// @brief Returns the DUID type
     DUIDType getType() const;
@@ -116,8 +117,15 @@ public:
     /// @brief Constructor based on array and array size
     ClientId(const uint8_t* clientid, size_t len);
 
-    /// @brief Returns reference to the client-id data
-    std::vector<uint8_t> getClientId() const;
+    /// @brief Returns reference to the client-id data.
+    ///
+    /// @warning Since this function returns a reference to the vector (not a
+    /// copy) the returned object must be used with caution because it remains
+    /// valid only for the time period when the object which returned it is
+    /// valid.
+    ///
+    /// @return A reference to a vector holding a client identifier.
+    const std::vector<uint8_t>& getClientId() const;
 
     /// @brief Returns textual representation of a DUID (e.g. 00:01:02:03:ff)
     std::string toText() const;

+ 20 - 0
src/lib/dhcpsrv/lease.cc

@@ -74,6 +74,16 @@ Lease4::Lease4(const Lease4& other)
     }
 }
 
+const std::vector<uint8_t>&
+Lease4::getClientIdVector() const {
+    if(!client_id_) {
+        static std::vector<uint8_t> empty_vec;
+        return (empty_vec);
+    }
+
+    return (client_id_->getClientId());
+}
+
 bool
 Lease4::matches(const Lease4& other) const {
     if ((client_id_ && !other.client_id_) ||
@@ -149,6 +159,16 @@ Lease6::Lease6(Type type, const isc::asiolink::IOAddress& addr,
     cltt_ = time(NULL);
 }
 
+const std::vector<uint8_t>&
+Lease6::getDuidVector() const {
+    if (!duid_) {
+        static std::vector<uint8_t> empty_vec;
+        return (empty_vec);
+    }
+
+    return (duid_->getDuid());
+}
+
 std::string
 Lease6::toText() const {
     ostringstream stream;

+ 21 - 0
src/lib/dhcpsrv/lease.h

@@ -209,6 +209,17 @@ struct Lease4 : public Lease {
     /// @param other the @c Lease4 object to be copied.
     Lease4(const Lease4& other);
 
+    /// @brief Returns a client identifier.
+    ///
+    /// @warning Since the function returns the reference to a vector (not a
+    /// copy), the returned object should be used with caution because it will
+    /// remain valid only for the period of time when an object which returned
+    /// it exists.
+    ///
+    /// @return A reference to a vector holding client identifier,
+    /// or an empty vector if client identifier is NULL.
+    const std::vector<uint8_t>& getClientIdVector() const;
+
     /// @brief Check if two objects encapsulate the lease for the same
     /// client.
     ///
@@ -334,6 +345,16 @@ struct Lease6 : public Lease {
         type_(TYPE_NA) {
     }
 
+    /// @brief Returns a reference to a vector representing a DUID.
+    ///
+    /// @warning Since the function returns the reference to a vector (not a
+    /// copy), the returned object should be used with caution because it will
+    /// remain valid only for the period of time when an object which returned
+    /// it exists.
+    ///
+    /// @return A reference to a vector holding a DUID.
+    const std::vector<uint8_t>& getDuidVector() const;
+
     /// @brief Checks if two lease objects encapsulate the lease for the same
     /// client.
     ///

+ 14 - 0
src/lib/dhcpsrv/lease_mgr.h

@@ -203,6 +203,20 @@ public:
     /// @return lease collection
     virtual Lease4Collection getLease4(const ClientId& clientid) const = 0;
 
+    /// @brief Returns existing IPv4 lease for a given client identifier,
+    /// HW address and subnet identifier.
+    ///
+    /// @todo Consider whether this function is needed or not. In the basic
+    /// DHCPv4 server implementation it is not used by the allocation engine.
+    ///
+    /// @param client_id A client identifier.
+    /// @param hwaddr Hardware address.
+    /// @param subnet_id A subnet identifier.
+    ///
+    /// @return A pointer to the lease or NULL if the lease is not found.
+    virtual Lease4Ptr getLease4(const ClientId& clientid, const HWAddr& hwaddr,
+                                SubnetID subnet_id) const = 0;
+
     /// @brief Returns existing IPv4 lease for specified client-id
     ///
     /// There can be at most one lease for a given HW address in a single

+ 6 - 6
src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -80,8 +80,8 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr) const {
         lease != idx.end(); ++lease) {
 
         // Every Lease4 has a hardware address, so we can compare it
-        if((* lease)->hwaddr_ == hwaddr.hwaddr_) {
-            collection.push_back((* lease));
+        if ((*lease)->hwaddr_ == hwaddr.hwaddr_) {
+            collection.push_back((*lease));
         }
     }
 
@@ -113,9 +113,9 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr, SubnetID subnet_id) const {
 }
 
 Lease4Collection
-Memfile_LeaseMgr::getLease4(const ClientId& clientid) const {
+Memfile_LeaseMgr::getLease4(const ClientId& client_id) const {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_CLIENTID).arg(clientid.toText());
+              DHCPSRV_MEMFILE_GET_CLIENTID).arg(client_id.toText());
     typedef Memfile_LeaseMgr::Lease4Storage::nth_index<0>::type SearchIndex;
     Lease4Collection collection;
     const SearchIndex& idx = storage4_.get<0>();
@@ -124,8 +124,8 @@ Memfile_LeaseMgr::getLease4(const ClientId& clientid) const {
 
         // client-id is not mandatory in DHCPv4. There can be a lease that does
         // not have a client-id. Dereferencing null pointer would be a bad thing
-        if((*lease)->client_id_ && *(*lease)->client_id_ == clientid) {
-            collection.push_back((* lease));
+        if((*lease)->client_id_ && *(*lease)->client_id_ == client_id) {
+            collection.push_back((*lease));
         }
     }
 

+ 14 - 45
src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -16,7 +16,6 @@
 #define MEMFILE_LEASE_MGR_H
 
 #include <dhcp/hwaddr.h>
-#include <dhcpsrv/key_from_key.h>
 #include <dhcpsrv/lease_mgr.h>
 
 #include <boost/multi_index/indexed_by.hpp>
@@ -102,8 +101,8 @@ public:
 
     /// @brief Returns existing IPv4 lease for specified client-id
     ///
-    /// @param clientid client identifier
-    virtual Lease4Collection getLease4(const ClientId& clientid) const;
+    /// @param client_id client identifier
+    virtual Lease4Collection getLease4(const ClientId& client_id) const;
 
     /// @brief Returns IPv4 lease for specified client-id/hwaddr/subnet-id tuple
     ///
@@ -267,20 +266,10 @@ protected:
                 // the lease using three attributes: DUID, IAID, Subnet Id.
                 boost::multi_index::composite_key<
                     Lease6,
-                    // The DUID value can't be directly accessed from the Lease6
-                    // object because it is wrapped with the DUID object (actually
-                    // pointer to this object). Therefore we need to use
-                    // KeyFromKeyExtractor class to extract the DUID value from
-                    // this cascaded structure.
-                    KeyFromKeyExtractor<
-                        // The value of the DUID is accessed by the getDuid() method
-                        // from the DUID object.
-                        boost::multi_index::const_mem_fun<DUID, std::vector<uint8_t>,
-                                                          &DUID::getDuid>,
-                        // The DUID object is stored in the duid_ member of the
-                        // Lease6 object.
-                        boost::multi_index::member<Lease6, DuidPtr, &Lease6::duid_>
-                    >,
+                    // The DUID can be retrieved from the Lease6 object using
+                    // a getDuidVector const function.
+                    boost::multi_index::const_mem_fun<Lease6, const std::vector<uint8_t>&,
+                                                      &Lease6::getDuidVector>,
                     // The two other ingredients of this index are IAID and
                     // subnet id.
                     boost::multi_index::member<Lease6, uint32_t, &Lease6::iaid_>,
@@ -330,20 +319,10 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    // The client id value is not directly accessible through the
-                    // Lease4 object as it is wrapped with the ClientIdPtr object.
-                    // Therefore we use the KeyFromKeyExtractor class to access
-                    // client id through this cascaded structure. The client id
-                    // is used as an index value.
-                    KeyFromKeyExtractor<
-                        // Specify that the vector holding client id value can be obtained
-                        // from the ClientId object.
-                        boost::multi_index::const_mem_fun<ClientId, std::vector<uint8_t>,
-                                                          &ClientId::getClientId>,
-                        // Specify that the ClientId object (actually pointer to it) can
-                        // be accessed by the client_id_ member of Lease4 class.
-                        boost::multi_index::member<Lease4, ClientIdPtr, &Lease4::client_id_>
-                    >,
+                    // The client id can be retrieved from the Lease4 object by
+                    // calling getClientIdVector const function.
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
+                                                      &Lease4::getClientIdVector>,
                     // The subnet id is accessed through the subnet_id_ member.
                     boost::multi_index::member<Lease, uint32_t, &Lease::subnet_id_>
                 >
@@ -355,20 +334,10 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    // The client id value is not directly accessible through the
-                    // Lease4 object as it is wrapped with the ClientIdPtr object.
-                    // Therefore we use the KeyFromKeyExtractor class to access
-                    // client id through this cascaded structure. The client id
-                    // is used as an index value.
-                    KeyFromKeyExtractor<
-                        // Specify that the vector holding client id value can be obtained
-                        // from the ClientId object.
-                        boost::multi_index::const_mem_fun<ClientId, std::vector<uint8_t>,
-                                                          &ClientId::getClientId>,
-                        // Specify that the ClientId object (actually pointer to it) can
-                        // be accessed by the client_id_ member of Lease4 class.
-                        boost::multi_index::member<Lease4, ClientIdPtr, &Lease4::client_id_>
-                    >,
+                    // The client id can be retrieved from the Lease4 object by
+                    // calling getClientIdVector const function.
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
+                                                      &Lease4::getClientIdVector>,
                     // The hardware address is held in the hwaddr_ member of the
                     // Lease4 object.
                     boost::multi_index::member<Lease4, std::vector<uint8_t>,

+ 9 - 0
src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -1622,6 +1622,15 @@ MySqlLeaseMgr::getLease4(const ClientId& clientid) const {
     return (result);
 }
 
+Lease4Ptr
+MySqlLeaseMgr::getLease4(const ClientId&, const HWAddr&, SubnetID) const {
+    /// This function is currently not implemented because allocation engine
+    /// searches for the lease using HW address or client identifier.
+    /// It never uses both parameters in the same time. We need to
+    /// consider if this function is needed at all.
+    isc_throw(NotImplemented, "The MySqlLeaseMgr::getLease4 function was"
+              " called, but it is not implemented");
+}
 
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const ClientId& clientid, SubnetID subnet_id) const {

+ 14 - 1
src/lib/dhcpsrv/mysql_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
@@ -220,6 +220,19 @@ public:
     ///        failed.
     virtual Lease4Collection getLease4(const ClientId& clientid) const;
 
+    /// @brief Returns IPv4 lease for the specified client identifier, HW
+    /// address and subnet identifier.
+    ///
+    /// @param client_id A client identifier.
+    /// @param hwaddr Hardware address.
+    /// @param subnet_id A subnet identifier.
+    ///
+    /// @return A pointer to the lease or NULL if the lease is not found.
+    /// @throw isc::NotImplemented On every call as this function is currently
+    /// not implemented for the MySQL backend.
+    virtual Lease4Ptr getLease4(const ClientId& client_id, const HWAddr& hwaddr,
+                                SubnetID subnet_id) const;
+
     /// @brief Returns existing IPv4 lease for specified client-id
     ///
     /// There can be at most one lease for a given HW address in a single

+ 1 - 0
src/lib/dhcpsrv/tests/Makefile.am

@@ -53,6 +53,7 @@ libdhcpsrv_unittests_SOURCES += alloc_engine_unittest.cc
 libdhcpsrv_unittests_SOURCES += callout_handle_store_unittest.cc
 libdhcpsrv_unittests_SOURCES += cfgmgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += dbaccess_parser_unittest.cc
+libdhcpsrv_unittests_SOURCES += lease_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_mgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc

+ 13 - 0
src/lib/dhcpsrv/tests/lease_mgr_unittest.cc

@@ -113,6 +113,19 @@ public:
         return (Lease4Collection());
     }
 
+    /// @brief Returns existing IPv4 lease for specified client identifier,
+    /// HW address and subnet identifier.
+    ///
+    /// @param client_id Aclient identifier
+    /// @param hwaddr A HW address.
+    /// @param subnet_id A subnet identifier.
+    ///
+    /// @return A pointer to an existing lease or NULL if lease not found.
+    virtual Lease4Ptr
+    getLease4(const ClientId&, const HWAddr&, SubnetID) const {
+        return (Lease4Ptr());
+    }
+
     /// @brief Returns existing IPv4 lease for specified client-id
     ///
     /// There can be at most one lease for a given HW address in a single

+ 71 - 0
src/lib/dhcpsrv/tests/lease_unittest.cc

@@ -0,0 +1,71 @@
+// Copyright (C) 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <dhcp/duid.h>
+#include <dhcpsrv/lease.h>
+#include <gtest/gtest.h>
+#include <vector>
+
+using namespace isc;
+using namespace isc::dhcp;
+
+namespace {
+
+// @todo Currently this file contains tests for new functions which return DUID
+// or client identifier. Other tests for Lease objects must be implemented.
+// See http://bind10.isc.org/ticket/3240.
+
+// Verify that the client id can be returned as a vector object and if client
+// id is NULL the empty vector is returned.
+TEST(Lease4Test, getClientIdVector) {
+    // Create a lease.
+    Lease4 lease;
+    // By default, the lease should have client id set to NULL. If it doesn't,
+    // continuing the test makes no sense.
+    ASSERT_FALSE(lease.client_id_);
+    // When client id is NULL the vector returned should be empty.
+    EXPECT_TRUE(lease.getClientIdVector().empty());
+    // Now, let's set the non NULL client id. Fill it with the 8 bytes, each
+    // holding a value of 0x42.
+    std::vector<uint8_t> client_id_vec(8, 0x42);
+    lease.client_id_ = ClientIdPtr(new ClientId(client_id_vec));
+    // Check that the returned vector, encapsulating client id is equal to
+    // the one that has been used to set the client id for the lease.
+    std::vector<uint8_t> returned_vec = lease.getClientIdVector();
+    EXPECT_TRUE(returned_vec == client_id_vec);
+}
+
+// Verify that the DUID can be returned as a vector object and if DUID is NULL
+// the empty vector is returned.
+TEST(Lease6Test, getDuidVector) {
+    // Create a lease.
+    Lease6 lease;
+    // By default, the lease should have client id set to NULL. If it doesn't,
+    // continuing the test makes no sense.
+    ASSERT_FALSE(lease.duid_);
+    // When client id is NULL the vector returned should be empty.
+    EXPECT_TRUE(lease.getDuidVector().empty());
+    // Now, let's set the non NULL DUID. Fill it with the 8 bytes, each
+    // holding a value of 0x42.
+    std::vector<uint8_t> duid_vec(8, 0x42);
+    lease.duid_ = DuidPtr(new DUID(duid_vec));
+    // Check that the returned vector, encapsulating DUID is equal to
+    // the one that has been used to set the DUID for the lease.
+    std::vector<uint8_t> returned_vec = lease.getDuidVector();
+    EXPECT_TRUE(returned_vec == duid_vec);
+}
+
+
+}; // end of anonymous namespace

+ 12 - 62
src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -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
@@ -35,7 +35,14 @@ namespace {
 class MemfileLeaseMgrTest : public GenericLeaseMgrTest {
 public:
     MemfileLeaseMgrTest() {
+        const LeaseMgr::ParameterMap pmap;
+        lmptr_ = new Memfile_LeaseMgr(pmap);
     }
+
+    virtual ~MemfileLeaseMgrTest() {
+        delete lmptr_;
+    }
+
 };
 
 // This test checks if the LeaseMgr can be instantiated and that it
@@ -141,79 +148,22 @@ TEST_F(MemfileLeaseMgrTest, addGetDelete6) {
 
 // Simple test about lease4 retrieval through client id method
 TEST_F(MemfileLeaseMgrTest, getLease4ClientId) {
-    const LeaseMgr::ParameterMap pmap;
-    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
-    // Let's initialize a specific lease ...
-    Lease4Ptr lease = initializeLease4(straddress4_[1]);
-    EXPECT_TRUE(lease_mgr->addLease(lease));
-    Lease4Collection returned = lease_mgr->getLease4(*lease->client_id_);
-
-    ASSERT_EQ(1, returned.size());
-    // We should retrieve our lease...
-    detailCompareLease(lease, *returned.begin());
-    lease = initializeLease4(straddress4_[2]);
-    returned = lease_mgr->getLease4(*lease->client_id_);
-
-    ASSERT_EQ(0, returned.size());
+    testGetLease4ClientId();
 }
 
 // Checks that lease4 retrieval client id is null is working
 TEST_F(MemfileLeaseMgrTest, getLease4NullClientId) {
-    const LeaseMgr::ParameterMap pmap;
-    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
-    // Let's initialize a specific lease ... But this time
-    // We keep its client id for further lookup and
-    // We clearly 'reset' it ...
-    Lease4Ptr lease = initializeLease4(straddress4_[4]);
-    ClientIdPtr client_id = lease->client_id_;
-    lease->client_id_ = ClientIdPtr();
-    EXPECT_TRUE(lease_mgr->addLease(lease));
-
-    Lease4Collection returned = lease_mgr->getLease4(*client_id);
-    // Shouldn't have our previous lease ...
-    ASSERT_EQ(0, returned.size());
+    testGetLease4NullClientId();
 }
 
 // Checks lease4 retrieval through HWAddr
 TEST_F(MemfileLeaseMgrTest, getLease4HWAddr) {
-    const LeaseMgr::ParameterMap pmap;
-    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
-    // Let's initialize two different leases 4 and just add the first ...
-    Lease4Ptr leaseA = initializeLease4(straddress4_[5]);
-    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
-    HWAddr hwaddrB(vector<uint8_t>(6, 0x80), HTYPE_ETHER);
-
-    EXPECT_TRUE(lease_mgr->addLease(leaseA));
-
-    // we should not have a lease, with this MAC Addr
-    Lease4Collection returned = lease_mgr->getLease4(hwaddrB);
-    ASSERT_EQ(0, returned.size());
-
-    // But with this one
-    returned = lease_mgr->getLease4(hwaddrA);
-    ASSERT_EQ(1, returned.size());
+    testGetLease4HWAddr();
 }
 
 // Checks lease4 retrieval with clientId, HWAddr and subnet_id
 TEST_F(MemfileLeaseMgrTest, getLease4ClientIdHWAddrSubnetId) {
-    const LeaseMgr::ParameterMap pmap;
-    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
-
-    Lease4Ptr leaseA = initializeLease4(straddress4_[4]);
-    Lease4Ptr leaseB = initializeLease4(straddress4_[5]);
-    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
-    HWAddr hwaddrB(leaseB->hwaddr_, HTYPE_ETHER);
-    EXPECT_TRUE(lease_mgr->addLease(leaseA));
-    // First case we should retrieve our lease
-    Lease4Ptr lease = lease_mgr->getLease4(*leaseA->client_id_, hwaddrA, leaseA->subnet_id_);
-    detailCompareLease(lease, leaseA);
-    lease = lease_mgr->getLease4(*leaseB->client_id_, hwaddrA, leaseA->subnet_id_);
-    detailCompareLease(lease, leaseA);
-    // But not the folowing, with different  hwaddr and subnet
-    lease = lease_mgr->getLease4(*leaseA->client_id_, hwaddrB, leaseA->subnet_id_);
-    EXPECT_TRUE(lease == Lease4Ptr());
-    lease = lease_mgr->getLease4(*leaseA->client_id_, hwaddrA, leaseB->subnet_id_);
-    EXPECT_TRUE(lease == Lease4Ptr());
+    testGetLease4ClientIdHWAddrSubnetId();
 }
 
 }; // end of anonymous namespace

+ 113 - 4
src/lib/dhcpsrv/tests/test_utils.cc

@@ -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
@@ -98,9 +98,8 @@ detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) {
     EXPECT_EQ(first->hostname_, second->hostname_);
 }
 
-
 GenericLeaseMgrTest::GenericLeaseMgrTest()
-    :lmptr_(NULL) {
+    : lmptr_(NULL) {
     // Initialize address strings and IOAddresses
     for (int i = 0; ADDRESS4[i] != NULL; ++i) {
         string addr(ADDRESS4[i]);
@@ -121,6 +120,11 @@ GenericLeaseMgrTest::GenericLeaseMgrTest()
     }
 }
 
+GenericLeaseMgrTest::~GenericLeaseMgrTest() {
+    // Does nothing. The derived classes are expected to clean up, i.e.
+    // remove the lmptr_ pointer.
+}
+
 /// @brief Initialize Lease4 Fields
 ///
 /// Returns a pointer to a Lease4 structure.  Different values are put into
@@ -217,7 +221,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->fqdn_rev_ = false;
         lease->fqdn_fwd_ = false;
         lease->hostname_ = "otherhost.example.com.";
-        } else if (address == straddress4_[6]) {
+    } else if (address == straddress4_[6]) {
         lease->hwaddr_ = vector<uint8_t>(6, 0x6e);
         // Same ClientId as straddress4_1
         lease->client_id_ = ClientIdPtr(
@@ -469,6 +473,111 @@ GenericLeaseMgrTest::createLeases6() {
     return (leases);
 }
 
+void
+GenericLeaseMgrTest::testGetLease4ClientId() {
+    // Let's initialize a specific lease ...
+    Lease4Ptr lease = initializeLease4(straddress4_[1]);
+    EXPECT_TRUE(lmptr_->addLease(lease));
+    Lease4Collection returned = lmptr_->getLease4(*lease->client_id_);
+
+    ASSERT_EQ(1, returned.size());
+    // We should retrieve our lease...
+    detailCompareLease(lease, *returned.begin());
+    lease = initializeLease4(straddress4_[2]);
+    returned = lmptr_->getLease4(*lease->client_id_);
+
+    ASSERT_EQ(0, returned.size());
+}
+
+void
+GenericLeaseMgrTest::testGetLease4NullClientId() {
+    // Let's initialize a specific lease ... But this time
+    // We keep its client id for further lookup and
+    // We clearly 'reset' it ...
+    Lease4Ptr leaseA = initializeLease4(straddress4_[4]);
+    ClientIdPtr client_id = leaseA->client_id_;
+    leaseA->client_id_ = ClientIdPtr();
+    ASSERT_TRUE(lmptr_->addLease(leaseA));
+
+    Lease4Collection returned = lmptr_->getLease4(*client_id);
+    // Shouldn't have our previous lease ...
+    ASSERT_TRUE(returned.empty());
+
+    // Add another lease with the non-NULL client id, and make sure that the
+    // lookup will not break due to existence of both leases with non-NULL and
+    // NULL client ids.
+    Lease4Ptr leaseB = initializeLease4(straddress4_[0]);
+    // Shouldn't throw any null pointer exception
+    ASSERT_TRUE(lmptr_->addLease(leaseB));
+    // Try to get the lease.
+    returned = lmptr_->getLease4(*client_id);
+    ASSERT_TRUE(returned.empty());
+
+    // Let's make it more interesting and add another lease with NULL client id.
+    Lease4Ptr leaseC = initializeLease4(straddress4_[5]);
+    leaseC->client_id_.reset();
+    ASSERT_TRUE(lmptr_->addLease(leaseC));
+    returned = lmptr_->getLease4(*client_id);
+    ASSERT_TRUE(returned.empty());
+
+    // But getting the lease with non-NULL client id should be successful.
+    returned = lmptr_->getLease4(*leaseB->client_id_);
+    ASSERT_EQ(1, returned.size());
+}
+
+void
+GenericLeaseMgrTest::testGetLease4HWAddr() {
+    // Let's initialize two different leases 4 and just add the first ...
+    Lease4Ptr leaseA = initializeLease4(straddress4_[5]);
+    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddrB(vector<uint8_t>(6, 0x80), HTYPE_ETHER);
+
+    EXPECT_TRUE(lmptr_->addLease(leaseA));
+
+    // we should not have a lease, with this MAC Addr
+    Lease4Collection returned = lmptr_->getLease4(hwaddrB);
+    ASSERT_EQ(0, returned.size());
+
+    // But with this one
+    returned = lmptr_->getLease4(hwaddrA);
+    ASSERT_EQ(1, returned.size());
+}
+
+void
+GenericLeaseMgrTest::testGetLease4ClientIdHWAddrSubnetId() {
+    Lease4Ptr leaseA = initializeLease4(straddress4_[4]);
+    Lease4Ptr leaseB = initializeLease4(straddress4_[5]);
+    Lease4Ptr leaseC = initializeLease4(straddress4_[6]);
+    // Set NULL client id for one of the leases. This is to make sure that such
+    // a lease may coexist with other leases with non NULL client id.
+    leaseC->client_id_.reset();
+
+    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddrB(leaseB->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddrC(leaseC->hwaddr_, HTYPE_ETHER);
+    EXPECT_TRUE(lmptr_->addLease(leaseA));
+    EXPECT_TRUE(lmptr_->addLease(leaseB));
+    EXPECT_TRUE(lmptr_->addLease(leaseC));
+    // First case we should retrieve our lease
+    Lease4Ptr lease = lmptr_->getLease4(*leaseA->client_id_, hwaddrA, leaseA->subnet_id_);
+    detailCompareLease(lease, leaseA);
+    // Retrieve the other lease.
+    lease = lmptr_->getLease4(*leaseB->client_id_, hwaddrB, leaseB->subnet_id_);
+    detailCompareLease(lease, leaseB);
+    // The last lease has NULL client id so we will use a different getLease4 function
+    // which doesn't require client id (just a hwaddr and subnet id).
+    lease = lmptr_->getLease4(hwaddrC, leaseC->subnet_id_);
+    detailCompareLease(lease, leaseC);
+
+    // An attempt to retrieve the lease with non matching lease parameters should
+    // result in NULL pointer being returned.
+    lease = lmptr_->getLease4(*leaseA->client_id_, hwaddrB, leaseA->subnet_id_);
+    EXPECT_FALSE(lease);
+    lease = lmptr_->getLease4(*leaseA->client_id_, hwaddrA, leaseB->subnet_id_);
+    EXPECT_FALSE(lease);
+}
+
+
 };
 };
 };

+ 20 - 3
src/lib/dhcpsrv/tests/test_utils.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
@@ -49,8 +49,13 @@ detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second);
 /// All concrete LeaseMgr test classes should be derived from it.
 class GenericLeaseMgrTest : public ::testing::Test {
 public:
+
+    /// @brief Default constructor.
     GenericLeaseMgrTest();
 
+    /// @brief Virtual destructor.
+    virtual ~GenericLeaseMgrTest();
+
     /// @brief Initialize Lease4 Fields
     ///
     /// Returns a pointer to a Lease4 structure.  Different values are put into
@@ -85,7 +90,7 @@ public:
     ///
     /// @param leases Vector of pointers to leases
     template <typename T>
-        void checkLeasesDifferent(const std::vector<T>& leases) const;
+    void checkLeasesDifferent(const std::vector<T>& leases) const;
 
     /// @brief Creates leases for the test
     ///
@@ -101,6 +106,18 @@ public:
     /// @return vector<Lease6Ptr> Vector of pointers to leases
     std::vector<Lease6Ptr> createLeases6();
 
+    /// @brief Test lease retrieval using client id.
+    void testGetLease4ClientId();
+
+    /// @brief Test lease retrieval when leases with NULL client id are present.
+    void testGetLease4NullClientId();
+
+    /// @brief Test lease retrieval using HW address.
+    void testGetLease4HWAddr();
+
+    /// @brief Test lease retrieval using client id, HW address and subnet id.
+    void testGetLease4ClientIdHWAddrSubnetId();
+
     // Member variables
     std::vector<std::string>  straddress4_;   ///< String forms of IPv4 addresses
     std::vector<isc::asiolink::IOAddress> ioaddress4_;  ///< IOAddress forms of IPv4 addresses
@@ -108,7 +125,7 @@ public:
     std::vector<Lease::Type> leasetype6_; ///< Lease types
     std::vector<isc::asiolink::IOAddress> ioaddress6_;  ///< IOAddress forms of IPv6 addresses
 
-    LeaseMgr*   lmptr_;             ///< Pointer to the lease manager
+    LeaseMgr* lmptr_;                     ///< Pointer to the lease manager
 };
 
 };