From 3b63c1b591b74dc7baddb51ff53cbcf06cd96f52 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Mon, 19 Oct 2015 00:34:47 +0200 Subject: [PATCH] [3682] Several corrections and compilation fixes. --- src/lib/dhcpsrv/base_host_data_source.h | 11 ++++++++++ src/lib/dhcpsrv/database_connection.h | 6 ++++- src/lib/dhcpsrv/host_data_source_factory.cc | 18 ++++++++++----- src/lib/dhcpsrv/host_mgr.cc | 20 +++++++---------- src/lib/dhcpsrv/host_mgr.h | 2 +- src/lib/dhcpsrv/hosts_messages.mes | 4 ++++ src/lib/dhcpsrv/mysql_connection.h | 2 +- src/lib/dhcpsrv/mysql_host_data_source.cc | 11 ++++++++++ src/lib/dhcpsrv/mysql_host_data_source.h | 17 +++++++++++--- .../generic_host_data_source_unittest.cc | 5 +++-- .../tests/generic_host_data_source_unittest.h | 2 -- .../tests/mysql_host_data_source_unittest.cc | 22 +++++++++---------- 12 files changed, 80 insertions(+), 40 deletions(-) diff --git a/src/lib/dhcpsrv/base_host_data_source.h b/src/lib/dhcpsrv/base_host_data_source.h index 67c5aed5ab..376a832972 100644 --- a/src/lib/dhcpsrv/base_host_data_source.h +++ b/src/lib/dhcpsrv/base_host_data_source.h @@ -187,6 +187,17 @@ public: /// @return Type of the backend. virtual std::string getType() const = 0; + /// @brief Commit Transactions + /// + /// Commits all pending database operations. On databases that don't + /// support transactions, this is a no-op. + virtual void commit() {}; + + /// @brief Rollback Transactions + /// + /// Rolls back all pending database operations. On databases that don't + /// support transactions, this is a no-op. + virtual void rollback() {}; }; } diff --git a/src/lib/dhcpsrv/database_connection.h b/src/lib/dhcpsrv/database_connection.h index 81a8c6f2b6..0479434917 100755 --- a/src/lib/dhcpsrv/database_connection.h +++ b/src/lib/dhcpsrv/database_connection.h @@ -57,7 +57,11 @@ class DatabaseConnection : public boost::noncopyable { public: /// @brief Defines maximum value for time that can be reliably stored. - // If I'm still alive I'll be too old to care. You fix it. + /// + /// @todo: Is this common for MySQL and Postgres? Maybe we should have + /// specific values for each backend? + /// + /// If I'm still alive I'll be too old to care. You fix it. static const time_t MAX_DB_TIME; /// @brief Database configuration parameter map diff --git a/src/lib/dhcpsrv/host_data_source_factory.cc b/src/lib/dhcpsrv/host_data_source_factory.cc index a0ffb976f5..245bf83fc1 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.cc +++ b/src/lib/dhcpsrv/host_data_source_factory.cc @@ -16,7 +16,11 @@ #include #include +#include + +#ifdef HAVE_MYSQL #include +#endif #include #include @@ -67,15 +71,18 @@ HostDataSourceFactory::create(const std::string& dbaccess) { return; } #endif + #ifdef HAVE_PGSQL if (parameters[type] == string("postgresql")) { LOG_INFO(dhcpsrv_logger, DHCPSRV_PGSQL_DB).arg(redacted); - // set pgsql data source here, when it will be featured + isc_throw(NotImplemented, "Sorry, Postgres backend for host reservations " + "is not implemented yet."); + // Set pgsql data source here, when it will be implemented. return; } #endif - // Get here on no match + // Get here on no match. LOG_ERROR(dhcpsrv_logger, DHCPSRV_UNKNOWN_DB).arg(parameters[type]); isc_throw(InvalidType, "Database access parameter 'type' does " "not specify a supported database backend"); @@ -86,16 +93,15 @@ HostDataSourceFactory::destroy() { // Destroy current host data source instance. This is a no-op if no host // data source is available. if (getHostDataSourcePtr()) { - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, - DHCPSRV_CLOSE_HOST_DATA_SOURCE) - .arg(getHostDataSourcePtr()->getType()); + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, HOSTS_CFG_CLOSE_HOST_DATA_SOURCE) + .arg(getHostDataSourcePtr()->getType()); } getHostDataSourcePtr().reset(); } BaseHostDataSource& HostDataSourceFactory::instance() { - BaseHostDataSource* hdsptr = getHostDataSourcePtr().get(); + BaseHostDataSource* hdsptr = getHostDataSourcePtr().get(); if (hdsptr == NULL) { isc_throw(NoHostDataSourceManager, "no current host data source instance is available"); diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 64dd65ee09..6b54804f89 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -38,6 +38,8 @@ namespace dhcp { using namespace isc::asiolink; +boost::shared_ptr HostMgr::alternate_source; + boost::scoped_ptr& HostMgr::getHostMgrPtr() { static boost::scoped_ptr host_mgr_ptr; @@ -45,21 +47,15 @@ HostMgr::getHostMgrPtr() { } void -HostMgr::create(const std::string& /*access*/) { +HostMgr::create(const std::string& access) { getHostMgrPtr().reset(new HostMgr()); -/* - try { + + if (!access.empty()) { HostDataSourceFactory::create(access); - } catch (...) { - std::cerr << "Unable to open database."; - throw; - } -*/ - //alternate_source = &(HostDataSourceFactory::instance()); - //alternate_source.reset(&(HostDataSourceFactory::instance())); - /// @todo Initialize alternate_source here, using the parameter. - /// For example: alternate_source.reset(new MysqlHostDataSource(access)). + /// @todo Initialize alternate_source here. + //alternate_source = HostDataSourceFactory::getHostDataSourcePtr(); + } } HostMgr& diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index 098468c6b2..80aac53786 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -216,7 +216,7 @@ private: /// @brief Pointer to an alternate host data source. /// /// If this pointer is NULL, the source is not in use. - boost::shared_ptr alternate_source; + static boost::shared_ptr alternate_source; /// @brief Returns a pointer to the currently used instance of the /// @c HostMgr. diff --git a/src/lib/dhcpsrv/hosts_messages.mes b/src/lib/dhcpsrv/hosts_messages.mes index cc3b8545a2..0644b4594d 100644 --- a/src/lib/dhcpsrv/hosts_messages.mes +++ b/src/lib/dhcpsrv/hosts_messages.mes @@ -19,6 +19,10 @@ This debug message is issued when new host (with reservations) is added to the server's configuration. The argument describes the host and its reservations in detail. +% HOSTS_CFG_CLOSE_HOST_DATA_SOURCE Closing host data source: %1 +This is a normal message being printed when the server closes host data +source connection. + % HOSTS_CFG_GET_ALL_ADDRESS4 get all hosts with reservations for IPv4 address %1 This debug message is issued when starting to retrieve all hosts, holding the reservation for the specific IPv4 address, from the configuration. The diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index a94e7e253c..fcce51f504 100755 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -17,7 +17,7 @@ #include #include -#include +#include #include namespace isc { diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index f5555dba27..b3958a2a27 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -970,6 +970,17 @@ std::pair MySqlHostDataSource::getVersion() const { return (std::make_pair(major, minor)); } +void +MySqlHostDataSource::commit() { + conn_.commit(); +} + + +void +MySqlHostDataSource::rollback() { + conn_.rollback(); +} + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 42f614d30a..65a8627397 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -21,7 +21,7 @@ #include #include -#include +#include namespace isc { namespace dhcp { @@ -32,10 +32,9 @@ class MySqlHostReservationExchange; /// @brief MySQL Host Data Source /// -/// This class provides the \ref isc::dhcp::BaseHostDataSource interface to the MySQL +/// This class provides the @ref isc::dhcp::BaseHostDataSource interface to the MySQL /// database. Use of this backend presupposes that a MySQL database is /// available and that the Kea schema has been created within it. - class MySqlHostDataSource: public BaseHostDataSource { public: @@ -219,6 +218,18 @@ public: /// has failed. virtual std::pair getVersion() const; + /// @brief Commit Transactions + /// + /// Commits all pending database operations. On databases that don't + /// support transactions, this is a no-op. + virtual void commit(); + + /// @brief Rollback Transactions + /// + /// Rolls back all pending database operations. On databases that don't + /// support transactions, this is a no-op. + virtual void rollback(); + MySqlConnection* getDatabaseConnection() { return &conn_; } diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index b0c744dc59..c8b70c7499 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -27,11 +27,12 @@ namespace dhcp { namespace test { GenericHostDataSourceTest::GenericHostDataSourceTest() - :hdsptr_(NULL){ + :hdsptr_(NULL) { } -GenericHostDataSourceTest::~GenericHostDataSourceTest(){} +GenericHostDataSourceTest::~GenericHostDataSourceTest() { +} diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h index e133406df5..7e4fa5cc57 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h @@ -42,10 +42,8 @@ public: /// @brief Virtual destructor. virtual ~GenericHostDataSourceTest(); - /// @brief Pointer to the host data source BaseHostDataSource* hdsptr_; - }; }; // namespace test diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index ece4422cf6..0190ea45eb 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -22,7 +22,6 @@ #include #include - #include #include @@ -58,8 +57,6 @@ const char* INVALID_USER = "user=invaliduser"; const char* VALID_PASSWORD = "password=keatest"; const char* INVALID_PASSWORD = "password=invalid"; -MySqlHostDataSource* myhdsptr_; - // Given a combination of strings above, produce a connection string. string connectionString(const char* type, const char* name, const char* host, const char* user, const char* password) { @@ -151,7 +148,7 @@ public: /// @brief Constructor /// /// Deletes everything from the database and opens it. - MySqlHostDataSourceTest() { + MySqlHostDataSourceTest() { // Ensure schema is the correct one. destroySchema(); @@ -168,7 +165,8 @@ public: "*** accompanying exception output.\n"; throw; } - myhdsptr_ = (MySqlHostDataSource *) &(HostDataSourceFactory::instance()); + + hdsptr_ = &(HostDataSourceFactory::instance()); } /// @brief Destructor @@ -176,7 +174,7 @@ public: /// Rolls back all pending transactions. The deletion of myhdsptr_ will close /// the database. Then reopen it and delete everything created by the test. virtual ~MySqlHostDataSourceTest() { - myhdsptr_->getDatabaseConnection()->rollback(); + hdsptr_->rollback(); HostDataSourceFactory::destroy(); destroySchema(); } @@ -189,21 +187,21 @@ public: /// Parameter is ignored for MySQL backend as the v4 and v6 leases share /// the same database. void reopen(Universe) { - HostDataSourceFactory::destroy(); - HostDataSourceFactory::create(validConnectionString()); - myhdsptr_ = (MySqlHostDataSource *) &(HostDataSourceFactory::instance()); + HostDataSourceFactory::destroy(); + HostDataSourceFactory::create(validConnectionString()); + hdsptr_ = &(HostDataSourceFactory::instance()); } }; /// @brief Check that database can be opened /// -/// This test checks if the MySqlLeaseMgr can be instantiated. This happens +/// This test checks if the MySqlHostDataSource can be instantiated. This happens /// only if the database can be opened. Note that this is not part of the /// MySqlLeaseMgr test fixure set. This test checks that the database can be /// opened: the fixtures assume that and check basic operations. -TEST(MySqlOpenTest, OpenDatabase) { +TEST(MySqlHostDataSource, OpenDatabase) { // Schema needs to be created for the test to work. destroySchema(); @@ -212,7 +210,7 @@ TEST(MySqlOpenTest, OpenDatabase) { // Check that lease manager open the database opens correctly and tidy up. // If it fails, print the error message. try { - HostDataSourceFactory::create(validConnectionString()); + HostDataSourceFactory::create(validConnectionString()); EXPECT_NO_THROW((void) HostDataSourceFactory::instance()); HostDataSourceFactory::destroy(); } catch (const isc::Exception& ex) { -- 2.47.2