[5126] Flatten ClientClassDefParser

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -4559,14 +4559,6 @@ TEST_F(Dhcp4ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
-    ConstElementPtr json;
-    ASSERT_NO_THROW(json = parseJSON(config));
-
-    ConstElementPtr status;
-    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
-    ASSERT_TRUE(status);
-    checkResult(status, 1);
-
     EXPECT_THROW(parseDHCP4(config), Dhcp4ParseError);
 }
 

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -4971,13 +4971,6 @@ TEST_F(Dhcp6ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
-    ConstElementPtr json = parseJSON(config);
-
-    ConstElementPtr status;
-    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
-    ASSERT_TRUE(status);
-    checkResult(status, 1);
-
     EXPECT_THROW(parseDHCP6(config), Dhcp6ParseError);
 }
 

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -62,86 +62,89 @@ void
 ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
                             ConstElementPtr class_def_cfg,
                             uint16_t family) {
+    // name is now mandatory
+    std::string name = getString(class_def_cfg, "name");
+    if (name.empty()) {
+        isc_throw(DhcpConfigError,
+                  "not empty parameter 'name' is required "
+                  << getPosition("name", class_def_cfg) << ")");
+    }
 
-    try {
-        std::string name;
-        std::string next_server_txt = "0.0.0.0";
-        std::string sname;
-        std::string filename;
-        ExpressionPtr match_expr;
-        CfgOptionPtr options(new CfgOption());
-
-        // Parse the elements that make up the client class definition.
-        BOOST_FOREACH(ConfigPair param, class_def_cfg->mapValue()) {
-            std::string entry(param.first);
-            ConstElementPtr value(param.second);
-
-            if (entry == "name") {
-                name = value->stringValue();
-
-            } else if (entry == "test") {
-                ExpressionParser parser;
-                parser.parse(match_expr, value, family);
-                
-            } else if (entry == "option-data") {
-                OptionDataListParser opts_parser(family);
-                opts_parser.parse(options, value);
-
-            } else if (entry == "next-server") {
-                next_server_txt = value->stringValue();
-
-            } else if (entry == "server-hostname") {
-                sname = value->stringValue();
-
-            } else if (entry == "boot-file-name") {
-                filename = value->stringValue();
-
-            } else {
-                isc_throw(DhcpConfigError, "invalid parameter '" << entry
-                          << "' (" << value->getPosition() << ")");
-            }
-        }
+    // Parse matching expression
+    ExpressionPtr match_expr;
+    ConstElementPtr test_cfg = class_def_cfg->get("test");
+    if (test_cfg) {
+        ExpressionParser parser;
+        parser.parse(match_expr, test_cfg, family);
+    }
 
-        // name is now mandatory
-        if (name.empty()) {
-            isc_throw(DhcpConfigError,
-                      "not empty parameter 'name' is required");
-        }
+    // Parse option data
+    CfgOptionPtr options(new CfgOption());
+    ConstElementPtr options_cfg = class_def_cfg->get("option-data");
+    if (options_cfg) {
+        OptionDataListParser opts_parser(family);
+        opts_parser.parse(options, options_cfg);
+    }
 
-        // Let's parse the next-server field
-        IOAddress next_server("0.0.0.0");
+    // Let's try to parse the next-server field
+    IOAddress next_server("0.0.0.0");
+    ConstElementPtr next_server_cfg = class_def_cfg->get("next-server");
+    if (next_server_cfg) {
         try {
-            next_server = IOAddress(next_server_txt);
+            next_server = IOAddress(getString(class_def_cfg, "next-server"));
         } catch (const IOError& ex) {
-            isc_throw(DhcpConfigError, "Invalid next-server value specified: '"
-                      << next_server_txt);
+            isc_throw(DhcpConfigError,
+                      "Invalid next-server value specified: '"
+                      << next_server_cfg->stringValue() << "' ("
+                      << next_server_cfg->getPosition() << ")");
         }
 
         if (next_server.getFamily() != AF_INET) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt << "', must be IPv4 address");
+                      << next_server_cfg->stringValue()
+                      << "', must be IPv4 address ("
+                      << next_server_cfg->getPosition() << ")");
         }
 
         if (next_server.isV4Bcast()) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt << "', must not be a broadcast");
+                      << next_server_cfg->stringValue()
+                      << "', must not be a broadcast ("
+                      << next_server_cfg->getPosition() << ")");
         }
+    }
+
+    // Let's try to parse server-hostname
+    std::string sname;
+    ConstElementPtr sname_cfg = class_def_cfg->get("server-hostname");
+    if (sname_cfg) {
+        sname = getString(class_def_cfg, "server-hostname");
 
-        // Let's try to parse server-hostname
         if (sname.length() >= Pkt4::MAX_SNAME_LEN) {
             isc_throw(DhcpConfigError, "server-hostname must be at most "
                       << Pkt4::MAX_SNAME_LEN - 1 << " bytes long, it is "
-                      << sname.length());
+                      << sname.length() << " ("
+                      << sname_cfg->getPosition() << ")");
         }
+    }
+
+    // Let's try to parse boot-file-name
+    std::string filename;
+    ConstElementPtr filename_cfg = class_def_cfg->get("boot-file-name");
+    if (filename_cfg) {
+        filename = getString(class_def_cfg, "boot-file-name");
 
-        // Let's try to parse boot-file-name
         if (filename.length() > Pkt4::MAX_FILE_LEN) {
             isc_throw(DhcpConfigError, "boot-file-name must be at most "
                       << Pkt4::MAX_FILE_LEN - 1 << " bytes long, it is "
-                      << filename.length());
+                      << filename.length() << " ("
+                      << filename_cfg->getPosition() << ")");
         }
 
-        // Add the client class definition
+    }
+
+    // Add the client class definition
+    try {
         class_dictionary->addClass(name, match_expr, options, next_server,
                                    sname, filename);
     } catch (const std::exception& ex) {

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -459,20 +459,6 @@ TEST_F(ClientClassDefParserTest, blankClassName) {
                  DhcpConfigError);
 }
 
-
-// Verifies that a class with an unknown element, fails to parse.
-TEST_F(ClientClassDefParserTest, unknownElement) {
-    std::string cfg_text =
-        "{ \n"
-        "    \"name\": \"one\", \n"
-        "    \"bogus\": \"bad\" \n"
-        "} \n";
-
-    ClientClassDefPtr cclass;
-    ASSERT_THROW(cclass = parseClientClassDef(cfg_text, AF_INET),
-                 DhcpConfigError);
-}
-
 // Verifies that a class with an invalid expression, fails to parse.
 TEST_F(ClientClassDefParserTest, invalidExpression) {
     std::string cfg_text =
@@ -565,22 +551,6 @@ TEST_F(ClientClassDefListParserTest, duplicateClass) {
                  DhcpConfigError);
 }
 
-// Verifies that a class list containing an invalid class entry, fails to
-// parse.
-TEST_F(ClientClassDefListParserTest, invalidClass) {
-    std::string cfg_text =
-        "[ \n"
-        "   { \n"
-        "       \"name\": \"one\", \n"
-        "       \"bogus\": \"bad\" \n"
-        "   } \n"
-        "] \n";
-
-    ClientClassDictionaryPtr dictionary;
-    ASSERT_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET6),
-                 DhcpConfigError);
-}
-
 // Test verifies that without any class specified, the fixed fields have their
 // default, empty value.
 // @todo same with AF_INET6