Commit 60d2db6f authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[master] Merge branch 'trac3121'

Internal refactor, moved error logic from commit() to build() for
D2 parsers
parents cb531793 ac5e4786
......@@ -201,30 +201,7 @@ TSIGKeyInfoParser::build(isc::data::ConstElementPtr key_config) {
parser->build(config_pair.second);
parser->commit();
}
}
isc::dhcp::ParserPtr
TSIGKeyInfoParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "name") ||
(config_id == "algorithm") ||
(config_id == "secret")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else {
isc_throw(NotImplemented,
"parser error: TSIGKeyInfo parameter not supported: "
<< config_id);
}
// Return the new parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
TSIGKeyInfoParser::commit() {
std::string name;
std::string algorithm;
std::string secret;
......@@ -266,6 +243,33 @@ TSIGKeyInfoParser::commit() {
(*keys_)[name]=key_info;
}
isc::dhcp::ParserPtr
TSIGKeyInfoParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "name") ||
(config_id == "algorithm") ||
(config_id == "secret")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else {
isc_throw(NotImplemented,
"parser error: TSIGKeyInfo parameter not supported: "
<< config_id);
}
// Return the new parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
TSIGKeyInfoParser::commit() {
/// @todo if at some point TSIG keys need some form of runtime resource
/// initialization, such as creating some sort of hash instance in
/// crytpolib. Once TSIG is fully implemented under Trac #3432 we'll know.
}
// *********************** TSIGKeyInfoListParser *************************
TSIGKeyInfoListParser::TSIGKeyInfoListParser(const std::string& list_name,
......@@ -278,12 +282,12 @@ TSIGKeyInfoListParser::TSIGKeyInfoListParser(const std::string& list_name,
}
}
TSIGKeyInfoListParser::~TSIGKeyInfoListParser(){
TSIGKeyInfoListParser::~TSIGKeyInfoListParser() {
}
void
TSIGKeyInfoListParser::
build(isc::data::ConstElementPtr key_list){
build(isc::data::ConstElementPtr key_list) {
int i = 0;
isc::data::ConstElementPtr key_config;
// For each key element in the key list:
......@@ -299,6 +303,10 @@ build(isc::data::ConstElementPtr key_list){
parser->build(key_config);
parsers_.push_back(parser);
}
// Now that we know we have a valid list, commit that list to the
// area given to us during construction (i.e. to the d2 context).
*keys_ = *local_keys_;
}
void
......@@ -308,10 +316,6 @@ TSIGKeyInfoListParser::commit() {
BOOST_FOREACH(isc::dhcp::ParserPtr parser, parsers_) {
parser->commit();
}
// Now that we know we have a valid list, commit that list to the
// area given to us during construction (i.e. to the d2 context).
*keys_ = *local_keys_;
}
// *********************** DnsServerInfoParser *************************
......@@ -343,32 +347,6 @@ DnsServerInfoParser::build(isc::data::ConstElementPtr server_config) {
parser->commit();
}
}
isc::dhcp::ParserPtr
DnsServerInfoParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "hostname") ||
(config_id == "ip_address")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else if (config_id == "port") {
parser = new isc::dhcp::Uint32Parser(config_id,
local_scalars_.getUint32Storage());
} else {
isc_throw(NotImplemented,
"parser error: DnsServerInfo parameter not supported: "
<< config_id);
}
// Return the new parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
DnsServerInfoParser::commit() {
std::string hostname;
std::string ip_address;
uint32_t port = DnsServerInfo::STANDARD_DNS_PORT;
......@@ -407,6 +385,32 @@ DnsServerInfoParser::commit() {
servers_->push_back(serverInfo);
}
isc::dhcp::ParserPtr
DnsServerInfoParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "hostname") ||
(config_id == "ip_address")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else if (config_id == "port") {
parser = new isc::dhcp::Uint32Parser(config_id,
local_scalars_.getUint32Storage());
} else {
isc_throw(NotImplemented,
"parser error: DnsServerInfo parameter not supported: "
<< config_id);
}
// Return the new parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
DnsServerInfoParser::commit() {
}
// *********************** DnsServerInfoListParser *************************
DnsServerInfoListParser::DnsServerInfoListParser(const std::string& list_name,
......@@ -439,17 +443,16 @@ build(isc::data::ConstElementPtr server_list){
parser->build(server_config);
parsers_.push_back(parser);
}
}
void
DnsServerInfoListParser::commit() {
// Domains must have at least one server.
if (parsers_.size() == 0) {
isc_throw (D2CfgError, "Server List must contain at least one server");
}
}
// Invoke commit on each server parser. This will cause each one to
// create it's server instance and commit it to storage.
void
DnsServerInfoListParser::commit() {
// Invoke commit on each server parser.
BOOST_FOREACH(isc::dhcp::ParserPtr parser, parsers_) {
parser->commit();
}
......@@ -486,34 +489,8 @@ DdnsDomainParser::build(isc::data::ConstElementPtr domain_config) {
parser->build(config_pair.second);
parser->commit();
}
}
isc::dhcp::ParserPtr
DdnsDomainParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "name") ||
(config_id == "key_name")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else if (config_id == "dns_servers") {
// Server list parser is given in our local server storage. It will pass
// this down to its server parsers and is where they will write their
// server instances upon commit.
parser = new DnsServerInfoListParser(config_id, local_servers_);
} else {
isc_throw(NotImplemented,
"parser error: DdnsDomain parameter not supported: "
<< config_id);
}
// Return the new domain parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
DdnsDomainParser::commit() {
// Now construct the domain.
std::string name;
std::string key_name;
......@@ -547,7 +524,35 @@ DdnsDomainParser::commit() {
DdnsDomainPtr domain(new DdnsDomain(name, key_name, local_servers_));
// Add the new domain to the domain storage.
(*domains_)[name]=domain;
(*domains_)[name] = domain;
}
isc::dhcp::ParserPtr
DdnsDomainParser::createConfigParser(const std::string& config_id) {
DhcpConfigParser* parser = NULL;
// Based on the configuration id of the element, create the appropriate
// parser. Scalars are set to use the parser's local scalar storage.
if ((config_id == "name") ||
(config_id == "key_name")) {
parser = new isc::dhcp::StringParser(config_id,
local_scalars_.getStringStorage());
} else if (config_id == "dns_servers") {
// Server list parser is given in our local server storage. It will pass
// this down to its server parsers and is where they will write their
// server instances upon commit.
parser = new DnsServerInfoListParser(config_id, local_servers_);
} else {
isc_throw(NotImplemented,
"parser error: DdnsDomain parameter not supported: "
<< config_id);
}
// Return the new domain parser instance.
return (isc::dhcp::ParserPtr(parser));
}
void
DdnsDomainParser::commit() {
}
// *********************** DdnsDomainListParser *************************
......@@ -620,6 +625,9 @@ DdnsDomainListMgrParser::build(isc::data::ConstElementPtr domain_config) {
parser->build(config_pair.second);
parser->commit();
}
// Add the new domain to the domain storage.
mgr_->setDomains(local_domains_);
}
isc::dhcp::ParserPtr
......@@ -641,8 +649,6 @@ DdnsDomainListMgrParser::createConfigParser(const std::string& config_id) {
void
DdnsDomainListMgrParser::commit() {
// Add the new domain to the domain storage.
mgr_->setDomains(local_domains_);
}
......
......@@ -68,7 +68,7 @@ namespace d2 {
/// any scalars which belong to the manager as well as creating and invoking a
/// DdnsDomainListParser to parse its list of domain entries.
///
/// A DdnsDomainLiatParser creates and invokes DdnsDomainListParser for each
/// A DdnsDomainListParser creates and invokes DdnsDomainListParser for each
/// domain entry in its list.
///
/// A DdnsDomainParser handles the scalars which belong to the domain as well as
......@@ -191,7 +191,7 @@ public:
private:
/// @brief The name of the key.
///
/// This value is the unique identifeir thay domains use to
/// This value is the unique identifier that domains use to
/// to specify which TSIG key they need.
std::string name_;
......@@ -540,8 +540,9 @@ public:
/// @brief Performs the actual parsing of the given "tsig_key" element.
///
/// The results of the parsing are retained internally for use during
/// commit.
/// Parses a configuration for the elements needed to instantiate a
/// TSIGKeyInfo, validates those entries, creates a TSIGKeyInfo instance
/// then attempts to add to a list of keys
///
/// @param key_config is the "tsig_key" configuration to parse
virtual void build(isc::data::ConstElementPtr key_config);
......@@ -559,9 +560,9 @@ public:
/// @return returns a pointer to newly created parser.
virtual isc::dhcp::ParserPtr createConfigParser(const std::string&
config_id);
/// @brief Instantiates a DnsServerInfo from internal data values
/// saves it to the storage area pointed to by servers_.
/// @brief Commits the TSIGKeyInfo configuration
/// Currently this method is a NOP, as the key instance is created and
/// then added to a local list of keys in build().
virtual void commit();
private:
......@@ -611,14 +612,11 @@ public:
/// @param key_list_config is the list of "tsig_key" elements to parse.
virtual void build(isc::data::ConstElementPtr key_list_config);
/// @brief Iterates over the internal list of TSIGKeyInfoParsers,
/// invoking commit on each. This causes each parser to instantiate a
/// TSIGKeyInfo from its internal data values and add that key
/// instance to the local key storage area, local_keys_. If all of the
/// key parsers commit cleanly, then update the context key map (keys_)
/// with the contents of local_keys_. This is done to allow for duplicate
/// key detection while parsing the keys, but not get stumped by it
/// updating the context with a valid list.
/// @brief Commits the list of TSIG keys
///
/// Iterates over the internal list of TSIGKeyInfoParsers, invoking
/// commit on each one. Then commits the local list of keys to
/// storage.
virtual void commit();
private:
......@@ -629,7 +627,7 @@ private:
/// the list of newly created TSIGKeyInfo instances. This is given to us
/// as a constructor argument by an upper level.
TSIGKeyInfoMapPtr keys_;
/// @brief Local storage area to which individual key parsers commit.
TSIGKeyInfoMapPtr local_keys_;
......@@ -657,8 +655,10 @@ public:
virtual ~DnsServerInfoParser();
/// @brief Performs the actual parsing of the given "dns_server" element.
/// The results of the parsing are retained internally for use during
/// commit.
///
/// Parses a configuration for the elements needed to instantiate a
/// DnsServerInfo, validates those entries, creates a DnsServerInfo instance
/// then attempts to add to a list of servers.
///
/// @param server_config is the "dns_server" configuration to parse
virtual void build(isc::data::ConstElementPtr server_config);
......@@ -677,8 +677,9 @@ public:
virtual isc::dhcp::ParserPtr createConfigParser(const std::string&
config_id);
/// @brief Instantiates a DnsServerInfo from internal data values
/// saves it to the storage area pointed to by servers_.
/// @brief Commits the configured DnsServerInfo
/// Currently this method is a NOP, as the server instance is created and
/// then added to the list of servers in build().
virtual void commit();
private:
......@@ -729,10 +730,10 @@ public:
/// @param server_list_config is the list of "dns_server" elements to parse.
virtual void build(isc::data::ConstElementPtr server_list_config);
/// @brief Iterates over the internal list of DnsServerInfoParsers,
/// invoking commit on each. This causes each parser to instantiate a
/// DnsServerInfo from its internal data values and add that that server
/// instance to the storage area, servers_.
/// @brief Commits the list of DnsServerInfos
///
/// Iterates over the internal list of DdnsServerInfoParsers, invoking
/// commit on each one.
virtual void commit();
private:
......@@ -769,8 +770,10 @@ public:
virtual ~DdnsDomainParser();
/// @brief Performs the actual parsing of the given "ddns_domain" element.
/// The results of the parsing are retained internally for use during
/// commit.
///
/// Parses a configuration for the elements needed to instantiate a
/// DdnsDomain, validates those entries, creates a DdnsDomain instance
/// then attempts to add it to a list of domains.
///
/// @param domain_config is the "ddns_domain" configuration to parse
virtual void build(isc::data::ConstElementPtr domain_config);
......@@ -789,8 +792,9 @@ public:
virtual isc::dhcp::ParserPtr createConfigParser(const std::string&
config_id);
/// @brief Instantiates a DdnsDomain from internal data values
/// saves it to the storage area pointed to by domains_.
/// @brief Commits the configured DdnsDomain
/// Currently this method is a NOP, as the domain instance is created and
/// then added to the list of domains in build().
virtual void commit();
private:
......@@ -842,7 +846,7 @@ public:
/// @brief Performs the actual parsing of the given list "ddns_domain"
/// elements.
/// It iterates over each server entry in the list:
/// It iterates over each domain entry in the list:
/// 1. Instantiate a DdnsDomainParser for the entry
/// 2. Pass the element configuration to the parser's build method
/// 3. Add the parser instance to local storage
......@@ -854,10 +858,10 @@ public:
/// parse.
virtual void build(isc::data::ConstElementPtr domain_list_config);
/// @brief Iterates over the internal list of DdnsDomainParsers, invoking
/// commit on each. This causes each parser to instantiate a DdnsDomain
/// from its internal data values and add that domain instance to the
/// storage area, domains_.
/// @brief Commits the list of DdnsDomains
///
/// Iterates over the internal list of DdnsDomainParsers, invoking
/// commit on each one.
virtual void commit();
private:
......@@ -902,8 +906,10 @@ public:
virtual ~DdnsDomainListMgrParser();
/// @brief Performs the actual parsing of the given manager element.
/// The results of the parsing are retained internally for use during
/// commit.
///
/// Parses a configuration for the elements needed to instantiate a
/// DdnsDomainListMgr, validates those entries, then creates a
/// DdnsDomainListMgr.
///
/// @param mgr_config is the manager configuration to parse
virtual void build(isc::data::ConstElementPtr mgr_config);
......@@ -920,8 +926,9 @@ public:
virtual isc::dhcp::ParserPtr createConfigParser(const std::string&
config_id);
/// @brief Populates the DdnsDomainListMgr from internal data values
/// set during parsing.
/// @brief Commits the configured DdsnDomainListMgr
/// Currently this method is a NOP, as the manager instance is created
/// in build().
virtual void commit();
private:
......
......@@ -261,9 +261,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
"}";
ASSERT_TRUE(fromJSON(config));
// Verify that build succeeds but commit fails on blank name.
EXPECT_NO_THROW(parser_->build(config_set_));
EXPECT_THROW(parser_->commit(), D2CfgError);
// Verify that build fails on blank name.
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
// Config with a blank algorithm entry.
config = "{"
......@@ -274,9 +273,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
ASSERT_TRUE(fromJSON(config));
// Verify that build succeeds but commit fails on blank algorithm.
EXPECT_NO_THROW(parser_->build(config_set_));
EXPECT_THROW(parser_->commit(), D2CfgError);
// Verify that build fails on blank algorithm.
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
// Config with a blank secret entry.
config = "{"
......@@ -287,9 +285,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
ASSERT_TRUE(fromJSON(config));
// Verify that build succeeds but commit fails on blank secret.
EXPECT_NO_THROW(parser_->build(config_set_));
EXPECT_THROW(parser_->commit(), D2CfgError);
// Verify that build fails blank secret
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
}
/// @brief Verifies that TSIGKeyInfo parsing creates a proper TSIGKeyInfo
......@@ -347,10 +344,7 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) {
ASSERT_NO_THROW(parser.reset(new TSIGKeyInfoListParser("test", keys_)));
// Verify that the list builds without errors.
ASSERT_NO_THROW(parser->build(config_set_));
// Verify that the list commit fails.
EXPECT_THROW(parser->commit(), D2CfgError);
EXPECT_THROW(parser->build(config_set_), D2CfgError);
}
/// @brief Verifies that attempting to parse an invalid list of TSIGKeyInfo
......@@ -380,10 +374,7 @@ TEST_F(TSIGKeyInfoTest, duplicateTSIGKey) {
ASSERT_NO_THROW(parser.reset(new TSIGKeyInfoListParser("test", keys_)));
// Verify that the list builds without errors.
ASSERT_NO_THROW(parser->build(config_set_));
// Verify that the list commit fails.
EXPECT_THROW(parser->commit(), D2CfgError);
EXPECT_THROW(parser->build(config_set_), D2CfgError);
}
/// @brief Verifies a valid list of TSIG Keys parses correctly.
......@@ -450,20 +441,18 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
/// 3. Specifying a negative port number is not allowed.
TEST_F(DnsServerInfoTest, invalidEntry) {
// Create a config in which both host and ip address are supplied.
// Verify that it builds without throwing but commit fails.
// Verify that build fails.
std::string config = "{ \"hostname\": \"pegasus.tmark\", "
" \"ip_address\": \"127.0.0.1\" } ";
ASSERT_TRUE(fromJSON(config));
EXPECT_NO_THROW(parser_->build(config_set_));
EXPECT_THROW(parser_->commit(), D2CfgError);
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
// Neither host nor ip address supplied
// Verify that it builds without throwing but commit fails.
// Verify that builds fails.
config = "{ \"hostname\": \"\", "
" \"ip_address\": \"\" } ";
ASSERT_TRUE(fromJSON(config));
EXPECT_NO_THROW(parser_->build(config_set_));
EXPECT_THROW(parser_->commit(), D2CfgError);
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
// Create a config with a negative port number.
// Verify that build fails.
......@@ -554,11 +543,8 @@ TEST_F(ConfigParseTest, invalidServerList) {
isc::dhcp::ParserPtr parser;
ASSERT_NO_THROW(parser.reset(new DnsServerInfoListParser("test", servers)));
// Verify that the list builds without errors.
ASSERT_NO_THROW(parser->build(config_set_));
// Verify that the list commit fails.
EXPECT_THROW(parser->commit(), D2CfgError);
// Verify that build fails.
EXPECT_THROW(parser->build(config_set_), D2CfgError);
}
/// @brief Verifies that a list of DnsServerInfo entries parses correctly given
......@@ -623,9 +609,8 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) {
" \"port\": 300 } ] } ";
ASSERT_TRUE(fromJSON(config));
// Verify that the domain configuration builds but commit fails.
ASSERT_NO_THROW(parser_->build(config_set_));
ASSERT_THROW(parser_->commit(), isc::dhcp::DhcpConfigError);
// Verify that the domain configuration builds fails.
EXPECT_THROW(parser_->build(config_set_), isc::dhcp::DhcpConfigError);
// Create a domain configuration with an empty server list.
config = "{ \"name\": \"tmark.org\" , "
......@@ -635,7 +620,7 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) {
ASSERT_TRUE(fromJSON(config));
// Verify that the domain configuration build fails.
ASSERT_THROW(parser_->build(config_set_), D2CfgError);
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
// Create a domain configuration with a mal-formed server entry.
config = "{ \"name\": \"tmark.org\" , "
......@@ -646,7 +631,7 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) {
ASSERT_TRUE(fromJSON(config));
// Verify that the domain configuration build fails.
ASSERT_THROW(parser_->build(config_set_), isc::BadValue);
EXPECT_THROW(parser_->build(config_set_), isc::BadValue);
// Create a domain configuration without an defined key name
config = "{ \"name\": \"tmark.org\" , "
......@@ -656,9 +641,8 @@ TEST_F(DdnsDomainTest, invalidDdnsDomainEntry) {
" \"port\": 300 } ] } ";
ASSERT_TRUE(fromJSON(config));
// Verify that the domain configuration build succeeds but commit fails.
ASSERT_NO_THROW(parser_->build(config_set_));
ASSERT_THROW(parser_->commit(), D2CfgError);
// Verify that the domain configuration build fails.
EXPECT_THROW(parser_->build(config_set_), D2CfgError);
}
/// @brief Verifies the basics of parsing DdnsDomains.
......@@ -853,9 +837,8 @@ TEST_F(DdnsDomainTest, duplicateDomain) {
ASSERT_NO_THROW(list_parser.reset(
new DdnsDomainListParser("test", domains_, keys_)));
// Verify that the parse build succeeds but the commit fails.
ASSERT_NO_THROW(list_parser->build(config_set_));
ASSERT_THROW(list_parser->commit(), D2CfgError);
// Verify that the parse build fails.
EXPECT_THROW(list_parser->build(config_set_), D2CfgError);
}
/// @brief Tests construction of D2CfgMgr
......@@ -1013,7 +996,7 @@ TEST_F(D2CfgMgrTest, fullConfig) {
EXPECT_TRUE(cfg_mgr_->reverseUpdatesEnabled());
// Verify that parsing the exact same configuration a second time
// does not cause a duplicate value errors.
// does not cause a duplicate value errors.
answer_ = cfg_mgr_->parseConfig(config_set_);
ASSERT_TRUE(checkAnswer(0));
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment