[2559] Add factory function for DbAccessParser

Also add ability to handle incremental changes.

src/bin/dhcp6/dbaccess_parser.cc

@@ -26,25 +26,43 @@ using namespace isc::data;
 namespace isc {
 namespace dhcp {
 
-/// @brief an auxiliary type used for storing an element name and its parser
 typedef map<string, ConstElementPtr> ConfigPairMap;
 typedef pair<string, ConstElementPtr> ConfigPair;
+typedef map<string, string> StringPairMap;
+typedef pair<string, string> StringPair;
 
 // Parse the configuration and check that the various keywords are consistent.
 void
 DbAccessParser::build(isc::data::ConstElementPtr config_value) {
     const ConfigPairMap& config_map = config_value->mapValue();
 
-    // Check if the "type" keyword exists and thrown an exception if not.
-    ConfigPairMap::const_iterator type_ptr = config_map.find("type");
-    if (type_ptr == config_map.end()) {
+    // To cope with incremental updates, the strategy is:
+    // 1. Take a copy of the stored keyword/value pairs.
+    // 2. Update the copy with the passed keywords.
+    // 3. Perform validation checks on the updated keyword/value pairs.
+    // 4. If all is OK, update the stored keyword/value pairs.
+    // 5. Construct the updated database access string.
+
+    // 1. Take a copy of the stored keyword/value pairs.
+    map<string, string> values_copy = values_;
+
+    // 2. Update the copy with the passed keywords.
+    BOOST_FOREACH(ConfigPair param, config_value->mapValue()) {
+        values_copy[param.first] = param.second->stringValue();
+    }
+
+    // 3. Perform validation checks on the updated set of keyword/values.
+    //
+    // a. Check if the "type" keyword exists and thrown an exception if not.
+    StringPairMap::const_iterator type_ptr = values_copy.find("type");
+    if (type_ptr == values_copy.end()) {
         isc_throw(TypeKeywordMissing, "lease database access parameters must "
                   "include the keyword 'type' to determine type of database "
                   "to be accessed");
     }
 
-    // Check if the 'type; keyword known and throw an exception if not.
-    string dbtype = type_ptr->second->stringValue();
+    // b. Check if the 'type; keyword known and throw an exception if not.
+    string dbtype = type_ptr->second;
     if ((dbtype != "memfile") && (dbtype != "mysql")) {
         isc_throw(BadValue, "unknown backend database type: " << dbtype);
     }
@@ -52,15 +70,16 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) {
     /// @todo Log a warning if the type is memfile and there are other keywords.
     ///       This will be done when the module is moved to libdhcpsrv
 
+    // 4. If all is OK, update the stored keyword/value pairs.
+    values_ = values_copy;
 
-    // All OK, build up the access string
+    // 5. Construct the updated database access string.
     dbaccess_ = "";
-    BOOST_FOREACH(ConfigPair param, config_value->mapValue()) {
+    BOOST_FOREACH(StringPair keyval, values_) {
         if (! dbaccess_.empty()) {
             dbaccess_ += std::string(" ");
         }
-        dbaccess_ += (param.first + std::string("=") +
-                      param.second->stringValue());
+        dbaccess_ += (keyval.first + std::string("=") + keyval.second);
     }
 }
 

src/bin/dhcp6/dbaccess_parser.h

@@ -47,7 +47,9 @@ public:
 
     /// @brief Default constructor
     ///
-    DbAccessParser()
+    /// @param param_name Name of the configuration parameter being parsed.
+    DbAccessParser(const std::string& param_name)
+        : dbaccess_(), param_name_(param_name), values_()
     {}
 
     /// The destructor.
@@ -69,6 +71,8 @@ public:
     /// @param config_value The configuration value for the "lease-database"
     ///        identifier.
     ///
+    /// @throw isc::BadValue The 'type' keyword contains an unknown database
+    ///        type.
     /// @throw isc::dhcp::MissingTypeKeyword The 'type' keyword is missing from
     ///        the list of database access keywords.
     virtual void build(isc::data::ConstElementPtr config_value);
@@ -82,6 +86,13 @@ public:
     /// The result is undefined otherwise.
     virtual void commit();
 
+    /// @brief Factory method to create parser
+    ///
+    /// Creates an instance of this parser.
+    static DhcpConfigParser* factory(const std::string& param_name) {
+        return (new DbAccessParser(param_name));
+    }
+
     /// @brief Get database access string
     ///
     /// Used in testing to check that the configuration information has been
@@ -92,6 +103,9 @@ public:
 
 private:
     std::string     dbaccess_;      ///< Database access string
+    std::string     param_name_;    ///< Parameter name
+    std::map<std::string, std::string> values_;
+                                    ///< Stored parameter values
 };
 
 };  // namespace dhcp

src/bin/dhcp6/tests/dbaccess_parser_unittest.cc

@@ -158,10 +158,9 @@ TEST_F(DbAccessParserTest, validTypeMemfile) {
     ConstElementPtr json_elements = Element::fromJSON(json_config);
     EXPECT_TRUE(json_elements);
 
-    DbAccessParser parser;
+    DbAccessParser parser("lease-database");
     EXPECT_NO_THROW(parser.build(json_elements));
     string dbaccess = parser.getDbAccessString();
-
     checkAccessString("Valid memfile", dbaccess, config);
 }
 
@@ -178,10 +177,9 @@ TEST_F(DbAccessParserTest, validTypeMysql) {
     ConstElementPtr json_elements = Element::fromJSON(json_config);
     EXPECT_TRUE(json_elements);
 
-    DbAccessParser parser;
+    DbAccessParser parser("lease-database");
     EXPECT_NO_THROW(parser.build(json_elements));
     string dbaccess = parser.getDbAccessString();
-
     checkAccessString("Valid mysql", dbaccess, config);
 }
 
@@ -197,7 +195,7 @@ TEST_F(DbAccessParserTest, missingTypeKeyword) {
     ConstElementPtr json_elements = Element::fromJSON(json_config);
     EXPECT_TRUE(json_elements);
 
-    DbAccessParser parser;
+    DbAccessParser parser("lease-database");
     EXPECT_THROW(parser.build(json_elements), TypeKeywordMissing);
 }
 
@@ -215,8 +213,126 @@ TEST_F(DbAccessParserTest, badTypeKeyword) {
     ConstElementPtr json_elements = Element::fromJSON(json_config);
     EXPECT_TRUE(json_elements);
 
-    DbAccessParser parser;
+    DbAccessParser parser("lease-database");
+    EXPECT_THROW(parser.build(json_elements), BadValue);
+}
+
+// Check that the factory function works.
+TEST_F(DbAccessParserTest, factory) {
+    const char* config[] = {"type", "memfile",
+                            NULL};
+
+    string json_config = toJson(config);
+    ConstElementPtr json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    // Check that the parser is built through the factory.
+    boost::scoped_ptr<DhcpConfigParser> parser(
+        DbAccessParser::factory("lease-database"));
+    EXPECT_NO_THROW(parser->build(json_elements));
+
+    // Access the "raw" parser.
+    DbAccessParser* dbap = dynamic_cast<DbAccessParser*>(parser.get());
+    EXPECT_NE(static_cast<DbAccessParser*>(NULL), dbap);
+    string dbaccess = dbap->getDbAccessString();
+    checkAccessString("Valid mysql", dbaccess, config);
+}
+
+// Check reconfiguration.  Checks that incremental changes applied to the
+// database configuration are incremental.
+TEST_F(DbAccessParserTest, incrementalChanges) {
+    const char* config1[] = {"type", "memfile",
+                             NULL};
+
+    // Applying config2 will cause a wholesale change.
+    const char* config2[] = {"type",     "mysql",
+                             "host",     "erewhon",
+                             "user",     "kea",
+                             "password", "keapassword",
+                             "name",     "keatest",
+                             NULL};
+
+    // Applying incremental2 should cause a change to config3.
+    const char* incremental2[] = {"user",     "me",
+                                  "password", "meagain",
+                                  NULL};
+    const char* config3[] = {"type",     "mysql",
+                             "host",     "erewhon",
+                             "user",     "me",
+                             "password", "meagain",
+                             "name",     "keatest",
+                             NULL};
+
+    // incremental3 will cause an exception.  There should be no change
+    // to the returned value.
+    const char* incremental3[] = {"type",     "invalid",
+                                  "user",     "you",
+                                  "password", "youagain",
+                                  NULL};
+
+    // incremental4 is a compatible change and should cause a transition
+    // to config4.
+    const char* incremental4[] = {"user",     "them",
+                                  "password", "themagain",
+                                  NULL};
+    const char* config4[] = {"type",     "mysql",
+                             "host",     "erewhon",
+                             "user",     "them",
+                             "password", "themagain",
+                             "name",     "keatest",
+                             NULL};
+
+    DbAccessParser parser("lease-database");
+
+    // First configuration string should cause a representation of that string
+    // to be held.
+    string json_config = toJson(config1);
+    ConstElementPtr json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    EXPECT_NO_THROW(parser.build(json_elements));
+    string dbaccess = parser.getDbAccessString();
+    checkAccessString("Initial configuration", dbaccess, config1);
+
+    // Applying a wholesale change will cause the access string to change
+    // to a representation of the new configuration.
+    json_config = toJson(config2);
+    json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    EXPECT_NO_THROW(parser.build(json_elements));
+    dbaccess = parser.getDbAccessString();
+    checkAccessString("Subsequent configuration", dbaccess, config2);
+
+    // Applying an incremental change will cause the representation to change
+    // incrementally.
+    json_config = toJson(incremental2);
+    json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    EXPECT_NO_THROW(parser.build(json_elements));
+    dbaccess = parser.getDbAccessString();
+    checkAccessString("Incremental configuration", dbaccess, config3);
+
+    // Applying the next incremental change should cause an exception to be
+    // thrown and there be no change to the access string.
+    json_config = toJson(incremental3);
+    json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
     EXPECT_THROW(parser.build(json_elements), BadValue);
+    dbaccess = parser.getDbAccessString();
+    checkAccessString("Incompatible incremental change", dbaccess, config3);
+
+    // Applying an incremental change will cause the representation to change
+    // incrementally.
+    json_config = toJson(incremental4);
+    json_elements = Element::fromJSON(json_config);
+    EXPECT_TRUE(json_elements);
+
+    EXPECT_NO_THROW(parser.build(json_elements));
+    dbaccess = parser.getDbAccessString();
+    checkAccessString("Compatible incremental change", dbaccess, config4);
 }
 
 };  // Anonymous namespace