[#1790] addressed review comments

Author: Razvan Becheriu

src/bin/dhcp4/tests/config_backend_unittest.cc

@@ -192,8 +192,8 @@ TEST_F(Dhcp4CBTest, mergeGlobals) {
     StampedValuePtr calc_tee_times(new StampedValue("calculate-tee-times", Element::create(bool(false))));
     StampedValuePtr t2_percent(new StampedValue("t2-percent", Element::create(0.75)));
     StampedValuePtr renew_timer(new StampedValue("renew-timer", Element::create(500)));
-    StampedValuePtr mt_enabled(new StampedValue("multi-threading/enable-multi-threading", Element::create(true)));
-    StampedValuePtr mt_pool_size(new StampedValue("multi-threading/thread-pool-size", Element::create(256)));
+    StampedValuePtr mt_enabled(new StampedValue("multi-threading.enable-multi-threading", Element::create(true)));
+    StampedValuePtr mt_pool_size(new StampedValue("multi-threading.thread-pool-size", Element::create(256)));
 
     // Let's add all of the globals to the second backend.  This will verify
     // we find them there.

src/bin/dhcp6/tests/config_backend_unittest.cc

@@ -192,8 +192,8 @@ TEST_F(Dhcp6CBTest, mergeGlobals) {
     StampedValuePtr server_tag(new StampedValue("server-tag", "second-server"));
     StampedValuePtr decline_period(new StampedValue("decline-probation-period", Element::create(86400)));
     StampedValuePtr renew_timer(new StampedValue("renew-timer", Element::create(500)));
-    StampedValuePtr mt_enabled(new StampedValue("multi-threading/enable-multi-threading", Element::create(true)));
-    StampedValuePtr mt_pool_size(new StampedValue("multi-threading/thread-pool-size", Element::create(256)));
+    StampedValuePtr mt_enabled(new StampedValue("multi-threading.enable-multi-threading", Element::create(true)));
+    StampedValuePtr mt_pool_size(new StampedValue("multi-threading.thread-pool-size", Element::create(256)));
 
     // Let's add all of the globals to the second backend.  This will verify
     // we find them there.

src/lib/cc/stamped_value.cc

@@ -187,7 +187,7 @@ StampedValue::validateConstruct() const {
                 (type != Element::boolean) &&
                 (type != Element::real)) {
                 isc_throw(BadValue, "StampedValue: provided value of the '"
-                          << name_ << "/" << value_->mapValue().begin()->first
+                          << name_ << "." << value_->mapValue().begin()->first
                           << "' parameter has invalid type: "
                           << Element::typeToName(type));
             }

src/lib/dhcpsrv/cb_ctl_dhcp.h

@@ -32,10 +32,34 @@ class CBControlDHCP : public process::CBControlBase<ConfigBackendMgrType> {
         : process::CBControlBase<ConfigBackendMgrType>() {
     }
 
+    /// @brief It translates the top level map parameters from flat naming
+    /// format (e.g. param-name.sub-param-name) to the respective param-name and
+    /// sub-param-name. If the name does not contain '.', the param-name will
+    /// contain the initial name.
+    ///
+    /// @param name The name in flat format (e.g. map-name.element-name).
+    /// @param[out] param_name The resulting top level param name.
+    /// @param[out] sub_param_name The resulting sub param name inside the map.
+    ///
+    /// @return The function returns true if any conversion is done, false
+    /// otherwise.
+    static bool translateName(std::string const& name, std::string& param_name,
+                              std::string& sub_param_name) {
+        param_name = name;
+        sub_param_name = std::string();
+        auto pos = param_name.find('.');
+        if (pos != std::string::npos) {
+            sub_param_name = param_name.substr(pos + 1);
+            param_name = param_name.substr(0, pos);
+            return (true);
+        }
+        return (false);
+    }
+
 protected:
 
     /// @brief It translates the top level map parameters from flat naming
-    /// format (e.g. map-name/element-name) to proper ElementMap objects and
+    /// format (e.g. param-name.sub-param-name) to proper ElementMap objects and
     /// adds all globals fetched from config backend(s) to a SrvConfig instance
     ///
     /// Iterates over the given collection of global parameters and adds them to
@@ -54,20 +78,18 @@ class CBControlDHCP : public process::CBControlBase<ConfigBackendMgrType> {
                 continue;
             }
 
-            std::string name = cb_global->getName();
-            auto pos = name.find('/');
-            if (pos != std::string::npos) {
-                const std::string sub_elem(name.substr(pos + 1));
-                name = name.substr(0, pos);
-                data::ElementPtr sub_param = boost::const_pointer_cast<data::Element>(external_cfg->getConfiguredGlobal(name));
+            std::string param_name;
+            std::string sub_param_name;
+            if (translateName(cb_global->getName(), param_name, sub_param_name)) {
+                data::ElementPtr sub_param = boost::const_pointer_cast<data::Element>(external_cfg->getConfiguredGlobal(param_name));
                 if (!sub_param) {
                     sub_param = data::Element::createMap();
                 }
-                sub_param->set(sub_elem, cb_global->getElementValue());
-                external_cfg->addConfiguredGlobal(name, sub_param);
+                sub_param->set(sub_param_name, cb_global->getElementValue());
+                external_cfg->addConfiguredGlobal(param_name, sub_param);
             } else {
                 // Reuse name and value.
-                external_cfg->addConfiguredGlobal(name, cb_global->getElementValue());
+                external_cfg->addConfiguredGlobal(param_name, cb_global->getElementValue());
             }
         }
     }

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1660,6 +1660,7 @@ D2ClientConfigParser::setAllDefaults(isc::data::ConstElementPtr d2_config) {
 void
 CompatibilityParser::parse(ConstElementPtr compatibility, SrvConfig& srv_cfg) {
     if (compatibility) {
+        auto family = CfgMgr::instance().getFamily();
         for (auto const& kv : compatibility->mapValue()) {
             if (!kv.second || (kv.second->getType() != Element::boolean)) {
                 isc_throw(DhcpConfigError,
@@ -1669,12 +1670,19 @@ CompatibilityParser::parse(ConstElementPtr compatibility, SrvConfig& srv_cfg) {
             }
             if (kv.first == "lenient-option-parsing") {
                 srv_cfg.setLenientOptionParsing(kv.second->boolValue());
-            } else if (kv.first == "ignore-dhcp-server-identifier") {
-                srv_cfg.setIgnoreServerIdentifier(kv.second->boolValue());
-            } else if (kv.first == "ignore-rai-link-selection") {
-                srv_cfg.setIgnoreRAILinkSelection(kv.second->boolValue());
-            } else if (kv.first == "exclude-first-last-24") {
-                srv_cfg.setExcludeFirstLast24(kv.second->boolValue());
+            } else if (family == AF_INET) {
+                if (kv.first == "ignore-dhcp-server-identifier") {
+                    srv_cfg.setIgnoreServerIdentifier(kv.second->boolValue());
+                } else if (kv.first == "ignore-rai-link-selection") {
+                    srv_cfg.setIgnoreRAILinkSelection(kv.second->boolValue());
+                } else if (kv.first == "exclude-first-last-24") {
+                    srv_cfg.setExcludeFirstLast24(kv.second->boolValue());
+                } else {
+                    isc_throw(DhcpConfigError,
+                              "unsupported compatibility parameter: "
+                              << kv.first << " (" << kv.second->getPosition()
+                              << ")");
+                }
             } else {
                 isc_throw(DhcpConfigError,
                           "unsupported compatibility parameter: "

src/lib/dhcpsrv/testutils/generic_backend_unittest.cc

@@ -8,6 +8,7 @@
 
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option_vendor.h>
+#include <dhcpsrv/cb_ctl_dhcp.h>
 #include <dhcpsrv/testutils/generic_backend_unittest.h>
 #include <util/buffer.h>
 #include <typeinfo>
@@ -115,22 +116,18 @@ GenericBackendTest::checkConfiguredGlobal(const SrvConfigPtr& srv_cfg,
                                           const std::string &name,
                                           ConstElementPtr exp_value) {
     ConstCfgGlobalsPtr globals = srv_cfg->getConfiguredGlobals();
-    std::string name_elem = name;
-    std::string sub_elem;
-    auto pos = name_elem.find('/');
-    if (pos != std::string::npos) {
-        sub_elem = name_elem.substr(pos + 1);
-        name_elem = name_elem.substr(0, pos);
-    }
+    std::string param_name;
+    std::string sub_param_name;
+    bool translated = CBControlDHCP<bool>::translateName(name, param_name, sub_param_name);
 
-    ConstElementPtr found_global = globals->get(name_elem);
+    ConstElementPtr found_global = globals->get(param_name);
     ASSERT_TRUE(found_global) << "expected global: "
-                              << name_elem << " not found";
+                              << param_name << " not found";
 
-    if (!sub_elem.empty()) {
+    if (translated) {
         ASSERT_EQ(Element::map, found_global->getType())
-            << "expected global: " << name_elem << " has wrong type";
-        found_global = found_global->get(sub_elem);
+            << "expected global: " << param_name << " has wrong type";
+        found_global = found_global->get(sub_param_name);
         ASSERT_TRUE(found_global) << "expected global: "
                                   << name << " not found";
     }

src/share/api/remote-global-parameter4-del.json

@@ -5,7 +5,7 @@
         "This command deletes a global DHCPv4 parameter from the configuration database. The server uses the value specified in the configuration file, or a default value if the parameter is not specified, after deleting the parameter from the database."
     ],
     "cmd-comment": [
-        "This command carries the list including exactly one name of the parameter to be deleted. If deleting a map parameter, the ``map-name/parameter-name`` format must be used. The ``server-tags`` list is mandatory and it must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error."
+        "This command carries the list including exactly one name of the parameter to be deleted. If deleting a map parameter, the ``map-name.parameter-name`` format must be used. The ``server-tags`` list is mandatory and it must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error."
     ],
     "cmd-syntax": [
         "{",

src/share/api/remote-global-parameter4-get.json

@@ -5,7 +5,7 @@
         "This command fetches the selected global parameter for the server from the specified database."
     ],
     "cmd-comment": [
-        "This command carries a list including exactly one name of the parameter to be fetched. If retrieving a map parameter, the ``map-name/parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error. The server tag \"all\" is allowed; it fetches the global parameter value shared by all servers."
+        "This command carries a list including exactly one name of the parameter to be fetched. If retrieving a map parameter, the ``map-name.parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error. The server tag \"all\" is allowed; it fetches the global parameter value shared by all servers."
     ],
     "cmd-syntax": [
         "{",

src/share/api/remote-global-parameter6-del.json

@@ -5,7 +5,7 @@
         "This command deletes a global DHCPv6 parameter from the configuration database. The server uses the value specified in the configuration file, or a default value if the parameter is not specified in the configuration file, after deleting the parameter from the database."
     ],
     "cmd-comment": [
-        "This command carries the list including exactly one name of the parameter to be deleted. If deleting a map parameter, the ``map-name/parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error."
+        "This command carries the list including exactly one name of the parameter to be deleted. If deleting a map parameter, the ``map-name.parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error."
     ],
     "cmd-syntax": [
         "{",

src/share/api/remote-global-parameter6-get.json

@@ -5,7 +5,7 @@
         "This command fetches the selected global parameter for the server from the specified database."
     ],
     "cmd-comment": [
-        "This command carries a list including exactly one name of the parameter to be fetched. If retrieving a map parameter, the ``map-name/parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error. The server tag \"all\" is allowed; it fetches the global parameter value shared by all servers."
+        "This command carries a list including exactly one name of the parameter to be fetched. If retrieving a map parameter, the ``map-name.parameter-name`` format must be used. The ``server-tags`` list is mandatory and must contain exactly one server tag. Specifying an empty list, a value of ``null``, or multiple server tags will result in an error. The server tag \"all\" is allowed; it fetches the global parameter value shared by all servers."
     ],
     "cmd-syntax": [
         "{",