[#32, !23] Addressed review comments

diff --git a/src/lib/config/Makefile.am b/src/lib/config/Makefile.am
index 2199752b732d13a010577f9c388b6d7874dcaeaf..8b2102dfd208fe535b15fd925e6517d27482938a 100644 (file)
--- a/src/lib/config/Makefile.am
+++ b/src/lib/config/Makefile.am
@@ -24,7 +24,6 @@ libkea_cfgclient_la_SOURCES += hooked_command_mgr.cc hooked_command_mgr.h
 libkea_cfgclient_la_SOURCES += timeouts.h
 
 libkea_cfgclient_la_LIBADD = $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
-libkea_cfgclient_la_LIBADD += $(top_builddir)/src/lib/database/libkea-database.la
 libkea_cfgclient_la_LIBADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
 libkea_cfgclient_la_LIBADD += $(top_builddir)/src/lib/cc/libkea-cc.la
 libkea_cfgclient_la_LIBADD += $(top_builddir)/src/lib/dns/libkea-dns++.la

diff --git a/src/lib/process/config_ctl_info.cc b/src/lib/process/config_ctl_info.cc
index 8d9ab66978dfe1b0c0c1235077ccf3c0b4503819..7cd9ad02f3a3a5afe72815406842ffee782ee261 100644 (file)
--- a/src/lib/process/config_ctl_info.cc
+++ b/src/lib/process/config_ctl_info.cc
@@ -13,7 +13,7 @@ namespace isc {
 namespace process {
 
 void
-ConfigDbInfo::setAccessString(const std::string access_str) {
+ConfigDbInfo::setAccessString(const std::string& access_str) {
     access_str_ = access_str;
     access_params_.clear();
     access_params_ = db::DatabaseConnection::parse(access_str_);
@@ -65,8 +65,8 @@ ConfigControlInfo::addConfigDatabase(const std::string& access_str) {
 }
 
 const ConfigDbInfo&
-ConfigControlInfo::findConfigDb(const std::string param_name,
-                                const std::string param_value) {
+ConfigControlInfo::findConfigDb(const std::string& param_name,
+                                const std::string& param_value) {
     for (ConfigDbInfoList::iterator db = db_infos_.begin();
          db != db_infos_.end(); ++db) {
         std::string db_value;

diff --git a/src/lib/process/config_ctl_info.h b/src/lib/process/config_ctl_info.h
index 27ff7bb46ae5612142a12d78f7818e6bb1d1b8d3..52aeab2ad3d80df6b9695a89e71fd95356285901 100644 (file)
--- a/src/lib/process/config_ctl_info.h
+++ b/src/lib/process/config_ctl_info.h
@@ -34,7 +34,7 @@ public:
     /// ensuring the validity of the string content is placed upon the caller.
     ///
     /// @param access_str string of name=value pairs seperated by spaces
-    void setAccessString(const std::string access_str);
+    void setAccessString(const std::string& access_str);
 
     /// @brief Retrieves the database access string.
     ///
@@ -107,7 +107,7 @@ typedef std::vector<ConfigDbInfo> ConfigDbInfoList;
 /// This is class conveys the configuration control information
 /// described by the following JSON text:
 ///
-/// "config-ctl" :
+/// "config-control" :
 /// {
 ///     "config-databases":
 ///     [
@@ -124,7 +124,6 @@ typedef std::vector<ConfigDbInfo> ConfigDbInfoList;
 ///     }
 ///     ]
 /// }
-
 class ConfigControlInfo : public isc::data::CfgToElement {
 public:
 
@@ -134,16 +133,19 @@ public:
     /// @brief Copy Constructor.
     ConfigControlInfo(const ConfigControlInfo& other);
 
-    /// @brief Sets host database access string.
+    /// @brief Sets configuration database access string.
+    ///
+    /// @param access_str database access string.
     ///
-    /// @param host_db_access New host database access string.
-    /// @param front Add at front if true, at back if false (default).
     /// @throw BadValue if an entry exists that matches the parameters
     /// in the given access string, or if the access string is invalid.
     void addConfigDatabase(const std::string& access_str);
 
     /// @brief Retrieves the list of databases
     ///
+    /// The entries in the list are stored in the order they were
+    /// added to it (FIFO).
+    ///
     /// @return a reference to a const list of databases
     const ConfigDbInfoList& getConfigDatabases() const {
         return (db_infos_);
@@ -153,8 +155,8 @@ public:
     ///
     /// @return A reference to the matching database or the not-found value
     /// available via @c EMPTY_DB()
-    const ConfigDbInfo& findConfigDb(const std::string param_name,
-                                     const std::string param_value);
+    const ConfigDbInfo& findConfigDb(const std::string& param_name,
+                                     const std::string& param_value);
 
     /// @brief Empties the contents of the class, including the database list
     void clear();

diff --git a/src/lib/process/config_ctl_parser.cc b/src/lib/process/config_ctl_parser.cc
index fe017a81e745ad366b8ba2a65a100d4468f733d4..a27228e25df9696fb6ef544f7755a831740b1c9d 100644 (file)
--- a/src/lib/process/config_ctl_parser.cc
+++ b/src/lib/process/config_ctl_parser.cc
@@ -23,28 +23,24 @@ ConfigControlParser::parse(const data::ConstElementPtr& config_control) {
 
     try {
         if (config_control->contains("config-databases")) {
-            const std::vector<data::ElementPtr>& db_list =
-                config_control->get("config-databases")->listValue();
 
+            auto elem = config_control->get("config-databases");
+            if (elem->getType() != Element::list) {
+                isc_throw (ConfigError, "config-databases must be a list ("
+                           << elem->getPosition() << ")");
+            }
+
+            const std::vector<data::ElementPtr>& db_list = elem->listValue();
             for (auto db = db_list.cbegin(); db != db_list.end(); ++db) {
                 db::DbAccessParser parser;
                 std::string access_string;
                 parser.parse(access_string, *db);
-                // @todo do we still need access_string for this at all?
-                // can't we just use params directly and get rid of the
-                // string now that DatabaseConnection::toElement(map) exists?
+                /// @todo do we still need access_string for this at all?
+                /// can't we just use params directly and get rid of the
+                /// string now that DatabaseConnection::toElement(map) exists?
                 ctl_info->addConfigDatabase(access_string);
             }
         }
-
-#if 0
-        // @todo Should it have user_context and what about comment?
-        ConstElementPtr user_context = shared_network_data->get("user-context");
-        if (user_context) {
-            shared_network->setContext(user_context);
-        }
-#endif
-
     } catch (const isc::ConfigError&) {
         // Position was already added
         throw;