]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1428] Final code cleanup
authorMarcin Siodelski <marcin@isc.org>
Sat, 3 Oct 2020 07:02:35 +0000 (09:02 +0200)
committerMarcin Siodelski <marcin@isc.org>
Mon, 5 Oct 2020 13:14:58 +0000 (13:14 +0000)
- Removed caching from getAllX functions in HostMgr
- Renamed setIPReservationUnique to setIPReservationsUnique

19 files changed:
src/lib/dhcpsrv/base_host_data_source.h
src/lib/dhcpsrv/cfg_hosts.cc
src/lib/dhcpsrv/cfg_hosts.h
src/lib/dhcpsrv/cql_host_data_source.cc
src/lib/dhcpsrv/cql_host_data_source.h
src/lib/dhcpsrv/host_mgr.cc
src/lib/dhcpsrv/host_mgr.h
src/lib/dhcpsrv/mysql_host_data_source.cc
src/lib/dhcpsrv/mysql_host_data_source.h
src/lib/dhcpsrv/pgsql_host_data_source.cc
src/lib/dhcpsrv/pgsql_host_data_source.h
src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc
src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
src/lib/dhcpsrv/tests/host_cache_unittest.cc
src/lib/dhcpsrv/tests/host_mgr_unittest.cc
src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
src/lib/dhcpsrv/testutils/memory_host_data_source.h

index 15370c5dc61f6c650607c830caa76b3d797d3a3d..9050feb83ca5fb5cf814daf5572d3e876784685a 100644 (file)
@@ -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
index bba9b39362a9aa5338b995990c069a089e76e5d9..362767d86eab56be1484983e01a0f34dd1d63d18 100644 (file)
@@ -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);
 }
index a41083b8a0a4e8e9218313521326b3a29a0e7afc..495cc81265ba22912c5fc014c499b51546acaccb 100644 (file)
@@ -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
     ///
index 0060f4387b0d8964577f66a6085059ec87dfa0ab..38cd0e2107d80be7463da64ba719d73d7ec245f1 100644 (file)
@@ -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
index f06c0da128516ffff9ec5eeaad7a0cc27264dae4..f63b9e253853d848d083eebe6edaff4ba9f0eca2 100644 (file)
@@ -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.
index a2e1d875c0e96f12c7493a551961dae74e9c44b3..97402cf5cb16ee3040bcfff90deb6cc3b43076a7 100644 (file)
@@ -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);
         }
index 5c42360d151fb93adca6e279fa2665d608ab10a4..48c678952abeef637f78dac05b7d3a0bcfb54c0f 100644 (file)
@@ -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.
index 2306cb9632fc059fad9f9bcbfd1a7e68af6e8f03..a1c712e78a0fce3ade5be7e983f1a6fa91685179 100644 (file)
@@ -3810,7 +3810,7 @@ MySqlHostDataSource::rollback() {
 }
 
 bool
-MySqlHostDataSource::setIPReservationUnique(const bool unique) {
+MySqlHostDataSource::setIPReservationsUnique(const bool unique) {
     impl_->ip_reservations_unique_ = unique;
     return (true);
 }
index 96c7f2d096d5bd165165ef7460048ccec12f2deb..0a0495d3d0755a1789313fc163b92e0077c4c869 100644 (file)
@@ -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 {
index 154ae7d305035684cb6f1d2060490866731f7bf3..18acf02b80c0df0768af68fe6f937ef07c603922 100644 (file)
@@ -3071,7 +3071,7 @@ PgSqlHostDataSource::rollback() {
 }
 
 bool
-PgSqlHostDataSource::setIPReservationUnique(const bool unique) {
+PgSqlHostDataSource::setIPReservationsUnique(const bool unique) {
     impl_->ip_reservations_unique_ = unique;
     return (true);
 }
index 5a4ae68f200307becbdf8fba82dffbec30a92a05..55b76afd89926a5739e54a372b01d8101ac0ccfb 100644 (file)
@@ -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 {
index c1141d95191f21d70a24c68ebc9662e5911f0cf1..37abff62f6a03d4f1434a44591b54987b841046d 100644 (file)
@@ -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",
index 1ac07352e4e4679ed362d5097afe55da6fbeac76..575f7730032b28838dbb091602cbd0b314b66c48 100644 (file)
@@ -70,7 +70,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
     }
 
     /// @brief Destroys the HDS and the schema.
index 34d0554b1da094208e5aa3b66f27bd0672a3691e..c6ad92009845c7a3a9e7c12097a24e9ce09fdf7f 100644 (file)
@@ -702,7 +702,7 @@ public:
         return ("one");
     }
 
-    bool setIPReservationUnique(const bool) {
+    bool setIPReservationsUnique(const bool) {
         return (true);
     }
 
index 214f446881f9afd3a76596003e38a85ad2897c68..84d67e6b9bbe5bbe737c95c025bc9244de4ac28e 100644 (file)
@@ -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
index f6334e2dc504526cdeac394e586ff9bda54d22bf..26ded18ed6f1f22f7a075b684de295d2e4308e25 100644 (file)
@@ -62,7 +62,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
 
 
         MultiThreadingMgr::instance().setMode(false);
index 2baac370a998988eb2a1ae874887158d295b4e65..aa984171eb939b2d12d45fbe58be67f597fe8436 100644 (file)
@@ -62,7 +62,7 @@ public:
         }
 
         hdsptr_ = HostMgr::instance().getHostDataSource();
-        hdsptr_->setIPReservationUnique(true);
+        hdsptr_->setIPReservationsUnique(true);
 
         MultiThreadingMgr::instance().setMode(false);
     }
index a5919e30565f8ba6b4663cae957fe0fd53bda7e2..e7bf0211f97adee1facab94c0d721b8f329b952f 100644 (file)
@@ -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
index cd39b70abc481a6a81529790f29b9048741eb57b..b2d06f48f2ff7db1ef0f97121641637d6862738c 100644 (file)
@@ -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);
     }