Parcourir la source

[3147] Addressed review comments.

Minor changes to logging messages, unit tests, and commentary.
Thomas Markwalder il y a 11 ans
Parent
commit
923a40c230

+ 2 - 2
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -289,12 +289,12 @@ A debug message issued when the server is attempting to obtain a set
 of IPv4 leases from the MySQL database for a client with the specified
 of IPv4 leases from the MySQL database for a client with the specified
 hardware address.
 hardware address.
 
 
-% DHCPSRV_MYSQL_GET_IAID_DUID obtaining IPv6 leases for IAID %1, DUID %2, and lease type %3
+% DHCPSRV_MYSQL_GET_IAID_DUID obtaining IPv6 leases for IAID %1, DUID %2, lease type %3
 A debug message issued when the server is attempting to obtain a set of
 A debug message issued when the server is attempting to obtain a set of
 IPv6 lease from the MySQL database for a client with the specified IAID
 IPv6 lease from the MySQL database for a client with the specified IAID
 (Identity Association ID) and DUID (DHCP Unique Identifier).
 (Identity Association ID) and DUID (DHCP Unique Identifier).
 
 
-% DHCPSRV_MYSQL_GET_IAID_SUBID_DUID obtaining IPv6 leases for IAID %1, Subnet ID %2, DUID %3, and lease type %4
+% DHCPSRV_MYSQL_GET_IAID_SUBID_DUID obtaining IPv6 leases for IAID %1, Subnet ID %2, DUID %3, lease type %4
 A debug message issued when the server is attempting to obtain an IPv6
 A debug message issued when the server is attempting to obtain an IPv6
 lease from the MySQL database for a client with the specified IAID
 lease from the MySQL database for a client with the specified IAID
 (Identity Association ID), Subnet ID and DUID (DHCP Unique Identifier).
 (Identity Association ID), Subnet ID and DUID (DHCP Unique Identifier).

+ 2 - 2
src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -163,7 +163,7 @@ TaggedStatement tagged_statements[] = {
                         "lease_type, iaid, prefix_len, "
                         "lease_type, iaid, prefix_len, "
                         "fqdn_fwd, fqdn_rev, hostname "
                         "fqdn_fwd, fqdn_rev, hostname "
                             "FROM lease6 "
                             "FROM lease6 "
-                            "WHERE address = ? and lease_type = ?"},
+                            "WHERE address = ? AND lease_type = ?"},
     {MySqlLeaseMgr::GET_LEASE6_DUID_IAID,
     {MySqlLeaseMgr::GET_LEASE6_DUID_IAID,
                     "SELECT address, duid, valid_lifetime, "
                     "SELECT address, duid, valid_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
@@ -1254,7 +1254,7 @@ MySqlLeaseMgr::openDatabase() {
                   mysql_error(mysql_));
                   mysql_error(mysql_));
     }
     }
 
 
-    // Set SQL mode options for the connection:  SQL mode governs how what 
+    // Set SQL mode options for the connection:  SQL mode governs how what
     // constitutes insertable data for a given column, and how to handle
     // constitutes insertable data for a given column, and how to handle
     // invalid data.  We want to ensure we get the strictest behavior and
     // invalid data.  We want to ensure we get the strictest behavior and
     // to reject invalid data with an error.
     // to reject invalid data with an error.

+ 38 - 33
src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -927,68 +927,73 @@ TEST_F(MySqlLeaseMgrTest, lease6LeaseTypeCheck) {
     empty_lease->hostname_ = "myhost.example.com.";
     empty_lease->hostname_ = "myhost.example.com.";
     empty_lease->prefixlen_ = 4;
     empty_lease->prefixlen_ = 4;
 
 
-    // Make Two leases per lease type, all with the same  duid, iad but
+    // Make Two leases per lease type, all with the same  DUID, IAID but
     // alternate the subnet_ids.
     // alternate the subnet_ids.
     vector<Lease6Ptr> leases;
     vector<Lease6Ptr> leases;
-    int tick = 0;
-    for (int i = 0; i < 3; i++) {
-        for (int j = 0; j < 2; j++) {
-            Lease6Ptr lease(new Lease6(*empty_lease));
-            lease->type_ = leasetype6_[i];
-            lease->addr_ = IOAddress(straddress6_[tick++]);
-            lease->subnet_id_ += j;
-            std::cout << "ok, subnet is: " << lease->subnet_id_
-                << " tick is:" << tick << std::endl;
-            leases.push_back(lease);
-            EXPECT_TRUE(lmptr_->addLease(lease));
-        }
-    }
+    for (int i = 0; i < 6; ++i) {
+          Lease6Ptr lease(new Lease6(*empty_lease));
+          lease->type_ = leasetype6_[i / 2];
+          lease->addr_ = IOAddress(straddress6_[i]);
+          lease->subnet_id_ += (i % 2);
+          leases.push_back(lease);
+          EXPECT_TRUE(lmptr_->addLease(lease));
+     }
 
 
     // Verify getting a single lease by type and address.
     // Verify getting a single lease by type and address.
-    for (int i = 0; i < 3; i++) {
+    for (int i = 0; i < 6; ++i) {
         // Look for exact match for each lease type.
         // Look for exact match for each lease type.
-        Lease6Ptr returned = lmptr_->getLease6(leasetype6_[i],
-                                                   leases[i*2]->addr_);
+        Lease6Ptr returned = lmptr_->getLease6(leasetype6_[i / 2],
+                                               leases[i]->addr_);
         // We should match one per lease type.
         // We should match one per lease type.
         ASSERT_TRUE(returned);
         ASSERT_TRUE(returned);
-        EXPECT_TRUE(*returned == *leases[i*2]);
+        EXPECT_TRUE(*returned == *leases[i]);
 
 
         // Same address but wrong lease type, should not match.
         // Same address but wrong lease type, should not match.
-        returned = lmptr_->getLease6(leasetype6_[i+1], leases[i*2]->addr_);
+        returned = lmptr_->getLease6(leasetype6_[i / 2 + 1], leases[i]->addr_);
         ASSERT_FALSE(returned);
         ASSERT_FALSE(returned);
     }
     }
 
 
-    // Verify getting a collection of leases by type, duid, and iad.
+    // Verify getting a collection of leases by type, DUID, and IAID.
     // Iterate over the lease types, asking for leases based on
     // Iterate over the lease types, asking for leases based on
-    // lease type, duid, and iad.
-    tick = 0;
-    for (int i = 0; i < 3; i++) {
+    // lease type, DUID, and IAID.
+    for (int i = 0; i < 3; ++i) {
         Lease6Collection returned = lmptr_->getLeases6(leasetype6_[i],
         Lease6Collection returned = lmptr_->getLeases6(leasetype6_[i],
                                                        *duid, 142);
                                                        *duid, 142);
         // We should match two per lease type.
         // We should match two per lease type.
         ASSERT_EQ(2, returned.size());
         ASSERT_EQ(2, returned.size());
-        EXPECT_TRUE(*(returned[0]) == *leases[tick++]);
-        EXPECT_TRUE(*(returned[1]) == *leases[tick++]);
+
+        // Collection order returned is not guaranteed.
+        // Easiest way to check is to look at the addresses.
+        vector<string> addresses;
+        for (Lease6Collection::const_iterator it = returned.begin();
+            it != returned.end(); ++it) {
+            addresses.push_back((*it)->addr_.toText());
+        }
+        sort(addresses.begin(), addresses.end());
+
+        // Now verify that the lease addresses match.
+        EXPECT_EQ(addresses[0], leases[(i * 2)]->addr_.toText());
+        EXPECT_EQ(addresses[1], leases[(i * 2 + 1)]->addr_.toText());
     }
     }
 
 
-    // Verify getting a collection of leases by type, duid, iad, and subnet id.
+    // Verify getting a collection of leases by type, DUID, IAID, and subnet id.
     // Iterate over the lease types, asking for leases based on
     // Iterate over the lease types, asking for leases based on
-    // lease type, duid, iad, and subnet_id.
-    for (int i = 0; i < 3; i++) {
+    // lease type, DUID, IAID, and subnet_id.
+    for (int i = 0; i < 3; ++i) {
         Lease6Collection returned = lmptr_->getLeases6(leasetype6_[i],
         Lease6Collection returned = lmptr_->getLeases6(leasetype6_[i],
                                                    *duid, 142, 23);
                                                    *duid, 142, 23);
         // We should match one per lease type.
         // We should match one per lease type.
         ASSERT_EQ(1, returned.size());
         ASSERT_EQ(1, returned.size());
-        EXPECT_TRUE(*(returned[0]) == *leases[i*2]);
+        EXPECT_TRUE(*(returned[0]) == *leases[i * 2]);
     }
     }
 
 
     // Verify getting a single lease by type, duid, iad, and subnet id.
     // Verify getting a single lease by type, duid, iad, and subnet id.
-    for (int i = 0; i < 3; i++) {
-        Lease6Ptr returned = lmptr_->getLease6(leasetype6_[i],
-                                                *duid, 142, 23);
+    for (int i = 0; i < 6; ++i) {
+        Lease6Ptr returned = lmptr_->getLease6(leasetype6_[i / 2],
+                                                *duid, 142, (23 + (i % 2)));
         // We should match one per lease type.
         // We should match one per lease type.
         ASSERT_TRUE(returned);
         ASSERT_TRUE(returned);
-        EXPECT_TRUE(*returned == *leases[i*2]);
+        EXPECT_TRUE(*returned == *leases[i]);
     }
     }
 }
 }