From: Thomas Markwalder Date: Thu, 11 Oct 2018 18:15:25 +0000 (-0400) Subject: [#101,!53] Addressed review comments X-Git-Tag: 165-netbsd-8-fixes_base~1^2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f2431e6c52ca5e9f0f87747e745a43b7995df53a;p=thirdparty%2Fkea.git [#101,!53] Addressed review comments --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index c537aa10bb..1a44182085 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -2690,12 +2690,14 @@ MySqlConfigBackendDHCPv4::getPort() const { return (0); } -void +bool MySqlConfigBackendDHCPv4::registerBackendType() { - dhcp::ConfigBackendDHCPv4Mgr::instance().registerBackendFactory("mysql", - [](const db::DatabaseConnection::ParameterMap& params) -> dhcp::ConfigBackendDHCPv4Ptr { + return ( + dhcp::ConfigBackendDHCPv4Mgr::instance().registerBackendFactory("mysql", + [](const db::DatabaseConnection::ParameterMap& params) -> dhcp::ConfigBackendDHCPv4Ptr { return (dhcp::MySqlConfigBackendDHCPv4Ptr(new dhcp::MySqlConfigBackendDHCPv4(params))); - }); + }) + ); } void diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h index da8f918316..0e5fba1841 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.h @@ -391,7 +391,8 @@ public: /// @brief Registers the MySQL backend factory with backend config manager /// /// This should be called by the hook lib load() function. - static void registerBackendType(); + /// @return True if the factory was registered successfully, false otherwise. + static bool registerBackendType(); /// @brief Unregisters the MySQL backend factory and discards MySQL backends /// diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc index e5d9bae6f2..efbf713367 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc @@ -9,18 +9,20 @@ #include #include #include +#include #include #include using namespace isc::data; using namespace isc::dhcp; +using namespace isc::dhcp::test; using namespace isc::db; using namespace isc::db::test; namespace { /// @brief Test fixture class for @c MySqlConfigBackendDHCPv4Mgr. -class MySqlConfigBackendDHCPv4MgrTest : public ::testing::Test { +class MySqlConfigBackendDHCPv4MgrTest : public GenericBackendTest { public: /// @brief Constructor. MySqlConfigBackendDHCPv4MgrTest() { @@ -34,6 +36,8 @@ public: /// @brief Destructor. virtual ~MySqlConfigBackendDHCPv4MgrTest() { + // Destroy the mgr. + ConfigBackendDHCPv4Mgr::destroy(); destroyMySQLSchema(); } }; diff --git a/src/lib/config_backend/base_config_backend_mgr.h b/src/lib/config_backend/base_config_backend_mgr.h index bf0dcca025..72690369b1 100644 --- a/src/lib/config_backend/base_config_backend_mgr.h +++ b/src/lib/config_backend/base_config_backend_mgr.h @@ -107,15 +107,15 @@ public: /// @brief Unregisters the backend factory function for a given backend type. /// - /// The typical usage of this function is remove the factory function - /// when its type of backend is no longer supported (i.e hook lib is unloaded). - /// It should mirror the use @c registerBackendFactory and be called from the - /// hooks library @c unload function. + /// This function is used to remove the factory function and all backend instances + /// for a given backend type. Typically, it would be called when unloading the + /// a config backend hook library, and thus called by the library's @c unload + /// function. /// /// @param db_type Backend type, e.g. "mysql". /// - /// @return false if no factory for the given type was not registered, true - /// true if the factory was removed. + /// @return false if no factory for the given type was unregistered, true + /// if the factory was removed. bool unregisterBackendFactory(const std::string& db_type) { // Look for it. auto index = factories_.find(db_type); diff --git a/src/lib/dhcpsrv/config_backend_dhcp4_mgr.cc b/src/lib/dhcpsrv/config_backend_dhcp4_mgr.cc index d89165eb88..134f0200c4 100644 --- a/src/lib/dhcpsrv/config_backend_dhcp4_mgr.cc +++ b/src/lib/dhcpsrv/config_backend_dhcp4_mgr.cc @@ -23,6 +23,11 @@ ConfigBackendDHCPv4Mgr::create() { getConfigBackendDHCPv4MgrPtr().reset(new ConfigBackendDHCPv4Mgr()); } +void +ConfigBackendDHCPv4Mgr::destroy() { + getConfigBackendDHCPv4MgrPtr().reset(new ConfigBackendDHCPv4Mgr()); +} + ConfigBackendDHCPv4Mgr& ConfigBackendDHCPv4Mgr::instance() { boost::scoped_ptr& cb_dhcp4_mgr = getConfigBackendDHCPv4MgrPtr(); diff --git a/src/lib/dhcpsrv/config_backend_dhcp4_mgr.h b/src/lib/dhcpsrv/config_backend_dhcp4_mgr.h index c13abe8355..53073c1e38 100644 --- a/src/lib/dhcpsrv/config_backend_dhcp4_mgr.h +++ b/src/lib/dhcpsrv/config_backend_dhcp4_mgr.h @@ -18,33 +18,40 @@ namespace dhcp { /// @brief Configuration Backend Manager for DHPCv4 servers. /// /// Implements the "manager" class which holds information about the -/// supported and configured backends and provides access to those -/// backends. This is similar to @c HostMgr and @c LeaseMgr singletons +/// supported and configured backends and provides access to those +/// backends. This is similar to @c HostMgr and @c LeaseMgr singletons /// being used by the DHCP servers. /// /// It is implemented as a singleton that can be accessed from any place -/// within the server code. This includes server configuration, data -/// fetching during normal server operation and data management, including +/// within the server code. This includes server configuration, data +/// fetching during normal server operation and data management, including /// processing of control commands implemented within hooks libraries. /// /// Unlike @c HostMgr, the it does not directly expose the API to fetch and /// manipulate the data in the database. This is done via, the Configuration -/// Backend Pool, see @c ConfigBackendPoolDHCPv4 for details. -class ConfigBackendDHCPv4Mgr : public cb::BaseConfigBackendMgr, +/// Backend Pool, see @c ConfigBackendPoolDHCPv4 for details. +class ConfigBackendDHCPv4Mgr : public cb::BaseConfigBackendMgr, public boost::noncopyable { public: /// @brief Creates new instance of the @c ConfigBackendDHCPv4Mgr. /// - /// If an instance of the @c ConfigBackendDHCPv4Mgr already exists, - /// it will be replaced by the new instance. Thus, any instances of - /// config databases will be dropped. + /// If an instance of the @c ConfigBackendDHCPv4Mgr already exists, + /// it will be replaced by the new instance. Thus, all factories + /// will be unregistered and config databases will be dropped. static void create(); + /// @brief Destroys the instance of the @c ConfigBackendDHCPv4Mgr. + /// + /// If an instance of the @c ConfigBackendDHCPv4Mgr exists, + /// it will be destroyed. Thus, all factories will be unregistered + /// and config databases will be dropped. + static void destroy(); + /// @brief Returns a sole instance of the @c ConfigBackendDHCPv4Mgr. /// /// This method should be used to retrieve an instance of the @c ConfigBackendDHCPv4Mgr /// to be used to gather/manage config backends. It returns an instance - /// of the @c ConfigBackendDHCPv4Mgr created by the @c create method. If + /// of the @c ConfigBackendDHCPv4Mgr created by the @c create method. If /// the instance doesn't exist yet, it is created using the @c create method /// with the an empty set of configuration databases. static ConfigBackendDHCPv4Mgr& instance();