From: Marcin Siodelski Date: Mon, 25 Mar 2019 18:28:53 +0000 (+0100) Subject: [#103,!277] Use enum instead of boolean in databaseConfigFetch. X-Git-Tag: Kea-1.6.0-beta~313 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=227bdc64b64135cfbe1820219f525216936553f8;p=thirdparty%2Fkea.git [#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 d922dd8cfc..a86b1b7369 100644 --- 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 a03b1ed176..01cf46d096 100644 --- 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 f398afe62d..9f10719994 100644 --- 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(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 d50a5f84f0..223828c1f0 100644 --- a/src/lib/process/cb_ctl_base.h +++ b/src/lib/process/cb_ctl_base.h @@ -80,6 +80,16 @@ template 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 bdaffba41a..97836af101 100644 --- 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());