[asterisk-scf-commits] asterisk-scf/integration/bridging.git branch "bridge_creation_extension_points" updated.

Commits to the Asterisk SCF project code repositories asterisk-scf-commits at lists.digium.com
Fri Aug 26 09:08:10 CDT 2011


branch "bridge_creation_extension_points" has been updated
       via  ca8bcee8337cc4e664f0ce8d25099e6f8b2cb67b (commit)
      from  1b329fda02cc03322ef8c7fa2e23f7d75b9d0bca (commit)

Summary of changes:
 src/BridgeManagerImpl.cpp |   65 ++++++++++++++++++++++++++++++++++++--------
 test/BridgeListenerI.cpp  |    4 ---
 test/CMakeLists.txt       |    1 -
 test/TestBridging.cpp     |    9 ++----
 4 files changed, 56 insertions(+), 23 deletions(-)


- Log -----------------------------------------------------------------
commit ca8bcee8337cc4e664f0ce8d25099e6f8b2cb67b
Author: Brent Eagles <beagles at digium.com>
Date:   Fri Aug 26 11:36:43 2011 -0230

    Respond to review feedback #1:
     - rename local variable to avoid confusion.
     - add some comments to clarify purpose of conditional statements.
     - remove bogus comments.

diff --git a/src/BridgeManagerImpl.cpp b/src/BridgeManagerImpl.cpp
index 64a3ad6..b2cdb7c 100644
--- a/src/BridgeManagerImpl.cpp
+++ b/src/BridgeManagerImpl.cpp
@@ -228,25 +228,55 @@ void BridgeManagerImpl::createBridge_async(const AMD_BridgeManager_createBridgeP
             listeners.push_back(listener);
         }
 
+        //
+        // "sessions" is a const, so we need to make a local non-const copy that can be modified
+        // by the hooks if need be.
+        //
         SessionSeq initialSessions(sessions);
-        BridgeCreationHookDataPtr originalData;
+
+        //
+        // We create a local outside of the block that initializes and runs the hooks if they are
+        // present. If this is '0' later on in the code, it signifies the bridge creation extension
+        // point mechanism was not initialized.
+        //
+        BridgeCreationHookDataPtr hookData;
         BridgeInfo info;
         info.proxy = prx;
         if (mCreationExtension)
         {
-            originalData = new BridgeCreationHookData;
-            originalData->bridge = prx;
-            originalData->listeners = listeners;
-            originalData->initialSessions = sessions;
-            originalData = mCreationExtension->runHooks(originalData);
-            listeners = originalData->listeners;
-            initialSessions = originalData->initialSessions;
-            info.decoratingPrx = originalData->bridge;
-            dump(mLogger, originalData);
+            //
+            // Set up hook data with initial values and run the hooks!
+            //
+            hookData = new BridgeCreationHookData;
+            hookData->bridge = prx;
+            hookData->listeners = listeners;
+            hookData->initialSessions = sessions;
+            hookData = mCreationExtension->runHooks(hookData);
+
+            //
+            // Update locals with the result of running the hooks.
+            //
+            listeners = hookData->listeners;
+            initialSessions = hookData->initialSessions;
+            info.decoratingPrx = hookData->bridge;
+            dump(mLogger, hookData);
         }
+        
+        //
+        // The bridge listener manager is a wrapper/helper class that manages the listeners and
+        // propogates the bridge events. We separate it's instantiation from the construction
+        // of the bridge itself as this is a natural area of refinement and extension.
+        //
         BridgeListenerMgrPtr mgr(new BridgeListenerMgr(mAdapter->getCommunicator(), stringId, info.decoratingPrx));
 
+        //
+        // Now we can get down to the creation of the bridge servant itself. Note that the 
+        // initialization is still not really complete as we need to set up and cookies,
+        // initial sessions, etc that may have been defined or added as part of the createBridge
+        // call or the bridge creation hooks.
+        //
         BridgeServantPtr bridge = BridgeServant::create(stringId, mAdapter, listeners, mgr, mReplicationContext->getReplicator(), mLogger);
+
         Ice::ObjectPrx obj = mAdapter->add(bridge, id);
 
         mLogger(Info) << objectIdFromCurrent(current) << ": creating new bridge " << obj->ice_toString() << "." ;
@@ -258,16 +288,27 @@ void BridgeManagerImpl::createBridge_async(const AMD_BridgeManager_createBridgeP
        
         //
         // It's very important to note that the bridge servant will not have it's own proxy!
+        // NOTE: This method is probably misnamed and misleading. The bridge servant was added to 
+        // object adapter above. It might be a better idea to move adding the servant to the adapter
+        // into the activate method.
         // 
         bridge->activate(info.decoratingPrx, stringId);
         mBridges.push_back(info);
 
-        if (originalData)
+        if (hookData)
         {
-            bridge->setCookiesImpl(originalData->cookies);
+            bridge->setCookiesImpl(hookData->cookies);
         }
 
+        //
+        // There are some finalization tasks that may be performed asynchronously (replication etc.)
+        //
         QueuedTasks tasks;
+
+        //
+        // If there are some sessions that need to be added to the bridge immediately
+        // upon creation, create some tasks to add to the queue.
+        //
         if (!initialSessions.empty())
         {
             bridge->getAddSessionsTasks(tasks, initialSessions);
diff --git a/test/BridgeListenerI.cpp b/test/BridgeListenerI.cpp
index 83b659f..9a46979 100644
--- a/test/BridgeListenerI.cpp
+++ b/test/BridgeListenerI.cpp
@@ -28,7 +28,6 @@ void BridgeListenerI::sessionsAdded(const AsteriskSCF::SessionCommunications::V1
     const AsteriskSCF::SessionCommunications::V1::SessionSeq& sessions, 
     const AsteriskSCF::SessionCommunications::V1::BridgeCookies& cookies, const Ice::Current&)
 {
-    cerr << "xxx added" << endl;
     updateCookieMap("sessionsAdded", cookies);
     IceUtil::Monitor<IceUtil::Mutex>::Lock lock(mAddMonitor);
     mAdded =sessions;
@@ -39,7 +38,6 @@ void BridgeListenerI::sessionsRemoved(const AsteriskSCF::SessionCommunications::
     const AsteriskSCF::SessionCommunications::V1::SessionSeq& sessions, 
     const AsteriskSCF::SessionCommunications::V1::BridgeCookies& cookies, const Ice::Current&)
 {
-    cerr << "xxx removed" << endl;
     updateCookieMap("sessionsRemoved", cookies);
     IceUtil::Monitor<IceUtil::Mutex>::Lock lock(mRemoveMonitor);
     mRemoved = sessions;
@@ -49,7 +47,6 @@ void BridgeListenerI::sessionsRemoved(const AsteriskSCF::SessionCommunications::
 void BridgeListenerI::stopping(const AsteriskSCF::SessionCommunications::V1::BridgePrx&, 
     const AsteriskSCF::SessionCommunications::V1::BridgeCookies& cookies, const Ice::Current&)
 {
-    cerr << "xxx stopping" << endl;
     updateCookieMap("stopping", cookies);
     IceUtil::Monitor<IceUtil::Mutex>::Lock lock(mStateMonitor);
     mShuttingDown = true;
@@ -59,7 +56,6 @@ void BridgeListenerI::stopping(const AsteriskSCF::SessionCommunications::V1::Bri
 void BridgeListenerI::stopped(const AsteriskSCF::SessionCommunications::V1::BridgePrx&, 
     const AsteriskSCF::SessionCommunications::V1::BridgeCookies& cookies, const Ice::Current&)
 {
-    cerr << "xxx stopped" << endl;
     updateCookieMap("stopped", cookies);
     IceUtil::Monitor<IceUtil::Mutex>::Lock lock(mStateMonitor);
     mStopped = true;
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 6f2f1af..1afebf4 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -26,7 +26,6 @@ target_link_libraries(bridge_component_test logging-client)
 astscf_test_icebox(bridge_component_test config/test_bridging.conf)
 
 astscf_component_init(bridge_unit_tests)
-astscf_component_add_slices(bridge_unit_tests PROJECT AsteriskSCF/Replication/BridgeService/BridgeReplicatorIf.ice)
 astscf_component_add_files(bridge_unit_tests ../src/SessionCollection.cpp)
 astscf_component_add_files(bridge_unit_tests ../src/SessionOperations.cpp)
 astscf_component_add_files(bridge_unit_tests ../src/SessionWrapper.cpp)
diff --git a/test/TestBridging.cpp b/test/TestBridging.cpp
index fc6c6c7..64bc827 100644
--- a/test/TestBridging.cpp
+++ b/test/TestBridging.cpp
@@ -83,9 +83,6 @@ public:
         newData->cookies.insert(newData->cookies.end(), mCookies.begin(), mCookies.end());
         newData->listeners.insert(newData->listeners.end(), mListeners.begin(), mListeners.end());
         newData->initialSessions.insert(newData->initialSessions.end(), mSessions.begin(), mSessions.end());
-        cerr << "XXX " << newData->cookies.size() << endl;
-        cerr << "XXX " << newData->listeners.size() << endl;
-        cerr << "XXX " << newData->initialSessions.size() << endl;
         return mResult;
     }
 
@@ -425,7 +422,7 @@ public:
         assert(eps.size() > 0 && eps[0] != 0);
         AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx proxy(AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx::checkedCast(eps[0]));
         assert(proxy);
-        AsteriskSCF::SessionCommunications::V1::SessionPrx session(proxy->createSession(id, 0));
+        AsteriskSCF::SessionCommunications::V1::SessionPrx session(proxy->createSession(id, 0, 0));
         assert(session);
         return session;
     }
@@ -982,7 +979,7 @@ public:
                 //
                 mgrPrx->listBridges();
 
-                BOOST_CHECK(servant->createCalls() == 1); // XXX
+                BOOST_CHECK(servant->createCalls() == 1); 
                 mgrPrx->removeListener(listenerPrx);
                 
                 bridge = mgrPrx->createBridge(sessions, 0);
@@ -1016,7 +1013,7 @@ public:
 
                 BridgeSeq bridges2 = mgrPrx2->listBridges();
 
-                BOOST_CHECK(bridges.size() == bridges2.size()); // XXX
+                BOOST_CHECK(bridges.size() == bridges2.size()); 
 
                 // Set the components back to original state.
                 secondaryReplica->standby();

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


-- 
asterisk-scf/integration/bridging.git



More information about the asterisk-scf-commits mailing list