Skip to content

GitLab

Commit ff8f84b6 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files
Options

[2634] Replaced DhcpConfigParser template function getParam<> with new 

template class ValueStorage in lib/dhcpsrv, dhcp4, and dhcp6.  Added unit
tests for new class in dhcpsrv/tests.
parent 49726757
Hide whitespace changes
Inline Side-by-side
Showing with 270 additions and 139 deletions
src/bin/dhcp4/config_parser.cc
View file @ ff8f84b6
......@@ -13,13 +13,13 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config/ccsession.h>
#include <dhcp4/config_parser.h>
#include <dhcp4/dhcp4_log.h>
#include <dhcp/libdhcp++.h>
#include <dhcp/option_definition.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcp4/config_parser.h>
#include <dhcpsrv/dbaccess_parser.h>
#include <dhcpsrv/dhcp_config_parser.h>
//#include <dhcpsrv/dhcp_config_parser.h>
#include <dhcpsrv/option_space_container.h>
#include <util/encode/hex.h>
#include <util/strutil.h>
......@@ -56,15 +56,6 @@ typedef isc::dhcp::DhcpConfigParser* ParserFactory(const std::string& config_id)
/// @brief a collection of factories that creates parsers for specified element names
typedef std::map<std::string, ParserFactory*> FactoryMap;
/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
typedef std::map<std::string, uint32_t> Uint32Storage;
/// @brief a collection of elements that store string values
typedef std::map<std::string, std::string> StringStorage;
/// @brief Storage for parsed boolean values.
typedef std::map<string, bool> BooleanStorage;
/// @brief Storage for option definitions.
typedef OptionSpaceContainer<OptionDefContainer,
OptionDefinitionPtr> OptionDefStorage;
......@@ -198,7 +189,7 @@ public:
/// @brief Put a parsed value to the storage.
virtual void commit() {
if (storage_ != NULL && !param_name_.empty()) {
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -292,7 +283,7 @@ public:
if (storage_ != NULL && !param_name_.empty()) {
// If a given parameter already exists in the storage we override
// its value. If it doesn't we insert a new element.
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -364,7 +355,7 @@ public:
if (storage_ != NULL && !param_name_.empty()) {
// If a given parameter already exists in the storage we override
// its value. If it doesn't we insert a new element.
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -744,7 +735,7 @@ private:
// Option code is held in the uint32_t storage but is supposed to
// be uint16_t value. We need to check that value in the configuration
// does not exceed range of uint8_t and is not zero.
uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
uint32_t option_code = uint32_values_.getParam("code");
if (option_code == 0) {
isc_throw(DhcpConfigError, "option code must not be zero."
<< " Option code '0' is reserved in DHCPv4.");
......@@ -753,9 +744,10 @@ private:
<< "', it must not exceed '"
<< std::numeric_limits<uint8_t>::max() << "'");
}
// Check that the option name has been specified, is non-empty and does not
// contain spaces.
std::string option_name = getParam<std::string>("name", string_values_);
// contain spaces
std::string option_name = string_values_.getParam("name");
if (option_name.empty()) {
isc_throw(DhcpConfigError, "name of the option with code '"
<< option_code << "' is empty");
......@@ -764,7 +756,7 @@ private:
<< "', space character is not allowed");
}
std::string option_space = getParam<std::string>("space", string_values_);
std::string option_space = string_values_.getParam("space");
if (!OptionSpace::validateName(option_space)) {
isc_throw(DhcpConfigError, "invalid option space name '"
<< option_space << "' specified for option '"
......@@ -805,8 +797,8 @@ private:
}
// Get option data from the configuration database ('data' field).
const std::string option_data = getParam<std::string>("data", string_values_);
const bool csv_format = getParam<bool>("csv-format", boolean_values_);
const std::string option_data = string_values_.getParam("data");
const bool csv_format = boolean_values_.getParam("csv-format");
// Transform string of hexadecimal digits into binary format.
std::vector<uint8_t> binary;
......@@ -1080,8 +1072,9 @@ private:
/// @brief Create option definition from the parsed parameters.
void createOptionDef() {
// Get the option space name and validate it.
std::string space = getParam<std::string>("space", string_values_);
std::string space = string_values_.getParam("space");
if (!OptionSpace::validateName(space)) {
isc_throw(DhcpConfigError, "invalid option space name '"
<< space << "'");
......@@ -1089,12 +1082,11 @@ private:
// Get other parameters that are needed to create the
// option definition.
std::string name = getParam<std::string>("name", string_values_);
uint32_t code = getParam<uint32_t>("code", uint32_values_);
std::string type = getParam<std::string>("type", string_values_);
bool array_type = getParam<bool>("array", boolean_values_);
std::string encapsulates = getParam<std::string>("encapsulate",
string_values_);
std::string name = string_values_.getParam("name");
uint32_t code = uint32_values_.getParam("code");
std::string type = string_values_.getParam("type");
bool array_type = boolean_values_.getParam("array");
std::string encapsulates = string_values_.getParam("encapsulate");
// Create option definition.
OptionDefinitionPtr def;
......@@ -1124,8 +1116,8 @@ private:
}
// The record-types field may carry a list of comma separated names
// of data types that form a record.
std::string record_types = getParam<std::string>("record-types",
string_values_);
std::string record_types = string_values_.getParam("record-types");
// Split the list of record types into tokens.
std::vector<std::string> record_tokens =
isc::util::str::tokens(record_types, ",");
......@@ -1422,13 +1414,16 @@ private:
///
/// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing failed.
void createSubnet() {
StringStorage::const_iterator it = string_values_.find("subnet");
if (it == string_values_.end()) {
std::string subnet_txt;
try {
subnet_txt = string_values_.getParam("subnet");
} catch (DhcpConfigError) {
// rethrow with precise error
isc_throw(DhcpConfigError,
"Mandatory subnet definition in subnet missing");
}
// Remove any spaces or tabs.
string subnet_txt = it->second;
boost::erase_all(subnet_txt, " ");
boost::erase_all(subnet_txt, "\t");
......@@ -1440,7 +1435,7 @@ private:
size_t pos = subnet_txt.find("/");
if (pos == string::npos) {
isc_throw(DhcpConfigError,
"Invalid subnet syntax (prefix/len expected):" << it->second);
"Invalid subnet syntax (prefix/len expected):" << subnet_txt);
}
// Try to create the address object. It also validates that
......@@ -1540,7 +1535,6 @@ private:
/// @throw NotImplemented if trying to create a parser for unknown config element
DhcpConfigParser* createSubnet4ConfigParser(const std::string& config_id) {
FactoryMap factories;
factories["valid-lifetime"] = Uint32Parser::factory;
factories["renew-timer"] = Uint32Parser::factory;
factories["rebind-timer"] = Uint32Parser::factory;
......@@ -1571,26 +1565,21 @@ private:
/// @throw DhcpConfigError when requested parameter is not present
Triplet<uint32_t> getParam(const std::string& name) {
uint32_t value = 0;
bool found = false;
Uint32Storage::iterator global = uint32_defaults.find(name);
if (global != uint32_defaults.end()) {
value = global->second;
found = true;
}
Uint32Storage::iterator local = uint32_values_.find(name);
if (local != uint32_values_.end()) {
value = local->second;
found = true;
}
if (found) {
return (Triplet<uint32_t>(value));
} else {
isc_throw(DhcpConfigError, "Mandatory parameter " << name
try {
// look for local value
value = uint32_values_.getParam(name);
} catch (DhcpConfigError) {
try {
// no local, use global value
value = uint32_defaults.getParam(name);
} catch (DhcpConfigError) {
isc_throw(DhcpConfigError, "Mandatory parameter " << name
<< " missing (no global default and no subnet-"
<< "specific value)");
}
}
return (Triplet<uint32_t>(value));
}
/// storage for subnet-specific uint32 values
......@@ -1859,7 +1848,7 @@ configureDhcp4Server(Dhcpv4Srv&, ConstElementPtr config_set) {
return (answer);
}
const std::map<std::string, uint32_t>& getUint32Defaults() {
const Uint32Storage& getUint32Defaults() {
return (uint32_defaults);
}
......
src/bin/dhcp4/config_parser.h
View file @ ff8f84b6
......@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <exceptions/exceptions.h>
#include <dhcpsrv/dhcp_config_parser.h>
#include <cc/data.h>
#include <stdint.h>
#include <string>
......@@ -66,7 +67,7 @@ configureDhcp4Server(Dhcpv4Srv&,
/// Uint32Parser works as expected.
///
/// @return a reference to a global uint32 values storage.
const std::map<std::string, uint32_t>& getUint32Defaults();
const Uint32Storage& getUint32Defaults();
}; // end of isc::dhcp namespace
}; // end of isc namespace
......
src/bin/dhcp4/tests/config_parser_unittest.cc
View file @ ff8f84b6
......@@ -52,15 +52,17 @@ public:
// Checks if global parameter of name have expected_value
void checkGlobalUint32(string name, uint32_t expected_value) {
const std::map<std::string, uint32_t>& uint32_defaults = getUint32Defaults();
std::map<std::string, uint32_t>::const_iterator it =
uint32_defaults.find(name);
if (it == uint32_defaults.end()) {
//const Uint32Storage& uint32_defaults = getUint32Defaults();
//const ValueStorage<uint32_t>& uint32_defaults = getUint32Defaults();
const Uint32Storage& uint32_defaults = getUint32Defaults();
try {
//uint32_defaults.addParam("boo", name);
uint32_t actual_value = uint32_defaults.getParam(name);
EXPECT_EQ(expected_value, actual_value);
} catch (DhcpConfigError) {
ADD_FAILURE() << "Expected uint32 with name " << name
<< " not found";
return;
}
EXPECT_EQ(expected_value, it->second);
}
// Checks if the result of DHCP server configuration has
......
src/bin/dhcp6/config_parser.cc
View file @ ff8f84b6
......@@ -66,15 +66,6 @@ typedef isc::dhcp::DhcpConfigParser* ParserFactory(const std::string& config_id)
/// @brief Collection of factories that create parsers for specified element names
typedef std::map<std::string, ParserFactory*> FactoryMap;
/// @brief Storage for parsed boolean values.
typedef std::map<string, bool> BooleanStorage;
/// @brief Collection of elements that store uint32 values (e.g. renew-timer = 900).
typedef std::map<string, uint32_t> Uint32Storage;
/// @brief Collection of elements that store string values.
typedef std::map<string, string> StringStorage;
/// @brief Storage for option definitions.
typedef OptionSpaceContainer<OptionDefContainer,
OptionDefinitionPtr> OptionDefStorage;
......@@ -209,7 +200,7 @@ public:
/// @brief Put a parsed value to the storage.
virtual void commit() {
if (storage_ != NULL && !param_name_.empty()) {
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -317,7 +308,7 @@ public:
if (storage_ != NULL) {
// If a given parameter already exists in the storage we override
// its value. If it doesn't we insert a new element.
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -393,7 +384,7 @@ public:
if (storage_ != NULL && !param_name_.empty()) {
// If a given parameter already exists in the storage we override
// its value. If it doesn't we insert a new element.
(*storage_)[param_name_] = value_;
storage_->setParam(param_name_, value_);
}
}
......@@ -774,7 +765,7 @@ private:
// Option code is held in the uint32_t storage but is supposed to
// be uint16_t value. We need to check that value in the configuration
// does not exceed range of uint16_t and is not zero.
uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
uint32_t option_code = uint32_values_.getParam("code");
if (option_code == 0) {
isc_throw(DhcpConfigError, "option code must not be zero."
<< " Option code '0' is reserved in DHCPv6.");
......@@ -785,7 +776,7 @@ private:
}
// Check that the option name has been specified, is non-empty and does not
// contain spaces.
std::string option_name = getParam<std::string>("name", string_values_);
std::string option_name = string_values_.getParam("name");
if (option_name.empty()) {
isc_throw(DhcpConfigError, "name of the option with code '"
<< option_code << "' is empty");
......@@ -794,7 +785,7 @@ private:
<< "', space character is not allowed");
}
std::string option_space = getParam<std::string>("space", string_values_);
std::string option_space = string_values_.getParam("space");
if (!OptionSpace::validateName(option_space)) {
isc_throw(DhcpConfigError, "invalid option space name '"
<< option_space << "' specified for option '"
......@@ -835,8 +826,8 @@ private:
}
// Get option data from the configuration database ('data' field).
const std::string option_data = getParam<std::string>("data", string_values_);
const bool csv_format = getParam<bool>("csv-format", boolean_values_);
const std::string option_data = string_values_.getParam("data");
const bool csv_format = boolean_values_.getParam("csv-format");
// Transform string of hexadecimal digits into binary format.
std::vector<uint8_t> binary;
......@@ -1109,7 +1100,7 @@ private:
/// @brief Create option definition from the parsed parameters.
void createOptionDef() {
// Get the option space name and validate it.
std::string space = getParam<std::string>("space", string_values_);
std::string space = string_values_.getParam("space");
if (!OptionSpace::validateName(space)) {
isc_throw(DhcpConfigError, "invalid option space name '"
<< space << "'");
......@@ -1117,12 +1108,11 @@ private:
// Get other parameters that are needed to create the
// option definition.
std::string name = getParam<std::string>("name", string_values_);
uint32_t code = getParam<uint32_t>("code", uint32_values_);
std::string type = getParam<std::string>("type", string_values_);
bool array_type = getParam<bool>("array", boolean_values_);
std::string encapsulates = getParam<std::string>("encapsulate",
string_values_);
std::string name = string_values_.getParam("name");
uint32_t code = uint32_values_.getParam("code");
std::string type = string_values_.getParam("type");
bool array_type = boolean_values_.getParam("array");
std::string encapsulates = string_values_.getParam("encapsulate");
// Create option definition.
OptionDefinitionPtr def;
......@@ -1153,8 +1143,7 @@ private:
// The record-types field may carry a list of comma separated names
// of data types that form a record.
std::string record_types = getParam<std::string>("record-types",
string_values_);
std::string record_types = string_values_.getParam("record-types");
// Split the list of record types into tokens.
std::vector<std::string> record_tokens =
isc::util::str::tokens(record_types, ",");
......@@ -1448,17 +1437,19 @@ private:
///
/// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing failed.
void createSubnet() {
// Find a subnet string.
StringStorage::const_iterator it = string_values_.find("subnet");
if (it == string_values_.end()) {
std::string subnet_txt;
try {
subnet_txt = string_values_.getParam("subnet");
} catch (DhcpConfigError) {
// rethrow with precise error
isc_throw(DhcpConfigError,
"Mandatory subnet definition in subnet missing");
}
// Remove any spaces or tabs.
string subnet_txt = it->second;
boost::erase_all(subnet_txt, " ");
boost::erase_all(subnet_txt, "\t");
// The subnet format is prefix/len. We are going to extract
// the prefix portion of a subnet string to create IOAddress
// object from it. IOAddress will be passed to the Subnet's
......@@ -1467,7 +1458,7 @@ private:
size_t pos = subnet_txt.find("/");
if (pos == string::npos) {
isc_throw(DhcpConfigError,
"Invalid subnet syntax (prefix/len expected):" << it->second);
"Invalid subnet syntax (prefix/len expected):" << subnet_txt);
}
// Try to create the address object. It also validates that
......@@ -1487,11 +1478,11 @@ private:
// Get interface name. If it is defined, then the subnet is available
// directly over specified network interface.
string iface;
StringStorage::const_iterator iface_iter = string_values_.find("interface");
if (iface_iter != string_values_.end()) {
iface = iface_iter->second;
std::string iface;
try {
iface = string_values_.getParam("interface");
} catch (DhcpConfigError) {
// iface not mandatory so swallow the exception
}
/// @todo: Convert this to logger once the parser is working reliably
......@@ -1624,26 +1615,21 @@ private:
/// @throw DhcpConfigError when requested parameter is not present
isc::dhcp::Triplet<uint32_t> getParam(const std::string& name) {
uint32_t value = 0;
bool found = false;
Uint32Storage::iterator global = uint32_defaults.find(name);
if (global != uint32_defaults.end()) {
value = global->second;
found = true;
}
Uint32Storage::iterator local = uint32_values_.find(name);
if (local != uint32_values_.end()) {
value = local->second;
found = true;
}
if (found) {
return (isc::dhcp::Triplet<uint32_t>(value));
} else {
isc_throw(isc::dhcp::DhcpConfigError, "Mandatory parameter " << name
try {
// look for local value
value = uint32_values_.getParam(name);
} catch (DhcpConfigError) {
try {
// no local, use global value
value = uint32_defaults.getParam(name);
} catch (DhcpConfigError) {
isc_throw(DhcpConfigError, "Mandatory parameter " << name
<< " missing (no global default and no subnet-"
<< "specific value)");
}
}
return (Triplet<uint32_t>(value));
}
/// storage for subnet-specific uint32 values
......
src/lib/dhcpsrv/dhcp_config_parser.h
View file @ ff8f84b6
......@@ -15,6 +15,12 @@
#ifndef DHCP_CONFIG_PARSER_H
#define DHCP_CONFIG_PARSER_H
#include <exceptions/exceptions.h>
#include <cc/data.h>
#include <stdint.h>
#include <string>
#include <map>
namespace isc {
namespace dhcp {
......@@ -122,38 +128,83 @@ public:
/// This method is expected to be called after @c build(), and only once.
/// The result is undefined otherwise.
virtual void commit() = 0;
};
protected:
/// @brief A template class that stores named elements of a given data type.
///
/// This template class is provides data value storage for configuration parameters
/// of a given data type. The values are stored by parmater name and as instances
/// of type "ValueType".
///
/// @tparam ValueType is the data type of the elements to store.
template<typename ValueType>
class ValueStorage {
public:
/// @brief Stores the the parameter and its value in the store.
///
/// If the parmater does not exist in the store, then it will be added,
/// otherwise its data value will be updated with the given value.
///
/// @param name is the name of the paramater to store.
/// @param value is the data value to store.
void setParam(const std::string name, const ValueType value) {
values_[name] = value;
}
/// @brief Returns the data value for the given parameter.
///
/// Finds and returns the data value for the given parameter.
/// @param name is the name of the parameter for which the data
/// value is desired.
///
/// @return The paramater's data value of type <ValueType>.
/// @throw DhcpConfigError if the parameter is not found.
ValueType getParam(const std::string& name) const {
typename std::map<std::string, ValueType>::const_iterator param
= values_.find(name);
if (param == values_.end()) {
isc_throw(DhcpConfigError, "missing parameter '"
<< name << "'");
}
ValueType value = param->second;
return (value);
}
/// @brief Return the parsed entry from the provided storage.
///
/// This method returns the parsed entry from the provided
/// storage. If the entry is not found, then exception is
/// thrown.
///
/// @param param_id name of the configuration entry.
/// @param storage storage where the entry should be searched.
/// @tparam ReturnType type of the returned value.
/// @tparam StorageType type of the storage.
///
/// @throw DhcpConfigError if the entry has not been found
/// in the storage.
template<typename ReturnType, typename StorageType>
static ReturnType getParam(const std::string& param_id,
const StorageType& storage) {
typename StorageType::const_iterator param = storage.find(param_id);
if (param == storage.end()) {
isc_throw(DhcpConfigError, "missing parameter '"
<< param_id << "'");
/// @brief Remove the parameter from the store.
///
/// Deletes the entry for the given parameter from the store if it
/// exists.
///
/// @param name is the name of the paramater to delete.
void delParam(const std::string name) {
values_.erase(name);
}
/// @brief Deletes all of the entries from the store.
///
void clear() {
values_.clear();
}
ReturnType value = param->second;
return (value);
}
private:
/// @brief An std::map of the data values, keyed by parameter names.
std::map<std::string, ValueType> values_;
};
} // end of isc::dhcp namespace
} // end of isc namespace
/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
typedef ValueStorage<uint32_t> Uint32Storage;
/// @brief a collection of elements that store string values
typedef ValueStorage<std::string> StringStorage;
/// @brief Storage for parsed boolean values.
typedef ValueStorage<bool> BooleanStorage;
}; // end of isc::dhcp namespace
}; // end of isc namespace
#endif // DHCP_CONFIG_PARSER_H
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
View file @ ff8f84b6
......@@ -15,6 +15,7 @@
#include <config.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/dhcp_config_parser.h>
#include <exceptions/exceptions.h>
#include <gtest/gtest.h>
......@@ -36,6 +37,107 @@ using boost::scoped_ptr;
namespace {
// This test verifies that BooleanStorage functions properly.
TEST(ValueStorageTest, BooleanTesting) {
BooleanStorage testStore;
// verify that we can add and retrieve them
testStore.setParam("firstBool", false);
testStore.setParam("secondBool", true);
EXPECT_FALSE(testStore.getParam("firstBool"));
EXPECT_TRUE(testStore.getParam("secondBool"));
// verify that we can update them
testStore.setParam("firstBool", true);
testStore.setParam("secondBool", false);
EXPECT_TRUE(testStore.getParam("firstBool"));
EXPECT_FALSE(testStore.getParam("secondBool"));