[trac2355] - Added basic parsers unit tests in new file,

dhcp_parsers_unittest.cc.  Added Element type checks to
Boolean and String parsers, corrected some commentary.

src/lib/dhcpsrv/dhcp_parsers.cc

@@ -76,8 +76,8 @@ void DebugParser::commit() {
 // **************************** BooleanParser  *************************
 
 BooleanParser::BooleanParser(const std::string& param_name, 
-                            BooleanStoragePtr storage_)
-    : storage_(storage_), param_name_(param_name), value_(false) {
+                            BooleanStoragePtr storage)
+    : storage_(storage), param_name_(param_name), value_(false) {
     // Empty parameter name is invalid.
     if (param_name_.empty()) {
         isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:"
@@ -94,9 +94,13 @@ BooleanParser::BooleanParser(const std::string& param_name,
 void BooleanParser::build(ConstElementPtr value) {
     // The Config Manager checks if user specified a
     // valid value for a boolean parameter: True or False.
-    // It is then ok to assume that if str() does not return
-    // 'true' the value is 'false'.
-    value_ = (value->str() == "true") ? true : false;
+    // We should have a boolean Element, use value directly
+    try {
+        value_ = value->boolValue();
+    } catch (const isc::data::TypeError &) {
+        isc_throw(BadValue, " Wrong value type for " << param_name_ 
+                  << " : build called with a non-boolean element.");
+    }
 }
 
 void BooleanParser::commit() {
@@ -137,7 +141,7 @@ void Uint32Parser::build(ConstElementPtr value) {
     }
     if (check < 0) {
         isc_throw(BadValue, "Value " << value->str() << "is negative."
-                  << " Only 0 or larger are allowed for unsigned 32-bit integer.");
+               << " Only 0 or larger are allowed for unsigned 32-bit integer.");
     }
 
     // value is small enough to fit
@@ -152,7 +156,8 @@ void Uint32Parser::commit() {
 
 // **************************** StringParser  *************************
 
-StringParser::StringParser(const std::string& param_name, StringStoragePtr storage)
+StringParser::StringParser(const std::string& param_name, 
+                           StringStoragePtr storage)
     :storage_(storage), param_name_(param_name) {
     // Empty parameter name is invalid.
     if (param_name_.empty()) {
@@ -168,8 +173,18 @@ StringParser::StringParser(const std::string& param_name, StringStoragePtr stora
 }
 
 void StringParser::build(ConstElementPtr value) {
+#if 0
     value_ = value->str();
     boost::erase_all(value_, "\"");
+#else
+    try {
+        value_ = value->stringValue();
+        boost::erase_all(value_, "\"");
+    } catch (const isc::data::TypeError &) {
+        isc_throw(BadValue, " Wrong value type for " << param_name_ 
+                  << " : build called with a non-boolean element.");
+    }
+#endif
 }
 
 void StringParser::commit() {
@@ -178,9 +193,10 @@ void StringParser::commit() {
     storage_->setParam(param_name_, value_);
 }
 
-// **************************** InterfaceListConfigParser *************************
+// ******************** InterfaceListConfigParser *************************
 
-InterfaceListConfigParser::InterfaceListConfigParser(const std::string& param_name) {
+InterfaceListConfigParser::InterfaceListConfigParser(const std::string& 
+                                                     param_name) {
     if (param_name != "interface") {
         isc_throw(BadValue, "Internal error. Interface configuration "
             "parser called for the wrong parameter: " << param_name);

src/lib/dhcpsrv/dhcp_parsers.h

@@ -136,6 +136,8 @@ public:
     /// @param param_name name of the parameter.
     /// @param storage is a pointer to the storage container where the parsed
     /// value be stored upon commit.
+    /// @throw isc::dhcp::DhcpConfigError if a provided parameter's
+    /// name is empty.
     /// @throw isc::dhcp::DhcpConfigError if storage is null.
     BooleanParser(const std::string& param_name, BooleanStoragePtr storage);
 
@@ -143,10 +145,7 @@ public:
     ///
     /// @param value a value to be parsed.
     ///
-    /// @throw isc::InvalidOperation if a storage has not been set
-    ///        prior to calling this function
-    /// @throw isc::dhcp::DhcpConfigError if a provided parameter's
-    ///        name is empty.
+    /// @throw isc::BadValue if value is not a boolean type Element.
     virtual void build(isc::data::ConstElementPtr value);
 
     /// @brief Put a parsed value to the storage.
@@ -180,6 +179,8 @@ public:
     /// @param param_name name of the configuration parameter being parsed
     /// @param storage is a pointer to the storage container where the parsed
     /// value be stored upon commit.
+    /// @throw isc::dhcp::DhcpConfigError if a provided parameter's
+    /// name is empty.
     /// @throw isc::dhcp::DhcpConfigError if storage is null.
     Uint32Parser(const std::string& param_name, Uint32StoragePtr storage);
 
@@ -222,6 +223,8 @@ public:
     /// @param param_name name of the configuration parameter being parsed
     /// @param storage is a pointer to the storage container where the parsed
     /// value be stored upon commit.
+    /// @throw isc::dhcp::DhcpConfigError if a provided parameter's
+    /// name is empty.
     /// @throw isc::dhcp::DhcpConfigError if storage is null.
     StringParser(const std::string& param_name, StringStoragePtr storage);
 
@@ -231,6 +234,7 @@ public:
     /// @ref setStorage() for details.
     ///
     /// @param value pointer to the content of parsed values
+    /// @throw isc::BadValue if value is not a string type Element.
     virtual void build(isc::data::ConstElementPtr value);
 
     /// @brief Stores the parsed value in a storage.

src/lib/dhcpsrv/tests/Makefile.am

@@ -34,6 +34,7 @@ libdhcpsrv_unittests_SOURCES += dbaccess_parser_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_mgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc
+libdhcpsrv_unittests_SOURCES += dhcp_parsers_unittest.cc
 if HAVE_MYSQL
 libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc
 endif

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+
+#include <config/ccsession.h>
+#include <dhcp/option.h>
+#include <dhcp/option_custom.h>
+#include <dhcp/option_int.h>
+#include <dhcpsrv/subnet.h>
+#include <dhcpsrv/dhcp_parsers.h>
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+#include <boost/foreach.hpp>
+
+#include <map>
+#include <string>
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::data;
+using namespace isc::config;
+
+namespace {
+
+/// @brief DHCP Parser test fixture class
+class DhcpParserTest : public ::testing::Test {
+public:
+    /// @brief Constructor
+    ///
+    DhcpParserTest() {
+    }
+};
+
+
+/// @brief Check BooleanParser basic functionality.
+/// 
+/// Verifies that the parser:
+/// 1. Does not allow empty for storage.
+/// 2. Rejects a non-boolean element.
+/// 3. Builds with a valid true value.
+/// 4. Bbuils with a valid false value.
+/// 5. Updates storage upon commit.
+TEST_F(DhcpParserTest, booleanParserTest) {
+
+    const std::string name = "boolParm";
+
+    // Verify that parser does not allow empty for storage.
+    BooleanStoragePtr bs;
+    EXPECT_THROW(BooleanParser(name, bs), isc::dhcp::DhcpConfigError);
+
+    // Construct parser for testing.
+    BooleanStoragePtr storage(new BooleanStorage());
+    BooleanParser parser(name, storage);
+
+    // Verify that parser with rejects a non-boolean element.
+    ElementPtr wrong_element = Element::create("I am a string");
+    EXPECT_THROW(parser.build(wrong_element), isc::BadValue);
+
+    // Verify that parser will build with a valid true value.
+    bool test_value = true;
+    ElementPtr element = Element::create(test_value);
+    ASSERT_NO_THROW(parser.build(element));
+
+    // Verify that commit updates storage.
+    bool actual_value = ~test_value;
+    parser.commit();
+    EXPECT_NO_THROW((actual_value = storage->getParam(name)));
+    EXPECT_EQ(test_value, actual_value);
+
+    // Verify that parser will build with a valid false value.
+    test_value = false;
+    element->setValue(test_value);
+    EXPECT_NO_THROW(parser.build(element));
+
+    // Verify that commit updates storage.
+    actual_value = ~test_value;
+    parser.commit();
+    EXPECT_NO_THROW((actual_value = storage->getParam(name)));
+    EXPECT_EQ(test_value, actual_value);
+}
+
+/// @brief Check StringParser basic functionality
+/// 
+/// Verifies that the parser:
+/// 1. Does not allow empty for storage.
+/// 2. Rejects a non-string element.
+/// 3. Builds with a string value.
+/// 4. Updates storage upon commit.
+TEST_F(DhcpParserTest, stringParserTest) {
+
+    const std::string name = "strParm";
+
+    // Verify that parser does not allow empty for storage.
+    StringStoragePtr bs;
+    EXPECT_THROW(StringParser(name, bs), isc::dhcp::DhcpConfigError);
+
+    // Construct parser for testing.
+    StringStoragePtr storage(new StringStorage());
+    StringParser parser(name, storage);
+
+    // Verify that parser with rejects a non-string element.
+    ElementPtr wrong_element = Element::create(9999);
+    EXPECT_THROW(parser.build(wrong_element), isc::BadValue);
+
+    // Verify that parser will build with a string value.
+    const std::string test_value = "test value";
+    ElementPtr element = Element::create(test_value);
+    ASSERT_NO_THROW(parser.build(element));
+
+    // Verify that commit updates storage.
+    parser.commit();
+    std::string actual_value;
+    EXPECT_NO_THROW((actual_value = storage->getParam(name)));
+    EXPECT_EQ(test_value, actual_value);
+}
+
+/// @brief Check Uint32Parser basic functionality
+/// 
+/// Verifies that the parser:
+/// 1. Does not allow empty for storage.
+/// 2. Rejects a non-integer element.
+/// 3. Rejects a negative value.
+/// 4. Rejects too large a value.
+/// 5. Builds with value of zero.
+/// 6. Builds with a value greater than zero.
+/// 7. Updates storage upon commit.
+TEST_F(DhcpParserTest, uint32ParserTest) {
+
+    const std::string name = "intParm";
+
+    // Verify that parser does not allow empty for storage.
+    Uint32StoragePtr bs;
+    EXPECT_THROW(Uint32Parser(name, bs), isc::dhcp::DhcpConfigError);
+
+    // Construct parser for testing.
+    Uint32StoragePtr storage(new Uint32Storage());
+    Uint32Parser parser(name, storage);
+
+    // Verify that parser with rejects a non-interger element.
+    ElementPtr wrong_element = Element::create("I am a string");
+    EXPECT_THROW(parser.build(wrong_element), isc::BadValue);
+
+    // Verify that parser with rejects a negative value.
+    ElementPtr int_element = Element::create(-1);
+    EXPECT_THROW(parser.build(int_element), isc::BadValue);
+
+    // Verify that parser with rejects too large a value.
+    long max = (long)(std::numeric_limits<uint32_t>::max()) + 1;
+    int_element->setValue((long)(max));
+    EXPECT_THROW(parser.build(int_element), isc::BadValue);
+
+    // Verify that parser will build with value of zero.
+    int test_value = 0;
+    int_element->setValue((long)test_value);
+    ASSERT_NO_THROW(parser.build(int_element));
+
+    // Verify that parser will build with a valid positive value.
+    test_value = 77;
+    int_element->setValue((long)test_value);
+    ASSERT_NO_THROW(parser.build(int_element));
+
+    // Verify that commit updates storage.
+    parser.commit();
+    uint32_t actual_value = 0;
+    EXPECT_NO_THROW((actual_value = storage->getParam(name)));
+    EXPECT_EQ(test_value, actual_value);
+}
+
+/// @brief Check InterfaceListParser  basic functionality
+///
+/// Verifies that the parser:
+/// 1. Does not allow empty for storage.
+/// 2. Does not allow name other than "interface"
+///
+/// InterfaceListParser doesn't do very much, this test will need to 
+/// expand once it does.
+TEST_F(DhcpParserTest, interfaceListParserTest) {
+
+    const std::string name = "interface";
+
+    // Verify that parser constructor fails if parameter name isn't "interface"
+    EXPECT_THROW(InterfaceListConfigParser("bogus_name"), isc::BadValue);
+
+    InterfaceListConfigParser parser(name);
+    ElementPtr list_element = Element::createList();
+    list_element->add(Element::create("eth0"));
+    list_element->add(Element::create("eth1"));
+}
+
+/// @brief Test Implementation of abstract OptionDataParser class. Allows 
+/// testing basic option parsing.   
+class UtestOptionDataParser : public OptionDataParser {
+public:
+
+    UtestOptionDataParser(const std::string&, 
+        OptionStoragePtr options, ParserContextPtr global_context) 
+        :OptionDataParser("", options, global_context) {
+    }
+
+    static OptionDataParser* factory(const std::string& param_name,
+        OptionStoragePtr options, ParserContextPtr global_context) {
+        return (new UtestOptionDataParser(param_name, options, global_context));
+    }
+
+protected:
+    // Dummy out last two params since test derivation doesn't use them.
+    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
+                std::string&, uint32_t) {
+        OptionDefinitionPtr def;
+        // always return empty 
+        return (def);
+    }
+};
+
+/// @brief Test Fixture class which provides basic structure for testing 
+/// configuration parsing.  This is essentially the same structure provided
+/// by dhcp servers.
+class ParseConfigTest : public ::testing::Test {
+public:
+    /// @brief Constructor
+    ParseConfigTest() {
+        reset_context();
+    }
+
+    /// @brief Parses a configuration.   
+    ///
+    /// Parse the given configuration, populating the context storage with
+    /// the parsed elements.  
+    /// 
+    /// @param config_set is the set of elements to parse.
+    /// @return returns an ConstElementPtr containing the numeric result
+    /// code and outcome comment.
+    isc::data::ConstElementPtr parseElementSet(isc::data::ConstElementPtr 
+                                           config_set) {
+        // Answer will hold the result.
+        ConstElementPtr answer;
+        if (!config_set) {
+            answer = isc::config::createAnswer(1,
+                                 string("Can't parse NULL config"));
+            return (answer);
+        }
+
+        // option parsing must be done last, so save it if we hit if first
+        ParserPtr option_parser;
+
+        ConfigPair config_pair;
+        try {
+            // Iteraate over the config elements.
+            const std::map<std::string, ConstElementPtr>& values_map =
+                                                      config_set->mapValue();
+            BOOST_FOREACH(config_pair, values_map) {
+                // Create the parser based on element name.
+                ParserPtr parser(createConfigParser(config_pair.first));
+                // Options must be parsed last
+                if (config_pair.first == "option-data") {
+                    option_parser = parser;
+                } else {
+                    // Anything else  we can call build straight away.
+                    parser->build(config_pair.second);
+                    parser->commit();
+                }
+            }
+
+            // 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();
+            }
+
+            // Everything was fine. Configuration is successful.
+            answer = isc::config::createAnswer(0, "Configuration committed.");
+        } catch (const isc::Exception& ex) {
+            answer = isc::config::createAnswer(1,
+                        string("Configuration parsing failed: ") + ex.what());
+
+        } catch (...) {
+            answer = isc::config::createAnswer(1,
+                                        string("Configuration parsing failed"));
+        }
+
+        return (answer);
+    }
+
+    /// @brief Create an element parser based on the element name.
+    ///
+    /// Note that currently it only supports option-defs and option-data, 
+    /// 
+    /// @param config_id is the name of the configuration element. 
+    /// @return returns a raw pointer to DhcpConfigParser. Note caller is
+    /// responsible for deleting it once no longer needed.
+    /// @throw throws NotImplemented if element name isn't supported.
+    DhcpConfigParser* createConfigParser(const std::string& config_id) {
+        DhcpConfigParser* parser = NULL;
+        if (config_id.compare("option-data") == 0) {
+            parser = new OptionDataListParser(config_id, 
+                                          parser_context_->options_, 
+                                          parser_context_,
+                                          UtestOptionDataParser::factory);
+        } else if (config_id.compare("option-def") == 0) {
+            parser  = new OptionDefListParser(config_id, 
+                                          parser_context_->option_defs_);
+        } else {
+            isc_throw(NotImplemented,
+                "Parser error: configuration parameter not supported: "
+                << config_id);
+        }
+
+        return (parser);
+    }
+
+    /// @brief Convenicee method for parsing a configuration 
+    /// 
+    /// Given a configuration string, convert it into Elements
+    /// and parse them. 
+    /// @param config is the configuration string to parse
+    ///
+    /// @return retuns 0 if the configuration parsed successfully, 
+    /// non-zero otherwise failure.
+    int parseConfiguration (std::string &config) {    
+        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) {
+            ConstElementPtr status = parseElementSet(json);
+            ConstElementPtr comment_ = parseAnswer(rcode_, status);
+        }
+
+        return (rcode_);
+    }
+
+    /// @brief Find an option definition for a given space and code within 
+    /// the parser context.
+    /// @param space is the space name of the desired option.
+    /// @param code is the numeric "type" of the desired option.
+    /// @return returns an OptionDefinitionPtr which points to the found
+    /// definition or is empty.
+    /// ASSERT_ tests don't work inside functions that return values 
+    OptionDefinitionPtr getOptionDef(std::string space, uint32_t code)
+    {
+        OptionDefinitionPtr def;
+        OptionDefContainerPtr defs = 
+                            parser_context_->option_defs_->getItems(space);
+        // Should always be able to get definitions list even if it is empty.
+        EXPECT_TRUE(defs);
+        if (defs) {
+            // Attempt to find desired definiton.
+            const OptionDefContainerTypeIndex& idx = defs->get<1>();
+            const OptionDefContainerTypeRange& range = idx.equal_range(code);
+            int cnt = std::distance(range.first, range.second);
+            EXPECT_EQ(1, cnt);
+            if (cnt == 1) {
+                def = *(idx.begin());
+            }
+        }
+        return (def); 
+    }
+
+    /// @brief Find an option for a given space and code within the parser 
+    /// context.
+    /// @param space is the space name of the desired option.
+    /// @param code is the numeric "type" of the desired option.
+    /// @return returns an OptionPtr which points to the found
+    /// option or is empty.
+    /// ASSERT_ tests don't work inside functions that return values 
+    OptionPtr getOptionPtr(std::string space, uint32_t code)
+    {
+        OptionPtr option_ptr;
+        Subnet::OptionContainerPtr options = 
+                            parser_context_->options_->getItems(space);
+        // Should always be able to get options list even if it is empty.
+        EXPECT_TRUE(options);
+        if (options) {
+            // Attempt to find desired option.
+            const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
+            const Subnet::OptionContainerTypeRange& range = 
+                                                        idx.equal_range(code);
+            int cnt = std::distance(range.first, range.second);
+            EXPECT_EQ(1, cnt);
+            if (cnt == 1) {
+                Subnet::OptionDescriptor desc = *(idx.begin()); 
+                option_ptr = desc.option; 
+                EXPECT_TRUE(option_ptr);
+            }
+        }
+
+        return (option_ptr); 
+    }
+
+    /// @brief Wipes the contents of the context to allowing another parsing 
+    /// during a given test if needed.
+    void reset_context(){
+        // Note set context universe to V6 as it has to be something.
+        parser_context_.reset(new ParserContext(Option::V6));
+    }
+
+    /// @brief Parser context - provides storage for options and definitions
+    ParserContextPtr parser_context_;
+};
+
+/// @brief Check Basic parsing of option definitions.
+/// 
+/// Note that this tests basic operation of the OptionDefinitionListParser and
+/// OptionDefinitionParser.  It uses a simple configuration consisting of one
+/// one definition and verifies that it is parsed and committed to storage
+/// correctly.
+TEST_F(ParseConfigTest, basicOptionDefTest) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\","
+        "      \"encapsulate\": \"\""
+        "  } ]"
+        "}";
+
+    // Verify that the configuration string parses.
+    int rcode = parseConfiguration(config);
+    ASSERT_TRUE(rcode == 0);
+
+    // Verify that the option definition can be retrieved.
+    OptionDefinitionPtr def = getOptionDef("isc", 100); 
+    ASSERT_TRUE(def);
+
+    // Verify that the option definition is correct.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_FALSE(def->getArrayType());
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, def->getType());
+    EXPECT_TRUE(def->getEncapsulatedSpace().empty());
+}
+
+/// @brief Check Basic parsing of options.
+/// 
+/// Note that this tests basic operation of the OptionDataListParser and
+/// OptionDataParser.  It uses a simple configuration consisting of one
+/// one definition and matching option data.  It verifies that the option
+/// is parsed and committed to storage correctly.
+TEST_F(ParseConfigTest, basicOptionDataTest) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\","
+        "      \"encapsulate\": \"\""
+        " } ], "
+        " \"option-data\": [ {"
+        "    \"name\": \"foo\","
+        "    \"space\": \"isc\","
+        "    \"code\": 100,"
+        "    \"data\": \"192.168.2.1\","
+        "    \"csv-format\": True"
+        " } ]"