From: Thomas Markwalder Date: Fri, 13 Nov 2015 15:16:11 +0000 (-0500) Subject: [4096] ClientClassDef now stores CfgOption not OptionCollection X-Git-Tag: trac4097a_base~1^2~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ceda1e2cd9f585f96d679002f963194f742f6b42;p=thirdparty%2Fkea.git [4096] ClientClassDef now stores CfgOption not OptionCollection The initial choice for storing options as an OptionCollection in the ClientClassDef was incorrect. They are now stored within a CfgOption which is symmetrical with how they are stored globally and in subnets. --- diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc index fc5c06409a..67a2f34620 100644 --- a/src/lib/dhcpsrv/client_class_def.cc +++ b/src/lib/dhcpsrv/client_class_def.cc @@ -21,8 +21,8 @@ namespace dhcp { ClientClassDef::ClientClassDef(const std::string& name, const ExpressionPtr& match_expr, - const OptionCollectionPtr& options) - : name_(name), match_expr_(match_expr), options_(options) { + const CfgOptionPtr& cfg_option) + : name_(name), match_expr_(match_expr), cfg_option_(cfg_option) { // Name can't be blank if (name_.empty()) { @@ -31,8 +31,8 @@ ClientClassDef::ClientClassDef(const std::string& name, // @todo Does it make sense for a class to NOT have match expression? // For classes without options, make sure we have an empty collection - if (!options_) { - options_.reset(new OptionCollection()); + if (!cfg_option_) { + cfg_option_.reset(new CfgOption()); } } @@ -59,26 +59,14 @@ ClientClassDef::setMatchExpr(const ExpressionPtr& match_expr) { match_expr_ = match_expr; } -const OptionCollectionPtr& -ClientClassDef::getOptions() const { - return (options_); +const CfgOptionPtr& +ClientClassDef::getCfgOption() const { + return (cfg_option_); } void -ClientClassDef::setOptions(const OptionCollectionPtr& options) { - options_ = options; -} - -OptionPtr -ClientClassDef::findOption(uint16_t option_code) const { - if (options_) { - isc::dhcp::OptionCollection::iterator it = options_->find(option_code); - if (it != options_->end()) { - return ((*it).second); - } - } - - return (OptionPtr()); +ClientClassDef::setCfgOption(const CfgOptionPtr& cfg_option) { + cfg_option_ = cfg_option; } std::ostream& operator<<(std::ostream& os, const ClientClassDef& x) { @@ -98,8 +86,8 @@ ClientClassDictionary::~ClientClassDictionary() { void ClientClassDictionary::addClass(const std::string& name, const ExpressionPtr& match_expr, - const OptionCollectionPtr& options) { - ClientClassDefPtr cclass(new ClientClassDef(name, match_expr, options)); + const CfgOptionPtr& cfg_option) { + ClientClassDefPtr cclass(new ClientClassDef(name, match_expr, cfg_option)); addClass(cclass); } diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h index 523c805120..01d56222ea 100644 --- a/src/lib/dhcpsrv/client_class_def.h +++ b/src/lib/dhcpsrv/client_class_def.h @@ -15,7 +15,7 @@ #ifndef CLIENT_CLASS_DEF_H #define CLIENT_CLASS_DEF_H -#include +#include #include #include @@ -53,7 +53,7 @@ class ClientClassDef { /// @param match_expr Expression the class will use to determine membership /// @param options Collection of options members should be given ClientClassDef(const std::string& name, const ExpressionPtr& match_expr, - const OptionCollectionPtr& options = OptionCollectionPtr()); + const CfgOptionPtr& options = CfgOptionPtr()); /// @brief Destructor virtual ~ClientClassDef(); @@ -75,19 +75,12 @@ class ClientClassDef { void setMatchExpr(const ExpressionPtr& match_expr); /// @brief Fetches the class's option collection - const OptionCollectionPtr& getOptions() const; + const CfgOptionPtr& getCfgOption() const; /// @brief Sets the class's option collection /// /// @param options the option collection to assign the class - void setOptions(const OptionCollectionPtr& options); - - /// @brief Fetches an option from the class's collection by code - /// - /// @param option_code Option code value of the desired option - /// @return A pointer to the option if found, otherwise an - /// empty pointer - OptionPtr findOption(uint16_t option_code) const; + void setCfgOption(const CfgOptionPtr& cfg_option); /// @brief Provides a convenient text representation of the class friend std::ostream& operator<<(std::ostream& os, const ClientClassDef& x); @@ -100,9 +93,8 @@ class ClientClassDef { /// this class. ExpressionPtr match_expr_; - /// @brief The collection of options members should be given - /// Currently this is a multimap, not sure we need/want that complexity - OptionCollectionPtr options_; + /// @brief The option data configuration for this class + CfgOptionPtr cfg_option_; }; /// @brief a pointer to an ClientClassDef @@ -134,7 +126,7 @@ class ClientClassDictionary { /// dictionary. See @ref dhcp::ClientClassDef::ClientClassDef() for /// others. void addClass(const std::string& name, const ExpressionPtr& match_expr, - const OptionCollectionPtr& options); + const CfgOptionPtr& options); /// @brief Adds a new class to the list /// diff --git a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc index 36b98ad545..8aaa713116 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc @@ -26,82 +26,85 @@ using namespace isc; namespace { +// Tests basic construction of ClientClassDef TEST(ClientClassDef, construction) { boost::scoped_ptr cclass; std::string name = "class1"; ExpressionPtr expr; - OptionCollectionPtr options; + CfgOptionPtr cfg_option; // Classes cannot have blank names - ASSERT_THROW(cclass.reset(new ClientClassDef("", expr, options)), BadValue); + ASSERT_THROW(cclass.reset(new ClientClassDef("", expr, cfg_option)), BadValue); - // Verify we can create a class with a name, expression, and no options + // Verify we can create a class with a name, expression, and no cfg_option ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr))); EXPECT_EQ(name, cclass->getName()); ASSERT_FALSE(cclass->getMatchExpr()); - // Verify we get an empty collection of options - options = cclass->getOptions(); - ASSERT_TRUE(options); - EXPECT_EQ(0, options->size()); + // Verify we get an empty collection of cfg_option + cfg_option = cclass->getCfgOption(); + ASSERT_TRUE(cfg_option); + //EXPECT_EQ(0, cfg_option->size()); } -TEST(ClientClassDef, optionsBasics) { +// Tests options operations. Note we just do the basics +// as CfgOption is heavily tested elsewhere. +TEST(ClientClassDef, cfgOptionBasics) { boost::scoped_ptr cclass; std::string name = "class1"; ExpressionPtr expr; - OptionCollectionPtr options; + CfgOptionPtr test_options; + CfgOptionPtr class_options; OptionPtr opt; // First construct the class with empty option pointer - ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr, options))); + ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr, test_options))); // We should get back a collection with no entries, // not an empty collection pointer - options = cclass->getOptions(); - ASSERT_TRUE(options); - EXPECT_EQ(0, options->size()); - - // We should not be able find an option - opt = cclass->findOption(17); - ASSERT_FALSE(opt); - - // Create an option container with two options - options.reset(new OptionCollection()); - EXPECT_NO_THROW(opt.reset(new Option(Option::V4, 17))); - options->insert(make_pair(17, opt)); - EXPECT_NO_THROW(opt.reset(new Option(Option::V4, 18))); - options->insert(make_pair(18, opt)); - - // Now remake the client class with options - ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr, options))); - - options = cclass->getOptions(); - ASSERT_TRUE(options); - EXPECT_EQ(2, options->size()); - - // We should be able to find option 17 - opt = cclass->findOption(17); - ASSERT_TRUE(opt); - EXPECT_EQ(17, opt->getType()); - - // We should be able to find option 18 - opt = cclass->findOption(18); - ASSERT_TRUE(opt); - EXPECT_EQ(18, opt->getType()); - - // We should not be able to find option 90 - opt = cclass->findOption(90); - ASSERT_FALSE(opt); + class_options = cclass->getCfgOption(); + ASSERT_TRUE(class_options); + + // Create an option container and add some options + OptionPtr option; + test_options.reset(new CfgOption()); + option.reset(new Option(Option::V4, 17, OptionBuffer(10, 0xFF))); + ASSERT_NO_THROW(test_options->add(option, false, "dhcp4")); + + option.reset(new Option(Option::V6, 101, OptionBuffer(10, 0xFF))); + ASSERT_NO_THROW(test_options->add(option, false, "isc")); + + option.reset(new Option(Option::V6, 100, OptionBuffer(10, 0xFF))); + ASSERT_NO_THROW(test_options->add(option, false, "dhcp6")); + + // Now remake the client class with cfg_option + ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr, test_options))); + class_options = cclass->getCfgOption(); + ASSERT_TRUE(class_options); + + // Now make sure we can find all the options + OptionDescriptor opt_desc = class_options->get("dhcp4",17); + ASSERT_TRUE(opt_desc.option_); + EXPECT_EQ(100, opt_desc.option_->getType()); + + opt_desc = class_options->get("isc",101); + ASSERT_TRUE(opt_desc.option_); + EXPECT_EQ(101, opt_desc.option_->getType()); + + opt_desc = class_options->get("dhcp6",100); + ASSERT_TRUE(opt_desc.option_); + EXPECT_EQ(100, opt_desc.option_->getType()); } +// Tests the basic operation of ClientClassDictionary +// This includes adding, finding, and removing classes TEST(ClientClassDictionary, basics) { ClientClassDictionaryPtr dictionary; ClientClassDefPtr cclass; ExpressionPtr expr; - OptionCollectionPtr options; + CfgOptionPtr cfg_option; // Verify constructor doesn't throw ASSERT_NO_THROW(dictionary.reset(new ClientClassDictionary())); @@ -113,19 +116,19 @@ TEST(ClientClassDictionary, basics) { EXPECT_EQ(0, classes->size()); // Verify that we can add classes with both addClass variants - // First addClass(name, expression, options) - ASSERT_NO_THROW(dictionary->addClass("cc1", expr, options)); - ASSERT_NO_THROW(dictionary->addClass("cc2", expr, options)); + // First addClass(name, expression, cfg_option) + ASSERT_NO_THROW(dictionary->addClass("cc1", expr, cfg_option)); + ASSERT_NO_THROW(dictionary->addClass("cc2", expr, cfg_option)); // Verify duplicate add attempt throws - ASSERT_THROW(dictionary->addClass("cc2", expr, options), + ASSERT_THROW(dictionary->addClass("cc2", expr, cfg_option), DuplicateClientClassDef); // Verify that you cannot add a class with no name. - ASSERT_THROW(dictionary->addClass("", expr, options), BadValue); + ASSERT_THROW(dictionary->addClass("", expr, cfg_option), BadValue); // Now with addClass(class pointer) - ASSERT_NO_THROW(cclass.reset(new ClientClassDef("cc3", expr, options))); + ASSERT_NO_THROW(cclass.reset(new ClientClassDef("cc3", expr, cfg_option))); ASSERT_NO_THROW(dictionary->addClass(cclass)); // Verify duplicate add attempt throws