From: Marcin Siodelski <marcin@isc.org>
Date: Sat, 3 Oct 2020 07:02:35 +0000 (+0200)
Subject: [#1428] Final code cleanup
X-Git-Tag: Kea-1.9.1~126
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7041e0b50ffa875617427153cbd843ea52eebc37;p=thirdparty%2Fkea.git

[#1428] Final code cleanup

- Removed caching from getAllX functions in HostMgr
- Renamed setIPReservationUnique to setIPReservationsUnique
---

diff --git a/src/lib/dhcpsrv/base_host_data_source.h b/src/lib/dhcpsrv/base_host_data_source.h
index 15370c5dc6..9050feb83c 100644
--- a/src/lib/dhcpsrv/base_host_data_source.h
+++ b/src/lib/dhcpsrv/base_host_data_source.h
@@ -488,7 +488,7 @@ public:
     /// unique or can be non-unique.
     /// @return true if the new setting was accepted by the backend or false
     /// otherwise.
-    virtual bool setIPReservationUnique(const bool unique) = 0;
+    virtual bool setIPReservationsUnique(const bool unique) = 0;
 };
 
 /// @brief HostDataSource pointer
diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc
index bba9b39362..362767d86e 100644
--- a/src/lib/dhcpsrv/cfg_hosts.cc
+++ b/src/lib/dhcpsrv/cfg_hosts.cc
@@ -1167,7 +1167,7 @@ CfgHosts::del6(const SubnetID& /*subnet_id*/,
 }
 
 bool
-CfgHosts::setIPReservationUnique(const bool unique) {
+CfgHosts::setIPReservationsUnique(const bool unique) {
     ip_reservations_unique_ = unique;
     return (true);
 }
diff --git a/src/lib/dhcpsrv/cfg_hosts.h b/src/lib/dhcpsrv/cfg_hosts.h
index a41083b8a0..495cc81265 100644
--- a/src/lib/dhcpsrv/cfg_hosts.h
+++ b/src/lib/dhcpsrv/cfg_hosts.h
@@ -613,7 +613,7 @@ public:
     /// unique or can be non-unique.
     /// @return always true because this data source supports both the case when
     /// the addresses must be unique and when they may be non-unique.
-    virtual bool setIPReservationUnique(const bool unique);
+    virtual bool setIPReservationsUnique(const bool unique);
 
     /// @brief Unparse a configuration object
     ///
diff --git a/src/lib/dhcpsrv/cql_host_data_source.cc b/src/lib/dhcpsrv/cql_host_data_source.cc
index 0060f4387b..38cd0e2107 100644
--- a/src/lib/dhcpsrv/cql_host_data_source.cc
+++ b/src/lib/dhcpsrv/cql_host_data_source.cc
@@ -3683,7 +3683,7 @@ CqlHostDataSource::rollback() {
 }
 
 bool
-CqlHostDataSource::setIPReservationUnique(const bool unique) {
+CqlHostDataSource::setIPReservationsUnique(const bool unique) {
     // This backend does not support the mode in which multiple reservations
     // for the same IP address are created. If selecting this mode is
     // attempted this function returns false to indicate that this is
diff --git a/src/lib/dhcpsrv/cql_host_data_source.h b/src/lib/dhcpsrv/cql_host_data_source.h
index f06c0da128..f63b9e2538 100644
--- a/src/lib/dhcpsrv/cql_host_data_source.h
+++ b/src/lib/dhcpsrv/cql_host_data_source.h
@@ -457,7 +457,7 @@ public:
     /// @return true when addresses must be unique, false otherwise because
     /// this backend does not support specifying the same IP address in multiple
     /// host reservations.
-    virtual bool setIPReservationUnique(const bool unique) override;
+    virtual bool setIPReservationsUnique(const bool unique) override;
 
 private:
     /// @brief Pointer to the implementation of the @ref CqlHostDataSource.
diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc
index a2e1d875c0..97402cf5cb 100644
--- a/src/lib/dhcpsrv/host_mgr.cc
+++ b/src/lib/dhcpsrv/host_mgr.cc
@@ -405,27 +405,9 @@ HostMgr::getAll4(const SubnetID& subnet_id,
         .arg(subnet_id)
         .arg(address.toText());
 
-    if (cache_ptr_) {
-        auto cached = cache_ptr_->getAll4(subnet_id, address);
-        if (!cached.empty()) {
-            for (auto host : cached) {
-                if (!host->getNegative()) {
-                    hosts.push_back(host);
-                }
-            }
-            return (hosts);
-        }
-    }
-
     for (auto source : alternate_sources_) {
-        if (source == cache_ptr_) {
-            continue;
-        }
-        auto alternate_hosts = source->getAll4(subnet_id, address);
-        for (auto host : alternate_hosts) {
-            cache(host);
-            hosts.push_back(host);
-        }
+        auto hosts_plus = source->getAll4(subnet_id, address);
+        hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end());
     }
     return (hosts);
 }
@@ -553,27 +535,9 @@ HostMgr::getAll6(const SubnetID& subnet_id,
         .arg(subnet_id)
         .arg(address.toText());
 
-    if (cache_ptr_) {
-        auto cached = cache_ptr_->getAll6(subnet_id, address);
-        if (!cached.empty()) {
-            for (auto host : cached) {
-                if (!host->getNegative()) {
-                    hosts.push_back(host);
-                }
-            }
-            return (hosts);
-        }
-    }
-
     for (auto source : alternate_sources_) {
-        if (source == cache_ptr_) {
-            continue;
-        }
-        auto alternate_hosts = source->getAll6(subnet_id, address);
-        for (auto host : alternate_hosts) {
-            cache(host);
-            hosts.push_back(host);
-        }
+        auto hosts_plus = source->getAll6(subnet_id, address);
+        hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end());
     }
     return (hosts);
 }
@@ -674,18 +638,18 @@ HostMgr::cacheNegative(const SubnetID& ipv4_subnet_id,
 }
 
 bool
-HostMgr::setIPReservationUnique(const bool unique) {
+HostMgr::setIPReservationsUnique(const bool unique) {
     // Iterate over the alternate sources first, because they may include those
     // for which the new setting is not supported.
     for (auto source : alternate_sources_) {
-        if (!source->setIPReservationUnique(unique)) {
+        if (!source->setIPReservationsUnique(unique)) {
             // One of the sources does not support this new mode of operation.
             // Let's log a warning and back off the changes to the default
             // setting which should always be supported.
             LOG_WARN(hosts_logger, HOSTS_MGR_NON_UNIQUE_IP_UNSUPPORTED)
                 .arg(source->getType());
             for (auto source : alternate_sources_) {
-                source->setIPReservationUnique(true);
+                source->setIPReservationsUnique(true);
             }
             return (false);
         }
diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h
index 5c42360d15..48c678952a 100644
--- a/src/lib/dhcpsrv/host_mgr.h
+++ b/src/lib/dhcpsrv/host_mgr.h
@@ -606,7 +606,7 @@ public:
     /// unique or can be non-unique.
     /// @return true if the new setting was accepted by the backend or false
     /// otherwise.
-    virtual bool setIPReservationUnique(const bool unique);
+    virtual bool setIPReservationsUnique(const bool unique);
 
 protected:
     /// @brief The negative caching flag.
diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index 2306cb9632..a1c712e78a 100644
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -3810,7 +3810,7 @@ MySqlHostDataSource::rollback() {
 }
 
 bool
-MySqlHostDataSource::setIPReservationUnique(const bool unique) {
+MySqlHostDataSource::setIPReservationsUnique(const bool unique) {
     impl_->ip_reservations_unique_ = unique;
     return (true);
 }
diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h
index 96c7f2d096..0a0495d3d0 100644
--- a/src/lib/dhcpsrv/mysql_host_data_source.h
+++ b/src/lib/dhcpsrv/mysql_host_data_source.h
@@ -453,7 +453,7 @@ public:
     /// unique within the subnet or can be non-unique.
     /// @return always true because this backend supports both the case when
     /// the addresses must be unique and when they may be non-unique.
-    virtual bool setIPReservationUnique(const bool unique);
+    virtual bool setIPReservationsUnique(const bool unique);
 
     /// @brief Context RAII Allocator.
     class MySqlHostContextAlloc {
diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc
index 154ae7d305..18acf02b80 100644
--- a/src/lib/dhcpsrv/pgsql_host_data_source.cc
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc
@@ -3071,7 +3071,7 @@ PgSqlHostDataSource::rollback() {
 }
 
 bool
-PgSqlHostDataSource::setIPReservationUnique(const bool unique) {
+PgSqlHostDataSource::setIPReservationsUnique(const bool unique) {
     impl_->ip_reservations_unique_ = unique;
     return (true);
 }
diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h
index 5a4ae68f20..55b76afd89 100644
--- a/src/lib/dhcpsrv/pgsql_host_data_source.h
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.h
@@ -504,7 +504,7 @@ public:
     /// unique within the subnet or can be non-unique.
     /// @return always true because this backend supports both the case when
     /// the addresses must be unique and when they may be non-unique.
-    virtual bool setIPReservationUnique(const bool unique);
+    virtual bool setIPReservationsUnique(const bool unique);
 
     /// @brief Context RAII Allocator.
     class PgSqlHostContextAlloc {
diff --git a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc
index c1141d9519..37abff62f6 100644
--- a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc
@@ -936,7 +936,7 @@ TEST_F(CfgHostsTest, add4AlreadyReserved) {
 TEST_F(CfgHostsTest, allow4AlreadyReserved) {
     CfgHosts cfg;
     // Allow creating multiple reservations for the same IP address.
-    ASSERT_TRUE(cfg.setIPReservationUnique(false));
+    ASSERT_TRUE(cfg.setIPReservationsUnique(false));
 
     // First host has a reservation for address 192.0.2.1
     HostPtr host1 = HostPtr(new Host(hwaddrs_[0]->toText(false),
@@ -995,7 +995,7 @@ TEST_F(CfgHostsTest, add6Invalid2Hosts) {
 TEST_F(CfgHostsTest, allowAddress6AlreadyReserved) {
     CfgHosts cfg;
     // Allow creating multiple reservations for the same IP address.
-    ASSERT_TRUE(cfg.setIPReservationUnique(false));
+    ASSERT_TRUE(cfg.setIPReservationsUnique(false));
 
     // First host has a reservation for address 2001:db8::1
     HostPtr host1 = HostPtr(new Host(duids_[0]->toText(), "duid",
@@ -1036,7 +1036,7 @@ TEST_F(CfgHostsTest, allowAddress6AlreadyReserved) {
 TEST_F(CfgHostsTest, allowPrefix6AlreadyReserved) {
     CfgHosts cfg;
     // Allow creating multiple reservations for the same IP address.
-    ASSERT_TRUE(cfg.setIPReservationUnique(false));
+    ASSERT_TRUE(cfg.setIPReservationsUnique(false));
 
     // First host has a reservation for address 3000::/64.
     HostPtr host1 = HostPtr(new Host(duids_[0]->toText(), "duid",
diff --git a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
index 1ac07352e4..575f773003 100644
--- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
@@ -70,7 +70,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
     }
 
     /// @brief Destroys the HDS and the schema.
diff --git a/src/lib/dhcpsrv/tests/host_cache_unittest.cc b/src/lib/dhcpsrv/tests/host_cache_unittest.cc
index 34d0554b1d..c6ad920098 100644
--- a/src/lib/dhcpsrv/tests/host_cache_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_cache_unittest.cc
@@ -702,7 +702,7 @@ public:
         return ("one");
     }
 
-    bool setIPReservationUnique(const bool) {
+    bool setIPReservationsUnique(const bool) {
         return (true);
     }
 
diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
index 214f446881..84d67e6b9b 100644
--- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
@@ -1163,8 +1163,8 @@ HostMgrTest::testGetAll4BySubnetIP(BaseHostDataSource& data_source1,
                                    BaseHostDataSource& data_source2) {
     // Set the mode of operation with multiple reservations for the same
     // IP address.
-    ASSERT_TRUE(HostMgr::instance().setIPReservationUnique(false));
-    CfgMgr::instance().getStagingCfg()->getCfgHosts()->setIPReservationUnique(false);
+    ASSERT_TRUE(HostMgr::instance().setIPReservationsUnique(false));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->setIPReservationsUnique(false);
 
     // Initially, no reservations should be present.
     ConstHostCollection hosts = HostMgr::instance().getAll4(SubnetID(1),
@@ -1201,8 +1201,8 @@ HostMgrTest::testGetAll6BySubnetIP(BaseHostDataSource& data_source1,
                                    BaseHostDataSource& data_source2) {
     // Set the mode of operation with multiple reservations for the same
     // IP address.
-    ASSERT_TRUE(HostMgr::instance().setIPReservationUnique(false));
-    CfgMgr::instance().getStagingCfg()->getCfgHosts()->setIPReservationUnique(false);
+    ASSERT_TRUE(HostMgr::instance().setIPReservationsUnique(false));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->setIPReservationsUnique(false);
 
     // Initially, no reservations should be present.
     ConstHostCollection hosts = HostMgr::instance().getAll6(SubnetID(1),
@@ -1638,9 +1638,9 @@ TEST_F(MySQLHostMgrTest, get6ByPrefix) {
 
 // This test verifies that it is possible to control whether the reserved
 // IP addresses are unique or non unique via the HostMgr.
-TEST_F(MySQLHostMgrTest, setIPReservationUnique) {
-    EXPECT_TRUE(HostMgr::instance().setIPReservationUnique(true));
-    EXPECT_TRUE(HostMgr::instance().setIPReservationUnique(false));
+TEST_F(MySQLHostMgrTest, setIPReservationsUnique) {
+    EXPECT_TRUE(HostMgr::instance().setIPReservationsUnique(true));
+    EXPECT_TRUE(HostMgr::instance().setIPReservationsUnique(false));
 }
 
 // Verifies that loss of connectivity to MySQL is handled correctly.
@@ -1816,8 +1816,8 @@ TEST_F(PostgreSQLHostMgrTest, get6ByPrefix) {
 // This test verifies that it is possible to control whether the reserved
 // IP addresses are unique or non unique via the HostMgr.
 TEST_F(PostgreSQLHostMgrTest, setIPReservationUnique) {
-    EXPECT_TRUE(HostMgr::instance().setIPReservationUnique(true));
-    EXPECT_TRUE(HostMgr::instance().setIPReservationUnique(false));
+    EXPECT_TRUE(HostMgr::instance().setIPReservationsUnique(true));
+    EXPECT_TRUE(HostMgr::instance().setIPReservationsUnique(false));
 }
 
 // Verifies that loss of connectivity to PostgreSQL is handled correctly.
@@ -1960,10 +1960,10 @@ TEST_F(CQLHostMgrTest, get6ByPrefix) {
 
 // This test verifies that it is possible to control whether the reserved
 // IP addresses are unique or non unique via the HostMgr.
-TEST_F(CQLHostMgrTest, setIPReservationUnique) {
-    EXPECT_TRUE(HostMgr::instance().setIPReservationUnique(true));
+TEST_F(CQLHostMgrTest, setIPReservationsUnique) {
+    EXPECT_TRUE(HostMgr::instance().setIPReservationsUnique(true));
     // This is currently not supported for Cassandra.
-    EXPECT_FALSE(HostMgr::instance().setIPReservationUnique(false));
+    EXPECT_FALSE(HostMgr::instance().setIPReservationsUnique(false));
 }
 
 #endif
diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
index f6334e2dc5..26ded18ed6 100644
--- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
@@ -62,7 +62,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
 
 
         MultiThreadingMgr::instance().setMode(false);
diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
index 2baac370a9..aa984171eb 100644
--- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
@@ -62,7 +62,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
 
         MultiThreadingMgr::instance().setMode(false);
     }
diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
index a5919e3056..e7bf0211f9 100644
--- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
@@ -1865,7 +1865,7 @@ void
 GenericHostDataSourceTest::testAllowDuplicateIPv6() {
     // Make sure we have the pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
-    ASSERT_TRUE(hdsptr_->setIPReservationUnique(false));
+    ASSERT_TRUE(hdsptr_->setIPReservationsUnique(false));
 
     // Create a host reservations.
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_HWADDR, true, true);
@@ -1934,7 +1934,7 @@ void
 GenericHostDataSourceTest::testAllowDuplicateIPv4() {
     // Make sure we have the pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
-    ASSERT_TRUE(hdsptr_->setIPReservationUnique(false));
+    ASSERT_TRUE(hdsptr_->setIPReservationsUnique(false));
 
     // Create a host reservations.
     HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_DUID, true);
@@ -1979,10 +1979,10 @@ GenericHostDataSourceTest::testDisallowDuplicateIP() {
     ASSERT_TRUE(hdsptr_);
     // The backend does not support switching to the mode in which multiple
     // reservations for the same address can be created.
-    EXPECT_FALSE(hdsptr_->setIPReservationUnique(false));
+    EXPECT_FALSE(hdsptr_->setIPReservationsUnique(false));
 
     // The default mode still can be used.
-    EXPECT_TRUE(hdsptr_->setIPReservationUnique(true));
+    EXPECT_TRUE(hdsptr_->setIPReservationsUnique(true));
 }
 
 void
diff --git a/src/lib/dhcpsrv/testutils/memory_host_data_source.h b/src/lib/dhcpsrv/testutils/memory_host_data_source.h
index cd39b70abc..b2d06f48f2 100644
--- a/src/lib/dhcpsrv/testutils/memory_host_data_source.h
+++ b/src/lib/dhcpsrv/testutils/memory_host_data_source.h
@@ -307,7 +307,7 @@ public:
     /// unique or can be non-unique.
     /// @return true if the new setting was accepted by the backend or false
     /// otherwise.
-    virtual bool setIPReservationUnique(const bool) {
+    virtual bool setIPReservationsUnique(const bool) {
         return (true);
     }