[3121] Moved validation logic to build method for DnsServeInfo parsing

Modified DnsServerInfoParser, DnsServerInfoListParser, and unit tests

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();
     }

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:

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