[474-optionally-disable-collection-host-lookups] Addressed comments

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index a39f2ee592c40858b5d32bda33eef837826cf066..df3d1b085fbbd5b05a1d6f7cc5b67a4d51716db3 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -544,10 +544,10 @@ void AllocEngine::findReservation(ClientContext6& ctx) {
     // client rather than a query for each subnet within this shared network.
     // The only case when it is going to be less efficient is when there are
     // more host identifier types in use than subnets within a shared network.
-    // As it breaks RADIUS and host caching this can be disabled by the
+    // As it breaks RADIUS use of host caching this can be disabled by the
     // host manager.
     const bool use_single_query = network &&
-        !HostMgr::instance().getPreventCollection() &&
+        !HostMgr::instance().getDisableSingleQuery() &&
         (network->getAllSubnets()->size() > ctx.host_identifiers_.size());
 
     if (use_single_query) {
@@ -3073,10 +3073,10 @@ AllocEngine::findReservation(ClientContext4& ctx) {
     // client rather than a query for each subnet within this shared network.
     // The only case when it is going to be less efficient is when there are
     // more host identifier types in use than subnets within a shared network.
-    // As it breaks RADIUS and host caching this can be disabled by the
+    // As it breaks RADIUS use of host caching this can be disabled by the
     // host manager.
     const bool use_single_query = network &&
-        !HostMgr::instance().getPreventCollection() &&
+        !HostMgr::instance().getDisableSingleQuery() &&
         (network->getAllSubnets()->size() > ctx.host_identifiers_.size());
 
     if (use_single_query) {

diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h
index 501a1f5c072325ef9b9ae2b6532feea0c526de57..caedff49b6eaa2ab69acd5e6301fbc78938fab1d 100644 (file)
--- a/src/lib/dhcpsrv/host_mgr.h
+++ b/src/lib/dhcpsrv/host_mgr.h
@@ -427,17 +427,17 @@ public:
         negative_caching_ = negative_caching;
     }
 
-    /// @brief Returns the prevent collection flag.
+    /// @brief Returns the disable single query flag.
     ///
-    /// @return the prevent collection flag.
-    bool getPreventCollection() const {
-        return (prevent_collection_);
+    /// @return the disable single query flag.
+    bool getDisableSingleQuery() const {
+        return (disable_single_query_);
     }
 
-    /// @brief Sets the prevent collection flag.
+    /// @brief Sets the disable single query flag.
     ///
-    void setPreventCollection(bool prevent_collection) {
-        prevent_collection_ = prevent_collection;
+    void setDisableSingleQuery(bool disable_single_query) {
+        disable_single_query_ = disable_single_query;
     }
 
 protected:
@@ -448,11 +448,12 @@ protected:
     /// This works for get[46] for a subnet and an identifier.
     bool negative_caching_;
 
-    /// @brief The prevent collection flag.
+    /// @brief The disable single query flag.
     ///
-    /// When true prevent the use of lookup methods returning a
-    /// collection when methods returning a single object are usable instead.
-    bool prevent_collection_;
+    /// When true prevent the use of lookup methods returning a collection
+    /// aka single queries when methods returning a host object are usable
+    /// instead.
+    bool disable_single_query_;
 
     /// @brief Cache an answer.
     ///
@@ -475,7 +476,7 @@ protected:
 private:
 
     /// @brief Private default constructor.
-    HostMgr() : negative_caching_(false), prevent_collection_(false) { }
+    HostMgr() : negative_caching_(false), disable_single_query_(false) { }
 
     /// @brief List of alternate host data sources.
     HostDataSourceList alternate_sources_;

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 4cd5212655ef53756d3b75bb9be877a257be5d48..da2d620ad430528637e949f4d38a627aaa746bd7 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -808,6 +808,8 @@ TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkPoolClassification) {
 // reservations belong.
 TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservations) {
 
+    EXPECT_FALSE(HostMgr::instance().getDisableSingleQuery());
+
     // Create reservation for the client.
     HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
                           Host::IDENT_HWADDR, subnet2_->getID(),
@@ -843,11 +845,13 @@ TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservations) {
 // Test that reservations within shared network take precedence over the
 // existing leases regardless in which subnet belonging to a shared network
 // reservations belong. Host lookups returning a collection are disabled.
+// As it is only an optimization the behavior (so the test) must stay
+// unchanged.
 TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservationsNoColl) {
 
     // Disable host lookups returning a collection.
-    ASSERT_FALSE(HostMgr::instance().getPreventCollection());
-    HostMgr::instance().setPreventCollection(true);
+    ASSERT_FALSE(HostMgr::instance().getDisableSingleQuery());
+    HostMgr::instance().setDisableSingleQuery(true);
 
     // Create reservation for the client.
     HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
@@ -1105,6 +1109,8 @@ TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkPoolClassification) {
 // reservations belong (DHCPREQUEST case).
 TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservations) {
 
+    EXPECT_FALSE(HostMgr::instance().getDisableSingleQuery());
+
     // Create reservation for the client.
     HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
                           Host::IDENT_HWADDR, subnet2_->getID(),
@@ -1143,12 +1149,13 @@ TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservations) {
 // Test that reservations within shared network take precedence over the
 // existing leases regardless in which subnet belonging to a shared network
 // reservations belong (DHCPREQUEST case). Host lookups returning a collection
-// are disabled.
+// are disabled. As it is only an optimization the behavior (so the test)
+// must stay unchanged.
 TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservationsNoColl) {
 
     // Disable host lookups returning a collection.
-    ASSERT_FALSE(HostMgr::instance().getPreventCollection());
-    HostMgr::instance().setPreventCollection(true);
+    ASSERT_FALSE(HostMgr::instance().getDisableSingleQuery());
+    HostMgr::instance().setDisableSingleQuery(true);
 
     // Create reservation for the client.
     HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index 011fb3da8990ecc1eca580ac76e30edf6e80cd60..698a5a370075dd3c6f13471113265c567121d043 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -2581,6 +2581,8 @@ TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkPoolClassification) {
 // even if this address belongs to another subnet within the same
 // shared network.
 TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservations) {
+    EXPECT_FALSE(HostMgr::instance().getDisableSingleQuery());
+
     // Create reservation for the client in the second subnet.
     subnet_ = subnet2_;
     createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:2::15"), 128);
@@ -2605,10 +2607,12 @@ TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservations) {
 // This test verifies that the client is offerred a reserved address
 // even if this address belongs to another subnet within the same
 // shared network. Host lookups returning a collection are disabled.
+// As it is only an optimization the behavior (so the test) must stay
+// unchanged.
 TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservationsNoColl) {
     // Disable host lookups returning a collection.
-    ASSERT_FALSE(HostMgr::instance().getPreventCollection());
-    HostMgr::instance().setPreventCollection(true);
+    ASSERT_FALSE(HostMgr::instance().getDisableSingleQuery());
+    HostMgr::instance().setDisableSingleQuery(true);
 
     // Create reservation for the client in the second subnet.
     subnet_ = subnet2_;
@@ -2635,6 +2639,8 @@ TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservationsNoColl) {
 // even if this address belongs to another subnet within the same
 // shared network.
 TEST_F(SharedNetworkAlloc6Test, requestSharedNetworkReservations) {
+    EXPECT_FALSE(HostMgr::instance().getDisableSingleQuery());
+
     // Create reservation for the client in the second subnet.
     subnet_ = subnet2_;
     createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:2::15"), 128);
@@ -2659,10 +2665,12 @@ TEST_F(SharedNetworkAlloc6Test, requestSharedNetworkReservations) {
 // This test verifies that the client is allocated a reserved address
 // even if this address belongs to another subnet within the same
 // shared network. Host lookups returning a collection are disabled.
+// As it is only an optimization the behavior (so the test) must stay
+// unchanged.
 TEST_F(SharedNetworkAlloc6Test, requestSharedNetworkReservationsNoColl) {
     // Disable host lookups returning a collection.
-    ASSERT_FALSE(HostMgr::instance().getPreventCollection());
-    HostMgr::instance().setPreventCollection(true);
+    ASSERT_FALSE(HostMgr::instance().getDisableSingleQuery());
+    HostMgr::instance().setDisableSingleQuery(true);
 
     // Create reservation for the client in the second subnet.
     subnet_ = subnet2_;