From: Stephen Morris Date: Fri, 21 Dec 2012 11:48:42 +0000 (+0000) Subject: [2559] Add factory function for DbAccessParser X-Git-Tag: bind10-1.0.0-rc-release~76^2~21^2~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=90f9161b787dfe68158c12bd07d2e362f2db8cc6;p=thirdparty%2Fkea.git [2559] Add factory function for DbAccessParser Also add ability to handle incremental changes. --- diff --git a/src/bin/dhcp6/dbaccess_parser.cc b/src/bin/dhcp6/dbaccess_parser.cc index ce4e63e848..94909d0477 100644 --- a/src/bin/dhcp6/dbaccess_parser.cc +++ b/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 ConfigPairMap; typedef pair ConfigPair; +typedef map StringPairMap; +typedef pair 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 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); } } diff --git a/src/bin/dhcp6/dbaccess_parser.h b/src/bin/dhcp6/dbaccess_parser.h index 030188943a..fe44ad6618 100644 --- a/src/bin/dhcp6/dbaccess_parser.h +++ b/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 values_; + ///< Stored parameter values }; }; // namespace dhcp diff --git a/src/bin/dhcp6/tests/dbaccess_parser_unittest.cc b/src/bin/dhcp6/tests/dbaccess_parser_unittest.cc index 8bef8360e9..291b8d46cc 100644 --- a/src/bin/dhcp6/tests/dbaccess_parser_unittest.cc +++ b/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 parser( + DbAccessParser::factory("lease-database")); + EXPECT_NO_THROW(parser->build(json_elements)); + + // Access the "raw" parser. + DbAccessParser* dbap = dynamic_cast(parser.get()); + EXPECT_NE(static_cast(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