[3696] Addressed review comments.

Addressed a couple of minor issues and added unit tests for
switching between various lease database backends.

src/bin/dhcp4/json_config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2016 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

src/bin/dhcp4/tests/kea_controller_unittest.cc

@@ -16,6 +16,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <log/logger_support.h>
 #include <util/stopwatch.h>
 
@@ -24,6 +25,7 @@
 
 #include <fstream>
 #include <iostream>
+#include <signal.h>
 #include <sstream>
 
 #include <arpa/inet.h>
@@ -35,6 +37,7 @@ using namespace isc::asiolink;
 using namespace isc::config;
 using namespace isc::data;
 using namespace isc::dhcp;
+using namespace isc::dhcp::test;
 using namespace isc::hooks;
 
 namespace {
@@ -43,6 +46,7 @@ class NakedControlledDhcpv4Srv: public ControlledDhcpv4Srv {
     // "Naked" DHCPv4 server, exposes internal fields
 public:
     NakedControlledDhcpv4Srv():ControlledDhcpv4Srv(0) { }
+    using ControlledDhcpv4Srv::signal_handler_;
 };
 
 /// @brief test class for Kea configuration backend
@@ -423,4 +427,147 @@ TEST_F(JSONFileBackendTest, defaultLeaseDbBackend) {
     EXPECT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
 }
 
+// Starting tests which require MySQL backend availability. Those tests
+// will not be executed if Kea has been compiled without the
+// --with-dhcp-mysql.
+#ifdef HAVE_MYSQL
+
+/// @brief Test fixture class for the tests utilizing MySQL database
+/// backend.
+class JSONFileBackendMySQLTest : public JSONFileBackendTest {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Recreates MySQL schema for a test.
+    JSONFileBackendMySQLTest() : JSONFileBackendTest() {
+        destroyMySQLSchema();
+        createMySQLSchema();
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Destroys MySQL schema.
+    virtual ~JSONFileBackendMySQLTest() {
+        destroyMySQLSchema();
+    }
+
+    /// @brief Creates server configuration with specified backend type.
+    ///
+    /// @param backend Backend type or empty string to indicate that the
+    /// backend configuration should not be placed in the resulting
+    /// JSON configuration.
+    ///
+    /// @return Server configuration.
+    std::string createConfiguration(const std::string& backend) const;
+
+    /// @brief Test reconfiguration with a backend change.
+    ///
+    /// If any of the parameters is an empty string it indicates that the
+    /// created configuration should exclude backend configuration.
+    ///
+    /// @param backend_first Type of a backend to be used initially.
+    /// @param backend_second Type of a backend to be used after
+    /// reconfiguration.
+    void testBackendReconfiguration(const std::string& backend_first,
+                                    const std::string& backend_second);
+};
+
+std::string
+JSONFileBackendMySQLTest::createConfiguration(const std::string& backend) const {
+    // This is basic server configuration which excludes lease database
+    // backend specification. The default Memfile backend should be
+    // initialized in this case.
+    std::ostringstream config;
+    config <<
+        "{ \"Dhcp4\": {"
+        "\"interfaces-config\": {"
+        "    \"interfaces\": [ ]"
+        "},";
+
+    // For non-empty lease backend type we have to add a backend configuration
+    // section.
+    if (!backend.empty()) {
+        config <<
+        "\"lease-database\": {"
+        "     \"type\": \"" << backend << "\"";
+
+        // SQL backends require database credentials.
+        if (backend != "memfile") {
+            config <<
+                ","
+                "     \"name\": \"keatest\","
+                "     \"user\": \"keatest\","
+                "     \"password\": \"keatest\"";
+        }
+        config << "},";
+    }
+
+    // Append the rest of the configuration.
+    config <<
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, \n"
+        "\"subnet4\": [ ],"
+        "\"valid-lifetime\": 4000 }"
+        "}";
+
+    return (config.str());
+}
+
+void
+JSONFileBackendMySQLTest::
+testBackendReconfiguration(const std::string& backend_first,
+                           const std::string& backend_second) {
+    // This is basic server configuration which excludes lease database
+    // backend specification. The default Memfile backend should be
+    // initialized in this case.
+    writeFile(createConfiguration(backend_first));
+
+    // Create an instance of the server and intialize it.
+    boost::scoped_ptr<NakedControlledDhcpv4Srv> srv;
+    ASSERT_NO_THROW(srv.reset(new NakedControlledDhcpv4Srv()));
+    srv->setConfigFile(TEST_FILE);
+    ASSERT_NO_THROW(srv->init(TEST_FILE));
+
+    // The backend should have been created and its type should be
+    // correct.
+    ASSERT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
+    EXPECT_EQ(backend_first.empty() ? "memfile" : backend_first,
+              LeaseMgrFactory::instance().getType());
+
+    // New configuration modifies the lease database backend type to MYSQL.
+    writeFile(createConfiguration(backend_second));
+
+    // Explicitly calling signal handler for SIGHUP to trigger server
+    // reconfiguration.
+    srv->signal_handler_(SIGHUP);
+
+    // The backend should have been created and its type should be
+    // correct.
+    ASSERT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
+    EXPECT_EQ(backend_second.empty() ? "memfile" : backend_second,
+              LeaseMgrFactory::instance().getType());
+}
+
+
+// This test verifies that backend specification can be added on
+// server reconfiguration.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendUndefinedToMySQL) {
+    testBackendReconfiguration("", "mysql");
+}
+
+// This test verifies that when backend specification is removed the
+// default backend is used.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMySQLToUndefined) {
+    testBackendReconfiguration("mysql", "");
+}
+
+// This test verifies that backend type can be changed from Memfile
+// to MySQL.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMemfileToMySQL) {
+    testBackendReconfiguration("memfile", "mysql");
+}
+
+#endif
+
 } // End of anonymous namespace

src/bin/dhcp6/json_config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2016 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

src/bin/dhcp6/tests/kea_controller_unittest.cc

@@ -16,6 +16,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <log/logger_support.h>
 #include <util/stopwatch.h>
 
@@ -35,6 +36,7 @@ using namespace isc::asiolink;
 using namespace isc::config;
 using namespace isc::data;
 using namespace isc::dhcp;
+using namespace isc::dhcp::test;
 using namespace isc::hooks;
 
 namespace {
@@ -43,6 +45,7 @@ class NakedControlledDhcpv6Srv: public ControlledDhcpv6Srv {
     // "Naked" DHCPv6 server, exposes internal fields
 public:
     NakedControlledDhcpv6Srv():ControlledDhcpv6Srv(0) { }
+    using ControlledDhcpv6Srv::signal_handler_;
 };
 
 
@@ -406,4 +409,148 @@ TEST_F(JSONFileBackendTest, defaultLeaseDbBackend) {
     EXPECT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
 }
 
+// Starting tests which require MySQL backend availability. Those tests
+// will not be executed if Kea has been compiled without the
+// --with-dhcp-mysql.
+#ifdef HAVE_MYSQL
+
+/// @brief Test fixture class for the tests utilizing MySQL database
+/// backend.
+class JSONFileBackendMySQLTest : public JSONFileBackendTest {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Recreates MySQL schema for a test.
+    JSONFileBackendMySQLTest() : JSONFileBackendTest() {
+        destroyMySQLSchema();
+        createMySQLSchema();
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Destroys MySQL schema.
+    virtual ~JSONFileBackendMySQLTest() {
+        destroyMySQLSchema();
+    }
+
+    /// @brief Creates server configuration with specified backend type.
+    ///
+    /// @param backend Backend type or empty string to indicate that the
+    /// backend configuration should not be placed in the resulting
+    /// JSON configuration.
+    ///
+    /// @return Server configuration.
+    std::string createConfiguration(const std::string& backend) const;
+
+    /// @brief Test reconfiguration with a backend change.
+    ///
+    /// If any of the parameters is an empty string it indicates that the
+    /// created configuration should exclude backend configuration.
+    ///
+    /// @param backend_first Type of a backend to be used initially.
+    /// @param backend_second Type of a backend to be used after
+    /// reconfiguration.
+    void testBackendReconfiguration(const std::string& backend_first,
+                                    const std::string& backend_second);
+};
+
+std::string
+JSONFileBackendMySQLTest::createConfiguration(const std::string& backend) const {
+    // This is basic server configuration which excludes lease database
+    // backend specification. The default Memfile backend should be
+    // initialized in this case.
+    std::ostringstream config;
+    config <<
+        "{ \"Dhcp6\": {"
+        "\"interfaces-config\": {"
+        "    \"interfaces\": [ ]"
+        "},";
+
+    // For non-empty lease backend type we have to add a backend configuration
+    // section.
+    if (!backend.empty()) {
+        config <<
+        "\"lease-database\": {"
+        "     \"type\": \"" << backend << "\"";
+
+        // SQL backends require database credentials.
+        if (backend != "memfile") {
+            config <<
+                ","
+                "     \"name\": \"keatest\","
+                "     \"user\": \"keatest\","
+                "     \"password\": \"keatest\"";
+        }
+        config << "},";
+    }
+
+    // Append the rest of the configuration.
+    config <<
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, \n"
+        "\"subnet6\": [ ],"
+        "\"preferred-lifetime\": 3000, "
+        "\"valid-lifetime\": 4000 }"
+        "}";
+
+    return (config.str());
+}
+
+void
+JSONFileBackendMySQLTest::
+testBackendReconfiguration(const std::string& backend_first,
+                           const std::string& backend_second) {
+    // This is basic server configuration which excludes lease database
+    // backend specification. The default Memfile backend should be
+    // initialized in this case.
+    writeFile(TEST_FILE, createConfiguration(backend_first));
+
+    // Create an instance of the server and intialize it.
+    boost::scoped_ptr<NakedControlledDhcpv6Srv> srv;
+    ASSERT_NO_THROW(srv.reset(new NakedControlledDhcpv6Srv()));
+    srv->setConfigFile(TEST_FILE);
+    ASSERT_NO_THROW(srv->init(TEST_FILE));
+
+    // The backend should have been created and its type should be
+    // correct.
+    ASSERT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
+    EXPECT_EQ(backend_first.empty() ? "memfile" : backend_first,
+              LeaseMgrFactory::instance().getType());
+
+    // New configuration modifies the lease database backend type to MYSQL.
+    writeFile(TEST_FILE, createConfiguration(backend_second));
+
+    // Explicitly calling signal handler for SIGHUP to trigger server
+    // reconfiguration.
+    srv->signal_handler_(SIGHUP);
+
+    // The backend should have been created and its type should be
+    // correct.
+    ASSERT_NO_THROW(static_cast<void>(LeaseMgrFactory::instance()));
+    EXPECT_EQ(backend_second.empty() ? "memfile" : backend_second,
+              LeaseMgrFactory::instance().getType());
+}
+
+
+// This test verifies that backend specification can be added on
+// server reconfiguration.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendUndefinedToMySQL) {
+    testBackendReconfiguration("", "mysql");
+}
+
+// This test verifies that when backend specification is removed the
+// default backend is used.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMySQLToUndefined) {
+    testBackendReconfiguration("mysql", "");
+}
+
+// This test verifies that backend type can be changed from Memfile
+// to MySQL.
+TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMemfileToMySQL) {
+    testBackendReconfiguration("memfile", "mysql");
+}
+
+#endif
+
 } // End of anonymous namespace

src/lib/dhcpsrv/cfg_db_access.h

@@ -26,6 +26,9 @@ public:
 
     /// @brief Sets parameters which will be appended to the database
     /// access strings.
+    ///
+    /// @param appended_parameters String holding collection of parameters
+    /// in the following format: "parameter0=value0 parameter1=value1 ...".
     void setAppendedParameters(const std::string& appended_parameters) {
         appended_parameters_ = appended_parameters;
     }

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -777,7 +777,7 @@ Memfile_LeaseMgr::deleteExpiredReclaimedLeases(const uint32_t secs,
 
 std::string
 Memfile_LeaseMgr::getDescription() const {
-    return (std::string("In Memory Database"));
+    return (std::string("In memory database with leases stored in a CSV file."));
 }
 
 void

src/lib/dhcpsrv/srv_config.h

@@ -279,7 +279,7 @@ public:
         return (cfg_duid_);
     }
 
-    /// @brief Returns pointer to the objec holding configuration of the
+    /// @brief Returns pointer to the object holding configuration of the
     /// lease and host database connection parameters.
     CfgDbAccessPtr getCfgDbAccess() {
         return (cfg_db_access_);

src/lib/dhcpsrv/tests/Makefile.am

@@ -105,7 +105,6 @@ libdhcpsrv_unittests_SOURCES += generic_host_data_source_unittest.cc generic_hos
 libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += dhcp_parsers_unittest.cc
 if HAVE_MYSQL
-libdhcpsrv_unittests_SOURCES += mysql_schema.cc mysql_schema.h
 libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += mysql_host_data_source_unittest.cc
 endif
@@ -116,7 +115,6 @@ if HAVE_PGSQL
 libdhcpsrv_unittests_SOURCES += pgsql_lease_mgr_unittest.cc
 endif
 libdhcpsrv_unittests_SOURCES += pool_unittest.cc
-libdhcpsrv_unittests_SOURCES += schema_mysql_copy.h
 libdhcpsrv_unittests_SOURCES += schema_pgsql_copy.h
 libdhcpsrv_unittests_SOURCES += srv_config_unittest.cc
 libdhcpsrv_unittests_SOURCES += subnet_unittest.cc

src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc

@@ -9,7 +9,7 @@
 #include <dhcpsrv/host_data_source_factory.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
-#include <dhcpsrv/tests/mysql_schema.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <gtest/gtest.h>
 
 using namespace isc;

src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc

@@ -9,7 +9,7 @@
 #include <cc/command_interpreter.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/parsers/dbaccess_parser.h>
-#include <dhcpsrv/tests/mysql_schema.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <dhcpsrv/host_mgr.h>
 #include <log/logger_support.h>
 

src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 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
@@ -12,7 +12,7 @@
 #include <dhcpsrv/mysql_connection.h>
 #include <dhcpsrv/mysql_host_data_source.h>
 #include <dhcpsrv/tests/generic_host_data_source_unittest.h>
-#include <dhcpsrv/tests/mysql_schema.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <dhcpsrv/host_data_source_factory.h>
 
 #include <gtest/gtest.h>

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2016 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
@@ -12,7 +12,7 @@
 #include <dhcpsrv/mysql_lease_mgr.h>
 #include <dhcpsrv/tests/test_utils.h>
 #include <dhcpsrv/tests/generic_lease_mgr_unittest.h>
-#include <dhcpsrv/tests/mysql_schema.h>
+#include <dhcpsrv/testutils/mysql_schema.h>
 #include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>

src/lib/dhcpsrv/testutils/Makefile.am

@@ -16,9 +16,20 @@ if HAVE_GTEST
 noinst_LTLIBRARIES = libdhcpsrvtest.la
 
 libdhcpsrvtest_la_SOURCES  = config_result_check.cc config_result_check.h
+if HAVE_MYSQL
+libdhcpsrvtest_la_SOURCES += mysql_schema.cc mysql_schema.h
+libdhcpsrvtest_la_SOURCES += schema_mysql_copy.h
+endif
+
 libdhcpsrvtest_la_CXXFLAGS = $(AM_CXXFLAGS)
-libdhcpsrvtest_la_CPPFLAGS = $(AM_CPPFLAGS)
-libdhcpsrvtest_la_LDFLAGS  = $(AM_LDFLAGS)
+libdhcpsrvtest_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
+if HAVE_MYSQL
+libdhcpsrvtest_la_CPPFLAGS += $(MYSQL_CPPFLAGS)
+endif
+libdhcpsrvtest_la_LDFLAGS  = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
+if HAVE_MYSQL
+libdhcpsrvtest_la_LDFLAGS  += $(MYSQL_LIBS)
+endif
 libdhcpsrvtest_la_LIBADD   = $(top_builddir)/src/lib/cc/libkea-cc.la
 libdhcpsrvtest_la_LIBADD  += $(top_builddir)/src/lib/log/libkea-log.la