From: Francis Dupont Date: Fri, 24 Nov 2017 00:07:17 +0000 (+0100) Subject: [5374] Updated ClientClasses definition X-Git-Tag: trac5458a_base~14^2~31 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4edac2d90ede7b4cb8676e515943551760084ffb;p=thirdparty%2Fkea.git [5374] Updated ClientClasses definition --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 8912441fa4..39550c1b1c 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -159,10 +159,9 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, const ClientClasses& classes = query_->getClasses(); if (!classes.empty()) { - std::string joined_classes = boost::algorithm::join(classes, ", "); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_ASSIGNED) .arg(query_->getLabel()) - .arg(joined_classes); + .arg(classes.toText()); } }; @@ -381,9 +380,10 @@ Dhcpv4Exchange::setHostIdentifiers() { void Dhcpv4Exchange::setReservedClientClasses() { if (context_->currentHost() && query_) { - BOOST_FOREACH(const std::string& client_class, - context_->currentHost()->getClientClasses4()) { - query_->addClass(client_class); + const ClientClasses& classes = context_->currentHost()->getClientClasses4(); + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { + query_->addClass(*cclass); } } } @@ -1199,8 +1199,8 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) { // Each class in the incoming packet const ClientClasses& classes = ex.getQuery()->getClasses(); - for (ClientClasses::const_iterator cclass = classes.begin(); - cclass != classes.end(); ++cclass) { + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { // Find the client class definition for this class const ClientClassDefPtr& ccdef = CfgMgr::instance().getCurrentCfg()-> getClientClassDictionary()->findClass(*cclass); @@ -2249,8 +2249,8 @@ Dhcpv4Srv::setFixedFields(Dhcpv4Exchange& ex) { // Now we need to iterate over the classes assigned to the // query packet and find corresponding class definitions for it. - for (ClientClasses::const_iterator name = classes.begin(); - name != classes.end(); ++name) { + for (ClientClasses::const_iterator name = classes.cbegin(); + name != classes.cend(); ++name) { ClientClassDefMap::const_iterator cl = defs->find(*name); if (cl == defs->end()) { @@ -2960,8 +2960,8 @@ void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) { // Note getClientClassDictionary() cannot be null const ClientClassDefMapPtr& defs_ptr = CfgMgr::instance().getCurrentCfg()-> getClientClassDictionary()->getClasses(); - for (ClientClassDefMap::const_iterator it = defs_ptr->begin(); - it != defs_ptr->end(); ++it) { + for (ClientClassDefMap::const_iterator it = defs_ptr->cbegin(); + it != defs_ptr->cend(); ++it) { // Note second cannot be null const ExpressionPtr& expr_ptr = it->second->getMatchExpr(); // Nothing to do without an expression to evaluate @@ -3003,8 +3003,8 @@ Dhcpv4Srv::deferredUnpack(Pkt4Ptr& query) OptionDefinitionPtr def; // Iterate on client classes const ClientClasses& classes = query->getClasses(); - for (ClientClasses::const_iterator cclass = classes.begin(); - cclass != classes.end(); ++cclass) { + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { // Get the client class definition for this class const ClientClassDefPtr& ccdef = CfgMgr::instance().getCurrentCfg()-> diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index a0aa2cb111..0416883276 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -896,8 +896,8 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question, // Each class in the incoming packet const ClientClasses& classes = question->getClasses(); - for (ClientClasses::const_iterator cclass = classes.begin(); - cclass != classes.end(); ++cclass) { + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { // Find the client class definition for this class const ClientClassDefPtr& ccdef = CfgMgr::instance().getCurrentCfg()-> getClientClassDictionary()->findClass(*cclass); @@ -3087,8 +3087,8 @@ void Dhcpv6Srv::classifyPacket(const Pkt6Ptr& pkt) { // Note getClientClassDictionary() cannot be null const ClientClassDefMapPtr& defs_ptr = CfgMgr::instance().getCurrentCfg()-> getClientClassDictionary()->getClasses(); - for (ClientClassDefMap::const_iterator it = defs_ptr->begin(); - it != defs_ptr->end(); ++it) { + for (ClientClassDefMap::const_iterator it = defs_ptr->cbegin(); + it != defs_ptr->cend(); ++it) { // Note second cannot be null const ExpressionPtr& expr_ptr = it->second->getMatchExpr(); // Nothing to do without an expression to evaluate @@ -3127,18 +3127,18 @@ void Dhcpv6Srv::setReservedClientClasses(const Pkt6Ptr& pkt, const AllocEngine::ClientContext6& ctx) { if (ctx.currentHost() && pkt) { - BOOST_FOREACH(const std::string& client_class, - ctx.currentHost()->getClientClasses6()) { - pkt->addClass(client_class); + const ClientClasses& classes = ctx.currentHost()->getClientClasses6(); + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { + pkt->addClass(*cclass); } } const ClientClasses& classes = pkt->getClasses(); if (!classes.empty()) { - std::string joined_classes = boost::algorithm::join(classes, ", "); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLASS_ASSIGNED) .arg(pkt->getLabel()) - .arg(joined_classes); + .arg(classes.toText()); } } diff --git a/src/lib/dhcp/classify.cc b/src/lib/dhcp/classify.cc index 95fc5dd08b..e234598e94 100644 --- a/src/lib/dhcp/classify.cc +++ b/src/lib/dhcp/classify.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-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 @@ -17,7 +17,7 @@ namespace isc { namespace dhcp { ClientClasses::ClientClasses(const std::string& class_names) - : std::set() { + : list_(), set_() { std::vector split_text; boost::split(split_text, class_names, boost::is_any_of(","), boost::algorithm::token_compress_off); @@ -33,8 +33,8 @@ ClientClasses::ClientClasses(const std::string& class_names) std::string ClientClasses::toText(const std::string& separator) const { std::stringstream s; - for (const_iterator class_it = begin(); class_it != end(); ++class_it) { - if (class_it != begin()) { + for (const_iterator class_it = cbegin(); class_it != cend(); ++class_it) { + if (class_it != cbegin()) { s << separator; } s << *class_it; @@ -44,4 +44,3 @@ ClientClasses::toText(const std::string& separator) const { } // end of namespace isc::dhcp } // end of namespace isc - diff --git a/src/lib/dhcp/classify.h b/src/lib/dhcp/classify.h index 174930c348..14614313fc 100644 --- a/src/lib/dhcp/classify.h +++ b/src/lib/dhcp/classify.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-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 @@ -7,8 +7,10 @@ #ifndef CLASSIFY_H #define CLASSIFY_H -#include #include +#include +#include +#include /// @file classify.h /// @@ -36,34 +38,67 @@ namespace dhcp { /// @brief Container for storing client class names /// - /// Depending on how you look at it, this is either a little more than just - /// a set of strings or a client classifier that performs access control. - /// For now, it is a simple access list that may contain zero or more - /// class names. It is expected to grow in complexity once support for - /// client classes becomes more feature rich. - /// - /// Note: This class is derived from std::set which may not have Doxygen - /// documentation. See http://www.cplusplus.com/reference/set/set/. - class ClientClasses : public std::set { + /// Both a list to iterate on it in insert order and unordered + /// set of names for existence. + class ClientClasses { public: + /// @brief Type of iterators + typedef std::list::const_iterator const_iterator; + /// @brief Default constructor. - ClientClasses() : std::set() { + ClientClasses() : list_(), set_() { } /// @brief Constructor from comma separated values. /// /// @param class_names A string containing a client classes separated /// with commas. The class names are trimmed before insertion to the set. - ClientClasses(const std::string& class_names); + ClientClasses(const ClientClass& class_names); + + /// @brief Insert an element. + /// + /// @param class_name The name of the class to insert + void insert(const ClientClass& class_name) { + list_.push_back(class_name); + set_.insert(class_name); + } + + /// @brief Check if classes is empty. + bool empty() const { + return (list_.empty()); + } + + /// @brief Returns the number of classes. + /// + /// @note; in C++ 11 list size complexity is constant so + /// there is no advantage to use the set part. + size_t size() const { + return (list_.size()); + } + + /// @brief Iterator to the first element. + const_iterator cbegin() const { + return (list_.cbegin()); + } + + /// @brief Iterator to the past the end element. + const_iterator cend() const { + return (list_.cend()); + } /// @brief returns if class x belongs to the defined classes /// /// @param x client class to be checked /// @return true if x belongs to the classes - bool - contains(const ClientClass& x) const { - return (find(x) != end()); + bool contains(const ClientClass& x) const { + return (set_.count(x) != 0); + } + + /// @brief Clears containers. + void clear() { + list_.clear(); + set_.clear(); } /// @brief Returns all class names as text @@ -72,6 +107,13 @@ namespace dhcp { /// default separator comprises comma sign followed by space /// character. std::string toText(const std::string& separator = ", ") const; + + private: + /// @brief List/ordered part + std::list list_; + + /// @brief Set/unordered part + std::unordered_set set_; }; }; diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index bfee10e0e4..ac633f078c 100644 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-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 @@ -93,12 +93,12 @@ Pkt::delOption(uint16_t type) { bool Pkt::inClass(const std::string& client_class) { - return (classes_.find(client_class) != classes_.end()); + return (classes_.contains(client_class)); } void Pkt::addClass(const std::string& client_class) { - if (classes_.find(client_class) == classes_.end()) { + if (!classes_.contains(client_class)) { classes_.insert(client_class); } } diff --git a/src/lib/dhcp/tests/classify_unittest.cc b/src/lib/dhcp/tests/classify_unittest.cc index 215551eb06..14e9d58443 100644 --- a/src/lib/dhcp/tests/classify_unittest.cc +++ b/src/lib/dhcp/tests/classify_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-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 @@ -86,8 +86,8 @@ TEST(ClassifyTest, ClientClassesIterator) { bool seenbeta = false; bool seengamma = false; bool seendelta = false; - for (ClientClasses::const_iterator it = classes.begin(); - it != classes.end(); ++it) { + for (ClientClasses::const_iterator it = classes.cbegin(); + it != classes.cend(); ++it) { ++count; if (*it == "alpha") { seenalpha = true; @@ -124,10 +124,10 @@ TEST(ClassifyTest, ClientClassesToText) { classes.insert("gamma"); EXPECT_EQ("alpha, gamma", classes.toText()); - // Insert third class and make sure they get ordered alphabetically. + // Insert third class and make sure they get ordered in insert order. classes.insert("beta"); - EXPECT_EQ("alpha, beta, gamma", classes.toText()); + EXPECT_EQ("alpha, gamma, beta", classes.toText()); // Check non-standard separator. - EXPECT_EQ("alpha.beta.gamma", classes.toText(".")); + EXPECT_EQ("alpha.gamma.beta", classes.toText(".")); } diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc index 573047bdaf..5e9295a1f4 100644 --- a/src/lib/dhcpsrv/host.cc +++ b/src/lib/dhcpsrv/host.cc @@ -451,7 +451,7 @@ Host::toElement4() const { const ClientClasses& cclasses = getClientClasses4(); ElementPtr classes = Element::createList(); for (ClientClasses::const_iterator cclass = cclasses.cbegin(); - cclass != cclasses.end(); ++cclass) { + cclass != cclasses.cend(); ++cclass) { classes->add(Element::create(*cclass)); } map->set("client-classes", classes); @@ -508,7 +508,7 @@ Host::toElement6() const { const ClientClasses& cclasses = getClientClasses6(); ElementPtr classes = Element::createList(); for (ClientClasses::const_iterator cclass = cclasses.cbegin(); - cclass != cclasses.end(); ++cclass) { + cclass != cclasses.cend(); ++cclass) { classes->add(Element::create(*cclass)); } map->set("client-classes", classes); @@ -568,18 +568,18 @@ Host::toText() const { } // Add DHCPv4 client classes. - for (ClientClasses::const_iterator cclass = dhcp4_client_classes_.begin(); - cclass != dhcp4_client_classes_.end(); ++cclass) { + for (ClientClasses::const_iterator cclass = dhcp4_client_classes_.cbegin(); + cclass != dhcp4_client_classes_.cend(); ++cclass) { s << " dhcp4_class" - << std::distance(dhcp4_client_classes_.begin(), cclass) + << std::distance(dhcp4_client_classes_.cbegin(), cclass) << "=" << *cclass; } // Add DHCPv6 client classes. - for (ClientClasses::const_iterator cclass = dhcp6_client_classes_.begin(); - cclass != dhcp6_client_classes_.end(); ++cclass) { + for (ClientClasses::const_iterator cclass = dhcp6_client_classes_.cbegin(); + cclass != dhcp6_client_classes_.cend(); ++cclass) { s << " dhcp6_class" - << std::distance(dhcp6_client_classes_.begin(), cclass) + << std::distance(dhcp6_client_classes_.cbegin(), cclass) << "=" << *cclass; } diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index 75e46c14b4..78491996c0 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -333,7 +333,7 @@ GenericHostDataSourceTest::compareReservations6(IPv6ResrvRange resrv1, void GenericHostDataSourceTest::compareClientClasses(const ClientClasses& classes1, const ClientClasses& classes2) { - EXPECT_TRUE(std::equal(classes1.begin(), classes1.end(), classes2.begin())); + EXPECT_TRUE(std::equal(classes1.cbegin(), classes1.cend(), classes2.cbegin())); } void diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc index 5d5934a6ff..7f8681b23d 100644 --- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc @@ -410,8 +410,8 @@ TEST_F(HostReservationParserTest, dhcp4ClientClasses) { const ClientClasses& classes = hosts[0]->getClientClasses4(); ASSERT_EQ(2, classes.size()); - EXPECT_EQ(1, classes.count("foo")); - EXPECT_EQ(1, classes.count("bar")); + EXPECT_TRUE(classes.contains("foo")); + EXPECT_TRUE(classes.contains("bar")); CfgMgr::instance().setFamily(AF_INET); CfgHostsSubnet cfg_subnet(cfg_hosts, SubnetID(10)); @@ -881,8 +881,8 @@ TEST_F(HostReservationParserTest, dhcp6ClientClasses) { const ClientClasses& classes = hosts[0]->getClientClasses6(); ASSERT_EQ(2, classes.size()); - EXPECT_EQ(1, classes.count("foo")); - EXPECT_EQ(1, classes.count("bar")); + EXPECT_TRUE(classes.contains("foo")); + EXPECT_TRUE(classes.contains("bar")); // lower duid value boost::algorithm::to_lower(config); diff --git a/src/lib/dhcpsrv/tests/host_unittest.cc b/src/lib/dhcpsrv/tests/host_unittest.cc index fad37178ec..bf08403111 100644 --- a/src/lib/dhcpsrv/tests/host_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_unittest.cc @@ -1000,13 +1000,14 @@ TEST_F(HostTest, toText) { host->addClientClass6("hub"); host->addClientClass6("device"); + // Note that now classes are in insert order. EXPECT_EQ("duid=1112131415 hostname=myhost ipv4_reservation=(no)" " siaddr=(no)" " sname=(empty)" " file=(empty)" " ipv6_reservations=(none)" " dhcp4_class0=modem dhcp4_class1=router" - " dhcp6_class0=device dhcp6_class1=hub", + " dhcp6_class0=hub dhcp6_class1=device", host->toText()); }