[master] Merge branch 'trac5123'

src/bin/dhcp4/json_config_parser.cc

@@ -307,7 +307,7 @@ public:
     }
 };
 
-/// @brief Parser that takes care of global DHCPv6 parameters.
+/// @brief Parser that takes care of global DHCPv4 parameters.
 ///
 /// See @ref parse method for a list of supported parameters.
 class Dhcp4ConfigParser : public isc::data::SimpleParser {
@@ -333,39 +333,29 @@ public:
         bool echo_client_id = getBoolean(global, "echo-client-id");
         CfgMgr::instance().echoClientId(echo_client_id);
 
-        std::string name;
-        ConstElementPtr value;
-        try {
         // Set the probation period for decline handling.
-            name = "decline-probation-period";
+        uint32_t probation_period =
-            value = global->get(name);
+            getUint32(global, "decline-probation-period");
-            uint32_t probation_period = getUint32(name, value);
         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);
+        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
         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
+    /// Instantiation of getIntType() to uint32_t
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return an uint32_t value
-    uint32_t getUint32(const std::string& name,
+    uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       isc::data::ConstElementPtr value) {
+                       const std::string& name) {
-        return (extractInt<uint32_t, DhcpConfigError>(name, value));
+        return (getIntType<uint32_t>(scope, name));
     }
 };
 

src/bin/dhcp6/json_config_parser.cc

@@ -155,52 +155,36 @@ public:
     /// that define a prefix delegation pool.
     ///
     /// @throw DhcpConfigError if configuration parsing fails.
-  void parse(PoolStoragePtr pools,
+    void parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) {
-             ConstElementPtr pd_pool_) {
+        std::string addr_str = getString(pd_pool_, "prefix");
-        std::string addr_str;
-        std::string excluded_prefix_str = "::";
-        uint8_t prefix_len = 0;
-        uint8_t delegated_len = 0;
-        uint8_t excluded_prefix_len = 0;
 
-        // Parse the elements that make up the option definition.
+        uint8_t prefix_len = getUint8(pd_pool_, "prefix-len");
-        BOOST_FOREACH(ConfigPair param, pd_pool_->mapValue()) {
+
-            std::string entry(param.first);
+        uint8_t delegated_len = getUint8(pd_pool_, "delegated-len");
-            ConstElementPtr value(param.second);
+
-            try {
+        std::string excluded_prefix_str = "::";
-                if (entry == "prefix") {
+        if (pd_pool_->contains("excluded-prefix")) {
-                    addr_str = value->stringValue();
+            excluded_prefix_str = getString(pd_pool_, "excluded-prefix");
-                } else if (entry == "excluded-prefix") {
+        }
-                    excluded_prefix_str = value->stringValue();
+
-                } else if (entry == "prefix-len") {
+        uint8_t excluded_prefix_len = 0;
-                    prefix_len = getUint8(entry, value);
+        if (pd_pool_->contains("excluded-prefix-len")) {
-                } else if (entry == "delegated-len") {
+            excluded_prefix_len = getUint8(pd_pool_, "excluded-prefix-len");
-                    delegated_len = getUint8(entry, value);
+        }
-                } else if (entry == "excluded-prefix-len") {
+
-                    excluded_prefix_len = getUint8(entry, value);
+        ConstElementPtr option_data = pd_pool_->get("option-data");
-                } else if (entry == "option-data") {
+        if (option_data) {
             OptionDataListParser opts_parser(AF_INET6);
-                    opts_parser.parse(options_, value);
+            opts_parser.parse(options_, option_data);
-                } else if (entry == "user-context") {
-                    user_context_ = value;
-                } else {
-                    isc_throw(isc::dhcp::DhcpConfigError,
-                              "unsupported parameter: " << entry
-                              << " (" << value->getPosition() << ")");
-                }
-            } catch (const isc::data::TypeError&) {
-                isc_throw(isc::dhcp::DhcpConfigError,
-                          "invalid value type specified for parameter '"
-                          << entry << "' ("
-                          << value->getPosition() << ")");
         }
+                    
+        ConstElementPtr user_context = pd_pool_->get("user-context");
+        if (user_context) {
+            user_context_ = user_context;
         }
 
         // Check the pool parameters. It will throw an exception if any
-        // of the required parameters are not present or invalid.
+        // of the required parameters are invalid.
-        requireParam("prefix", pd_pool_);
-        requireParam("prefix-len", pd_pool_);
-        requireParam("delegated-len", pd_pool_);
         try {
             // Attempt to construct the local pool.
             pool_.reset(new Pool6(IOAddress(addr_str),
@@ -228,28 +212,16 @@ public:
 
 private:
 
-    /// @brief Require a mandatory element
-    ///
-    /// @param name Entry name
-    /// @param config Pools configuration
-    /// @throw isc::dhcp::DhcpConfigError if not present
-    void requireParam(const std::string& name, ConstElementPtr config) const {
-        if (!config->contains(name)) {
-            isc_throw(isc::dhcp::DhcpConfigError,
-                      "Missing parameter '" << name << "' ("
-                      << config->getPosition() << ")");
-        }
-    }
-
     /// @brief Get an uint8_t value
     ///
-    /// @param name Entry name
+    /// Instantiation of getIntType() to uint8_t
-    /// @param value Integer element value
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return uint8_t value
-    /// @throw isc::data::TypeError when it is not an integer
+    /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t
-    /// isc::dhcp::DhcpConfigError when it does not fit in an uint8_t
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
-    uint8_t getUint8(const std::string& name, ConstElementPtr value) const {
+        return (getIntType<uint8_t>(scope, name));
-        return (extractInt<uint8_t, DhcpConfigError>(name, value));
     }
 
     /// Pointer to the created pool object.
@@ -567,39 +539,29 @@ public:
     /// or having incorrect values.
     void parse(SrvConfigPtr srv_config, ConstElementPtr global) {
 
-        std::string name;
-        ConstElementPtr value;
-        try {
         // Set the probation period for decline handling.
-            name = "decline-probation-period";
+        uint32_t probation_period =
-            value = global->get(name);
+            getUint32(global, "decline-probation-period");
-            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);
+        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
         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
+    /// Instantiation of getIntType() to uint32_t
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return an uint32_t value
-    uint32_t getUint32(const std::string& name,
+    uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       isc::data::ConstElementPtr value) {
+                       const std::string& name) {
-        return (extractInt<uint32_t, DhcpConfigError>(name, value));
+        return (getIntType<uint32_t>(scope, name));
     }
 };
 

src/lib/cc/Makefile.am

@@ -6,6 +6,7 @@ AM_CXXFLAGS = $(KEA_CXXFLAGS)
 
 lib_LTLIBRARIES = libkea-cc.la
 libkea_cc_la_SOURCES = data.cc data.h
+libkea_cc_la_SOURCES += dhcp_config_error.h
 libkea_cc_la_SOURCES += command_interpreter.cc command_interpreter.h
 libkea_cc_la_SOURCES += simple_parser.cc simple_parser.h
 
@@ -17,7 +18,7 @@ libkea_cc_la_LDFLAGS = -no-undefined -version-info 1:0:0
 # Since data.h is now used in the hooks interface, it needs to be
 # installed on target system.
 libkea_cc_includedir = $(pkgincludedir)/cc
-libkea_cc_include_HEADERS = data.h
+libkea_cc_include_HEADERS = data.h dhcp_config_error.h
 
 EXTRA_DIST = cc.dox
 

src/lib/cc/dhcp_config_error.h

@@ -0,0 +1,51 @@
+// Copyright (C) 2017 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 DHCP_CONFIG_ERROR_H
+#define DHCP_CONFIG_ERROR_H
+
+#include <exceptions/exceptions.h>
+
+namespace isc {
+namespace dhcp {
+
+/// An exception that is thrown if an error occurs while configuring
+/// DHCP server.
+/// By convention when this exception is thrown there is a position
+/// between parentheses so the code style should be like this:
+///
+/// try {
+///     ...
+/// } catch (const DhcpConfigError&) {
+///     throw;
+/// } catch (const std::exception& ex) {
+///    isc_throw(DhcpConfigError, "message" << ex.what()
+///              << " (" << getPosition(what) << ")");
+/// }
+
+/// @todo: move this header into simple_parser.h as soon as
+/// there is no dependency through DhcpConfigParser
+/// @todo: create an isc_throw like macro to add the
+/// position more easily.
+/// @todo: rename the exception for instance into ConfigError
+
+class DhcpConfigError : public isc::Exception {
+public:
+
+    /// @brief constructor
+    ///
+    /// @param file name of the file, where exception occurred
+    /// @param line line of the file, where exception occurred
+    /// @param what text description of the issue that caused exception
+    DhcpConfigError(const char* file, size_t line, const char* what)
+        : isc::Exception(file, line, what) {}
+};
+
+}; // end of isc::dhcp namespace
+}; // end of isc namespace
+
+#endif // DHCP_CONFIG_ERROR_H
+

src/lib/cc/simple_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2017 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
@@ -11,6 +11,7 @@
 #include <string>
 
 using namespace std;
+using isc::dhcp::DhcpConfigError;
 
 namespace isc {
 namespace data {
@@ -19,12 +20,14 @@ std::string
 SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) {
     ConstElementPtr x = scope->get(name);
     if (!x) {
-        isc_throw(BadValue, "String parameter " << name << " not found"
+        isc_throw(DhcpConfigError,
-                  << "(" << scope->getPosition() << ")");
+                  "missing parameter '" << name << "' ("
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::string) {
-        isc_throw(BadValue, "Element " << name << " found, but is not a string"
+        isc_throw(DhcpConfigError,
-                  << "(" << x->getPosition() << ")");
+                  "invalid type specified for parameter '" << name
+                  << "' (" << x->getPosition() << ")");
     }
 
     return (x->stringValue());
@@ -34,12 +37,14 @@ int64_t
 SimpleParser::getInteger(isc::data::ConstElementPtr scope, const std::string& name) {
     ConstElementPtr x = scope->get(name);
     if (!x) {
-        isc_throw(BadValue, "Integer parameter " << name << " not found "
+        isc_throw(DhcpConfigError,
-                  << "(" << scope->getPosition() << ")");
+                  "missing parameter '" << name << "' ("
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::integer) {
-        isc_throw(BadValue, "Element " << name << " found, but is not an integer"
+        isc_throw(DhcpConfigError,
-                  << "(" << x->getPosition() << ")");
+                  "invalid type specified for parameter '" << name
+                  << "' (" << x->getPosition() << ")");
     }
 
     return (x->intValue());
@@ -49,13 +54,14 @@ bool
 SimpleParser::getBoolean(isc::data::ConstElementPtr scope, const std::string& name) {
     ConstElementPtr x = scope->get(name);
     if (!x) {
-        isc_throw(BadValue, "Boolean element " << name << " not found "
+        isc_throw(DhcpConfigError,
-                  << "(" << scope->getPosition() << ")");
+                  "missing parameter '" << name << "' ("
-
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::boolean) {
-        isc_throw(BadValue, "Element " << name << " found, but is not a boolean"
+        isc_throw(DhcpConfigError,
-                  << "(" << x->getPosition() << ")");
+                  "invalid type specified for parameter '" << name
+                  << "' (" << x->getPosition() << ")");
     }
 
     return (x->boolValue());
@@ -112,7 +118,8 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope,
             } else if (def_value.value_ == string("false")) {
                 bool_value = false;
             } else {
-                isc_throw(BadValue, "Internal error. Boolean value specified as "
+                isc_throw(DhcpConfigError,
+                          "Internal error. Boolean value specified as "
                           << def_value.value_ << ", expected true or false");
             }
             x.reset(new BoolElement(bool_value, pos));
@@ -125,7 +132,8 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope,
         }
         default:
             // No default values for null, list or map
-            isc_throw(BadValue, "Internal error. Incorrect default value type.");
+            isc_throw(DhcpConfigError,
+                      "Internal error. Incorrect default value type.");
         }
 
         // ... and insert it into the provided Element tree.

src/lib/cc/simple_parser.h

@@ -8,6 +8,7 @@
 #define SIMPLE_PARSER_H
 
 #include <cc/data.h>
+#include <cc/dhcp_config_error.h>
 #include <vector>
 #include <string>
 #include <stdint.h>
@@ -117,86 +118,90 @@ protected:
 
     /// @brief Returns a string parameter from a scope
     ///
-    /// Unconditionally returns a parameter. If the parameter is not there or
+    /// Unconditionally returns a parameter.
-    /// 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
+    /// @throw DhcpConfigError if the parameter is not there or is not of
+    /// appropriate type
     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
+    /// Unconditionally returns a parameter.
-    /// 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
+    /// @throw DhcpConfigError if the parameter is not there or is not of
+    /// appropriate type
     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
+    /// Unconditionally returns a parameter.
-    /// 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
+    /// @throw DhcpConfigError if the parameter is not there or is not of
+    /// appropriate type
     static bool getBoolean(isc::data::ConstElementPtr scope,
                            const std::string& name);
 
-    /// @brief Returns an integer value with range checking
+    /// @brief Returns an integer value with range checking from a scope
     ///
     /// This template should be instantied in parsers when useful
     ///
     /// @tparam int_type the integer type e.g. uint32_t
-    /// @tparam out_of_range always @c isc::dhcp::DhcpConfigError
+    /// @param scope specified parameter will be extracted from this scope
     /// @param name name of the parameter for error report
-    /// @param value value of the parameter
     /// @return a value of int_type
-    /// @throw isc::data::TypeError when the value is not an integer
+    /// @throw DhcpConfigError if the parameter is not there, is not of
-    /// @throw out_of_range when the value does not fit in int_type
+    /// appropriate type or is out of type value range
-    template <typename int_type, class out_of_range> int_type
+    template <typename int_type> int_type
-    extractInt(const std::string& name, ConstElementPtr value) const {
+    getIntType(isc::data::ConstElementPtr scope,
-        int64_t val_int = value->intValue();
+               const std::string& name) {
+        int64_t val_int = getInteger(scope, name);
         if ((val_int < std::numeric_limits<int_type>::min()) ||
             (val_int > std::numeric_limits<int_type>::max())) {
-            isc_throw(out_of_range, "out of range value (" << val_int
+          isc_throw(isc::dhcp::DhcpConfigError,
+                    "out of range value (" << val_int
                     << ") specified for parameter '" << name
-                      << "' (" << value->getPosition() << ")");
+                    << "' (" << getPosition(name, scope) << ")");
         }
         return (static_cast<int_type>(val_int));
     }
 
-    /// @brief Returns a converted value
+    /// @brief Returns a converted value from a scope
     ///
     /// This template should be instantied in parsers when useful
     ///
     /// @tparam target_type the type of the result
     /// @tparam convert the conversion function std::string -> target_type
-    /// @tparam exception_type always @c isc::dhcp::DhcpConfigError
+    /// @param scope specified parameter will be extracted from this scope
     /// @param name name of the parameter for error report
     /// @param type_name name of target_type for error report
     /// @param value value of the parameter
     /// @return a converted value of target_type
-    /// @throw isc::data::TypeError when the value is not an integer
+    /// @throw DhcpConfigError if the parameter is not there, is not of
-    /// @throw exception_type when the value cannot be converted
+    /// appropriate type or can not be converted
     template <typename target_type,
-              target_type convert(const std::string&),
+              target_type convert(const std::string&)> target_type
-              class exception_type> target_type
+    getAndConvert(isc::data::ConstElementPtr scope,
-    extractConvert(const std::string& name,
+                  const std::string& name,
-                   const std::string& type_name,
+                  const std::string& type_name) {
-                   ConstElementPtr value) const {
+        std::string str = getString(scope, name);
-        std::string str = value->stringValue();
         try {
             return (convert(str));
         } catch (const std::exception&) {
-            isc_throw(exception_type, "invalid " << type_name << " (" << str
+            isc_throw(isc::dhcp::DhcpConfigError,
+                      "invalid " << type_name << " (" << str
                       << ") specified for parameter '" << name
-                      << "' (" << value->getPosition() << ")");
+                      << "' (" << getPosition(name, scope) << ")");
         }
     }
 };

src/lib/cc/tests/simple_parser_unittest.cc

@@ -9,6 +9,7 @@
 #include <gtest/gtest.h>
 
 using namespace isc::data;
+using isc::dhcp::DhcpConfigError;
 
 /// This table defines sample default values. Although these are DHCPv6
 /// specific, the mechanism is generic and can be used by any other component.
@@ -57,23 +58,22 @@ public:
 
 class SimpleParserClassTest : public SimpleParser {
 public:
-    /// @brief Instantiation of extractInt for uint8_t
+    /// @brief Instantiation of getIntType for uint8_t
     ///
+    /// @param scope specified parameter will be extracted from this scope
     /// @param name name of the parameter for error report
-    /// @param value value of the parameter
     /// @return an uint8_t value
-    uint8_t extractUint8(const std::string& name, ConstElementPtr value) const {
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
-        return (extractInt<uint8_t, isc::OutOfRange>(name, value));
+        return (getIntType<uint8_t>(scope, name));
     }
 
-    /// @brief Instantiation of extractConvert
+    /// @brief Instantiation of getAndConvert
     ///
+    /// @param scope specified parameter will be extracted from this scope
     /// @param name name of the parameter for error report
-    /// @param value value of the parameter
     /// @return a bool value
-    bool extractBool(const std::string& name, ConstElementPtr value) const {
+    bool getAsBool(ConstElementPtr scope, const std::string& name) {
-        return (extractConvert<bool, toBool, isc::BadValue>
+        return (getAndConvert<bool, toBool>(scope, name, "boolean"));
-                    (name, "boolean", value));
     }
 
     /// @brief Convert to boolean
@@ -169,45 +169,53 @@ TEST_F(SimpleParserTest, setListDefaults) {
     checkIntegerValue(third, "renew-timer", 900);
 }
 
-// This test exercises the extractInt template
+// This test exercises the getIntType template
-TEST_F(SimpleParserTest, extractInt) {
+TEST_F(SimpleParserTest, getIntType) {
 
     SimpleParserClassTest parser;
 
-    // extractInt checks if it is an integer
+    // getIntType checks it can be found
-    ConstElementPtr not_int(new StringElement("xyz"));
+    ElementPtr not_found = Element::fromJSON("{ \"bar\": 1 }");
-    EXPECT_THROW(parser.extractUint8("foo", not_int), TypeError);
+    EXPECT_THROW(parser.getUint8(not_found, "foo"), DhcpConfigError);
 
-    // extractInt checks bounds
+    // getIntType checks if it is an integer
-    ConstElementPtr negative(new IntElement(-1));
+    ElementPtr not_int = Element::fromJSON("{ \"foo\": \"xyz\" }");
-    EXPECT_THROW(parser.extractUint8("foo", negative), isc::OutOfRange);
+    EXPECT_THROW(parser.getUint8(not_int, "foo"), DhcpConfigError);
-    ConstElementPtr too_large(new IntElement(1024));
-    EXPECT_THROW(parser.extractUint8("foo", too_large),isc::OutOfRange);
 
-    // checks if extractInt can return the expected value
+    // getIntType checks bounds
-    ConstElementPtr hundred(new IntElement(100));
+    ElementPtr negative = Element::fromJSON("{ \"foo\": -1 }");
+    EXPECT_THROW(parser.getUint8(negative, "foo"), DhcpConfigError);
+    ElementPtr too_large = Element::fromJSON("{ \"foo\": 1024 }");
+    EXPECT_THROW(parser.getUint8(too_large, "foo"), DhcpConfigError);
+
+    // checks if getIntType can return the expected value
+    ElementPtr hundred = Element::fromJSON("{ \"foo\": 100 }");
     uint8_t val = 0;
-    EXPECT_NO_THROW(val = parser.extractUint8("foo", hundred));
+    EXPECT_NO_THROW(val = parser.getUint8(hundred, "foo"));
     EXPECT_EQ(100, val);
 }
 
-// This test exercises the extractConvert template
+// This test exercises the getAndConvert template
-TEST_F(SimpleParserTest, extractConvert) {
+TEST_F(SimpleParserTest, getAndConvert) {
 
     SimpleParserClassTest parser;
 
-    // extractConvert checks if it is a string
+    // getAndConvert checks it can be found
-    ConstElementPtr not_bool(new IntElement(1));
+    ElementPtr not_found = Element::fromJSON("{ \"bar\": \"true\" }");
-    EXPECT_THROW(parser.extractBool("foo", not_bool), TypeError);
+    EXPECT_THROW(parser.getAsBool(not_found, "foo"), DhcpConfigError);
 
-    // checks if extractConvert can return the expected value
+    // getAndConvert checks if it is a string
-    ConstElementPtr a_bool(new StringElement("false"));
+    ElementPtr not_bool = Element::fromJSON("{ \"foo\": 1 }");
+    EXPECT_THROW(parser.getAsBool(not_bool, "foo"), DhcpConfigError);
+
+    // checks if getAndConvert can return the expected value
+    ElementPtr a_bool = Element::fromJSON("{ \"foo\": \"false\" }");
     bool val = true;
-    EXPECT_NO_THROW(val = parser.extractBool("foo", a_bool));
+    EXPECT_NO_THROW(val = parser.getAsBool(a_bool, "foo"));
     EXPECT_FALSE(val);
 
-    // extractConvert checks convertion
+    // getAndConvert checks convertion
-    ConstElementPtr bad_bool(new StringElement("foo"));
+    ElementPtr bad_bool = Element::fromJSON("{ \"foo\": \"bar\" }");
-    EXPECT_THROW(parser.extractBool("bar", bad_bool), isc::BadValue);
+    EXPECT_THROW(parser.getAsBool(bad_bool, "bar"), DhcpConfigError);
 }
 

src/lib/dhcpsrv/parsers/dhcp_config_parser.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015,2017 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
@@ -9,6 +9,7 @@
 
 #include <exceptions/exceptions.h>
 #include <cc/data.h>
+#include <cc/dhcp_config_error.h>
 #include <stdint.h>
 #include <string>
 #include <map>
@@ -16,20 +17,6 @@
 namespace isc {
 namespace dhcp {
 
-/// An exception that is thrown if an error occurs while configuring
-/// DHCP server.
-class DhcpConfigError : public isc::Exception {
-public:
-
-    /// @brief constructor
-    ///
-    /// @param file name of the file, where exception occurred
-    /// @param line line of the file, where exception occurred
-    /// @param what text description of the issue that caused exception
-    DhcpConfigError(const char* file, size_t line, const char* what)
-        : isc::Exception(file, line, what) {}
-};
-
 /// @brief Forward declaration to DhcpConfigParser class.
 ///
 /// It is only needed here to define types that are

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1076,51 +1076,45 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
 //**************************** D2ClientConfigParser **********************
 
 uint32_t
-D2ClientConfigParser::getUint32(const std::string& name,
+D2ClientConfigParser::getUint32(ConstElementPtr scope,
-                                ConstElementPtr value) const {
+                                const std::string& name) {
-    return (extractInt<uint32_t, DhcpConfigError>(name, value));
+    return (getIntType<uint32_t>(scope, name));
 }
 
+// Can't use a constructor as a function
 namespace {
 IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); }
 };
 
 IOAddress
-D2ClientConfigParser::getIOAddress(const std::string& name,
+D2ClientConfigParser::getIOAddress(ConstElementPtr scope,
-                                   ConstElementPtr value) const {
+                                   const std::string& name) {
-    return (extractConvert<IOAddress,
+    return (getAndConvert<IOAddress,
-                           buildIOAddress,
+            buildIOAddress>(scope, name, "address"));
-                           DhcpConfigError>(name, "address", value));
 }
 
 dhcp_ddns::NameChangeProtocol
-D2ClientConfigParser::getProtocol(const std::string& name,
+D2ClientConfigParser::getProtocol(ConstElementPtr scope,
-                                  ConstElementPtr value) const {
+                                  const std::string& name) {
-    return (extractConvert<dhcp_ddns::NameChangeProtocol,
+    return (getAndConvert<dhcp_ddns::NameChangeProtocol,
-                           dhcp_ddns::stringToNcrProtocol,
+            dhcp_ddns::stringToNcrProtocol>
-                            DhcpConfigError>(name,
+            (scope, name, "NameChangeRequest protocol"));
-                                             "NameChangeRequest protocol",
-                                             value));
 }
 
 dhcp_ddns::NameChangeFormat
-D2ClientConfigParser::getFormat(const std::string& name,
+D2ClientConfigParser::getFormat(ConstElementPtr scope,
-                                ConstElementPtr value) const {
+                                const std::string& name) {
-    return (extractConvert<dhcp_ddns::NameChangeFormat,
+    return (getAndConvert<dhcp_ddns::NameChangeFormat,
-                           dhcp_ddns::stringToNcrFormat,
+            dhcp_ddns::stringToNcrFormat>
-                           DhcpConfigError>(name,
+            (scope, name, "NameChangeRequest format"));
-                                            "NameChangeRequest format",
-                                            value));
 }
 
 D2ClientConfig::ReplaceClientNameMode
-D2ClientConfigParser::getMode(const std::string& name,
+D2ClientConfigParser::getMode(ConstElementPtr scope,
-                              ConstElementPtr value) const {
+                              const std::string& name) {
-    return (extractConvert<D2ClientConfig::ReplaceClientNameMode,
+    return (getAndConvert<D2ClientConfig::ReplaceClientNameMode,
-                           D2ClientConfig::stringToReplaceClientNameMode,
+            D2ClientConfig::stringToReplaceClientNameMode>
-                           DhcpConfigError>(name,
+            (scope, name, "ReplaceClientName mode"));
-                                            "ReplaceClientName mode",
-                                            value));
 }
 
 D2ClientConfigPtr
@@ -1142,70 +1136,45 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
     // Get all parameters that are needed to create the D2ClientConfig.
     std::string qualifying_suffix;
     bool found_qualifying_suffix = false;
-    IOAddress server_ip(0);
+    if (client_config->contains("qualifying-suffix")) {
-    uint32_t server_port = 0;
+            qualifying_suffix = getString(client_config, "qualifying-suffix");
-    std::string sender_ip_str;
-    uint32_t sender_port = 0;
-    uint32_t max_queue_size = 1024;
-    dhcp_ddns::NameChangeProtocol ncr_protocol;
-    dhcp_ddns::NameChangeFormat ncr_format;
-    bool always_include_fqdn = false;
-    bool allow_client_update;
-    bool override_no_update = false;
-    bool override_client_update = false;
-    D2ClientConfig::ReplaceClientNameMode replace_client_name_mode =
-        D2ClientConfig::ReplaceClientNameMode::RCM_NEVER;
-    std::string generated_prefix;
-
-    BOOST_FOREACH(ConfigPair param, client_config->mapValue()) {
-        std::string entry(param.first);
-        ConstElementPtr value(param.second);
-        try {
-            if (entry == "enable-updates") {
-                // already done.
-            } else if (entry == "qualifying-suffix") {
-                qualifying_suffix = value->stringValue();
             found_qualifying_suffix = true;
-            } else if (entry == "server-ip") {
-                server_ip = getIOAddress("server-ip", value);
-            } else if (entry == "server-port") {
-                server_port = getUint32("server-port", value);
-            } else if (entry == "sender-ip") {
-                sender_ip_str = value->stringValue();
-            } else if (entry == "sender-port") {
-                sender_port = getUint32("sender-port", value);
-            } else if (entry == "max-queue-size") {
-                max_queue_size = getUint32("max-queue-size", value);
-            } else if (entry == "ncr-protocol") {
-                ncr_protocol = getProtocol("ncr-protocol", value);
-            } else if (entry == "ncr-format") {
-                ncr_format = getFormat("ncr-format", value);
-            } else if (entry == "always-include-fqdn") {
-                always_include_fqdn = value->boolValue();
-            } else if (entry == "allow-client-update") {
-                allow_client_update = value->boolValue();
-                // currently unused
-                (void)allow_client_update;
-            } else if (entry == "override-no-update") {
-                override_no_update = value->boolValue();
-            } else if (entry == "override-client-update") {
-                override_client_update = value->boolValue();
-            } else if (entry == "replace-client-name") {
-                replace_client_name_mode = getMode("replace-client-name", value);
-            } else if (entry == "generated-prefix") {
-                generated_prefix = value->stringValue();
-            } else {
-                isc_throw(DhcpConfigError,
-                          "unsupported parameter '" << entry
-                          << " (" << value->getPosition() << ")");
-            }
-        } catch (const isc::data::TypeError&) {
-            isc_throw(DhcpConfigError,
-                      "invalid value type specified for parameter '" << entry
-                      << " (" << value->getPosition() << ")");
-        }
     }   
 
+    IOAddress server_ip = getIOAddress(client_config, "server-ip");
+
+    uint32_t server_port = getUint32(client_config, "server-port");
+
+    std::string sender_ip_str = getString(client_config, "sender-ip");
+
+    uint32_t sender_port = getUint32(client_config, "sender-port"); 
+
+    uint32_t max_queue_size = getUint32(client_config, "max-queue-size"); 
+
+    dhcp_ddns::NameChangeProtocol ncr_protocol =
+        getProtocol(client_config, "ncr-protocol");
+
+    dhcp_ddns::NameChangeFormat ncr_format =
+        getFormat(client_config, "ncr-format");
+
+    bool always_include_fqdn =
+        getBoolean(client_config, "always-include-fqdn");
+
+    // bool allow_client_update; (unused)
+
+    bool override_no_update =
+        getBoolean(client_config, "override-no-update");
+
+    bool override_client_update =
+        getBoolean(client_config, "override-client-update");
+
+    D2ClientConfig::ReplaceClientNameMode replace_client_name_mode =
+        getMode(client_config, "replace-client-name");
+
+    std::string generated_prefix =
+        getString(client_config, "generated-prefix");
+
+
     // Qualifying-suffix is required when updates are enabled
     if (enable_updates && !found_qualifying_suffix) {
         isc_throw(DhcpConfigError,
@@ -1291,18 +1260,8 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
 
 bool
 D2ClientConfigParser::isShortCutDisabled(isc::data::ConstElementPtr d2_config) {
-    if (!d2_config->contains("enable-updates")) {
+    bool value = getBoolean(d2_config, "enable-updates");
-        isc_throw(DhcpConfigError,
+    return (!value && (d2_config->mapValue().size() == 1));
-                  "Mandatory parameter 'enable-updates' missing ("
-                  << d2_config->getPosition() << ")");
-    }
-    ConstElementPtr enable = d2_config->get("enable-updates");
-    if (enable->getType() != Element::boolean) {
-        isc_throw(DhcpConfigError,
-                  "invalid value type specified for parameter"
-                  " 'enable-updates' (" << enable->getPosition() << ")");
-    }
-    return (!enable->boolValue() && (d2_config->mapValue().size() == 1));
 }
 
 /// @brief This table defines default values for D2 client configuration

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -893,52 +893,53 @@ private:
 
     /// @brief Returns a value converted to uint32_t
     ///
-    /// Instantiation of extractInt() to uint32_t
+    /// Instantiation of getIntType() to uint32_t
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return an uint32_t value
     uint32_t
-    getUint32(const std::string& name, isc::data::ConstElementPtr value) const;
+    getUint32(isc::data::ConstElementPtr scope, const std::string& name);
 
     /// @brief Returns a value converted to IOAddress
     ///
-    /// Instantiation of extractConvert() to IOAddress
+    /// Instantiation of getAndConvert() to IOAddress
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return an IOAddress value
     isc::asiolink::IOAddress
-    getIOAddress(const std::string& name,
+    getIOAddress(isc::data::ConstElementPtr scope, const std::string& name);
-                 isc::data::ConstElementPtr value) const;
 
     /// @brief Returns a value converted to NameChangeProtocol
     ///
-    /// Instantiation of extractInt() to NameChangeProtocol
+    /// Instantiation of getAndConvert() to NameChangeProtocol
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return a NameChangeProtocol value
     dhcp_ddns::NameChangeProtocol
-    getProtocol(const std::string& name,
+    getProtocol(isc::data::ConstElementPtr scope, const std::string& name);
-                isc::data::ConstElementPtr value) const;
 
     /// @brief Returns a value converted to NameChangeFormat
     ///
-    /// Instantiation of extractConvert() to NameChangeFormat
+    /// Instantiation of getAndConvert() to NameChangeFormat
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return a NameChangeFormat value
     dhcp_ddns::NameChangeFormat
-    getFormat(const std::string& name,
+    getFormat(isc::data::ConstElementPtr scope, const std::string& name);
-              isc::data::ConstElementPtr value) const;
 
     /// @brief Returns a value converted to ReplaceClientNameMode
     ///
-    /// Instantiation of extractConvert() to ReplaceClientNameMode
+    /// Instantiation of getAndConvert() to ReplaceClientNameMode
     ///
-    /// @param value value of the parameter
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
     /// @return a NameChangeFormat value
     D2ClientConfig::ReplaceClientNameMode
-    getMode(const std::string& name,
+    getMode(isc::data::ConstElementPtr scope, const std::string& name);
-            isc::data::ConstElementPtr value) const;
 };
 
 }; // end of isc::dhcp namespace