[trac2355] Addressed interrim review comments, and completed remainder

of the common parser refactoring. Replaced individual global variables with
instance of new ParserContext class. Created new base classes PoolParser
and SubnetConfigParser.

src/bin/dhcp4/config_parser.h

@@ -30,7 +30,8 @@ namespace dhcp {
 
 class Dhcpv4Srv;
 
-/// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration values.
+/// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration 
+/// values.
 ///
 /// This function parses configuration information stored in @c config_set
 /// and configures the @c server by applying the configuration to it.
@@ -43,9 +44,9 @@ class Dhcpv4Srv;
 /// (such as malformed configuration or invalid configuration parameter),
 /// this function returns appropriate error code.
 ///
-/// This function is called every time a new configuration is received. The extra
-/// parameter is a reference to DHCPv4 server component. It is currently not used
-/// and CfgMgr::instance() is accessed instead.
+/// This function is called every time a new configuration is received. The 
+/// extra parameter is a reference to DHCPv4 server component. It is currently
+/// not used and CfgMgr::instance() is accessed instead.
 ///
 /// This method does not throw. It catches all exceptions and returns them as
 /// reconfiguration statuses. It may return the following response codes:
@@ -60,58 +61,13 @@ isc::data::ConstElementPtr
 configureDhcp4Server(Dhcpv4Srv&,
                      isc::data::ConstElementPtr config_set);
 
-
-/// @brief Returns the global uint32_t values storage.
+/// @brief Returns the global context
 ///
 /// This function must be only used by unit tests that need
-/// to access uint32_t global storage to verify that the
-/// Uint32Parser works as expected.
-///
-/// @return a reference to a global uint32 values storage.
-const Uint32Storage& getUint32Defaults();
-
-
-/// @brief Parser for DHCP4 option data value.
+/// to access global context.
 ///
-/// This parser parses configuration entries that specify value of
-/// a single option specific to DHCP4.  It provides the DHCP4-specific
-/// implementation of the abstract class OptionDataParser.
-class Dhcp4OptionDataParser : public OptionDataParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// Class constructor.
-    Dhcp4OptionDataParser(const std::string&, OptionStorage *options,
-        OptionDefStorage *option_defs);
-
-    /// @brief static factory method for instantiating Dhcp4OptionDataParsers
-    ///
-    /// @param param_name name fo the parameter to be parsed.
-    /// @param options storage where the parameter value is to be stored.
-    /// @param global_option_defs global option definitions storage 
-    static OptionDataParser* factory(const std::string& param_name, OptionStorage *options,
-        OptionDefStorage *global_option_defs);
-
-protected:
-    /// @brief Finds an option definition within the server's option space
-    /// 
-    /// Given an option space and an option code, find the correpsonding 
-    /// option defintion within the server's option defintion storage.
-    ///
-    /// @param option_space name of the parameter option space 
-    /// @param option_code numeric value of the parameter to find 
-    /// @return OptionDefintionPtr of the option defintion or an 
-    /// empty OptionDefinitionPtr if not found.
-    /// @throw DhcpConfigError if the option space requested is not valid 
-    /// for this server. 
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-        std::string& option_space, uint32_t option_code);
-
-private:
-    // Private default Constructor declared for safety.
-    Dhcp4OptionDataParser() :OptionDataParser("",NULL,NULL,Option::V4) {}
-};
-
+/// @returns a const reference to the global context
+const ParserContext& getGlobalParserContext();
 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -52,9 +52,10 @@ public:
 
     // Checks if global parameter of name have expected_value
     void checkGlobalUint32(string name, uint32_t expected_value) {
-        const Uint32Storage& uint32_defaults = getUint32Defaults();
+        const Uint32StoragePtr uint32_defaults = 
+                                        getGlobalParserContext().uint32_values_;
         try {
-            uint32_t actual_value = uint32_defaults.getParam(name);
+            uint32_t actual_value = uint32_defaults->getParam(name);
             EXPECT_EQ(expected_value, actual_value);
         } catch (DhcpConfigError) {
             ADD_FAILURE() << "Expected uint32 with name " << name

src/bin/dhcp6/config_parser.h

@@ -31,9 +31,9 @@ class Dhcpv6Srv;
 
 /// @brief Configures DHCPv6 server
 ///
-/// This function is called every time a new configuration is received. The extra
-/// parameter is a reference to DHCPv6 server component. It is currently not used
-/// and CfgMgr::instance() is accessed instead.
+/// This function is called every time a new configuration is received. The 
+/// extra parameter is a reference to DHCPv6 server component. It is currently 
+/// not used and CfgMgr::instance() is accessed instead.
 ///
 /// This method does not throw. It catches all exceptions and returns them as
 /// reconfiguration statuses. It may return the following response codes:
@@ -49,48 +49,14 @@ class Dhcpv6Srv;
 isc::data::ConstElementPtr
 configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set);
 
-/// @brief Parser for DHCP6 option data value.
+/// @brief Returns the global context
 ///
-/// This parser parses configuration entries that specify value of
-/// a single option specific to DHCP6.  It provides the DHCP6-specific
-/// implementation of the abstract class OptionDataParser.
-class Dhcp6OptionDataParser : public OptionDataParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// Class constructor.
-    Dhcp6OptionDataParser(const std::string&, OptionStorage *options,
-        OptionDefStorage *option_defs);
-
-    /// @brief static factory method for instantiating Dhcp4OptionDataParsers
-    ///
-    /// @param param_name name fo the parameter to be parsed.
-    /// @param options storage where the parameter value is to be stored.
-    /// @param global_option_defs global option definitions storage 
-    static OptionDataParser* factory(const std::string& param_name, OptionStorage *options,
-        OptionDefStorage *global_option_defs);
-
-protected:
-    /// @brief Finds an option definition within the server's option space
-    /// 
-    /// Given an option space and an option code, find the correpsonding 
-    /// option defintion within the server's option defintion storage.
-    ///
-    /// @param option_space name of the parameter option space 
-    /// @param option_code numeric value of the parameter to find 
-    /// @return OptionDefintionPtr of the option defintion or an 
-    /// empty OptionDefinitionPtr if not found.
-    /// @throw DhcpConfigError if the option space requested is not valid 
-    /// for this server. 
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-        std::string& option_space, uint32_t option_code);
-
-private:
-    // Private default Constructor declared for safety.
-    Dhcp6OptionDataParser() :OptionDataParser("",NULL,NULL,Option::V6) {}
-};
-
-
+/// This function must be only used by unit tests that need
+/// to access global context.
+///
+/// @returns a const reference to the global context
+const ParserContext& getGlobalParserContext();
+ 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
 

src/lib/dhcpsrv/dhcp_config_parser.h

@@ -147,7 +147,7 @@ class ValueStorage {
         ///
         /// @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) {
+        void setParam(const std::string& name, const ValueType& value) {
             values_[name] = value;
         }
 
@@ -196,12 +196,15 @@ class ValueStorage {
 
 /// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900)
 typedef ValueStorage<uint32_t> Uint32Storage;
+typedef boost::shared_ptr<Uint32Storage> Uint32StoragePtr;
 
 /// @brief a collection of elements that store string values
 typedef ValueStorage<std::string> StringStorage;
+typedef boost::shared_ptr<StringStorage> StringStoragePtr;
 
 /// @brief Storage for parsed boolean values.
 typedef ValueStorage<bool> BooleanStorage;
+typedef boost::shared_ptr<BooleanStorage> BooleanStoragePtr;
 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/lib/dhcpsrv/subnet.cc

@@ -135,6 +135,15 @@ PoolPtr Subnet::getPool(isc::asiolink::IOAddress hint) {
     return (candidate);
 }
 
+void Subnet::setIface(const std::string& iface_name) {
+    iface_ = iface_name;
+}
+
+std::string Subnet::getIface() const {
+    return (iface_);
+}
+
+
 
 void
 Subnet4::validateOption(const OptionPtr& option) const {
@@ -183,7 +192,7 @@ Subnet6::validateOption(const OptionPtr& option) const {
     }
 }
 
-
+#if 0
 void Subnet6::setIface(const std::string& iface_name) {
     iface_ = iface_name;
 }
@@ -191,7 +200,7 @@ void Subnet6::setIface(const std::string& iface_name) {
 std::string Subnet6::getIface() const {
     return (iface_);
 }
-
+#endif
 
 } // end of isc::dhcp namespace
 } // end of isc namespace

src/lib/dhcpsrv/subnet.h

@@ -299,7 +299,18 @@ public:
         return pools_;
     }
 
-    /// @brief returns textual representation of the subnet (e.g. "2001:db8::/64")
+    /// @brief sets name of the network interface for directly attached networks
+    ///
+    /// @param iface_name name of the interface
+    void setIface(const std::string& iface_name);
+
+    /// @brief network interface name used to reach subnet (or "" for remote 
+    /// subnets)
+    /// @return network interface name for directly attached subnets or ""
+    std::string getIface() const;
+
+    /// @brief returns textual representation of the subnet (e.g. 
+    /// "2001:db8::/64")
     ///
     /// @return textual representation
     virtual std::string toText() const;
@@ -451,18 +462,6 @@ public:
         return (preferred_);
     }
 
-    /// @brief sets name of the network interface for directly attached networks
-    ///
-    /// A subnet may be reachable directly (not via relays). In DHCPv6 it is not
-    /// possible to decide that based on addresses assigned to network interfaces,
-    /// as DHCPv6 operates on link-local (and site local) addresses.
-    /// @param iface_name name of the interface
-    void setIface(const std::string& iface_name);
-
-    /// @brief network interface name used to reach subnet (or "" for remote subnets)
-    /// @return network interface name for directly attached subnets or ""
-    std::string getIface() const;
-
 protected:
 
     /// @brief Check if option is valid and can be added to a subnet.