[#103,!277] Use enum instead of boolean in databaseConfigFetch.

As a result of review.

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index d922dd8cfced2bc4f0291a832267224c470094ea..a86b1b73698612873c94a82bf3ecdf5442ed013b 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -973,7 +973,8 @@ ControlledDhcpv4Srv::cbFetchUpdates(const SrvConfigPtr& srv_cfg) {
         // The true value indicates that the server should not reconnect
         // to the configuration backends and should take into account
         // audit entries stored in the database since last fetch.
-        server_->getCBControl()->databaseConfigFetch(srv_cfg, true);
+        server_->getCBControl()->databaseConfigFetch(srv_cfg,
+                                                     CBControlDHCPv4::FetchMode::FETCH_UPDATE);
 
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_CB_FETCH_UPDATES_FAIL)

diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index a03b1ed176560a6a33b66657cf454917fee85997..01cf46d096d1f8aeb252c34ad6d4087a528ba3c3 100644 (file)
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -625,7 +625,8 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
             libraries.loadLibraries();
 
             // If there are config backends, fetch and merge into staging config
-            server.getCBControl()->databaseConfigFetch(srv_cfg, false);
+            server.getCBControl()->databaseConfigFetch(srv_cfg,
+                                                       CBControlDHCPv4::FetchMode::FETCH_ALL);
         }
         catch (const isc::Exception& ex) {
             LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what());

diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc
index f398afe62d32d8889c6c5bc97525d0fe7a79066d..9f107199941716ca07ec4e8aeef47dc8a22dcea5 100644 (file)
--- a/src/bin/dhcp4/tests/kea_controller_unittest.cc
+++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc
@@ -72,7 +72,7 @@ public:
     /// @brief Constructor.
     TestCBControlDHCPv4()
         : CBControlDHCPv4(), db_config_fetch_calls_(0),
-          enable_check_fetch_updates_only_(false), enable_throw_(false) {
+          enable_check_fetch_mode_(false), enable_throw_(false) {
     }
 
     /// @brief Stub implementation of the "fetch" function.
@@ -82,23 +82,22 @@ public:
     /// throws an exception when desired by a test, to verify that the
     /// server gracefully handles such exception.
     ///
-    /// @param fetch_updates_only Boolean flag indicating that the function
-    /// is executed to fetch configuration updates from the database. It
-    /// should be set to false when the server is starting up.
+    /// @param fetch_mode value indicating if the method is called upon the
+    /// server start up or it is called to fetch configuration updates.
     ///
     /// @throw Unexpected when configured to do so.
     virtual void databaseConfigFetch(const process::ConfigPtr&,
-                                     const bool fetch_updates_only) {
+                                     const FetchMode& fetch_mode) {
         ++db_config_fetch_calls_;
 
-        if (enable_check_fetch_updates_only_) {
-            if ((db_config_fetch_calls_ <= 1) && fetch_updates_only) {
+        if (enable_check_fetch_mode_) {
+            if ((db_config_fetch_calls_ <= 1) && (fetch_mode == FetchMode::FETCH_UPDATE)) {
                 ADD_FAILURE() << "databaseConfigFetch was called with the value "
-                    "of fetch_updates_only=true upon the server configuration";
+                    "of fetch_mode=FetchMode::FETCH_UPDATE upon the server configuration";
 
-            } else if ((db_config_fetch_calls_ > 1) && !fetch_updates_only) {
+            } else if ((db_config_fetch_calls_ > 1) && (fetch_mode == FetchMode::FETCH_ALL)) {
                 ADD_FAILURE() << "databaseConfigFetch was called with the value "
-                    "of fetch_updates_only=false during fetching the updates";
+                    "of fetch_mode=FetchMode::FETCH_ALL during fetching the updates";
             }
         }
 
@@ -112,9 +111,9 @@ public:
         return (db_config_fetch_calls_);
     }
 
-    /// @brief Enables checking of the @c fetch_updates_only value.
-    void enableCheckFetchUpdatesOnly() {
-        enable_check_fetch_updates_only_ = true;
+    /// @brief Enables checking of the @c fetch_mode value.
+    void enableCheckFetchMode() {
+        enable_check_fetch_mode_ = true;
     }
 
     /// @brief Enables the object to throw from @c databaseConfigFetch.
@@ -127,9 +126,9 @@ private:
     /// @brief Counter holding number of invocations of the @c databaseConfigFetch.
     size_t db_config_fetch_calls_;
 
-    /// @brief Boolean flag indicated if the value of the @c fetch_updates_only
+    /// @brief Boolean flag indicated if the value of the @c fetch_mode
     /// should be verified.
-    bool enable_check_fetch_updates_only_;
+    bool enable_check_fetch_mode_;
 
     /// @brief Boolean flag indicating if the @c databaseConfigFetch should
     /// throw.
@@ -245,9 +244,9 @@ public:
         // Get the CBControlDHCPv4 object belonging to this server.
         auto cb_control = boost::dynamic_pointer_cast<TestCBControlDHCPv4>(srv->getCBControl());
 
-        // Verify that the boolean parameter passed to the databaseConfigFetch
-        // has an expected value.
-        cb_control->enableCheckFetchUpdatesOnly();
+        // Verify that the parameter passed to the databaseConfigFetch has an
+        // expected value.
+        cb_control->enableCheckFetchMode();
 
         // Instruct our stub implementation of the CBControlDHCPv4 to throw as a
         // result of fetch if desired.

diff --git a/src/lib/process/cb_ctl_base.h b/src/lib/process/cb_ctl_base.h
index d50a5f84f0ce2f9cd861fd604cb4a5542ecf3fee..223828c1f0f4ac9338ab1c7d1b887b79d88acfdb 100644 (file)
--- a/src/lib/process/cb_ctl_base.h
+++ b/src/lib/process/cb_ctl_base.h
@@ -80,6 +80,16 @@ template<typename ConfigBackendMgrType>
 class CBControlBase {
 public:
 
+    /// @brief Fetch mode used in invocations to @c databaseConfigFetch.
+    ///
+    /// One of the values indicates that the entire configuration should
+    /// be fetched. The other one indicates that only configuration updates
+    /// should be fetched.
+    enum class FetchMode {
+        FETCH_ALL,
+        FETCH_UPDATE
+    };
+
     /// @brief Constructor.
     ///
     /// Sets the time of the last fetched audit entry to Jan 1st, 1970.
@@ -155,14 +165,13 @@ public:
     /// from the file in case the method is called upon the server's start
     /// up. It is a pointer to the current server configuration if the
     /// method is called to fetch configuration updates.
-    /// @param fetch_updates_only boolean value indicating if the method is
-    /// called upon the server start up (false) or it is called to fetch
-    /// configuration updates (true).
+    /// @param fetch_mode value indicating if the method is called upon the
+    /// server start up or it is called to fetch configuration updates.
     virtual void databaseConfigFetch(const ConfigPtr& srv_cfg,
-                                     const bool fetch_updates_only = false) {
+                                     const FetchMode& fetch_mode = FetchMode::FETCH_ALL) {
         // If the server starts up we need to connect to the database(s).
         // If there are no databases available simply do nothing.
-        if (!fetch_updates_only && !databaseConfigConnect(srv_cfg)) {
+        if ((fetch_mode == FetchMode::FETCH_ALL) && !databaseConfigConnect(srv_cfg)) {
             // There are no CB databases so we're done
             return;
         }
@@ -201,7 +210,7 @@ public:
 
         // If this is full reconfiguration we don't need the audit entries anymore.
         // Let's remove them and proceed as if they don't exist.
-        if (!fetch_updates_only) {
+        if (fetch_mode == FetchMode::FETCH_ALL) {
             audit_entries.clear();
         }
 
@@ -209,7 +218,7 @@ public:
         // audit entries indicating that there are some pending updates, let's
         // execute the server specific function that fetches and merges the data
         // into the given configuration.
-        if (!fetch_updates_only || !audit_entries.empty()) {
+        if ((fetch_mode == FetchMode::FETCH_ALL) || !audit_entries.empty()) {
             try {
                 databaseConfigApply(backend_selector, server_selector,
                                     lb_modification_time, audit_entries);

diff --git a/src/lib/process/tests/cb_ctl_base_unittests.cc b/src/lib/process/tests/cb_ctl_base_unittests.cc
index bdaffba41a052a91d9befc5f9ae43c910e601e70..97836af1016fc85f0d11004d0925f0b39b8a7efe 100644 (file)
--- a/src/lib/process/tests/cb_ctl_base_unittests.cc
+++ b/src/lib/process/tests/cb_ctl_base_unittests.cc
@@ -549,7 +549,8 @@ TEST_F(CBControlBaseTest, fetchUpdates) {
     ASSERT_EQ(-1, cb_ctl_.getAuditEntriesNum());
     EXPECT_EQ(cb_ctl_.getInitialAuditEntryTime(), cb_ctl_.getLastAuditEntryTime());
 
-    ASSERT_NO_THROW(cb_ctl_.databaseConfigFetch(config_base, true));
+    ASSERT_NO_THROW(cb_ctl_.databaseConfigFetch(config_base,
+                                                CBControl::FetchMode::FETCH_UPDATE));
 
     // There should be one invocation to databaseConfigApply recorded.
     ASSERT_EQ(1, cb_ctl_.getMergesNum());
@@ -581,7 +582,8 @@ TEST_F(CBControlBaseTest, fetchNoUpdates) {
 
     ASSERT_EQ(0, cb_ctl_.getMergesNum());
 
-    ASSERT_NO_THROW(cb_ctl_.databaseConfigFetch(config_base, true));
+    ASSERT_NO_THROW(cb_ctl_.databaseConfigFetch(config_base,
+                                                CBControl::FetchMode::FETCH_UPDATE));
 
     // The databaseConfigApply should not be called because there are
     // no new audit entires to process.
@@ -612,7 +614,8 @@ TEST_F(CBControlBaseTest, fetchFailure) {
     ASSERT_EQ(-1, cb_ctl_.getAuditEntriesNum());
     EXPECT_EQ(cb_ctl_.getInitialAuditEntryTime(), cb_ctl_.getLastAuditEntryTime());
 
-    ASSERT_THROW(cb_ctl_.databaseConfigFetch(config_base, true), isc::Unexpected);
+    ASSERT_THROW(cb_ctl_.databaseConfigFetch(config_base, CBControl::FetchMode::FETCH_UPDATE),
+                 isc::Unexpected);
 
     // There should be one invocation to databaseConfigApply recorded.
     ASSERT_EQ(1, cb_ctl_.getMergesNum());