From cc65a33e2a84e88a4c1703d360d18187b09a8850 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Mon, 15 Oct 2012 12:29:17 +0100 Subject: [PATCH] [2342] Separate out lease manager creation into separate factory functions --- src/lib/dhcp/Makefile.am | 1 + src/lib/dhcp/lease_mgr.cc | 46 +++------- src/lib/dhcp/lease_mgr.h | 17 ++-- src/lib/dhcp/lease_mgr_factory.cc | 87 +++++++++++++++++++ src/lib/dhcp/lease_mgr_factory.h | 65 ++++++++++++++ src/lib/dhcp/tests/Makefile.am | 1 + .../dhcp/tests/lease_mgr_factory_unittest.cc | 47 ++++++++++ src/lib/dhcp/tests/lease_mgr_unittest.cc | 28 +++--- 8 files changed, 236 insertions(+), 56 deletions(-) create mode 100644 src/lib/dhcp/lease_mgr_factory.cc create mode 100644 src/lib/dhcp/lease_mgr_factory.h create mode 100644 src/lib/dhcp/tests/lease_mgr_factory_unittest.cc diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index 6585a386c3..2502aa4aa1 100644 --- a/src/lib/dhcp/Makefile.am +++ b/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 diff --git a/src/lib/dhcp/lease_mgr.cc b/src/lib/dhcp/lease_mgr.cc index d09bd583c0..1fd64d8066 100644 --- a/src/lib/dhcp/lease_mgr.cc +++ b/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 -#include -#include -#include -#include -#include #include +#include #include -#include +#include +#include +#include +#include + #include #include -#include "lease_mgr.h" + +#include +#include using namespace std; using namespace isc::dhcp; -LeaseMgr::LeaseMgr(const std::string& dbconfig) { - - if (dbconfig.length() == 0) { - return; - } - - vector 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(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::const_iterator param - = parameters_.find(name); + ParameterMap::const_iterator param = parameters_.find(name); if (param == parameters_.end()) { isc_throw(BadValue, "Parameter not found"); } diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h index 4b7a1afa25..57d7f02031 100644 --- a/src/lib/dhcp/lease_mgr.h +++ b/src/lib/dhcp/lease_mgr.h @@ -274,14 +274,14 @@ public: /// Client Hardware address typedef std::vector HWAddr; + /// Database configuration parameter map + typedef std::map 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 parameters_; + ParameterMap parameters_; }; +/// @brief Pointer to a Lease Manager +typedef boost::shared_ptr LeaseMgrPtr; + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcp/lease_mgr_factory.cc b/src/lib/dhcp/lease_mgr_factory.cc new file mode 100644 index 0000000000..3518d96dda --- /dev/null +++ b/src/lib/dhcp/lease_mgr_factory.cc @@ -0,0 +1,87 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +using namespace std; + +namespace isc { +namespace dhcp { + +LeaseMgr::ParameterMap +LeaseMgrFactory::parse(const std::string& dbconfig) { + LeaseMgr::ParameterMap mapped_tokens; + + if (! dbconfig.empty()) { + vector 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 diff --git a/src/lib/dhcp/lease_mgr_factory.h b/src/lib/dhcp/lease_mgr_factory.h new file mode 100644 index 0000000000..bf2b8b4318 --- /dev/null +++ b/src/lib/dhcp/lease_mgr_factory.h @@ -0,0 +1,65 @@ +// 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 +#include + + +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 diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am index a15d9579d2..77eef8a105 100644 --- a/src/lib/dhcp/tests/Makefile.am +++ b/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 diff --git a/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc b/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc new file mode 100644 index 0000000000..c3cb5cfcbe --- /dev/null +++ b/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc @@ -0,0 +1,47 @@ +// 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 +#include +#include +#include + +#include +#include + +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 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 diff --git a/src/lib/dhcp/tests/lease_mgr_unittest.cc b/src/lib/dhcp/tests/lease_mgr_unittest.cc index 97659a142e..2b38f2badc 100644 --- a/src/lib/dhcp/tests/lease_mgr_unittest.cc +++ b/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