[2957] Made forward and reverse ddns elements optional, altered clone

method to return smart pointer.

7 files changed, 42 insertions, 33 deletions

diff --git a/src/bin/d2/d2_cfg_mgr.h b/src/bin/d2/d2_cfg_mgr.h
index 785c204b18..dad1847e04 100644
--- a/src/bin/d2/d2_cfg_mgr.h
+++ b/src/bin/d2/d2_cfg_mgr.h
@@ -27,6 +27,10 @@
 namespace isc {
 namespace d2 {
 
+class D2CfgContext;
+/// @brief Pointer to a configuration context.
+typedef boost::shared_ptr<D2CfgContext> D2CfgContextPtr;
+
 /// @brief  DHCP-DDNS Configuration Context
 ///
 /// Implements the storage container for configuration context.
@@ -44,9 +48,9 @@ public:
 
     /// @brief Creates a clone of this context object.
     ///
-    /// @return returns a raw pointer to the new clone.
-    virtual D2CfgContext* clone() {
-        return (new D2CfgContext(*this));
+    /// @return returns a pointer to the new clone.
+    virtual DCfgContextBasePtr clone() {
+        return (DCfgContextBasePtr(new D2CfgContext(*this)));
     }
 
     /// @brief Fetches the forward DNS domain list manager.
@@ -92,8 +96,6 @@ private:
 typedef boost::shared_ptr<DdnsDomainListMgr> DdnsDomainListMgrPtr;
 
 
-/// @brief Pointer to a configuration context.
-typedef boost::shared_ptr<D2CfgContext> D2CfgContextPtr;
 
 /// @brief DHCP-DDNS Configuration Manager
 ///
diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h
index f53d1e27a2..6a00b5a619 100644
--- a/src/bin/d2/d2_config.h
+++ b/src/bin/d2/d2_config.h
@@ -484,9 +484,9 @@ public:
 
     /// @brief Creates a clone of a DStubContext.
     ///
-    /// @return returns a raw pointer to the new clone.
-    virtual DScalarContext* clone() {
-        return (new DScalarContext(*this));
+    /// @return returns a pointer to the new clone.
+    virtual DCfgContextBasePtr clone() {
+        return (DCfgContextBasePtr(new DScalarContext(*this)));
     }
 
 protected:
@@ -499,6 +499,9 @@ private:
     DScalarContext& operator=(const DScalarContext& rhs);
 };
 
+/// @brief Defines a pointer for DScalarContext instances.
+typedef boost::shared_ptr<DScalarContext> DScalarContextPtr;
+
 /// @brief Parser for  TSIGKeyInfo
 ///
 /// This class parses the configuration element "tsig_key" defined in
diff --git a/src/bin/d2/d_cfg_mgr.cc b/src/bin/d2/d_cfg_mgr.cc
index 6a3175dc72..a8394a98fe 100644
--- a/src/bin/d2/d_cfg_mgr.cc
+++ b/src/bin/d2/d_cfg_mgr.cc
@@ -122,7 +122,7 @@ 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();
 
     // Answer will hold the result returned to the caller.
     ConstElementPtr answer;
@@ -143,9 +143,11 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
         if (parse_order_.size() > 0) {
             // 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
-            // associated data in for parsing.  
-            // If there is no matching entry in the value map an error is 
-            // thrown.  Optional elements may not be used with ordered parsing.
+            // associated data in for parsing.
+            // If there is no matching entry in the value map an error is
+            // thrown.  Note, that elements tagged as "optional" from the user
+            // perspective must still have default or empty entries in the
+            // configuration set to be parsed.
             int parsed_count = 0;
             std::map<std::string, ConstElementPtr>::const_iterator it;
             BOOST_FOREACH(element_id, parse_order_) {
@@ -157,7 +159,7 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
                 else {
                     LOG_ERROR(dctl_logger, DCTL_ORDER_NO_ELEMENT)
                               .arg(element_id);
-                    isc_throw(DCfgMgrBaseError, "Element:" << element_id <<  
+                    isc_throw(DCfgMgrBaseError, "Element:" << element_id <<
                               " is listed in the parse order but is not "
                               " present in the configuration");
                 }
@@ -172,10 +174,10 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
             // Better to hold the engineer accountable.  So, if we parsed none
             // or we parsed fewer than are in the map; then either the parse i
             // order is incomplete OR the map has unsupported values.
-            if (!parsed_count || 
+            if (!parsed_count ||
                 (parsed_count && ((parsed_count + 1) < values_map.size()))) {
                 LOG_ERROR(dctl_logger, DCTL_ORDER_ERROR);
-                isc_throw(DCfgMgrBaseError, 
+                isc_throw(DCfgMgrBaseError,
                         "Configuration contains elements not in parse order");
             }
         } else {
@@ -226,7 +228,7 @@ void DCfgMgrBase::buildAndCommit(std::string& element_id,
         // nothing something we are concerned with here.)
         parser->commit();
     } catch (const isc::Exception& ex) {
-        isc_throw(DCfgMgrBaseError, 
+        isc_throw(DCfgMgrBaseError,
                   "Could not build and commit: " << ex.what());
     } catch (...) {
         isc_throw(DCfgMgrBaseError, "Non-ISC exception occurred");
diff --git a/src/bin/d2/d_cfg_mgr.h b/src/bin/d2/d_cfg_mgr.h
index a527fdbdd1..a0bd9bb267 100644
--- a/src/bin/d2/d_cfg_mgr.h
+++ b/src/bin/d2/d_cfg_mgr.h
@@ -32,6 +32,10 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+class DCfgContextBase;
+/// @brief Pointer to a configuration context.
+typedef boost::shared_ptr<DCfgContextBase> DCfgContextBasePtr;
+
 /// @brief Abstract class that implements a container for configuration context.
 /// It provides a single enclosure for the storage of configuration parameters
 /// and any other context specific information that needs to be accessible
@@ -141,8 +145,8 @@ public:
     /// public:
     ///  :
     ///     // Clone calls its own copy constructor
-    ///     virtual DStubContext* clone() {
-    ///         return (new DStubContext(*this));
+    ///     virtual DCfgContextBasePtr clone() {
+    ///         return (DCfgContextBasePtr(new DStubContext(*this)));
     ///     }
     ///
     ///     // Note that the copy constructor calls the base class copy ctor
@@ -156,8 +160,8 @@ public:
     ///  :
     /// @endcode
     ///
-    /// @return returns a raw pointer to the new clone.
-    virtual DCfgContextBase* clone() = 0;
+    /// @return returns a pointer to the new clone.
+    virtual DCfgContextBasePtr clone() = 0;
 
 protected:
     /// @brief Copy constructor for use by derivations in clone().
@@ -177,9 +181,6 @@ private:
     isc::dhcp::StringStoragePtr string_values_;
 };
 
-/// @brief Pointer to a configuration context.
-typedef boost::shared_ptr<DCfgContextBase> DCfgContextBasePtr;
-
 /// @brief Defines an unsorted, list of string Element IDs.
 typedef std::vector<std::string> ElementIdList;
 
diff --git a/src/bin/d2/dhcp-ddns.spec b/src/bin/d2/dhcp-ddns.spec
index 8e75e05161..e29bc88e76 100644
--- a/src/bin/d2/dhcp-ddns.spec
+++ b/src/bin/d2/dhcp-ddns.spec
@@ -59,7 +59,7 @@
     {
         "item_name": "forward_ddns",
         "item_type": "map",
-        "item_optional": false,
+        "item_optional": true,
          "item_default": {},
          "map_item_spec": [ 
          {
@@ -127,7 +127,7 @@
     {
         "item_name": "reverse_ddns",
         "item_type": "map",
-        "item_optional": false,
+        "item_optional": true,
          "item_default": {},
          "map_item_spec": [ 
          { 
diff --git a/src/bin/d2/tests/d_test_stubs.cc b/src/bin/d2/tests/d_test_stubs.cc
index 5d9077d3f2..4e1cee688d 100644
--- a/src/bin/d2/tests/d_test_stubs.cc
+++ b/src/bin/d2/tests/d_test_stubs.cc
@@ -260,9 +260,9 @@ DStubContext::getExtraStorage() {
     return (extra_values_);
 }
 
-DStubContext*
+DCfgContextBasePtr
 DStubContext::clone() {
-    return (new DStubContext(*this));
+    return (DCfgContextBasePtr(new DStubContext(*this)));
 }
 
 DStubContext::DStubContext(const DStubContext& rhs): DCfgContextBase(rhs),
diff --git a/src/bin/d2/tests/d_test_stubs.h b/src/bin/d2/tests/d_test_stubs.h
index ec23efea16..0f0084f9ed 100644
--- a/src/bin/d2/tests/d_test_stubs.h
+++ b/src/bin/d2/tests/d_test_stubs.h
@@ -521,8 +521,8 @@ public:
 
     /// @brief Creates a clone of a DStubContext.
     ///
-    /// @return returns a raw pointer to the new clone.
-    virtual DStubContext* clone();
+    /// @return returns a pointer to the new clone.
+    virtual DCfgContextBasePtr clone();
 
 protected:
     /// @brief Copy constructor
@@ -607,10 +607,11 @@ public:
         try  {
             config_set_ = isc::data::Element::fromJSON(json_text);
         } catch (...) {
-            // This is so we can diagnose parsing mistakes during test
-            // development.
-            std::cerr << "fromJSON failed to parse text" << json_text
+            #if 0
+            // Handy for diagnostics
+            std::cout << "fromJSON failed to parse text" << json_text
                       << std::endl;
+            #endif
             return (false);
         }
 
@@ -627,8 +628,8 @@ public:
         int rcode = 0;
         isc::data::ConstElementPtr comment;
         comment = isc::config::parseAnswer(rcode, answer_);
+        #if 0
         // Handy for diagnostics
-        #if 1
         if (rcode != 0) {
             std::cout << "checkAnswer rcode:" << rcode << " comment: "
                   << *comment << std::endl;