[asterisk-scf-commits] asterisk-scf/integration/sip.git branch "sip-configuration-refactor" updated.

Commits to the Asterisk SCF project code repositories asterisk-scf-commits at lists.digium.com
Mon May 9 10:29:16 CDT 2011


branch "sip-configuration-refactor" has been updated
       via  4463bb246f0b492894df7341e7023f11c4d5cd73 (commit)
      from  11629e962692477a673e861f3e5502eb6eca7ca0 (commit)

Summary of changes:
 src/SipConfiguration.cpp |  118 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 101 insertions(+), 17 deletions(-)


- Log -----------------------------------------------------------------
commit 4463bb246f0b492894df7341e7023f11c4d5cd73
Author: Brent Eagles <beagles at digium.com>
Date:   Mon May 9 12:58:56 2011 -0230

    Add some comments and remove some unused code.

diff --git a/src/SipConfiguration.cpp b/src/SipConfiguration.cpp
index e60dc74..fe70c56 100644
--- a/src/SipConfiguration.cpp
+++ b/src/SipConfiguration.cpp
@@ -44,15 +44,28 @@ namespace AsteriskSCF
 namespace SipSessionManager
 {
 
+/**
+ *
+ * The createGroupTemplate() pattern implements boilerplate for creating "empty" copies of a group. All of the basic
+ * group elements should be copied except for the group dictionary. This will be copied separately.
+ *
+ **/
+
+//
+// A generic instance of the createGroupTemplate() that creates a new, empty instance of a named group.
+//
 template <class T>
 T createGroupTemplate(const T& group)
 {
-    T result = T::dynamicCast(group->ice_clone());
+    T result = new T::element_type;
     assert(result);
     result->name = group->name;
     return result;
 }
 
+//
+// A specialization for SipGeneralGroup, which contains no additional members.
+//
 template<>
 SipGeneralGroupPtr createGroupTemplate(const SipGeneralGroupPtr&)
 {
@@ -60,7 +73,7 @@ SipGeneralGroupPtr createGroupTemplate(const SipGeneralGroupPtr&)
 }
 
 //
-// Specialization require since the name is different.
+// Specialization required for SipDomainGroup for different members.
 //
 template <>
 SipDomainGroupPtr createGroupTemplate(const SipDomainGroupPtr& source)
@@ -70,6 +83,10 @@ SipDomainGroupPtr createGroupTemplate(const SipDomainGroupPtr& source)
     return r;
 }
 
+//
+// Checks to see if an update is out of order and currently ... does nothing. We need to add some logging or
+// update filtering.
+//
 static void performSerialCheck(const ConfigurationItemDict& changedItems, const ConfigurationItemDict& localItems)
 {
     for (ConfigurationItemDict::const_iterator item = changedItems.begin();
@@ -99,8 +116,17 @@ static void performSerialCheck(const ConfigurationItemDict& changedItems, const
     }
 }
 
+//
+// A helper class and visitor for propogating configuration changes to the SIPEndPoint implementation.
+//
 class EndpointConfigHelper
 {
+    //
+    // Helper visitor that will be used to interpret the contents of the relevant configuration group.
+    // Reference counting and destruction plays an important part of the processing. The destructor for
+    // the visitor calls back into the helper to do any finalization or cleanup. This pattern will be
+    // revisited in the other helpers in this source file.
+    //
     class Visitor : public SipConfigurationItemVisitor
     {
     public:
@@ -111,9 +137,33 @@ class EndpointConfigHelper
 
         ~Visitor()
         {
-            mConfig->updated();
+            try
+            {
+                //
+                // Don't forget to tell the helper that all of the updates have been performed
+                // and it can perform final processing.
+                //
+                mConfig->updated();
+            }
+            catch(...)
+            {
+                //
+                // NOTE TODO XXX?
+                // Now here's the tricky part and where we need to look hard. If we gone this far
+                // in configuration and we fail at this final stage, we really should undo all of
+                // our configuration changes. This is easier said that done, so perhaps its
+                // best if, for the time being at least, simply strive to keep *internally* consistent and
+                // deal with reconciling with other components as a TBD.
+                //
+            }
         }
 
+        //
+        // Map visits to overloaded methods. Strictly speaking this isn't necessary as the helper
+        // could have implemented these methods itself. This is more for consistency and illustration
+        // than anything else and shows how an visitor implementation could "glue" the visitor code
+        // to a non-visitor implementation with total disregard to function signatures etc..
+        //
         void visitSipAllowableCallDirectionItem(const SipAllowableCallDirectionItemPtr& direction)
         {
             mConfig->update(direction);
@@ -130,6 +180,10 @@ class EndpointConfigHelper
         };
 
     private:
+        //
+        // We keep a reference to the parent as the helper itself isn't held by the configuration
+        // object or client.
+        //
         boost::shared_ptr<EndpointConfigHelper> mConfig;
     };
 
@@ -192,6 +246,9 @@ public:
 
     void updated()
     {
+        //
+        // TODO: What about exception handling here... we probably don't want it
+        //
         RegExSeq destinations;
         mFactory->generateRoutingDestinations(destinations);
         mRegistry->setEndpointLocatorDestinationIds(mRoutingId, destinations);
@@ -217,7 +274,12 @@ public:
     virtual void updated() { }
 };
 typedef boost::shared_ptr<ConfigBase> ConfigBasePtr;
-    
+
+//
+// The Transport specializations take care of any transport specific configuration and
+// initialization. The UDP and TCP transports are so similar that they could probably
+// share some functionality.
+//
 class UDPTransportConfig : public ConfigBase
 {
     class Visitor : public SipConfigurationItemVisitor
@@ -230,7 +292,16 @@ class UDPTransportConfig : public ConfigBase
 
         ~Visitor()
         {
-            mConfig->updated();
+            try
+            {
+                mConfig->updated();
+            }
+            catch(...)
+            {
+                //
+                // See comment in the EndpointConfigHelper's Visitor::~Visitor.
+                //
+            }
         }
 
         void visitSipHostItem(const SipHostItemPtr& hostItem)
@@ -361,7 +432,16 @@ class TCPTransportConfig : public ConfigBase
 
         ~Visitor()
         {
-            mConfig->updated();
+            try
+            {
+                mConfig->updated();
+            }
+            catch(...)
+            {
+                //
+                // See comment in the EndpointConfigHelper's Visitor::~Visitor.
+                //
+            }
         }
 
         void visitSipHostItem(const SipHostItemPtr& hostItem)
@@ -493,7 +573,16 @@ class TLSTransportConfig : public ConfigBase
 
         ~Visitor()
         {
-            mConfig->updated();
+            try
+            {
+                mConfig->updated();
+            }
+            catch(...)
+            {
+                //
+                // See comment in the EndpointConfigHelper's Visitor::~Visitor.
+                //
+            }
         }
 
         void visitSipHostItem(const SipHostItemPtr& hostItem)
@@ -679,17 +768,12 @@ bool getItem(const C& collection, I& iter, const K& key)
     return iter != collection.end();
 }
 
-template <typename C, typename K>
-bool hasItem(const C& collection, const K& key)
-{
-    typedef typename C::const_iterator Iter;
-    Iter iter = collection.find(key);
-    return iter != collection.end();
-}
-
 //
-// ConfigurationData goes into a class by itself to aide in sharing and
-// locking.
+// ConfigurationData goes into a class by itself to aide in sharing and locking. Operations on the configuration data
+// have been heavily generalized into templates and type specific helpers that map the protocols described by the
+// template members to type specific instantiations. The general premise is to remove/reduce repeated code
+// and keep it restricted the most specialized uses possible. There are definitely a few more things that
+// could be done.
 //
 class ConfigurationData
 {

-----------------------------------------------------------------------


-- 
asterisk-scf/integration/sip.git



More information about the asterisk-scf-commits mailing list