[#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 d34bc6f06f764a5d431af233af1291faa7003c24..b9b040529745ef4784811c4004c6a23aca1ba301 100644 (file)
--- 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 1aa830c22d78b7a2fea63e0cfcbcd1aed72dcf39..7a9c4be54f1c471041305830a180f7ffb72c12e2 100644 (file)
--- 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<Subnet4Ptr, const SubnetID&>
-    ///         (&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<typename PropertyType, typename... InputType>
     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<std::string>
     /// getGlobalParameter4(const std::string& parameter_name,
-    ///                     const BackendSelector& selector,
+    ///                     const BackendSelector& backend_selector,
     ///                     const ServerSelector& server_selector) const {
     ///     std::string parameter;
     ///     getOptionalPropertyConst<std::string, const std::string&>
-    ///         (&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<PropertyType>
                                   (ConfigBackendType::*MethodPointer)
                                   (const db::ServerSelector&, InputType...) const,
-                                  const db::BackendSelector& selector,
+                                  const db::BackendSelector& backend_selector,
                                   const db::ServerSelector& server_selector,
                                   util::OptionalValue<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.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<Subnet6Collection, const ptime&>
-    ///         (&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<typename PropertyCollectionType, typename... InputType>
     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<Subnet6Collection>
-    ///         (&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<typename PropertyCollectionType>
     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<const Subnet6Ptr&>
-    ///         (&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<typename... InputType>
     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<ConfigBackendTypePtr>
-    selectBackends(const db::BackendSelector& selector) const {
+    selectBackends(const db::BackendSelector& backend_selector) const {
 
         std::list<ConfigBackendTypePtr> 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 37e5f250aa2e6aa7c6602f4ae777ba4ac8b09865..040d1048a23fc1f1d133799c24ed73f8c40f7358 100644 (file)
--- 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<int, const std::string&>
-            (&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<int, const std::string&, int>
-            (&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<PropertiesList, const std::string&>
-            (&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<PropertiesList>
-            (&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<std::string, int>& new_property,
-                                const BackendSelector& selector,
+                                const BackendSelector& backend_selector,
                                 const ServerSelector& server_selector =
                                 ServerSelector::ALL()) {
         createUpdateDeleteProperty<const std::pair<std::string, int>&>
-            (&TestConfigBackend::createProperty, selector, server_selector,
+            (&TestConfigBackend::createProperty, backend_selector, server_selector,
              new_property);
     }
 };