From: Francis Dupont Date: Mon, 14 Jan 2019 01:17:29 +0000 (+0100) Subject: [313-return-a-list-of-all-reservations-by-subnet-id] Finished getPage[46] X-Git-Tag: 429-Updated-StampedValue-to-support-reals_base~84 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=55b0c7c1312086c50c621dff50badffe6795214c;p=thirdparty%2Fkea.git [313-return-a-list-of-all-reservations-by-subnet-id] Finished getPage[46] --- diff --git a/src/lib/dhcpsrv/base_host_data_source.h b/src/lib/dhcpsrv/base_host_data_source.h index 9c56abd258..dae3977be2 100644 --- a/src/lib/dhcpsrv/base_host_data_source.h +++ b/src/lib/dhcpsrv/base_host_data_source.h @@ -143,7 +143,8 @@ public: /// starting host id of the range. If not zero this host id is /// excluded from the returned range. When a source is exhausted /// the index is updated. There is no guarantee about the order - /// of returned host reservations, only the sources are ordered. + /// of returned host reservations, only the sources and + /// reservations from the same source are ordered. /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source. @@ -165,7 +166,8 @@ public: /// starting host id of the range. If not zero this host id is /// excluded from the returned range. When a source is exhausted /// the index is updated. There is no guarantee about the order - /// of returned host reservations, only the sources are ordered. + /// of returned host reservations, only the sources and + /// reservations from the same source are ordered. /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source. diff --git a/src/lib/dhcpsrv/cql_host_data_source.cc b/src/lib/dhcpsrv/cql_host_data_source.cc index 0b513c63f1..af9ab3e766 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.cc +++ b/src/lib/dhcpsrv/cql_host_data_source.cc @@ -2170,6 +2170,12 @@ CqlHostDataSourceImpl::getPage4(const SubnetID& subnet_id, CqlHostExchange::GET_HOST_BY_IPV4_SUBNET_ID_PAGE, where_values); + // Sort result. + std::sort(result.begin(), result.end(), + [] (const ConstHostPtr& host1, + const ConstHostPtr& host2) { + return (host1->getHostId() < host2->getHostId()); }); + return (result); } @@ -2200,6 +2206,12 @@ CqlHostDataSourceImpl::getPage6(const SubnetID& subnet_id, CqlHostExchange::GET_HOST_BY_IPV6_SUBNET_ID_PAGE, where_values); + // Sort result. + std::sort(result.begin(), result.end(), + [] (const ConstHostPtr& host1, + const ConstHostPtr& host2) { + return (host1->getHostId() < host2->getHostId()); }); + return (result); } diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 9c256577df..cbc28146f7 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -142,15 +142,11 @@ HostMgr::getPage4(const SubnetID& subnet_id, } ConstHostCollection hosts; if (source_index == 0) { - hosts = getCfgHosts()->getPage4(subnet_id, - source_index, - lower_host_id, - page_size); + hosts = getCfgHosts()-> + getPage4(subnet_id, source_index, lower_host_id, page_size); } else { - hosts = alternate_sources_[source_index]->getPage4(subnet_id, - source_index, - lower_host_id, - page_size); + hosts = alternate_sources_[source_index - 1]-> + getPage4(subnet_id, source_index, lower_host_id, page_size); } if (!hosts.empty()) { return (hosts); @@ -172,15 +168,11 @@ HostMgr::getPage6(const SubnetID& subnet_id, } ConstHostCollection hosts; if (source_index == 0) { - hosts = getCfgHosts()->getPage6(subnet_id, - source_index, - lower_host_id, - page_size); + hosts = getCfgHosts()-> + getPage6(subnet_id, source_index, lower_host_id, page_size); } else { - hosts = alternate_sources_[source_index]->getPage6(subnet_id, - source_index, - lower_host_id, - page_size); + hosts = alternate_sources_[source_index - 1]-> + getPage6(subnet_id, source_index, lower_host_id, page_size); } if (!hosts.empty()) { return (hosts); diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index ec1d6508c0..d24e413155 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -2403,7 +2403,7 @@ TaggedStatementArray tagged_statements = { { "LEFT JOIN ipv6_reservations AS r " "ON h.host_id = r.host_id " "WHERE h.dhcp6_subnet_id = ? AND h.host_id > ? " - "ORDER BY h.host_id, o.option_id, r.reservation_id" + "ORDER BY h.host_id, o.option_id, r.reservation_id " "LIMIT ?"} } }; diff --git a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc index 010550a5ae..97ce820176 100644 --- a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -264,6 +264,85 @@ TEST_F(CfgHostsTest, getAll6BySubnet) { } } +// This test checks that hosts in the same subnet can be retrieved from +// the host configuration by pages. +TEST_F(CfgHostsTest, getPage4) { + CfgHosts cfg; + // Add 25 hosts identified by DUID in the same subnet. + for (unsigned i = 0; i < 25; ++i) { + cfg.add(HostPtr(new Host(duids_[i]->toText(), "duid", + SubnetID(1), SubnetID(1), + addressesa_[i]))); + } + size_t idx(0); + uint64_t host_id(0); + HostPageSize page_size(10); + + // Check that other subnets are empty. + HostCollection page = cfg.getPage4(SubnetID(100), idx, host_id, page_size); + EXPECT_EQ(0, page.size()); + + // Try to retrieve all added reservations. + // Get first page. + page = cfg.getPage4(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + + // Get second and last pages. + page = cfg.getPage4(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + page = cfg.getPage4(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(5, page.size()); + host_id = page[4]->getHostId(); + + // Verify we have everything. + page = cfg.getPage4(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(0, page.size()); +} + +// This test checks that hosts in the same subnet can be retrieved from +// the host configuration by pages. +TEST_F(CfgHostsTest, getPage6) { + CfgHosts cfg; + // Add 25 hosts identified by HW address in the same subnet. + for (unsigned i = 0; i < 25; ++i) { + HostPtr host = HostPtr(new Host(hwaddrs_[i]->toText(false), + "hw-address", + SubnetID(1), SubnetID(1), + IOAddress("0.0.0.0"))); + host->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, + increase(IOAddress("2001:db8:1::1"), + i))); + cfg.add(host); + } + size_t idx(0); + uint64_t host_id(0); + HostPageSize page_size(10); + + // Check that other subnets are empty. + HostCollection page = cfg.getPage6(SubnetID(100), idx, host_id, page_size); + EXPECT_EQ(0, page.size()); + + // Try to retrieve all added reservations. + // Get first page. + page = cfg.getPage6(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + + // Get second and last pages. + page = cfg.getPage6(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + page = cfg.getPage6(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(5, page.size()); + host_id = page[4]->getHostId(); + + // Verify we have everything. + page = cfg.getPage6(SubnetID(1), idx, host_id, page_size); + EXPECT_EQ(0, page.size()); +} + // This test checks that all reservations for the specified IPv4 address can // be retrieved. TEST_F(CfgHostsTest, getAll4ByAddress) { 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 7380f95010..71fd3e6448 100644 --- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc @@ -303,6 +303,19 @@ TEST_F(CqlHostDataSourceTest, getAll6BySubnet) { testGetAll6(Host::IDENT_DUID); } +// 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); +} + +// Verifies that IPv6 host reservations in the same subnet can be retrieved +// by pages. +TEST_F(CqlHostDataSourceTest, DISABLED_getPage6) { + testGetPage6(Host::IDENT_HWADDR); +} + // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses hw address as identifier. TEST_F(CqlHostDataSourceTest, basic4HWAddr) { 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 7fb48610a1..b915484ff5 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -321,6 +321,18 @@ TEST_F(MySqlHostDataSourceTest, getAll6BySubnet) { testGetAll6(Host::IDENT_DUID); } +// Verifies that IPv4 host reservations in the same subnet can be retrieved +// by pages. +TEST_F(MySqlHostDataSourceTest, getPage4) { + testGetPage4(Host::IDENT_DUID); +} + +// Verifies that IPv6 host reservations in the same subnet can be retrieved +// by pages. +TEST_F(MySqlHostDataSourceTest, getPage6) { + testGetPage6(Host::IDENT_HWADDR); +} + // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses client-id (DUID) as identifier. TEST_F(MySqlHostDataSourceTest, basic4ClientId) { 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 22bd07fa67..ea6af4620c 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -306,6 +306,18 @@ TEST_F(PgSqlHostDataSourceTest, getAll6BySubnet) { testGetAll6(Host::IDENT_DUID); } +// Verifies that IPv4 host reservations in the same subnet can be retrieved +// by pages. +TEST_F(PgSqlHostDataSourceTest, getPage4) { + testGetPage4(Host::IDENT_DUID); +} + +// Verifies that IPv6 host reservations in the same subnet can be retrieved +// by pages. +TEST_F(PgSqlHostDataSourceTest, getPage6) { + testGetPage6(Host::IDENT_HWADDR); +} + // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses client-id (DUID) as identifier. TEST_F(PgSqlHostDataSourceTest, basic4ClientId) { 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 e1530a0aec..d6003e669b 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc @@ -442,6 +442,100 @@ GenericHostDataSourceTest::testGetAll6(const Host::IdentifierType& id) { } } +void +GenericHostDataSourceTest::testGetPage4(const Host::IdentifierType& id) { + // Make sure we have a pointer to the host data source. + ASSERT_TRUE(hdsptr_); + + // Let's create some hosts... + IOAddress addr("192.0.2.0"); + SubnetID subnet4(4); + SubnetID subnet6(6); + for (unsigned i = 0; i < 25; ++i) { + addr = IOAddress::increase(addr); + + HostPtr host = HostDataSourceUtils::initializeHost4(addr.toText(), id); + host->setIPv4SubnetID(subnet4); + host->setIPv6SubnetID(subnet6); + + ASSERT_NO_THROW(hdsptr_->add(host)); + } + + // Get first page. + size_t idx(1); + uint64_t host_id(0); + HostPageSize page_size(10); + ConstHostCollection page; + ASSERT_NO_THROW(page = hdsptr_->getPage4(subnet4, idx, host_id, page_size)); + ASSERT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + ASSERT_NE(0, host_id); + + // Get second and last pages. + ASSERT_NO_THROW(page = hdsptr_->getPage4(subnet4, idx, host_id, page_size)); + ASSERT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + ASSERT_NO_THROW(page = hdsptr_->getPage4(subnet4, idx, host_id, page_size)); + ASSERT_EQ(5, page.size()); + host_id = page[4]->getHostId(); + + // Verify we have everything. + ASSERT_NO_THROW(page = hdsptr_->getPage4(subnet4, idx, host_id, page_size)); + ASSERT_EQ(0, page.size()); + host_id = 0; + + // Other subnets are empty. + ASSERT_NO_THROW(page = hdsptr_->getPage4(subnet6, idx, host_id, page_size)); + ASSERT_EQ(0, page.size()); +} + +void +GenericHostDataSourceTest::testGetPage6(const Host::IdentifierType& id) { + // Make sure we have a pointer to the host data source. + ASSERT_TRUE(hdsptr_); + + // Let's create some hosts... + IOAddress addr("2001:db8:1::"); + SubnetID subnet4(4); + SubnetID subnet6(6); + for (unsigned i = 0; i < 25; ++i) { + addr = IOAddress::increase(addr); + + HostPtr host = HostDataSourceUtils::initializeHost6(addr.toText(), id, false); + host->setIPv4SubnetID(subnet4); + host->setIPv6SubnetID(subnet6); + + ASSERT_NO_THROW(hdsptr_->add(host)); + } + + // Get first page. + size_t idx(1); + uint64_t host_id(0); + HostPageSize page_size(10); + ConstHostCollection page; + ASSERT_NO_THROW(page = hdsptr_->getPage6(subnet6, idx, host_id, page_size)); + ASSERT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + ASSERT_NE(0, host_id); + + // Get second and last pages. + ASSERT_NO_THROW(page = hdsptr_->getPage6(subnet6, idx, host_id, page_size)); + ASSERT_EQ(10, page.size()); + host_id = page[9]->getHostId(); + ASSERT_NO_THROW(page = hdsptr_->getPage6(subnet6, idx, host_id, page_size)); + ASSERT_EQ(5, page.size()); + host_id = page[4]->getHostId(); + + // Verify we have everything. + ASSERT_NO_THROW(page = hdsptr_->getPage6(subnet6, idx, host_id, page_size)); + ASSERT_EQ(0, page.size()); + host_id = 0; + + // Other subnets are empty. + ASSERT_NO_THROW(page = hdsptr_->getPage6(subnet4, idx, host_id, page_size)); + ASSERT_EQ(0, page.size()); +} + void GenericHostDataSourceTest::testGetByIPv4(const Host::IdentifierType& id) { // Make sure we have a pointer to the host data source. 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 c6cc028c19..e9eaef3f49 100644 --- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h @@ -192,6 +192,20 @@ public: /// @param id Identifier type. void testGetAll6(const Host::IdentifierType& id); + /// @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. + void testGetPage4(const Host::IdentifierType& id); + + /// @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. + void testGetPage6(const Host::IdentifierType& id); + /// @brief Test inserts several hosts with unique IPv4 address and /// checks that they can be retrieved properly. /// diff --git a/src/lib/dhcpsrv/testutils/memory_host_data_source.cc b/src/lib/dhcpsrv/testutils/memory_host_data_source.cc index 7a465b5c59..ba9a6bfd63 100644 --- a/src/lib/dhcpsrv/testutils/memory_host_data_source.cc +++ b/src/lib/dhcpsrv/testutils/memory_host_data_source.cc @@ -47,19 +47,47 @@ MemHostDataSource::getAll6(const SubnetID& subnet_id) const { } ConstHostCollection -MemHostDataSource::getPage4(const SubnetID& /*subnet_id*/, +MemHostDataSource::getPage4(const SubnetID& subnet_id, size_t& /*source_index*/, - uint64_t /*lower_host_id*/, - const HostPageSize& /*page_size*/) const { - return (ConstHostCollection()); + uint64_t lower_host_id, + const HostPageSize& page_size) const { + ConstHostCollection hosts; + for (auto h = store_.begin(); h != store_.end(); ++h) { + // Skip it when subnet_id does not match. + if ((*h)->getIPv4SubnetID() != subnet_id) { + continue; + } + if (lower_host_id && ((*h)->getHostId() <= lower_host_id)) { + continue; + } + hosts.push_back(*h); + if (hosts.size() == page_size.page_size_) { + break; + } + } + return (hosts); } ConstHostCollection -MemHostDataSource::getPage6(const SubnetID& /*subnet_id*/, +MemHostDataSource::getPage6(const SubnetID& subnet_id, size_t& /*source_index*/, - uint64_t /*lower_host_id*/, - const HostPageSize& /*page_size*/) const { - return (ConstHostCollection()); + uint64_t lower_host_id, + const HostPageSize& page_size) const { + ConstHostCollection hosts; + for (auto h = store_.begin(); h != store_.end(); ++h) { + // Skip it when subnet_id does not match. + if ((*h)->getIPv6SubnetID() != subnet_id) { + continue; + } + if (lower_host_id && ((*h)->getHostId() <= lower_host_id)) { + continue; + } + hosts.push_back(*h); + if (hosts.size() == page_size.page_size_) { + break; + } + } + return (hosts); } ConstHostCollection diff --git a/src/lib/dhcpsrv/testutils/memory_host_data_source.h b/src/lib/dhcpsrv/testutils/memory_host_data_source.h index 43fc74fe0c..a1577a41c0 100644 --- a/src/lib/dhcpsrv/testutils/memory_host_data_source.h +++ b/src/lib/dhcpsrv/testutils/memory_host_data_source.h @@ -60,8 +60,6 @@ public: /// @brief Return range of hosts in a DHCPv4 subnet. /// - /// Currently not implemented. - /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source (unused). /// @param lower_host_id Host identifier used as lower bound for the @@ -75,8 +73,6 @@ public: /// @brief Return range of hosts in a DHCPv6 subnet. /// - /// Currently not implemented. - /// /// @param subnet_id Subnet identifier. /// @param source_index Index of the source (unused). /// @param lower_host_id Host identifier used as lower bound for the