From: Marcin Siodelski Date: Mon, 25 Mar 2019 16:18:27 +0000 (+0100) Subject: [#490,!284] Working PoC of inheritance in networks and globals. X-Git-Tag: Kea-1.6.0-beta~305 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6c3b2abb00cb0cd3e24c10a5dd9ba51e038eeafa;p=thirdparty%2Fkea.git [#490,!284] Working PoC of inheritance in networks and globals. --- diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index 39a13ec4d1..2c590feeab 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -62,7 +62,6 @@ libkea_dhcpsrv_la_SOURCES = libkea_dhcpsrv_la_SOURCES += alloc_engine.cc alloc_engine.h libkea_dhcpsrv_la_SOURCES += alloc_engine_log.cc alloc_engine_log.h libkea_dhcpsrv_la_SOURCES += alloc_engine_messages.h alloc_engine_messages.cc -libkea_dhcpsrv_la_SOURCES += assignable_network.h libkea_dhcpsrv_la_SOURCES += base_host_data_source.h libkea_dhcpsrv_la_SOURCES += cache_host_data_source.h libkea_dhcpsrv_la_SOURCES += callout_handle_store.h @@ -281,7 +280,6 @@ libkea_dhcpsrv_include_HEADERS = \ alloc_engine.h \ alloc_engine_log.h \ alloc_engine_messages.h \ - assignable_network.h \ base_host_data_source.h \ cache_host_data_source.h \ callout_handle_store.h \ diff --git a/src/lib/dhcpsrv/assignable_network.h b/src/lib/dhcpsrv/assignable_network.h deleted file mode 100644 index 7b2eb2d8d6..0000000000 --- a/src/lib/dhcpsrv/assignable_network.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (C) 2017 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#ifndef ASSIGNABLE_NETWORK_H -#define ASSIGNABLE_NETWORK_H - -#include - -namespace isc { -namespace dhcp { - -/// @brief Represents a network that can be associated with a subnet. -/// -/// This class represents a network that can be associated with a subnet -/// using @c Subnet::setSharedNetwork method. This class is a friend -/// of a @ref Subnet class, so it can call its @c Subnet::setSharedNetwork -/// private method. Association of a network with a subnet must be always -/// conducted using this class. This prevents unwanted replacements of -/// shared networks within subnets. -class AssignableNetwork { -protected: - - /// @brief Virtual destructor. - virtual ~AssignableNetwork() { } - - /// @brief Returns shared pointer to this object. - /// - /// This abstract method must be implemented by derived classes to - /// return shared pointers the derivation. - /// - /// @return Pointer to this network. - virtual NetworkPtr sharedFromThis() = 0; -}; - -} // end of namespace isc::dhcp -} // end of namespace isc - -#endif // ASSIGNABLE_NETWORK_H diff --git a/src/lib/dhcpsrv/network.cc b/src/lib/dhcpsrv/network.cc index adf2827f38..2c13a4779c 100644 --- a/src/lib/dhcpsrv/network.cc +++ b/src/lib/dhcpsrv/network.cc @@ -249,8 +249,8 @@ Network4::toElement() const { } // Set authoritative - if (!getAuthoritative().unspecified()) { - map->set("authoritative", Element::create(getAuthoritative().get())); + if (!authoritative_.unspecified()) { + map->set("authoritative", Element::create(authoritative_.get())); } // Set next-server diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index a1e2a70b4e..f3d3d9f059 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -21,6 +22,7 @@ #include #include #include +#include #include namespace isc { @@ -37,6 +39,9 @@ typedef boost::shared_ptr NetworkPtr; /// @brief Weak pointer to the @ref Network object. typedef boost::weak_ptr WeakNetworkPtr; +/// @brief Callback function for @c Network that retrieves globally +/// configured parameters. +typedef std::function FetchNetworkGlobalsFn; /// @brief Common interface representing a network to which the DHCP clients /// are connected. @@ -187,58 +192,12 @@ public: /// Does nothing at the moment. virtual ~Network() { }; - /// @brief Retrieves pointer to a shared network associated with a subnet. + /// @brief Sets the optional callback function used to fetch globally + /// configured parameters. /// - /// By implementing it as a template function we overcome a need to - /// include shared_network.h header file to specify return type explicitly. - /// The header can't be included because it would cause circular dependency - /// between subnet.h and shared_network.h. - /// - /// This method uses an argument to hold a return value to allow the compiler - /// to infer the return type without a need to call this function with an - /// explicit return type as template argument. - /// - /// @param [out] shared_network Pointer to the shared network where returned - /// value should be assigned. - /// - /// @tparam Type of the shared network, i.e. @ref SharedNetwork4 or a - /// @ref SharedNetwork6. - template - void getSharedNetwork(SharedNetworkPtrType& shared_network) const { - shared_network = boost::dynamic_pointer_cast< - typename SharedNetworkPtrType::element_type>(parent_network_.lock()); - } - - /// @brief Assigns shared network to a subnet. - /// - /// This method replaces any shared network associated with a subnet with - /// a new shared network. - /// - /// @param shared_network Pointer to a new shared network to be associated - /// with the subnet. - void setSharedNetwork(const NetworkPtr& shared_network) { - parent_network_ = shared_network; - } - - /// @brief Returns shared network name. - std::string getSharedNetworkName() const { - return (shared_network_name_); - } - - /// @brief Sets new shared network name. - /// - /// In certain cases the subnet must be associated with the shared network - /// but the shared network object is not available. In particular, subnets - /// are returned from the configuration database with only names of the - /// shared networks. The actual shared networks must be fetched from the - /// database using a separate query. In order to not loose associations - /// of subnets with shared networks, the configuration backends will use - /// this method to store the shared network names. The servers will later - /// use those names to associate subnets with shared network instances. - /// - /// @param shared_network_name New shared network name. - void setSharedNetworkName(const std::string& shared_network_name) { - shared_network_name_ = shared_network_name; + /// @param fetch_globals_fn Pointer to the function. + void setFetchGlobalsFn(FetchNetworkGlobalsFn fetch_globals_fn) { + fetch_globals_fn_ = fetch_globals_fn; } /// @brief Sets local name of the interface for which this network is @@ -258,7 +217,7 @@ public: /// /// @return Interface name as text. util::Optional getIface() const { - return (iface_name_); + return (getProperty(&Network::getIface, iface_name_, "interface")); }; /// @brief Sets information about relay @@ -402,7 +361,9 @@ public: /// @return whether in-pool host reservations are allowed. util::Optional getHostReservationMode() const { - return (getProperty(&Network::getHostReservationMode, host_reservation_mode_)); + return (getProperty(&Network::getHostReservationMode, + host_reservation_mode_, + "reservation-mode")); } /// @brief Sets host reservation mode. @@ -468,21 +429,32 @@ public: protected: - template + template util::Optional - getProperty(util::Optional(Network::*MethodPointer)() const, - PropertyType property) const { + getProperty(util::Optional(BaseType::*MethodPointer)() const, + util::Optional property, const std::string& global_name) const { if (property.unspecified()) { - auto sn = parent_network_.lock(); - if (sn) { - auto value = ((*sn).*MethodPointer)(); - if (!value.unspecified()) { - return (value); + auto parent = boost::dynamic_pointer_cast(parent_network_.lock()); + if (parent) { + auto parent_property = ((*parent).*MethodPointer)(); + if (!parent_property.unspecified()) { + return (parent_property); + + } + } + + if (fetch_globals_fn_) { + data::ConstElementPtr globals = fetch_globals_fn_(); + if (globals && (globals->getType() == data::Element::map)) { + data::ConstElementPtr global_param = globals->get(global_name); + if (global_param) { + return (data::ElementExtractor()(global_param)); + } } } } - return (property); + return (property); } /// @brief Holds interface name for which this network is selected. @@ -540,12 +512,13 @@ protected: /// a shared network this pointer is null. WeakNetworkPtr parent_network_; - /// @brief Shared network name. - std::string shared_network_name_; + /// @brief Pointer to the optional callback used to fetch globally + /// configured parameters inherited to the @c Network object. + FetchNetworkGlobalsFn fetch_globals_fn_; }; /// @brief Specialization of the @ref Network object for DHCPv4 case. -class Network4 : public Network { +class Network4 : public virtual Network { public: /// @brief Constructor. @@ -577,7 +550,8 @@ public: /// @return true if requests for unknown IP addresses should be rejected, /// false otherwise. util::Optional getAuthoritative() const { - return (authoritative_); + return (getProperty(&Network4::getAuthoritative, authoritative_, + "authoritative")); } /// @brief Sets the flag indicating if requests for unknown IP addresses @@ -652,7 +626,7 @@ private: }; /// @brief Specialization of the @ref Network object for DHCPv6 case. -class Network6 : public Network { +class Network6 : public virtual Network { public: /// @brief Constructor. diff --git a/src/lib/dhcpsrv/shared_network.cc b/src/lib/dhcpsrv/shared_network.cc index 679667aaee..ff81a39d93 100644 --- a/src/lib/dhcpsrv/shared_network.cc +++ b/src/lib/dhcpsrv/shared_network.cc @@ -244,16 +244,11 @@ public: namespace isc { namespace dhcp { -NetworkPtr -SharedNetwork4::sharedFromThis() { - return (shared_from_this()); -} - void SharedNetwork4::add(const Subnet4Ptr& subnet) { Impl::add(subnets_, subnet); // Associate the subnet with this network. - subnet->setSharedNetwork(sharedFromThis()); + subnet->setSharedNetwork(shared_from_this()); subnet->setSharedNetworkName(name_); } @@ -309,16 +304,11 @@ SharedNetwork4::toElement() const { return (map); } -NetworkPtr -SharedNetwork6::sharedFromThis() { - return (shared_from_this()); -} - void SharedNetwork6::add(const Subnet6Ptr& subnet) { Impl::add(subnets_, subnet); // Associate the subnet with this network. - subnet->setSharedNetwork(sharedFromThis()); + subnet->setSharedNetwork(shared_from_this()); subnet->setSharedNetworkName(name_); } diff --git a/src/lib/dhcpsrv/shared_network.h b/src/lib/dhcpsrv/shared_network.h index fc0f9220eb..185c27f01e 100644 --- a/src/lib/dhcpsrv/shared_network.h +++ b/src/lib/dhcpsrv/shared_network.h @@ -1,4 +1,4 @@ -// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2017-2019 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 @@ -9,8 +9,6 @@ #include #include -#include -#include #include #include #include @@ -41,9 +39,8 @@ struct SharedNetworkModificationTimeIndexTag { }; /// @brief Shared network holding IPv4 subnets. /// /// Specialization of the @ref Network4 class for IPv4 shared networks. -class SharedNetwork4 : public Network4, - public boost::enable_shared_from_this, - public AssignableNetwork { + class SharedNetwork4 : public virtual Network4, + public boost::enable_shared_from_this { public: /// @brief Constructor. @@ -53,13 +50,6 @@ public: : name_(name), subnets_() { } - /// @brief Returns shared pointer to this network. - /// - /// This method is required by the parent @ref AssignableNetwork class. - /// - /// @return Shared pointer to this object. - virtual NetworkPtr sharedFromThis(); - /// @brief Returns a name of the shared network. std::string getName() const { return (name_); @@ -206,9 +196,8 @@ typedef boost::multi_index_container< /// @brief Shared network holding IPv6 subnets. /// /// Specialization of the @ref Network6 class for IPv6 shared networks. -class SharedNetwork6 : public Network6, - public boost::enable_shared_from_this, - public AssignableNetwork { +class SharedNetwork6 : public virtual Network6, + public boost::enable_shared_from_this { public: /// @brief Constructor. @@ -218,13 +207,6 @@ public: : name_(name), subnets_() { } - /// @brief Returns shared pointer to this network. - /// - /// This method is required by the parent @ref AssignableNetwork class. - /// - /// @return Shared pointer to this object. - virtual NetworkPtr sharedFromThis(); - /// @brief Returns a name of the shared network. std::string getName() const { return (name_); diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index e3ae220549..d676c2380c 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/src/lib/dhcpsrv/subnet.cc @@ -59,7 +59,9 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len, last_allocated_ia_(lastAddrInPrefix(prefix, len)), last_allocated_ta_(lastAddrInPrefix(prefix, len)), last_allocated_pd_(lastAddrInPrefix(prefix, len)), - last_allocated_time_() { + last_allocated_time_(), + iface_(), + shared_network_name_() { if ((prefix.isV6() && len > 128) || (prefix.isV4() && len > 32)) { isc_throw(BadValue, diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 7b2abc4da3..676853b68f 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -11,8 +11,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -31,12 +31,7 @@ namespace isc { namespace dhcp { -class Subnet : public virtual data::UserContext, public data::CfgToElement { - - // Assignable network is our friend to allow it to call - // @ref Subnet::setSharedNetwork private function. - friend class AssignableNetwork; - +class Subnet : public virtual Network { public: /// @brief checks if specified address is in range @@ -231,6 +226,60 @@ public: static_id_ = 1; } + /// @brief Retrieves pointer to a shared network associated with a subnet. + /// + /// By implementing it as a template function we overcome a need to + /// include shared_network.h header file to specify return type explicitly. + /// The header can't be included because it would cause circular dependency + /// between subnet.h and shared_network.h. + /// + /// This method uses an argument to hold a return value to allow the compiler + /// to infer the return type without a need to call this function with an + /// explicit return type as template argument. + /// + /// @param [out] shared_network Pointer to the shared network where returned + /// value should be assigned. + /// + /// @tparam Type of the shared network, i.e. @ref SharedNetwork4 or a + /// @ref SharedNetwork6. + template + void getSharedNetwork(SharedNetworkPtrType& shared_network) const { + shared_network = boost::dynamic_pointer_cast< + typename SharedNetworkPtrType::element_type>(parent_network_.lock()); + } + + /// @brief Assigns shared network to a subnet. + /// + /// This method replaces any shared network associated with a subnet with + /// a new shared network. + /// + /// @param shared_network Pointer to a new shared network to be associated + /// with the subnet. + void setSharedNetwork(const NetworkPtr& shared_network) { + parent_network_ = shared_network; + } + + /// @brief Returns shared network name. + std::string getSharedNetworkName() const { + return (shared_network_name_); + } + + /// @brief Sets new shared network name. + /// + /// In certain cases the subnet must be associated with the shared network + /// but the shared network object is not available. In particular, subnets + /// are returned from the configuration database with only names of the + /// shared networks. The actual shared networks must be fetched from the + /// database using a separate query. In order to not loose associations + /// of subnets with shared networks, the configuration backends will use + /// this method to store the shared network names. The servers will later + /// use those names to associate subnets with shared network instances. + /// + /// @param shared_network_name New shared network name. + void setSharedNetworkName(const std::string& shared_network_name) { + shared_network_name_ = shared_network_name; + } + /// @brief Returns all pools (non-const variant) /// /// The reference is only valid as long as the object that returned it. @@ -385,6 +434,9 @@ protected: /// @brief Name of the network interface (if connected directly) std::string iface_; + + /// @brief Shared network name. + std::string shared_network_name_; }; /// @brief A generic pointer to either Subnet4 or Subnet6 object diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index b5c4610d78..b5e3349d4a 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -128,6 +128,7 @@ libdhcpsrv_unittests_SOURCES += triplet_unittest.cc libdhcpsrv_unittests_SOURCES += test_utils.cc test_utils.h libdhcpsrv_unittests_SOURCES += timer_mgr_unittest.cc libdhcpsrv_unittests_SOURCES += network_state_unittest.cc +libdhcpsrv_unittests_SOURCES += network_unittest.cc libdhcpsrv_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) if HAVE_MYSQL diff --git a/src/lib/dhcpsrv/tests/network_unittest.cc b/src/lib/dhcpsrv/tests/network_unittest.cc new file mode 100644 index 0000000000..996a9ee7b6 --- /dev/null +++ b/src/lib/dhcpsrv/tests/network_unittest.cc @@ -0,0 +1,98 @@ +// Copyright (C) 2019 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 +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include + +using namespace isc::data; +using namespace isc::dhcp; + +namespace { + +class TestNetwork; +typedef boost::shared_ptr TestNetworkPtr; + +class TestNetwork : public Network { +public: + + void setParent(TestNetworkPtr parent) { + parent_network_ = boost::dynamic_pointer_cast(parent); + } + +}; + +class NetworkTest : public ::testing::Test { +public: + + NetworkTest() + : globals_(Element::createMap()) { + } + + FetchNetworkGlobalsFn getFetchGlobalsFn() { + return (std::bind(&NetworkTest::fetchGlobalsFn, this)); + } + + ConstElementPtr fetchGlobalsFn() { + return (globals_); + } + + ElementPtr globals_; +}; + +TEST_F(NetworkTest, getPropertyNoParentNoChild) { + NetworkPtr net_child(new Network()); + EXPECT_TRUE(net_child->getIface().unspecified()); +} + +TEST_F(NetworkTest, getPropertyNoParentChild) { + NetworkPtr net_child(new Network()); + net_child->setIface("child_iface"); + EXPECT_EQ("child_iface", net_child->getIface().get()); +} + +TEST_F(NetworkTest, getPropertyParentNoChild) { + TestNetworkPtr net_child(new TestNetwork()); + EXPECT_TRUE(net_child->getIface().unspecified()); + + TestNetworkPtr net_parent(new TestNetwork()); + net_parent->setIface("parent_iface"); + EXPECT_EQ("parent_iface", net_parent->getIface().get()); + + ASSERT_NO_THROW(net_child->setParent(net_parent)); + + EXPECT_FALSE(net_child->getIface().unspecified()); + EXPECT_EQ("parent_iface", net_child->getIface().get()); +} + +TEST_F(NetworkTest, getPropertyParentChild) { + TestNetworkPtr net_child(new TestNetwork()); + net_child->setIface("child_iface"); + EXPECT_EQ("child_iface", net_child->getIface().get()); + + TestNetworkPtr net_parent(new TestNetwork()); + net_parent->setIface("parent_iface"); + EXPECT_EQ("parent_iface", net_parent->getIface().get()); + + ASSERT_NO_THROW(net_child->setParent(net_parent)); + + EXPECT_FALSE(net_child->getIface().unspecified()); + EXPECT_EQ("child_iface", net_child->getIface().get()); +} + +TEST_F(NetworkTest, getPropertyGlobalNoParentNoChild) { + TestNetworkPtr net_child(new TestNetwork()); + + globals_->set("interface", Element::create("global_iface")); + + net_child->setFetchGlobalsFn(getFetchGlobalsFn()); + + EXPECT_FALSE(net_child->getIface().unspecified()); +} + +}