From 991b2fadcb9e8171a78e27f95e4bfba6393d7824 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 28 Jan 2019 22:26:12 +0100 Subject: [PATCH] [313-return-a-list-of-all-reservations-by-subnet-id] Last changes --- src/lib/dhcpsrv/cql_host_data_source.cc | 4 +- src/lib/dhcpsrv/host_mgr.cc | 88 ++++++++++--------- .../tests/cql_host_data_source_unittest.cc | 8 +- .../tests/mysql_host_data_source_unittest.cc | 8 +- .../tests/pgsql_host_data_source_unittest.cc | 8 +- .../generic_host_data_source_unittest.cc | 12 ++- .../generic_host_data_source_unittest.h | 12 +-- 7 files changed, 74 insertions(+), 66 deletions(-) diff --git a/src/lib/dhcpsrv/cql_host_data_source.cc b/src/lib/dhcpsrv/cql_host_data_source.cc index a94121e437..e31071b86e 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.cc +++ b/src/lib/dhcpsrv/cql_host_data_source.cc @@ -1529,7 +1529,7 @@ public: /// @brief Implementation of @ref CqlHostDataSource::getPage4() /// /// Not implemented. - /// @todo: implement it. + /// @todo: implement it (#427). /// /// See @ref CqlHostDataSource::getPage4() for parameter details. /// @@ -1545,7 +1545,7 @@ public: /// @brief Implementation of @ref CqlHostDataSource::getPage6() /// /// Not implemented. - /// @todo: implement it. + /// @todo: implement it (#427). /// /// See @ref CqlHostDataSource::getPage6() for parameter details. /// diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index bfb146900f..94859dfad3 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -136,55 +136,63 @@ HostMgr::getPage4(const SubnetID& subnet_id, size_t& source_index, uint64_t lower_host_id, const HostPageSize& page_size) const { - for (;;) { - if (source_index > alternate_sources_.size()) { - return (ConstHostCollection()); - } - ConstHostCollection hosts; - if (source_index == 0) { - hosts = getCfgHosts()-> - getPage4(subnet_id, source_index, lower_host_id, page_size); - } else { - hosts = alternate_sources_[source_index - 1]-> - getPage4(subnet_id, source_index, lower_host_id, page_size); - } - if (!hosts.empty()) { - return (hosts); - } else { - ++source_index; - lower_host_id = 0; - continue; - } + // Return empty if (and only if) sources are exhausted. + if (source_index > alternate_sources_.size()) { + return (ConstHostCollection()); + } + + ConstHostCollection hosts; + // Source index 0 means config file. + if (source_index == 0) { + hosts = getCfgHosts()-> + getPage4(subnet_id, source_index, lower_host_id, page_size); + } else { + hosts = alternate_sources_[source_index - 1]-> + getPage4(subnet_id, source_index, lower_host_id, page_size); } + + // When got something return it. + if (!hosts.empty()) { + return (hosts); + } + + // Nothing from this source: try the next one. + // Note the recursion is limited to the number of sources in all cases. + ++source_index; + return (getPage4(subnet_id, source_index, 0UL, page_size)); } - + ConstHostCollection HostMgr::getPage6(const SubnetID& subnet_id, size_t& source_index, uint64_t lower_host_id, const HostPageSize& page_size) const { - for (;;) { - if (source_index > alternate_sources_.size()) { - return (ConstHostCollection()); - } - ConstHostCollection hosts; - if (source_index == 0) { - hosts = getCfgHosts()-> - getPage6(subnet_id, source_index, lower_host_id, page_size); - } else { - hosts = alternate_sources_[source_index - 1]-> - getPage6(subnet_id, source_index, lower_host_id, page_size); - } - if (!hosts.empty()) { - return (hosts); - } else { - ++source_index; - lower_host_id = 0; - continue; - } + // Return empty if (and only if) sources are exhausted. + if (source_index > alternate_sources_.size()) { + return (ConstHostCollection()); + } + + ConstHostCollection hosts; + // Source index 0 means config file. + if (source_index == 0) { + hosts = getCfgHosts()-> + getPage6(subnet_id, source_index, lower_host_id, page_size); + } else { + hosts = alternate_sources_[source_index - 1]-> + getPage6(subnet_id, source_index, lower_host_id, page_size); } + + // When got something return it. + if (!hosts.empty()) { + return (hosts); + } + + // Nothing from this source: try the next one. + // Note the recursion is limited to the number of sources in all cases. + ++source_index; + return (getPage6(subnet_id, source_index, 0UL, page_size)); } - + ConstHostCollection HostMgr::getAll4(const IOAddress& address) const { ConstHostCollection hosts = getCfgHosts()->getAll4(address); 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 71fd3e6448..0fe732c72c 100644 --- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc @@ -295,25 +295,25 @@ TEST_F(CqlHostDataSourceTest, DISABLED_testReadOnlyDatabase) { // Verifies that IPv4 host reservations in the same subnet can be retrieved TEST_F(CqlHostDataSourceTest, getAll4BySubnet) { - testGetAll4(Host::IDENT_HWADDR); + testGetAll4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved TEST_F(CqlHostDataSourceTest, getAll6BySubnet) { - testGetAll6(Host::IDENT_DUID); + testGetAll6(); } // Verifies that IPv4 host reservations in the same subnet can be retrieved // by pages. // Does not work because TOKEN(id) order is not the same than id... TEST_F(CqlHostDataSourceTest, DISABLED_getPage4) { - testGetPage4(Host::IDENT_DUID); + testGetPage4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved // by pages. TEST_F(CqlHostDataSourceTest, DISABLED_getPage6) { - testGetPage6(Host::IDENT_HWADDR); + testGetPage6(); } // Test verifies if a host reservation can be added and later retrieved by IPv4 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 6a1c3e1169..57c3258d15 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -313,24 +313,24 @@ TEST_F(MySqlHostDataSourceTest, maxSubnetId6) { // Verifies that IPv4 host reservations in the same subnet can be retrieved TEST_F(MySqlHostDataSourceTest, getAll4BySubnet) { - testGetAll4(Host::IDENT_HWADDR); + testGetAll4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved TEST_F(MySqlHostDataSourceTest, getAll6BySubnet) { - testGetAll6(Host::IDENT_DUID); + testGetAll6(); } // Verifies that IPv4 host reservations in the same subnet can be retrieved // by pages. TEST_F(MySqlHostDataSourceTest, getPage4) { - testGetPage4(Host::IDENT_DUID); + testGetPage4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved // by pages. TEST_F(MySqlHostDataSourceTest, getPage6) { - testGetPage6(Host::IDENT_HWADDR); + testGetPage6(); } // Verifies that IPv4 host reservations in the same subnet can be retrieved 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 01343dd7a5..67f8ecc1b7 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -298,24 +298,24 @@ TEST_F(PgSqlHostDataSourceTest, maxSubnetId6) { // Verifies that IPv4 host reservations in the same subnet can be retrieved TEST_F(PgSqlHostDataSourceTest, getAll4BySubnet) { - testGetAll4(Host::IDENT_HWADDR); + testGetAll4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved TEST_F(PgSqlHostDataSourceTest, getAll6BySubnet) { - testGetAll6(Host::IDENT_DUID); + testGetAll6(); } // Verifies that IPv4 host reservations in the same subnet can be retrieved // by pages. TEST_F(PgSqlHostDataSourceTest, getPage4) { - testGetPage4(Host::IDENT_DUID); + testGetPage4(); } // Verifies that IPv6 host reservations in the same subnet can be retrieved // by pages. TEST_F(PgSqlHostDataSourceTest, getPage6) { - testGetPage6(Host::IDENT_HWADDR); + testGetPage6(); } // Verifies that IPv4 host reservations in the same subnet can be retrieved 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 dabf94c15b..5d47aea68a 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc @@ -357,11 +357,12 @@ void GenericHostDataSourceTest::testMaxSubnetId6() { } void -GenericHostDataSourceTest::testGetAll4(const Host::IdentifierType& id) { +GenericHostDataSourceTest::testGetAll4() { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); // Let's create a couple of hosts... + const Host::IdentifierType& id = Host::IDENT_HWADDR; HostPtr host1 = HostDataSourceUtils::initializeHost4("192.0.2.1", id); HostPtr host2 = HostDataSourceUtils::initializeHost4("192.0.2.2", id); HostPtr host3 = HostDataSourceUtils::initializeHost4("192.0.2.3", id); @@ -400,11 +401,12 @@ GenericHostDataSourceTest::testGetAll4(const Host::IdentifierType& id) { } void -GenericHostDataSourceTest::testGetAll6(const Host::IdentifierType& id) { +GenericHostDataSourceTest::testGetAll6() { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); // Let's create a couple of hosts... + const Host::IdentifierType& id = Host::IDENT_DUID; HostPtr host1 = HostDataSourceUtils::initializeHost6("2001:db8::1", id, false); HostPtr host2 = HostDataSourceUtils::initializeHost6("2001:db8::2", id, false); HostPtr host3 = HostDataSourceUtils::initializeHost6("2001:db8::3", id, false); @@ -443,7 +445,7 @@ GenericHostDataSourceTest::testGetAll6(const Host::IdentifierType& id) { } void -GenericHostDataSourceTest::testGetPage4(const Host::IdentifierType& id) { +GenericHostDataSourceTest::testGetPage4() { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); @@ -451,6 +453,7 @@ GenericHostDataSourceTest::testGetPage4(const Host::IdentifierType& id) { IOAddress addr("192.0.2.0"); SubnetID subnet4(4); SubnetID subnet6(6); + const Host::IdentifierType& id = Host::IDENT_DUID; for (unsigned i = 0; i < 25; ++i) { addr = IOAddress::increase(addr); @@ -490,7 +493,7 @@ GenericHostDataSourceTest::testGetPage4(const Host::IdentifierType& id) { } void -GenericHostDataSourceTest::testGetPage6(const Host::IdentifierType& id) { +GenericHostDataSourceTest::testGetPage6() { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); @@ -498,6 +501,7 @@ GenericHostDataSourceTest::testGetPage6(const Host::IdentifierType& id) { IOAddress addr("2001:db8:1::"); SubnetID subnet4(4); SubnetID subnet6(6); + const Host::IdentifierType& id = Host::IDENT_HWADDR; for (unsigned i = 0; i < 25; ++i) { addr = IOAddress::increase(addr); diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h index 8ab7140ea8..d8e58fcbc3 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h @@ -182,29 +182,25 @@ public: /// same subnet can be retrieved properly. /// /// Uses gtest macros to report failures. - /// @param id Identifier type (hwaddr or duid). - void testGetAll4(const Host::IdentifierType& id); + void testGetAll4(); /// @brief Test that Verifies that IPv6 host reservations in the /// same subnet can be retrieved properly. /// /// Uses gtest macros to report failures. - /// @param id Identifier type (hwaddr or duid). - void testGetAll6(const Host::IdentifierType& id); + void testGetAll6(); /// @brief Test that Verifies that pages of host reservations in the /// same subnet can be retrieved properly. /// /// Uses gtest macros to report failures. - /// @param id Identifier type (hwaddr or duid). - void testGetPage4(const Host::IdentifierType& id); + void testGetPage4(); /// @brief Test that Verifies that pages of host reservations in the /// same subnet can be retrieved properly. /// /// Uses gtest macros to report failures. - /// @param id Identifier type (hwaddr or duid). - void testGetPage6(const Host::IdentifierType& id); + void testGetPage6(); /// @brief Test that Verifies that pages of complex host reservations /// are not truncated, i.e. the limit applies on the number of hosts -- 2.47.2