[5123] Moved DhcpConfigError, added new templates, updated simpleparser code

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,33 @@
+// 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.
+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"
-                  << "(" << scope->getPosition() << ")");
+        isc_throw(DhcpConfigError,
+                  "missing parameter '" << name << "' ("
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::string) {
-        isc_throw(BadValue, "Element " << name << " found, but is not a string"
-                  << "(" << x->getPosition() << ")");
+        isc_throw(DhcpConfigError,
+                  "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 "
-                  << "(" << scope->getPosition() << ")");
+        isc_throw(DhcpConfigError,
+                  "missing parameter '" << name << "' ("
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::integer) {
-        isc_throw(BadValue, "Element " << name << " found, but is not an integer"
-                  << "(" << x->getPosition() << ")");
+        isc_throw(DhcpConfigError,
+                  "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 "
-                  << "(" << scope->getPosition() << ")");
-
+        isc_throw(DhcpConfigError,
+                  "missing parameter '" << name << "' ("
+                  << scope->getPosition() << ")");
     }
     if (x->getType() != Element::boolean) {
-        isc_throw(BadValue, "Element " << name << " found, but is not a boolean"
-                  << "(" << x->getPosition() << ")");
+        isc_throw(DhcpConfigError,
+                  "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>
@@ -148,6 +149,59 @@ protected:
     static bool getBoolean(isc::data::ConstElementPtr scope,
                            const std::string& name);
 
+    /// @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
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter for error report
+    /// @return a value of int_type
+    template <typename int_type> int_type
+    getIntType(isc::data::ConstElementPtr scope,
+               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(isc::dhcp::DhcpConfigError,
+                    "out of range value (" << val_int
+                    << ") specified for parameter '" << name
+                    << "' (" << getPosition(name, scope) << ")");
+        }
+        return (static_cast<int_type>(val_int));
+    }
+
+    /// @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
+    /// @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 exception_type when the value cannot be converted
+    template <typename target_type,
+              target_type convert(const std::string&)> target_type
+    getAndConvert(isc::data::ConstElementPtr scope,
+                  const std::string& name,
+                  const std::string& type_name) {
+        std::string str = getString(scope, name);
+        try {
+            return (convert(str));
+        } catch (const std::exception&) {
+            isc_throw(isc::dhcp::DhcpConfigError,
+                      "invalid " << type_name << " (" << str
+                      << ") specified for parameter '" << name
+                      << "' (" << getPosition(name, scope) << ")");
+        }
+    }
+
+    /// @todo remove this when they'll be no longer used.
+
     /// @brief Returns an integer value with range checking
     ///
     /// This template should be instantied in parsers when useful

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 {
-        return (extractInt<uint8_t, isc::OutOfRange>(name, value));
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
+        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 {
-        return (extractConvert<bool, toBool, isc::BadValue>
-                    (name, "boolean", value));
+    bool getAsBool(ConstElementPtr scope, const std::string& name) {
+        return (getAndConvert<bool, toBool>(scope, name, "boolean"));
     }
 
     /// @brief Convert to boolean
@@ -169,45 +169,53 @@ TEST_F(SimpleParserTest, setListDefaults) {
     checkIntegerValue(third, "renew-timer", 900);
 }
 
-// This test exercises the extractInt template
-TEST_F(SimpleParserTest, extractInt) {
+// This test exercises the getIntType template
+TEST_F(SimpleParserTest, getIntType) {
 
     SimpleParserClassTest parser;
 
-    // extractInt checks if it is an integer
-    ConstElementPtr not_int(new StringElement("xyz"));
-    EXPECT_THROW(parser.extractUint8("foo", not_int), TypeError);
+    // getIntType checks it can be found
+    ElementPtr not_found = Element::fromJSON("{ \"bar\": 1 }");
+    EXPECT_THROW(parser.getUint8(not_found, "foo"), DhcpConfigError);
 
-    // extractInt checks bounds
-    ConstElementPtr negative(new IntElement(-1));
-    EXPECT_THROW(parser.extractUint8("foo", negative), isc::OutOfRange);
-    ConstElementPtr too_large(new IntElement(1024));
-    EXPECT_THROW(parser.extractUint8("foo", too_large),isc::OutOfRange);
+    // getIntType checks if it is an integer
+    ElementPtr not_int = Element::fromJSON("{ \"foo\": \"xyz\" }");
+    EXPECT_THROW(parser.getUint8(not_int, "foo"), DhcpConfigError);
 
-    // checks if extractInt can return the expected value
-    ConstElementPtr hundred(new IntElement(100));
+    // getIntType checks bounds
+    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
-TEST_F(SimpleParserTest, extractConvert) {
+// This test exercises the getAndConvert template
+TEST_F(SimpleParserTest, getAndConvert) {
 
     SimpleParserClassTest parser;
 
-    // extractConvert checks if it is a string
-    ConstElementPtr not_bool(new IntElement(1));
-    EXPECT_THROW(parser.extractBool("foo", not_bool), TypeError);
+    // getAndConvert checks it can be found
+    ElementPtr not_found = Element::fromJSON("{ \"bar\": \"true\" }");
+    EXPECT_THROW(parser.getAsBool(not_found, "foo"), DhcpConfigError);
+
+    // getAndConvert checks if it is a string
+    ElementPtr not_bool = Element::fromJSON("{ \"foo\": 1 }");
+    EXPECT_THROW(parser.getAsBool(not_bool, "foo"), DhcpConfigError);
 
-    // checks if extractConvert can return the expected value
-    ConstElementPtr a_bool(new StringElement("false"));
+    // 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
-    ConstElementPtr bad_bool(new StringElement("foo"));
-    EXPECT_THROW(parser.extractBool("bar", bad_bool), isc::BadValue);
+    // getAndConvert checks convertion
+    ElementPtr bad_bool = Element::fromJSON("{ \"foo\": \"bar\" }");
+    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