From: Thomas Markwalder Date: Thu, 17 Apr 2014 11:38:25 +0000 (-0400) Subject: [3121] Moved validation logic to build method for DnsServeInfo parsing X-Git-Tag: trac3434_base~46^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=75cf3605cf7d48689abdd067f5fd7d7a16487bd9;p=thirdparty%2Fkea.git [3121] Moved validation logic to build method for DnsServeInfo parsing Modified DnsServerInfoParser, DnsServerInfoListParser, and unit tests --- diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index 435cc982cf..fedfcdc1a2 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -346,32 +346,6 @@ DnsServerInfoParser::build(isc::data::ConstElementPtr server_config) { parser->commit(); } -} - -isc::dhcp::ParserPtr -DnsServerInfoParser::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 == "hostname") || - (config_id == "ip_address")) { - parser = new isc::dhcp::StringParser(config_id, - local_scalars_.getStringStorage()); - } else if (config_id == "port") { - parser = new isc::dhcp::Uint32Parser(config_id, - local_scalars_.getUint32Storage()); - } else { - isc_throw(NotImplemented, - "parser error: DnsServerInfo parameter not supported: " - << config_id); - } - - // Return the new parser instance. - return (isc::dhcp::ParserPtr(parser)); -} - -void -DnsServerInfoParser::commit() { std::string hostname; std::string ip_address; uint32_t port = DnsServerInfo::STANDARD_DNS_PORT; @@ -410,6 +384,32 @@ DnsServerInfoParser::commit() { servers_->push_back(serverInfo); } +isc::dhcp::ParserPtr +DnsServerInfoParser::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 == "hostname") || + (config_id == "ip_address")) { + parser = new isc::dhcp::StringParser(config_id, + local_scalars_.getStringStorage()); + } else if (config_id == "port") { + parser = new isc::dhcp::Uint32Parser(config_id, + local_scalars_.getUint32Storage()); + } else { + isc_throw(NotImplemented, + "parser error: DnsServerInfo parameter not supported: " + << config_id); + } + + // Return the new parser instance. + return (isc::dhcp::ParserPtr(parser)); +} + +void +DnsServerInfoParser::commit() { +} + // *********************** DnsServerInfoListParser ************************* DnsServerInfoListParser::DnsServerInfoListParser(const std::string& list_name, @@ -442,17 +442,16 @@ build(isc::data::ConstElementPtr server_list){ parser->build(server_config); parsers_.push_back(parser); } -} -void -DnsServerInfoListParser::commit() { // Domains must have at least one server. if (parsers_.size() == 0) { isc_throw (D2CfgError, "Server List must contain at least one server"); } +} - // Invoke commit on each server parser. This will cause each one to - // create it's server instance and commit it to storage. +void +DnsServerInfoListParser::commit() { + // Invoke commit on each server parser. BOOST_FOREACH(isc::dhcp::ParserPtr parser, parsers_) { parser->commit(); } diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index e460e2258b..78fe60909b 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -68,7 +68,7 @@ namespace d2 { /// any scalars which belong to the manager as well as creating and invoking a /// DdnsDomainListParser to parse its list of domain entries. /// -/// A DdnsDomainLiatParser creates and invokes DdnsDomainListParser for each +/// A DdnsDomainListParser creates and invokes DdnsDomainListParser for each /// domain entry in its list. /// /// A DdnsDomainParser handles the scalars which belong to the domain as well as @@ -561,7 +561,6 @@ public: virtual isc::dhcp::ParserPtr createConfigParser(const std::string& config_id); /// @brief commits the TSIGKeyInfo configuration - /// At the moment this method is a NOP. virtual void commit(); private: @@ -611,14 +610,7 @@ public: /// @param key_list_config is the list of "tsig_key" elements to parse. virtual void build(isc::data::ConstElementPtr key_list_config); - /// @brief Iterates over the internal list of TSIGKeyInfoParsers, - /// invoking commit on each. This causes each parser to instantiate a - /// TSIGKeyInfo from its internal data values and add that key - /// instance to the local key storage area, local_keys_. If all of the - /// key parsers commit cleanly, then update the context key map (keys_) - /// with the contents of local_keys_. This is done to allow for duplicate - /// key detection while parsing the keys, but not get stumped by it - /// updating the context with a valid list. + /// @brief Commits the list of TSIG keys virtual void commit(); private: @@ -657,8 +649,10 @@ public: virtual ~DnsServerInfoParser(); /// @brief Performs the actual parsing of the given "dns_server" element. - /// The results of the parsing are retained internally for use during - /// commit. + /// + /// Parses a configuration for the elements needed to instantiate a + /// DnsServerInfo, validates those entries, creates a DnsServerInfo instance + /// then attempts to add to a list of servers. /// /// @param server_config is the "dns_server" configuration to parse virtual void build(isc::data::ConstElementPtr server_config); @@ -677,8 +671,7 @@ public: virtual isc::dhcp::ParserPtr createConfigParser(const std::string& config_id); - /// @brief Instantiates a DnsServerInfo from internal data values - /// saves it to the storage area pointed to by servers_. + /// @brief commits the configured DnsServerInfo virtual void commit(); private: @@ -729,10 +722,7 @@ public: /// @param server_list_config is the list of "dns_server" elements to parse. virtual void build(isc::data::ConstElementPtr server_list_config); - /// @brief Iterates over the internal list of DnsServerInfoParsers, - /// invoking commit on each. This causes each parser to instantiate a - /// DnsServerInfo from its internal data values and add that that server - /// instance to the storage area, servers_. + /// @brief Commits the list of DnsServerInfos 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 52ff2b5bba..f11c70cb6c 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -441,20 +441,18 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { /// 3. Specifying a negative port number is not allowed. TEST_F(DnsServerInfoTest, invalidEntry) { // Create a config in which both host and ip address are supplied. - // Verify that it builds without throwing but commit fails. + // Verify that build fails. std::string config = "{ \"hostname\": \"pegasus.tmark\", " " \"ip_address\": \"127.0.0.1\" } "; ASSERT_TRUE(fromJSON(config)); - EXPECT_NO_THROW(parser_->build(config_set_)); - EXPECT_THROW(parser_->commit(), D2CfgError); + EXPECT_THROW(parser_->build(config_set_), D2CfgError); // Neither host nor ip address supplied - // Verify that it builds without throwing but commit fails. + // Verify that builds fails. config = "{ \"hostname\": \"\", " " \"ip_address\": \"\" } "; ASSERT_TRUE(fromJSON(config)); - EXPECT_NO_THROW(parser_->build(config_set_)); - EXPECT_THROW(parser_->commit(), D2CfgError); + EXPECT_THROW(parser_->build(config_set_), D2CfgError); // Create a config with a negative port number. // Verify that build fails. @@ -545,11 +543,8 @@ TEST_F(ConfigParseTest, invalidServerList) { isc::dhcp::ParserPtr parser; ASSERT_NO_THROW(parser.reset(new DnsServerInfoListParser("test", servers))); - // Verify that the list builds without errors. - ASSERT_NO_THROW(parser->build(config_set_)); - - // Verify that the list commit fails. - EXPECT_THROW(parser->commit(), D2CfgError); + // Verify that build fails. + EXPECT_THROW(parser->build(config_set_), D2CfgError); } /// @brief Verifies that a list of DnsServerInfo entries parses correctly given