From: Thomas Markwalder Date: Thu, 17 Apr 2014 13:09:45 +0000 (-0400) Subject: [3121] Moved validation logic to build method for DdnsDomain parsing X-Git-Tag: trac3434_base~46^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=da53f8f2d67a775f625bddcd3089df7b53f651f1;p=thirdparty%2Fkea.git [3121] Moved validation logic to build method for DdnsDomain parsing Modified code for domain parsing. --- diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index fedfcdc1a2..7c3d46a7df 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -302,6 +302,10 @@ build(isc::data::ConstElementPtr key_list){ parser->build(key_config); parsers_.push_back(parser); } + + // Now that we know we have a valid list, commit that list to the + // area given to us during construction (i.e. to the d2 context). + *keys_ = *local_keys_; } void @@ -311,10 +315,6 @@ TSIGKeyInfoListParser::commit() { BOOST_FOREACH(isc::dhcp::ParserPtr parser, parsers_) { parser->commit(); } - - // Now that we know we have a valid list, commit that list to the - // area given to us during construction (i.e. to the d2 context). - *keys_ = *local_keys_; } // *********************** DnsServerInfoParser ************************* @@ -488,34 +488,8 @@ DdnsDomainParser::build(isc::data::ConstElementPtr domain_config) { parser->build(config_pair.second); parser->commit(); } -} - -isc::dhcp::ParserPtr -DdnsDomainParser::createConfigParser(const std::string& config_id) { - DhcpConfigParser* parser = NULL; - // Based on the configuration id of the element, create the appropriate - // parser. Scalars are set to use the parser's local scalar storage. - if ((config_id == "name") || - (config_id == "key_name")) { - parser = new isc::dhcp::StringParser(config_id, - local_scalars_.getStringStorage()); - } else if (config_id == "dns_servers") { - // Server list parser is given in our local server storage. It will pass - // this down to its server parsers and is where they will write their - // server instances upon commit. - parser = new DnsServerInfoListParser(config_id, local_servers_); - } else { - isc_throw(NotImplemented, - "parser error: DdnsDomain parameter not supported: " - << config_id); - } - - // Return the new domain parser instance. - return (isc::dhcp::ParserPtr(parser)); -} -void -DdnsDomainParser::commit() { + // Now construct the domain. std::string name; std::string key_name; @@ -549,7 +523,35 @@ DdnsDomainParser::commit() { DdnsDomainPtr domain(new DdnsDomain(name, key_name, local_servers_)); // Add the new domain to the domain storage. - (*domains_)[name]=domain; + (*domains_)[name] = domain; +} + +isc::dhcp::ParserPtr +DdnsDomainParser::createConfigParser(const std::string& config_id) { + DhcpConfigParser* parser = NULL; + // Based on the configuration id of the element, create the appropriate + // parser. Scalars are set to use the parser's local scalar storage. + if ((config_id == "name") || + (config_id == "key_name")) { + parser = new isc::dhcp::StringParser(config_id, + local_scalars_.getStringStorage()); + } else if (config_id == "dns_servers") { + // Server list parser is given in our local server storage. It will pass + // this down to its server parsers and is where they will write their + // server instances upon commit. + parser = new DnsServerInfoListParser(config_id, local_servers_); + } else { + isc_throw(NotImplemented, + "parser error: DdnsDomain parameter not supported: " + << config_id); + } + + // Return the new domain parser instance. + return (isc::dhcp::ParserPtr(parser)); +} + +void +DdnsDomainParser::commit() { } // *********************** DdnsDomainListParser ************************* @@ -622,6 +624,9 @@ DdnsDomainListMgrParser::build(isc::data::ConstElementPtr domain_config) { parser->build(config_pair.second); parser->commit(); } + + // Add the new domain to the domain storage. + mgr_->setDomains(local_domains_); } isc::dhcp::ParserPtr @@ -643,8 +648,6 @@ DdnsDomainListMgrParser::createConfigParser(const std::string& config_id) { void DdnsDomainListMgrParser::commit() { - // Add the new domain to the domain storage. - mgr_->setDomains(local_domains_); } diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index 78fe60909b..d16e523501 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -611,6 +611,9 @@ public: virtual void build(isc::data::ConstElementPtr key_list_config); /// @brief Commits the list of TSIG keys + /// + /// Iterates over the internal list of TSIGKeyInfoParsers, invoking + /// commit on each one. virtual void commit(); private: @@ -723,6 +726,9 @@ public: virtual void build(isc::data::ConstElementPtr server_list_config); /// @brief Commits the list of DnsServerInfos + /// + /// Iterates over the internal list of DdnsServerInfoParsers, invoking + /// commit on each one. virtual void commit(); private: @@ -759,8 +765,10 @@ public: virtual ~DdnsDomainParser(); /// @brief Performs the actual parsing of the given "ddns_domain" element. - /// The results of the parsing are retained internally for use during - /// commit. + /// + /// Parses a configuration for the elements needed to instantiate a + /// DdnsDomain, validates those entries, creates a DdnsDomain instance + /// then attempts to add it to a list of domains. /// /// @param domain_config is the "ddns_domain" configuration to parse virtual void build(isc::data::ConstElementPtr domain_config); @@ -779,8 +787,7 @@ public: virtual isc::dhcp::ParserPtr createConfigParser(const std::string& config_id); - /// @brief Instantiates a DdnsDomain from internal data values - /// saves it to the storage area pointed to by domains_. + /// @brief commits the configured DdnsDomain virtual void commit(); private: @@ -832,7 +839,7 @@ public: /// @brief Performs the actual parsing of the given list "ddns_domain" /// elements. - /// It iterates over each server entry in the list: + /// It iterates over each domain entry in the list: /// 1. Instantiate a DdnsDomainParser for the entry /// 2. Pass the element configuration to the parser's build method /// 3. Add the parser instance to local storage @@ -844,10 +851,10 @@ public: /// parse. virtual void build(isc::data::ConstElementPtr domain_list_config); - /// @brief Iterates over the internal list of DdnsDomainParsers, invoking - /// commit on each. This causes each parser to instantiate a DdnsDomain - /// from its internal data values and add that domain instance to the - /// storage area, domains_. + /// @brief Commits the list of DdnsDomains + /// + /// Iterates over the internal list of DdnsDomainParsers, invoking + /// commit on each one. virtual void commit(); private: @@ -892,8 +899,10 @@ public: virtual ~DdnsDomainListMgrParser(); /// @brief Performs the actual parsing of the given manager element. - /// The results of the parsing are retained internally for use during - /// commit. + /// + /// Parses a configuration for the elements needed to instantiate a + /// DdnsDomainListMgr, validates those entries, then creates a + /// DdnsDomainListMgr. /// /// @param mgr_config is the manager configuration to parse virtual void build(isc::data::ConstElementPtr mgr_config); @@ -910,8 +919,7 @@ public: virtual isc::dhcp::ParserPtr createConfigParser(const std::string& config_id); - /// @brief Populates the DdnsDomainListMgr from internal data values - /// set during parsing. + /// @brief Commits the configured DdsnDomainListMgr virtual void commit(); private: diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index f11c70cb6c..e1b3313f1d 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -609,9 +609,8 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) { " \"port\": 300 } ] } "; ASSERT_TRUE(fromJSON(config)); - // Verify that the domain configuration builds but commit fails. - ASSERT_NO_THROW(parser_->build(config_set_)); - ASSERT_THROW(parser_->commit(), isc::dhcp::DhcpConfigError); + // Verify that the domain configuration builds fails. + EXPECT_THROW(parser_->build(config_set_), isc::dhcp::DhcpConfigError); // Create a domain configuration with an empty server list. config = "{ \"name\": \"tmark.org\" , " @@ -621,7 +620,7 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) { ASSERT_TRUE(fromJSON(config)); // Verify that the domain configuration build fails. - ASSERT_THROW(parser_->build(config_set_), D2CfgError); + EXPECT_THROW(parser_->build(config_set_), D2CfgError); // Create a domain configuration with a mal-formed server entry. config = "{ \"name\": \"tmark.org\" , " @@ -632,7 +631,7 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) { ASSERT_TRUE(fromJSON(config)); // Verify that the domain configuration build fails. - ASSERT_THROW(parser_->build(config_set_), isc::BadValue); + EXPECT_THROW(parser_->build(config_set_), isc::BadValue); // Create a domain configuration without an defined key name config = "{ \"name\": \"tmark.org\" , " @@ -642,9 +641,8 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) { " \"port\": 300 } ] } "; ASSERT_TRUE(fromJSON(config)); - // Verify that the domain configuration build succeeds but commit fails. - ASSERT_NO_THROW(parser_->build(config_set_)); - ASSERT_THROW(parser_->commit(), D2CfgError); + // Verify that the domain configuration build fails. + EXPECT_THROW(parser_->build(config_set_), D2CfgError); } /// @brief Verifies the basics of parsing DdnsDomains. @@ -839,9 +837,8 @@ TEST_F(DdnsDomainTest, duplicateDomain) { ASSERT_NO_THROW(list_parser.reset( new DdnsDomainListParser("test", domains_, keys_))); - // Verify that the parse build succeeds but the commit fails. - ASSERT_NO_THROW(list_parser->build(config_set_)); - ASSERT_THROW(list_parser->commit(), D2CfgError); + // Verify that the parse build fails. + EXPECT_THROW(list_parser->build(config_set_), D2CfgError); } /// @brief Tests construction of D2CfgMgr