[3400] Several improvements in JSON backend:

 - file read can now ignore # comments in file
 - there are JSON specific unit-tests now in src/bin/dhcp6
 - BIND10 backend specific tests moved to separate file

src/bin/dhcp6/ctrl_json_dhcp6_srv.cc

@@ -64,7 +64,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) {
 
     try {
         // Read contents of the file
-        string config = readFile(file_name);
+        string config = readFile(file_name, true);
 
         // Try to parse it as JSON
         json = Element::fromJSON(config);

src/bin/dhcp6/main.cc

@@ -39,13 +39,13 @@ using namespace std;
 namespace {
 const char* const DHCP6_NAME = "b10-dhcp6";
 
-const char* const DHCP6_LOGGER_NAME = "bind10";
+const char* const DHCP6_LOGGER_NAME = "kea";
 
 void
 usage() {
     cerr << "Usage: " << DHCP6_NAME << " [-v] [-s] [-p port_number] [-c cfgfile]" << endl;
     cerr << "  -v: verbose output" << endl;
-    cerr << "  -s: stand-alone mode (don't connect to BIND10)" << endl;
+    cerr << "  -s: skip configuration (don't connect to BIND10 or don't read config file)" << endl;
     cerr << "  -p number: specify non-standard port number 1-65535 "
          << "(useful for testing only)" << endl;
     cerr << "  -c file: specify configuration file" << endl;

src/bin/dhcp6/tests/Makefile.am

@@ -85,12 +85,13 @@ dhcp6_unittests_SOURCES += rebind_unittest.cc
 
 if CONFIG_BACKEND_BIND10
 dhcp6_unittests_SOURCES += ../json_config_parser.cc ../json_config_parser.h ../ctrl_bind10_dhcp6_srv.cc
-dhcp6_unittests_SOURCES += ctrl_dhcp6_srv_unittest.cc
+dhcp6_unittests_SOURCES += ctrl_bind10_dhcp6_srv_unittest.cc
 dhcp6_unittests_SOURCES += config_parser_unittest.cc
 endif
 
 if CONFIG_BACKEND_JSON
 dhcp6_unittests_SOURCES += ../json_config_parser.cc ../json_config_parser.h ../ctrl_json_dhcp6_srv.cc
+dhcp6_unittests_SOURCES += ctrl_json_dhcp6_srv_unittest.cc
 dhcp6_unittests_SOURCES += config_parser_unittest.cc
 endif
 

src/bin/dhcp6/tests/ctrl_json_dhcp6_srv_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/dhcp6.h>
+#include <dhcp6/ctrl_dhcp6_srv.h>
+#include <dhcpsrv/cfgmgr.h>
+
+#include <boost/scoped_ptr.hpp>
+#include <gtest/gtest.h>
+
+#include <fstream>
+#include <iostream>
+#include <sstream>
+
+#include <arpa/inet.h>
+#include <unistd.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::config;
+using namespace isc::data;
+using namespace isc::dhcp;
+using namespace isc::hooks;
+
+namespace {
+
+class NakedControlledDhcpv6Srv: public ControlledDhcpv6Srv {
+    // "Naked" DHCPv6 server, exposes internal fields
+public:
+    NakedControlledDhcpv6Srv():ControlledDhcpv6Srv(0) { }
+};
+
+
+class ControlledJSONDhcpv6SrvTest : public ::testing::Test {
+public:
+    ControlledJSONDhcpv6SrvTest() {
+    }
+
+    ~ControlledJSONDhcpv6SrvTest() {
+    };
+
+    void writeFile(const std::string& file_name, const std::string& content) {
+        static_cast<void>(unlink(file_name.c_str()));
+
+        ofstream out(file_name.c_str(), ios::trunc);
+        EXPECT_TRUE(out.is_open());
+        out << content;
+        out.close();
+    }
+
+    static const char* TEST_FILE;
+};
+
+const char* ControlledJSONDhcpv6SrvTest::TEST_FILE = "test-config.json";
+
+// This test checks if configuration can be read from a JSON file.
+TEST_F(ControlledJSONDhcpv6SrvTest, jsonFile) {
+
+    // Prepare configuration file.
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::/80\" ],"
+        "    \"subnet\": \"2001:db8:1::/64\" "
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:2::/80\" ],"
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 0"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:3::/80\" ],"
+        "    \"subnet\": \"2001:db8:3::/64\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+    writeFile(TEST_FILE, config);
+
+    // Now initialize the server
+    boost::scoped_ptr<ControlledDhcpv6Srv> srv;
+    ASSERT_NO_THROW(
+        srv.reset(new ControlledDhcpv6Srv(0))
+    );
+
+    // And configure it using the config file.
+    EXPECT_TRUE(srv->init(TEST_FILE));
+
+    // Now check if the configuration has been applied correctly.
+    const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
+    ASSERT_TRUE(subnets);
+    ASSERT_EQ(3, subnets->size()); // We expect 3 subnets.
+
+
+    // Check subnet 1.
+    EXPECT_EQ("2001:db8:1::", subnets->at(0)->get().first.toText());
+    EXPECT_EQ(64, subnets->at(0)->get().second);
+
+    // Check pools in the first subnet.
+    const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_NA);
+    ASSERT_EQ(1, pools1.size());
+    EXPECT_EQ("2001:db8:1::", pools1.at(0)->getFirstAddress().toText());
+    EXPECT_EQ("2001:db8:1::ffff:ffff:ffff", pools1.at(0)->getLastAddress().toText());
+    EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType());
+
+    // Check subnet 2.
+    EXPECT_EQ("2001:db8:2::", subnets->at(1)->get().first.toText());
+    EXPECT_EQ(64, subnets->at(1)->get().second);
+
+    // Check pools in the second subnet.
+    const PoolCollection& pools2 = subnets->at(1)->getPools(Lease::TYPE_NA);
+    ASSERT_EQ(1, pools2.size());
+    EXPECT_EQ("2001:db8:2::", pools2.at(0)->getFirstAddress().toText());
+    EXPECT_EQ("2001:db8:2::ffff:ffff:ffff", pools2.at(0)->getLastAddress().toText());
+    EXPECT_EQ(Lease::TYPE_NA, pools2.at(0)->getType());
+
+    // And finally check subnet 3.
+    EXPECT_EQ("2001:db8:3::", subnets->at(2)->get().first.toText());
+    EXPECT_EQ(64, subnets->at(2)->get().second);
+
+    // ... and it's only pool.
+    const PoolCollection& pools3 = subnets->at(2)->getPools(Lease::TYPE_NA);
+    EXPECT_EQ("2001:db8:3::", pools3.at(0)->getFirstAddress().toText());
+    EXPECT_EQ("2001:db8:3::ffff:ffff:ffff", pools3.at(0)->getLastAddress().toText());
+    EXPECT_EQ(Lease::TYPE_NA, pools3.at(0)->getType());
+}
+
+// This test checks if configuration can be read from a JSON file.
+TEST_F(ControlledJSONDhcpv6SrvTest, comments) {
+
+    string config_hash_comments = "# This is a comment. It should be \n"
+        "#ignored. Real config starts in line below\n"
+        "{ \"interfaces\": [ \"*\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, \n"
+        "# comments in the middle should be ignored, too\n"
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::/80\" ],"
+        "    \"subnet\": \"2001:db8:1::/64\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    /// @todo: Implement C++-style (// ...) comments
+    /// @todo: Implement C-style (/* ... */) comments
+
+    writeFile(TEST_FILE, config_hash_comments);
+
+    // Now initialize the server
+    boost::scoped_ptr<ControlledDhcpv6Srv> srv;
+    ASSERT_NO_THROW(
+        srv.reset(new ControlledDhcpv6Srv(0))
+    );
+
+    // And configure it using config without
+    EXPECT_TRUE(srv->init(TEST_FILE));
+
+    // Now check if the configuration has been applied correctly.
+    const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
+    ASSERT_TRUE(subnets);
+    ASSERT_EQ(1, subnets->size());
+
+    // Check subnet 1.
+    EXPECT_EQ("2001:db8:1::", subnets->at(0)->get().first.toText());
+    EXPECT_EQ(64, subnets->at(0)->get().second);
+
+    // Check pools in the first subnet.
+    const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_NA);
+    ASSERT_EQ(1, pools1.size());
+    EXPECT_EQ("2001:db8:1::", pools1.at(0)->getFirstAddress().toText());
+    EXPECT_EQ("2001:db8:1::ffff:ffff:ffff", pools1.at(0)->getLastAddress().toText());
+    EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType());
+}
+
+} // End of anonymous namespace

src/lib/dhcpsrv/daemon.cc

@@ -49,21 +49,42 @@ void Daemon::shutdown() {
 Daemon::~Daemon() {
 }
 
-std::string Daemon::readFile(const std::string& file_name) {
+std::string Daemon::readFile(const std::string& file_name, bool ignore_comments) {
 
-    // This is the fastest method to read a file according to:
-    // http://insanecoding.blogspot.com/2011/11/how-to-read-in-file-in-c.html
 
     std::ifstream infile(file_name.c_str(), std::ios::in | std::ios::binary);
     if (infile.is_open())
     {
         std::string contents;
-        infile.seekg(0, std::ios::end);
-        contents.resize(infile.tellg());
-        infile.seekg(0, std::ios::beg);
-        infile.read(&contents[0], contents.size());
-        infile.close();
-        return(contents);
+        std::string line;
+
+        if (ignore_comments) {
+            while (std::getline(infile, line)) {
+
+                // If this is a comments line, replace it with empty line
+                // (so the line numbers will still match
+                if (!line.empty() && line[0] == '#') {
+                    line = "";
+                }
+
+                // getline() removes end line charaters. Unfortunately, we need
+                // it for getting the line numbers right (in case we report an
+                // error.
+                contents.append(line);
+                contents.append("\n");
+            }
+            return (contents);
+        } else {
+
+            // This is the fastest method to read a file according to:
+            // http://insanecoding.blogspot.com/2011/11/how-to-read-in-file-in-c.html
+            infile.seekg(0, std::ios::end);
+            contents.resize(infile.tellg());
+            infile.seekg(0, std::ios::beg);
+            infile.read(&contents[0], contents.size());
+            infile.close();
+            return(contents);
+        }
     }
     isc_throw(InvalidOperation, "Failed to read file " << file_name << ",error:"
               << errno);

src/lib/dhcpsrv/daemon.h

@@ -51,7 +51,17 @@ public:
     /// or reads the 
     /// Creates session that will be used to receive commands and updated
     /// configuration from cfgmgr (or indirectly from user via bindctl).
-    /// @return true if initialization was successful, false if it failed
+    ///
+    /// Note: This function may throw to report enountered problems. It may
+    /// also return false if the initialization was skipped. That may seem
+    /// redundant, but the idea here is that in some cases the configuration
+    /// was read, understood and the decision was made to not start. One
+    /// case where such capability could be needed is when we have a single
+    /// config file for Kea4 and D2, but the DNS Update is disabled. It is
+    /// likely that the D2 will be started, it will analyze its config file,
+    /// decide that it is not needed and will shut down.
+    ///
+    /// @return true if initialization was successful, false if it was not
     virtual bool init(const std::string& config_file);
 
     /// @brief Performs final deconfiguration.
@@ -85,9 +95,13 @@ public:
     /// This is an utility method that is expected to be used by several daemons.
     /// It reads contents of a text file and return it as a string.
     ///
+    /// For now, comments are defined as lines starting with a hash.
+    ///
     /// @param file_name name of the file to read
+    /// @param ignore_comments whether ignore comment lines
     /// @return content of the file
-    std::string readFile(const std::string& file_name);
+    std::string readFile(const std::string& file_name,
+                         bool ingore_comments = false);
 };
 
 }; // end of isc::dhcp namespace

src/lib/dhcpsrv/tests/daemon_unittest.cc

@@ -21,9 +21,6 @@
 using namespace isc;
 using namespace isc::dhcp;
 
-/// Temporary file name used in some tests
-const char* TEMP_FILE="temp-file.json";
-
 namespace {
 
 /// @brief Test-friendly version of the daemon
@@ -31,35 +28,106 @@ namespace {
 /// This class exposes internal Daemon class methods.
 class NakedDaemon: public Daemon {
 public:
-
     using Daemon::readFile;
 };
 
-TEST(Daemon, readFile) {
-    NakedDaemon x;
+/// @brief Test class for testing Deamon class
+class DaemonTest : public ::testing::Test {
+public:
+
+    /// @brief writes specified text to a file
+    ///
+    /// That is an auxilliary funtion used in fileRead() tests.
+    ///
+    /// @param content text to be written to disk
+    void writeFile(const std::string& content) {
+        // Write sample content to disk
+        unlink(TEMP_FILE);
+        std::ofstream write_me(TEMP_FILE);
+        EXPECT_TRUE(write_me.is_open());
+        write_me << content;
+        write_me.close();
+    }
+
+    /// destructor
+    ~DaemonTest() {
+        static_cast<void>(unlink(TEMP_FILE));
+    }
+
+    /// Name of the temporary file
+    static const char* TEMP_FILE;
+
+    /// The daemon implementation being tested
+    NakedDaemon daemon_;
+};
+
+/// Temporary file name used in some tests
+const char* DaemonTest::TEMP_FILE="temp-file.json";
+
+// Test checks whether a text file can be read from disk.
+TEST_F(DaemonTest, readFile) {
 
     const char* content = "Horse doesn't eat cucumber salad";
 
-    // Write sample content to disk
-    unlink(TEMP_FILE);
-    std::ofstream write_me(TEMP_FILE);
-    EXPECT_TRUE(write_me.is_open());
-    write_me << content;
-    write_me.close();
+    writeFile(content);
 
     // Check that the read content is correct
-    EXPECT_EQ(std::string(content), x.readFile(TEMP_FILE));
+    EXPECT_EQ(std::string(content), daemon_.readFile(TEMP_FILE));
 
     unlink(TEMP_FILE);
 }
 
-TEST(Daemon, readFileError) {
-    NakedDaemon x;
+// Test checks whether a text file can be read from disk.
+TEST_F(DaemonTest, readFileMultiline) {
+
+    const char* no_endline = "A text\nwithout\nendline in the last line";
+    const char* with_endline = "A text\nwith\endline in the last line\n";
+
+    // Write sample content to disk
+    writeFile(no_endline);
 
     // Check that the read content is correct
-    EXPECT_THROW(x.readFile("no-such-file.txt"), isc::InvalidOperation);
+    EXPECT_EQ(std::string(no_endline), daemon_.readFile(TEMP_FILE));
 
-    unlink(TEMP_FILE);
+    // Write sample content to disk
+    writeFile(with_endline);
+
+    // Check that the read content is correct
+    EXPECT_EQ(std::string(with_endline), daemon_.readFile(TEMP_FILE));
+}
+
+
+// Test checks whether comments in file are ignored as expected.
+TEST_F(DaemonTest, readFileComments) {
+    NakedDaemon x;
+
+    const char* commented_content = "# This is a comment\n"
+        "this is a normal line\n"
+        "Second line\n"
+        "# another comment";
+
+    // The same as above with comments removed and comment lines
+    // substituted with empty lines.
+    const char* filtered_content ="\nthis is a normal line\nSecond line\n\n";
+
+    // Write sample content to disk
+    writeFile(commented_content);
+
+    // Check that the read content is correct (without comment elimination)
+    EXPECT_EQ(std::string(commented_content), x.readFile(TEMP_FILE, false));
+
+    // Check that the read content is correct (with comment elimination)
+    EXPECT_EQ(std::string(filtered_content), x.readFile(TEMP_FILE, true));
+
+
+
+}
+
+// This test checks that missing file will generate an exception.
+TEST_F(DaemonTest, readFileError) {
+
+    // Check that the read content is correct
+    EXPECT_THROW(daemon_.readFile("no-such-file.txt"), isc::InvalidOperation);
 }
 
 };