From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 10 Jun 2019 11:17:39 +0000 (+0200)
Subject: [#641,!352] Addressed review comments.
X-Git-Tag: Kea-1.6.0-beta2~315
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=39c51fde2fe965198ce01c5c6bb8af40017faae8;p=thirdparty%2Fkea.git

[#641,!352] Addressed review comments.
---

diff --git a/src/lib/cc/server_tag.cc b/src/lib/cc/server_tag.cc
index 931d2561d4..e99d75bef4 100644
--- a/src/lib/cc/server_tag.cc
+++ b/src/lib/cc/server_tag.cc
@@ -7,6 +7,7 @@
 #include <cc/server_tag.h>
 #include <exceptions/exceptions.h>
 #include <util/strutil.h>
+#include <boost/algorithm/string.hpp>
 
 namespace isc {
 namespace data {
@@ -25,6 +26,8 @@ ServerTag::ServerTag(const std::string& tag)
     } else if (tag_.length() > 256) {
         isc_throw(BadValue, "server-tag must not be longer than 256 characters");
     }
+
+    boost::algorithm::to_lower(tag_);
 }
 
 bool
diff --git a/src/lib/cc/server_tag.h b/src/lib/cc/server_tag.h
index 95502ae898..c85fc1bd5c 100644
--- a/src/lib/cc/server_tag.h
+++ b/src/lib/cc/server_tag.h
@@ -33,7 +33,8 @@ public:
 
     /// @brief Constructor.
     ///
-    /// @param tag server tag provided as string.
+    /// @param tag server tag provided as string. The tag is converted to
+    /// lower case.
     explicit ServerTag(const std::string& tag);
 
     /// @brief Checks if the server tag is set to "all servers".
@@ -41,7 +42,10 @@ public:
     /// @return true if the server tag is set to all servers, false
     /// otherwise.
     bool amAll() const;
+
     /// @brief Returns server tag as string.
+    ///
+    /// @return lower case server tag.
     std::string get() const {
         return (tag_);
     }
diff --git a/src/lib/cc/tests/server_tag_unittest.cc b/src/lib/cc/tests/server_tag_unittest.cc
index 95cfd4f393..f782a065a0 100644
--- a/src/lib/cc/tests/server_tag_unittest.cc
+++ b/src/lib/cc/tests/server_tag_unittest.cc
@@ -62,6 +62,13 @@ TEST(ServerTagTest, constructors) {
         EXPECT_EQ("both left-right", tag->get());
         EXPECT_FALSE(tag->amAll());
     }
+
+    {
+        SCOPED_TRACE("upper to lower case");
+        ASSERT_NO_THROW(tag.reset(new ServerTag("UPPER CASE TAG")));
+        EXPECT_EQ("upper case tag", tag->get());
+        EXPECT_FALSE(tag->amAll());
+    }
 }
 
 // This test verifies that malformed server tags are rejected.
diff --git a/src/lib/config_backend/base_config_backend_pool.h b/src/lib/config_backend/base_config_backend_pool.h
index 2fc67b5164..337f332f65 100644
--- a/src/lib/config_backend/base_config_backend_pool.h
+++ b/src/lib/config_backend/base_config_backend_pool.h
@@ -401,7 +401,6 @@ protected:
     ///
     /// @param MethodPointer Pointer to the backend method to be called.
     /// @param backend_selector Backend selector.
-    /// @param server_selector Server selector.
     /// @param [out] properties Reference to the collection of retrieved properties.
     ///
     /// @throw db::NoSuchDatabase if no database matching the given selector
diff --git a/src/lib/database/server.h b/src/lib/database/server.h
index aa4e5b27c3..fc1d84f4bf 100644
--- a/src/lib/database/server.h
+++ b/src/lib/database/server.h
@@ -19,7 +19,7 @@ class Server;
 /// @brief Shared pointer to the @c Server class.
 typedef boost::shared_ptr<Server> ServerPtr;
 
-/// @brief Represents information about the Kea server in the database.
+/// @brief Represents information about a Kea server in the database.
 ///
 /// The configuration backend holds the information about the servers
 /// for which the backend holds the configuration information. The
@@ -33,14 +33,14 @@ public:
     /// @brief Constructor.
     ///
     /// @param tag server tag.
-    /// @param server description.
+    /// @param description server description.
     Server(const data::ServerTag& tag, const std::string& description);
 
     /// @brief Factory function to be used to create an instance of the
     /// @c Server object.
     ///
     /// @param tag server tag.
-    /// @param server description.
+    /// @param description server description.
     ///
     /// @return Pointer to the server instance.
     /// @throw BadValue if the server tag exceeds 256 characters or the
diff --git a/src/lib/database/tests/server_unittest.cc b/src/lib/database/tests/server_unittest.cc
index 75e96b20b6..b9a4def5bd 100644
--- a/src/lib/database/tests/server_unittest.cc
+++ b/src/lib/database/tests/server_unittest.cc
@@ -37,9 +37,13 @@ TEST(ServerTest, tooLongDescription) {
 // Tests that it is possible to fetch server by tag fromn the collection.
 TEST(ServerFetcherTest, getByTag) {
     ServerCollection servers;
-    servers.insert(Server::create(ServerTag("alpha"), "alpha description"));
-    servers.insert(Server::create(ServerTag("beta"), "beta description"));
-    servers.insert(Server::create(ServerTag("gamma"), "gamma description"));
+
+    EXPECT_TRUE(servers.insert(Server::create(ServerTag("alpha"), "alpha description")).second);
+    EXPECT_TRUE(servers.insert(Server::create(ServerTag("beta"), "beta description")).second);
+    EXPECT_TRUE(servers.insert(Server::create(ServerTag("gamma"), "gamma description")).second);
+
+    // Inserting an element with duplicated server tag should be unsuccessful.
+    EXPECT_FALSE(servers.insert(Server::create(ServerTag("gamma"), "gamma 2 description")).second);
 
     auto alpha = ServerFetcher::get(servers, ServerTag("alpha"));
     ASSERT_TRUE(alpha);