Commit baa06740 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[3268] Treat top-level scalars as a group of globals parameters

Restructured DCfgMgrBase to group the top level elements in a configuration
into scalars (strings, bools, ints, etc...) and objects (maps, lists, etc),
and parse the scalars first, then objects.  This permits the top level
scalars to be treated as a group of global parameters that are parsed first.

Ordered parsing is now relegated to only object elements. Scalars are parsed
first before any objects.

Also added the ability to reset config manager's context and rather than
than starting configuration parsing with a copy of the current context, it
starts with an empty context.

Modified unit tests accordingly.
parent a3598893
......@@ -97,15 +97,28 @@ DCfgContextBase::~DCfgContextBase() {
// *********************** DCfgMgrBase *************************
DCfgMgrBase::DCfgMgrBase(DCfgContextBasePtr context)
: parse_order_(), context_(context) {
if (!context_) {
isc_throw(DCfgMgrBaseError, "DCfgMgrBase ctor: context cannot be NULL");
}
: parse_order_() {
setContext(context);
}
DCfgMgrBase::~DCfgMgrBase() {
}
void
DCfgMgrBase::resetContext() {
DCfgContextBasePtr context = createNewContext();
setContext(context);
}
void
DCfgMgrBase::setContext(DCfgContextBasePtr& context) {
if (!context) {
isc_throw(DCfgMgrBaseError, "DCfgMgrBase: context cannot be NULL");
}
context_ = context;
}
isc::data::ConstElementPtr
DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
LOG_DEBUG(dctl_logger, DBGLVL_COMMAND,
......@@ -122,7 +135,9 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
// inconsistency if the parsing operation fails after the context has been
// modified. We need to preserve the original context here
// so as we can rollback changes when an error occurs.
DCfgContextBasePtr original_context = context_->clone();
// DCfgContextBasePtr original_context = context_->clone();
DCfgContextBasePtr original_context = context_;
resetContext();
// Answer will hold the result returned to the caller.
ConstElementPtr answer;
......@@ -131,15 +146,43 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
std::string element_id;
try {
// Grab a map of element_ids and their data values from the new
// configuration set.
const std::map<std::string, ConstElementPtr>& values_map =
config_set->mapValue();
// Split the configuration into two maps. The first containing only
// top-level scalar parameters (i.e. globals), the second containing
// non-scalar or object elements (maps, lists, etc...). This allows
// us to parse and validate all of the global values before we do
// objects which may depend on them.
ElementMap params_map;
ElementMap objects_map;
isc::dhcp::ConfigPair config_pair;
BOOST_FOREACH(config_pair, config_set->mapValue()) {
std::string element_id = config_pair.first;
isc::data::ConstElementPtr element = config_pair.second;
switch (element->getType()) {
case isc::data::Element::integer:
case isc::data::Element::real:
case isc::data::Element::boolean:
case isc::data::Element::string:
params_map[element_id] = element;
break;
default:
objects_map[element_id] = element;
break;
}
}
// Parse the global, scalar parameters. These are "committed" to
// the context to make them available during object parsing.
boost::shared_ptr<MapElement> params_config(new MapElement());
params_config->setValue(params_map);
buildParams(params_config);
// Now parse the configuration objects.
const ElementMap& values_map = objects_map;
// Use a pre-ordered list of element ids to parse the elements in a
// specific order if the list (parser_order_) is not empty; otherwise
// elements are parsed in the order the value_map presents them.
if (!parse_order_.empty()) {
// For each element_id in the parse order list, look for it in the
// value map. If the element exists in the map, pass it and it's
......@@ -207,6 +250,17 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
return (answer);
}
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 method to create the proper parser.
dhcp::ParserPtr parser(createConfigParser(param.first));
parser->build(param.second);
parser->commit();
}
}
void DCfgMgrBase::buildAndCommit(std::string& element_id,
isc::data::ConstElementPtr value) {
// Call derivation's implementation to create the appropriate parser
......
......@@ -25,6 +25,9 @@
namespace isc {
namespace d2 {
/// @brief Defines a map of ConstElementPtrs keyed by name
typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
/// @brief Exception thrown if the configuration manager encounters an error.
class DCfgMgrBaseError : public isc::Exception {
public:
......@@ -113,7 +116,7 @@ public:
/// into parsers.
///
/// @return returns a pointer to the Boolean Storage.
isc::dhcp::BooleanStoragePtr getBooleanStorage() {
isc::dhcp::BooleanStoragePtr& getBooleanStorage() {
return (boolean_values_);
}
......@@ -121,7 +124,7 @@ public:
/// into parsers.
///
/// @return returns a pointer to the uint32 Storage.
isc::dhcp::Uint32StoragePtr getUint32Storage() {
isc::dhcp::Uint32StoragePtr& getUint32Storage() {
return (uint32_values_);
}
......@@ -129,7 +132,7 @@ public:
/// into parsers.
///
/// @return returns a pointer to the string Storage.
isc::dhcp::StringStoragePtr getStringStorage() {
isc::dhcp::StringStoragePtr& getStringStorage() {
return (string_values_);
}
......@@ -184,6 +187,9 @@ private:
/// @brief Defines an unsorted, list of string Element IDs.
typedef std::vector<std::string> ElementIdList;
/// @brief Defines an unsorted, list of string Element IDs.
typedef std::vector<std::string> ElementIdList;
/// @brief Configuration Manager
///
/// DCfgMgrBase is an abstract class that provides the mechanisms for managing
......@@ -198,7 +204,16 @@ typedef std::vector<std::string> ElementIdList;
///
/// @code
/// make backup copy of configuration context
/// for each top level element in new configuration
/// Split top-level configuration elements into to sets:
/// 1. Set of scalar elements (strings, booleans, ints, etc..)
/// 2. Set of object elements (maps, lists, etc...)
/// For each entry in the scalar set:
/// get derivation-specific parser for element
/// run parser
/// update context with parsed results
/// break on error
///
/// For each entry in the object set;
/// get derivation-specific parser for element
/// run parser
/// update context with parsed results
......@@ -208,9 +223,9 @@ typedef std::vector<std::string> ElementIdList;
/// restore configuration context from backup
/// @endcode
///
/// After making a backup of the current context, it iterates over the top-level
/// elements in the new configuration. The order in which the elements are
/// processed is either:
/// The above structuring ensures that global parameters are parsed first
/// making them available during subsequent object element parsing. The order
/// in which the object elements are processed is either:
///
/// 1. Natural order presented by the configuration set
/// 2. Specific order determined by a list of element ids
......@@ -256,9 +271,10 @@ public:
/// @brief Adds a given element id to the end of the parse order list.
///
/// The order in which elements are retrieved from this is the order in
/// which they are added to the list. Derivations should use this method
/// to populate the parse order as part of their constructor.
/// The order in which object elements are retrieved from this is the
/// order in which they are added to the list. Derivations should use this
/// method to populate the parse order as part of their constructor.
/// Scalar parameters should NOT be included in this list.
///
/// @param element_id is the string name of the element as it will appear
/// in the configuration set.
......@@ -281,6 +297,20 @@ public:
}
protected:
/// @brief Parses a set of scalar configuration elements into global
/// parameters
///
/// For each scalar element in the set:
/// - create a parser for the element
/// - invoke the parser's build method
/// - invoke the parser's commit method
///
/// This will commit the values to context storage making them accessible
/// during object parsing.
///
/// @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.
......@@ -295,6 +325,21 @@ protected:
virtual isc::dhcp::ParserPtr
createConfigParser(const std::string& element_id) = 0;
/// @brief Abstract factory which creates a context instance.
///
/// @return Returns a DCfgContextBasePtr to the new context instance.
virtual DCfgContextBasePtr createNewContext() = 0;
/// @brief Replaces existing context with a new, emtpy context.
void resetContext();
/// @brief Update the current context.
///
/// Replaces the existing context with the given context.
/// @param context Pointer to the new context.
/// @throw DCfgMgrBaseError if context is NULL.
void setContext(DCfgContextBasePtr& context);
private:
/// @brief Parse a configuration element.
......
......@@ -18,6 +18,7 @@
#include <d2/d_cfg_mgr.h>
#include <d_test_stubs.h>
#include <boost/foreach.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <gtest/gtest.h>
......@@ -45,6 +46,11 @@ public:
virtual ~DCtorTestCfgMgr() {
}
/// @brief Dummy implementation as this method is abstract.
virtual DCfgContextBasePtr createNewContext() {
return (DCfgContextBasePtr());
}
/// @brief Dummy implementation as this method is abstract.
virtual isc::dhcp::ParserPtr
createConfigParser(const std::string& /* element_id */) {
......@@ -112,7 +118,7 @@ TEST(DCfgMgrBase, construction) {
/// 4. An unknown element error is handled.
TEST_F(DStubCfgMgrTest, basicParseTest) {
// Create a simple configuration.
string config = "{ \"test-value\": 1000 } ";
string config = "{ \"test-value\": [] } ";
ASSERT_TRUE(fromJSON(config));
// Verify that we can parse a simple configuration.
......@@ -151,6 +157,9 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
std::string charlie("charlie");
std::string bravo("bravo");
std::string alpha("alpha");
std::string string_test("string_test");
std::string uint32_test("uint32_test");
std::string bool_test("bool_test");
// Create the test configuration with the elements in "random" order.
......@@ -158,9 +167,15 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
// are in lexical order by element_id. This means that iterating over
// such an element set, will present the elements in lexical order. Should
// this change, this test will need to be modified accordingly.
string config = "{ \"bravo\": 2, "
" \"alpha\": 1, "
" \"charlie\": 3 } ";
string config = "{"
" \"string_test\": \"hoopla\", "
" \"bravo\": [], "
" \"uint32_test\": 55, "
" \"alpha\": {}, "
" \"charlie\": [], "
" \"bool_test\": true "
"} ";
ASSERT_TRUE(fromJSON(config));
// Verify that non-ordered parsing, results in an as-they-come parse order.
......@@ -169,6 +184,13 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
// present the elements in lexical order. Should this change, the expected
// order list below would need to be changed accordingly).
ElementIdList order_expected;
// scalar params should be first and lexically
order_expected.push_back(bool_test);
order_expected.push_back(string_test);
order_expected.push_back(uint32_test);
// objects second and lexically
order_expected.push_back(alpha);
order_expected.push_back(bravo);
order_expected.push_back(charlie);
......@@ -183,6 +205,9 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
// Verify that the parsed order matches what we expected.
EXPECT_TRUE(cfg_mgr_->parsed_order_ == order_expected);
for (int i = 0; i < cfg_mgr_->parsed_order_.size(); ++i) {
std::cout << i << ":" << cfg_mgr_->parsed_order_[i] << std::endl;
}
// Clear the manager's parse order "memory".
cfg_mgr_->parsed_order_.clear();
......@@ -212,8 +237,20 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
answer_ = cfg_mgr_->parseConfig(config_set_);
EXPECT_TRUE(checkAnswer(0));
// Build expected order
// primitives should be first and lexically
order_expected.clear();
order_expected.push_back(bool_test);
order_expected.push_back(string_test);
order_expected.push_back(uint32_test);
// objects second and by the parse order
order_expected.push_back(charlie);
order_expected.push_back(bravo);
order_expected.push_back(alpha);
// Verify that the parsed order is the order we configured.
EXPECT_TRUE(cfg_mgr_->getParseOrder() == cfg_mgr_->parsed_order_);
EXPECT_TRUE(cfg_mgr_->parsed_order_ == order_expected);
// Create a parse order list that has too many entries. Verify that
// when parsing the test config, it fails.
......@@ -233,24 +270,24 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) {
/// 1. Boolean parameters can be parsed and retrieved.
/// 2. Uint32 parameters can be parsed and retrieved.
/// 3. String parameters can be parsed and retrieved.
/// 4. Derivation-specific parameters can be parsed and retrieved.
/// 5. Parsing a second configuration, updates the existing context values
/// 4. Map elements can be parsed and retrieved.
/// 5. List elements can be parsed and retrieved.
/// 6. Parsing a second configuration, updates the existing context values
/// correctly.
TEST_F(DStubCfgMgrTest, simpleTypesTest) {
// Fetch a derivation specific pointer to the context.
DStubContextPtr context = getStubContext();
ASSERT_TRUE(context);
// Create a configuration with all of the parameters.
string config = "{ \"bool_test\": true , "
" \"uint32_test\": 77 , "
" \"string_test\": \"hmmm chewy\" , "
" \"extra_test\": 430 } ";
" \"map_test\" : {} , "
" \"list_test\": [] }";
ASSERT_TRUE(fromJSON(config));
// 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 boolean parameter was parsed correctly by retrieving
// its value from the context.
......@@ -270,22 +307,26 @@ TEST_F(DStubCfgMgrTest, simpleTypesTest) {
EXPECT_NO_THROW(context->getParam("string_test", actual_string));
EXPECT_EQ("hmmm chewy", actual_string);
// Verify that the "extra" parameter was parsed correctly by retrieving
// its value from the context.
uint32_t actual_extra = 0;
EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra));
EXPECT_EQ(430, actual_extra);
isc::data::ConstElementPtr object;
EXPECT_NO_THROW(context->getObjectParam("map_test", object));
EXPECT_TRUE(object);
EXPECT_NO_THROW(context->getObjectParam("list_test", object));
EXPECT_TRUE(object);
// Create a configuration which "updates" all of the parameter values.
string config2 = "{ \"bool_test\": false , "
" \"uint32_test\": 88 , "
" \"string_test\": \"ewww yuk!\" , "
" \"extra_test\": 11 } ";
" \"map_test2\" : {} , "
" \"list_test2\": [] }";
ASSERT_TRUE(fromJSON(config2));
// Verify that the configuration parses without error.
answer_ = cfg_mgr_->parseConfig(config_set_);
EXPECT_TRUE(checkAnswer(0));
context = getStubContext();
ASSERT_TRUE(context);
// Verify that the boolean parameter was updated correctly by retrieving
// its value from the context.
......@@ -305,31 +346,38 @@ TEST_F(DStubCfgMgrTest, simpleTypesTest) {
EXPECT_NO_THROW(context->getParam("string_test", actual_string));
EXPECT_EQ("ewww yuk!", actual_string);
// Verify that the "extra" parameter was updated correctly by retrieving
// its value from the context.
actual_extra = 0;
EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra));
EXPECT_EQ(11, actual_extra);
// Verify previous objects are not there.
EXPECT_THROW(context->getObjectParam("map_test", object),
isc::dhcp::DhcpConfigError);
EXPECT_THROW(context->getObjectParam("list_test", object),
isc::dhcp::DhcpConfigError);
// Verify new map object is there.
EXPECT_NO_THROW(context->getObjectParam("map_test2", object));
EXPECT_TRUE(object);
// Verify new list object is there.
EXPECT_NO_THROW(context->getObjectParam("list_test2", object));
EXPECT_TRUE(object);
}
/// @brief Tests that the configuration context is preserved after failure
/// during parsing causes a rollback.
/// 1. Verifies configuration context rollback.
TEST_F(DStubCfgMgrTest, rollBackTest) {
// Fetch a derivation specific pointer to the context.
DStubContextPtr context = getStubContext();
ASSERT_TRUE(context);
// Create a configuration with all of the parameters.
string config = "{ \"bool_test\": true , "
" \"uint32_test\": 77 , "
" \"string_test\": \"hmmm chewy\" , "
" \"extra_test\": 430 } ";
" \"map_test\" : {} , "
" \"list_test\": [] }";
ASSERT_TRUE(fromJSON(config));
// Verify that the configuration parses without error.
answer_ = cfg_mgr_->parseConfig(config_set_);
EXPECT_TRUE(checkAnswer(0));
DStubContextPtr context = getStubContext();
ASSERT_TRUE(context);
// Verify that all of parameters have the expected values.
bool actual_bool = false;
......@@ -344,16 +392,20 @@ TEST_F(DStubCfgMgrTest, rollBackTest) {
EXPECT_NO_THROW(context->getParam("string_test", actual_string));
EXPECT_EQ("hmmm chewy", actual_string);
uint32_t actual_extra = 0;
EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra));
EXPECT_EQ(430, actual_extra);
isc::data::ConstElementPtr object;
EXPECT_NO_THROW(context->getObjectParam("map_test", object));
EXPECT_TRUE(object);
EXPECT_NO_THROW(context->getObjectParam("list_test", object));
EXPECT_TRUE(object);
// Create a configuration which "updates" all of the parameter values
// plus one unknown at the end.
string config2 = "{ \"bool_test\": false , "
" \"uint32_test\": 88 , "
" \"string_test\": \"ewww yuk!\" , "
" \"extra_test\": 11 , "
" \"map_test2\" : {} , "
" \"list_test2\": [] , "
" \"zeta_unknown\": 33 } ";
ASSERT_TRUE(fromJSON(config2));
......@@ -361,9 +413,8 @@ TEST_F(DStubCfgMgrTest, rollBackTest) {
SimFailure::set(SimFailure::ftElementUnknown);
answer_ = cfg_mgr_->parseConfig(config_set_);
EXPECT_TRUE(checkAnswer(1));
// Refresh our local pointer.
context = getStubContext();
ASSERT_TRUE(context);
// Verify that all of parameters have the original values.
actual_bool = false;
......@@ -378,9 +429,11 @@ TEST_F(DStubCfgMgrTest, rollBackTest) {
EXPECT_NO_THROW(context->getParam("string_test", actual_string));
EXPECT_EQ("hmmm chewy", actual_string);
actual_extra = 0;
EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra));
EXPECT_EQ(430, actual_extra);
EXPECT_NO_THROW(context->getObjectParam("map_test", object));
EXPECT_TRUE(object);
EXPECT_NO_THROW(context->getObjectParam("list_test", object));
EXPECT_TRUE(object);
}
} // end of anonymous namespace
......@@ -22,7 +22,6 @@ namespace isc {
namespace d2 {
const char* valid_d2_config = "{ "
"\"interface\" : \"eth1\" , "
"\"ip_address\" : \"127.0.0.1\" , "
"\"port\" : 5031, "
"\"tsig_keys\": ["
......@@ -218,16 +217,18 @@ DStubController::~DStubController() {
// Initialize controller wrapper's static instance getter member.
DControllerTest::InstanceGetter DControllerTest::instanceGetter_ = NULL;
//************************** TestParser *************************
//************************** ObjectParser *************************
TestParser::TestParser(const std::string& param_name):param_name_(param_name) {
ObjectParser::ObjectParser(const std::string& param_name,
ObjectStoragePtr& object_values)
: param_name_(param_name), object_values_(object_values) {
}
TestParser::~TestParser(){
ObjectParser::~ObjectParser(){
}
void
TestParser::build(isc::data::ConstElementPtr new_config) {
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");
......@@ -237,29 +238,33 @@ TestParser::build(isc::data::ConstElementPtr new_config) {
}
void
TestParser::commit() {
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(): extra_values_(new isc::dhcp::Uint32Storage()) {
DStubContext::DStubContext(): object_values_(new ObjectStorage()) {
}
DStubContext::~DStubContext() {
}
void
DStubContext::getExtraParam(const std::string& name, uint32_t& value) {
value = extra_values_->getParam(name);
DStubContext::getObjectParam(const std::string& name,
isc::data::ConstElementPtr& value) {
value = object_values_->getParam(name);
}
isc::dhcp::Uint32StoragePtr
DStubContext::getExtraStorage() {
return (extra_values_);
ObjectStoragePtr&
DStubContext::getObjectStorage() {
return (object_values_);
}
DCfgContextBasePtr
......@@ -268,7 +273,7 @@ DStubContext::clone() {
}
DStubContext::DStubContext(const DStubContext& rhs): DCfgContextBase(rhs),
extra_values_(new isc::dhcp::Uint32Storage(*(rhs.extra_values_))) {
object_values_(new ObjectStorage(*(rhs.object_values_))) {
}
//************************** DStubCfgMgr *************************
......@@ -280,40 +285,43 @@ DStubCfgMgr::DStubCfgMgr()
DStubCfgMgr::~DStubCfgMgr() {
}
DCfgContextBasePtr
DStubCfgMgr::createNewContext() {
return (DCfgContextBasePtr (new DStubContext()));
}
isc::dhcp::ParserPtr
DStubCfgMgr::createConfigParser(const std::string& element_id) {
isc::dhcp::DhcpConfigParser* parser = NULL;
DStubContextPtr context =
boost::dynamic_pointer_cast<DStubContext>(getContext());
isc::dhcp::ParserPtr parser;
DStubContextPtr context
= boost::dynamic_pointer_cast<DStubContext>(getContext());
if (element_id == "bool_test") {
parser = new isc::dhcp::BooleanParser(element_id,
context->getBooleanStorage());
parser.reset(new isc::dhcp::
BooleanParser(element_id,
context->getBooleanStorage()));
} else if (element_id == "uint32_test") {
parser = new isc::dhcp::Uint32Parser(element_id,
context->getUint32Storage());
parser.reset(new isc::dhcp::Uint32Parser(element_id,
context->getUint32Storage()));
} else if (element_id == "string_test") {
parser = new isc::dhcp::StringParser(element_id,
context->getStringStorage());
} else if (element_id == "extra_test") {
parser = new isc::dhcp::Uint32Parser(element_id,
context->getExtraStorage());
parser.reset(new isc::dhcp::StringParser(element_id,
context->getStringStorage()));
} else {
// Fail only if SimFailure dictates we should. This makes it easier
// to test parse ordering, by permitting a wide range of element ids