[#402,!224] Addressed review comments

diff --git a/src/bin/dhcp4/tests/config_backend_unittest.cc b/src/bin/dhcp4/tests/config_backend_unittest.cc
index 59078ac8f6f8684b0ba9a42ca6bb2fc561715504..4a446cc92887736005167df027ec64d3e697030a 100644 (file)
--- a/src/bin/dhcp4/tests/config_backend_unittest.cc
+++ b/src/bin/dhcp4/tests/config_backend_unittest.cc
@@ -217,19 +217,19 @@ TEST_F(Dhcp4CBTest, mergeGlobals) {
     extractConfig(base_config);
 
     // Make some globals:
-    StampedValuePtr serverHostname(new StampedValue("server-hostname", "isc.example.org"));
-    StampedValuePtr declinePeriod(new StampedValue("decline-probation-period", Element::create(86400)));
-    StampedValuePtr calcTeeTimes(new StampedValue("calculate-tee-times", Element::create(bool(false))));
-    StampedValuePtr t2Percent(new StampedValue("t2-percent", Element::create(0.75)));
-    StampedValuePtr renewTimer(new StampedValue("renew-timer", Element::create(500)));
+    StampedValuePtr server_hostname(new StampedValue("server-hostname", "isc.example.org"));
+    StampedValuePtr decline_period(new StampedValue("decline-probation-period", Element::create(86400)));
+    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)));
 
     // Let's add all of the globals to the second backend.  This will verify
     // we find them there.
-    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), serverHostname);
-    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), declinePeriod);
-    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), calcTeeTimes);
-    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), t2Percent);
-    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), renewTimer);
+    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), server_hostname);
+    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), decline_period);
+    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), calc_tee_times);
+    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), t2_percent);
+    db2_->createUpdateGlobalParameter4(ServerSelector::ALL(), renew_timer);
 
     // Should parse and merge without error.
     ASSERT_NO_FATAL_FAILURE(configure(base_config, CONTROL_RESULT_SUCCESS, ""));
@@ -251,10 +251,10 @@ TEST_F(Dhcp4CBTest, mergeGlobals) {
     ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal("rebind-timer", Element::create(800)));
 
     // Verify that the implicit globals from the backend are there.
-    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(serverHostname));
-    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(calcTeeTimes));
-    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(t2Percent));
-    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(renewTimer));
+    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(server_hostname));
+    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(calc_tee_times));
+    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(t2_percent));
+    ASSERT_NO_FATAL_FAILURE(checkConfiguredGlobal(renew_timer));
 }
 
 // This test verifies that externally configured option definitions

diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc
index 6794dc1209141a373c156675a26a3f4e87100a89..162aedc17baa1d2d50776715b3a42f7d1d0fe804 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.cc
+++ b/src/lib/dhcpsrv/srv_config.cc
@@ -262,7 +262,6 @@ SrvConfig::extractConfiguredGlobals(isc::data::ConstElementPtr config) {
     for (auto value = values.begin(); value != values.end(); ++value) {
         if (value->second->getType() != Element::list &&
             value->second->getType() != Element::map) {
-                std::cout << "adding config'd global: " << value->first << std::endl;
                 addConfiguredGlobal(value->first, value->second);
         }
     }

diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h
index 54849ff272be1c579a54de84a62d964ed680de15..d1c4a91b27f2645ed91c74a26f1ae1df72a7253c 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.h
+++ b/src/lib/dhcpsrv/srv_config.h
@@ -668,14 +668,23 @@ private:
     /// @brief Merges the DHCPv4 globals specified in the given configuration
     /// into this configuration.
     ///
-    /// This function iterates over the given config's explicitly
-    /// configured globals and either adds them to or updates the value
-    /// of this config's configured globals.
-    /// 
-    /// It then iterates over the merged list, setting all of the
-    /// corresponding configuration member values (e.g. c@ 
-    /// SrvConfig::echo_client_id_, @c SrvConfig::server_tag_).
-    /// 
+    /// Configurable global values may be specified either via JSON
+    /// configuration (e.g. "echo-client-id":true) or as global parameters
+    /// within a configuration back end.  Regardless of the source, these
+    /// values once provided, are stored in @c SrvConfig::configured_globals_.
+    /// Any such value that does not have an explicit specification should be
+    /// considered "unspecified" at the global scope.
+    ///
+    /// This function adds the configured globals from the "other" config
+    /// into this config's configured globals.  If a value already exists
+    /// in this config, it will be overwritten with the value from the
+    /// "other" config.
+    ///
+    /// It then iterates over this merged list of globals, setting
+    /// any of the corresponding SrvConfig members that map to a
+    /// a configurable parameter (e.g. c@ SrvConfig::echo_client_id_,
+    /// @c SrvConfig::server_tag_).
+    ///
     /// @param other An object holding the configuration to be merged
     /// into this configuration.
     void mergeGlobals4(const SrvConfig& other);

diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc
index cff1d6b23d7178a8cbb1b8a965433aa91de9e262..f46a07a83b619acf2c31cd5bd354b0e279721961 100644 (file)
--- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc
+++ b/src/lib/dhcpsrv/tests/srv_config_unittest.cc
@@ -1055,7 +1055,8 @@ TEST_F(SrvConfigTest, mergeGlobals4) {
     ASSERT_NO_THROW(expected_globals = Element::fromJSON(exp_globals))
                     << "exp_globals didn't parse, test is broken";
 
-    EXPECT_TRUE(expected_globals->equals(*(cfg_to.getConfiguredGlobals())));
+    EXPECT_TRUE(isEquivalent(expected_globals, cfg_to.getConfiguredGlobals()));
+
 }
 
 } // end of anonymous namespace