[5039] Changes after review:

- address_family_, default ctor removed from two parsers
 - copyright year updated
 - Element::getNonConst is now documented
 - user's guide is now clear about csv-format default value is now true

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
+          <simpara><command>csv-format</command> - if this value is not
-          and the definition for the particular option exists, the server will assume
+          specified the server will assume that the option data is specified as
-          that the option data is specified as a list of comma separated values to be
+          a list of comma separated values to be assigned to individual fields
-          assigned to individual fields of the DHCP option. If the definition
+          of the DHCP option. This behavior has changed in Kea 1.2. Older
-          does not exist for this option, the server will assume that the data
+          versions used additional logic to determined whether the csv-format
-          parameter contains the option payload in the binary format (represented
+          should be true or false. That is no longer the case.
-          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>
         </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
+          <simpara><command>csv-format</command> - if this value is not
-          and the definition for the particular option exists, the server will assume
+          specified the server will assume that the option data is specified as
-          that the option data is specified as a list of comma separated values to be
+          a list of comma separated values to be assigned to individual fields
-          assigned to individual fields of the DHCP option. If the definition
+          of the DHCP option. This behavior has changed in Kea 1.2. Older
-          does not exist for this option, the server will assume that the data
+          versions used additional logic to determined whether the csv-format
-          parameter contains the option payload in the binary format (represented
+          should be true or false. That is no longer the case.
-          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>
         </listitem>
       </itemizedlist>

src/bin/dhcp4/json_config_parser.cc

@@ -575,7 +575,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
         // We need definitions first
         ConstElementPtr option_defs = mutable_cfg->get("option-def");
         if (option_defs) {
-            OptionDefListParser parser(AF_INET);
+            OptionDefListParser parser;
             CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
             parser.parse(cfg_option_def, option_defs);
         }

src/bin/dhcp6/json_config_parser.cc

@@ -851,7 +851,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
         // We need definitions first
         ConstElementPtr option_defs = mutable_cfg->get("option-def");
         if (option_defs) {
-            OptionDefListParser parser(AF_INET6);
+            OptionDefListParser parser;
             CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
             parser.parse(cfg_option_def, option_defs);
         }

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -2024,7 +2024,6 @@ TEST_F(Dhcpv6SrvTest, rsooOverride) {
         "    \"option-def\": [ {"
         "      \"name\": \"foo\","
         "      \"code\": 120,"
-        "      \"csv-format\": false,"
         "      \"type\": \"binary\""
         "    } ],"
         "    \"option-data\": [ {"

src/bin/dhcp6/tests/simple_parser6_unittest.cc~

@@ -0,0 +1,79 @@
+#include <config.h>
+#include <cc/data.h>
+#include <gtest/gtest.h>
+
+using namespace isc;
+using namespace isc::data;
+
+namespace {
+
+/// @brief DHCP Parser test fixture class
+class SimpleParser6Test : public ::testing::Test {
+
+    /// @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;
+
+    EXPECT_NO_THROW(num = SimpleParser6::setDefaults(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 interited, 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/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

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
@@ -277,6 +277,10 @@ 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);
 
     /// Sets the ElementPtr at the given index. If the index is out

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -693,9 +693,6 @@ void OptionDataListParser::parse(const CfgOptionPtr& cfg,
 }
 
 // ******************************** OptionDefParser ****************************
-OptionDefParser::OptionDefParser(const uint16_t address_family)
-    : address_family_(address_family) {
-}
 
 std::pair<isc::dhcp::OptionDefinitionPtr, std::string>
 OptionDefParser::parse(ConstElementPtr option_def) {
@@ -779,10 +776,6 @@ OptionDefParser::parse(ConstElementPtr option_def) {
 }
 
 // ******************************** OptionDefListParser ************************
-OptionDefListParser::OptionDefListParser(const uint16_t address_family)
-    : address_family_(address_family) {
-}
-
 void
 OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) {
     if (!option_def_list) {
@@ -791,7 +784,7 @@ OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_l
                   << option_def_list->getPosition() << ")");
     }
 
-    OptionDefParser parser(address_family_);
+    OptionDefParser parser;
     BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) {
         OptionDefinitionTuple def;
 

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -681,11 +681,6 @@ typedef std::pair<isc::dhcp::OptionDefinitionPtr, std::string> OptionDefinitionT
 /// This parser creates an instance of a single option definition.
 class OptionDefParser : public isc::data::SimpleParser {
 public:
-    /// @brief Constructor.
-    ///
-    /// @param address_family Address family: @c AF_INET or AF_INET6
-    OptionDefParser(const uint16_t address_family);
-
     /// @brief Parses an entry that describes single option definition.
     ///
     /// @param option_def a configuration entry to be parsed.
@@ -694,10 +689,6 @@ public:
     /// @throw DhcpConfigError if parsing was unsuccessful.
     OptionDefinitionTuple
     parse(isc::data::ConstElementPtr option_def);
-
-private:
-    /// @brief Address family: @c AF_INET or @c AF_INET6
-    uint16_t address_family_;
 };
 
 /// @brief Parser for a list of option definitions.
@@ -708,14 +699,6 @@ private:
 /// is put into the provided storage.
 class OptionDefListParser : public isc::data::SimpleParser {
 public:
-    /// @brief Constructor.
-    ///
-    /// Stores address family that will be used to make certain decisions
-    /// during parsing.
-    ///
-    /// @param address_family @c AF_INET or @c AF_INET6
-    OptionDefListParser(const uint16_t address_family);
-
     /// @brief Parses a list of option defintions, create them and store in cfg
     ///
     /// This method iterates over def_list, which is a JSON list of option defintions,
@@ -725,11 +708,6 @@ public:
     /// @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);
-
-protected:
-
-    /// @brief Address family: @c AF_INET or @c AF_INET6
-    uint16_t address_family_;
 };
 
 /// @brief a collection of pools

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -352,7 +352,7 @@ public:
             if (def_config != values_map.end()) {
 
                 CfgOptionDefPtr cfg_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
-                OptionDefListParser def_list_parser(family);
+                OptionDefListParser def_list_parser;
                 def_list_parser.parse(cfg_def, def_config->second);
             }