From fc4a404d4052d1e06a3a9bce8ecd20c4e0d52c28 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 1 Feb 2017 11:30:41 -0500 Subject: [PATCH] [5110] DhcpConfigParser no longer used in libprocess or D2 src/lib/process/d_cfg_mgr.h src/lib/process/d_cfg_mgr.cc DCfgMgrBase::parseElement() - changed to void return DCfgMgrBase::createConfigParser() - deleted this method DCfgMgrBase::buildParams() DCfgMgrBase::buildAndCommit() - now only call parseElement() src/lib/process/tests/d_cfg_mgr_unittests.cc ParseElementMgr, ParseElementMgrTest - deleted, no longer relevant src/lib/process/testutils/d_test_stubs.h src/lib/process/testutils/d_test_stubs.cc ObjectParser - deleted this object DStubCfgMgr::createConfigParser() - deleted this method DStubCfgMgr::parseElement() - new method which replaces functionality formerly in createConfigParser src/bin/d2/d2_cfg_mgr.cc src/bin/d2/d2_cfg_mgr.h D2CfgMgr::createConfigParser() - deleted this method D2CfgMgr::parseElement() - changed to void return, and throws on unknown element id --- src/bin/d2/d2_cfg_mgr.cc | 25 +-- src/bin/d2/d2_cfg_mgr.h | 30 +--- src/lib/process/d_cfg_mgr.cc | 38 +--- src/lib/process/d_cfg_mgr.h | 42 +---- src/lib/process/tests/d_cfg_mgr_unittests.cc | 180 ------------------- src/lib/process/testutils/d_test_stubs.cc | 62 ++----- src/lib/process/testutils/d_test_stubs.h | 78 +++----- 7 files changed, 57 insertions(+), 398 deletions(-) diff --git a/src/bin/d2/d2_cfg_mgr.cc b/src/bin/d2/d2_cfg_mgr.cc index 7c1a5974ef..22d331eded 100644 --- a/src/bin/d2/d2_cfg_mgr.cc +++ b/src/bin/d2/d2_cfg_mgr.cc @@ -252,13 +252,13 @@ getFormat(const std::string& name, isc::data::ConstElementPtr value) { } // anon -bool +void D2CfgMgr::parseElement(const std::string& element_id, isc::data::ConstElementPtr element) { - - // Get D2 specific context. try { + // Get D2 specific context. D2CfgContextPtr context = getD2CfgContext(); + if ((element_id == "ip-address") || (element_id == "ncr-protocol") || (element_id == "ncr-format") || @@ -267,7 +267,7 @@ D2CfgMgr::parseElement(const std::string& element_id, // global scalar params require nothing extra be done } else if (element_id == "tsig-keys") { TSIGKeyInfoListParser parser; - getD2CfgContext()->setKeys(parser.parse(element)); + context->setKeys(parser.parse(element)); } else if (element_id == "forward-ddns") { DdnsDomainListMgrParser parser; DdnsDomainListMgrPtr mgr = parser.parse(element, element_id, @@ -279,8 +279,9 @@ D2CfgMgr::parseElement(const std::string& element_id, context->getKeys()); context->setReverseMgr(mgr); } else { - // not something we handle here - return (false); + // Shouldn't occur if the JSON parser is doing its job. + isc_throw(D2CfgError, "Unsupported element: " + << element_id << element->getPosition()); } } catch (const D2CfgError& ex) { // Should already have a specific error and postion info @@ -289,8 +290,6 @@ D2CfgMgr::parseElement(const std::string& element_id, isc_throw(D2CfgError, "element: " << element_id << " : " << ex.what() << element->getPosition()); } - - return (true); }; void @@ -367,15 +366,5 @@ D2CfgMgr::buildParams(isc::data::ConstElementPtr params_config) { getD2CfgContext()->getD2Params() = params; } -dhcp::ParserPtr -D2CfgMgr::createConfigParser(const std::string& config_id, - const isc::data::Element::Position& pos) { - isc_throw(NotImplemented, - "parser error: D2CfgMgr parameter not supported : " - " (" << config_id << pos << ")"); - - return (dhcp::ParserPtr()); -} - }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/bin/d2/d2_cfg_mgr.h b/src/bin/d2/d2_cfg_mgr.h index d96523f75b..a557161fe1 100644 --- a/src/bin/d2/d2_cfg_mgr.h +++ b/src/bin/d2/d2_cfg_mgr.h @@ -270,9 +270,7 @@ protected: /// /// @param element_id name of the element as it is expected in the cfg /// @param element value of the element as ElementPtr - /// - /// @return true if the element was parsed, false otherwise - virtual bool parseElement(const std::string& element_id, + virtual void parseElement(const std::string& element_id, isc::data::ConstElementPtr element); /// @brief Adds default values to the given config @@ -303,32 +301,6 @@ protected: /// -# ncr_format is invalid, currently only FMT_JSON is supported virtual void buildParams(isc::data::ConstElementPtr params_config); - /// @brief Given an element_id returns an instance of the appropriate - /// parser. - /// - /// It is responsible for top-level or outermost DHCP-DDNS configuration - /// elements (see dhcp-ddns.spec): - /// -# ip_address - /// -# port - /// -# dns_server_timeout - /// -# ncr_protocol - /// -# ncr_format - /// -# tsig_keys - /// -# forward_ddns - /// -# reverse_ddns - /// - /// @param element_id is the string name of the element as it will appear - /// in the configuration set. - /// @param pos position within the configuration text (or file) of element - /// to be parsed. This is passed for error messaging. - /// - /// @return returns a ParserPtr to the parser instance. - /// @throw throws DCfgMgrBaseError if an error occurs. - virtual isc::dhcp::ParserPtr - createConfigParser(const std::string& element_id, - const isc::data::Element::Position& pos = - isc::data::Element::Position()); - /// @brief Creates an new, blank D2CfgContext context /// /// This method is used at the beginning of configuration process to diff --git a/src/lib/process/d_cfg_mgr.cc b/src/lib/process/d_cfg_mgr.cc index befadcf005..3a994026b8 100644 --- a/src/lib/process/d_cfg_mgr.cc +++ b/src/lib/process/d_cfg_mgr.cc @@ -264,45 +264,15 @@ void DCfgMgrBase::buildParams(isc::data::ConstElementPtr params_config) { // Loop through scalars parsing them and committing them to storage. BOOST_FOREACH(dhcp::ConfigPair param, params_config->mapValue()) { - // Call derivation's element parser, if it handled the element - // go on to the next one, otherwise use the old methods - if (parseElement(param.first, param.second)) { - continue; - } - - // Call derivation's method to create the proper parser. - dhcp::ParserPtr parser(createConfigParser(param.first, - param.second->getPosition())); - parser->build(param.second); - parser->commit(); + // Call derivation's element parser to parse the element. + parseElement(param.first, param.second); } } void DCfgMgrBase::buildAndCommit(std::string& element_id, isc::data::ConstElementPtr value) { - // Call derivation's element parser, if it handled the element - // go on to the next one, otherwise use the old methods - if (parseElement(element_id, value)) { - return; - } - - // Call derivation's implementation to create the appropriate parser - // based on the element id. - ParserPtr parser = createConfigParser(element_id, value->getPosition()); - if (!parser) { - isc_throw(DCfgMgrBaseError, "Could not create parser"); - } - - // Invoke the parser's build method passing in the value. This will - // "convert" the Element form of value into the actual data item(s) - // and store them in parser's local storage. - parser->build(value); - - // Invoke the parser's commit method. This "writes" the data - // item(s) stored locally by the parser into the context. (Note that - // parsers are free to do more than update the context, but that is an - // nothing something we are concerned with here.) - parser->commit(); + // Call derivation's element parser to parse the element. + parseElement(element_id, value); } }; // end of isc::dhcp namespace diff --git a/src/lib/process/d_cfg_mgr.h b/src/lib/process/d_cfg_mgr.h index 3c0b088b94..1128c0657f 100644 --- a/src/lib/process/d_cfg_mgr.h +++ b/src/lib/process/d_cfg_mgr.h @@ -244,15 +244,10 @@ typedef std::vector ElementIdList; /// This allows a derivation to specify the order in which its elements are /// parsed if there are dependencies between elements. /// -/// To parse a given element, its id is passed into createConfigParser, -/// which returns an instance of the appropriate parser. This method is -/// abstract so the derivation's implementation determines the type of parser -/// created. This isolates the knowledge of specific element ids and which -/// application specific parsers to derivation. -/// -/// Once the parser has been created, it is used to parse the data value -/// associated with the element id and update the context with the parsed -/// results. +/// To parse a given element, its id along with the element itself, +/// is passed into the virtual method, @c parseElement. Derivations are +/// expected to converts the element into application specific object(s), +/// thereby isolating the CPL from application details. /// /// In the event that an error occurs, parsing is halted and the /// configuration context is restored from backup. @@ -331,19 +326,14 @@ protected: /// @brief Parses the an element using an alternate parsing mechanism /// - /// Each element to be parsed is passed first into this method to allow - /// alternate parsing mechanisms to process specific elements. The method - /// should return true if it has processed the element or false if the - /// element should be passed onto the original parsing mechanisms. - /// The default implementation simply returns false. + /// Each element to be parsed is passed into this method to be converted + /// into the requisite application object(s). /// /// @param element_id name of the element as it is expected in the cfg /// @param element value of the element as ElementPtr /// - /// @return true if the element was parsed, false otherwise - virtual bool parseElement(const std::string& element_id, + virtual void parseElement(const std::string& element_id, isc::data::ConstElementPtr element) { - return (false); }; /// @brief Parses a set of scalar configuration elements into global @@ -362,24 +352,6 @@ protected: /// @param params_config set of scalar configuration elements to parse virtual void buildParams(isc::data::ConstElementPtr params_config); - /// @brief Create a parser instance based on an element id. - /// - /// Given an element_id returns an instance of the appropriate parser. - /// This method is abstract, isolating any direct knowledge of element_ids - /// and parsers to within the application-specific derivation. - /// - /// @param element_id is the string name of the element as it will appear - /// in the configuration set. - /// @param pos position within the configuration text (or file) of element - /// to be parsed. This is passed for error messaging. - /// - /// @return returns a ParserPtr to the parser instance. - /// @throw throws DCfgMgrBaseError if an error occurs. - virtual isc::dhcp::ParserPtr - createConfigParser(const std::string& element_id, - const isc::data::Element::Position& pos - = isc::data::Element::Position()) = 0; - /// @brief Abstract factory which creates a context instance. /// /// This method is used at the beginning of configuration process to diff --git a/src/lib/process/tests/d_cfg_mgr_unittests.cc b/src/lib/process/tests/d_cfg_mgr_unittests.cc index f5407de91a..51125a02d2 100644 --- a/src/lib/process/tests/d_cfg_mgr_unittests.cc +++ b/src/lib/process/tests/d_cfg_mgr_unittests.cc @@ -45,13 +45,6 @@ public: return (DCfgContextBasePtr()); } - /// @brief Dummy implementation as this method is abstract. - virtual isc::dhcp::ParserPtr - createConfigParser(const std::string& /* element_id */, - const isc::data::Element::Position& /* pos */) { - return (isc::dhcp::ParserPtr()); - } - /// @brief Returns summary of configuration in the textual format. virtual std::string getConfigSummary(const uint32_t) { return (""); @@ -87,98 +80,6 @@ public: DStubCfgMgrPtr cfg_mgr_; }; -/// @brief Cfg manager which implements the parseElement() method -/// This allows testing managers which use the new and/or old parsing -/// mechanisms to parse configurations. Eventually the latter will -/// likely be removed. -class ParseElementMgr : public DStubCfgMgr { -public: - - /// @brief Constructor - ParseElementMgr(){ - } - - /// @brief Destructor - ~ParseElementMgr() { - } - - /// @brief Parse the given element if appropriate - /// - /// Overrides the DCfgMgrBase implementation. - /// Looks for the given element by name in the list of "parsable" - /// elements. If it is found, it is added to the map of "parsed" - /// elements. - /// - /// @param element_id name of the element as it is expected in the cfg - /// @param element value of the element as ElementPtr - /// @return true if the element was parsed, false otherwise - virtual bool parseElement(const std::string& element_id, - isc::data::ConstElementPtr element) { - std::string id; - BOOST_FOREACH(id, parsable_elements_) { - if (element_id == id) { - parsed_elements_.set(element_id, element); - return (true); - } - } - - return (false); - } - - /// @brief Adds default elements to the given config - /// - /// Overrides the DCfgMgrBase implementation. - /// Adds the string parameter, "defaulted_parm" with a - /// value of "default value" to mutable_cfg if isn't - /// already there. - /// - /// @param mutable_cfg config to modify - virtual void setCfgDefaults(isc::data::ElementPtr mutable_cfg) { - if (!mutable_cfg->contains("defaulted_parm")) { - ConstElementPtr def(new StringElement("default value")); - mutable_cfg->set("defaulted_parm", def); - } - } - - /// @brief List of element ids which should be parsed by parseElement - ElementIdList parsable_elements_; - - /// @brief Map of elements parsed by parseElement - MapElement parsed_elements_; -}; - -typedef boost::shared_ptr ParseElementMgrPtr; - -/// @brief Test fixture for testing a ParseElementMgr -class ParseElementMgrTest : public ConfigParseTest { -public: - - /// @brief Constructor - ParseElementMgrTest():cfg_mgr_(new ParseElementMgr()) { - } - - /// @brief Destructor - ~ParseElementMgrTest() { - } - - /// @brief Convenience method which returns a DStubContextPtr to the - /// configuration context. - /// - /// @return returns a DStubContextPtr. - DStubContextPtr getStubContext() { - return (boost::dynamic_pointer_cast - (cfg_mgr_->getContext())); - } - - /// @brief Adds an element id to the list of "parsable" elements - void addToParsableElements(const std::string& element_id) { - cfg_mgr_->parsable_elements_.push_back(element_id); - } - - /// @brief Configuration manager instance. - ParseElementMgrPtr cfg_mgr_; -}; - ///@brief Tests basic construction/destruction of configuration manager. /// Verifies that: /// 1. Proper construction succeeds. @@ -217,18 +118,6 @@ TEST_F(DStubCfgMgrTest, basicParseTest) { answer_ = cfg_mgr_->parseConfig(config_set_); EXPECT_TRUE(checkAnswer(0)); - // Verify that an error building the element is caught and returns a - // failed parse result. - SimFailure::set(SimFailure::ftElementBuild); - answer_ = cfg_mgr_->parseConfig(config_set_); - EXPECT_TRUE(checkAnswer(1)); - - // Verify that an error committing the element is caught and returns a - // failed parse result. - SimFailure::set(SimFailure::ftElementCommit); - answer_ = cfg_mgr_->parseConfig(config_set_); - EXPECT_TRUE(checkAnswer(1)); - // Verify that an unknown element error is caught and returns a failed // parse result. SimFailure::set(SimFailure::ftElementUnknown); @@ -570,73 +459,4 @@ TEST_F(DStubCfgMgrTest, paramPosition) { EXPECT_EQ(pos.file_, isc::data::Element::ZERO_POSITION().file_); } -// Tests that: -// -// 1. Elements not handled by the parseElement() method are -// handled by the old parsing mechanisms -// 2. Default values are supplied for elements not supplied in -// the configuration -TEST_F(ParseElementMgrTest, basic) { - // Create the test config - string config = "{ \"bool_test\": true , \n" - " \"uint32_test\": 77 , \n" - " \"parse_one\": 1, \n" - " \"parse_two\": 2, \n" - " \"parse_three\": \"3\", \n" - " \"string_test\": \"hmmm chewy\" }"; - ASSERT_TRUE(fromJSON(config)); - - // Add two elements to the list of elements handled by parseElement - addToParsableElements("parse_one"); - addToParsableElements("defaulted_parm"); - - // Verify that the configuration parses without error. - answer_ = cfg_mgr_->parseConfig(config_set_); - ASSERT_TRUE(checkAnswer(0)); - DStubContextPtr context = getStubContext(); - ASSERT_TRUE(context); - - // Verify that the list of parsed elements is as expected - // It should have two entries: "parse_one" and "defaulted_parm" - ASSERT_EQ(cfg_mgr_->parsed_elements_.size(), 2); - EXPECT_TRUE(cfg_mgr_->parsed_elements_.contains("parse_one")); - ConstElementPtr element = cfg_mgr_->parsed_elements_.get("parse_one"); - EXPECT_EQ(element->intValue(), 1); - - // "parse_two" should not be in the parsed list - EXPECT_FALSE(cfg_mgr_->parsed_elements_.contains("parse_two")); - - // "parse_three" should be there - EXPECT_TRUE(cfg_mgr_->parsed_elements_.contains("defaulted_parm")); - element = cfg_mgr_->parsed_elements_.get("defaulted_parm"); - EXPECT_EQ(element->stringValue(), "default value"); - - // Now verify the original mechanism elements were parsed correctly - // Verify that the boolean parameter was parsed correctly by retrieving - // its value from the context. - bool actual_bool = false; - isc::data::Element::Position pos; - EXPECT_NO_THROW(pos = context->getParam("bool_test", actual_bool)); - EXPECT_EQ(true, actual_bool); - EXPECT_EQ(1, pos.line_); - - // Verify that the uint32 parameter was parsed correctly by retrieving - // its value from the context. - uint32_t actual_uint32 = 0; - EXPECT_NO_THROW(pos = context->getParam("uint32_test", actual_uint32)); - EXPECT_EQ(77, actual_uint32); - EXPECT_EQ(2, pos.line_); - - // Verify that the string parameter was parsed correctly by retrieving - // its value from the context. - std::string actual_string = ""; - EXPECT_NO_THROW(pos = context->getParam("string_test", actual_string)); - EXPECT_EQ("hmmm chewy", actual_string); - EXPECT_EQ(6, pos.line_); - - // Verify that "parse_two" wasn't parsed by old parsing either - EXPECT_THROW(context->getParam("parse_two", actual_string, false), - isc::dhcp::DhcpConfigError); -} - } // end of anonymous namespace diff --git a/src/lib/process/testutils/d_test_stubs.cc b/src/lib/process/testutils/d_test_stubs.cc index 2c7f83aa14..28f738db3a 100644 --- a/src/lib/process/testutils/d_test_stubs.cc +++ b/src/lib/process/testutils/d_test_stubs.cc @@ -308,37 +308,6 @@ DControllerTest::InstanceGetter DControllerTest::instanceGetter_ = NULL; /// @brief Defines the name of the configuration file to use const char* DControllerTest::CFG_TEST_FILE = "d2-test-config.json"; -//************************** ObjectParser ************************* - -ObjectParser::ObjectParser(const std::string& param_name, - ObjectStoragePtr& object_values) - : param_name_(param_name), object_values_(object_values) { -} - -ObjectParser::~ObjectParser(){ -} - -void -ObjectParser::build(isc::data::ConstElementPtr new_config) { - if (SimFailure::shouldFailOn(SimFailure::ftElementBuild)) { - // Simulates an error during element data parsing. - isc_throw (DCfgMgrBaseError, "Simulated build exception"); - } - - value_ = new_config; -} - -void -ObjectParser::commit() { - if (SimFailure::shouldFailOn(SimFailure::ftElementCommit)) { - // Simulates an error while committing the parsed element data. - throw std::runtime_error("Simulated commit exception"); - } - - object_values_->setParam(param_name_, value_, - isc::data::Element::Position()); -} - //************************** DStubContext ************************* DStubContext::DStubContext(): object_values_(new ObjectStorage()) { @@ -381,22 +350,25 @@ DStubCfgMgr::createNewContext() { return (DCfgContextBasePtr (new DStubContext())); } -isc::dhcp::ParserPtr -DStubCfgMgr::createConfigParser(const std::string& element_id, - const isc::data::Element::Position& pos) { - isc::dhcp::ParserPtr parser; +void +DStubCfgMgr::parseElement(const std::string& element_id, + isc::data::ConstElementPtr element) { DStubContextPtr context = boost::dynamic_pointer_cast(getContext()); + if (element_id == "bool_test") { - parser.reset(new isc::dhcp:: - BooleanParser(element_id, - context->getBooleanStorage())); + bool value = element->boolValue(); + context->getBooleanStorage()->setParam(element_id, value, + element->getPosition()); } else if (element_id == "uint32_test") { - parser.reset(new isc::dhcp::Uint32Parser(element_id, - context->getUint32Storage())); + uint32_t value = element->intValue(); + context->getUint32Storage()->setParam(element_id, value, + element->getPosition()); + } else if (element_id == "string_test") { - parser.reset(new isc::dhcp::StringParser(element_id, - context->getStringStorage())); + std::string value = element->stringValue(); + context->getStringStorage()->setParam(element_id, value, + element->getPosition()); } else { // Fail only if SimFailure dictates we should. This makes it easier // to test parse ordering, by permitting a wide range of element ids @@ -404,15 +376,15 @@ DStubCfgMgr::createConfigParser(const std::string& element_id, if (SimFailure::shouldFailOn(SimFailure::ftElementUnknown)) { isc_throw(DCfgMgrBaseError, "Configuration parameter not supported: " << element_id - << pos); + << element->getPosition()); } // Going to assume anything else is an object element. - parser.reset(new ObjectParser(element_id, context->getObjectStorage())); + context->getObjectStorage()->setParam(element_id, element, + element->getPosition()); } parsed_order_.push_back(element_id); - return (parser); } }; // namespace isc::process diff --git a/src/lib/process/testutils/d_test_stubs.h b/src/lib/process/testutils/d_test_stubs.h index efb03e20b4..b7e4d344ed 100644 --- a/src/lib/process/testutils/d_test_stubs.h +++ b/src/lib/process/testutils/d_test_stubs.h @@ -600,51 +600,6 @@ public: typedef isc::dhcp::ValueStorage ObjectStorage; typedef boost::shared_ptr ObjectStoragePtr; -/// @brief Simple parser derivation for parsing object elements. -class ObjectParser : public isc::dhcp::DhcpConfigParser { -public: - - /// @brief Constructor - /// - /// See @ref DhcpConfigParser class for details. - /// - /// @param param_name name of the parsed parameter - ObjectParser(const std::string& param_name, ObjectStoragePtr& object_values); - - /// @brief Destructor - virtual ~ObjectParser(); - - /// @brief Builds parameter value. - /// - /// See @ref DhcpConfigParser class for details. - /// - /// @param new_config pointer to the new configuration - /// @throw throws DCfgMgrBaseError if the SimFailure is set to - /// ftElementBuild. This allows for the simulation of an - /// exception during the build portion of parsing an element. - virtual void build(isc::data::ConstElementPtr new_config); - - /// @brief Commits the parsed value to storage. - /// - /// See @ref DhcpConfigParser class for details. - /// - /// @throw throws DCfgMgrBaseError if SimFailure is set to ftElementCommit. - /// This allows for the simulation of an exception during the commit - /// portion of parsing an element. - virtual void commit(); - -private: - /// name of the parsed parameter - std::string param_name_; - - /// pointer to the parsed value of the parameter - isc::data::ConstElementPtr value_; - - /// Pointer to the storage where committed value is stored. - ObjectStoragePtr object_values_; -}; - - /// @brief Test Derivation of the DCfgContextBase class. /// /// This class is used to test basic functionality of configuration context. @@ -720,20 +675,29 @@ public: /// @brief Destructor virtual ~DStubCfgMgr(); - /// @brief Given an element_id returns an instance of the appropriate - /// parser. It supports the element ids as described in the class brief. + /// @brief Parses the given element into the appropriate object + /// + /// The method supports three named elements: + /// + /// -# "bool_test" + /// -# "uint32_test" + /// -# "string_test" + /// + /// which are parsed and whose value is then stored in the + /// the appropriate context value store. + /// + /// Any other element_id is treated generically and stored + /// in the context's object store, unless the simulated + /// error has been set to SimFailure::ftElementUnknown. /// - /// @param element_id is the string name of the element as it will appear - /// in the configuration set. - /// @param pos position within the configuration text (or file) of element - /// to be parsed. This is passed for error messaging. + /// @param element_id name of the element to parse + /// @param element Element to parse /// - /// @return returns a ParserPtr to the parser instance. - /// @throw throws DCfgMgrBaseError if SimFailure is ftElementUnknown. - virtual isc::dhcp::ParserPtr - createConfigParser(const std::string& element_id, - const isc::data::Element::Position& pos - = isc::data::Element::Position()); + /// @throw DCfgMgrBaseError if simulated error is set + /// to ftElementUnknown and element_id is not one of + /// the named elements. + virtual void parseElement(const std::string& element_id, + isc::data::ConstElementPtr element); /// @brief Returns a summary of the configuration in the textual format. /// -- 2.47.3