Browse Source

[4300] Addressed review comments.

Marcin Siodelski 9 years ago
parent
commit
7741ab8ce8

+ 23 - 10
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 <type>=<value> 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<uint8_t> 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,

+ 4 - 1
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 <type>=<value>.
     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.

+ 24 - 57
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<uint8_t>(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<void*>(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<void*>(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<void*>(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<char*>(&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<char*>(&dhcp_identifier_type_);
-                bind_[2].is_unsigned = MLM_TRUE;
-            }
+            dhcp_identifier_type_ = static_cast<uint8_t>(host->getIdentifierType());
+            bind_[2].buffer_type = MYSQL_TYPE_TINY;
+            bind_[2].buffer = reinterpret_cast<char*>(&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<int>(dhcp_identifier_type_)
-                      << ". Only 0 or 1 are supported.");
+                      << static_cast<int>(dhcp_identifier_type_));
         }
+        // Set the dhcp identifier type in a variable of the appropriate data type.
+        Host::IdentifierType type =
+            static_cast<Host::IdentifierType>(dhcp_identifier_type_);
 
         // Set DHCPv4 subnet ID to the value returned. If NULL returned, set to 0.
         SubnetID ipv4_subnet_id(0);

+ 51 - 108
src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc

@@ -26,7 +26,7 @@ GenericHostDataSourceTest::GenericHostDataSourceTest()
 GenericHostDataSourceTest::~GenericHostDataSourceTest() {
 }
 
-std::string
+std::vector<uint8_t>
 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<unsigned int>(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<uint8_t>(hwaddr, hwaddr + sizeof(hwaddr)));
 }
 
-std::string
-GenericHostDataSourceTest::generateDuid(const bool new_identifier) {
+std::vector<uint8_t>
+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<unsigned int>(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<uint8_t>(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<uint8_t> 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<uint8_t> 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));

+ 17 - 21
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<uint8_t> 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<uint8_t> 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.

+ 11 - 22
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<Host::IdentifierType>(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) {

+ 16 - 10
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.