[5315_rebase] Changes after rebase and review:

 - Renamed SubnetIdIndexTag to avoid collision
 - Moved OptionDataParser to option_data_parser.cc|h
 - Updated hooks.xml to reflect recent changes

6 files changed, 24 insertions, 477 deletions

diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc
index af890d1100..6c4b4f9afb 100644
--- a/src/lib/dhcpsrv/cfg_subnets4.cc
+++ b/src/lib/dhcpsrv/cfg_subnets4.cc
@@ -41,7 +41,7 @@ CfgSubnets4::add(const Subnet4Ptr& subnet) {
 
 void
 CfgSubnets4::del(const ConstSubnet4Ptr& subnet) {
-    auto& index = subnets_.get<SubnetIdIndexTag>();
+    auto& index = subnets_.get<SubnetSubnetIdIndexTag>();
     auto subnet_it = index.find(subnet->getID());
     if (subnet_it == index.end()) {
         isc_throw(BadValue, "no subnet with ID of '" << subnet->getID()
diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc
index f4278a1878..e35b3773d4 100644
--- a/src/lib/dhcpsrv/cfg_subnets6.cc
+++ b/src/lib/dhcpsrv/cfg_subnets6.cc
@@ -40,7 +40,7 @@ CfgSubnets6::add(const Subnet6Ptr& subnet) {
 
 void
 CfgSubnets6::del(const ConstSubnet6Ptr& subnet) {
-    auto& index = subnets_.get<SubnetIdIndexTag>();
+    auto& index = subnets_.get<SubnetSubnetIdIndexTag>();
     auto subnet_it = index.find(subnet->getID());
     if (subnet_it == index.end()) {
         isc_throw(BadValue, "no subnet with ID of '" << subnet->getID()
@@ -212,18 +212,6 @@ CfgSubnets6::getSubnet(const SubnetID id) const {
     return (Subnet6Ptr());
 }
 
-
-bool
-CfgSubnets6::isDuplicate(const Subnet6& subnet) const {
-    for (Subnet6Collection::const_iterator subnet_it = subnets_.begin();
-         subnet_it != subnets_.end(); ++subnet_it) {
-        if ((*subnet_it)->getID() == subnet.getID()) {
-            return (true);
-        }
-    }
-    return (false);
-}
-
 void
 CfgSubnets6::removeStatistics() {
     using namespace isc::stats;
diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 58bddc0847..30995de0d0 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -168,157 +168,11 @@ void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr v
     srv_cfg.setControlSocketInfo(value);
 }
 
-// **************************** OptionDataParser *************************
-OptionDataParser::OptionDataParser(const uint16_t address_family)
-    : address_family_(address_family) {
-}
-
-std::pair<OptionDescriptor, std::string>
-OptionDataParser::parse(isc::data::ConstElementPtr single_option) {
-
-    // Try to create the option instance.
-    std::pair<OptionDescriptor, std::string> opt = createOption(single_option);
-
-    if (!opt.first.option_) {
-        isc_throw(isc::InvalidOperation,
-            "parser logic error: no option has been configured and"
-            " thus there is nothing to commit. Has build() been called?");
-    }
-
-    return (opt);
-}
-
-OptionalValue<uint32_t>
-OptionDataParser::extractCode(ConstElementPtr parent) const {
-    uint32_t code;
-    try {
-        code = getInteger(parent, "code");
-
-    } catch (const exception&) {
-        // The code parameter was not found. Return an unspecified
-        // value.
-        return (OptionalValue<uint32_t>());
-    }
-
-    if (code == 0) {
-        isc_throw(DhcpConfigError, "option code must not be zero "
-                  "(" << getPosition("code", parent) << ")");
-
-    } else if (address_family_ == AF_INET &&
-               code > std::numeric_limits<uint8_t>::max()) {
-        isc_throw(DhcpConfigError, "invalid option code '" << code
-                << "', it must not be greater than '"
-                  << static_cast<int>(std::numeric_limits<uint8_t>::max())
-                  << "' (" << getPosition("code", parent)
-                  << ")");
-
-    } else if (address_family_ == AF_INET6 &&
-               code > std::numeric_limits<uint16_t>::max()) {
-        isc_throw(DhcpConfigError, "invalid option code '" << code
-                << "', it must not exceed '"
-                  << std::numeric_limits<uint16_t>::max()
-                  << "' (" << getPosition("code", parent)
-                  << ")");
-
-    }
-
-    return (OptionalValue<uint32_t>(code, OptionalValueState(true)));
-}
-
-OptionalValue<std::string>
-OptionDataParser::extractName(ConstElementPtr parent) const {
-    std::string name;
-    try {
-        name = getString(parent, "name");
-
-    } catch (...) {
-        return (OptionalValue<std::string>());
-    }
 
-    if (name.find(" ") != std::string::npos) {
-        isc_throw(DhcpConfigError, "invalid option name '" << name
-                  << "', space character is not allowed ("
-                  << getPosition("name", parent) << ")");
-    }
-
-    return (OptionalValue<std::string>(name, OptionalValueState(true)));
-}
 
-std::string
-OptionDataParser::extractData(ConstElementPtr parent) const {
-    std::string data;
-    try {
-        data = getString(parent, "data");
-
-    } catch (...) {
-        // The "data" parameter was not found. Return an empty value.
-        return (data);
-    }
 
-    return (data);
-}
 
-OptionalValue<bool>
-OptionDataParser::extractCSVFormat(ConstElementPtr parent) const {
-    bool csv_format = true;
-    try {
-        csv_format = getBoolean(parent, "csv-format");
 
-    } catch (...) {
-        return (OptionalValue<bool>(csv_format));
-    }
-
-    return (OptionalValue<bool>(csv_format, OptionalValueState(true)));
-}
-
-std::string
-OptionDataParser::extractSpace(ConstElementPtr parent) const {
-    std::string space = address_family_ == AF_INET ?
-        DHCP4_OPTION_SPACE : DHCP6_OPTION_SPACE;
-    try {
-        space = getString(parent, "space");
-
-    } catch (...) {
-        return (space);
-    }
-
-    try {
-        if (!OptionSpace::validateName(space)) {
-            isc_throw(DhcpConfigError, "invalid option space name '"
-                      << space << "'");
-        }
-
-        if ((space == DHCP4_OPTION_SPACE) && (address_family_ == AF_INET6)) {
-            isc_throw(DhcpConfigError, "'" << DHCP4_OPTION_SPACE
-                      << "' option space name is reserved for DHCPv4 server");
-
-        } else if ((space == DHCP6_OPTION_SPACE) &&
-                   (address_family_ == AF_INET)) {
-            isc_throw(DhcpConfigError, "'" << DHCP6_OPTION_SPACE
-                      << "' option space name is reserved for DHCPv6 server");
-        }
-
-    } catch (std::exception& ex) {
-        // Append position of the option space parameter.
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << getPosition("space", parent) << ")");
-    }
-
-    return (space);
-}
-
-OptionalValue<bool>
-OptionDataParser::extractPersistent(ConstElementPtr parent) const {
-    bool persist = false;
-    try {
-        persist = getBoolean(parent, "always-send");
-
-    } catch (...) {
-        return (OptionalValue<bool>(persist));
-    }
-
-    return (OptionalValue<bool>(persist, OptionalValueState(true)));
-}
 
 template<typename SearchKey>
 OptionDefinitionPtr
@@ -346,163 +200,6 @@ OptionDataParser::findOptionDefinition(const std::string& option_space,
     return (def);
 }
 
-std::pair<OptionDescriptor, std::string>
-OptionDataParser::createOption(ConstElementPtr option_data) {
-    const Option::Universe universe = address_family_ == AF_INET ?
-        Option::V4 : Option::V6;
-
-    OptionalValue<uint32_t> code_param =  extractCode(option_data);
-    OptionalValue<std::string> name_param = extractName(option_data);
-    OptionalValue<bool> csv_format_param = extractCSVFormat(option_data);
-    OptionalValue<bool> persist_param = extractPersistent(option_data);
-    std::string data_param = extractData(option_data);
-    std::string space_param = extractSpace(option_data);
-
-    // Require that option code or option name is specified.
-    if (!code_param.isSpecified() && !name_param.isSpecified()) {
-        isc_throw(DhcpConfigError, "option data configuration requires one of"
-                  " 'code' or 'name' parameters to be specified"
-                  << " (" << option_data->getPosition() << ")");
-    }
-
-    // Try to find a corresponding option definition using option code or
-    // option name.
-    OptionDefinitionPtr def = code_param.isSpecified() ?
-        findOptionDefinition(space_param, code_param) :
-        findOptionDefinition(space_param, name_param);
-
-    // If there is no definition, the user must not explicitly enable the
-    // use of csv-format.
-    if (!def) {
-        // If explicitly requested that the CSV format is to be used,
-        // the option definition is a must.
-        if (csv_format_param.isSpecified() && csv_format_param) {
-            isc_throw(DhcpConfigError, "definition for the option '"
-                      << space_param << "." << name_param
-                      << "' having code '" << code_param
-                      << "' does not exist ("
-                      << getPosition("name", option_data)
-                      << ")");
-
-        // If there is no option definition and the option code is not specified
-        // we have no means to find the option code.
-        } else if (name_param.isSpecified() && !code_param.isSpecified()) {
-            isc_throw(DhcpConfigError, "definition for the option '"
-                      << space_param << "." << name_param
-                      << "' does not exist ("
-                      << getPosition("name", option_data)
-                      << ")");
-        }
-    }
-
-    // Transform string of hexadecimal digits into binary format.
-    std::vector<uint8_t> binary;
-    std::vector<std::string> data_tokens;
-
-    // If the definition is available and csv-format hasn't been explicitly
-    // disabled, we will parse the data as comma separated values.
-    if (def && (!csv_format_param.isSpecified() || csv_format_param)) {
-        // If the option data is specified as a string of comma
-        // separated values then we need to split this string into
-        // individual values - each value will be used to initialize
-        // one data field of an option.
-        // It is the only usage of the escape option: this allows
-        // to embed commas in individual values and to return
-        // for instance a string value with embedded commas.
-        data_tokens = isc::util::str::tokens(data_param, ",", true);
-
-    } else {
-        // Otherwise, the option data is specified as a string of
-        // hexadecimal digits that we have to turn into binary format.
-        try {
-            // The decodeHex function expects that the string contains an
-            // even number of digits. If we don't meet this requirement,
-            // we have to insert a leading 0.
-            if (!data_param.empty() && ((data_param.length() % 2) != 0)) {
-                data_param = data_param.insert(0, "0");
-            }
-            util::encode::decodeHex(data_param, binary);
-        } catch (...) {
-            isc_throw(DhcpConfigError, "option data is not a valid"
-                      << " string of hexadecimal digits: " << data_param
-                      << " ("
-                      << getPosition("data", option_data)
-                      << ")");
-        }
-    }
-
-    OptionPtr option;
-    OptionDescriptor desc(false);
-
-    if (!def) {
-        // @todo We have a limited set of option definitions initialized at
-        // the moment.  In the future we want to initialize option definitions
-        // for all options.  Consequently an error will be issued if an option
-        // definition does not exist for a particular option code. For now it is
-        // ok to create generic option if definition does not exist.
-        OptionPtr option(new Option(universe, static_cast<uint16_t>(code_param),
-                                    binary));
-
-        desc.option_ = option;
-        desc.persistent_ = persist_param.isSpecified() && persist_param;
-    } else {
-
-        // Option name is specified it should match the name in the definition.
-        if (name_param.isSpecified() && (def->getName() != name_param.get())) {
-            isc_throw(DhcpConfigError, "specified option name '"
-                      << name_param << "' does not match the "
-                      << "option definition: '" << space_param
-                      << "." << def->getName() << "' ("
-                      << getPosition("name", option_data)
-                      << ")");
-        }
-
-        // Option definition has been found so let's use it to create
-        // an instance of our option.
-        try {
-            bool use_csv = !csv_format_param.isSpecified() || csv_format_param;
-            OptionPtr option = use_csv ?
-                def->optionFactory(universe, def->getCode(), data_tokens) :
-                def->optionFactory(universe, def->getCode(), binary);
-            desc.option_ = option;
-            desc.persistent_ = persist_param.isSpecified() && persist_param;
-            if (use_csv) {
-                desc.formatted_value_ = data_param;
-            }
-        } catch (const isc::Exception& ex) {
-            isc_throw(DhcpConfigError, "option data does not match"
-                      << " option definition (space: " << space_param
-                      << ", code: " << def->getCode() << "): "
-                      << ex.what() << " ("
-                      << getPosition("data", option_data)
-                      << ")");
-        }
-    }
-
-    // All went good, so we can set the option space name.
-    return make_pair(desc, space_param);
-}
-
-// **************************** OptionDataListParser *************************
-OptionDataListParser::OptionDataListParser(//const std::string&,
-                                           //const CfgOptionPtr& cfg,
-                                           const uint16_t address_family)
-    : address_family_(address_family) {
-}
-
-
-void OptionDataListParser::parse(const CfgOptionPtr& cfg,
-                                 isc::data::ConstElementPtr option_data_list) {
-    OptionDataParser option_parser(address_family_);
-    BOOST_FOREACH(ConstElementPtr data, option_data_list->listValue()) {
-        std::pair<OptionDescriptor, std::string> option =
-            option_parser.parse(data);
-        // Use the option description to keep the formatted value
-        cfg->add(option.first, option.second);
-        cfg->encapsulate();
-    }
-}
-
 // ******************************** OptionDefParser ****************************
 
 std::pair<isc::dhcp::OptionDefinitionPtr, std::string>
diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h
index 5c1b5291ae..5e03b00d51 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h
@@ -341,163 +341,6 @@ public:
     void parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value);
 };
 
-/// @brief Parser for option data value.
-///
-/// This parser parses configuration entries that specify value of
-/// a single option. These entries include option name, option code
-/// and data carried by the option. The option data can be specified
-/// in one of the two available formats: binary value represented as
-/// a string of hexadecimal digits or a list of comma separated values.
-/// The format being used is controlled by csv-format configuration
-/// parameter. When setting this value to True, the latter format is
-/// used. The subsequent values in the CSV format apply to relevant
-/// option data fields in the configured option. For example the
-/// configuration: "data" : "192.168.2.0, 56, hello world" can be
-/// used to set values for the option comprising IPv4 address,
-/// integer and string data field. Note that order matters. If the
-/// order of values does not match the order of data fields within
-/// an option the configuration will not be accepted. If parsing
-/// is successful then an instance of an option is created and
-/// added to the storage provided by the calling class.
-class OptionDataParser : public isc::data::SimpleParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// @param address_family Address family: @c AF_INET or @c AF_INET6.
-    explicit OptionDataParser(const uint16_t address_family);
-
-    /// @brief Parses ElementPtr containing option definition
-    ///
-    /// This method parses ElementPtr containing the option definition,
-    /// instantiates the option for it and then returns a pair
-    /// of option descriptor (that holds that new option) and
-    /// a string that specifies the option space.
-    ///
-    /// Note: ElementPtr is expected to contain all fields. If your
-    /// ElementPtr does not have them, please use
-    /// @ref isc::data::SimpleParser::setDefaults to fill the missing fields
-    /// with default values.
-    ///
-    /// @param single_option ElementPtr containing option definition
-    /// @return Option object wrapped in option description and an option
-    ///         space
-    std::pair<OptionDescriptor, std::string>
-    parse(isc::data::ConstElementPtr single_option);
-private:
-
-    /// @brief Finds an option definition within an option space
-    ///
-    /// Given an option space and an option code, find the corresponding
-    /// option definition within the option definition storage.
-    ///
-    /// @param option_space name of the parameter option space
-    /// @param search_key an option code or name to be used to lookup the
-    /// option definition.
-    /// @tparam A numeric type for searching using an option code or the
-    /// string for searching using the option name.
-    ///
-    /// @return OptionDefinitionPtr of the option definition or an
-    /// empty OptionDefinitionPtr if not found.
-    /// @throw DhcpConfigError if the option space requested is not valid
-    /// for this server.
-    template<typename SearchKey>
-    OptionDefinitionPtr findOptionDefinition(const std::string& option_space,
-                                             const SearchKey& search_key) const;
-
-    /// @brief Create option instance.
-    ///
-    /// Creates an instance of an option and adds it to the provided
-    /// options storage. If the option data parsed by \ref build function
-    /// are invalid or insufficient this function emits an exception.
-    ///
-    /// @param option_data An element holding data for a single option being
-    /// created.
-    ///
-    /// @return created option descriptor
-    ///
-    /// @throw DhcpConfigError if parameters provided in the configuration
-    /// are invalid.
-    std::pair<OptionDescriptor, std::string>
-    createOption(isc::data::ConstElementPtr option_data);
-
-    /// @brief Retrieves parsed option code as an optional value.
-    ///
-    /// @param parent A data element holding full option data configuration.
-    ///
-    /// @return Option code, possibly unspecified.
-    /// @throw DhcpConfigError if option code is invalid.
-    util::OptionalValue<uint32_t>
-    extractCode(data::ConstElementPtr parent) const;
-
-    /// @brief Retrieves parsed option name as an optional value.
-    ///
-    /// @param parent A data element holding full option data configuration.
-    ///
-    /// @return Option name, possibly unspecified.
-    /// @throw DhcpConfigError if option name is invalid.
-    util::OptionalValue<std::string>
-    extractName(data::ConstElementPtr parent) const;
-
-    /// @brief Retrieves csv-format parameter as an optional value.
-    ///
-    /// @return Value of the csv-format parameter, possibly unspecified.
-    util::OptionalValue<bool> extractCSVFormat(data::ConstElementPtr parent) const;
-
-    /// @brief Retrieves option data as a string.
-    ///
-    /// @param parent A data element holding full option data configuration.
-    /// @return Option data as a string. It will return empty string if
-    /// option data is unspecified.
-    std::string extractData(data::ConstElementPtr parent) const;
-
-    /// @brief Retrieves option space name.
-    ///
-    /// If option space name is not specified in the configuration the
-    /// 'dhcp4' or 'dhcp6' option space name is returned, depending on
-    /// the universe specified in the parser context.
-    ///
-    /// @param parent A data element holding full option data configuration.
-    ///
-    /// @return Option space name.
-    std::string extractSpace(data::ConstElementPtr parent) const;
-
-    /// @brief Retrieves persistent/always-send parameter as an optional value.
-    ///
-    /// @return Value of the persistent parameter, possibly unspecified.
-    util::OptionalValue<bool> extractPersistent(data::ConstElementPtr parent) const;
-
-    /// @brief Address family: @c AF_INET or @c AF_INET6.
-    uint16_t address_family_;
-};
-
-/// @brief Parser for option data values within a subnet.
-///
-/// This parser iterates over all entries that define options
-/// data for a particular subnet and creates a collection of options.
-/// If parsing is successful, all these options are added to the Subnet
-/// object.
-class OptionDataListParser : public isc::data::SimpleParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// @param address_family Address family: @c AF_INET or AF_INET6
-    explicit OptionDataListParser(const uint16_t address_family);
-
-    /// @brief Parses a list of options, instantiates them and stores in cfg
-    ///
-    /// This method expects to get a list of options in option_data_list,
-    /// iterates over them, creates option objects, wraps them with
-    /// option descriptor and stores in specified cfg.
-    ///
-    /// @param cfg created options will be stored here
-    /// @param option_data_list configuration that describes the options
-    void parse(const CfgOptionPtr& cfg,
-               isc::data::ConstElementPtr option_data_list);
-private:
-    /// @brief Address family: @c AF_INET or @c AF_INET6
-    uint16_t address_family_;
-};
-
 typedef std::pair<isc::dhcp::OptionDefinitionPtr, std::string> OptionDefinitionTuple;
 
 /// @brief Parser for a single option definition.
diff --git a/src/lib/dhcpsrv/parsers/option_data_parser.cc b/src/lib/dhcpsrv/parsers/option_data_parser.cc
index bce9da0094..ec989e0bfe 100644
--- a/src/lib/dhcpsrv/parsers/option_data_parser.cc
+++ b/src/lib/dhcpsrv/parsers/option_data_parser.cc
@@ -162,6 +162,19 @@ OptionDataParser::extractSpace(ConstElementPtr parent) const {
     return (space);
 }
 
+OptionalValue<bool>
+OptionDataParser::extractPersistent(ConstElementPtr parent) const {
+    bool persist = false;
+    try {
+        persist = getBoolean(parent, "always-send");
+
+    } catch (...) {
+        return (OptionalValue<bool>(persist));
+    }
+
+    return (OptionalValue<bool>(persist, OptionalValueState(true)));
+}
+
 template<typename SearchKey>
 OptionDefinitionPtr
 OptionDataParser::findOptionDefinition(const std::string& option_space,
@@ -203,6 +216,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
     OptionalValue<uint32_t> code_param =  extractCode(option_data);
     OptionalValue<std::string> name_param = extractName(option_data);
     OptionalValue<bool> csv_format_param = extractCSVFormat(option_data);
+    OptionalValue<bool> persist_param = extractPersistent(option_data);
     std::string data_param = extractData(option_data);
     std::string space_param = extractSpace(option_data);
 
@@ -283,7 +297,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
     OptionDescriptor desc(false);
 
     if (!def) {
-        // @todo We have a limited set of option definitions initalized at
+        // @todo We have a limited set of option definitions initialized at
         // the moment.  In the future we want to initialize option definitions
         // for all options.  Consequently an error will be issued if an option
         // definition does not exist for a particular option code. For now it is
@@ -292,7 +306,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                                     binary));
 
         desc.option_ = option;
-        desc.persistent_ = false;
+        desc.persistent_ = persist_param.isSpecified() && persist_param;
     } else {
 
         // Option name is specified it should match the name in the definition.
@@ -313,7 +327,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                 def->optionFactory(universe, def->getCode(), data_tokens) :
                 def->optionFactory(universe, def->getCode(), binary);
             desc.option_ = option;
-            desc.persistent_ = false;
+            desc.persistent_ = persist_param.isSpecified() && persist_param;
             if (use_csv) {
                 desc.formatted_value_ = data_param;
             }
diff --git a/src/lib/dhcpsrv/parsers/option_data_parser.h b/src/lib/dhcpsrv/parsers/option_data_parser.h
index 471f7a8d49..b4fefeb65e 100644
--- a/src/lib/dhcpsrv/parsers/option_data_parser.h
+++ b/src/lib/dhcpsrv/parsers/option_data_parser.h
@@ -138,6 +138,11 @@ private:
     /// @return Option space name.
     std::string extractSpace(data::ConstElementPtr parent) const;
 
+    /// @brief Retrieves persistent/always-send parameter as an optional value.
+    ///
+    /// @return Value of the persistent parameter, possibly unspecified.
+    util::OptionalValue<bool> extractPersistent(data::ConstElementPtr parent) const;
+
     /// @brief Address family: @c AF_INET or @c AF_INET6.
     uint16_t address_family_;
 };