[master] Merge branch 'trac5039' (SimpleParser)

# Conflicts:
#	src/bin/dhcp4/Makefile.am
#	src/bin/dhcp4/json_config_parser.cc
#	src/bin/dhcp6/Makefile.am
#	src/bin/dhcp6/dhcp6.dox
#	src/bin/dhcp6/json_config_parser.cc
#	src/bin/dhcp6/tests/Makefile.am

doc/guide/dhcp4-srv.xml

@@ -1578,22 +1578,12 @@ It is merely echoed by the server
         </listitem>
 
         <listitem>
-          <simpara><command>csv-format</command> - if this value is not specified
-          and the definition for the particular option exists, the server will assume
-          that the option data is specified as a list of comma separated values to be
-          assigned to individual fields of the DHCP option. If the definition
-          does not exist for this option, the server will assume that the data
-          parameter contains the option payload in the binary format (represented
-          as a string of hexadecimal digits). Note that not specifying this
-          parameter doesn't imply that it defaults to a fixed value, but
-          the configuration data interpretation also depends on the presence
-          of the option definition. An administrator must be aware if the
-          definition for the particular option exists when this parameter
-          is not specified. It is generally recommended to not specify this
-          parameter only for the options for which the definition exists, e.g.
-          standard options. Setting <command>csv-format</command> to an explicit
-          value will cause the server to strictly check the format of the option
-          data specified.
+          <simpara><command>csv-format</command> - if this value is not
+          specified the server will assume that the option data is specified as
+          a list of comma separated values to be assigned to individual fields
+          of the DHCP option. This behavior has changed in Kea 1.2. Older
+          versions used additional logic to determined whether the csv-format
+          should be true or false. That is no longer the case.
           </simpara>
         </listitem>
       </itemizedlist>

doc/guide/dhcp6-srv.xml

@@ -1721,22 +1721,12 @@ should include options from the isc option space:
         </listitem>
 
         <listitem>
-          <simpara><command>csv-format</command> - if this value is not specified
-          and the definition for the particular option exists, the server will assume
-          that the option data is specified as a list of comma separated values to be
-          assigned to individual fields of the DHCP option. If the definition
-          does not exist for this option, the server will assume that the data
-          parameter contains the option payload in the binary format (represented
-          as a string of hexadecimal digits). Note that not specifying this
-          parameter doesn't imply that it defaults to a fixed value, but
-          the configuration data interpretation also depends on the presence
-          of the option definition. An administrator must be aware if the
-          definition for the particular option exists when this parameter
-          is not specified. It is generally recommended to not specify this
-          parameter only for the options for which the definition exists, e.g.
-          standard options. Setting <command>csv-format</command> to an explicit
-          value will cause the server to strictly check the format of the option
-          data specified.
+          <simpara><command>csv-format</command> - if this value is not
+          specified the server will assume that the option data is specified as
+          a list of comma separated values to be assigned to individual fields
+          of the DHCP option. This behavior has changed in Kea 1.2. Older
+          versions used additional logic to determined whether the csv-format
+          should be true or false. That is no longer the case.
           </simpara>
         </listitem>
       </itemizedlist>

src/bin/dhcp4/Makefile.am

@@ -62,12 +62,11 @@ libdhcp4_la_SOURCES += json_config_parser.cc json_config_parser.h
 libdhcp4_la_SOURCES += dhcp4_log.cc dhcp4_log.h
 libdhcp4_la_SOURCES += dhcp4_srv.cc dhcp4_srv.h
 libdhcp4_la_SOURCES += dhcp4to6_ipc.cc dhcp4to6_ipc.h
-
 libdhcp4_la_SOURCES += dhcp4_lexer.ll location.hh position.hh stack.hh
 libdhcp4_la_SOURCES += dhcp4_parser.cc dhcp4_parser.h
 libdhcp4_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h
-
 libdhcp4_la_SOURCES += kea_controller.cc
+libdhcp4_la_SOURCES += simple_parser4.cc simple_parser4.h
 
 nodist_libdhcp4_la_SOURCES = dhcp4_messages.h dhcp4_messages.cc
 EXTRA_DIST += dhcp4_messages.mes

src/bin/dhcp4/dhcp4.dox

@@ -24,6 +24,9 @@ component implementation.
 
 @section dhcpv4ConfigParser Configuration Parser in DHCPv4
 
+Note: parsers are currently being migrated to @ref isc::data::SimpleParser. See
+@ref ccSimpleParser page for details.
+
 The common configuration parsers for the DHCP servers are located in the
 src/lib/dhcpsrv/parsers/ directory. Parsers specific to the DHCPv4 component
 are located in the src/bin/dhcp4/json_config_parser.cc. These parsers derive

src/bin/dhcp4/json_config_parser.cc

@@ -8,6 +8,7 @@
 
 #include <cc/command_interpreter.h>
 #include <dhcp4/dhcp4_log.h>
+#include <dhcp4/simple_parser4.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option_definition.h>
 #include <dhcpsrv/cfg_option.h>
@@ -190,8 +191,7 @@ protected:
             parser = new Pools4ListParser(config_id, pools_);
         } else if (config_id.compare("relay") == 0) {
             parser = new RelayInfoParser(config_id, relay_info_, Option::V4);
-        } else if (config_id.compare("option-data") == 0) {
-            parser = new OptionDataListParser(config_id, options_, AF_INET);
+        // option-data has been converted to SimpleParser already.
         } else if (config_id.compare("match-client-id") == 0) {
             parser = new BooleanParser(config_id, boolean_values_);
         } else if (config_id.compare("4o6-subnet") == 0) {
@@ -424,10 +424,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id,
         parser = new IfacesConfigParser4();
     } else if (config_id.compare("subnet4") == 0) {
         parser = new Subnets4ListConfigParser(config_id);
-    } else if (config_id.compare("option-data") == 0) {
-        parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET);
-    } else if (config_id.compare("option-def") == 0) {
-        parser  = new OptionDefListParser(config_id, globalContext());
+    // option-data and option-def have been converted to SimpleParser already.
     } else if ((config_id.compare("next-server") == 0)) {
         parser  = new StringParser(config_id,
                                     globalContext()->string_values_);
@@ -537,7 +534,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // Please do not change this order!
     ParserCollection independent_parsers;
     ParserPtr subnet_parser;
-    ParserPtr option_parser;
     ParserPtr iface_parser;
     ParserPtr leases_parser;
     ParserPtr client_classes_parser;
@@ -566,10 +562,40 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // the name of the failing parser can be retrieved in the "catch" clause.
     ConfigPair config_pair;
     try {
+
+        // This is a way to convert ConstElementPtr to ElementPtr.
+        // We need a config that can be edited, because we will insert
+        // default values and will insert derived values as well.
+        ElementPtr mutable_cfg = Element::getMutableMap(config_set);
+
+        // Set all default values if not specified by the user.
+        SimpleParser4::setAllDefaults(mutable_cfg);
+
+        // We need definitions first
+        ConstElementPtr option_defs = mutable_cfg->get("option-def");
+        if (option_defs) {
+            OptionDefListParser parser;
+            CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
+            parser.parse(cfg_option_def, option_defs);
+        }
+
         // Make parsers grouping.
         const std::map<std::string, ConstElementPtr>& values_map =
-                                                        config_set->mapValue();
+                                                        mutable_cfg->mapValue();
         BOOST_FOREACH(config_pair, values_map) {
+
+            if (config_pair.first == "option-def") {
+                // This is converted to SimpleParser and is handled already above.
+                continue;
+            }
+
+            if (config_pair.first == "option-data") {
+                OptionDataListParser parser(AF_INET);
+                CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption();
+                parser.parse(cfg_option, config_pair.second);
+                continue;
+            }
+
             ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first,
                                                            config_pair.second));
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED)
@@ -578,8 +604,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                 subnet_parser = parser;
             } else if (config_pair.first == "lease-database") {
                 leases_parser = parser;
-            } else if (config_pair.first == "option-data") {
-                option_parser = parser;
             } else if (config_pair.first == "interfaces-config") {
                 // The interface parser is independent from any other
                 // parser and can be run here before any other parsers.
@@ -605,15 +629,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
             }
         }
 
-        // The option values parser is the next one to be run.
-        std::map<std::string, ConstElementPtr>::const_iterator option_config =
-            values_map.find("option-data");
-        if (option_config != values_map.end()) {
-            config_pair.first = "option-data";
-            option_parser->build(option_config->second);
-            option_parser->commit();
-        }
-
         // The class definitions parser is the next one to be run.
         std::map<std::string, ConstElementPtr>::const_iterator cc_config =
             values_map.find("client-classes");

src/bin/dhcp4/simple_parser4.cc

@@ -0,0 +1,106 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <dhcp4/simple_parser4.h>
+#include <cc/data.h>
+#include <boost/foreach.hpp>
+
+using namespace isc::data;
+
+namespace isc {
+namespace dhcp {
+/// @brief This sets of arrays define the default values and
+///        values inherited (derived) between various scopes.
+///
+/// Each of those is documented in @file simple_parser4.cc. This
+/// is different than most other comments in Kea code. The reason
+/// for placing those in .cc rather than .h file is that it
+/// is expected to be one centralized place to look at for
+/// the default values. This is expected to be looked at also by
+/// people who are not skilled in C or C++, so they may be
+/// confused with the differences between declaration and definition.
+/// As such, there's one file to look at that hopefully is readable
+/// without any C or C++ skills.
+///
+/// @{
+
+/// @brief This table defines default values for option definitions in DHCPv4.
+///
+/// Dhcp4 may contain an array called option-def that enumerates new option
+/// definitions. This array lists default values for those option definitions.
+const SimpleDefaults SimpleParser4::OPTION4_DEF_DEFAULTS = {
+    { "record-types", Element::string,  ""},
+    { "space",        Element::string,  "dhcp4"},
+    { "array",        Element::boolean, "false"},
+    { "encapsulate",  Element::string,  "" }
+};
+
+/// @brief This table defines default values for options in DHCPv4.
+///
+/// Dhcp4 usually contains option values (option-data) defined in global,
+/// subnet, class or host reservations scopes. This array lists default values
+/// for those option-data declarations.
+const SimpleDefaults SimpleParser4::OPTION4_DEFAULTS = {
+    { "space",        Element::string,  "dhcp4"},
+    { "csv-format",   Element::boolean, "true"},
+    { "encapsulate",  Element::string,  "" }
+};
+
+/// @brief This table defines default global values for DHCPv4
+///
+/// Some of the global parameters defined in the global scope (i.e. directly
+/// in Dhcp4) are optional. If not defined, the following values will be
+/// used.
+const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = {
+    { "renew-timer",        Element::integer, "900" },
+    { "rebind-timer",       Element::integer, "1800" },
+    { "valid-lifetime",     Element::integer, "7200" }
+};
+
+/// @brief List of parameters that can be inherited from the global to subnet4 scope.
+///
+/// Some parameters may be defined on both global (directly in Dhcp4) and
+/// subnet (Dhcp4/subnet4/...) scope. If not defined in the subnet scope,
+/// the value is being inherited (derived) from the global scope. This
+/// array lists all of such parameters.
+const ParamsList SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4 = {
+    "renew-timer",
+    "rebind-timer",
+    "valid-lifetime"
+};
+/// @}
+
+/// ---------------------------------------------------------------------------
+/// --- end of default values -------------------------------------------------
+/// ---------------------------------------------------------------------------
+
+size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) {
+    size_t cnt = 0;
+
+    // Set global defaults first.
+    cnt = setDefaults(global, GLOBAL4_DEFAULTS);
+
+    // Now set option definition defaults for each specified option definition
+    ConstElementPtr option_defs = global->get("option-def");
+    if (option_defs) {
+        BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) {
+            cnt += SimpleParser::setDefaults(option_def, OPTION4_DEF_DEFAULTS);
+        }
+    }
+
+    // Finally, set the defaults for option data
+    ConstElementPtr options = global->get("option-data");
+    if (options) {
+        BOOST_FOREACH(ElementPtr single_option, options->listValue()) {
+            cnt += SimpleParser::setDefaults(single_option, OPTION4_DEFAULTS);
+        }
+    }
+
+    return (cnt);
+}
+
+};
+};

src/bin/dhcp4/simple_parser4.h

@@ -0,0 +1,40 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef SIMPLE_PARSER4_H
+#define SIMPLE_PARSER4_H
+
+#include <cc/simple_parser.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief SimpleParser specialized for DHCPv4
+///
+/// This class is a @ref isc::data::SimpleParser dedicated to DHCPv4 parser.
+/// In particular, it contains all the default values and names of the
+/// parameters that are to be derived (inherited) between scopes.
+/// For the actual values, see @file simple_parser4.cc
+class SimpleParser4 : public isc::data::SimpleParser {
+public:
+    /// @brief Sets all defaults for DHCPv4 configuration
+    ///
+    /// This method sets global, option data and option definitions defaults.
+    ///
+    /// @param global scope to be filled in with defaults.
+    /// @return number of default values added
+    static size_t setAllDefaults(isc::data::ElementPtr global);
+
+    // see simple_parser4.cc for comments for those parameters
+    static const isc::data::SimpleDefaults OPTION4_DEF_DEFAULTS;
+    static const isc::data::SimpleDefaults OPTION4_DEFAULTS;
+    static const isc::data::SimpleDefaults GLOBAL4_DEFAULTS;
+    static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET4;
+};
+
+};
+};
+#endif

src/bin/dhcp4/tests/Makefile.am

@@ -94,6 +94,7 @@ dhcp4_unittests_SOURCES += out_of_range_unittest.cc
 dhcp4_unittests_SOURCES += decline_unittest.cc
 dhcp4_unittests_SOURCES += kea_controller_unittest.cc
 dhcp4_unittests_SOURCES += dhcp4to6_ipc_unittest.cc
+dhcp4_unittests_SOURCES += simple_parser4_unittest.cc
 
 nodist_dhcp4_unittests_SOURCES = marker_file.h test_libraries.h
 

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -662,8 +662,9 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) {
     Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()->
         getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200"));
     ASSERT_TRUE(subnet);
-    EXPECT_TRUE(subnet->getT1().unspecified());
+    EXPECT_FALSE(subnet->getT1().unspecified());
     EXPECT_FALSE(subnet->getT2().unspecified());
+    EXPECT_EQ(900, subnet->getT1()); // that's the default value
     EXPECT_EQ(2000, subnet->getT2());
     EXPECT_EQ(4000, subnet->getValid());
 
@@ -699,7 +700,8 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) {
     ASSERT_TRUE(subnet);
     EXPECT_FALSE(subnet->getT1().unspecified());
     EXPECT_EQ(1000, subnet->getT1());
-    EXPECT_TRUE(subnet->getT2().unspecified());
+    EXPECT_FALSE(subnet->getT2().unspecified());
+    EXPECT_EQ(1800, subnet->getT2()); // that's the default value
     EXPECT_EQ(4000, subnet->getValid());
 
     // Check that subnet-id is 1

src/bin/dhcp4/tests/host_options_unittest.cc

@@ -203,7 +203,7 @@ const char* HOST_CONFIGS[] = {
         "\"valid-lifetime\": 600,"
         "\"option-data\": [ {"
         "    \"name\": \"vivso-suboptions\","
-        "    \"data\": 4491"
+        "    \"data\": \"4491\""
         "},"
         "{"
         "    \"name\": \"tftp-servers\","
@@ -221,7 +221,7 @@ const char* HOST_CONFIGS[] = {
         "        \"ip-address\": \"10.0.0.7\","
         "        \"option-data\": [ {"
         "            \"name\": \"vivso-suboptions\","
-        "            \"data\": 4491"
+        "            \"data\": \"4491\""
         "        },"
         "        {"
         "            \"name\": \"tftp-servers\","

src/bin/dhcp4/tests/simple_parser4_unittest.cc

@@ -0,0 +1,91 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <config.h>
+#include <gtest/gtest.h>
+#include <dhcp4/simple_parser4.h>
+#include <cc/data.h>
+
+using namespace isc::data;
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
+/// @brief DHCP Parser test fixture class
+class SimpleParser4Test : public ::testing::Test {
+public:
+    /// @brief Checks if specified map has an integer parameter with expected value
+    ///
+    /// @param map map to be checked
+    /// @param param_name name of the parameter to be checked
+    /// @param exp_value expected value of the parameter.
+    void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name,
+                           int64_t exp_value) {
+
+        // First check if the passed element is a map.
+        ASSERT_EQ(Element::map, map->getType());
+
+        // Now try to get the element being checked
+        ConstElementPtr elem = map->get(param_name);
+        ASSERT_TRUE(elem);
+
+        // Now check if it's indeed integer
+        ASSERT_EQ(Element::integer, elem->getType());
+
+        // Finally, check if its value meets expectation.
+        EXPECT_EQ(exp_value, elem->intValue());
+    }
+};
+
+// This test checks if global defaults are properly set for DHCPv4.
+TEST_F(SimpleParser4Test, globalDefaults4) {
+
+    ElementPtr empty = Element::fromJSON("{ }");
+    size_t num = 0;
+
+    EXPECT_NO_THROW(num = SimpleParser4::setAllDefaults(empty));
+
+
+    // We expect at least 3 parameters to be inserted.
+    EXPECT_TRUE(num >= 3);
+
+    checkIntegerValue(empty, "valid-lifetime", 7200);
+    checkIntegerValue(empty, "rebind-timer", 1800);
+    checkIntegerValue(empty, "renew-timer", 900);
+
+    // Make sure that preferred-lifetime is not set for v4 (it's v6 only
+    // parameter)
+    EXPECT_FALSE(empty->get("preferred-lifetime"));
+}
+
+// This test checks if the parameters can be inherited from the global
+// scope to the subnet scope.
+TEST_F(SimpleParser4Test, inheritGlobalToSubnet4) {
+    ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1,"
+                                          "  \"rebind-timer\": 2,"
+                                          "  \"preferred-lifetime\": 3,"
+                                          "  \"valid-lifetime\": 4"
+                                          "}");
+    ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }");
+
+    // we should inherit 3 parameters. Renew-timer should remain intact,
+    // as it was already defined in the subnet scope.
+    size_t num;
+    EXPECT_NO_THROW(num = SimpleParser4::deriveParams(global, subnet,
+                          SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4));
+    EXPECT_EQ(2, num);
+
+    // Check the values. 2 of them are inherited, while the third one
+    // was already defined in the subnet, so should not be inherited.
+    checkIntegerValue(subnet, "renew-timer", 100);
+    checkIntegerValue(subnet, "rebind-timer", 2);
+    checkIntegerValue(subnet, "valid-lifetime", 4);
+}
+
+};
+};
+};

src/bin/dhcp6/Makefile.am

@@ -31,8 +31,8 @@ if GENERATE_DOCS
 
 kea-dhcp6.8: kea-dhcp6.xml
 	@XSLTPROC@ --novalid --xinclude --nonet -o $@ \
-        http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl \
-        $(srcdir)/kea-dhcp6.xml
+	http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl \
+	$(srcdir)/kea-dhcp6.xml
 
 else
 
@@ -63,12 +63,11 @@ libdhcp6_la_SOURCES += dhcp6_srv.cc dhcp6_srv.h
 libdhcp6_la_SOURCES += ctrl_dhcp6_srv.cc ctrl_dhcp6_srv.h
 libdhcp6_la_SOURCES += json_config_parser.cc json_config_parser.h
 libdhcp6_la_SOURCES += dhcp6to4_ipc.cc dhcp6to4_ipc.h
-
 libdhcp6_la_SOURCES += dhcp6_lexer.ll location.hh position.hh stack.hh
 libdhcp6_la_SOURCES += dhcp6_parser.cc dhcp6_parser.h
 libdhcp6_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h
-
 libdhcp6_la_SOURCES += kea_controller.cc
+libdhcp6_la_SOURCES += simple_parser6.cc simple_parser6.h
 
 nodist_libdhcp6_la_SOURCES = dhcp6_messages.h dhcp6_messages.cc
 EXTRA_DIST += dhcp6_messages.mes

src/bin/dhcp6/dhcp6.dox

@@ -34,7 +34,9 @@ the new parser, here's how you use it:
  // can be replaced with this:
  Parser6Context parser;
  json = parser.parseFile(file_name, Parser6Context::PARSER_DHCP6);
-@endcode
+
+Note: parsers are currently being migrated to @ref isc::data::SimpleParser. See
+@ref ccSimpleParser page for details.
 
 The common configuration parsers for the DHCP servers are located in the
 src/lib/dhcpsrv/parsers/ directory. Parsers specific to the DHCPv6 component

src/bin/dhcp6/json_config_parser.cc

@@ -13,6 +13,7 @@
 #include <dhcp/libdhcp++.h>
 #include <dhcp6/json_config_parser.h>
 #include <dhcp6/dhcp6_log.h>
+#include <dhcp6/simple_parser6.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -184,10 +185,12 @@ public:
                                                              uint32_values_));
                 parser = code_parser;
             } else if (entry == "option-data") {
-                OptionDataListParserPtr option_parser(new OptionDataListParser(entry,
-                                                                               options_,
-                                                                               AF_INET6));
-                parser = option_parser;
+                OptionDataListParser opts_parser(AF_INET6);
+                opts_parser.parse(options_, param.second);
+
+                // OptionDataListParser is converted to SimpleParser already,
+                // no need to go through build/commit phases.
+                continue;
             } else if (entry == "user-context") {
                 user_context_ = param.second;
                 continue; // no parser to remember, simply store the value
@@ -425,8 +428,7 @@ protected:
             parser = new RelayInfoParser(config_id, relay_info_, Option::V6);
         } else if (config_id.compare("pd-pools") == 0) {
             parser = new PdPoolListParser(config_id, pools_);
-        } else if (config_id.compare("option-data") == 0) {
-            parser = new OptionDataListParser(config_id, options_, AF_INET6);
+        // option-data was here, but it is now converted to SimpleParser
         } else if (config_id.compare("rapid-commit") == 0) {
             parser = new BooleanParser(config_id, boolean_values_);
         } else {
@@ -703,10 +705,11 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
         parser = new IfacesConfigParser6();
     } else if (config_id.compare("subnet6") == 0) {
         parser = new Subnets6ListConfigParser(config_id);
-    } else if (config_id.compare("option-data") == 0) {
-        parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET6);
-    } else if (config_id.compare("option-def") == 0) {
-        parser  = new OptionDefListParser(config_id, globalContext());
+    // option-data and option-def are no longer needed here. They're now
+    //  converted to SimpleParser and are handled in configureDhcp6Server
+    }  else if (config_id.compare("version") == 0) {
+        parser  = new StringParser(config_id,
+                                   globalContext()->string_values_);
     } else if (config_id.compare("lease-database") == 0) {
         parser = new DbAccessParser(config_id, DbAccessParser::LEASE_DB);
     } else if (config_id.compare("hosts-database") == 0) {
@@ -805,7 +808,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     // Please do not change this order!
     ParserCollection independent_parsers;
     ParserPtr subnet_parser;
-    ParserPtr option_parser;
     ParserPtr iface_parser;
     ParserPtr leases_parser;
     ParserPtr client_classes_parser;
@@ -835,10 +837,39 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     ConfigPair config_pair;
     try {
 
+        // This is a way to convert ConstElementPtr to ElementPtr.
+        // We need a config that can be edited, because we will insert
+        // default values and will insert derived values as well.
+        ElementPtr mutable_cfg = Element::getMutableMap(config_set);
+
+        SimpleParser6::setAllDefaults(mutable_cfg);
+
         // Make parsers grouping.
         const std::map<std::string, ConstElementPtr>& values_map =
-            config_set->mapValue();
+            mutable_cfg->mapValue();
+
+        // We need definitions first
+        ConstElementPtr option_defs = mutable_cfg->get("option-def");
+        if (option_defs) {
+            OptionDefListParser parser;
+            CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
+            parser.parse(cfg_option_def, option_defs);
+        }
+
         BOOST_FOREACH(config_pair, values_map) {
+
+            if (config_pair.first == "option-def") {
+                // This is converted to SimpleParser and is handled already above.
+                continue;
+            }
+
+            if (config_pair.first == "option-data") {
+                OptionDataListParser parser(AF_INET6);
+                CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption();
+                parser.parse(cfg_option, config_pair.second);
+                continue;
+            }
+
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
                                                            config_pair.second));
             LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED)
@@ -847,8 +878,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
                 subnet_parser = parser;
             } else if (config_pair.first == "lease-database") {
                 leases_parser = parser;
-            } else if (config_pair.first == "option-data") {
-                option_parser = parser;
             } else if (config_pair.first == "hooks-libraries") {
                 // Executing the commit will alter currently loaded hooks
                 // libraries. Check if the supplied libraries are valid,
@@ -875,15 +904,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
             }
         }
 
-        // The option values parser is the next one to be run.
-        std::map<std::string, ConstElementPtr>::const_iterator option_config =
-            values_map.find("option-data");
-        if (option_config != values_map.end()) {
-            config_pair.first = "option-data";
-            option_parser->build(option_config->second);
-            option_parser->commit();
-        }
-
         // The class definitions parser is the next one to be run.
         std::map<std::string, ConstElementPtr>::const_iterator cc_config =
             values_map.find("client-classes");

src/bin/dhcp6/simple_parser6.cc

@@ -0,0 +1,108 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <dhcp6/simple_parser6.h>
+#include <cc/data.h>
+#include <boost/foreach.hpp>
+
+using namespace isc::data;
+
+namespace isc {
+namespace dhcp {
+/// @brief This sets of arrays define the default values and
+///        values inherited (derived) between various scopes.
+///
+/// Each of those is documented in @file simple_parser6.cc. This
+/// is different than most other comments in Kea code. The reason
+/// for placing those in .cc rather than .h file is that it
+/// is expected to be one centralized place to look at for
+/// the default values. This is expected to be looked at also by
+/// people who are not skilled in C or C++, so they may be
+/// confused with the differences between declaration and definition.
+/// As such, there's one file to look at that hopefully is readable
+/// without any C or C++ skills.
+///
+/// @{
+
+/// @brief This table defines default values for option definitions in DHCPv6.
+///
+/// Dhcp6 may contain an array called option-def that enumerates new option
+/// definitions. This array lists default values for those option definitions.
+const SimpleDefaults SimpleParser6::OPTION6_DEF_DEFAULTS = {
+    { "record-types", Element::string,  ""},
+    { "space",        Element::string,  "dhcp6"},
+    { "array",        Element::boolean, "false"},
+    { "encapsulate",  Element::string,  "" }
+};
+
+/// @brief This table defines default values for options in DHCPv6.
+///
+/// Dhcp6 usually contains option values (option-data) defined in global,
+/// subnet, class or host reservations scopes. This array lists default values
+/// for those option-data declarations.
+const SimpleDefaults SimpleParser6::OPTION6_DEFAULTS = {
+    { "space",        Element::string,  "dhcp6"},
+    { "csv-format",   Element::boolean, "true"},
+    { "encapsulate",  Element::string,  "" }
+};
+
+/// @brief This table defines default global values for DHCPv6
+///
+/// Some of the global parameters defined in the global scope (i.e. directly
+/// in Dhcp6) are optional. If not defined, the following values will be
+/// used.
+const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = {
+    { "renew-timer",        Element::integer, "900" },
+    { "rebind-timer",       Element::integer, "1800" },
+    { "preferred-lifetime", Element::integer, "3600" },
+    { "valid-lifetime",     Element::integer, "7200" }
+};
+
+/// @brief List of parameters that can be inherited from the global to subnet6 scope.
+///
+/// Some parameters may be defined on both global (directly in Dhcp6) and
+/// subnet (Dhcp6/subnet6/...) scope. If not defined in the subnet scope,
+/// the value is being inherited (derived) from the global scope. This
+/// array lists all of such parameters.
+const ParamsList SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6 = {
+    "renew-timer",
+    "rebind-timer",
+    "preferred-lifetime",
+    "valid-lifetime"
+};
+/// @}
+
+/// ---------------------------------------------------------------------------
+/// --- end of default values -------------------------------------------------
+/// ---------------------------------------------------------------------------
+
+size_t SimpleParser6::setAllDefaults(isc::data::ElementPtr global) {
+    size_t cnt = 0;
+
+    // Set global defaults first.
+    cnt = setDefaults(global, GLOBAL6_DEFAULTS);
+
+    // Now set the defaults for each specified option definition
+    ConstElementPtr option_defs = global->get("option-def");
+    if (option_defs) {
+        BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) {
+            cnt += SimpleParser::setDefaults(option_def, OPTION6_DEF_DEFAULTS);
+        }
+    }
+
+    // Finally, set the defaults for option data
+    ConstElementPtr options = global->get("option-data");
+    if (options) {
+        BOOST_FOREACH(ElementPtr single_option, options->listValue()) {
+            cnt += SimpleParser::setDefaults(single_option, OPTION6_DEFAULTS);
+        }
+    }
+
+    return (cnt);
+}
+
+};
+};

src/bin/dhcp6/simple_parser6.h

@@ -0,0 +1,42 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef SIMPLE_PARSER6_H
+#define SIMPLE_PARSER6_H
+
+#include <cc/simple_parser.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief SimpleParser specialized for DHCPv6
+///
+/// This class is a @ref isc::data::SimpleParser dedicated to DHCPv6 parser.
+/// In particular, it contains all the default values and names of the
+/// parameters that are to be derived (inherited) between scopes.
+/// For the actual values, see @file simple_parser6.cc
+class SimpleParser6 : public isc::data::SimpleParser {
+public:
+
+    /// @brief Sets all defaults for DHCPv6 configuration
+    ///
+    /// This method sets global, option data and option definitions defaults.
+    ///
+    /// @param global scope to be filled in with defaults.
+    /// @return number of default values added
+    static size_t setAllDefaults(isc::data::ElementPtr global);
+
+    // see simple_parser6.cc for comments for those parameters
+    static const isc::data::SimpleDefaults OPTION6_DEF_DEFAULTS;
+    static const isc::data::SimpleDefaults OPTION6_DEFAULTS;
+    static const isc::data::SimpleDefaults GLOBAL6_DEFAULTS;
+    static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET6;
+};
+
+};
+};
+
+#endif

src/bin/dhcp6/tests/Makefile.am

@@ -95,6 +95,7 @@ dhcp6_unittests_SOURCES += kea_controller_unittest.cc
 dhcp6_unittests_SOURCES += dhcp6to4_ipc_unittest.cc
 dhcp6_unittests_SOURCES += classify_unittests.cc
 dhcp6_unittests_SOURCES += parser_unittest.cc
+dhcp6_unittests_SOURCES += simple_parser6_unittest.cc
 
 nodist_dhcp6_unittests_SOURCES  = marker_file.h test_libraries.h
 

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -2030,6 +2030,7 @@ TEST_F(Dhcpv6SrvTest, rsooOverride) {
         "    } ],"
         "    \"option-data\": [ {"
         "      \"code\": 120,"
+        "      \"csv-format\": false,"
         "      \"data\": \"05\""
         "    } ],"
         "    \"preferred-lifetime\": 3000,"

src/bin/dhcp6/tests/host_unittest.cc

@@ -229,7 +229,7 @@ const char* CONFIGS[] = {
         "\"renew-timer\": 1000, "
         "\"option-data\": [ {"
         "    \"name\": \"vendor-opts\","
-        "    \"data\": 4491"
+        "    \"data\": \"4491\""
         "},"
         "{"
         "    \"name\": \"tftp-servers\","
@@ -247,7 +247,7 @@ const char* CONFIGS[] = {
         "        \"ip-addresses\": [ \"2001:db8:1::2\" ],"
         "        \"option-data\": [ {"
         "            \"name\": \"vendor-opts\","
-        "            \"data\": 4491"
+        "            \"data\": \"4491\""
         "        },"
         "        {"
         "            \"name\": \"tftp-servers\","

src/bin/dhcp6/tests/simple_parser6_unittest.cc

@@ -0,0 +1,87 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <config.h>
+#include <cc/data.h>
+#include <dhcp6/simple_parser6.h>
+#include <gtest/gtest.h>
+
+using namespace isc;
+using namespace isc::data;
+using namespace isc::dhcp;
+
+namespace {
+
+/// @brief DHCP Parser test fixture class
+class SimpleParser6Test : public ::testing::Test {
+public:
+    /// @brief Checks if specified map has an integer parameter with expected value
+    ///
+    /// @param map map to be checked
+    /// @param param_name name of the parameter to be checked
+    /// @param exp_value expected value of the parameter.
+    void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name,
+                           int64_t exp_value) {
+
+        // First check if the passed element is a map.
+        ASSERT_EQ(Element::map, map->getType());
+
+        // Now try to get the element being checked
+        ConstElementPtr elem = map->get(param_name);
+        ASSERT_TRUE(elem);
+
+        // Now check if it's indeed integer
+        ASSERT_EQ(Element::integer, elem->getType());
+
+        // Finally, check if its value meets expectation.
+        EXPECT_EQ(exp_value, elem->intValue());
+    }
+};
+
+// This test checks if global defaults are properly set for DHCPv6.
+TEST_F(SimpleParser6Test, globalDefaults6) {
+
+    ElementPtr empty = Element::fromJSON("{ }");
+    size_t num = 0;
+
+    EXPECT_NO_THROW(num = SimpleParser6::setAllDefaults(empty));
+
+    // We expect at least 4 parameters to be inserted.
+    EXPECT_TRUE(num >= 4);
+
+    checkIntegerValue(empty, "valid-lifetime", 7200);
+    checkIntegerValue(empty, "preferred-lifetime", 3600);
+    checkIntegerValue(empty, "rebind-timer", 1800);
+    checkIntegerValue(empty, "renew-timer", 900);
+}
+
+// This test checks if the parameters can be inherited from the global
+// scope to the subnet scope.
+TEST_F(SimpleParser6Test, inheritGlobalToSubnet6) {
+    ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1,"
+                                          "  \"rebind-timer\": 2,"
+                                          "  \"preferred-lifetime\": 3,"
+                                          "  \"valid-lifetime\": 4"
+                                          "}");
+    ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }");
+
+    // we should inherit 3 parameters. Renew-timer should remain intact,
+    // as it was already defined in the subnet scope.
+    size_t num;
+    EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet,
+                                                     SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6));
+    EXPECT_EQ(3, num);
+
+    // Check the values. 3 of them are inherited, while the fourth one
+    // was already defined in the subnet, so should not be inherited.
+    checkIntegerValue(subnet, "renew-timer", 100);
+    checkIntegerValue(subnet, "rebind-timer", 2);
+    checkIntegerValue(subnet, "preferred-lifetime", 3);
+    checkIntegerValue(subnet, "valid-lifetime", 4);
+}
+
+};
+

src/lib/cc/Makefile.am

@@ -7,6 +7,7 @@ AM_CXXFLAGS = $(KEA_CXXFLAGS)
 lib_LTLIBRARIES = libkea-cc.la
 libkea_cc_la_SOURCES = data.cc data.h
 libkea_cc_la_SOURCES += command_interpreter.cc command_interpreter.h
+libkea_cc_la_SOURCES += simple_parser.cc simple_parser.h
 
 libkea_cc_la_LIBADD  = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 libkea_cc_la_LIBADD += $(BOOST_LIBS)
@@ -18,4 +19,6 @@ libkea_cc_la_LDFLAGS = -no-undefined -version-info 1:0:0
 libkea_cc_includedir = $(pkgincludedir)/cc
 libkea_cc_include_HEADERS = data.h
 
+EXTRA_DIST = cc.dox
+
 CLEANFILES = *.gcno *.gcda

src/lib/cc/cc.dox

@@ -0,0 +1,99 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+/**
+ @page libcc libcc - Kea configuration commands library
+
+@section ccSimpleParser Simple JSON Parser
+
+Since the early beginnings, our configuration parsing code was a mess. It
+started back in 2011 when Tomek joined ISC recently and was told to implement
+Kea configuration handling in similar way as DNS Auth module. The code grew
+over time (DHCP configuration is significantly more complex than DNS, with
+more interdependent values) and as of Kea 1.1 release it became very difficult
+to manage. The decision has been made to significantly refactor or even
+partially rewrite the parser code. The design for this effort is documented
+here: http://kea.isc.org/wiki/SimpleParser It discusses the original issues
+and the proposed architecture.
+
+There are several aspects of this new approach. The base class for all parsers
+is @ref isc::data::SimpleParser. It simplifies the parsers based on
+@ref isc::dhcp::DhcpConfigParser by rejecting the
+concept of build/commit phases. Instead, there should be a single method
+called parse that takes ConstElementPtr as a single parameter (that's the
+JSON structures to be parsed) and returns the config structure to be used
+in CfgMgr. An example of such a method can be the following:
+
+@code
+std::pair<OptionDescriptor, std::string>
+OptionDataParser::parse(isc::data::ConstElementPtr single_option)
+@endcode
+
+Since each derived class will have the same parameter, but a different return
+type, it's not possible to use virtual methods mechanism. That's perfectly
+ok, though, as there is only a single instance of the class needed to parse
+arbitrary number of parameters of the same type. There is no need to
+keep pointers to the parser object. As such there's fewer incentives to have
+one generic way to handle all parsers.
+
+@subsection ccSimpleParserDefaults Default values in Simple Parser
+
+Another simplification comes from the fact that almost all parameters
+are mandatory in SimpleParser. One source of complexities in the old
+parser was the necessity to deal with optional parameters. Simple
+parser deals with that by explicitly requiring the input structure to
+have all parameters filled. Obviously, it's not feasible to expect
+everyone to always specify all parameters, therefore there's an easy
+way to fill missing parameters with their default values. There are
+several methods to do this, but the most generic one is:
+
+@code
+static size_t
+isc::data::SimpleParser::setDefaults(isc::data::ElementPtr scope,
+                                     const SimpleDefaults& default_values);
+@endcode
+
+It takes a pointer to element to be filled with default values and
+vector of default values. Having those values specified in a single
+place in a way that can easily be read even by non-programmers is a
+big advantage of this approach. Here's an example from simple_parser.cc file:
+
+@code
+/// This table defines default values for option definitions in DHCPv6
+const SimpleDefaults OPTION6_DEF_DEFAULTS = {
+    { "record-types", Element::string,  ""},
+    { "space",        Element::string,  "dhcp6"},
+    { "array",        Element::boolean, "false"},
+    { "encapsulate",  Element::string,  "" }
+};
+@endcode
+
+This array (which technically is implemented as a vector and
+initialized the C++11 way) can be passed to the aforementioned
+setDefaults. That code will iterate over all default values and see if
+there are explicit values provided. If not, the gaps will be filled
+with default values. There are also convenience methods specified for
+filling in option data defaults, option definition defaults and
+setAllDefaults that sets all defaults (starts with global, but then
+walks down the Element tree and fills defaults in subsequent scopes).
+
+@subsection ccSimpleParserInherits Inheriting parameters between scopes
+
+SimpleParser provides a mechanism to inherit parameters between scopes,
+e.g. to inherit global parameters in the subnet scope if more specific
+values are not defined in the subnet scope. This is achieved by calling
+@code
+static size_t SimpleParser::deriveParams(isc::data::ConstElementPtr parent,
+                                         isc::data::ElementPtr child,
+                                         const ParamsList& params);
+
+@endcode
+
+ParamsList is a simple vector<string>. There will be more specific
+methods implemented in the future, but for the time being only
+@ref isc::data::SimpleParser::deriveParams is implemented.
+
+*/

src/lib/cc/data.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2010-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2010-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -86,7 +86,7 @@ Element::getValue(std::string&) const {
 }
 
 bool
-Element::getValue(std::vector<ConstElementPtr>&) const {
+Element::getValue(std::vector<ElementPtr>&) const {
     return (false);
 }
 
@@ -116,7 +116,7 @@ Element::setValue(const std::string&) {
 }
 
 bool
-Element::setValue(const std::vector<ConstElementPtr>&) {
+Element::setValue(const std::vector<ElementPtr>&) {
     return (false);
 }
 
@@ -130,13 +130,18 @@ Element::get(const int) const {
     throwTypeError("get(int) called on a non-list Element");
 }
 
+ElementPtr
+Element::getNonConst(const int) const {
+    throwTypeError("get(int) called on a non-list Element");
+}
+
 void
-Element::set(const size_t, ConstElementPtr) {
+Element::set(const size_t, ElementPtr) {
     throwTypeError("set(int, element) called on a non-list Element");
 }
 
 void
-Element::add(ConstElementPtr) {
+Element::add(ElementPtr) {
     throwTypeError("add() called on a non-list Element");
 }
 
@@ -507,7 +512,7 @@ fromStringstreamList(std::istream& in, const std::string& file, int& line,
 {
     int c = 0;
     ElementPtr list = Element::createList(Element::Position(file, line, pos));
-    ConstElementPtr cur_list_element;
+    ElementPtr cur_list_element;
 
     skipChars(in, WHITESPACE, line, pos);
     while (c != EOF && c != ']') {
@@ -810,8 +815,8 @@ void
 ListElement::toJSON(std::ostream& ss) const {
     ss << "[ ";
 
-    const std::vector<ConstElementPtr>& v = listValue();
-    for (std::vector<ConstElementPtr>::const_iterator it = v.begin();
+    const std::vector<ElementPtr>& v = listValue();
+    for (std::vector<ElementPtr>::const_iterator it = v.begin();
          it != v.end(); ++it) {
         if (it != v.begin()) {
             ss << ", ";
@@ -1079,5 +1084,14 @@ void Element::preprocess(std::istream& in, std::stringstream& out) {
     }
 }
 
+ElementPtr Element::getMutableMap(ConstElementPtr& const_map) {
+    std::map<std::string, ConstElementPtr> values;
+    const_map->getValue(values);
+    ElementPtr mutable_map(new MapElement());
+    mutable_map->setValue(values);
+
+    return (mutable_map);
+}
+
 }
 }

src/lib/cc/data.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2010-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2010-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -216,7 +216,7 @@ public:
     { throwTypeError("boolValue() called on non-Bool Element"); };
     virtual std::string stringValue() const
     { throwTypeError("stringValue() called on non-string Element"); };
-    virtual const std::vector<ConstElementPtr>& listValue() const {
+    virtual const std::vector<ElementPtr>& listValue() const {
         // replace with real exception or empty vector?
         throwTypeError("listValue() called on non-list Element");
     };
@@ -239,7 +239,7 @@ public:
     virtual bool getValue(double& t) const;
     virtual bool getValue(bool& t) const;
     virtual bool getValue(std::string& t) const;
-    virtual bool getValue(std::vector<ConstElementPtr>& t) const;
+    virtual bool getValue(std::vector<ElementPtr>& t) const;
     virtual bool getValue(std::map<std::string, ConstElementPtr>& t) const;
     //@}
 
@@ -259,7 +259,7 @@ public:
     virtual bool setValue(const double v);
     virtual bool setValue(const bool t);
     virtual bool setValue(const std::string& v);
-    virtual bool setValue(const std::vector<ConstElementPtr>& v);
+    virtual bool setValue(const std::vector<ElementPtr>& v);
     virtual bool setValue(const std::map<std::string, ConstElementPtr>& v);
     //@}
 
@@ -277,15 +277,21 @@ public:
     /// \param i The position of the ElementPtr to return
     virtual ConstElementPtr get(const int i) const;
 
+    /// \brief returns element as non-const pointer
+    ///
+    /// \param i The position of the ElementPtr to retrieve
+    /// \return specified element pointer
+    virtual ElementPtr getNonConst(const int i) const;
+
     /// Sets the ElementPtr at the given index. If the index is out
     /// of bounds, this function throws an std::out_of_range exception.
     /// \param i The position of the ElementPtr to set
     /// \param element The ElementPtr to set at the position
-    virtual void set(const size_t i, ConstElementPtr element);
+    virtual void set(const size_t i, ElementPtr element);
 
     /// Adds an ElementPtr to the list
     /// \param element The ElementPtr to add
-    virtual void add(ConstElementPtr element);
+    virtual void add(ElementPtr element);
 
     /// Removes the element at the given position. If the index is out
     /// of nothing happens.
@@ -521,6 +527,12 @@ public:
     /// \return ElementPtr with the data that is parsed.
     static ElementPtr fromWire(const std::string& s);
     //@}
+
+    /// @brief Creates mutable map based on const map
+    ///
+    /// @param const_map const map to be used as a donor
+    /// @return mutable map
+    static ElementPtr getMutableMap(ConstElementPtr& const_map);
 };
 
 /// Notes: IntElement type is changed to int64_t.
@@ -603,29 +615,30 @@ public:
 };
 
 class ListElement : public Element {
-    std::vector<ConstElementPtr> l;
+    std::vector<ElementPtr> l;
 
 public:
     ListElement(const Position& pos = ZERO_POSITION())
         : Element(list, pos) {}
-    const std::vector<ConstElementPtr>& listValue() const { return (l); }
+    const std::vector<ElementPtr>& listValue() const { return (l); }
     using Element::getValue;
-    bool getValue(std::vector<ConstElementPtr>& t) const {
+    bool getValue(std::vector<ElementPtr>& t) const {
         t = l;
         return (true);
     }
     using Element::setValue;
-    bool setValue(const std::vector<ConstElementPtr>& v) {
+    bool setValue(const std::vector<ElementPtr>& v) {
         l = v;
         return (true);
     }
     using Element::get;
     ConstElementPtr get(int i) const { return (l.at(i)); }
+    ElementPtr getNonConst(int i) const  { return (l.at(i)); }
     using Element::set;
-    void set(size_t i, ConstElementPtr e) {
+    void set(size_t i, ElementPtr e) {
         l.at(i) = e;
     }
-    void add(ConstElementPtr e) { l.push_back(e); };
+    void add(ElementPtr e) { l.push_back(e); };
     using Element::remove;
     void remove(int i) { l.erase(l.begin() + i); };
     void toJSON(std::ostream& ss) const;

src/lib/cc/simple_parser.cc

@@ -0,0 +1,175 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <cc/simple_parser.h>
+#include <boost/foreach.hpp>
+#include <boost/lexical_cast.hpp>
+#include <cc/data.h>
+#include <string>
+
+using namespace std;
+
+namespace isc {
+namespace data {
+
+std::string
+SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) {
+    ConstElementPtr x = scope->get(name);
+    if (!x) {
+        isc_throw(BadValue, "Element " << name << " not found");
+    }
+    if (x->getType() != Element::string) {
+        isc_throw(BadValue, "Element " << name << " found, but is not a string");
+    }
+
+    return (x->stringValue());
+}
+
+int64_t
+SimpleParser::getInteger(isc::data::ConstElementPtr scope, const std::string& name) {
+    ConstElementPtr x = scope->get(name);
+    if (!x) {
+        isc_throw(BadValue, "Element " << name << " not found");
+    }
+    if (x->getType() != Element::integer) {
+        isc_throw(BadValue, "Element " << name << " found, but is not an integer");
+    }
+
+    return (x->intValue());
+}
+
+bool
+SimpleParser::getBoolean(isc::data::ConstElementPtr scope, const std::string& name) {
+    ConstElementPtr x = scope->get(name);
+    if (!x) {
+        isc_throw(BadValue, "Element " << name << " not found");
+    }
+    if (x->getType() != Element::boolean) {
+        isc_throw(BadValue, "Element " << name << " found, but is not a boolean");
+    }
+
+    return (x->boolValue());
+}
+
+const data::Element::Position&
+SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr parent) {
+    if (!parent) {
+        return (data::Element::ZERO_POSITION());
+    }
+    ConstElementPtr elem = parent->get(name);
+    if (!elem) {
+        return (data::Element::ZERO_POSITION());
+    }
+    return (elem->getPosition());
+}
+
+size_t SimpleParser::setDefaults(isc::data::ElementPtr scope,
+                                 const SimpleDefaults& default_values) {
+    size_t cnt = 0;
+
+    // This is the position representing a default value. As the values
+    // we're inserting here are not present in whatever the config file
+    // came from, we need to make sure it's clearly labeled as default.
+    const Element::Position pos("<default-value>", 0, 0);
+
+    // Let's go over all parameters we have defaults for.
+    BOOST_FOREACH(SimpleDefault def_value, default_values) {
+
+        // Try if such a parameter is there. If it is, let's
+        // skip it, because user knows best *cough*.
+        ConstElementPtr x = scope->get(string(def_value.name_));
+        if (x) {
+            // There is such a value already, skip it.
+            continue;
+        }
+
+        // There isn't such a value defined, let's create the default
+        // value...
+        switch (def_value.type_) {
+        case Element::string: {
+            x.reset(new StringElement(def_value.value_, pos));
+            break;
+        }
+        case Element::integer: {
+            int int_value = boost::lexical_cast<int>(def_value.value_);
+            x.reset(new IntElement(int_value, pos));
+            break;
+        }
+        case Element::boolean: {
+            bool bool_value;
+            if (def_value.value_ == string("true")) {
+                bool_value = true;
+            } else if (def_value.value_ == string("false")) {
+                bool_value = false;
+            } else {
+                isc_throw(BadValue, "Internal error. Boolean value specified as "
+                          << def_value.value_ << ", expected true or false");
+            }
+            x.reset(new BoolElement(bool_value, pos));
+            break;
+        }
+        case Element::real: {
+            double dbl_value = boost::lexical_cast<double>(def_value.value_);
+            x.reset(new DoubleElement(dbl_value, pos));
+            break;
+        }
+        default:
+            // No default values for null, list or map
+            isc_throw(BadValue, "Internal error. Incorrect default value type.");
+        }
+
+        // ... and insert it into the provided Element tree.
+        scope->set(def_value.name_, x);
+        cnt++;
+    }
+
+    return (cnt);
+}
+
+size_t
+SimpleParser::setListDefaults(isc::data::ElementPtr list,
+                              const SimpleDefaults& default_values) {
+    size_t cnt = 0;
+    BOOST_FOREACH(ElementPtr entry, list->listValue()) {
+        cnt += setDefaults(entry, default_values);
+    }
+    return (cnt);
+}
+
+size_t
+SimpleParser::deriveParams(isc::data::ConstElementPtr parent,
+                           isc::data::ElementPtr child,
+                           const ParamsList& params) {
+    if ( (parent->getType() != Element::map) ||
+         (child->getType() != Element::map)) {
+        return (0);
+    }
+
+    size_t cnt = 0;
+    BOOST_FOREACH(string param, params) {
+        ConstElementPtr x = parent->get(param);
+        if (!x) {
+            // Parent doesn't define this parameter, so there's
+            // nothing to derive from
+            continue;
+        }
+
+        if (child->get(param)) {
+            // Child defines this parameter already. There's
+            // nothing to do here.
+            continue;
+        }
+
+        // Copy the parameters to the child scope.
+        child->set(param, x);
+        cnt++;
+    }
+
+    return (cnt);
+}
+
+}; // end of isc::dhcp namespace
+}; // end of isc namespace

src/lib/cc/simple_parser.h

@@ -0,0 +1,149 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef SIMPLE_PARSER_H
+#define SIMPLE_PARSER_H
+
+#include <cc/data.h>
+#include <string>
+#include <stdint.h>
+
+namespace isc {
+namespace data {
+
+/// This array defines a single entry of default values
+struct SimpleDefault {
+    SimpleDefault(const char* name, isc::data::Element::types type, const char* value)
+        :name_(name), type_(type), value_(value) {}
+    std::string name_;
+    const isc::data::Element::types type_;
+    const char* value_;
+};
+
+/// This specifies all default values in a given scope (e.g. a subnet)
+typedef std::vector<SimpleDefault> SimpleDefaults;
+
+/// This defines a list of all parameters that are derived (or inherited) between
+/// contexts
+typedef std::vector<std::string> ParamsList;
+
+
+/// @brief A simple parser
+///
+/// This class is intended to be a simpler replacement for
+/// @ref isc::dhcp::DhcpConfigParser.
+/// The simplification comes from several factors:
+/// - no build/commit nonsense. There's a single step:
+///   CfgStorage parse(ConstElementPtr json)
+///   that converts JSON configuration into an object and returns it.
+/// - almost no state kept. The only state kept in most cases is whether the
+///   parsing is done in v4 or v6 context. This greatly simplifies the
+///   parsers (no contexts, no child parsers list, no separate storage for
+///   uint32, strings etc. In fact, there's so little state kept, that this
+///   parser is mostly a collection of static methods.
+/// - no optional parameters (all are mandatory). This simplifies the parser,
+///   but introduces a new step before parsing where we insert the default
+///   values into client configuration before parsing. This is actually a good
+///   thing, because we now have a clear picture of the default parameters as
+///   they're defined in a single place (the DhcpConfigParser had the defaults
+///   spread out in multiple files in multiple directories).
+class SimpleParser {
+ public:
+
+    /// @brief Derives (inherits) parameters from parent scope to a child
+    ///
+    /// This method derives parameters from the parent scope to the child,
+    /// if there are no values specified in the child scope. For example,
+    /// this method can be used to derive timers from global scope (e.g. for
+    /// the whole DHCPv6 server) to a subnet scope. This method checks
+    /// if the child scope doesn't have more specific values defined. If
+    /// it doesn't, then the value from parent scope is copied over.
+    ///
+    /// @param parent scope to copy from (e.g. global)
+    /// @param child scope to copy from (e.g. subnet)
+    /// @param params names of the parameters to copy
+    /// @return number of parameters copied
+    static size_t deriveParams(isc::data::ConstElementPtr parent,
+                               isc::data::ElementPtr child,
+                               const ParamsList& params);
+
+    /// @brief Sets the default values
+    ///
+    /// This method sets the default values for parameters that are not
+    /// defined. The list of default values is specified by default_values.
+    /// If not present, those will be inserted into the scope. If
+    /// a parameter is already present, the default value will not
+    /// be inserted.
+    ///
+    /// @param scope default values will be inserted here
+    /// @param default_values list of default values
+    /// @return number of parameters inserted
+    static size_t setDefaults(isc::data::ElementPtr scope,
+                              const SimpleDefaults& default_values);
+
+    /// @brief Sets the default values for all entries in a list
+    ///
+    /// This is a simple utility method that iterates over all
+    /// parameters in a list and calls setDefaults for each
+    /// entry.
+    ///
+    /// @param list list to be iterated over
+    /// @param default_values list of default values
+    /// @return number of parameters inserted
+    static size_t setListDefaults(isc::data::ElementPtr list,
+                                  const SimpleDefaults& default_values);
+
+    /// @brief Utility method that returns position of an element
+    ///
+    /// It's mostly useful for logging.
+    ///
+    /// @param name position of that element will be returned
+    /// @param parent parent element (optional)
+    /// @return position of the element specified.
+    static const data::Element::Position&
+    getPosition(const std::string& name, const data::ConstElementPtr parent =
+                data::ConstElementPtr());
+
+ protected:
+
+    /// @brief Returns a string parameter from a scope
+    ///
+    /// Unconditionally returns a parameter. If the parameter is not there or
+    /// is not of appropriate type, BadValue exception is thrown.
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return a string value of the parameter
+    static std::string getString(isc::data::ConstElementPtr scope,
+                                 const std::string& name);
+
+    /// @brief Returns an integer parameter from a scope
+    ///
+    /// Unconditionally returns a parameter. If the parameter is not there or
+    /// is not of appropriate type, BadValue exception is thrown.
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an integer value of the parameter
+    static int64_t getInteger(isc::data::ConstElementPtr scope,
+                              const std::string& name);
+
+    /// @brief Returns a boolean parameter from a scope
+    ///
+    /// Unconditionally returns a parameter. If the parameter is not there or
+    /// is not of appropriate type, BadValue exception is thrown.
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return a boolean value of the parameter
+    static bool getBoolean(isc::data::ConstElementPtr scope,
+                           const std::string& name);
+};
+
+};
+};
+
+#endif

src/lib/cc/tests/Makefile.am

@@ -16,6 +16,7 @@ if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES = command_interpreter_unittests.cc data_unittests.cc
 run_unittests_SOURCES += data_file_unittests.cc run_unittests.cc
+run_unittests_SOURCES += simple_parser_unittest.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 

src/lib/cc/tests/data_unittests.cc

@@ -206,7 +206,7 @@ testGetValueInt() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::create(1);
@@ -266,7 +266,7 @@ testGetValueDouble() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::create(1.1);
@@ -293,7 +293,7 @@ testGetValueBool() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::create(true);
@@ -320,7 +320,7 @@ testGetValueString() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::create("foo");
@@ -347,7 +347,7 @@ testGetValueList() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::createList();
@@ -374,7 +374,7 @@ testGetValueMap() {
     double d;
     bool b;
     std::string s;
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
 
     el = Element::createMap();
@@ -402,7 +402,7 @@ TEST(Element, create_and_value_throws) {
     double d = 0.0;
     bool b = false;
     std::string s("asdf");
-    std::vector<ConstElementPtr> v;
+    std::vector<ElementPtr> v;
     std::map<std::string, ConstElementPtr> m;
     ConstElementPtr tmp;
 

src/lib/cc/tests/simple_parser_unittest.cc

@@ -0,0 +1,132 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <cc/simple_parser.h>
+#include <gtest/gtest.h>
+
+using namespace isc::data;
+
+/// This table defines sample default values. Although these are DHCPv6
+/// specific, the mechanism is generic and can be used by any other component.
+const SimpleDefaults SAMPLE_DEFAULTS = {
+    { "renew-timer",        Element::integer, "900" },
+    { "rebind-timer",       Element::integer, "1800" },
+    { "preferred-lifetime", Element::integer, "3600" },
+    { "valid-lifetime",     Element::integer, "7200" }
+};
+
+/// This list defines parameters that can be inherited from one scope
+/// to another. Although these are DHCPv6 specific, the mechanism is generic and
+/// can be used by any other component.
+const ParamsList SAMPLE_INHERITS = {
+    "renew-timer",
+    "rebind-timer",
+    "preferred-lifetime",
+    "valid-lifetime"
+};
+
+/// @brief Simple Parser test fixture class
+class SimpleParserTest : public ::testing::Test {
+public:
+    /// @brief Checks if specified map has an integer parameter with expected value
+    ///
+    /// @param map map to be checked
+    /// @param param_name name of the parameter to be checked
+    /// @param exp_value expected value of the parameter.
+    void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name,
+                           int64_t exp_value) {
+
+        // First check if the passed element is a map.
+        ASSERT_EQ(Element::map, map->getType());
+
+        // Now try to get the element being checked
+        ConstElementPtr elem = map->get(param_name);
+        ASSERT_TRUE(elem);
+
+        // Now check if it's indeed integer
+        ASSERT_EQ(Element::integer, elem->getType());
+
+        // Finally, check if its value meets expectation.
+        EXPECT_EQ(exp_value, elem->intValue());
+    }
+};
+
+// This test checks if the parameters can be inherited from the global
+// scope to the subnet scope.
+TEST_F(SimpleParserTest, deriveParams) {
+    ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1,"
+                                          "  \"rebind-timer\": 2,"
+                                          "  \"preferred-lifetime\": 3,"
+                                          "  \"valid-lifetime\": 4"
+                                          "}");
+    ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }");
+
+    // we should inherit 3 parameters. Renew-timer should remain intact,
+    // as it was already defined in the subnet scope.
+    size_t num;
+    EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet,
+                                                     SAMPLE_INHERITS));
+    EXPECT_EQ(3, num);
+
+    // Check the values. 3 of them are inherited, while the fourth one
+    // was already defined in the subnet, so should not be inherited.
+    checkIntegerValue(subnet, "renew-timer", 100);
+    checkIntegerValue(subnet, "rebind-timer", 2);
+    checkIntegerValue(subnet, "preferred-lifetime", 3);
+    checkIntegerValue(subnet, "valid-lifetime", 4);
+}
+
+// This test checks if global defaults are properly set for DHCPv6.
+TEST_F(SimpleParserTest, setDefaults) {
+
+    ElementPtr empty = Element::fromJSON("{ }");
+    size_t num = 0;
+
+    EXPECT_NO_THROW(num = SimpleParser::setDefaults(empty, SAMPLE_DEFAULTS));
+
+    // We expect at least 4 parameters to be inserted.
+    EXPECT_GE(num, 3);
+
+    checkIntegerValue(empty, "valid-lifetime", 7200);
+    checkIntegerValue(empty, "preferred-lifetime", 3600);
+    checkIntegerValue(empty, "rebind-timer", 1800);
+    checkIntegerValue(empty, "renew-timer", 900);
+}
+
+// This test checks if global defaults are properly set for DHCPv6.
+TEST_F(SimpleParserTest, setListDefaults) {
+
+    ElementPtr empty = Element::fromJSON("[{}, {}, {}]");
+    size_t num;
+
+    EXPECT_NO_THROW(num = SimpleParser::setListDefaults(empty, SAMPLE_DEFAULTS));
+
+    // We expect at least 12 parameters to be inserted (3 entries, with
+    // 4 parameters inserted in each)
+    EXPECT_EQ(12, num);
+
+    ASSERT_EQ(Element::list, empty->getType());
+    ASSERT_EQ(3, empty->size());
+
+    ConstElementPtr first = empty->get(0);
+    ConstElementPtr second = empty->get(1);
+    ConstElementPtr third = empty->get(2);
+
+    checkIntegerValue(first, "valid-lifetime", 7200);
+    checkIntegerValue(first, "preferred-lifetime", 3600);
+    checkIntegerValue(first, "rebind-timer", 1800);
+    checkIntegerValue(first, "renew-timer", 900);
+
+    checkIntegerValue(second, "valid-lifetime", 7200);
+    checkIntegerValue(second, "preferred-lifetime", 3600);
+    checkIntegerValue(second, "rebind-timer", 1800);
+    checkIntegerValue(second, "renew-timer", 900);
+
+    checkIntegerValue(third, "valid-lifetime", 7200);
+    checkIntegerValue(third, "preferred-lifetime", 3600);
+    checkIntegerValue(third, "rebind-timer", 1800);
+    checkIntegerValue(third, "renew-timer", 900);
+}

src/lib/dhcpsrv/Makefile.am

@@ -20,8 +20,6 @@ if HAVE_CQL
 AM_CPPFLAGS += $(CQL_CPPFLAGS)
 endif
 
-AM_CXXFLAGS = $(KEA_CXXFLAGS)
-
 # The files in the subfolder must be explicitly specified here so
 # as they are copied to the distribution. The other option would
 # be to specify a whole 'parsers' folder here but that would also

src/lib/dhcpsrv/libdhcpsrv.dox

@@ -476,4 +476,5 @@ is used by DHCPv4 and DHCPv6 components.
 
 DHCPv4-over-DHCPv6 which are relayed by a DHCPv6 relay are not yet supported.
 
+
 */

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -91,12 +91,16 @@ ClientClassDefParser::build(ConstElementPtr class_def_cfg) {
                                                                 global_context_));
             parser = exp_parser;
         } else if (entry == "option-data") {
-            OptionDataListParserPtr opts_parser;
+
             uint16_t family = (global_context_->universe_ == Option::V4 ?
                                                              AF_INET : AF_INET6);
 
-            opts_parser.reset(new OptionDataListParser(entry, options_, family));
-            parser = opts_parser;
+            OptionDataListParser opts_parser(family);
+            opts_parser.parse(options_, param.second);
+
+            // OptionDataListParser is converted to SimpleParser already,
+            // no need to go through build/commit phases.
+            continue;
         } else if (entry == "next-server") {
             StringParserPtr str_parser(new StringParser(entry, string_values_));
             parser = str_parser;

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -376,75 +376,30 @@ HooksLibrariesParser::getLibraries(isc::hooks::HookLibsCollection& libraries,
 }
 
 // **************************** OptionDataParser *************************
-OptionDataParser::OptionDataParser(const std::string&, const CfgOptionPtr& cfg,
-                                   const uint16_t address_family)
-    : boolean_values_(new BooleanStorage()),
-      string_values_(new StringStorage()), uint32_values_(new Uint32Storage()),
-      option_descriptor_(false), cfg_(cfg),
-      address_family_(address_family) {
-    // If configuration not specified, then it is a global configuration
-    // scope.
-    if (!cfg_) {
-        cfg_ = CfgMgr::instance().getStagingCfg()->getCfgOption();
-    }
+OptionDataParser::OptionDataParser(const uint16_t address_family)
+    : address_family_(address_family) {
 }
 
-void
-OptionDataParser::build(ConstElementPtr option_data_entries) {
-    BOOST_FOREACH(ConfigPair param, option_data_entries->mapValue()) {
-        ParserPtr parser;
-        if (param.first == "name" || param.first == "data" ||
-            param.first == "space") {
-            StringParserPtr name_parser(new StringParser(param.first,
-                                        string_values_));
-            parser = name_parser;
-        } else if (param.first == "code") {
-            Uint32ParserPtr code_parser(new Uint32Parser(param.first,
-                                       uint32_values_));
-            parser = code_parser;
-        } else if (param.first == "csv-format") {
-            BooleanParserPtr value_parser(new BooleanParser(param.first,
-                                         boolean_values_));
-            parser = value_parser;
-        } else {
-            isc_throw(DhcpConfigError,
-                      "option-data parameter not supported: " << param.first
-                      << " (" << param.second->getPosition() << ")");
-        }
-
-        parser->build(param.second);
-        // Before we can create an option we need to get the data from
-        // the child parsers. The only way to do it is to invoke commit
-        // on them so as they store the values in appropriate storages
-        // that this class provided to them. Note that this will not
-        // modify values stored in the global storages so the configuration
-        // will remain consistent even parsing fails somewhere further on.
-        parser->commit();
-    }
+std::pair<OptionDescriptor, std::string>
+OptionDataParser::parse(isc::data::ConstElementPtr single_option) {
 
     // Try to create the option instance.
-    createOption(option_data_entries);
+    std::pair<OptionDescriptor, std::string> opt = createOption(single_option);
 
-    if (!option_descriptor_.option_) {
+    if (!opt.first.option_) {
         isc_throw(isc::InvalidOperation,
             "parser logic error: no option has been configured and"
             " thus there is nothing to commit. Has build() been called?");
     }
 
-    cfg_->add(option_descriptor_.option_, option_descriptor_.persistent_,
-              option_space_);
-}
-
-void
-OptionDataParser::commit() {
-    // Does nothing
+    return (opt);
 }
 
 OptionalValue<uint32_t>
 OptionDataParser::extractCode(ConstElementPtr parent) const {
     uint32_t code;
     try {
-        code = uint32_values_->getParam("code");
+        code = getInteger(parent, "code");
 
     } catch (const exception&) {
         // The code parameter was not found. Return an unspecified
@@ -454,14 +409,14 @@ OptionDataParser::extractCode(ConstElementPtr parent) const {
 
     if (code == 0) {
         isc_throw(DhcpConfigError, "option code must not be zero "
-                  "(" << uint32_values_->getPosition("code", parent) << ")");
+                  "(" << getPosition("code", parent) << ")");
 
     } else if (address_family_ == AF_INET &&
                code > std::numeric_limits<uint8_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
                 << "', it must not be greater than '"
                   << static_cast<int>(std::numeric_limits<uint8_t>::max())
-                  << "' (" << uint32_values_->getPosition("code", parent)
+                  << "' (" << getPosition("code", parent)
                   << ")");
 
     } else if (address_family_ == AF_INET6 &&
@@ -469,7 +424,7 @@ OptionDataParser::extractCode(ConstElementPtr parent) const {
         isc_throw(DhcpConfigError, "invalid option code '" << code
                 << "', it must not exceed '"
                   << std::numeric_limits<uint16_t>::max()
-                  << "' (" << uint32_values_->getPosition("code", parent)
+                  << "' (" << getPosition("code", parent)
                   << ")");
 
     }
@@ -481,7 +436,7 @@ OptionalValue<std::string>
 OptionDataParser::extractName(ConstElementPtr parent) const {
     std::string name;
     try {
-        name = string_values_->getParam("name");
+        name = getString(parent, "name");
 
     } catch (...) {
         return (OptionalValue<std::string>());
@@ -490,17 +445,17 @@ OptionDataParser::extractName(ConstElementPtr parent) const {
     if (name.find(" ") != std::string::npos) {
         isc_throw(DhcpConfigError, "invalid option name '" << name
                   << "', space character is not allowed ("
-                  << string_values_->getPosition("name", parent) << ")");
+                  << getPosition("name", parent) << ")");
     }
 
     return (OptionalValue<std::string>(name, OptionalValueState(true)));
 }
 
 std::string
-OptionDataParser::extractData() const {
+OptionDataParser::extractData(ConstElementPtr parent) const {
     std::string data;
     try {
-        data = string_values_->getParam("data");
+        data = getString(parent, "data");
 
     } catch (...) {
         // The "data" parameter was not found. Return an empty value.
@@ -511,10 +466,10 @@ OptionDataParser::extractData() const {
 }
 
 OptionalValue<bool>
-OptionDataParser::extractCSVFormat() const {
+OptionDataParser::extractCSVFormat(ConstElementPtr parent) const {
     bool csv_format = true;
     try {
-        csv_format = boolean_values_->getParam("csv-format");
+        csv_format = getBoolean(parent, "csv-format");
 
     } catch (...) {
         return (OptionalValue<bool>(csv_format));
@@ -524,11 +479,11 @@ OptionDataParser::extractCSVFormat() const {
 }
 
 std::string
-OptionDataParser::extractSpace() const {
+OptionDataParser::extractSpace(ConstElementPtr parent) const {
     std::string space = address_family_ == AF_INET ?
         DHCP4_OPTION_SPACE : DHCP6_OPTION_SPACE;
     try {
-        space = string_values_->getParam("space");
+        space = getString(parent, "space");
 
     } catch (...) {
         return (space);
@@ -556,7 +511,7 @@ OptionDataParser::extractSpace() const {
         // should never get here. Therefore, it is ok to call getPosition for
         // the space parameter here as this parameter will always be specified.
         isc_throw(DhcpConfigError, ex.what() << " ("
-                  << string_values_->getPosition("space") << ")");
+                  << getPosition("space", parent) << ")");
     }
 
     return (space);
@@ -588,16 +543,16 @@ OptionDataParser::findOptionDefinition(const std::string& option_space,
     return (def);
 }
 
-void
+std::pair<OptionDescriptor, std::string>
 OptionDataParser::createOption(ConstElementPtr option_data) {
     const Option::Universe universe = address_family_ == AF_INET ?
         Option::V4 : Option::V6;
 
     OptionalValue<uint32_t> code_param =  extractCode(option_data);
     OptionalValue<std::string> name_param = extractName(option_data);
-    OptionalValue<bool> csv_format_param = extractCSVFormat();
-    std::string data_param = extractData();
-    std::string space_param = extractSpace();
+    OptionalValue<bool> csv_format_param = extractCSVFormat(option_data);
+    std::string data_param = extractData(option_data);
+    std::string space_param = extractSpace(option_data);
 
     // Require that option code or option name is specified.
     if (!code_param.isSpecified() && !name_param.isSpecified()) {
@@ -622,7 +577,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                       << space_param << "." << name_param
                       << "' having code '" << code_param
                       << "' does not exist ("
-                      << string_values_->getPosition("name", option_data)
+                      << getPosition("name", option_data)
                       << ")");
 
         // If there is no option definition and the option code is not specified
@@ -631,7 +586,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
             isc_throw(DhcpConfigError, "definition for the option '"
                       << space_param << "." << name_param
                       << "' does not exist ("
-                      << string_values_->getPosition("name", option_data)
+                      << getPosition("name", option_data)
                       << ")");
         }
     }
@@ -664,12 +619,14 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
             isc_throw(DhcpConfigError, "option data is not a valid"
                       << " string of hexadecimal digits: " << data_param
                       << " ("
-                      << string_values_->getPosition("data", option_data)
+                      << getPosition("data", option_data)
                       << ")");
         }
     }
 
     OptionPtr option;
+    OptionDescriptor desc(false);
+
     if (!def) {
         // @todo We have a limited set of option definitions initalized at
         // the moment.  In the future we want to initialize option definitions
@@ -678,13 +635,9 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         // ok to create generic option if definition does not exist.
         OptionPtr option(new Option(universe, static_cast<uint16_t>(code_param),
                                     binary));
-        // The created option is stored in option_descriptor_ class member
-        // until the commit stage when it is inserted into the main storage.
-        // If an option with the same code exists in main storage already the
-        // old option is replaced.
-        option_descriptor_.option_ = option;
-        option_descriptor_.persistent_ = false;
 
+        desc.option_ = option;
+        desc.persistent_ = false;
     } else {
 
         // Option name is specified it should match the name in the definition.
@@ -693,7 +646,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                       << name_param << "' does not match the "
                       << "option definition: '" << space_param
                       << "." << def->getName() << "' ("
-                      << string_values_->getPosition("name", option_data)
+                      << getPosition("name", option_data)
                       << ")");
         }
 
@@ -704,144 +657,62 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                 !csv_format_param.isSpecified() || csv_format_param ?
                 def->optionFactory(universe, def->getCode(), data_tokens) :
                 def->optionFactory(universe, def->getCode(), binary);
-            OptionDescriptor desc(option, false);
-            option_descriptor_.option_ = option;
-            option_descriptor_.persistent_ = false;
-
+            desc.option_ = option;
+            desc.persistent_ = false;
         } catch (const isc::Exception& ex) {
             isc_throw(DhcpConfigError, "option data does not match"
                       << " option definition (space: " << space_param
                       << ", code: " << def->getCode() << "): "
                       << ex.what() << " ("
-                      << string_values_->getPosition("data", option_data)
+                      << getPosition("data", option_data)
                       << ")");
         }
     }
 
     // All went good, so we can set the option space name.
-    option_space_ = space_param;
+    return make_pair(desc, space_param);
 }
 
 // **************************** OptionDataListParser *************************
-OptionDataListParser::OptionDataListParser(const std::string&,
-                                           const CfgOptionPtr& cfg,
+OptionDataListParser::OptionDataListParser(//const std::string&,
+                                           //const CfgOptionPtr& cfg,
                                            const uint16_t address_family)
-    : cfg_(cfg), address_family_(address_family) {
+    : address_family_(address_family) {
 }
 
-void
-OptionDataListParser::build(ConstElementPtr option_data_list) {
-    BOOST_FOREACH(ConstElementPtr option_value, option_data_list->listValue()) {
-        boost::shared_ptr<OptionDataParser>
-            parser(new OptionDataParser("option-data", cfg_, address_family_));
 
-        parser->build(option_value);
-        parsers_.push_back(parser);
-    }
-}
-
-void
-OptionDataListParser::commit() {
-    BOOST_FOREACH(ParserPtr parser, parsers_) {
-        parser->commit();
-    }
-    // Append suboptions to the top-level options
-    if (cfg_) {
-        cfg_->encapsulate();
-    } else {
-        CfgMgr::instance().getStagingCfg()->getCfgOption()->encapsulate();
+void OptionDataListParser::parse(const CfgOptionPtr& cfg,
+                                 isc::data::ConstElementPtr option_data_list) {
+    OptionDataParser option_parser(address_family_);
+    BOOST_FOREACH(ConstElementPtr data, option_data_list->listValue()) {
+        std::pair<OptionDescriptor, std::string> option =
+            option_parser.parse(data);
+        cfg->add(option.first.option_, option.first.persistent_, option.second);
+        cfg->encapsulate();
     }
 }
 
 // ******************************** OptionDefParser ****************************
-OptionDefParser::OptionDefParser(const std::string&,
-                                 ParserContextPtr global_context)
-    : boolean_values_(new BooleanStorage()),
-      string_values_(new StringStorage()),
-      uint32_values_(new Uint32Storage()),
-      global_context_(global_context) {
-}
-
-void
-OptionDefParser::build(ConstElementPtr option_def) {
-    // Parse the elements that make up the option definition.
-    BOOST_FOREACH(ConfigPair param, option_def->mapValue()) {
-        std::string entry(param.first);
-        ParserPtr parser;
-        if (entry == "name" || entry == "type" || entry == "record-types"
-            || entry == "space" || entry == "encapsulate") {
-            StringParserPtr str_parser(new StringParser(entry,
-                                       string_values_));
-            parser = str_parser;
-        } else if (entry == "code") {
-            Uint32ParserPtr code_parser(new Uint32Parser(entry,
-                                        uint32_values_));
-            parser = code_parser;
-        } else if (entry == "array") {
-            BooleanParserPtr array_parser(new BooleanParser(entry,
-                                         boolean_values_));
-            parser = array_parser;
-        } else {
-            isc_throw(DhcpConfigError, "invalid parameter '" << entry
-                      << "' (" << param.second->getPosition() << ")");
-        }
-
-        parser->build(param.second);
-        parser->commit();
-    }
-    // Create an instance of option definition.
-    createOptionDef(option_def);
 
-    try {
-        CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->
-            add(option_definition_, option_space_name_);
+std::pair<isc::dhcp::OptionDefinitionPtr, std::string>
+OptionDefParser::parse(ConstElementPtr option_def) {
 
-    } catch (const std::exception& ex) {
-        // Append position if there is a failure.
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << option_def->getPosition() << ")");
-    }
-
-    // All definitions have been prepared. Put them as runtime options into
-    // the libdhcp++.
-    const OptionDefSpaceContainer& container =
-        CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->getContainer();
-    LibDHCP::setRuntimeOptionDefs(container);
-}
+    // Get mandatory parameters.
+    std::string name = getString(option_def, "name");
+    uint32_t code = getInteger(option_def, "code");
+    std::string type = getString(option_def, "type");
 
-void
-OptionDefParser::commit() {
-    // Do nothing.
-}
-
-void
-OptionDefParser::createOptionDef(ConstElementPtr option_def_element) {
-    // Check if mandatory parameters have been specified.
-    std::string name;
-    uint32_t code;
-    std::string type;
-    try {
-        name = string_values_->getParam("name");
-        code = uint32_values_->getParam("code");
-        type = string_values_->getParam("type");
-    } catch (const std::exception& ex) {
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << option_def_element->getPosition() << ")");
-    }
-
-    bool array_type = boolean_values_->getOptionalParam("array", false);
-    std::string record_types =
-        string_values_->getOptionalParam("record-types", "");
-    std::string space = string_values_->getOptionalParam("space",
-              global_context_->universe_ == Option::V4 ? DHCP4_OPTION_SPACE :
-                                                         DHCP6_OPTION_SPACE);
-    std::string encapsulates =
-        string_values_->getOptionalParam("encapsulate", "");
+    // Get optional parameters. Whoever called this parser, should have
+    // called SimpleParser::setDefaults first.
+    bool array_type = getBoolean(option_def, "array");
+    std::string record_types = getString(option_def, "record-types");
+    std::string space = getString(option_def, "space");
+    std::string encapsulates = getString(option_def, "encapsulate");
 
     if (!OptionSpace::validateName(space)) {
         isc_throw(DhcpConfigError, "invalid option space name '"
                   << space << "' ("
-                  << string_values_->getPosition("space") << ")");
+                  << getPosition("space", option_def) << ")");
     }
 
     // Create option definition.
@@ -854,14 +725,14 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) {
             isc_throw(DhcpConfigError, "option '" << space << "."
                       << "name" << "', comprising an array of data"
                       << " fields may not encapsulate any option space ("
-                      << option_def_element->getPosition() << ")");
+                      << option_def->getPosition() << ")");
 
         } else if (encapsulates == space) {
             isc_throw(DhcpConfigError, "option must not encapsulate"
                       << " an option space it belongs to: '"
                       << space << "." << name << "' is set to"
                       << " encapsulate '" << space << "' ("
-                      << option_def_element->getPosition() << ")");
+                      << option_def->getPosition() << ")");
 
         } else {
             def.reset(new OptionDefinition(name, code, type,
@@ -888,7 +759,7 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) {
             isc_throw(DhcpConfigError, "invalid record type values"
                       << " specified for the option definition: "
                       << ex.what() << " ("
-                      << string_values_->getPosition("record-types") << ")");
+                      << getPosition("record-types", option_def) << ")");
         }
     }
 
@@ -897,38 +768,39 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) {
         def->validate();
     } catch (const std::exception& ex) {
         isc_throw(DhcpConfigError, ex.what()
-                  << " (" << option_def_element->getPosition() << ")");
+                  << " (" << option_def->getPosition() << ")");
     }
 
     // Option definition has been created successfully.
-    option_space_name_ = space;
-    option_definition_ = def;
+    return make_pair(def, space);
 }
 
 // ******************************** OptionDefListParser ************************
-OptionDefListParser::OptionDefListParser(const std::string&,
-                                         ParserContextPtr global_context)
-    : global_context_(global_context) {
-}
-
 void
-OptionDefListParser::build(ConstElementPtr option_def_list) {
+OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) {
     if (!option_def_list) {
         isc_throw(DhcpConfigError, "parser error: a pointer to a list of"
                   << " option definitions is NULL ("
                   << option_def_list->getPosition() << ")");
     }
 
+    OptionDefParser parser;
     BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) {
-        boost::shared_ptr<OptionDefParser>
-            parser(new OptionDefParser("single-option-def", global_context_));
-        parser->build(option_def);
+        OptionDefinitionTuple def;
+
+        def = parser.parse(option_def);
+        try {
+            storage->add(def.first, def.second);
+        } catch (const std::exception& ex) {
+            // Append position if there is a failure.
+            isc_throw(DhcpConfigError, ex.what() << " ("
+                      << option_def->getPosition() << ")");
+        }
     }
-}
 
-void
-OptionDefListParser::commit() {
-    // Do nothing.
+    // All definitions have been prepared. Put them as runtime options into
+    // the libdhcp++.
+    LibDHCP::setRuntimeOptionDefs(storage->getContainer());
 }
 
 //****************************** RelayInfoParser ********************************
@@ -1138,13 +1010,9 @@ PoolParser::build(ConstElementPtr pool_structure) {
                           " address pools");
             }
 
-            OptionDataListParserPtr option_parser(new OptionDataListParser("option-data",
-                                                                           options_,
-                                                                           address_family_));
-            option_parser->build(option_data);
-            option_parser->commit();
-            options_->copyTo(*pool->getCfgOption());;
-
+            CfgOptionPtr cfg = pool->getCfgOption();
+            OptionDataListParser option_parser(address_family_);
+            option_parser.parse(cfg, option_data);
         } catch (const std::exception& ex) {
             isc_throw(isc::dhcp::DhcpConfigError, ex.what()
                       << " (" << option_data->getPosition() << ")");
@@ -1194,6 +1062,13 @@ SubnetConfigParser::build(ConstElementPtr subnet) {
             continue;
         }
 
+        if (param.first == "option-data") {
+            uint16_t family = global_context_->universe_ == Option::V4 ? AF_INET : AF_INET6;
+            OptionDataListParser opt_parser(family);
+            opt_parser.parse(options_, param.second);
+            continue;
+        }
+
         ParserPtr parser;
         // When unsupported parameter is specified, the function called
         // below will thrown an exception. We have to catch this exception

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -15,7 +15,9 @@
 #include <dhcpsrv/cfg_iface.h>
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/subnet.h>
+#include <dhcpsrv/cfg_option_def.h>
 #include <dhcpsrv/parsers/dhcp_config_parser.h>
+#include <cc/simple_parser.h>
 #include <hooks/libinfo.h>
 #include <exceptions/exceptions.h>
 #include <util/optional_value.h>
@@ -534,39 +536,30 @@ private:
 /// an option the configuration will not be accepted. If parsing
 /// is successful then an instance of an option is created and
 /// added to the storage provided by the calling class.
-class OptionDataParser : public DhcpConfigParser {
+class OptionDataParser : public isc::data::SimpleParser {
 public:
     /// @brief Constructor.
     ///
-    /// @param dummy first argument is ignored, all Parser constructors
-    /// accept string as first argument.
-    /// @param [out] cfg Pointer to the configuration object where parsed option
-    /// should be stored or NULL if this is a global option.
     /// @param address_family Address family: @c AF_INET or @c AF_INET6.
-    /// @throw isc::dhcp::DhcpConfigError if options or global_context are null.
-    OptionDataParser(const std::string& dummy, const CfgOptionPtr& cfg,
-                     const uint16_t address_family);
+    OptionDataParser(const uint16_t address_family);
 
-    /// @brief Parses the single option data.
+    /// @brief Parses ElementPtr containing option definition
     ///
-    /// This method parses the data of a single option from the configuration.
-    /// The option data includes option name, option code and data being
-    /// carried by this option. Eventually it creates the instance of the
-    /// option and adds it to the Configuration Manager.
+    /// This method parses ElementPtr containing the option defintion,
+    /// instantiates the option for it and then returns a pair
+    /// of option descritor (that holds that new option) and
+    /// a string that specifies the option space.
     ///
-    /// @param option_data_entries collection of entries that define value
-    /// for a particular option.
-    /// @throw DhcpConfigError if invalid parameter specified in
-    /// the configuration.
-    /// @throw isc::InvalidOperation if failed to set storage prior to
-    /// calling build.
-    virtual void build(isc::data::ConstElementPtr option_data_entries);
-
-    /// @brief Does nothing.
-    virtual void commit();
-
-    /// @brief virtual destructor to ensure orderly destruction of derivations.
-    virtual ~OptionDataParser(){};
+    /// Note: ElementPtr is expected to contain all fields. If your
+    /// ElementPtr does not have them, please use
+    /// @ref isc::data::SimpleParser::setDefaults to fill the missing fields
+    /// with default values.
+    ///
+    /// @param single_option ElementPtr containing option defintion
+    /// @return Option object wrapped in option description and an option
+    ///         space
+    std::pair<OptionDescriptor, std::string>
+    parse(isc::data::ConstElementPtr single_option);
 private:
 
     /// @brief Finds an option definition within an option space
@@ -594,22 +587,19 @@ private:
     /// options storage. If the option data parsed by \ref build function
     /// are invalid or insufficient this function emits an exception.
     ///
-    /// @warning this function does not check if options_ storage pointer
-    /// is intitialized but this check is not needed here because it is done
-    /// in the \ref build function.
-    ///
     /// @param option_data An element holding data for a single option being
     /// created.
     ///
+    /// @return created option descriptor
+    ///
     /// @throw DhcpConfigError if parameters provided in the configuration
     /// are invalid.
-    void createOption(isc::data::ConstElementPtr option_data);
+    std::pair<OptionDescriptor, std::string>
+    createOption(isc::data::ConstElementPtr option_data);
 
     /// @brief Retrieves parsed option code as an optional value.
     ///
     /// @param parent A data element holding full option data configuration.
-    /// It is used here to log a position if the element holding a code
-    /// is not specified and its position is therefore unavailable.
     ///
     /// @return Option code, possibly unspecified.
     /// @throw DhcpConfigError if option code is invalid.
@@ -619,8 +609,6 @@ private:
     /// @brief Retrieves parsed option name as an optional value.
     ///
     /// @param parent A data element holding full option data configuration.
-    /// It is used here to log a position if the element holding a name
-    /// is not specified and its position is therefore unavailable.
     ///
     /// @return Option name, possibly unspecified.
     /// @throw DhcpConfigError if option name is invalid.
@@ -630,13 +618,14 @@ private:
     /// @brief Retrieves csv-format parameter as an optional value.
     ///
     /// @return Value of the csv-format parameter, possibly unspecified.
-    util::OptionalValue<bool> extractCSVFormat() const;
+    util::OptionalValue<bool> extractCSVFormat(data::ConstElementPtr parent) const;
 
     /// @brief Retrieves option data as a string.
     ///
+    /// @param parent A data element holding full option data configuration.
     /// @return Option data as a string. It will return empty string if
     /// option data is unspecified.
-    std::string extractData() const;
+    std::string extractData(data::ConstElementPtr parent) const;
 
     /// @brief Retrieves option space name.
     ///
@@ -644,27 +633,10 @@ private:
     /// 'dhcp4' or 'dhcp6' option space name is returned, depending on
     /// the universe specified in the parser context.
     ///
+    /// @param parent A data element holding full option data configuration.
+    ///
     /// @return Option space name.
-    std::string extractSpace() const;
-
-    /// Storage for boolean values.
-    BooleanStoragePtr boolean_values_;
-
-    /// Storage for string values (e.g. option name or data).
-    StringStoragePtr string_values_;
-
-    /// Storage for uint32 values (e.g. option code).
-    Uint32StoragePtr uint32_values_;
-
-    /// Option descriptor holds newly configured option.
-    OptionDescriptor option_descriptor_;
-
-    /// Option space name where the option belongs to.
-    std::string option_space_;
-
-    /// @brief Configuration holding option being parsed or NULL if the option
-    /// is global.
-    CfgOptionPtr cfg_;
+    std::string extractSpace(data::ConstElementPtr parent) const;
 
     /// @brief Address family: @c AF_INET or @c AF_INET6.
     uint16_t address_family_;
@@ -680,97 +652,43 @@ typedef OptionDataParser *OptionDataParserFactory(const std::string&,
 /// data for a particular subnet and creates a collection of options.
 /// If parsing is successful, all these options are added to the Subnet
 /// object.
-class OptionDataListParser : public DhcpConfigParser {
+class OptionDataListParser : public isc::data::SimpleParser {
 public:
     /// @brief Constructor.
     ///
-    /// @param dummy nominally would be param name, this is always ignored.
-    /// @param [out] cfg Pointer to the configuration object where options
-    /// should be stored or NULL if this is global option scope.
     /// @param address_family Address family: @c AF_INET or AF_INET6
-    OptionDataListParser(const std::string& dummy, const CfgOptionPtr& cfg,
-                         const uint16_t address_family);
+    OptionDataListParser(const uint16_t address_family);
 
-    /// @brief Parses entries that define options' data for a subnet.
-    ///
-    /// This method iterates over all entries that define option data
-    /// for options within a single subnet and creates options' instances.
+    /// @brief Parses a list of options, instantiates them and stores in cfg
     ///
-    /// @param option_data_list pointer to a list of options' data sets.
-    /// @throw DhcpConfigError if option parsing failed.
-    void build(isc::data::ConstElementPtr option_data_list);
-
-    /// @brief Commit all option values.
+    /// This method expects to get a list of options in option_data_list,
+    /// iterates over them, creates option objects, wraps them with
+    /// option descriptor and stores in specified cfg.
     ///
-    /// This function invokes commit for all option values
-    /// and append suboptions to the top-level options.
-    void commit();
-
+    /// @param cfg created options will be stored here
+    /// @param option_data_list configuration that describes the options
+    void parse(const CfgOptionPtr& cfg,
+               isc::data::ConstElementPtr option_data_list);
 private:
-
-    /// Collection of parsers;
-    ParserCollection parsers_;
-
-    /// @brief Pointer to a configuration where options are stored.
-    CfgOptionPtr cfg_;
-
     /// @brief Address family: @c AF_INET or @c AF_INET6
     uint16_t address_family_;
-
 };
 
-typedef boost::shared_ptr<OptionDataListParser> OptionDataListParserPtr;
-
+typedef std::pair<isc::dhcp::OptionDefinitionPtr, std::string> OptionDefinitionTuple;
 
 /// @brief Parser for a single option definition.
 ///
 /// This parser creates an instance of a single option definition.
-class OptionDefParser : public DhcpConfigParser {
+class OptionDefParser : public isc::data::SimpleParser {
 public:
-    /// @brief Constructor.
-    ///
-    /// @param dummy first argument is ignored, all Parser constructors
-    /// accept string as first argument.
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    OptionDefParser(const std::string& dummy, ParserContextPtr global_context);
-
     /// @brief Parses an entry that describes single option definition.
     ///
     /// @param option_def a configuration entry to be parsed.
+    /// @return tuple (option definition, option space) of the parsed structure
     ///
     /// @throw DhcpConfigError if parsing was unsuccessful.
-    void build(isc::data::ConstElementPtr option_def);
-
-    /// @brief Stores the parsed option definition in a storage.
-    void commit();
-
-private:
-
-    /// @brief Create option definition from the parsed parameters.
-    ///
-    /// @param option_def_element A data element holding the configuration
-    /// for an option definition.
-    void createOptionDef(isc::data::ConstElementPtr option_def_element);
-
-    /// Instance of option definition being created by this parser.
-    OptionDefinitionPtr option_definition_;
-
-    /// Name of the space the option definition belongs to.
-    std::string option_space_name_;
-
-    /// Storage for boolean values.
-    BooleanStoragePtr boolean_values_;
-
-    /// Storage for string values.
-    StringStoragePtr string_values_;
-
-    /// Storage for uint32 values.
-    Uint32StoragePtr uint32_values_;
-
-    /// Parsing context which contains global values, options and option
-    /// definitions.
-    ParserContextPtr global_context_;
+    OptionDefinitionTuple
+    parse(isc::data::ConstElementPtr option_def);
 };
 
 /// @brief Parser for a list of option definitions.
@@ -779,37 +697,17 @@ private:
 /// option definitions and creates instances of these definitions.
 /// If the parsing is successful, the collection of created definitions
 /// is put into the provided storage.
-class OptionDefListParser : public DhcpConfigParser {
+class OptionDefListParser : public isc::data::SimpleParser {
 public:
-    /// @brief Constructor.
-    ///
-    /// @param dummy first argument is ignored, all Parser constructors
-    /// accept string as first argument.
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    OptionDefListParser(const std::string& dummy,
-                        ParserContextPtr global_context);
-
-    /// @brief Parse configuration entries.
+    /// @brief Parses a list of option defintions, create them and store in cfg
     ///
-    /// This function parses configuration entries, creates instances
-    /// of option definitions and tries to add them to the Configuration
-    /// Manager.
-    ///
-    /// @param option_def_list pointer to an element that holds entries
-    /// that define option definitions.
-    /// @throw DhcpConfigError if configuration parsing fails.
-    void build(isc::data::ConstElementPtr option_def_list);
-
-    /// @brief Commits option definitions.
+    /// This method iterates over def_list, which is a JSON list of option defintions,
+    /// then creates corresponding option defintions and store them in the
+    /// configuration structure.
     ///
-    /// Currently this function is no-op, because option definitions are
-    /// added to the Configuration Manager in the @c build method.
-    void commit();
-
-    /// Parsing context which contains global values, options and option
-    /// definitions.
-    ParserContextPtr global_context_;
+    /// @param def_list JSON list describing option definitions
+    /// @param cfg parsed option definitions will be stored here
+    void parse(CfgOptionDefPtr cfg, isc::data::ConstElementPtr def_list);
 };
 
 /// @brief a collection of pools

src/lib/dhcpsrv/parsers/host_reservation_parser.cc

@@ -196,8 +196,12 @@ HostReservationParser4::build(isc::data::ConstElementPtr reservation_data) {
         // surround it with try-catch.
         if (element.first == "option-data") {
             CfgOptionPtr cfg_option = host_->getCfgOption4();
-            OptionDataListParser parser(element.first, cfg_option, AF_INET);
-            parser.build(element.second);
+
+            // This parser is converted to SimpleParser already. It
+            // parses the Element structure immediately, there's no need
+            // to go through build/commit phases.
+            OptionDataListParser parser(AF_INET);
+            parser.parse(cfg_option, element.second);
 
        // Everything else should be surrounded with try-catch to append
        // position.
@@ -255,8 +259,12 @@ HostReservationParser6::build(isc::data::ConstElementPtr reservation_data) {
         // appended).
         if (element.first == "option-data") {
             CfgOptionPtr cfg_option = host_->getCfgOption6();
-            OptionDataListParser parser(element.first, cfg_option, AF_INET6);
-            parser.build(element.second);
+
+            // This parser is converted to SimpleParser already. It
+            // parses the Element structure immediately, there's no need
+            // to go through build/commit phases.
+            OptionDataListParser parser(AF_INET6);
+            parser.parse(cfg_option, element.second);
 
         } else if (element.first == "ip-addresses" || element.first == "prefixes") {
             BOOST_FOREACH(ConstElementPtr prefix_element,

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -7,6 +7,7 @@
 #include <config.h>
 #include <cc/command_interpreter.h>
 #include <cc/data.h>
+#include <cc/simple_parser.h>
 #include <dhcp/option.h>
 #include <dhcp/option_custom.h>
 #include <dhcp/option_int.h>
@@ -292,7 +293,7 @@ public:
     /// @return returns an ConstElementPtr containing the numeric result
     /// code and outcome comment.
     isc::data::ConstElementPtr parseElementSet(isc::data::ConstElementPtr
-                                           config_set) {
+                                               config_set) {
         // Answer will hold the result.
         ConstElementPtr answer;
         if (!config_set) {
@@ -310,6 +311,26 @@ public:
             const std::map<std::string, ConstElementPtr>& values_map =
                                                       config_set->mapValue();
             BOOST_FOREACH(config_pair, values_map) {
+
+                // These are the simple parsers. No need to go through
+                // the ParserPtr hooplas with them.
+                if ( (config_pair.first == "option-data") ||
+                     (config_pair.first == "option-def")) {
+                    continue;
+                }
+
+                // We also don't care about the default values that may be been
+                // inserted
+                if ( (config_pair.first == "preferred-lifetime") ||
+                     (config_pair.first == "valid-lifetime") ||
+                     (config_pair.first == "renew-timer") ||
+                     (config_pair.first == "rebind-timer")) {
+                    continue;
+                }
+
+                // Remaining ones are old style parsers. Need go do
+                // the build/commit dance with them.
+
                 // Create the parser based on element name.
                 ParserPtr parser(createConfigParser(config_pair.first));
                 // Options must be parsed last
@@ -322,12 +343,27 @@ public:
                 }
             }
 
+            int family = parser_context_->universe_ == Option::V4
+                ? AF_INET : AF_INET6;
+
+            // The option definition parser is the next one to be run.
+            std::map<std::string, ConstElementPtr>::const_iterator
+                                def_config = values_map.find("option-def");
+            if (def_config != values_map.end()) {
+
+                CfgOptionDefPtr cfg_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
+                OptionDefListParser def_list_parser;
+                def_list_parser.parse(cfg_def, def_config->second);
+            }
+
             // The option values parser is the next one to be run.
             std::map<std::string, ConstElementPtr>::const_iterator
                                 option_config = values_map.find("option-data");
             if (option_config != values_map.end()) {
-                option_parser->build(option_config->second);
-                option_parser->commit();
+                CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption();
+
+                OptionDataListParser option_list_parser(family);
+                option_list_parser.parse(cfg_option, option_config->second);
             }
 
             // Everything was fine. Configuration is successful.
@@ -359,17 +395,9 @@ public:
     /// @throw throws NotImplemented if element name isn't supported.
     ParserPtr createConfigParser(const std::string& config_id) {
         ParserPtr parser;
-        int family = parser_context_->universe_ == Option::V4 ?
-            AF_INET : AF_INET6;
-        if (config_id.compare("option-data") == 0) {
-            parser.reset(new OptionDataListParser(config_id, CfgOptionPtr(),
-                                                  family));
-
-        } else if (config_id.compare("option-def") == 0) {
-            parser.reset(new OptionDefListParser(config_id,
-                                                 parser_context_));
-
-        } else if (config_id.compare("hooks-libraries") == 0) {
+        // option-data and option-def converted to SimpleParser, so they
+        // are no longer here.
+        if (config_id.compare("hooks-libraries") == 0) {
             parser.reset(new HooksLibrariesParser(config_id));
             hooks_libraries_parser_ =
                 boost::dynamic_pointer_cast<HooksLibrariesParser>(parser);
@@ -384,6 +412,101 @@ public:
         return (parser);
     }
 
+    /// @brief DHCP-specific method that sets global, and option specific defaults
+    ///
+    /// This method sets the defaults in the global scope, in option definitions,
+    /// and in option data.
+    ///
+    /// @param global pointer to the Element tree that holds configuration
+    /// @param global_defaults array with global default values
+    /// @param option_defaults array with option-data default values
+    /// @param option_def_defaults array with default values for option definitions
+    /// @return number of default values inserted.
+    size_t setAllDefaults(isc::data::ElementPtr global,
+                          const SimpleDefaults& global_defaults,
+                          const SimpleDefaults& option_defaults,
+                          const SimpleDefaults& option_def_defaults) {
+        size_t cnt = 0;
+        // Set global defaults first.
+        /// @todo: Uncomment as part of the ticket 5019 work.
+        cnt = SimpleParser::setDefaults(global, global_defaults);
+
+        // Now set option defintion defaults for each specified option definition
+        ConstElementPtr option_defs = global->get("option-def");
+        if (option_defs) {
+            BOOST_FOREACH(ElementPtr single_def, option_defs->listValue()) {
+                cnt += SimpleParser::setDefaults(single_def, option_def_defaults);
+            }
+        }
+
+        ConstElementPtr options = global->get("option-data");
+        if (options) {
+            BOOST_FOREACH(ElementPtr single_option, options->listValue()) {
+                cnt += SimpleParser::setDefaults(single_option, option_defaults);
+            }
+        }
+
+        return (cnt);
+    }
+
+    /// @brief sets all default values for DHCPv4 and DHCPv6
+    ///
+    /// This function largely duplicates what SimpleParser4 and SimpleParser6 classes
+    /// provide. However, since there are tons of unit-tests in dhcpsrv that need
+    /// this functionality and there are good reasons to keep those classes in
+    /// src/bin/dhcp{4,6}, the most straightforward way is to simply copy the
+    /// minimum code here. Hence this method.
+    ///
+    /// @param config configuration structure to be filled with default values
+    /// @param v6 true = DHCPv6, false = DHCPv4
+    void setAllDefaults(ElementPtr config, bool v6) {
+        /// This table defines default values for option definitions in DHCPv6
+        const SimpleDefaults OPTION6_DEF_DEFAULTS = {
+            { "record-types", Element::string,  ""},
+            { "space",        Element::string,  "dhcp6"},
+            { "array",        Element::boolean, "false"},
+            { "encapsulate",  Element::string,  "" }
+        };
+
+        /// This table defines default values for option definitions in DHCPv4
+        const SimpleDefaults OPTION4_DEF_DEFAULTS = {
+            { "record-types", Element::string,  ""},
+            { "space",        Element::string,  "dhcp4"},
+            { "array",        Element::boolean, "false"},
+            { "encapsulate",  Element::string,  "" }
+        };
+
+        /// This table defines default values for options in DHCPv6
+        const SimpleDefaults OPTION6_DEFAULTS = {
+            { "space",        Element::string,  "dhcp6"},
+            { "csv-format",   Element::boolean, "true"},
+            { "encapsulate",  Element::string,  "" }
+        };
+
+        /// This table defines default values for options in DHCPv4
+        const SimpleDefaults OPTION4_DEFAULTS = {
+            { "space",        Element::string,  "dhcp4"},
+            { "csv-format",   Element::boolean, "true"},
+            { "encapsulate",  Element::string,  "" }
+        };
+
+        /// This table defines default values for both DHCPv4 and DHCPv6
+        const SimpleDefaults GLOBAL6_DEFAULTS = {
+            { "renew-timer",        Element::integer, "900" },
+            { "rebind-timer",       Element::integer, "1800" },
+            { "preferred-lifetime", Element::integer, "3600" },
+            { "valid-lifetime",     Element::integer, "7200" }
+        };
+
+        if (v6) {
+            setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION6_DEFAULTS,
+                           OPTION6_DEF_DEFAULTS);
+        } else {
+            setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION4_DEFAULTS,
+                           OPTION4_DEF_DEFAULTS);
+        }
+    }
+
     /// @brief Convenience method for parsing a configuration
     ///
     /// Given a configuration string, convert it into Elements
@@ -392,13 +515,15 @@ public:
     ///
     /// @return retuns 0 if the configuration parsed successfully,
     /// non-zero otherwise failure.
-    int parseConfiguration(const std::string& config) {
+    int parseConfiguration(const std::string& config, bool v6 = false) {
         int rcode_ = 1;
         // Turn config into elements.
         // Test json just to make sure its valid.
         ElementPtr json = Element::fromJSON(config);
         EXPECT_TRUE(json);
         if (json) {
+            setAllDefaults(json, v6);
+
             ConstElementPtr status = parseElementSet(json);
             ConstElementPtr comment = parseAnswer(rcode_, status);
             error_text_ = comment->stringValue();
@@ -566,7 +691,7 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) {
         "}";
 
     // Verify that the configuration string parses.
-    int rcode = parseConfiguration(config);
+    int rcode = parseConfiguration(config, true);
     ASSERT_EQ(0, rcode);
 
 
@@ -828,7 +953,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) {
         " } ]"
         "}";
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_NE(0, rcode);
 
     CfgMgr::instance().clear();
@@ -844,7 +969,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) {
         "    \"data\": \"0\""
         " } ]"
         "}";
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_NE(0, rcode);
 
     CfgMgr::instance().clear();
@@ -859,7 +984,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) {
         "    \"data\": \"0\""
         " } ]"
         "}";
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     ASSERT_EQ(0, rcode);
     OptionPtr opt = getOptionPtr(DHCP6_OPTION_SPACE, 25000);
     ASSERT_TRUE(opt);
@@ -875,10 +1000,11 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) {
         "    \"name\": \"foo-name\","
         "    \"space\": \"dhcp6\","
         "    \"code\": 25000,"
+        "    \"csv-format\": false,"
         "    \"data\": \"123456\""
         " } ]"
         "}";
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     opt = getOptionPtr(DHCP6_OPTION_SPACE, 25000);
     ASSERT_TRUE(opt);
@@ -899,7 +1025,7 @@ TEST_F(ParseConfigTest, optionDataNoName) {
         " } ]"
         "}";
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
         Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23));
@@ -919,7 +1045,7 @@ TEST_F(ParseConfigTest, optionDataNoCode) {
         " } ]"
         "}";
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
         Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23));
@@ -938,7 +1064,7 @@ TEST_F(ParseConfigTest, optionDataMinimal) {
         " } ]"
         "}";
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
         Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23));
@@ -955,7 +1081,7 @@ TEST_F(ParseConfigTest, optionDataMinimal) {
         " } ]"
         "}";
     rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     opt = boost::dynamic_pointer_cast<Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE,
                                                                    23));
@@ -984,7 +1110,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) {
         "}";
 
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
         Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 2345));
@@ -1010,7 +1136,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) {
         "}";
 
     rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     opt = boost::dynamic_pointer_cast<Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE,
                                                                    2345));
@@ -1031,7 +1157,7 @@ TEST_F(ParseConfigTest, emptyOptionData) {
         "}";
 
     int rcode = 0;
-    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_NO_THROW(rcode = parseConfiguration(config, true));
     EXPECT_EQ(0, rcode);
     const Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
         Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, D6O_DHCPV4_O_DHCPV6_SERVER));