[5019] Addressed some (but not all) comments/concerns

src/bin/dhcp4/json_config_parser.cc

@@ -125,10 +125,16 @@ public:
             parser.parse(pools_, pools);
         }
 
-        SubnetPtr generic = SubnetConfigParser::build(subnet);
+        SubnetPtr generic = SubnetConfigParser::parse(subnet);
 
-        Subnet4Ptr sub4ptr = boost::dynamic_pointer_cast<Subnet4>(generic);
-        if (!sub4ptr) {
+        if (!generic) {
+            isc_throw(DhcpConfigError,
+                      "Failed to create an IPv4 subnet (" <<
+                      subnet->getPosition() << ")");
+        }
+
+        Subnet4Ptr sn4ptr = boost::dynamic_pointer_cast<Subnet4>(subnet_);
+        if (!sn4ptr) {
             // If we hit this, it is a programming error.
             isc_throw(Unexpected,
                       "Invalid Subnet4 cast in Subnet4ConfigParser::parse");
@@ -136,7 +142,7 @@ public:
 
         // Set relay information if it was parsed
         if (relay_info_) {
-            sub4ptr->setRelayInfo(*relay_info_);
+            sn4ptr->setRelayInfo(*relay_info_);
         }
 
         // Parse Host Reservations for this subnet if any.
@@ -146,7 +152,7 @@ public:
             parser.parse(subnet_->getID(), reservations);
         }
 
-        return (sub4ptr);
+        return (sn4ptr);
     }
 
 protected:
@@ -378,6 +384,65 @@ public:
     }
 };
 
+class Dhcp4ConfigParser : public isc::data::SimpleParser {
+public:
+
+    /// @brief Sets global parameters in staging configuration
+    ///
+    /// @param global global configuration scope
+    ///
+    /// Currently this method sets the following global parameters:
+    ///
+    /// - echo-client-id
+    /// - decline-probation-period
+    /// - dhcp4o6-port
+    ///
+    /// @throw DhcpConfigError if parameters are missing or
+    /// or having incorrect values.
+    void parse(ConstElementPtr global) {
+
+        // Set whether v4 server is supposed to echo back client-id
+        // (yes = RFC6842 compatible, no = backward compatibility)
+        bool echo_client_id = getBoolean(global, "echo-client-id");
+        CfgMgr::instance().echoClientId(echo_client_id);
+
+        SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg();
+        std::string name;
+        ConstElementPtr value;
+        try {
+            // Set the probation period for decline handling.
+            name = "decline-probation-period";
+            value = global->get(name);
+            uint32_t probation_period = getUint32(name, value);
+            srv_cfg->setDeclinePeriod(probation_period);
+
+            // Set the DHCPv4-over-DHCPv6 interserver port.
+            name = "dhcp4o6-port";
+            value = global->get(name);
+            // @todo Change for uint16_t
+            uint32_t dhcp4o6_port = getUint32(name, value);
+            srv_cfg->setDhcp4o6Port(dhcp4o6_port);
+        } catch (const isc::data::TypeError& ex) {
+            isc_throw(DhcpConfigError,
+                      "invalid value type specified for parameter '" << name
+                      << "' (" << value->getPosition() << ")");
+        }
+    }
+
+private:
+
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of extractInt() to uint32_t
+    ///
+    /// @param value value of the parameter
+    /// @return an uint32_t value
+    uint32_t getUint32(const std::string& name,
+                       isc::data::ConstElementPtr value) {
+        return (extractInt<uint32_t, DhcpConfigError>(name, value));
+    }
+};
+
 } // anonymous namespace
 
 namespace isc {
@@ -420,49 +485,6 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id,
     return (parser);
 }
 
-/// @brief Sets global parameters in staging configuration
-///
-/// @param global global configuration scope
-///
-/// Currently this method sets the following global parameters:
-///
-/// - echo-client-id
-/// - decline-probation-period
-/// - dhcp4o6-port
-void setGlobalParameters4(ConstElementPtr global) {
-    // Set whether v4 server is supposed to echo back client-id (yes = RFC6842
-    // compatible, no = backward compatibility)
-    bool echo_client_id = SimpleParser::getBoolean(global, "echo-client-id");
-    CfgMgr::instance().echoClientId(echo_client_id);
-
-    /// @todo: Use Francis' template from SimpleParser once 5097 is merged
-    /// This will be done as part of #5116.
-    int64_t probation_period = SimpleParser::getInteger(global,
-                                                        "decline-probation-period");
-    if (probation_period < std::numeric_limits<uint32_t>::min() ||
-        probation_period > std::numeric_limits<uint32_t>::max()) {
-        isc_throw(DhcpConfigError, "Invalid decline-probation-period value: "
-                  << probation_period << ", allowed range: "
-                  << std::numeric_limits<uint32_t>::min() << ".."
-                  << std::numeric_limits<uint32_t>::max());
-    }
-    CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period);
-
-    // Set the DHCPv4-over-DHCPv6 interserver port.
-
-    /// @todo: Use Francis' template from SimpleParser once 5097 is merged
-    /// This will be done as part of #5116.
-    uint32_t dhcp4o6_port = SimpleParser::getInteger(global, "dhcp4o6-port");
-    if (dhcp4o6_port < std::numeric_limits<uint16_t>::min() ||
-        dhcp4o6_port > std::numeric_limits<uint16_t>::max()) {
-        isc_throw(DhcpConfigError, "Invalid dhcp4o6-port value: "
-                  << dhcp4o6_port << ", allowed range: "
-                  << std::numeric_limits<uint16_t>::min() << ".."
-                  << std::numeric_limits<uint16_t>::max());
-    }
-    CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port);
-}
-
 /// @brief Initialize the command channel based on the staging configuration
 ///
 /// Only close the current channel, if the new channel configuration is
@@ -685,7 +707,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
             // Timers are not used in the global scope. Their values are derived
             // to specific subnets (see SimpleParser6::deriveParameters).
             // decline-probation-period, dhcp4o6-port, echo-client-id are
-            // handlded in the setGlobalParameters4.
+            // handled in global_parser.parse() which sets global parameters.
             // match-client-id is derived to subnet scope level.
             if ( (config_pair.first == "renew-timer") ||
                  (config_pair.first == "rebind-timer") ||
@@ -718,6 +740,19 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
         // Setup the command channel.
         configureCommandChannel();
 
+        // the leases database parser is the last to be run.
+        std::map<std::string, ConstElementPtr>::const_iterator leases_config =
+            values_map.find("lease-database");
+        if (leases_config != values_map.end()) {
+            config_pair.first = "lease-database";
+            leases_parser->build(leases_config->second);
+            leases_parser->commit();
+        }
+
+        // Apply global options in the staging config.
+        Dhcp4ConfigParser global_parser;
+        global_parser.parse(mutable_cfg);
+
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL)
                   .arg(config_pair.first).arg(ex.what());
@@ -791,7 +826,5 @@ ParserContextPtr& globalContext() {
     return (global_context_ptr);
 }
 
-
-
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp6/json_config_parser.cc

@@ -323,7 +323,7 @@ public:
             parser.parse(pools_, pd_pools);
         }
 
-        SubnetPtr generic = SubnetConfigParser::build(subnet);
+        SubnetPtr generic = SubnetConfigParser::parse(subnet);
 
         if (!generic) {
             isc_throw(DhcpConfigError,
@@ -331,8 +331,8 @@ public:
                       subnet->getPosition() << ")");
         }
 
-        Subnet6Ptr sub6ptr = boost::dynamic_pointer_cast<Subnet6>(subnet_);
-        if (!sub6ptr) {
+        Subnet6Ptr sn6ptr = boost::dynamic_pointer_cast<Subnet6>(subnet_);
+        if (!sn6ptr) {
             // If we hit this, it is a programming error.
             isc_throw(Unexpected,
                       "Invalid Subnet6 cast in Subnet6ConfigParser::parse");
@@ -340,7 +340,7 @@ public:
 
         // Set relay information if it was provided
         if (relay_info_) {
-            sub6ptr->setRelayInfo(*relay_info_);
+            sn6ptr->setRelayInfo(*relay_info_);
         }
 
 
@@ -351,7 +351,7 @@ public:
             parser.parse(subnet_->getID(), reservations);
         }
 
-        return (sub6ptr);
+        return (sn6ptr);
     }
 
 protected:
@@ -616,6 +616,58 @@ public:
     virtual void commit() {}
 };
 
+class Dhcp6ConfigParser : public isc::data::SimpleParser {
+public:
+
+    /// @brief Sets global parameters in staging configuration
+    ///
+    /// @param global global configuration scope
+    ///
+    /// Currently this method sets the following global parameters:
+    ///
+    /// - decline-probation-period
+    /// - dhcp4o6-port
+    ///
+    /// @throw DhcpConfigError if parameters are missing or
+    /// or having incorrect values.
+    void parse(ConstElementPtr global) {
+
+        SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg();
+        std::string name;
+        ConstElementPtr value;
+        try {
+            // Set the probation period for decline handling.
+            name = "decline-probation-period";
+            value = global->get(name);
+            uint32_t probation_period = getUint32(name, value);
+            srv_config->setDeclinePeriod(probation_period);
+
+            // Set the DHCPv4-over-DHCPv6 interserver port.
+            name = "dhcp4o6-port";
+            value = global->get(name);
+            // @todo Change for uint16_t
+            uint32_t dhcp4o6_port = getUint32(name, value);
+            srv_config->setDhcp4o6Port(dhcp4o6_port);
+        } catch (const isc::data::TypeError& ex) {
+            isc_throw(DhcpConfigError,
+                      "invalid value type specified for parameter '" << name
+                      << "' (" << value->getPosition() << ")");
+        }
+    }
+
+private:
+
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of extractInt() to uint32_t
+    ///
+    /// @param value value of the parameter
+    /// @return an uint32_t value
+    uint32_t getUint32(const std::string& name,
+                       isc::data::ConstElementPtr value) {
+        return (extractInt<uint32_t, DhcpConfigError>(name, value));
+    }
+};
 
 } // anonymous namespace
 
@@ -667,47 +719,6 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
     return (parser);
 }
 
-/// @brief Sets global parameters in the staging configuration
-///
-/// @param global global configuration scope
-///
-/// Currently this method sets the following global parameters:
-///
-/// - decline-probation-period
-/// - dhcp4o6-port
-///
-/// @throw DhcpConfigError if parameters are missing or having incorrect values.
-void setGlobalParameters6(ConstElementPtr global) {
-
-    // Set the probation period for decline handling.
-    /// @todo: Use Francis' template from SimpleParser once 5097 is merged
-    /// This will be done as part of #5116.
-    int64_t probation_period = SimpleParser::getInteger(global,
-                                                        "decline-probation-period");
-    if (probation_period < std::numeric_limits<uint32_t>::min() ||
-        probation_period > std::numeric_limits<uint32_t>::max()) {
-        isc_throw(DhcpConfigError, "Invalid decline-probation-period value: "
-                  << probation_period << ", allowed range: "
-                  << std::numeric_limits<uint32_t>::min() << ".."
-                  << std::numeric_limits<uint32_t>::max());
-    }
-    CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period);
-
-    // Set the DHCPv4-over-DHCPv6 interserver port.
-
-    /// @todo: Use Francis' template from SimpleParser once 5097 is merged
-    /// This will be done as part of #5116.
-    uint32_t dhcp4o6_port = SimpleParser::getInteger(global, "dhcp4o6-port");
-    if (dhcp4o6_port < std::numeric_limits<uint16_t>::min() ||
-        dhcp4o6_port > std::numeric_limits<uint16_t>::max()) {
-        isc_throw(DhcpConfigError, "Invalid dhcp4o6-port value: "
-                  << dhcp4o6_port << ", allowed range: "
-                  << std::numeric_limits<uint16_t>::min() << ".."
-                  << std::numeric_limits<uint16_t>::max());
-    }
-    CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port);
-}
-
 /// @brief Initialize the command channel based on the staging configuration
 ///
 /// Only close the current channel, if the new channel configuration is
@@ -946,7 +957,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
             // Timers are not used in the global scope. Their values are derived
             // to specific subnets (see SimpleParser6::deriveParameters).
             // decline-probation-period and dhcp4o6-port are handled in the
-            // setGlobalParameters6.
+            // global_parser.parse() which sets global parameters.
             if ( (config_pair.first == "renew-timer") ||
                  (config_pair.first == "rebind-timer") ||
                  (config_pair.first == "preferred-lifetime") ||
@@ -977,7 +988,8 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
         configureCommandChannel();
 
         // Apply global options in the staging config.
-        setGlobalParameters6(mutable_cfg);
+        Dhcp6ConfigParser global_parser;
+        global_parser.parse(mutable_cfg);
 
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL)

src/lib/cc/simple_parser.h

@@ -113,6 +113,8 @@ class SimpleParser {
     static const data::Element::Position&
     getPosition(const std::string& name, const data::ConstElementPtr parent);
 
+protected:
+
     /// @brief Returns a string parameter from a scope
     ///
     /// Unconditionally returns a parameter. If the parameter is not there or

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -999,7 +999,7 @@ SubnetConfigParser::SubnetConfigParser(const std::string&,
 }
 
 SubnetPtr
-SubnetConfigParser::build(ConstElementPtr subnet) {
+SubnetConfigParser::parse(ConstElementPtr subnet) {
     BOOST_FOREACH(ConfigPair param, subnet->mapValue()) {
         // Pools has been converted to SimpleParser.
         if (param.first == "pools") {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -831,7 +831,7 @@ protected:
     /// @return a pointer to newly created subnet
     ///
     /// @throw isc::DhcpConfigError if subnet configuration parsing failed.
-    virtual SubnetPtr build(isc::data::ConstElementPtr subnet);
+    SubnetPtr parse(isc::data::ConstElementPtr subnet);
 
     /// @brief creates parsers for entries in subnet definition
     ///
@@ -840,8 +840,8 @@ protected:
     /// @return parser object for specified entry name
     /// @throw isc::dhcp::DhcpConfigError if trying to create a parser
     ///        for unknown config element
-    virtual DhcpConfigParser* createSubnetConfigParser(
-                                            const std::string& config_id) = 0;
+    virtual DhcpConfigParser*
+    createSubnetConfigParser(const std::string& config_id) = 0;
 
     /// @brief Issues a server specific warning regarding duplicate subnet
     /// options.
@@ -851,7 +851,7 @@ protected:
     /// @todo a means to know the correct logger and perhaps a common
     /// message would allow this method to be emitted by the base class.
     virtual void duplicate_option_warning(uint32_t code,
-        isc::asiolink::IOAddress& addr) = 0;
+                                          isc::asiolink::IOAddress& addr) = 0;
 
     /// @brief Instantiates the subnet based on a given IP prefix and prefix
     /// length.