[2404] Now able to get IPv4 leases by hardware address

src/lib/dhcpsrv/lease_mgr.h

@@ -208,7 +208,7 @@ struct Lease4 {
 typedef boost::shared_ptr<Lease4> Lease4Ptr;
 
 /// @brief A collection of IPv4 leases.
-typedef std::vector< boost::shared_ptr<Lease4Ptr> > Lease4Collection;
+typedef std::vector<Lease4Ptr> Lease4Collection;
 
 /// @brief Structure that holds a lease for IPv6 address and/or prefix
 ///

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -61,6 +61,11 @@ TaggedStatement tagged_statements[] = {
                     "DELETE FROM lease4 WHERE address = ?"},
     {MySqlLeaseMgr::DELETE_LEASE6,
                     "DELETE FROM lease6 WHERE address = ?"},
+    {MySqlLeaseMgr::GET_LEASE4_HWADDR,
+                    "SELECT address, hwaddr, client_id, "
+                        "valid_lifetime, expire, subnet_id "
+                            "FROM lease4 "
+                            "WHERE hwaddr = ?"},
     {MySqlLeaseMgr::GET_LEASE4_ADDR,
                     "SELECT address, hwaddr, client_id, "
                         "valid_lifetime, expire, subnet_id "
@@ -985,6 +990,55 @@ void MySqlLeaseMgr::getLease(StatementIndex stindex, MYSQL_BIND* inbind,
     }
 }
 
+// Extraction of leases from the database.  Much the code has common logic
+// with the difference between V4 and V6 being the data types of the
+// objects involved.  For this reason, the common logic is inside a
+// template method.
+
+template <typename Exchange, typename LeaseCollection>
+void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
+                                       MYSQL_BIND* inbind,
+                                       Exchange& exchange,
+                                       LeaseCollection& result) const {
+
+    // Bind the input parameters to the statement and bind the output
+    // to fields in the exchange object, then execute the prepared statement.
+    bindAndExecute(stindex, exchange, inbind);
+
+    // Ensure that all the lease information is retrieved in one go to avoid
+    // overhead of going back and forth between client and server.
+    int status = mysql_stmt_store_result(statements_[stindex]);
+    checkError(status, stindex, "unable to set up for storing all results");
+
+    // Initialize for returning the data
+    result.clear();
+
+    // Set up the fetch "release" object to release resources associated
+    // with the call to mysql_stmt_fetch when this method exits, then
+    // retrieve the data.
+    MySqlFreeResult fetch_release(statements_[stindex]);
+    while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) {
+        try {
+            result.push_back(exchange->getLeaseData());
+
+        } catch (const isc::BadValue& ex) {
+            // Rethrow the exception with a bit more data.
+            isc_throw(BadValue, ex.what() << ". Statement is <" <<
+                      text_statements_[stindex] << ">");
+        }
+    }
+
+    // How did the fetch end?
+    if (status == 1) {
+        // Error - unable to fecth results
+        checkError(status, stindex, "unable to fetch results");
+    } else if (status == MYSQL_DATA_TRUNCATED) {
+        // @TODO Handle truncation
+        ;
+    }
+}
+
+
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const {
     // Set up the WHERE clause value
@@ -996,6 +1050,7 @@ MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const {
     inbind[0].buffer = reinterpret_cast<char*>(&addr4);
     inbind[0].is_unsigned = static_cast<my_bool>(1);
 
+    // Get the data
     Lease4Ptr result;
     getLease(GET_LEASE4_ADDR, inbind, exchange4_, result);
 
@@ -1022,10 +1077,29 @@ MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr,
 
 
 Lease4Collection
-MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */) const {
-    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const HWAddr&) "
-              "not implemented yet");
-    return (Lease4Collection());
+MySqlLeaseMgr::getLease4(const HWAddr& hwaddr) const {
+    // Set up the WHERE clause value
+    MYSQL_BIND inbind[1];
+    memset(inbind, 0, sizeof(inbind));
+
+    // As "buffer" is "char*" - even though the data is being read - we need
+    // to cast away the "const"ness as well as reinterpreting the data as
+    // a "char*". (We could avoid the "const_cast" by copying the data to a
+    // local variable, but as the data is only being read, this introduces
+    // an unnecessary copy).
+    unsigned long hwaddr_length = hwaddr.size();
+    uint8_t* data = const_cast<uint8_t*>(&hwaddr[0]);
+
+    inbind[0].buffer_type = MYSQL_TYPE_BLOB;
+    inbind[0].buffer = reinterpret_cast<char*>(data);
+    inbind[0].buffer_length = hwaddr_length;
+    inbind[0].length = &hwaddr_length;
+
+    // Get the data
+    Lease4Collection result;
+    getLeaseCollection(GET_LEASE4_HWADDR, inbind, exchange4_, result);
+
+    return (result);
 }
 
 
@@ -1082,7 +1156,6 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
 
 Lease6Collection
 MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
-    const StatementIndex stindex = GET_LEASE6_DUID_IAID;
 
     // Set up the WHERE clause value
     MYSQL_BIND inbind[2];
@@ -1113,43 +1186,9 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
     inbind[1].buffer = reinterpret_cast<char*>(&iaid);
     inbind[1].is_unsigned = static_cast<my_bool>(1);
 
-    // Bind the input parameters to the statement and bind the output
-    // to fields in the exchange object, then execute the prepared statement.
-    bindAndExecute(stindex, exchange6_, inbind);
-
-    // Ensure that all the lease information is retrieved in one go to avoid
-    // overhead of going back and forth between client and server.
-    int status = mysql_stmt_store_result(statements_[stindex]);
-    checkError(status, stindex, "unable to set up for storing all results");
-
-    // Fetch the data.  There could be multiple rows, so we need to iterate
-    // until all data has been retrieved.
+    // ... and get the data
     Lease6Collection result;
-
-    // Set up the fetch "release" object to release resources associated
-    // with the call to mysql_stmt_fetch when this method exits, then
-    // retrieve the data.
-    MySqlFreeResult fetch_release(statements_[stindex]);
-    while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) {
-        try {
-            Lease6Ptr lease = exchange6_->getLeaseData();
-            result.push_back(lease);
-
-        } catch (const isc::BadValue& ex) {
-            // Rethrow the exception with a bit more data.
-            isc_throw(BadValue, ex.what() << ". Statement is <" <<
-                      text_statements_[stindex] << ">");
-        }
-    }
-
-    // How did the fetch end?
-    if (status == 1) {
-        // Error - unable to fecth results
-        checkError(status, stindex, "unable to fetch results");
-    } else if (status == MYSQL_DATA_TRUNCATED) {
-        // @TODO Handle truncation
-        ;
-    }
+    getLeaseCollection(GET_LEASE6_DUID_IAID, inbind, exchange6_, result);
 
     return (result);
 }
@@ -1158,7 +1197,7 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
 Lease6Ptr
 MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
                          SubnetID subnet_id) const {
-    const StatementIndex stindex = GET_LEASE6_DUID_IAID_SUBID;
+//    const StatementIndex stindex = GET_LEASE6_DUID_IAID_SUBID;
 
     // Set up the WHERE clause value
     MYSQL_BIND inbind[3];
@@ -1184,42 +1223,8 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
     inbind[2].buffer = reinterpret_cast<char*>(&subnet_id);
     inbind[2].is_unsigned = static_cast<my_bool>(1);
 
-    // Bind the input parameters to the statement and bind the output
-    // to fields in the exchange object, then execute the prepared statement.
-    bindAndExecute(stindex, exchange6_, inbind);
-
-    // Fetch the data and set up the "release" object to release associated
-    // resources when this method exits then retrieve the data.
     Lease6Ptr result;
-    MySqlFreeResult fetch_release(statements_[stindex]);
-    int status = mysql_stmt_fetch(statements_[stindex]);
-    if (status == 0) {
-        try {
-            result = exchange6_->getLeaseData();
-
-            // TODO: check for more than one row returned.  At present, just
-            // ignore the excess and take the first.
-
-        } catch (const isc::BadValue& ex) {
-            // Lease type is returned, to rethrow the exception with a bit
-            // more data.
-            isc_throw(BadValue, ex.what() << ". Statement is <" <<
-                      text_statements_[stindex] << ">");
-        }
-
-        // As the address is the primary key in the table, we can't return
-        // two rows, so we don't bother checking whether multiple rows have
-        // been returned.
-
-    } else if (status == 1) {
-        checkError(status, stindex, "unable to fetch results");
-
-    } else {
-        // @TODO Handle truncation
-        // We are ignoring truncation for now, so the only other result is
-        // no data was found.  In that case, we return a null Lease6 structure.
-        // This has already been set, so the action is a no-op.
-    }
+    getLease(GET_LEASE6_DUID_IAID_SUBID, inbind, exchange6_, result);
 
     return (result);
 }

src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -355,6 +355,7 @@ public:
         DELETE_LEASE4,              // Delete from lease4 by address
         DELETE_LEASE6,              // Delete from lease6 by address
         GET_LEASE4_ADDR,            // Get lease4 by address
+        GET_LEASE4_HWADDR,          // Get lease4 by hardward address
         GET_LEASE6_ADDR,            // Get lease6 by address
         GET_LEASE6_DUID_IAID,       // Get lease6 by DUID and IAID
         GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and Subnet ID
@@ -419,7 +420,8 @@ private:
 
     /// @brief Get Lease Common Code
     ///
-    /// This method performs the common actions for the getLease methods.
+    /// This method performs the common actions for obtaining a single lease
+    /// from the database.
     ///
     /// @param stindex Index of statement being executed
     /// @param inbind MYSQL_BIND array for input parameters
@@ -429,6 +431,20 @@ private:
     void getLease(StatementIndex stindex, MYSQL_BIND* inbind,
                   Exchange& exchange, LeasePtr& result) const;
 
+    /// @brief Get Lease Collection Common Code
+    ///
+    /// This method performs the common actions for obtaining multiple leases
+    /// from the database.
+    ///
+    /// @param stindex Index of statement being executed
+    /// @param inbind MYSQL_BIND array for input parameters
+    /// @param exchange Exchange object to use
+    /// @param lease LeaseCollection object returned.  Note that any data in
+    ///        the collection is cleared before new data is added.
+    template <typename Exchange, typename LeaseCollection>
+    void getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind,
+                            Exchange& exchange, LeaseCollection& result) const;
+
     /// @brief Binds Parameters and Executes
     ///
     /// This method abstracts a lot of common processing from the getXxxx()

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -276,7 +276,7 @@ public:
             lease->subnet_id_ = 73;        // Same as for straddress4_1
 
         } else if (address == straddress4_[3]) {
-            lease->hwaddr_ = vector<uint8_t>(6, 0x3b);
+            lease->hwaddr_ = vector<uint8_t>(6, 0x19);  // Same as lease 1
             vector<uint8_t> clientid;
             for (uint8_t i = 31; i < 126; ++i) {
                 clientid.push_back(i);
@@ -302,7 +302,7 @@ public:
             lease->subnet_id_ = 75;        // Arbitrary number
 
         } else if (address == straddress4_[5]) {
-            lease->hwaddr_ = vector<uint8_t>(6, 0x5d);
+            lease->hwaddr_ = vector<uint8_t>(6, 0x19);  // Same as lease 1
             // Same ClientId and IAID as straddress4_1
             lease->client_id_ = boost::shared_ptr<ClientId>(
                 new ClientId(vector<uint8_t>(8, 0x42)));
@@ -320,9 +320,9 @@ public:
             lease->subnet_id_ = 112;       // Arbitrary number
 
         } else if (address == straddress4_[7]) {
-            lease->hwaddr_ = vector<uint8_t>(6, 0x7f);
+            lease->hwaddr_ = vector<uint8_t>();     // Deliberately empty
             lease->client_id_ = boost::shared_ptr<ClientId>(
-                new ClientId(vector<uint8_t>(8, 0xe5)));
+                new ClientId(vector<uint8_t>()));   // Deliberately empty
             lease->valid_lft_ = 7975;      // Actual lifetime
             lease->cltt_ = 213876;         // Current time of day
             lease->subnet_id_ = 19;        // Arbitrary number
@@ -838,43 +838,60 @@ TEST_F(MySqlLeaseMgrTest, getLease4AddressSubnetId) {
     EXPECT_FALSE(l_returned);
 }
 
+
+
 // @brief Check GetLease4 methods - Access by Hardware Address
 //
 // Adds leases to the database and checks that they can be accessed via
-// hardware address
-TEST_F(MySqlLeaseMgrTest, getLease4AddressHwaddr) {
-    FAIL() << "Test not complete";
-/*
-    // Get the leases to be used for the test and add to the database.
+// a combination of DIUID and IAID.
+TEST_F(MySqlLeaseMgrTest, getLease4Hwaddr) {
+    // Get the leases to be used for the test and add to the database
     vector<Lease4Ptr> leases = createLeases4();
     for (int i = 0; i < leases.size(); ++i) {
         EXPECT_TRUE(lmptr_->addLease(leases[i]));
     }
 
-    // Get a known hardware address
-    vector<uint8_t> hwaddr = leases[1]->hwaddr_;
-    EXPECT_FALSE(hwaddr.empty());
+    // Get the leases matching the hardware address of lease 1
+    Lease4Collection returned = lmptr_->getLease4(leases[1]->hwaddr_);
 
-    // Look for a lease with a valid hardware address
-    Lease4Ptr l_returned = lmptr_->getLease4(hwaddr);
-    ASSERT_TRUE(l_returned);
-    detailCompareLease(leases[1], l_returned);
-
-    // Look for a lease with an invalid valid hardware address
-    hwaddr[0] += 1;
-    Lease4Ptr l_returned = lmptr_->getLease4(hwaddr);
-    EXPECT_FALSE(l_returned);
+    // Should be three leases, matching leases[1], [3] and [5].
+    ASSERT_EQ(3, returned.size());
 
-    // Check it handles an empty hardware address
-    hwaddr.clear();
-    Lease4Ptr l_returned = lmptr_->getLease4(hwaddr);
-    EXPECT_FALSE(l_returned);
+    // Easiest way to check is to look at the addresses.
+    vector<string> addresses;
+    for (Lease4Collection::const_iterator i = returned.begin();
+         i != returned.end(); ++i) {
+        addresses.push_back((*i)->addr_.toText());
+    }
+    sort(addresses.begin(), addresses.end());
+    EXPECT_EQ(straddress4_[1], addresses[0]);
+    EXPECT_EQ(straddress4_[3], addresses[1]);
+    EXPECT_EQ(straddress4_[5], addresses[2]);
+
+    // Repeat test with just one expected match
+    returned = lmptr_->getLease4(leases[2]->hwaddr_);
+    EXPECT_EQ(1, returned.size());
+    detailCompareLease(leases[2], *returned.begin());
+
+    // Check that an empty vector is valid
+    EXPECT_TRUE(leases[7]->hwaddr_.empty());
+    returned = lmptr_->getLease4(leases[7]->hwaddr_);
+    EXPECT_EQ(1, returned.size());
+    detailCompareLease(leases[7], *returned.begin());
+
+    // Try to get something with invalid hardware address
+    vector<uint8_t> invalid(6, 0);
+    returned = lmptr_->getLease4(invalid);
+    EXPECT_EQ(0, returned.size());
 
-    // Add a lease with an empty hardware address to the database and
-    // check that it find that.
-*/
+    // And check that size of the vector matters
+    invalid = leases[4]->hwaddr_;
+    invalid.push_back(0);
+    returned = lmptr_->getLease4(invalid);
+    EXPECT_EQ(0, returned.size());
 }
 
+
 // @brief Check GetLease6 methods - Access by DUID/IAID
 //
 // Adds leases to the database and checks that they can be accessed via