From: Thomas Markwalder Date: Wed, 7 Mar 2018 16:10:34 +0000 (-0500) Subject: [5477] Addressed review comments X-Git-Tag: trac5530a_base~5^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f5ce7060067456b68a6df39d3d20f122b96596e;p=thirdparty%2Fkea.git [5477] Addressed review comments src/bin/dhcp4/ctrl_dhcp4_srv.* src/bin/dhcp6/ctrl_dhcp6_srv.* Changed dbReconnect() to accept ReconnectCtlPtr Added commentary for dbReconnect and dbLostCallback src/lib/dhcpsrv/database_connection.h Removed extraneous typedef many files: Changed DatabaseConnection::Callback to ::DbLostCallback src/lib/dhcpsrv/tests/database_connection_unittest.cc Added commentary to text fixture and tests --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index db76346b03..6767602acb 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -650,8 +650,7 @@ ControlledDhcpv4Srv::checkConfig(isc::data::ConstElementPtr config) { } ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) - : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()), - db_reconnect_ctl_(NULL) { + : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) { if (getInstance()) { isc_throw(InvalidOperation, "There is another Dhcpv4Srv instance already."); @@ -795,7 +794,7 @@ ControlledDhcpv4Srv::deleteExpiredReclaimedLeases(const uint32_t secs) { } void -ControlledDhcpv4Srv::dbReconnect() { +ControlledDhcpv4Srv::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) { bool reopened = false; // Re-open lease and host database with new parameters. @@ -817,24 +816,25 @@ ControlledDhcpv4Srv::dbReconnect() { network_state_.enableService(); // Toss the reconnect control, we're done with it - db_reconnect_ctl_.reset(); + db_reconnect_ctl.reset(); } else { - if (!db_reconnect_ctl_->checkRetries()) { + if (!db_reconnect_ctl->checkRetries()) { LOG_ERROR(dhcp4_logger, DHCP4_DB_RECONNECT_RETRIES_EXHAUSTED) - .arg(db_reconnect_ctl_->maxRetries()); + .arg(db_reconnect_ctl->maxRetries()); shutdown(); return; } LOG_INFO(dhcp4_logger, DHCP4_DB_RECONNECT_ATTEMPT_SCHEDULE) - .arg(db_reconnect_ctl_->maxRetries() - db_reconnect_ctl_->retriesLeft() + 1) - .arg(db_reconnect_ctl_->maxRetries()) - .arg(db_reconnect_ctl_->retryInterval()); + .arg(db_reconnect_ctl->maxRetries() - db_reconnect_ctl->retriesLeft() + 1) + .arg(db_reconnect_ctl->maxRetries()) + .arg(db_reconnect_ctl->retryInterval()); if (!TimerMgr::instance()->isTimerRegistered("Dhcp4DbReconnectTimer")) { TimerMgr::instance()->registerTimer("Dhcp4DbReconnectTimer", - boost::bind(&ControlledDhcpv4Srv::dbReconnect, this), - db_reconnect_ctl_->retryInterval() * 1000, + boost::bind(&ControlledDhcpv4Srv::dbReconnect, this, + db_reconnect_ctl), + db_reconnect_ctl->retryInterval() * 1000, asiolink::IntervalTimer::ONE_SHOT); } @@ -862,11 +862,8 @@ ControlledDhcpv4Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) { return(false); } - // Save the reconnect control - db_reconnect_ctl_ = db_reconnect_ctl; - // Invoke reconnect method - dbReconnect(); + dbReconnect(db_reconnect_ctl); return(true); } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 740a7dcfe9..06d5ee6472 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -319,9 +319,43 @@ private: void deleteExpiredReclaimedLeases(const uint32_t secs); /// @brief Attempts to reconnect the server to the DB backend managers - void dbReconnect(); + /// + /// This is a self-rescheduling function that attempts to reconnect to the + /// server's DB backends after connectivity to one or more have been + /// lost. Upon entry it will attempt to reconnect via @ref CfgDdbAccess:: + /// createManagers. If this is succesful, DHCP servicing is re-enabled and + /// server returns to normal operation. + /// + /// If reconnection fails and the maximum number of retries has not been + /// exhausted, it will schedule a call to itself to occur at the + /// configured retry interval. DHCP service remains disabled. + /// + /// If the maximum number of retries has been exhausted an error is logged + /// and the server shuts down. + /// + /// @param db_reconnect_ctl pointer to the ReconnectCtl containing the + /// configured reconnect parameters + /// + void dbReconnect(ReconnectCtlPtr db_reconnect_ctl); /// @brief Callback DB backends should invoke upon loss of connectivity + /// + /// This function is invoked by DB backends when they detect a loss of + /// connectivity. The parameter, db_reconnect_ctl, conveys the configured + /// maximum number of reconnect retries as well as the interval to wait + /// between retry attempts. + /// + /// If either value is zero, reconnect is presumed to be disabled and + /// the function will returns false. This instructs the DB backend + /// layer (the caller) to treat the connectivity loss as fatal. + /// + /// Otherwise, the function saves db_reconnect_ctl and invokes + /// dbReconnect to initiate the reconnect process. + /// + /// @param db_reconnect_ctl pointer to the ReconnectCtl containing the + /// configured reconnect parameters + /// + /// @return false if reconnect is not configured, true otherwise bool dbLostCallback(ReconnectCtlPtr db_reconnect_ctl); /// @brief Static pointer to the sole instance of the DHCP server. @@ -338,9 +372,6 @@ private: /// Shared pointer to the instance of timer @c TimerMgr is held here to /// make sure that the @c TimerMgr outlives instance of this class. TimerMgrPtr timer_mgr_; - - /// @brief Pointer the current DB reconnect control values - ReconnectCtlPtr db_reconnect_ctl_; }; }; // namespace isc::dhcp diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index bfa8348d2a..46a524c854 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -673,8 +673,7 @@ ControlledDhcpv6Srv::checkConfig(isc::data::ConstElementPtr config) { } ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port) - : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()), - db_reconnect_ctl_(NULL) { + : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) { if (server_) { isc_throw(InvalidOperation, "There is another Dhcpv6Srv instance already."); @@ -817,7 +816,7 @@ ControlledDhcpv6Srv::deleteExpiredReclaimedLeases(const uint32_t secs) { } void -ControlledDhcpv6Srv::dbReconnect() { +ControlledDhcpv6Srv::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) { bool reopened = false; // Re-open lease and host database with new parameters. @@ -838,24 +837,25 @@ ControlledDhcpv6Srv::dbReconnect() { network_state_.enableService(); // Toss the reconnct control, we're done with it - db_reconnect_ctl_.reset(); + db_reconnect_ctl.reset(); } else { - if (!db_reconnect_ctl_->checkRetries()) { + if (!db_reconnect_ctl->checkRetries()) { LOG_ERROR(dhcp6_logger, DHCP6_DB_RECONNECT_RETRIES_EXHAUSTED) - .arg(db_reconnect_ctl_->maxRetries()); + .arg(db_reconnect_ctl->maxRetries()); shutdown(); return; } LOG_INFO(dhcp6_logger, DHCP6_DB_RECONNECT_ATTEMPT_SCHEDULE) - .arg(db_reconnect_ctl_->maxRetries() - db_reconnect_ctl_->retriesLeft() + 1) - .arg(db_reconnect_ctl_->maxRetries()) - .arg(db_reconnect_ctl_->retryInterval()); + .arg(db_reconnect_ctl->maxRetries() - db_reconnect_ctl->retriesLeft() + 1) + .arg(db_reconnect_ctl->maxRetries()) + .arg(db_reconnect_ctl->retryInterval()); if (!TimerMgr::instance()->isTimerRegistered("Dhcp6DbReconnectTimer")) { TimerMgr::instance()->registerTimer("Dhcp6DbReconnectTimer", - boost::bind(&ControlledDhcpv6Srv::dbReconnect, this), - db_reconnect_ctl_->retryInterval() * 1000, + boost::bind(&ControlledDhcpv6Srv::dbReconnect, this, + db_reconnect_ctl), + db_reconnect_ctl->retryInterval() * 1000, asiolink::IntervalTimer::ONE_SHOT); } @@ -883,11 +883,8 @@ ControlledDhcpv6Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) { return(false); } - // Save the reconnect control - db_reconnect_ctl_ = db_reconnect_ctl; - // Invoke reconnect method - dbReconnect(); + dbReconnect(db_reconnect_ctl); return(true); } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 49201b87da..31e738a91e 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -319,9 +319,40 @@ private: void deleteExpiredReclaimedLeases(const uint32_t secs); /// @brief Attempts to reconnect the server to the DB backend managers - void dbReconnect(); + /// + /// This is a self-rescheduling function that attempts to reconnect to the + /// server's DB backends after connectivity to one or more have been + /// lost. Upon entry it will attempt to reconnect via @ref CfgDdbAccess:: + /// createManagers. If this is succesful, DHCP servicing is re-enabled and + /// server returns to normal operation. + /// + /// If reconnection fails and the maximum number of retries has not been + /// exhausted, it will schedule a call to itself to occur at the + /// configured retry interval. DHCP service remains disabled. + /// + /// If the maximum number of retries has been exhausted an error is logged + /// and the server shuts down. + /// @param db_reconnect_ctl pointer to the ReconnectCtl containing the + /// configured reconnect parameters + /// + void dbReconnect(ReconnectCtlPtr db_reconnect_ctl); /// @brief Callback DB backends should invoke upon loss of connectivity + /// + /// This function is invoked by DB backends when they detect a loss of + /// connectivity. The parameter, db_reconnect_ctl, conveys the configured + /// maximum number of reconnect retries as well as the interval to wait + /// between retry attempts. + /// + /// If either value is zero, reconnect is presumed to be disabled and + /// the function will returns false. This instructs the DB backend + /// layer (the caller) to treat the connectivity loss as fatal. + /// + /// Otherwise, the function saves db_reconnect_ctl and invokes + /// dbReconnect to initiate the reconnect process. + /// + /// @param db_reconnect_ctl pointer to the ReconnectCtl containing the + /// configured reconnect parameters bool dbLostCallback(ReconnectCtlPtr db_reconnect_ctl); /// @brief Static pointer to the sole instance of the DHCP server. @@ -338,9 +369,6 @@ private: /// Shared pointer to the instance of timer @c TimerMgr is held here to /// make sure that the @c TimerMgr outlives instance of this class. TimerMgrPtr timer_mgr_; - - /// @brief Pointer the current DB reconnect control values - ReconnectCtlPtr db_reconnect_ctl_; }; }; // namespace isc::dhcp diff --git a/src/lib/dhcpsrv/cfg_db_access.cc b/src/lib/dhcpsrv/cfg_db_access.cc index 942b944646..72ebc8ab19 100644 --- a/src/lib/dhcpsrv/cfg_db_access.cc +++ b/src/lib/dhcpsrv/cfg_db_access.cc @@ -38,7 +38,7 @@ CfgDbAccess::getHostDbAccessString() const { void -CfgDbAccess::createManagers(DatabaseConnection::Callback db_lost_callback) const { +CfgDbAccess::createManagers(DatabaseConnection::DbLostCallback db_lost_callback) const { // Recreate lease manager. LeaseMgrFactory::destroy(); LeaseMgrFactory::create(getLeaseDbAccessString(), db_lost_callback); diff --git a/src/lib/dhcpsrv/cfg_db_access.h b/src/lib/dhcpsrv/cfg_db_access.h index 3459edf1e8..845535d646 100644 --- a/src/lib/dhcpsrv/cfg_db_access.h +++ b/src/lib/dhcpsrv/cfg_db_access.h @@ -68,7 +68,7 @@ public: /// @param db_lost_callback function to invoke if connectivity to /// either the lease or host managers, once established, is subsequently /// lost. - void createManagers(DatabaseConnection::Callback db_lost_callback = NULL) const; + void createManagers(DatabaseConnection::DbLostCallback db_lost_callback = NULL) const; /// @brief Unparse an access string /// diff --git a/src/lib/dhcpsrv/database_connection.h b/src/lib/dhcpsrv/database_connection.h index d4d9e48411..7f89ad2edb 100644 --- a/src/lib/dhcpsrv/database_connection.h +++ b/src/lib/dhcpsrv/database_connection.h @@ -141,8 +141,7 @@ public: typedef std::map ParameterMap; /// @brief Defines a callback prototype for propogating events upward - /// typedef boost::function Callback; - typedef boost::function Callback; + typedef boost::function DbLostCallback; /// @brief Constructor /// @@ -151,7 +150,7 @@ public: /// @param db_lost_callback Optional call back function to invoke if a /// successfully open connection subsequently fails DatabaseConnection(const ParameterMap& parameters, - Callback db_lost_callback = NULL) + DbLostCallback db_lost_callback = NULL) :parameters_(parameters), db_lost_callback_(db_lost_callback) { } @@ -221,7 +220,7 @@ private: protected: /// @brief Optional function to invoke if the connectivity is lost - Callback db_lost_callback_; + DbLostCallback db_lost_callback_; }; }; // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/host_data_source_factory.cc b/src/lib/dhcpsrv/host_data_source_factory.cc index 0f1e153725..2216ff768b 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.cc +++ b/src/lib/dhcpsrv/host_data_source_factory.cc @@ -46,7 +46,7 @@ HostDataSourceFactory::getHostDataSourcePtr() { void HostDataSourceFactory::create(const std::string& dbaccess, - DatabaseConnection::Callback db_lost_callback) { + DatabaseConnection::DbLostCallback db_lost_callback) { // Parse the access string and create a redacted string for logging. DatabaseConnection::ParameterMap parameters = DatabaseConnection::parse(dbaccess); diff --git a/src/lib/dhcpsrv/host_data_source_factory.h b/src/lib/dhcpsrv/host_data_source_factory.h index 151f46ffc6..b941aa730d 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.h +++ b/src/lib/dhcpsrv/host_data_source_factory.h @@ -67,7 +67,7 @@ public: /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not /// identify a supported backend. static void create(const std::string& dbaccess, - DatabaseConnection::Callback db_lost_callback = NULL); + DatabaseConnection::DbLostCallback db_lost_callback = NULL); /// @brief Destroy host data source /// diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 75cb6ecdec..e977eae7d6 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -38,7 +38,7 @@ HostMgr::getHostMgrPtr() { void HostMgr::create(const std::string& access, - DatabaseConnection::Callback db_lost_callback) { + DatabaseConnection::DbLostCallback db_lost_callback) { getHostMgrPtr().reset(new HostMgr()); if (!access.empty()) { diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index a0e88a36a9..7cef82e7b8 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -73,7 +73,7 @@ public: /// @param db_lost_callback function to invoke if connectivity to /// the host database is lost. static void create(const std::string& access = "", - DatabaseConnection::Callback db_lost_callback = NULL); + DatabaseConnection::DbLostCallback db_lost_callback = NULL); /// @brief Returns a sole instance of the @c HostMgr. /// diff --git a/src/lib/dhcpsrv/lease_mgr_factory.cc b/src/lib/dhcpsrv/lease_mgr_factory.cc index 54ee057715..261ece96b3 100644 --- a/src/lib/dhcpsrv/lease_mgr_factory.cc +++ b/src/lib/dhcpsrv/lease_mgr_factory.cc @@ -42,7 +42,7 @@ LeaseMgrFactory::getLeaseMgrPtr() { void LeaseMgrFactory::create(const std::string& dbaccess, - DatabaseConnection::Callback db_lost_callback) { + DatabaseConnection::DbLostCallback db_lost_callback) { const std::string type = "type"; // Parse the access string and create a redacted string for logging. diff --git a/src/lib/dhcpsrv/lease_mgr_factory.h b/src/lib/dhcpsrv/lease_mgr_factory.h index 54c57c3d9c..83fd6017fd 100644 --- a/src/lib/dhcpsrv/lease_mgr_factory.h +++ b/src/lib/dhcpsrv/lease_mgr_factory.h @@ -71,7 +71,7 @@ public: /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not /// identify a supported backend. static void create(const std::string& dbaccess, - DatabaseConnection::Callback db_lost_callback = NULL); + DatabaseConnection::DbLostCallback db_lost_callback = NULL); /// @brief Destroy lease manager /// diff --git a/src/lib/dhcpsrv/pgsql_connection.h b/src/lib/dhcpsrv/pgsql_connection.h index 23d07a3ce1..fc8e9de648 100644 --- a/src/lib/dhcpsrv/pgsql_connection.h +++ b/src/lib/dhcpsrv/pgsql_connection.h @@ -304,7 +304,7 @@ public: } PgSqlConnection(const ParameterMap& parameters, - Callback db_lost_callback) + DbLostCallback db_lost_callback) : DatabaseConnection(parameters, db_lost_callback) { } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index bd1e330da7..fa04926ef6 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1285,7 +1285,7 @@ public: /// This constructor opens database connection and initializes prepared /// statements used in the queries. PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback); + DatabaseConnection::DbLostCallback db_lost_callback); /// @brief Destructor. ~PgSqlHostDataSourceImpl(); @@ -1701,7 +1701,7 @@ TaggedStatementArray tagged_statements = { { PgSqlHostDataSourceImpl:: PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback) + DatabaseConnection::DbLostCallback db_lost_callback) : host_exchange_(new PgSqlHostWithOptionsExchange(PgSqlHostWithOptionsExchange::DHCP4_ONLY)), host_ipv6_exchange_(new PgSqlHostIPv6Exchange(PgSqlHostWithOptionsExchange::DHCP6_ONLY)), host_ipv46_exchange_(new PgSqlHostIPv6Exchange(PgSqlHostWithOptionsExchange:: @@ -1929,7 +1929,7 @@ PgSqlHostDataSourceImpl::checkReadOnly() const { PgSqlHostDataSource:: PgSqlHostDataSource(const PgSqlConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback) + DatabaseConnection::DbLostCallback db_lost_callback) : impl_(new PgSqlHostDataSourceImpl(parameters, db_lost_callback)) { } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h index 91ffd7643e..0c15f87393 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.h +++ b/src/lib/dhcpsrv/pgsql_host_data_source.h @@ -61,7 +61,7 @@ public: /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. PgSqlHostDataSource(const DatabaseConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback = NULL); + DatabaseConnection::DbLostCallback db_lost_callback = NULL); /// @brief Virtual destructor. /// Frees database resources and closes the database connection through diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 049533acf1..6c016333b6 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -821,7 +821,7 @@ protected: }; PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback) + DatabaseConnection::DbLostCallback db_lost_callback) : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()), exchange6_(new PgSqlLease6Exchange()), conn_(parameters, db_lost_callback) { conn_.openDatabase(); diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index fb891ae349..5c211b602b 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -59,7 +59,7 @@ public: /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters, - DatabaseConnection::Callback db_lost_callback = NULL); + DatabaseConnection::DbLostCallback db_lost_callback = NULL); /// @brief Destructor (closes database) virtual ~PgSqlLeaseMgr(); diff --git a/src/lib/dhcpsrv/tests/database_connection_unittest.cc b/src/lib/dhcpsrv/tests/database_connection_unittest.cc index d5edcc38ca..37fe123530 100644 --- a/src/lib/dhcpsrv/tests/database_connection_unittest.cc +++ b/src/lib/dhcpsrv/tests/database_connection_unittest.cc @@ -13,21 +13,28 @@ using namespace isc::dhcp; +/// @brief Test fixture for excersizing DbLostCallback invocation class DatabaseConnectionCallbackTest : public ::testing::Test { public: + /// Constructor DatabaseConnectionCallbackTest() : db_reconnect_ctl_(0) { } - bool dbLostCallback(ReconnectCtlPtr db_conn_retry) { - if (!db_conn_retry) { - isc_throw(isc::BadValue, "db_conn_retry should not null"); + /// @brief Callback to register with a DatabaseConnection + /// + /// @param db_reconnect_ctl ReconnectCtl containing reconnect + /// parameters + bool dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) { + if (!db_reconnect_ctl) { + isc_throw(isc::BadValue, "db_reconnect_ctl should not null"); } - db_reconnect_ctl_ = db_conn_retry; + db_reconnect_ctl_ = db_reconnect_ctl; return (true); } + /// @brief Retainer for the control passed into the callback ReconnectCtlPtr db_reconnect_ctl_; }; @@ -49,6 +56,11 @@ TEST(DatabaseConnectionTest, getParameter) { EXPECT_THROW(datasrc.getParameter("param3"), isc::BadValue); } +/// @brief NoDbLostCallback +/// +/// This test verifies that DatabaseConnection::invokeDbLostCallback +/// returns a false if there is connection has no registered +/// DbLostCallback. TEST_F(DatabaseConnectionCallbackTest, NoDbLostCallback) { DatabaseConnection::ParameterMap pmap; pmap[std::string("max-reconnect-tries")] = std::string("3"); @@ -61,12 +73,25 @@ TEST_F(DatabaseConnectionCallbackTest, NoDbLostCallback) { EXPECT_FALSE(db_reconnect_ctl_); } + +/// @brief NoDbLostCallback +/// +/// This test verifies that DatabaseConnection::invokeDbLostCallback +/// safely invokes the registered DbLostCallback. It also tests +/// operation of DbReconnectCtl retry accounting methods. TEST_F(DatabaseConnectionCallbackTest, dbLostCallback) { + /// Create a Database configuration that includes the reconnect + /// control parameters. DatabaseConnection::ParameterMap pmap; pmap[std::string("max-reconnect-tries")] = std::string("3"); pmap[std::string("reconnect-wait-time")] = std::string("60"); + + /// Create the connection with a DbLostCallback. DatabaseConnection datasrc(pmap, boost::bind(&DatabaseConnectionCallbackTest ::dbLostCallback, this, _1)); + + /// We should be able to invoke the callback and glean + /// the correct reconnect contorl parameters from it. bool ret; ASSERT_NO_THROW(ret = datasrc.invokeDbLostCallback()); EXPECT_TRUE(ret); @@ -75,11 +100,15 @@ TEST_F(DatabaseConnectionCallbackTest, dbLostCallback) { ASSERT_EQ(3, db_reconnect_ctl_->retriesLeft()); EXPECT_EQ(60, db_reconnect_ctl_->retryInterval()); + /// Verify that checkRetries() correctly decrements + /// down to zero, and that retriesLeft() returns + /// the correct value. for (int i = 3; i > 1 ; --i) { ASSERT_EQ(i, db_reconnect_ctl_->retriesLeft()); ASSERT_TRUE(db_reconnect_ctl_->checkRetries()); } + /// Retries are exhausted, verify that's reflected. EXPECT_FALSE(db_reconnect_ctl_->checkRetries()); EXPECT_EQ(0, db_reconnect_ctl_->retriesLeft()); EXPECT_EQ(3, db_reconnect_ctl_->maxRetries());