summaryrefslogtreecommitdiffstats
path: root/src/lib
diff options
context:
space:
mode:
authorThomas Markwalder <tmark@isc.org>2018-03-07 17:10:34 +0100
committerThomas Markwalder <tmark@isc.org>2018-03-07 17:10:34 +0100
commit8f5ce7060067456b68a6df39d3d20f122b96596e (patch)
treeb97aea749071f605da80dd6224725af65cc57815 /src/lib
parent[5477] Corrected some typos and updated copyrights as a result of review. (diff)
downloadkea-8f5ce7060067456b68a6df39d3d20f122b96596e.tar.xz
kea-8f5ce7060067456b68a6df39d3d20f122b96596e.zip
[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
Diffstat (limited to 'src/lib')
-rw-r--r--src/lib/dhcpsrv/cfg_db_access.cc2
-rw-r--r--src/lib/dhcpsrv/cfg_db_access.h2
-rw-r--r--src/lib/dhcpsrv/database_connection.h7
-rw-r--r--src/lib/dhcpsrv/host_data_source_factory.cc2
-rw-r--r--src/lib/dhcpsrv/host_data_source_factory.h2
-rw-r--r--src/lib/dhcpsrv/host_mgr.cc2
-rw-r--r--src/lib/dhcpsrv/host_mgr.h2
-rw-r--r--src/lib/dhcpsrv/lease_mgr_factory.cc2
-rw-r--r--src/lib/dhcpsrv/lease_mgr_factory.h2
-rw-r--r--src/lib/dhcpsrv/pgsql_connection.h2
-rw-r--r--src/lib/dhcpsrv/pgsql_host_data_source.cc6
-rw-r--r--src/lib/dhcpsrv/pgsql_host_data_source.h2
-rw-r--r--src/lib/dhcpsrv/pgsql_lease_mgr.cc2
-rw-r--r--src/lib/dhcpsrv/pgsql_lease_mgr.h2
-rw-r--r--src/lib/dhcpsrv/tests/database_connection_unittest.cc37
15 files changed, 51 insertions, 23 deletions
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<std::string, std::string> ParameterMap;
/// @brief Defines a callback prototype for propogating events upward
- /// typedef boost::function<bool (const ParameterMap& parameters)> Callback;
- typedef boost::function<bool (ReconnectCtlPtr db_retry)> Callback;
+ typedef boost::function<bool (ReconnectCtlPtr db_retry)> 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());