From: Francis Dupont Date: Sun, 11 Feb 2018 21:46:07 +0000 (+0100) Subject: [5531] Checkpoint: 3/4 code done X-Git-Tag: trac5531p_base~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=405ae2f6a85bbc2c2123fb3075bd5259adea523c;p=thirdparty%2Fkea.git [5531] Checkpoint: 3/4 code done --- diff --git a/src/lib/dhcpsrv/base_host_data_source.h b/src/lib/dhcpsrv/base_host_data_source.h index 2e3ed16db1..e68f5d1729 100644 --- a/src/lib/dhcpsrv/base_host_data_source.h +++ b/src/lib/dhcpsrv/base_host_data_source.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -14,6 +14,8 @@ #include #include +#include + namespace isc { namespace dhcp { @@ -245,7 +247,8 @@ public: /// is identified by HW address, another one by DUID. /// /// @param host Pointer to the new @c Host object being added. - virtual void add(const HostPtr& host) = 0; + /// @return true if addition was successful. + virtual bool add(const HostPtr& host) = 0; /// @brief Attempts to delete a host by (subnet-id, address) /// @@ -310,6 +313,9 @@ public: /// @brief HostDataSource pointer typedef boost::shared_ptr HostDataSourcePtr; +/// @brief HostDataSource list +typedef std::vector HostDataSourceList; + } } diff --git a/src/lib/dhcpsrv/cfg_db_access.cc b/src/lib/dhcpsrv/cfg_db_access.cc index 366cdf4be2..44c4b67040 100644 --- a/src/lib/dhcpsrv/cfg_db_access.cc +++ b/src/lib/dhcpsrv/cfg_db_access.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -44,9 +44,9 @@ CfgDbAccess::createManagers() const { LeaseMgrFactory::create(getLeaseDbAccessString()); // Recreate host data source. - HostDataSourceFactory::destroy(); + HostMgr::create(); if (!host_db_access_.empty()) { - HostMgr::create(getHostDbAccessString()); + HostMgr::addSource(getHostDbAccessString()); } } diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index b7f52d405e..1a99efba58 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/src/lib/dhcpsrv/cfg_hosts.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -549,7 +549,7 @@ CfgHosts::getHostInternal(const SubnetID& subnet_id, const bool subnet6, return (host); } -void +bool CfgHosts::add(const HostPtr& host) { LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_CFG_ADD_HOST) .arg(host ? host->toText() : "(no-host)"); @@ -569,6 +569,8 @@ CfgHosts::add(const HostPtr& host) { add4(host); add6(host); + + return (true); } void diff --git a/src/lib/dhcpsrv/cfg_hosts.h b/src/lib/dhcpsrv/cfg_hosts.h index 0717e9a174..519101215e 100644 --- a/src/lib/dhcpsrv/cfg_hosts.h +++ b/src/lib/dhcpsrv/cfg_hosts.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -318,9 +318,10 @@ public: /// /// @param host Pointer to the new @c Host object being added. /// + /// @return always return true as additions are successful or throw. /// @throw DuplicateHost If a host for a particular HW address or DUID /// has already been added to the IPv4 or IPv6 subnet. - virtual void add(const HostPtr& host); + virtual bool add(const HostPtr& host); /// @brief Attempts to delete a host by address. /// diff --git a/src/lib/dhcpsrv/cql_host_data_source.cc b/src/lib/dhcpsrv/cql_host_data_source.cc index 7e2416cceb..38d0bb84b5 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.cc +++ b/src/lib/dhcpsrv/cql_host_data_source.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Deutsche Telekom AG. +// Copyright (C) 2016-2018 Deutsche Telekom AG. // // Author: Andrei Pavel // @@ -1820,11 +1820,13 @@ CqlHostDataSource::~CqlHostDataSource() { delete impl_; } -void +bool CqlHostDataSource::add(const HostPtr& host) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_CQL_HOST_ADD); impl_->add(host); + + return (true); } ConstHostCollection diff --git a/src/lib/dhcpsrv/cql_host_data_source.h b/src/lib/dhcpsrv/cql_host_data_source.h index 60ea0a805c..ceb721bd98 100644 --- a/src/lib/dhcpsrv/cql_host_data_source.h +++ b/src/lib/dhcpsrv/cql_host_data_source.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Deutsche Telekom AG. +// Copyright (C) 2016-2018 Deutsche Telekom AG. // // Author: Andrei Pavel // @@ -98,7 +98,8 @@ public: /// Host, where one instance is identified by different identifier types. /// /// @param host pointer to the new @ref Host being added. - virtual void add(const HostPtr& host) override; + /// @return true as addition is successful or throws. + virtual bool add(const HostPtr& host) override; /// @brief Retrieves a single @ref Host connected to an IPv4 subnet. /// diff --git a/src/lib/dhcpsrv/host_data_source_factory.cc b/src/lib/dhcpsrv/host_data_source_factory.cc index 3a63770cba..3bb700be81 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.cc +++ b/src/lib/dhcpsrv/host_data_source_factory.cc @@ -40,14 +40,9 @@ namespace dhcp { map HostDataSourceFactory::map_; -HostDataSourcePtr& -HostDataSourceFactory::getHostDataSourcePtr() { - static HostDataSourcePtr hostDataSourcePtr; - return (hostDataSourcePtr); -} - void -HostDataSourceFactory::create(const string& dbaccess) { +HostDataSourceFactory::add(HostDataSourceList& sources, + const string& dbaccess) { // Parse the access string and create a redacted string for logging. DatabaseConnection::ParameterMap parameters = DatabaseConnection::parse(dbaccess); @@ -68,25 +63,30 @@ HostDataSourceFactory::create(const string& dbaccess) { db_type << " is invalid"); } - // Call the factory - getHostDataSourcePtr().reset(index->second(parameters)); + // Call the factory and push the pointer on sources. + sources.push_back(boost::shared_ptr(index->second(parameters))); // Check the factory did not return NULL. - if (!getHostDataSourcePtr()) { + if (!sources.back()) { + sources.pop_back(); isc_throw(Unexpected, "Hosts database " << db_type << " factory returned NULL"); } } -void -HostDataSourceFactory::destroy() { - // Destroy current host data source instance. This is a no-op if no host - // data source is available. - if (getHostDataSourcePtr()) { +bool +HostDataSourceFactory::del(HostDataSourceList& sources, + const string& db_type) { + for (auto it = sources.begin(); it != sources.end(); ++it) { + if ((*it)->getType() != db_type) { + continue; + } LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, HOSTS_CFG_CLOSE_HOST_DATA_SOURCE) - .arg(getHostDataSourcePtr()->getType()); + .arg(db_type); + sources.erase(it); + return (true); } - getHostDataSourcePtr().reset(); + return (false); } bool diff --git a/src/lib/dhcpsrv/host_data_source_factory.h b/src/lib/dhcpsrv/host_data_source_factory.h index 68492213dc..a8e2038d49 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.h +++ b/src/lib/dhcpsrv/host_data_source_factory.h @@ -40,21 +40,18 @@ public: class HostDataSourceFactory { public: - /// @brief Create an instance of a host data source. + /// @brief Create and add an instance of a host data source. /// /// Each database backend has its own host data source type. This static - /// method sets the "current" host data source to be an object of the - /// appropriate type. The actual host data source is returned by the - /// "instance" method. - /// - /// @note When called, the current host data source is always destroyed - /// and a new one created - even if the parameters are the same. + /// method adds an object of the appropriate type to a list of + /// host data sources. /// /// dbaccess is a generic way of passing parameters. Parameters are passed /// in the "name=value" format, separated by spaces. The data MUST include /// a keyword/value pair of the form "type=dbtype" giving the database /// type, e.q. "mysql" or "sqlite3". /// + /// @param sources host data source list. /// @param dbaccess Database access parameters. These are in the form of /// "keyword=value" pairs, separated by spaces. They are backend- /// -end specific, although must include the "type" keyword which @@ -64,21 +61,17 @@ public: /// keyword. /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not /// identify a supported backend. - static void create(const std::string& dbaccess); + static void add(HostDataSourceList& sources, const std::string& dbaccess); - /// @brief Destroy host data source + /// @brief Delete a host data source. /// - /// Destroys the current host data source object. This should have the effect - /// of closing the database connection. The method is a no-op if no - /// host data source is available. - static void destroy(); - - /// @brief Hold pointer to host data source instance + /// Delete the first instance of a host data source of the given type. + /// This should have the effect of closing the database connection. /// - /// Holds a pointer to the singleton host data source. The singleton - /// is encapsulated in this method to avoid a "static initialization - /// fiasco" if defined in an external static variable. - static HostDataSourcePtr& getHostDataSourcePtr(); + /// @param sources host data source list. + /// @param db_type database backend type. + /// @return true when found and removed, false when not found. + static bool del(HostDataSourceList& sources, const std::string& db_type); /// @brief Type of host data source factory /// diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 302da6c0d7..b04bd3f0f9 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -37,24 +37,31 @@ HostMgr::getHostMgrPtr() { } void -HostMgr::create(const std::string& access) { +HostMgr::create() { getHostMgrPtr().reset(new HostMgr()); +} - if (!access.empty()) { - // If the user specified parameters, let's pass them to the create - // method. It will destroy any prior instances and will create - // the new one. - HostDataSourceFactory::create(access); - } else { - // Ok, no parameters were specified. We should destroy the existing - // instance. - HostDataSourceFactory::destroy(); - } +void +HostMgr::addSource(const std::string& access) { + HostDataSourceFactory::add(getHostMgrPtr()->alternate_sources_, access); +} + +bool +HostMgr::delSource(const std::string& db_type) { + return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_, db_type)); +} - // Now store the host data source pointer. It may be NULL. That's ok as - // NULL value indicates that there's no host data source configured. - getHostMgrPtr()->alternate_source_ = - HostDataSourceFactory::getHostDataSourcePtr(); +void +HostMgr::delAllSources() { + getHostMgrPtr()->alternate_sources_.clear(); +} + +HostDataSourcePtr +HostMgr::getHostDataSource() const { + if (alternate_sources_.empty()) { + return (HostDataSourcePtr()); + } + return (alternate_sources_[0]); } HostMgr& @@ -69,8 +76,9 @@ HostMgr::instance() { ConstHostCollection HostMgr::getAll(const HWAddrPtr& hwaddr, const DuidPtr& duid) const { ConstHostCollection hosts = getCfgHosts()->getAll(hwaddr, duid); - if (alternate_source_) { - ConstHostCollection hosts_plus = alternate_source_->getAll(hwaddr, duid); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + ConstHostCollection hosts_plus = (*it)->getAll(hwaddr, duid); hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end()); } return (hosts); @@ -83,10 +91,10 @@ HostMgr::getAll(const Host::IdentifierType& identifier_type, ConstHostCollection hosts = getCfgHosts()->getAll(identifier_type, identifier_begin, identifier_len); - if (alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { ConstHostCollection hosts_plus = - alternate_source_->getAll(identifier_type, identifier_begin, - identifier_len); + (*it)->getAll(identifier_type, identifier_begin, identifier_len); hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end()); } return (hosts); @@ -96,8 +104,9 @@ HostMgr::getAll(const Host::IdentifierType& identifier_type, ConstHostCollection HostMgr::getAll4(const IOAddress& address) const { ConstHostCollection hosts = getCfgHosts()->getAll4(address); - if (alternate_source_) { - ConstHostCollection hosts_plus = alternate_source_->getAll4(address); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + ConstHostCollection hosts_plus = (*it)->getAll4(address); hosts.insert(hosts.end(), hosts_plus.begin(), hosts_plus.end()); } return (hosts); @@ -107,17 +116,21 @@ ConstHostPtr HostMgr::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, const DuidPtr& duid) const { ConstHostPtr host = getCfgHosts()->get4(subnet_id, hwaddr, duid); - if (!host && alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_HWADDR_DUID) .arg(subnet_id) .arg(hwaddr ? hwaddr->toText() : "(no-hwaddr)") .arg(duid ? duid->toText() : "(duid)"); if (duid) { - host = alternate_source_->get4(subnet_id, HWAddrPtr(), duid); + host = (*it)->get4(subnet_id, HWAddrPtr(), duid); } if (!host && hwaddr) { - host = alternate_source_->get4(subnet_id, hwaddr, DuidPtr()); + host = (*it)->get4(subnet_id, hwaddr, DuidPtr()); } } return (host); @@ -130,32 +143,37 @@ HostMgr::get4(const SubnetID& subnet_id, const size_t identifier_len) const { ConstHostPtr host = getCfgHosts()->get4(subnet_id, identifier_type, identifier_begin, identifier_len); - if (!host && alternate_source_) { - + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER) .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, identifier_len)); - host = alternate_source_->get4(subnet_id, identifier_type, - identifier_begin, identifier_len); + host = (*it)->get4(subnet_id, identifier_type, + identifier_begin, identifier_len); if (host) { LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_HOST) .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, identifier_len)) .arg(host->toText()); - - } else { - LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, - HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_NULL) - .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, - identifier_len)); + break; } + LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, + HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_NULL) + .arg(subnet_id) + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, + identifier_len)); } return (host); @@ -165,12 +183,16 @@ ConstHostPtr HostMgr::get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const { ConstHostPtr host = getCfgHosts()->get4(subnet_id, address); - if (!host && alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4) .arg(subnet_id) .arg(address.toText()); - host = alternate_source_->get4(subnet_id, address); + host = (*it)->get4(subnet_id, address); } return (host); } @@ -180,17 +202,21 @@ ConstHostPtr HostMgr::get6(const SubnetID& subnet_id, const DuidPtr& duid, const HWAddrPtr& hwaddr) const { ConstHostPtr host = getCfgHosts()->get6(subnet_id, duid, hwaddr); - if (!host && alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_DUID_HWADDR) .arg(subnet_id) .arg(duid ? duid->toText() : "(duid)") .arg(hwaddr ? hwaddr->toText() : "(no-hwaddr)"); if (duid) { - host = alternate_source_->get6(subnet_id, duid, HWAddrPtr()); + host = (*it)->get6(subnet_id, duid, HWAddrPtr()); } if (!host && hwaddr) { - host = alternate_source_->get6(subnet_id, DuidPtr(), hwaddr); + host = (*it)->get6(subnet_id, DuidPtr(), hwaddr); } } return (host); @@ -199,12 +225,16 @@ HostMgr::get6(const SubnetID& subnet_id, const DuidPtr& duid, ConstHostPtr HostMgr::get6(const IOAddress& prefix, const uint8_t prefix_len) const { ConstHostPtr host = getCfgHosts()->get6(prefix, prefix_len); - if (!host && alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET6_PREFIX) .arg(prefix.toText()) .arg(static_cast(prefix_len)); - host = alternate_source_->get6(prefix, prefix_len); + host = (*it)->get6(prefix, prefix_len); } return (host); } @@ -216,34 +246,37 @@ HostMgr::get6(const SubnetID& subnet_id, const size_t identifier_len) const { ConstHostPtr host = getCfgHosts()->get6(subnet_id, identifier_type, identifier_begin, identifier_len); - if (!host && alternate_source_) { - + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_IDENTIFIER) .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, identifier_len)); - - host = alternate_source_->get6(subnet_id, identifier_type, - identifier_begin, identifier_len); + host = (*it)->get6(subnet_id, identifier_type, + identifier_begin, identifier_len); if (host) { - LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, - HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_IDENTIFIER_HOST) - .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, - identifier_len)) - .arg(host->toText()); - - } else { - LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, - HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_IDENTIFIER_NULL) - .arg(subnet_id) - .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, - identifier_len)); + LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, + HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_IDENTIFIER_HOST) + .arg(subnet_id) + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, + identifier_len)) + .arg(host->toText()); + break; } - + LOG_DEBUG(hosts_logger, HOSTS_DBG_RESULTS, + HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_IDENTIFIER_NULL) + .arg(subnet_id) + .arg(Host::getIdentifierAsText(identifier_type, + identifier_begin, + identifier_len)); } return (host); } @@ -252,57 +285,86 @@ ConstHostPtr HostMgr::get6(const SubnetID& subnet_id, const asiolink::IOAddress& addr) const { ConstHostPtr host = getCfgHosts()->get6(subnet_id, addr); - if (!host && alternate_source_) { + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if (host) { + break; + } LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_MGR_ALTERNATE_GET6_SUBNET_ID_ADDRESS6) .arg(subnet_id) .arg(addr.toText()); - host = alternate_source_->get6(subnet_id, addr); + host = (*it)->get6(subnet_id, addr); } return (host); } -void +bool HostMgr::add(const HostPtr& host) { - if (!alternate_source_) { + if (alternate_sources_.empty()) { isc_throw(NoHostDataSourceManager, "Unable to add new host because there is " "no hosts-database configured."); } - alternate_source_->add(host); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if ((*it)->add(host)) { + return (true); + } + } + // This should never happen as at least one backend implements addition. + return (false); } bool HostMgr::del(const SubnetID& subnet_id, const asiolink::IOAddress& addr) { - if (!alternate_source_) { + if (alternate_sources_.empty()) { isc_throw(NoHostDataSourceManager, "Unable to delete a host because there is " "no hosts-database configured."); } - return (alternate_source_->del(subnet_id, addr)); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if ((*it)->del(subnet_id, addr)) { + return (true); + } + } + return (false); } bool HostMgr::del4(const SubnetID& subnet_id, const Host::IdentifierType& identifier_type, const uint8_t* identifier_begin, const size_t identifier_len) { - if (!alternate_source_) { + if (alternate_sources_.empty()) { isc_throw(NoHostDataSourceManager, "Unable to delete a host because there is " "no hosts-database configured."); } - return (alternate_source_->del4(subnet_id, identifier_type, - identifier_begin, identifier_len)); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if ((*it)->del4(subnet_id, identifier_type, + identifier_begin, identifier_len)) { + return (true); + } + } + return (false); } bool HostMgr::del6(const SubnetID& subnet_id, const Host::IdentifierType& identifier_type, const uint8_t* identifier_begin, const size_t identifier_len) { - if (!alternate_source_) { + if (alternate_sources_.empty()) { isc_throw(NoHostDataSourceManager, "unable to delete a host because there is " "no alternate host data source present"); } - return (alternate_source_->del6(subnet_id, identifier_type, - identifier_begin, identifier_len)); + for (auto it = alternate_sources_.begin(); + it != alternate_sources_.end(); ++it) { + if ((*it)->del6(subnet_id, identifier_type, + identifier_begin, identifier_len)) { + return (true); + } + } + return (false); } } // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index c177f58829..dca2697efa 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -33,42 +33,53 @@ namespace dhcp { /// configuration, accessible through the @c CfgHosts object in the @c CfgMgr. /// The @c CfgHosts object holds all reservations specified in the DHCP server /// configuration file. If a particular reservation is not found in the -/// @c CfgHosts object, the @c HostMgr will try to find it using the alternate -/// host data storage. The alternate host data storage is usually a database +/// @c CfgHosts object, the @c HostMgr will try to find it using alternate +/// host data storages. An alternate host data storage is usually a database /// (e.g. SQL database), accessible through a dedicated host data source /// object (a.k.a. database backend). This datasource is responsible for /// managing the connection with the database and forming appropriate queries /// to retrieve (or update) the information about the reservations. /// -/// The use of the alternate host data source is optional and usually requires +/// The use of alternate host data sources is optional and usually requires /// additional configuration to be specified by the server administrator. /// For example, for the SQL database the user's credentials, database address, /// and database name are required. The @c HostMgr passes these parameters /// to an appropriate datasource which is responsible for opening a connection /// and maintaining it. /// -/// It is possible to switch to a different alternate data source or disable -/// the use of the alternate datasource, e.g. as a result of server's +/// It is possible to switch to different alternate data sources or disable +/// the use of alternate datasources, e.g. as a result of server's /// reconfiguration. However, the use of the primary host data source (i.e. /// reservations specified in the configuration file) can't be disabled. -/// -/// @todo Implement alternate host data sources: MySQL, PostgreSQL, etc. class HostMgr : public boost::noncopyable, public BaseHostDataSource { public: /// @brief Creates new instance of the @c HostMgr. /// /// If an instance of the @c HostMgr already exists, it will be replaced - /// by the new instance. Thus, any instances of the alternate host data + /// by the new instance. Thus, any instances of alternate host data /// sources will be dropped. /// + static void create(); + + /// @brief Add an alternate host data source. + /// /// @param access Host data source access parameters for the alternate /// host data source. It holds "keyword=value" pairs, separated by spaces. /// The supported values are specific to the alternate data source in use. /// However, the "type" parameter will be common and it will specify which /// data source is to be used. Currently, no parameters are supported /// and the parameter is ignored. - static void create(const std::string& access = ""); + static void addSource(const std::string& access); + + /// @brief Delete an alternate host data source. + /// + /// @param db_type_type database backend type. + /// @return true when found and removed, false when not found. + static bool delSource(const std::string& db_type); + + /// @brief Delete all alternate host data source. + static void delAllSources(); /// @brief Returns a sole instance of the @c HostMgr. /// @@ -250,7 +261,8 @@ public: /// in use. /// /// @param host Pointer to the new @c Host object being added. - virtual void add(const HostPtr& host); + /// @return true if addition was successful. + virtual bool add(const HostPtr& host); /// @brief Return backend type /// @@ -261,28 +273,33 @@ public: return (std::string("host_mgr")); } - /// @brief Returns pointer to the host data source + /// @brief Returns the host data source list. /// - /// May return NULL - /// @return pointer to the host data source (or NULL) - HostDataSourcePtr getHostDataSource() const { - return (alternate_source_); + /// @return reference to the host data source list. + HostDataSourceList& getHostDataSourceList() { + return (alternate_sources_); } - /// @brief Sets the alternate host data source. + /// @brief Returns the fist host data source. + /// + /// May return NULL if the host data source list is empty. + /// @return pointer to the first host data source (or NULL) + HostDataSourcePtr getHostDataSource() const; + + /// @brief Sets alternate host data source list. /// /// Note: This should be used only for testing. Do not use /// in production. Normal control flow assumes that - /// HostMgr::create(...) is called and it instantiates - /// appropriate host data source. However, some tests + /// HostMgr::create() and HostMgr::add() is called and it instantiates + /// appropriate host data sources. However, some tests /// (e.g. host_cmds) implement their own very simple /// data source. It's not production ready by any means, /// so it does not belong in host_data_source_factory.cc. /// The testing nature of this method is reflected in its name. /// - /// @param source new source to be set (may be NULL) - void setTestHostDataSource(const HostDataSourcePtr& source) { - alternate_source_ = source; + /// @param sources new source list to be set + void setTestHostDataSourceList(const HostDataSourceList& sources) { + alternate_sources_ = sources; } /// @brief Attempts to delete a host by address. @@ -327,10 +344,8 @@ private: /// @brief Private default constructor. HostMgr() { } - /// @brief Pointer to an alternate host data source. - /// - /// If this pointer is NULL, the source is not in use. - HostDataSourcePtr alternate_source_; + /// @brief List of alternate host data sources. + HostDataSourceList alternate_sources_; /// @brief Returns a pointer to the currently used instance of the /// @c HostMgr. diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index db8437f1cb..1d41789674 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -2562,7 +2562,7 @@ MySqlHostDataSource::~MySqlHostDataSource() { delete impl_; } -void +bool MySqlHostDataSource::add(const HostPtr& host) { // If operating in read-only mode, throw exception. impl_->checkReadOnly(); @@ -2607,6 +2607,8 @@ MySqlHostDataSource::add(const HostPtr& host) { // Everything went fine, so explicitly commit the transaction. transaction.commit(); + + return (true); } bool diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 3850d8cd3c..7bedf1bff1 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -223,7 +223,8 @@ public: /// one instance is identified by HW address, another one by DUID. /// /// @param host Pointer to the new @c Host object being added. - virtual void add(const HostPtr& host); + /// @return true as addition is successful or throws. + virtual bool add(const HostPtr& host); /// @brief Attempts to delete a host by (subnet-id, address) /// diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 252f22cc4c..02f9b75e27 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1934,7 +1934,7 @@ PgSqlHostDataSource::~PgSqlHostDataSource() { delete impl_; } -void +bool PgSqlHostDataSource::add(const HostPtr& host) { // If operating in read-only mode, throw exception. impl_->checkReadOnly(); @@ -1977,6 +1977,8 @@ PgSqlHostDataSource::add(const HostPtr& host) { // Everything went fine, so explicitly commit the transaction. transaction.commit(); + + return (true); } bool diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h index a737342fc7..c8a2b96c2d 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.h +++ b/src/lib/dhcpsrv/pgsql_host_data_source.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -247,9 +247,10 @@ public: /// -# Address and Prefix Length must be unique (DuplicateEntry) /// /// @param host Pointer to the new @c Host object being added. + /// @return true as addition is successful or throws. /// @throw DuplicateEntry or DbOperationError dependent on the constraint /// violation - virtual void add(const HostPtr& host); + virtual bool add(const HostPtr& host); /// @brief Attempts to delete a host by (subnet-id, address) /// diff --git a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc index e8b5544da1..59af1339e1 100644 --- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -42,7 +43,8 @@ public: // Connect to the database try { - HostDataSourceFactory::create(validCqlConnectionString()); + HostMgr::create(); + HostMgr::addSource(validCqlConnectionString()); } catch (...) { std::cerr << "*** ERROR: unable to open database. The test" "*** environment is broken and must be fixed before" @@ -52,7 +54,7 @@ public: throw; } - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + hdsptr_ = HostMgr::instance().getHostDataSource(); } /// @brief Destroys the HDS and the schema. @@ -63,7 +65,8 @@ public: // Rollback should never fail, as Cassandra doesn't support transactions // (commit and rollback are both no-op). } - HostDataSourceFactory::destroy(); + HostMgr::delAllSources(); + hdsptr_.reset(); destroyCqlSchema(false, true); } @@ -91,9 +94,9 @@ public: /// Parameter is ignored for CQL backend as the v4 and v6 leases share /// the same database. void reopen(Universe) { - HostDataSourceFactory::destroy(); - HostDataSourceFactory::create(validCqlConnectionString()); - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + HostMgr::create(); + HostMgr::addSource(validCqlConnectionString()); + hdsptr_ = HostMgr::instance().getHostDataSource(); } }; @@ -112,9 +115,9 @@ TEST(CqlHostDataSource, OpenDatabase) { // Check that lease manager open the database opens correctly and tidy up. // If it fails, print the error message. try { - HostDataSourceFactory::create(validCqlConnectionString()); - EXPECT_NO_THROW((void)HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + HostMgr::create(); + EXPECT_NO_THROW(HostMgr::addSource(validCqlConnectionString())); + HostMgr::delSource("cql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -127,9 +130,9 @@ TEST(CqlHostDataSource, OpenDatabase) { try { std::string connection_string = validCqlConnectionString() + std::string(" ") + std::string(VALID_TIMEOUT); - HostDataSourceFactory::create(connection_string); - EXPECT_NO_THROW((void)HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + HostMgr::create(); + EXPECT_NO_THROW(HostMgr::addSource(connection_string)); + HostMgr::delSource("cql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -139,39 +142,39 @@ TEST(CqlHostDataSource, OpenDatabase) { // Check that attempting to get an instance of the host data source when // none is set throws an exception. - EXPECT_FALSE(HostDataSourceFactory::getHostDataSourcePtr()); + EXPECT_FALSE(HostMgr::instance().getHostDataSource()); // Check that wrong specification of backend throws an exception. // (This is really a check on HostDataSourceFactory, but is convenient to // perform here.) - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( NULL, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)), InvalidParameter); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( INVALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)), InvalidType); // Check that invalid login data does not cause an exception, CQL should use // default values. - EXPECT_NO_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_NO_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, INVALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD))); - EXPECT_NO_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_NO_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD))); - EXPECT_NO_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_NO_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD))); - EXPECT_NO_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_NO_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD))); // Check that invalid timeouts throw DbOperationError. - EXPECT_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_1)), DbOperationError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_2)), DbOperationError); // Check that CQL allows the hostname to not be specified. - EXPECT_NO_THROW(HostDataSourceFactory::create(connectionString(CQL_VALID_TYPE, + EXPECT_NO_THROW(HostMgr::addSource(connectionString(CQL_VALID_TYPE, NULL, VALID_HOST, INVALID_USER, VALID_PASSWORD))); // Tidy up after the test 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 1209677ace..a66d5e2c9e 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -585,12 +586,12 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { // Close the database connection and reopen in "read-only" mode as // specified by the "VALID_READONLY_DB" parameter. - HostDataSourceFactory::destroy(); - HostDataSourceFactory::create(connectionString( + HostMgr::create(); + HostMgr::addSource(connectionString( valid_db_type, VALID_NAME, VALID_HOST, VALID_READONLY_USER, VALID_PASSWORD, VALID_READONLY_DB)); - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + hdsptr_ = HostMgr::instance().getHostDataSource(); // Check that an attempt to insert new host would result in // exception. diff --git a/src/lib/dhcpsrv/tests/host_data_source_factory_unittest.cc b/src/lib/dhcpsrv/tests/host_data_source_factory_unittest.cc new file mode 100644 index 0000000000..0ff2a34e2e --- /dev/null +++ b/src/lib/dhcpsrv/tests/host_data_source_factory_unittest.cc @@ -0,0 +1,248 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include + +#include +#include + +#include + +#include +#include + +using namespace std; +using namespace isc; +using namespace isc::dhcp; + +namespace { + +// @brief Names of test backends +enum Names { FOOBAR, FOO, BAR }; + +// @brief Convert a name to a string +string toString(Names name) { + switch (name) { + case FOOBAR: + return ("foobar"); + case FOO: + return ("foo"); + default: + return ("bar"); + } +} + +// A host data source +template +class FakeHostDataSource : public BaseHostDataSource { +public: + // @brief Constructor + FakeHostDataSource(const DatabaseConnection::ParameterMap&) { } + + virtual ~FakeHostDataSource() { } + + // The base class is abstract so we need to define methods + + virtual ConstHostCollection + getAll(const HWAddrPtr&, const DuidPtr&) const { + return (ConstHostCollection()); + } + + virtual ConstHostCollection + getAll(const Host::IdentifierType&, const uint8_t*, const size_t) const { + return (ConstHostCollection()); + } + + virtual ConstHostCollection + getAll4(const asiolink::IOAddress&) const { + return (ConstHostCollection()); + } + + virtual ConstHostPtr + get4(const SubnetID&, const HWAddrPtr&, const DuidPtr&) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get4(const SubnetID&, const Host::IdentifierType&, + const uint8_t*, const size_t) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get4(const SubnetID&, const asiolink::IOAddress&) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get6(const SubnetID&, const DuidPtr&, const HWAddrPtr&) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get6(const SubnetID&, const Host::IdentifierType&, + const uint8_t*, const size_t) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get6(const asiolink::IOAddress&, const uint8_t) const { + return (ConstHostPtr()); + } + + virtual ConstHostPtr + get6(const SubnetID&, const asiolink::IOAddress&) const { + return (ConstHostPtr()); + } + + virtual bool add(const HostPtr&) { + return (false); + } + + virtual bool del(const SubnetID&, const asiolink::IOAddress&) { + return (false); + } + + virtual bool del4(const SubnetID&, const Host::IdentifierType&, + const uint8_t*, const size_t) { + return (false); + } + + virtual bool del6(const SubnetID&, const Host::IdentifierType&, + const uint8_t*, const size_t) { + return (false); + } + + virtual string getType() const { + return (toString(NAME)); + } +}; + +// @brief Factory function template +template +BaseHostDataSource* +factory(const DatabaseConnection::ParameterMap& parameters) { + return (new FakeHostDataSource(parameters)); +} + +// @brief Register factory template +template +bool registerFactory() { + return (HostDataSourceFactory::registerFactory(toString(NAME), + factory)); +} + +// @brief Factory function returning 0 +BaseHostDataSource* factory0(const DatabaseConnection::ParameterMap&) { + return (0); +} + +// @brief Test fixture class +class HostDataSourceFactoryTest : public ::testing::Test { +private: + // @brief Prepares the class for a test. + virtual void SetUp() { + } + + // @brief Cleans up after the test. + virtual void TearDown() { + sources_.clear(); + HostDataSourceFactory::deregisterFactory("foobar"); + HostDataSourceFactory::deregisterFactory("foo"); + HostDataSourceFactory::deregisterFactory("bar"); + } +public: + HostDataSourceList sources_; +}; + +// Verify a factory can be registered and only once. +TEST_F(HostDataSourceFactoryTest, registerFactory) { + EXPECT_TRUE(registerFactory()); + + // Only once + EXPECT_FALSE(registerFactory()); +} + +// Verify a factory can be registered and deregistered +TEST_F(HostDataSourceFactoryTest, deregisterFactory) { + // Does not exist at the beginning + EXPECT_FALSE(HostDataSourceFactory::deregisterFactory("foobar")); + + // Register and deregister + EXPECT_TRUE(registerFactory()); + EXPECT_TRUE(HostDataSourceFactory::deregisterFactory("foobar")); + + // No longer exists + EXPECT_FALSE(HostDataSourceFactory::deregisterFactory("foobar")); +} + +// Verify a registered factory can be called +TEST_F(HostDataSourceFactoryTest, add) { + EXPECT_TRUE(registerFactory()); + EXPECT_NO_THROW(HostDataSourceFactory::add(sources_, "type=foobar")); + ASSERT_EQ(1, sources_.size()); + EXPECT_EQ("foobar", sources_[0]->getType()); +} + +// Verify that type is required +TEST_F(HostDataSourceFactoryTest, notype) { + EXPECT_THROW(HostDataSourceFactory::add(sources_, "tp=foobar"), + InvalidParameter); + EXPECT_THROW(HostDataSourceFactory::add(sources_, "type=foobar"), + InvalidType); +} + +// Verify that factory must not return NULL +TEST_F(HostDataSourceFactoryTest, null) { + EXPECT_TRUE(HostDataSourceFactory::registerFactory("foobar", factory0)); + EXPECT_THROW(HostDataSourceFactory::add(sources_, "type=foobar"), + Unexpected); +} + +// Verify del class method +TEST_F(HostDataSourceFactoryTest, del) { + // No sources at the beginning + EXPECT_FALSE(HostDataSourceFactory::del(sources_, "foobar")); + + // Add foobar + EXPECT_TRUE(registerFactory()); + EXPECT_NO_THROW(HostDataSourceFactory::add(sources_, "type=foobar")); + ASSERT_EQ(1, sources_.size()); + + // Delete another + EXPECT_FALSE(HostDataSourceFactory::del(sources_, "another")); + + // Delete foobar + EXPECT_TRUE(HostDataSourceFactory::del(sources_, "foobar")); + + // No longer foobar in sources + EXPECT_FALSE(HostDataSourceFactory::del(sources_, "foobar")); +} + +// Verify add and del class method on multiple backends +TEST_F(HostDataSourceFactoryTest, multiple) { + // Add foo twice + EXPECT_TRUE(registerFactory()); + EXPECT_NO_THROW(HostDataSourceFactory::add(sources_, "type=foo")); + EXPECT_NO_THROW(HostDataSourceFactory::add(sources_, "type=foo")); + + // Add bar once + EXPECT_TRUE(registerFactory()); + EXPECT_NO_THROW(HostDataSourceFactory::add(sources_, "type=bar")); + + // Delete them + EXPECT_TRUE(HostDataSourceFactory::del(sources_, "foo")); + EXPECT_TRUE(HostDataSourceFactory::del(sources_, "bar")); + // Second instance of foo + EXPECT_TRUE(HostDataSourceFactory::del(sources_, "foo")); + + // No more sources + EXPECT_EQ(0, sources_.size()); + EXPECT_FALSE(HostDataSourceFactory::del(sources_, "foo")); + EXPECT_FALSE(HostDataSourceFactory::del(sources_, "bar")); +} + +}; // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc index 1f7cc680f5..0a42513c02 100644 --- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -210,8 +210,8 @@ HostMgrTest::testGetAll(BaseHostDataSource& data_source1, // If there non-matching HW address is specified, nothing should be // returned. hosts = HostMgr::instance().getAll(Host::IDENT_HWADDR, - &hwaddrs_[1]->hwaddr_[0], - hwaddrs_[1]->hwaddr_.size()); + &hwaddrs_[1]->hwaddr_[0], + hwaddrs_[1]->hwaddr_.size()); ASSERT_TRUE(hosts.empty()); // For the correct HW address, there should be two reservations. @@ -225,7 +225,7 @@ HostMgrTest::testGetAll(BaseHostDataSource& data_source1, // Look for the first reservation. bool found = false; - for (int i = 0; i < 2; ++i) { + for (unsigned i = 0; i < 2; ++i) { if (hosts[0]->getIPv4Reservation() == IOAddress("192.0.2.5")) { ASSERT_EQ(1, hosts[0]->getIPv4SubnetID()); found = true; @@ -238,7 +238,7 @@ HostMgrTest::testGetAll(BaseHostDataSource& data_source1, // Look for the second reservation. found = false; - for (int i = 0; i < 2; ++i) { + for (unsigned i = 0; i < 2; ++i) { if (hosts[1]->getIPv4Reservation() == IOAddress("192.0.3.10")) { ASSERT_EQ(10, hosts[1]->getIPv4SubnetID()); found = true; @@ -425,7 +425,7 @@ MySQLHostMgrTest::SetUp() { // Connect to the database try { - HostMgr::create(test::validMySQLConnectionString()); + HostMgr::addSource(test::validMySQLConnectionString()); } catch (...) { std::cerr << "*** ERROR: unable to open database. The test\n" "*** environment is broken and must be fixed before\n" @@ -438,8 +438,8 @@ MySQLHostMgrTest::SetUp() { void MySQLHostMgrTest::TearDown() { - HostDataSourceFactory::getHostDataSourcePtr()->rollback(); - HostDataSourceFactory::destroy(); + HostMgr::instance().getHostDataSource()->rollback(); + HostMgr::delSource("mysql"); test::destroyMySQLSchema(); } @@ -502,7 +502,7 @@ PostgreSQLHostMgrTest::SetUp() { // Connect to the database try { - HostMgr::create(test::validPgSQLConnectionString()); + HostMgr::addSource(test::validPgSQLConnectionString()); } catch (...) { std::cerr << "*** ERROR: unable to open database. The test\n" "*** environment is broken and must be fixed before\n" @@ -515,8 +515,8 @@ PostgreSQLHostMgrTest::SetUp() { void PostgreSQLHostMgrTest::TearDown() { - HostDataSourceFactory::getHostDataSourcePtr()->rollback(); - HostDataSourceFactory::destroy(); + HostMgr::instance().getHostDataSource()->rollback(); + HostMgr::delSource("postgresql"); test::destroyPgSQLSchema(); } 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 63a81b4539..ccad94341a 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -46,7 +47,8 @@ public: // Connect to the database try { - HostDataSourceFactory::create(validMySQLConnectionString()); + HostMgr::create(); + HostMgr::addSource(validMySQLConnectionString()); } catch (...) { std::cerr << "*** ERROR: unable to open database. The test\n" "*** environment is broken and must be fixed before\n" @@ -56,7 +58,7 @@ public: throw; } - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + hdsptr_ = HostMgr::instance().getHostDataSource(); } /// @brief Destructor @@ -69,7 +71,7 @@ public: } catch (...) { // Rollback may fail if backend is in read only mode. That's ok. } - HostDataSourceFactory::destroy(); + HostMgr::delAllSources(); hdsptr_.reset(); destroyMySQLSchema(); } @@ -82,9 +84,9 @@ public: /// Parameter is ignored for MySQL backend as the v4 and v6 leases share /// the same database. void reopen(Universe) { - HostDataSourceFactory::destroy(); - HostDataSourceFactory::create(validMySQLConnectionString()); - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + HostMgr::create(); + HostMgr::addSource(validMySQLConnectionString()); + hdsptr_ = HostMgr::instance().getHostDataSource(); } /// @brief returns number of rows in a table @@ -148,9 +150,9 @@ TEST(MySqlHostDataSource, OpenDatabase) { // Check that lease manager open the database opens correctly and tidy up. // If it fails, print the error message. try { - HostDataSourceFactory::create(validMySQLConnectionString()); - EXPECT_NO_THROW((void) HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + HostMgr::create(); + EXPECT_NO_THROW(HostMgr::addSource(validMySQLConnectionString())); + HostMgr::delSource("mysql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -163,9 +165,9 @@ TEST(MySqlHostDataSource, OpenDatabase) { try { string connection_string = validMySQLConnectionString() + string(" ") + string(VALID_TIMEOUT); - HostDataSourceFactory::create(connection_string); - EXPECT_NO_THROW((void) HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + HostMgr::create(); + EXPECT_NO_THROW(HostMgr::addSource(connection_string)); + HostMgr::delSource("mysql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -175,43 +177,43 @@ TEST(MySqlHostDataSource, OpenDatabase) { // Check that attempting to get an instance of the lease manager when // none is set throws an exception. - EXPECT_FALSE(HostDataSourceFactory::getHostDataSourcePtr()); + EXPECT_FALSE(HostMgr::instance().getHostDataSource()); // Check that wrong specification of backend throws an exception. // (This is really a check on LeaseMgrFactory, but is convenient to // perform here.) - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( NULL, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)), InvalidParameter); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( INVALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)), InvalidType); // Check that invalid login data causes an exception. - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, INVALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_1)), DbInvalidTimeout); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_2)), DbInvalidTimeout); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, VALID_TIMEOUT, INVALID_READONLY_DB)), DbInvalidReadOnly); // Check for missing parameters - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( MYSQL_VALID_TYPE, NULL, VALID_HOST, INVALID_USER, VALID_PASSWORD)), NoDatabaseName); diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc index 269fe77e95..d9e422259f 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -46,7 +47,8 @@ public: // Connect to the database try { - HostDataSourceFactory::create(validPgSQLConnectionString()); + HostMgr::create(); + HostMgr::addSource(validPgSQLConnectionString()); } catch (...) { std::cerr << "*** ERROR: unable to open database. The test\n" "*** environment is broken and must be fixed before\n" @@ -56,7 +58,7 @@ public: throw; } - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + hdsptr_ = HostMgr::instance().getHostDataSource(); } /// @brief Destructor @@ -70,7 +72,7 @@ public: } catch (...) { // Rollback may fail if backend is in read only mode. That's ok. } - HostDataSourceFactory::destroy(); + HostMgr::delAllSources(); hdsptr_.reset(); destroyPgSQLSchema(); } @@ -83,9 +85,9 @@ public: /// Parameter is ignored for PostgreSQL backend as the v4 and v6 leases /// share the same database. void reopen(Universe) { - HostDataSourceFactory::destroy(); - HostDataSourceFactory::create(validPgSQLConnectionString()); - hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + HostMgr::create(); + HostMgr::addSource(validPgSQLConnectionString()); + hdsptr_ = HostMgr::instance().getHostDataSource(); } /// @brief returns number of rows in a table @@ -148,9 +150,9 @@ TEST(PgSqlHostDataSource, OpenDatabase) { // Check that lease manager open the database opens correctly and tidy up. // If it fails, print the error message. try { - HostDataSourceFactory::create(validPgSQLConnectionString()); - EXPECT_NO_THROW((void) HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + HostMgr::create(); + EXPECT_NO_THROW(HostMgr::addSource(validPgSQLConnectionString())); + HostMgr::delSource("postgresql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -163,9 +165,8 @@ TEST(PgSqlHostDataSource, OpenDatabase) { try { string connection_string = validPgSQLConnectionString() + string(" ") + string(VALID_TIMEOUT); - HostDataSourceFactory::create(connection_string); - EXPECT_NO_THROW((void) HostDataSourceFactory::getHostDataSourcePtr()); - HostDataSourceFactory::destroy(); + EXPECT_NO_THROW(HostMgr::addSource(connection_string)); + HostMgr::delSource("postgresql"); } catch (const isc::Exception& ex) { FAIL() << "*** ERROR: unable to open database, reason:\n" << " " << ex.what() << "\n" @@ -175,40 +176,40 @@ TEST(PgSqlHostDataSource, OpenDatabase) { // Check that attempting to get an instance of the lease manager when // none is set throws an exception. - EXPECT_FALSE(HostDataSourceFactory::getHostDataSourcePtr()); + EXPECT_FALSE(HostMgr::instance().getHostDataSource()); // Check that wrong specification of backend throws an exception. // (This is really a check on LeaseMgrFactory, but is convenient to // perform here.) - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( NULL, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)), InvalidParameter); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( INVALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)), InvalidType); // Check that invalid login data causes an exception. - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, INVALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD)), DbOpenError); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_1)), DbInvalidTimeout); - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_2)), DbInvalidTimeout); // Check for missing parameters - EXPECT_THROW(HostDataSourceFactory::create(connectionString( + EXPECT_THROW(HostMgr::addSource(connectionString( PGSQL_VALID_TYPE, NULL, VALID_HOST, INVALID_USER, VALID_PASSWORD)), NoDatabaseName);