[2342] Separate out lease manager creation into separate factory functions

src/lib/dhcp/Makefile.am

@@ -17,6 +17,7 @@ lib_LTLIBRARIES = libb10-dhcp++.la libb10-dhcpsrv.la
 libb10_dhcp___la_SOURCES  =
 libb10_dhcp___la_SOURCES += libdhcp++.cc libdhcp++.h
 libb10_dhcp___la_SOURCES += lease_mgr.cc lease_mgr.h
+libb10_dhcp___la_SOURCES += lease_mgr_factory.cc lease_mgr_factory.h
 libb10_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h
 libb10_dhcp___la_SOURCES += iface_mgr_linux.cc
 libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc

src/lib/dhcp/lease_mgr.cc

@@ -12,52 +12,30 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <sstream>
+#include <algorithm>
 #include <iostream>
+#include <iterator>
 #include <map>
-#include <iostream>
-#include <string>
 #include <sstream>
-#include <algorithm>
-#include <iterator>
-#include <exceptions/exceptions.h>
+#include <string>
+#include <utility>
+
 #include <boost/foreach.hpp>
 #include <boost/algorithm/string.hpp>
-#include "lease_mgr.h"
+
+#include <exceptions/exceptions.h>
+#include <dhcp/lease_mgr.h>
 
 using namespace std;
 
 using namespace isc::dhcp;
 
-LeaseMgr::LeaseMgr(const std::string& dbconfig) {
-
-    if (dbconfig.length() == 0) {
-        return;
-    }
-
-    vector<string> tokens;
-
-    // we need to pass a string to is_any_of, not just char *. Otherwise there
-    // are cryptic warnings on Debian6 running g++ 4.4 in /usr/include/c++/4.4
-    // /bits/stl_algo.h:2178 "array subscript is above array bounds"
-    boost::split(tokens, dbconfig, boost::is_any_of( string("\t ") ));
-    BOOST_FOREACH(std::string token, tokens) {
-        size_t pos = token.find("=");
-        if (pos != string::npos) {
-            string name = token.substr(0, pos);
-            string value = token.substr(pos + 1, -1);
-            parameters_.insert(pair<string,string>(name, value));
-        } else {
-            isc_throw(InvalidParameter, "Cannot parse " << token
-                      << ", expected format is name=value");
-        }
-
-    }
+LeaseMgr::LeaseMgr(const LeaseMgr::ParameterMap& parameters)
+    : parameters_(parameters) {
 }
 
 std::string LeaseMgr::getParameter(const std::string& name) const {
-    std::map<std::string, std::string>::const_iterator param
-        = parameters_.find(name);
+    ParameterMap::const_iterator param = parameters_.find(name);
     if (param == parameters_.end()) {
         isc_throw(BadValue, "Parameter not found");
     }

src/lib/dhcp/lease_mgr.h

@@ -274,14 +274,14 @@ public:
     /// Client Hardware address
     typedef std::vector<uint8_t> HWAddr;
 
+    /// Database configuration parameter map
+    typedef std::map<std::string, std::string> ParameterMap;
+
     /// @brief The sole lease manager constructor
     ///
-    /// dbconfig is a generic way of passing parameters. Parameters
-    /// are passed in the "name=value" format, separated by spaces.
-    /// Values may be enclosed in double quotes, if needed.
-    ///
-    /// @param dbconfig database configuration
-    LeaseMgr(const std::string& dbconfig);
+    /// @param parameters A data structure relating keywords and values
+    ///        concerned with the database.
+    LeaseMgr(const ParameterMap& parameters);
 
     /// @brief Destructor (closes file)
     virtual ~LeaseMgr();
@@ -472,9 +472,12 @@ protected:
     /// That will be mostly used for storing database name, username,
     /// password and other parameters required for DB access. It is not
     /// intended to keep any DHCP-related parameters.
-    std::map<std::string, std::string> parameters_;
+    ParameterMap parameters_;
 };
 
+/// @brief Pointer to a Lease Manager
+typedef boost::shared_ptr<LeaseMgr> LeaseMgrPtr;
+
 }; // end of isc::dhcp namespace
 
 }; // end of isc namespace

src/lib/dhcp/lease_mgr_factory.cc

+// Copyright (C) 2012 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 <algorithm>
+#include <iostream>
+#include <iterator>
+#include <map>
+#include <sstream>
+#include <string>
+#include <utility>
+
+#include <boost/foreach.hpp>
+#include <boost/algorithm/string.hpp>
+#include <exceptions/exceptions.h>
+#include <dhcp/lease_mgr_factory.h>
+
+using namespace std;
+
+namespace isc {
+namespace dhcp {
+
+LeaseMgr::ParameterMap
+LeaseMgrFactory::parse(const std::string& dbconfig) {
+    LeaseMgr::ParameterMap mapped_tokens;
+
+    if (! dbconfig.empty()) {
+        vector<string> tokens;
+
+        // We need to pass a string to is_any_of, not just char*. Otherwise
+        // there are cryptic warnings on Debian6 running g++ 4.4 in
+        // /usr/include/c++/4.4/bits/stl_algo.h:2178 "array subscript is above
+        // array bounds"
+        boost::split(tokens, dbconfig, boost::is_any_of( string("\t ") ));
+        BOOST_FOREACH(std::string token, tokens) {
+            size_t pos = token.find("=");
+            if (pos != string::npos) {
+                string name = token.substr(0, pos);
+                string value = token.substr(pos + 1, -1);
+                mapped_tokens.insert(make_pair(name, value));
+            } else {
+                isc_throw(InvalidParameter, "Cannot parse " << token
+                          << ", expected format is name=value");
+            }
+        }
+    }
+
+    return (mapped_tokens);
+}
+
+LeaseMgrPtr
+LeaseMgrFactory::create(const std::string& dbconfig) {
+    const std::string type = "type";
+
+    // Is "type" present?
+    LeaseMgr::ParameterMap parameters = parse(dbconfig);
+    if (parameters.find(type) == parameters.end()) {
+        isc_throw(InvalidParameter, "Database configuration parameters do not "
+                  "contain the 'type' keyword");
+    }
+
+    // Yes, check what it is.
+#ifdef HAVE_MYSQL
+    if (parameters[type] == string("mysql")) {
+        return LeaseMgrPtr(new LeaseMgr(parameters));
+    }
+#endif
+
+    // Get here on no match
+    isc_throw(InvalidParameter, "Database configuration parameter 'type' does "
+              "not specify a supported database backend");
+}
+
+}; // namespace dhcp
+}; // namespace isc

src/lib/dhcp/lease_mgr_factory.h

+// Copyright (C) 2012 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 <string>
+#include <dhcp/lease_mgr.h>
+
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Lease Manager Factory
+///
+/// This class comprises nothing but static methods used to create a lease
+/// manager.  It analyzes the database information passed to the creation
+/// function and instantiates an appropriate lease manager based on the type
+/// requested.
+///
+/// Strictly speaking these functions could be stand-alone functions.  However,
+/// it is convenient to encapsulate them in a class for naming purposes.
+class LeaseMgrFactory {
+public:
+    /// @brief Create an instance of a lease manager.
+    ///
+    /// Each database backend has its own lease manager type.  This static
+    /// method returns a lease manager of the appropriate type, based on the
+    /// the data in the input argument.
+    ///
+    /// dbconfig is a generic way of passing parameters. Parameters are passed
+    /// in the "name=value" format, separated by spaces.  The data MUST include
+    /// a keyword/value pair of the form "type=dbtype" giving the database
+    /// type, e.q. "mysql" or "sqlite3".
+    ///
+    /// @param dbconfig Database configuration parameters.  These are in
+    ///        the form of "keyword=value" pairs, separated by spaces. These
+    ///        are back-end specific, although must include the "type" keyword
+    ///        which gives the backend in use.
+    ///
+    /// @return Implementation of lease manager for the specified database.
+    static LeaseMgrPtr create(const std::string& dbconfig);
+
+    /// @brief Parse Database Parameters
+    ///
+    /// Parses the string of "keyword=value" pairs and separates them
+    /// out into the map.
+    ///
+    /// @param dbconfig Database configuration string
+    ///
+    /// @return std::map<>std::string, std::string> Map of keyword/value pairs.
+    static LeaseMgr::ParameterMap parse(const std::string& dbconfig);
+};
+
+}; // end of isc::dhcp namespace
+
+}; // end of isc namespace

src/lib/dhcp/tests/Makefile.am

@@ -29,6 +29,7 @@ libdhcp___unittests_SOURCES  = run_unittests.cc
 libdhcp___unittests_SOURCES += libdhcp++_unittest.cc
 libdhcp___unittests_SOURCES += iface_mgr_unittest.cc
 libdhcp___unittests_SOURCES += lease_mgr_unittest.cc
+libdhcp___unittests_SOURCES += lease_mgr_factory_unittest.cc
 libdhcp___unittests_SOURCES += option6_iaaddr_unittest.cc
 libdhcp___unittests_SOURCES += option6_ia_unittest.cc
 libdhcp___unittests_SOURCES += option6_addrlst_unittest.cc

src/lib/dhcp/tests/lease_mgr_factory_unittest.cc

+// Copyright (C) 2011-2012 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 <iostream>
+#include <sstream>
+#include <gtest/gtest.h>
+
+#include <asiolink/io_address.h>
+#include <dhcp/lease_mgr_factory.h>
+
+using namespace std;
+using namespace isc::dhcp;
+
+
+namespace {
+// empty class for now, but may be extended once Addr6 becomes bigger
+class LeaseMgrFactoryTest : public ::testing::Test {
+public:
+    LeaseMgrFactoryTest() {
+    }
+};
+
+// This test checks if the LeaseMgr can be instantiated and that it
+// parses parameters string properly.
+TEST_F(LeaseMgrFactoryTest, parse) {
+
+    std::map<std::string, std::string> parameters = LeaseMgrFactory::parse(
+        "param1=value1 param2=value2 param3=value3");
+
+    EXPECT_EQ("value1", parameters["param1"]);
+    EXPECT_EQ("value2", parameters["param2"]);
+    EXPECT_TRUE(parameters.find("type") == parameters.end());
+}
+
+}; // end of anonymous namespace

src/lib/dhcp/tests/lease_mgr_unittest.cc

@@ -39,8 +39,9 @@ public:
     /// are passed in the "name=value" format, separated by spaces.
     /// Values may be enclosed in double quotes, if needed.
     ///
-    /// @param dbconfig database configuration
-    Memfile_LeaseMgr(const std::string& dbconfig);
+    /// @param parameters A data structure relating keywords and values
+    ///        concerned with the database.
+    Memfile_LeaseMgr(const LeaseMgr::ParameterMap& parameters);
 
     /// @brief Destructor (closes file)
     virtual ~Memfile_LeaseMgr();
@@ -184,8 +185,8 @@ protected:
 
 };
 
-Memfile_LeaseMgr::Memfile_LeaseMgr(const std::string& dbconfig)
-    : LeaseMgr(dbconfig) {
+Memfile_LeaseMgr::Memfile_LeaseMgr(const LeaseMgr::ParameterMap& parameters)
+    : LeaseMgr(parameters) {
 }
 
 Memfile_LeaseMgr::~Memfile_LeaseMgr() {
@@ -271,19 +272,16 @@ public:
 
 // This test checks if the LeaseMgr can be instantiated and that it
 // parses parameters string properly.
-TEST_F(LeaseMgrTest, constructor) {
+TEST_F(LeaseMgrTest, getParameter) {
 
-    // should not throw any exceptions here
-    Memfile_LeaseMgr * leaseMgr = new Memfile_LeaseMgr("");
-    delete leaseMgr;
+    LeaseMgr::ParameterMap pmap;
+    pmap[std::string("param1")] = std::string("value1");
+    pmap[std::string("param2")] = std::string("value2");
+    Memfile_LeaseMgr leasemgr(pmap);
 
-    leaseMgr = new Memfile_LeaseMgr("param1=value1 param2=value2");
-
-    EXPECT_EQ("value1", leaseMgr->getParameter("param1"));
-    EXPECT_EQ("value2", leaseMgr->getParameter("param2"));
-    EXPECT_THROW(leaseMgr->getParameter("param3"), BadValue);
-
-    delete leaseMgr;
+    EXPECT_EQ("value1", leasemgr.getParameter("param1"));
+    EXPECT_EQ("value2", leasemgr.getParameter("param2"));
+    EXPECT_THROW(leasemgr.getParameter("param3"), BadValue);
 }
 
 // There's no point in calling any other methods in LeaseMgr, as they