From: Marcin Siodelski Date: Tue, 5 Apr 2016 11:30:37 +0000 (+0200) Subject: [4300] Addressed review comments. X-Git-Tag: trac4106_update_base~58^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7741ab8ce81fa1971332e86b1e1eb121c62c6bde;p=thirdparty%2Fkea.git [4300] Addressed review comments. --- diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc index 4ca51a0e0a..73f6042f18 100644 --- a/src/lib/dhcpsrv/host.cc +++ b/src/lib/dhcpsrv/host.cc @@ -83,7 +83,7 @@ Host::Host(const uint8_t* identifier, const size_t identifier_len, dhcp6_client_classes_(dhcp6_client_classes), host_id_(0), cfg_option4_(), cfg_option6_() { - // Initialize HWAddr or DUID + // Initialize host identifier. setIdentifier(identifier, identifier_len, identifier_type); if (!ipv4_reservation.isV4Zero()) { @@ -147,12 +147,6 @@ Host::getIdentifierAsText() const { std::string Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value, const size_t length) { - // Length 0 doesn't make sense. - if (length == 0) { - isc_throw(BadValue, "invalid length 0 of the host identifier while" - " converting the identifier to a textual form"); - } - // Convert identifier into = form. std::ostringstream s; switch (type) { @@ -166,14 +160,33 @@ Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value, s << "circuit-id"; break; default: - isc_throw(BadValue, "requested conversion of the unsupported" - " identifier into textual form"); + // This should never happen actually, unless we add new identifier + // and forget to add a case for it above. + s << "(invalid-type)"; } std::vector vec(value, value + length); - s << "=" << util::encode::encodeHex(vec); + s << "=" << (length > 0 ? util::encode::encodeHex(vec) : "(null)"); return (s.str()); } +std::string +Host::getIdentifierName(const IdentifierType& type) { + switch (type) { + case Host::IDENT_HWADDR: + return ("hw-address"); + + case Host::IDENT_DUID: + return ("duid"); + + case Host::IDENT_CIRCUIT_ID: + return ("circuit-id"); + + default: + ; + } + return ("(unknown)"); +} + void Host::setIdentifier(const uint8_t* identifier, const size_t len, diff --git a/src/lib/dhcpsrv/host.h b/src/lib/dhcpsrv/host.h index 2c1c9e4fee..df5b5cd247 100644 --- a/src/lib/dhcpsrv/host.h +++ b/src/lib/dhcpsrv/host.h @@ -229,7 +229,7 @@ public: /// /// In case of identifiers other than HW address and DUID it is possible to use /// textual representation, e.g. 'some identifier', which is converted to a - /// vector of ASCII codes represnting characters in a given string, excluding + /// vector of ASCII codes representing characters in a given string, excluding /// quotes. This is useful in cases when specific identifiers, e.g. circuit-id /// are manually assigned user friendly values. /// @@ -309,6 +309,9 @@ public: /// @return Identifier in the form of =. std::string getIdentifierAsText() const; + /// @brief Returns name of the identifier of a specified type. + static std::string getIdentifierName(const IdentifierType& type); + /// @brief Returns host identifier in textual form. /// /// @param type Identifier type. diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 0d31a0772d..dfef3373c0 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -42,6 +42,11 @@ const size_t CLIENT_CLASSES_MAX_LEN = 255; /// in the Client FQDN %Option (see RFC4702 and RFC4704). const size_t HOSTNAME_MAX_LEN = 255; +/// @brief Numeric value representing last supported identifier. +/// +/// This value is used to validate whether the identifier type stored in +/// a database is within bounds. of supported identifiers. +const uint8_t MAX_IDENTIFIER_TYPE = static_cast(Host::IDENT_CIRCUIT_ID); /// @brief Prepared MySQL statements used by the backend to insert and /// retrieve hosts from the database. @@ -299,48 +304,21 @@ public: bind_[0].is_unsigned = MLM_TRUE; // dhcp_identifier : VARBINARY(128) NOT NULL - // Check which of the identifiers is used and set values accordingly - if (host->getDuid()) { - dhcp_identifier_length_ = host->getDuid()->getDuid().size(); - memcpy(static_cast(dhcp_identifier_buffer_), - &(host->getDuid()->getDuid()[0]), - host->getDuid()->getDuid().size()); - - bind_[1].buffer_type = MYSQL_TYPE_BLOB; - bind_[1].buffer = dhcp_identifier_buffer_; - bind_[1].buffer_length = dhcp_identifier_length_; - bind_[1].length = &dhcp_identifier_length_; - - } else if (host->getHWAddress()){ - dhcp_identifier_length_ = host->getHWAddress()->hwaddr_.size(); - memcpy(static_cast(dhcp_identifier_buffer_), - &(host->getHWAddress()->hwaddr_[0]), - host->getHWAddress()->hwaddr_.size()); - - bind_[1].buffer_type = MYSQL_TYPE_BLOB; - bind_[1].buffer = dhcp_identifier_buffer_; - bind_[1].buffer_length = dhcp_identifier_length_; - bind_[1].length = &dhcp_identifier_length_; - - } else { - isc_throw(DbOperationError, "Host object doesn't contain any" - " identifier which can be used to retrieve information" - " from the database about its static reservations"); - } + dhcp_identifier_length_ = host->getIdentifier().size(); + memcpy(static_cast(dhcp_identifier_buffer_), + &(host->getIdentifier())[0], + host->getIdentifier().size()); + + bind_[1].buffer_type = MYSQL_TYPE_BLOB; + bind_[1].buffer = dhcp_identifier_buffer_; + bind_[1].buffer_length = dhcp_identifier_length_; + bind_[1].length = &dhcp_identifier_length_; // dhcp_identifier_type : TINYINT NOT NULL - // Check which of the identifier types is used and set values accordingly - if (host->getHWAddress()) { - dhcp_identifier_type_ = BaseHostDataSource::ID_HWADDR; // 0 - bind_[2].buffer_type = MYSQL_TYPE_TINY; - bind_[2].buffer = reinterpret_cast(&dhcp_identifier_type_); - bind_[2].is_unsigned = MLM_TRUE; - } else if (host->getDuid()) { - dhcp_identifier_type_ = BaseHostDataSource::ID_DUID; // 1 - bind_[2].buffer_type = MYSQL_TYPE_TINY; - bind_[2].buffer = reinterpret_cast(&dhcp_identifier_type_); - bind_[2].is_unsigned = MLM_TRUE; - } + dhcp_identifier_type_ = static_cast(host->getIdentifierType()); + bind_[2].buffer_type = MYSQL_TYPE_TINY; + bind_[2].buffer = reinterpret_cast(&dhcp_identifier_type_); + bind_[2].is_unsigned = MLM_TRUE; // dhcp4_subnet_id : INT UNSIGNED NULL // Can't take an address of intermediate object, so let's store it @@ -501,25 +479,14 @@ public: /// @return Host Pointer to a @ref HostPtr object holding a pointer to the /// @ref Host object returned. HostPtr retrieveHost() { - - // Set the dhcp identifier type in a variable of the appropriate data type, - // which has been initialized with an arbitrary (but valid) value. - Host::IdentifierType type = Host::IDENT_HWADDR; - - switch (dhcp_identifier_type_) { - case 0: - type = Host::IDENT_HWADDR; - break; - - case 1: - type = Host::IDENT_DUID; - break; - - default: + // Check if the identifier stored in the database is correct. + if (dhcp_identifier_type_ > MAX_IDENTIFIER_TYPE) { isc_throw(BadValue, "invalid dhcp identifier type returned: " - << static_cast(dhcp_identifier_type_) - << ". Only 0 or 1 are supported."); + << static_cast(dhcp_identifier_type_)); } + // Set the dhcp identifier type in a variable of the appropriate data type. + Host::IdentifierType type = + static_cast(dhcp_identifier_type_); // Set DHCPv4 subnet ID to the value returned. If NULL returned, set to 0. SubnetID ipv4_subnet_id(0); diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index 90b70a6e30..01b5316a0f 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -26,7 +26,7 @@ GenericHostDataSourceTest::GenericHostDataSourceTest() GenericHostDataSourceTest::~GenericHostDataSourceTest() { } -std::string +std::vector GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) { /// @todo: Consider moving this somewhere to lib/testutils. @@ -34,15 +34,6 @@ GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) { // if you need to enter MySQL queries by hand. static uint8_t hwaddr[] = {65, 66, 67, 68, 69, 70}; - stringstream tmp; - for (int i = 0; i < sizeof(hwaddr); ++i) { - if (i) { - tmp << ":"; - } - tmp << std::setw(2) << std::hex << std::setfill('0') - << static_cast(hwaddr[i]); - } - if (new_identifier) { // Increase the address for the next time we use it. // This is primitive, but will work for 65k unique @@ -52,46 +43,38 @@ GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) { hwaddr[sizeof(hwaddr) - 2]++; } } - return (tmp.str()); + return (std::vector(hwaddr, hwaddr + sizeof(hwaddr))); } -std::string -GenericHostDataSourceTest::generateDuid(const bool new_identifier) { +std::vector +GenericHostDataSourceTest::generateIdentifier(const bool new_identifier) { /// @todo: Consider moving this somewhere to lib/testutils. // Let's use something that is easily printable. That's convenient // if you need to enter MySQL queries by hand. - static uint8_t duid[] = { 65, 66, 67, 68, 69, 70, 71, 72, 73, 74 }; + static uint8_t ident[] = { 65, 66, 67, 68, 69, 70, 71, 72, 73, 74 }; - stringstream tmp; - for (int i = 0; i < sizeof(duid); ++i) { - tmp << std::setw(2) << std::hex << std::setfill('0') - << static_cast(duid[i]); - } - - // Increase the DUID for the next time we use it. + // Increase the identifier for the next time we use it. // This is primitive, but will work for 65k unique - // DUIDs. + // identifiers. if (new_identifier) { - duid[sizeof(duid) - 1]++; - if (duid[sizeof(duid) - 1] == 0) { - duid[sizeof(duid) - 2]++; + ident[sizeof(ident) - 1]++; + if (ident[sizeof(ident) - 1] == 0) { + ident[sizeof(ident) - 2]++; } } - return (tmp.str()); + return (std::vector(ident, ident + sizeof(ident))); } -HostPtr GenericHostDataSourceTest::initializeHost4(std::string address, - bool hwaddr) { - string ident; - string ident_type; - - if (hwaddr) { +HostPtr +GenericHostDataSourceTest::initializeHost4(const std::string& address, + const Host::IdentifierType& id) { + std::vector ident; + if (id == Host::IDENT_HWADDR) { ident = generateHWAddr(); - ident_type = "hw-address"; + } else { - ident = generateDuid(); - ident_type = "duid"; + ident = generateIdentifier(); } // Let's create ever increasing subnet-ids. Let's keep those different, @@ -103,7 +86,7 @@ HostPtr GenericHostDataSourceTest::initializeHost4(std::string address, subnet6++; IOAddress addr(address); - HostPtr host(new Host(ident, ident_type, subnet4, subnet6, addr)); + HostPtr host(new Host(&ident[0], ident.size(), id, subnet4, subnet6, addr)); return (host); } @@ -112,17 +95,13 @@ HostPtr GenericHostDataSourceTest::initializeHost6(std::string address, Host::IdentifierType identifier, bool prefix, bool new_identifier) { - string ident; - string ident_type; - + std::vector ident; switch (identifier) { case Host::IDENT_HWADDR: ident = generateHWAddr(new_identifier); - ident_type = "hw-address"; break; case Host::IDENT_DUID: - ident = generateDuid(new_identifier); - ident_type = "duid"; + ident = generateIdentifier(new_identifier); break; default: ADD_FAILURE() << "Unknown IdType: " << identifier; @@ -137,7 +116,8 @@ HostPtr GenericHostDataSourceTest::initializeHost6(std::string address, subnet4++; subnet6++; - HostPtr host(new Host(ident, ident_type, subnet4, subnet6, IOAddress("0.0.0.0"))); + HostPtr host(new Host(&ident[0], ident.size(), identifier, subnet4, + subnet6, IOAddress("0.0.0.0"))); if (!prefix) { // Create IPv6 reservation (for an address) @@ -333,12 +313,12 @@ GenericHostDataSourceTest::compareClientClasses(const ClientClasses& /*classes1* /// This is part of the work for #4213. } -void GenericHostDataSourceTest::testBasic4(bool hwaddr) { +void GenericHostDataSourceTest::testBasic4(const Host::IdentifierType& id) { // Make sure we have the pointer to the host data source. ASSERT_TRUE(hdsptr_); // Create a host reservation. - HostPtr host = initializeHost4("192.0.2.1", hwaddr); + HostPtr host = initializeHost4("192.0.2.1", id); ASSERT_TRUE(host); // Make sure the host is generate properly. SubnetID subnet = host->getIPv4SubnetID(); @@ -358,15 +338,15 @@ void GenericHostDataSourceTest::testBasic4(bool hwaddr) { } -void GenericHostDataSourceTest::testGetByIPv4(bool hwaddr) { +void GenericHostDataSourceTest::testGetByIPv4(const Host::IdentifierType& id) { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); // Let's create a couple of hosts... - HostPtr host1 = initializeHost4("192.0.2.1", hwaddr); - HostPtr host2 = initializeHost4("192.0.2.2", hwaddr); - HostPtr host3 = initializeHost4("192.0.2.3", hwaddr); - HostPtr host4 = initializeHost4("192.0.2.4", hwaddr); + HostPtr host1 = initializeHost4("192.0.2.1", id); + HostPtr host2 = initializeHost4("192.0.2.2", id); + HostPtr host3 = initializeHost4("192.0.2.3", id); + HostPtr host4 = initializeHost4("192.0.2.4", id); // ... and add them to the data source. ASSERT_NO_THROW(hdsptr_->add(host1)); @@ -402,52 +382,16 @@ void GenericHostDataSourceTest::testGetByIPv4(bool hwaddr) { EXPECT_FALSE(hdsptr_->get4(subnet1, IOAddress("192.0.1.5"))); } -void GenericHostDataSourceTest::testGet4ByHWAddr() { - // Make sure we have a pointer to the host data source. - ASSERT_TRUE(hdsptr_); - - HostPtr host1 = initializeHost4("192.0.2.1", true); - HostPtr host2 = initializeHost4("192.0.2.2", true); - - // Sanity check: make sure the hosts have different HW addresses. - ASSERT_TRUE(host1->getHWAddress()); - ASSERT_TRUE(host2->getHWAddress()); - compareHwaddrs(host1, host2, false); - - // Try to add both of them to the host data source. - ASSERT_NO_THROW(hdsptr_->add(host1)); - ASSERT_NO_THROW(hdsptr_->add(host2)); - - SubnetID subnet1 = host1->getIPv4SubnetID(); - SubnetID subnet2 = host2->getIPv4SubnetID(); - - ConstHostPtr from_hds1 = hdsptr_->get4(subnet1, - Host::IDENT_HWADDR, - &host1->getIdentifier()[0], - host1->getIdentifier().size()); - ConstHostPtr from_hds2 = hdsptr_->get4(subnet2, - Host::IDENT_HWADDR, - &host2->getIdentifier()[0], - host2->getIdentifier().size()); - - // Now let's check if we got what we expected. - ASSERT_TRUE(from_hds1); - ASSERT_TRUE(from_hds2); - compareHosts(host1, from_hds1); - compareHosts(host2, from_hds2); -} - -void GenericHostDataSourceTest::testGet4ByClientId() { +void +GenericHostDataSourceTest::testGet4ByIdentifier(const Host::IdentifierType& identifier_type) { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); - HostPtr host1 = initializeHost4("192.0.2.1", false); - HostPtr host2 = initializeHost4("192.0.2.2", false); + HostPtr host1 = initializeHost4("192.0.2.1", identifier_type); + HostPtr host2 = initializeHost4("192.0.2.2", identifier_type); - // Sanity check: make sure the hosts have different client-ids. - ASSERT_TRUE(host1->getDuid()); - ASSERT_TRUE(host2->getDuid()); - compareDuids(host1, host2, false); + // Sanity check: make sure the hosts have different identifiers.. + ASSERT_FALSE(host1->getIdentifier() == host2->getIdentifier()); // Try to add both of them to the host data source. ASSERT_NO_THROW(hdsptr_->add(host1)); @@ -457,12 +401,12 @@ void GenericHostDataSourceTest::testGet4ByClientId() { SubnetID subnet2 = host2->getIPv4SubnetID(); ConstHostPtr from_hds1 = hdsptr_->get4(subnet1, - Host::IDENT_DUID, + identifier_type, &host1->getIdentifier()[0], host1->getIdentifier().size()); ConstHostPtr from_hds2 = hdsptr_->get4(subnet2, - Host::IDENT_DUID, + identifier_type, &host2->getIdentifier()[0], host2->getIdentifier().size()); @@ -478,7 +422,7 @@ void GenericHostDataSourceTest::testHWAddrNotClientId() { ASSERT_TRUE(hdsptr_); // Create a host with HW address - HostPtr host = initializeHost4("192.0.2.1", true); + HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_HWADDR); ASSERT_TRUE(host->getHWAddress()); ASSERT_FALSE(host->getDuid()); @@ -509,7 +453,7 @@ void GenericHostDataSourceTest::testClientIdNotHWAddr() { ASSERT_TRUE(hdsptr_); // Create a host with client-id - HostPtr host = initializeHost4("192.0.2.1", false); + HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_DUID); ASSERT_FALSE(host->getHWAddress()); ASSERT_TRUE(host->getDuid()); @@ -553,7 +497,7 @@ GenericHostDataSourceTest::testHostname(std::string name, int num) { addr = IOAddress::increase(addr); - HostPtr host = initializeHost4(addr.toText(), false); + HostPtr host = initializeHost4(addr.toText(), Host::IDENT_DUID); stringstream hostname; hostname.str(""); @@ -589,12 +533,13 @@ GenericHostDataSourceTest::testHostname(std::string name, int num) { } void -GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) { +GenericHostDataSourceTest::testMultipleSubnets(int subnets, + const Host::IdentifierType& id) { // Make sure we have a pointer to the host data source. ASSERT_TRUE(hdsptr_); - HostPtr host = initializeHost4("192.0.2.1", hwaddr); + HostPtr host = initializeHost4("192.0.2.1", id); for (int i = 0; i < subnets; ++i) { host->setIPv4SubnetID(i + 1000); @@ -616,8 +561,7 @@ GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) { // Try to retrieve the host by either HW address of client-id from_hds = hdsptr_->get4(i + 1000, - hwaddr ? Host::IDENT_HWADDR : Host::IDENT_DUID, - &host->getIdentifier()[0], + id, &host->getIdentifier()[0], host->getIdentifier().size()); ASSERT_TRUE(from_hds); EXPECT_EQ(i + 1000, from_hds->getIPv4SubnetID()); @@ -637,8 +581,7 @@ GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) { // Finally, check that the hosts can be retrived by HW address or DUID ConstHostCollection all_by_id = - hdsptr_->getAll(hwaddr ? Host::IDENT_HWADDR : Host::IDENT_DUID, - &host->getIdentifier()[0], + hdsptr_->getAll(id, &host->getIdentifier()[0], host->getIdentifier().size()); ASSERT_EQ(subnets, all_by_id.size()); @@ -656,8 +599,8 @@ void GenericHostDataSourceTest::testGet6ByHWAddr() { ASSERT_TRUE(hdsptr_); // Create a host reservations. - HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_HWADDR, true); - HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_HWADDR, true); + HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_HWADDR, "hw-address"); + HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_HWADDR, "hw-address"); // Sanity check: make sure the hosts have different HW addresses. ASSERT_TRUE(host1->getHWAddress()); @@ -692,8 +635,8 @@ void GenericHostDataSourceTest::testGet6ByClientId() { ASSERT_TRUE(hdsptr_); // Create a host reservations. - HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_DUID, true); - HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, true); + HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_DUID, "hw-address"); + HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, "hw-address"); // Sanity check: make sure the hosts have different HW addresses. ASSERT_TRUE(host1->getDuid()); @@ -848,7 +791,7 @@ void GenericHostDataSourceTest::testAddDuplicate4() { ASSERT_TRUE(hdsptr_); // Create a host reservations. - HostPtr host = initializeHost4("192.0.2.1", false); + HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_DUID); // Add this reservation once. ASSERT_NO_THROW(hdsptr_->add(host)); diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h index e2b468dbc8..d9f50ee357 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h @@ -39,10 +39,11 @@ public: /// @brief Creates a host reservation for specified IPv4 address. /// /// @param address IPv4 address to be set - /// @param hwaddr type of identifier (true = hwaddr, false = client-id) + /// @param id Identifier type. /// /// @return generated Host object - HostPtr initializeHost4(std::string address, bool hwaddr); + HostPtr initializeHost4(const std::string& address, + const Host::IdentifierType& id); /// @brief Creates a host reservation for specified IPv6 address. /// @@ -61,14 +62,14 @@ public: /// @param increase A boolean value indicating if new address (increased) /// must be generated or the same address as previously. /// @return HW address in textual form acceptable by Host constructor - std::string generateHWAddr(const bool new_identifier = true); + std::vector generateHWAddr(const bool new_identifier = true); - /// @brief Generates a hardware address in text version. + /// @brief Generates a host identifier in a textual form.. /// - /// @param increase A boolean value indicating if new DUID (increased) - /// must be generated or the same DUID as previously. - /// @return DUID in textual form acceptable by Host constructor - std::string generateDuid(const bool new_identifier = true); + /// @param increase A boolean value indicating if new identifier (increased) + /// must be generated or the same identifier as previously. + /// @return Identifier in textual form acceptable by Host constructor + std::vector generateIdentifier(const bool new_identifier = true); /// @brief Compares hardware addresses of the two hosts. /// @@ -130,25 +131,20 @@ public: /// can be inserted and later retrieved. /// /// Uses gtest macros to report failures. - /// @param hwaddr true = use HW address as identifier, false = use client-id(DUID) - void testBasic4(bool hwaddr); + /// @param id Identifier type. + void testBasic4(const Host::IdentifierType& id); /// @brief Test inserts several hosts with unique IPv4 address and /// checks that they can be retrieved properly. /// /// Uses gtest macros to report failures. - /// @param hwaddr true = use HW address as identifier, false = use client-id(DUID) - void testGetByIPv4(bool hwaddr); - - /// @brief Test that hosts can be retrieved by hardware address. - /// - /// Uses gtest macros to report failures. - void testGet4ByHWAddr(); + /// @param id Identifier type. + void testGetByIPv4(const Host::IdentifierType& id); - /// @brief Test that hosts can be retrieved by client-id + /// @brief Test that hosts can be retrieved by host identifier. /// /// Uses gtest macros to report failures. - void testGet4ByClientId(); + void testGet4ByIdentifier(const Host::IdentifierType& identifier_type); /// @brief Test that clients with stored HW address can't be retrieved /// by DUID with the same value. @@ -184,8 +180,8 @@ public: /// Uses gtest macros to report failures. /// /// @param subnets number of subnets to test - /// @param hwaddr true = use HW address, false = use client-id - void testMultipleSubnets(int subnets, bool hwaddr); + /// @param id Host identifier type. + void testMultipleSubnets(int subnets, const Host::IdentifierType& id); /// @brief Test inserts several hosts with unique IPv6 addresses and /// checks that they can be retrieved properly. diff --git a/src/lib/dhcpsrv/tests/host_unittest.cc b/src/lib/dhcpsrv/tests/host_unittest.cc index 5d99b733eb..8ad46663ea 100644 --- a/src/lib/dhcpsrv/tests/host_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_unittest.cc @@ -159,24 +159,6 @@ public: return (false); } - /// @brief Converts identifier type to its name. - /// - /// @param type identifier type. - /// @return Identifier name as specified in a configuration file. - std::string identifierName(const Host::IdentifierType& type) const { - switch (type) { - case Host::IDENT_HWADDR: - return ("hw-address"); - case Host::IDENT_DUID: - return ("duid"); - case Host::IDENT_CIRCUIT_ID: - return ("circuit-id"); - default: - ; - } - return ("unknown"); - } - /// @brief Returns upper bound of the supported identifier types. /// /// Some unit tests verify the @c Host class behavior for all @@ -364,7 +346,7 @@ TEST_F(HostTest, createFromIdentifierHex) { const std::string identifier_hex = (hwaddr ? hwaddr->toText(false) : util::encode::encodeHex(identifier)); - const std::string identifier_name = identifierName(type); + const std::string identifier_name = Host::getIdentifierName(type); // Try to create Host instance. ASSERT_NO_THROW(host.reset(new Host(identifier_hex, identifier_name, @@ -394,7 +376,7 @@ TEST_F(HostTest, createFromIdentifierString) { // It is not allowed to specify HW address or DUID as a string in quotes. for (unsigned int i = 2; i < identifierTypeUpperBound(); ++i) { const Host::IdentifierType type = static_cast(i); - const std::string identifier_name = identifierName(type); + const std::string identifier_name = Host::getIdentifierName(type); // Construct unique identifier for a host. This is a string // consisting of a word "idenetifier", hyphen and the name of @@ -462,7 +444,7 @@ TEST_F(HostTest, setIdentifierHex) { std::string identifier_hex = (hwaddr ? hwaddr->toText(false) : util::encode::encodeHex(identifier)); - std::string identifier_name = identifierName(type); + std::string identifier_name = Host::getIdentifierName(type); // Try to create Host instance. ASSERT_NO_THROW(host.reset(new Host(identifier_hex, identifier_name, @@ -497,7 +479,7 @@ TEST_F(HostTest, setIdentifierHex) { // Convert identifier to hexadecimal representation. identifier_hex = (hwaddr ? hwaddr->toText(false) : util::encode::encodeHex(identifier)); - identifier_name = identifierName(type); + identifier_name = Host::getIdentifierName(type); // Try to replace identifier for a host. ASSERT_NO_THROW(host->setIdentifier(identifier_hex, identifier_name)) @@ -892,6 +874,13 @@ TEST_F(HostTest, getIdentifierAsText) { host3.getIdentifierAsText()); } +// This test verifies that conversion of the identifier type to a +// name works correctly. +TEST_F(HostTest, getIdentifierName) { + EXPECT_EQ("hw-address", Host::getIdentifierName(Host::IDENT_HWADDR)); + +} + // This test checks that Host object is correctly described in the // textual format using the toText method. TEST_F(HostTest, toText) { 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 d3a0c3af92..ced78e34a0 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -190,37 +190,43 @@ TEST(MySqlConnection, checkTimeConversion) { // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses hw address as identifier. TEST_F(MySqlHostDataSourceTest, basic4HWAddr) { - testBasic4(true); + testBasic4(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) { - testBasic4(false); + testBasic4(Host::IDENT_DUID); } // Test verifies that multiple hosts can be added and later retrieved by their // reserved IPv4 address. This test uses HW addresses as identifiers. TEST_F(MySqlHostDataSourceTest, getByIPv4HWaddr) { - testGetByIPv4(true); + testGetByIPv4(Host::IDENT_HWADDR); } // Test verifies that multiple hosts can be added and later retrieved by their // reserved IPv4 address. This test uses client-id (DUID) as identifiers. TEST_F(MySqlHostDataSourceTest, getByIPv4ClientId) { - testGetByIPv4(false); + testGetByIPv4(Host::IDENT_DUID); } // Test verifies if a host reservation can be added and later retrieved by // hardware address. TEST_F(MySqlHostDataSourceTest, get4ByHWaddr) { - testGet4ByHWAddr(); + testGet4ByIdentifier(Host::IDENT_HWADDR); } // Test verifies if a host reservation can be added and later retrieved by -// client identifier. -TEST_F(MySqlHostDataSourceTest, get4ByClientId) { - testGet4ByClientId(); +// DUID. +TEST_F(MySqlHostDataSourceTest, get4ByDUID) { + testGet4ByIdentifier(Host::IDENT_DUID); +} + +// Test verifies if a host reservation can be added and later retrieved by +// circuit id. +TEST_F(MySqlHostDataSourceTest, get4ByCircuitId) { + testGet4ByIdentifier(Host::IDENT_CIRCUIT_ID); } // Test verifies if hardware address and client identifier are not confused. @@ -355,7 +361,7 @@ TEST_F(MySqlHostDataSourceTest, DISABLED_multipleClientClassesBoth) { // hardware address), but for different subnets (different subnet-ids). // Make sure that getAll() returns them all correctly. TEST_F(MySqlHostDataSourceTest, multipleSubnetsHWAddr) { - testMultipleSubnets(10, true); + testMultipleSubnets(10, Host::IDENT_HWADDR); } // Test if the same host can have reservations in different subnets (with the @@ -365,7 +371,7 @@ TEST_F(MySqlHostDataSourceTest, multipleSubnetsHWAddr) { // client-identifier), but for different subnets (different subnet-ids). // Make sure that getAll() returns them correctly. TEST_F(MySqlHostDataSourceTest, multipleSubnetsClientId) { - testMultipleSubnets(10, false); + testMultipleSubnets(10, Host::IDENT_DUID); } // Test if host reservations made for different IPv6 subnets are handled correctly.