[5374] Updated ClientClasses definition

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()->

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());
     }
 }
 

src/lib/dhcp/classify.cc

-// 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<ClientClass>() {
+    : list_(), set_() {
     std::vector<std::string> 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
-

src/lib/dhcp/classify.h

-// 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 <set>
 #include <string>
+#include <iterator>
+#include <list>
+#include <unordered_set>
 
 /// @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<ClientClass> {
+    /// 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<ClientClass>::const_iterator const_iterator;
+
         /// @brief Default constructor.
-        ClientClasses() : std::set<ClientClass>() {
+        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<ClientClass> list_;
+
+        /// @brief Set/unordered part
+        std::unordered_set<ClientClass> set_;
     };
 
 };

src/lib/dhcp/pkt.cc

-// 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);
     }
 }

src/lib/dhcp/tests/classify_unittest.cc

-// 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("."));
 }

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;
     }
 

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

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);

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());
 }