From: Marcin Siodelski Date: Wed, 12 Sep 2018 13:18:36 +0000 (+0200) Subject: [#28,!20] Addressed straight forward review comments. X-Git-Tag: 134-bugs--xcode-10_base~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=85bdbe2e91cc69163f385c8271439f3d6dbc267b;p=thirdparty%2Fkea.git [#28,!20] Addressed straight forward review comments. - Better description of BaseConfigBackend - Rename selector to backend_selector - Fix commentary in selectBackends function. --- diff --git a/src/lib/config_backend/base_config_backend.h b/src/lib/config_backend/base_config_backend.h index d34bc6f06f..b9b0405297 100644 --- a/src/lib/config_backend/base_config_backend.h +++ b/src/lib/config_backend/base_config_backend.h @@ -15,7 +15,20 @@ namespace isc { namespace cb { -/// @brief Base class for server specific configuration backends. +/// @brief Interface for Kea server specific configuration backend +/// implementations. +/// +/// Each Kea server (e.g. DHCPv4 server) needs to implement its own +/// interface to store and fetch its configuration from the databases. +/// This is because each Kea server uses a different set of +/// configuration information. This is a base interface which should +/// be implemented (and extended) by respective Kea servers to provide +/// API to store and fetch configuration information from a database. +/// Such implementation is called configuration backend. Each +/// configuration backend faciliates a single database type, e.g. MySQL +/// database. In order to support multiple database types, i.e. MySQL, +/// Posrgres, Cassandra, each Kea server will have to implement +/// 3 separate configuration backends, one for each database type. class BaseConfigBackend { public: @@ -23,6 +36,9 @@ public: virtual ~BaseConfigBackend() { } /// @brief Returns backend type in the textual format. + /// + /// @return Name of the storage for configurations, e.g. "mysql", + /// "pgsql" and so forth. virtual std::string getType() const = 0; /// @brief Returns backend host. diff --git a/src/lib/config_backend/base_config_backend_pool.h b/src/lib/config_backend/base_config_backend_pool.h index 1aa830c22d..7a9c4be54f 100644 --- a/src/lib/config_backend/base_config_backend_pool.h +++ b/src/lib/config_backend/base_config_backend_pool.h @@ -73,11 +73,12 @@ protected: /// /// @code /// Subnet4Ptr getSubnet4(const SubnetID& subnet_id, - /// const BackendSelector& selector, + /// const BackendSelector& backend_selector, /// const ServerSelector& server_selector) const { /// Subnet4Ptr subnet; /// getPropertyPtrConst - /// (&ConfigBackendDHCPv4::getSubnet4, selector, subnet, subnet_id); + /// (&ConfigBackendDHCPv4::getSubnet4, backend_selector, + /// server_selector, subnet, subnet_id); /// return (subnet); /// } /// @endcode @@ -99,7 +100,7 @@ protected: /// @tparam InputType Type of the objects used as input to the backend call. /// /// @param MethodPointer Pointer to the backend method to be called. - /// @param selector Backend selector. + /// @param backend_selector Backend selector. /// @param server_selector Server selector. /// @param [out] property Reference to the shared pointer where retrieved /// property should be assigned. @@ -110,14 +111,14 @@ protected: template void getPropertyPtrConst(PropertyType (ConfigBackendType::*MethodPointer) (const db::ServerSelector&, InputType...) const, - const db::BackendSelector& selector, + const db::BackendSelector& backend_selector, const db::ServerSelector& server_selector, PropertyType& property, InputType... input) const { // If no particular backend is selected, call each backend and return // the first non-null (non zero) value. - if (selector.amUnspecified()) { + if (backend_selector.amUnspecified()) { for (auto backend : backends_) { property = ((*backend).*MethodPointer)(server_selector, input...); if (property) { @@ -127,7 +128,7 @@ protected: } else { // Backend selected, find the one that matches selection. - auto backends = selectBackends(selector); + auto backends = selectBackends(backend_selector); if (!backends.empty()) { for (auto backend : backends) { property = ((*backend).*MethodPointer)(server_selector, input...); @@ -138,7 +139,7 @@ protected: } else { isc_throw(db::NoSuchDatabase, "no such database found for selector: " - << selector.toText()); + << backend_selector.toText()); } } } @@ -155,12 +156,12 @@ protected: /// @code /// OptionalValue /// getGlobalParameter4(const std::string& parameter_name, - /// const BackendSelector& selector, + /// const BackendSelector& backend_selector, /// const ServerSelector& server_selector) const { /// std::string parameter; /// getOptionalPropertyConst - /// (&ConfigBackendDHCPv4::getGlobalParameter4, selector, server_selector, - /// parameter, parameter_name); + /// (&ConfigBackendDHCPv4::getGlobalParameter4, backend_selector, + /// server_selector, parameter, parameter_name); /// return (parameter); /// } /// @endcode @@ -176,7 +177,7 @@ protected: /// @tparam InputType Type of the objects used as input to the backend call. /// /// @param MethodPointer Pointer to the backend method to be called. - /// @param selector Backend selector. + /// @param backend_selector Backend selector. /// @param server_selector Server selector. /// @param [out] property Reference to the shared pointer where retrieved /// property should be assigned. @@ -188,14 +189,14 @@ protected: void getOptionalPropertyConst(util::OptionalValue (ConfigBackendType::*MethodPointer) (const db::ServerSelector&, InputType...) const, - const db::BackendSelector& selector, + const db::BackendSelector& backend_selector, const db::ServerSelector& server_selector, util::OptionalValue& property, InputType... input) const { // If no particular backend is selected, call each backend and return // the first non-null (non zero) value. - if (selector.amUnspecified()) { + if (backend_selector.amUnspecified()) { for (auto backend : backends_) { property = ((*backend).*MethodPointer)(server_selector, input...); if (property.isSpecified()) { @@ -205,7 +206,7 @@ protected: } else { // Backend selected, find the one that matches selection. - auto backends = selectBackends(selector); + auto backends = selectBackends(backend_selector); if (!backends.empty()) { for (auto backend : backends) { property = ((*backend).*MethodPointer)(server_selector, input...); @@ -216,7 +217,7 @@ protected: } else { isc_throw(db::NoSuchDatabase, "no such database found for selector: " - << selector.toText()); + << backend_selector.toText()); } } } @@ -230,13 +231,13 @@ protected: /// @c getSubnets6 method: /// /// @code - /// Subnet6Collection getModifiedSubnets6(const BackendSelector& selector, + /// Subnet6Collection getModifiedSubnets6(const BackendSelector& backend_selector, /// const ServerSelector& server_selector, /// const ptime& modification_time) const { /// Subnet6Collection subnets; /// getMultiplePropertiesConst - /// (&ConfigBackendDHCPv6::getSubnets6, selector, subnets, - /// modification_time); + /// (&ConfigBackendDHCPv6::getSubnets6, backend_selector, server_selector, + /// subnets, modification_time); /// return (subnets); /// } /// @endcode @@ -257,7 +258,7 @@ protected: /// @tparam InputType Type of the objects used as input to the backend call. /// /// @param MethodPointer Pointer to the backend method to be called. - /// @param selector Backend selector. + /// @param backend_selector Backend selector. /// @param server_selector Server selector. /// @param [out] properties Reference to the collection of retrieved properties. /// @param input Values to be used as input to the backend call. @@ -267,11 +268,11 @@ protected: template void getMultiplePropertiesConst(PropertyCollectionType (ConfigBackendType::*MethodPointer) (const db::ServerSelector&, InputType...) const, - const db::BackendSelector& selector, + const db::BackendSelector& backend_selector, const db::ServerSelector& server_selector, PropertyCollectionType& properties, InputType... input) const { - if (selector.amUnspecified()) { + if (backend_selector.amUnspecified()) { for (auto backend : backends_) { properties = ((*backend).*MethodPointer)(server_selector, input...); if (!properties.empty()) { @@ -280,7 +281,7 @@ protected: } } else { - auto backends = selectBackends(selector); + auto backends = selectBackends(backend_selector); if (!backends.empty()) { for (auto backend : backends) { properties = ((*backend).*MethodPointer)(server_selector, input...); @@ -291,7 +292,7 @@ protected: } else { isc_throw(db::NoSuchDatabase, "no database found for selector: " - << selector.toText()); + << backend_selector.toText()); } } } @@ -308,7 +309,7 @@ protected: /// Subnet4Collection getAllSubnets4(const BackendSelector&, const ServerSelector&) const { /// Subnet4Collection subnets; /// getAllPropertiesConst - /// (&ConfigBackendDHCPv4::getAllSubnets4, subnets, selector, + /// (&ConfigBackendDHCPv4::getAllSubnets4, subnets, backend_selector, /// server_selector); /// return (subnets); /// } @@ -329,7 +330,7 @@ protected: /// properties are stored. /// /// @param MethodPointer Pointer to the backend method to be called. - /// @param selector Backend selector. + /// @param backend_selector Backend selector. /// @param server_selector Server selector. /// @param [out] properties Reference to the collection of retrieved properties. /// @@ -338,10 +339,10 @@ protected: template void getAllPropertiesConst(PropertyCollectionType (ConfigBackendType::*MethodPointer) (const db::ServerSelector&) const, - const db::BackendSelector& selector, + const db::BackendSelector& backend_selector, const db::ServerSelector& server_selector, PropertyCollectionType& properties) const { - if (selector.amUnspecified()) { + if (backend_selector.amUnspecified()) { for (auto backend : backends_) { properties = ((*backend).*MethodPointer)(server_selector); if (!properties.empty()) { @@ -350,7 +351,7 @@ protected: } } else { - auto backends = selectBackends(selector); + auto backends = selectBackends(backend_selector); if (!backends.empty()) { for (auto backend : backends) { properties = ((*backend).*MethodPointer)(server_selector); @@ -361,7 +362,7 @@ protected: } else { isc_throw(db::NoSuchDatabase, "no database found for selector: " - << selector.toText()); + << backend_selector.toText()); } } } @@ -377,10 +378,10 @@ protected: /// /// @code /// void createUpdateSubnet6(const Subnet6Ptr& subnet, - /// const BackendSelector& selector, + /// const BackendSelector& backend_selector, /// const ServerSelector& server_selector) { /// createUpdateDeleteProperty - /// (&ConfigBackendDHCPv6::createUpdateSubnet6, selector, + /// (&ConfigBackendDHCPv6::createUpdateSubnet6, backend_selector, /// server_selector, subnet, selector); /// } /// @endcode @@ -400,7 +401,7 @@ protected: /// backend method, e.g. new property to be added, updated or deleted. /// /// @param MethodPointer Pointer to the backend method to be called. - /// @param selector Backend selector. + /// @param backend_selector Backend selector. /// @param server_selector Server selector. /// @param input Objects used as arguments to the backend method to be /// called. @@ -412,17 +413,17 @@ protected: template void createUpdateDeleteProperty(void (ConfigBackendType::*MethodPointer) (const db::ServerSelector&, InputType...), - const db::BackendSelector& selector, + const db::BackendSelector& backend_selector, const db::ServerSelector& server_selector, InputType... input) { - auto backends = selectBackends(selector); + auto backends = selectBackends(backend_selector); if (backends.empty()) { isc_throw(db::NoSuchDatabase, "no database found for selector: " - << selector.toText()); + << backend_selector.toText()); } else if (backends.size() > 1) { isc_throw(db::AmbiguousDatabase, "more than 1 database found for " - "selector: " << selector.toText()); + "selector: " << backend_selector.toText()); } (*(*(backends.begin())).*MethodPointer)(server_selector, input...); @@ -435,19 +436,19 @@ protected: /// /// @param selector Selector for which matching backends should be selected. std::list - selectBackends(const db::BackendSelector& selector) const { + selectBackends(const db::BackendSelector& backend_selector) const { std::list selected; - // In case there is only one backend, it is allowed to not select the - // database backend. - if ((backends_.size() == 1) && selector.amUnspecified()) { + // In case there is only one backend and the caller hasn't specified + // any particular backend, simply return it. + if ((backends_.size() == 1) && backend_selector.amUnspecified()) { selected.push_back(*backends_.begin()); return (selected); } // For other cases we return empty list. - if (backends_.empty() || selector.amUnspecified()) { + if (backends_.empty() || backend_selector.amUnspecified()) { return (selected); } @@ -455,23 +456,23 @@ protected: for (auto backend : backends_) { // If backend type is specified and it is not matching, // do not select this backend. - if ((selector.getBackendType() != db::BackendSelector::Type::UNSPEC) && - (selector.getBackendType() != + if ((backend_selector.getBackendType() != db::BackendSelector::Type::UNSPEC) && + (backend_selector.getBackendType() != db::BackendSelector::stringToBackendType(backend->getType()))) { continue; } // If the host has been specified by the backend's host is not // matching, do not select this backend. - if ((!selector.getBackendHost().empty()) && - (selector.getBackendHost() != backend->getHost())) { + if ((!backend_selector.getBackendHost().empty()) && + (backend_selector.getBackendHost() != backend->getHost())) { continue; } // If the port has been specified by the backend's port is not // matching, do not select this backend. - if ((selector.getBackendPort() != 0) && - (selector.getBackendPort() != backend->getPort())) { + if ((backend_selector.getBackendPort() != 0) && + (backend_selector.getBackendPort() != backend->getPort())) { continue; } diff --git a/src/lib/config_backend/tests/config_backend_mgr_unittest.cc b/src/lib/config_backend/tests/config_backend_mgr_unittest.cc index 37e5f250aa..040d1048a2 100644 --- a/src/lib/config_backend/tests/config_backend_mgr_unittest.cc +++ b/src/lib/config_backend/tests/config_backend_mgr_unittest.cc @@ -205,13 +205,13 @@ public: /// @brief Retrieves a value of the property. /// /// @param property_name Name of the property which value should be returned. - /// @param selector Backend selector. The default value of the selector + /// @param backend_selector Backend selector. The default value of the selector /// is @c UNSPEC which means that the property will be searched in all backends /// and the first value found will be returned. /// @param server_selector Server selector. The default value is set to @c ALL, /// which means that the property for all servers will be returned. virtual int getProperty(const std::string& property_name, - const BackendSelector& selector = + const BackendSelector& backend_selector = BackendSelector::UNSPEC(), const ServerSelector& server_selector = ServerSelector::ALL()) const { @@ -227,8 +227,8 @@ public: // The template arguments specify the returned value type and the // argument of the getProperty method. getPropertyPtrConst - (&TestConfigBackend::getProperty, selector, server_selector, property, - property_name); + (&TestConfigBackend::getProperty, backend_selector, server_selector, + property, property_name); return (property); } @@ -236,21 +236,21 @@ public: /// /// @param property_name Name of the property which value should be returned. /// @param property_value Value of the property to be retrieved. - /// @param selector Backend selector. The default value of the selector + /// @param backend_selector Backend selector. The default value of the selector /// is @c UNSPEC which means that the property will be searched in all backends /// and the first value found will be returned. /// @param server_selector Server selector. The default value is set to @c ALL, /// which means that the property for all servers will be returned. virtual int getProperty(const std::string& property_name, const int property_value, - const BackendSelector& selector = + const BackendSelector& backend_selector = BackendSelector::UNSPEC(), const ServerSelector& server_selector = ServerSelector::ALL()) const { int property; getPropertyPtrConst - (&TestConfigBackend::getProperty, selector, server_selector, property, - property_name, property_value); + (&TestConfigBackend::getProperty, backend_selector, server_selector, + property, property_name, property_value); return (property); } @@ -258,13 +258,13 @@ public: /// @brief Retrieves multiple properties. /// /// @param property_name Name of the properties which should be retrieved. - /// @param selector Backend selector. The default value of the selector + /// @param backend_selector Backend selector. The default value of the selector /// is @c UNSPEC which means that the properties will be searched in all /// backends and the first non-empty list will be returned. /// @param server_selector Server selector. The default value is set to @c ALL, /// which means that the properties for all servers will be returned. virtual PropertiesList getProperties(const std::string& property_name, - const BackendSelector& selector = + const BackendSelector& backend_selector = BackendSelector::UNSPEC(), const ServerSelector& server_selector = ServerSelector::ALL()) const { @@ -278,19 +278,19 @@ public: // The template arguments specify the type of the list of properties // and the argument of the getProperties method. getMultiplePropertiesConst - (&TestConfigBackend::getProperties, selector, server_selector, + (&TestConfigBackend::getProperties, backend_selector, server_selector, properties, property_name); return (properties); } /// @brief Retrieves all properties. /// - /// @param selector Backend selector. The default value of the selector + /// @param backend_selector Backend selector. The default value of the selector /// is @c UNSPEC which means that the properties will be searched in all /// backends and the first non-empty list will be returned. /// @param server_selector Server selector. The default value is set to @c ALL, /// which means that the properties for all servers will be returned. - virtual PropertiesList getAllProperties(const BackendSelector& selector = + virtual PropertiesList getAllProperties(const BackendSelector& backend_selector = BackendSelector::UNSPEC(), const ServerSelector& server_selector = ServerSelector::ALL()) const { @@ -299,7 +299,7 @@ public: // This method is similar to getMultiplePropertiesConst but it lacks // an argument and it simply returns all properties. getAllPropertiesConst - (&TestConfigBackend::getAllProperties, selector, server_selector, + (&TestConfigBackend::getAllProperties, backend_selector, server_selector, properties); return (properties); } @@ -307,15 +307,15 @@ public: /// @brief Creates new property in a selected backend. /// /// @param new_property New property to be added to a backend. - /// @param selector Backend selector. It has no default value. + /// @param backend_selector Backend selector. It has no default value. /// @param server_selector The default value is @c ALL which means that /// new property is going to be shared by all servers. virtual void createProperty(const std::pair& new_property, - const BackendSelector& selector, + const BackendSelector& backend_selector, const ServerSelector& server_selector = ServerSelector::ALL()) { createUpdateDeleteProperty&> - (&TestConfigBackend::createProperty, selector, server_selector, + (&TestConfigBackend::createProperty, backend_selector, server_selector, new_property); } };