[asterisk-scf-commits] asterisk-scf/integration/servicediscovery.git branch "ami-collector" created.

Commits to the Asterisk SCF project code repositories asterisk-scf-commits at lists.digium.com
Thu Jan 6 19:17:01 UTC 2011


branch "ami-collector" has been created
        at  af8f47d218d96feef91bdd8ab11595b65d543bf5 (commit)

- Log -----------------------------------------------------------------
commit af8f47d218d96feef91bdd8ab11595b65d543bf5
Author: David M. Lee <dlee at digium.com>
Date:   Thu Jan 6 13:02:39 2011 -0600

    Simplified the isSupported callbacks.
    
    Instead of building a couple of layers of objects, added a callback
    function directly on ServiceLocatorManagementImpl.  It's good to delete
    code :-)

diff --git a/src/ServiceLocatorManagement.cpp b/src/ServiceLocatorManagement.cpp
index 3253736..f620d85 100644
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@ -43,88 +43,6 @@ namespace ServiceLocatorManagementImplNS
 
 const Logger& lg = getLoggerFactory().getLogger("AsteriskSCF.System.Discovery");
 
-/**
- * Callback class for isSupported AMI calls.  Acts as a bridge between
- * Ice callbacks and IsSupportedCallback.
- * @see IsSupportedCallback
- */
-class Comparator_IsSupported_Callback : public IceUtil::Shared
-{
-public:
-    Comparator_IsSupported_Callback(const IsSupportedCallbackPtr& callback) :
-        callback(callback)
-    {
-    }
-
-    void finished(const Ice::AsyncResultPtr& r)
-    {
-        ServiceLocatorParamsComparePrx comparator =
-            ServiceLocatorParamsComparePrx::uncheckedCast(r->getProxy());
-
-        try
-        {
-            // forward result to callback
-            callback->result(comparator->end_isSupported(r));
-        }
-        catch (const std::exception& e)
-        {
-            lg(Error) << "Error communicating with comparator: " << e.what();
-            callback->result(false);
-        }
-        catch(...)
-        {
-            lg(Error) << "Error communicating with comparator";
-            callback->result(false);
-        }
-    }
-
-private:
-    IsSupportedCallbackPtr callback;
-};
-typedef IceUtil::Handle<Comparator_IsSupported_Callback> Comparator_IsSupported_CallbackPtr;
-
-/**
- * Small internal class which is used to store information about a comparator.
- */
-class ServiceLocatorComparator
-{
-public:
-    /**
-     * Constructor for the ServiceLocatorComparator class.
-     *
-     * @param compare A proxy to the comparator service we are wrapping.
-     */
-    ServiceLocatorComparator(const ServiceLocatorParamsComparePrx& compare) :
-        mCompare(compare)
-    {
-    }
-
-    /**
-     * API call which forwards an isSupported over ICE to a remote comparator.
-     * Results are returned via callback object.
-     *
-     * @param params Service locator parameters that describe a locator request.
-     * @param callback Callback object which receives the results.
-     */
-    void isSupported(const ServiceLocatorParamsPtr& params,
-        const IsSupportedCallbackPtr& callback)
-    {
-        // wrap our callback with an Ice callback
-        Comparator_IsSupported_CallbackPtr iceCallback =
-            new Comparator_IsSupported_Callback(callback);
-        // which is wrapped again...
-        Ice::CallbackPtr d = Ice::newCallback(iceCallback,
-            &Comparator_IsSupported_Callback::finished);
-        // async forward to the comparator
-        mCompare->begin_isSupported(params, d);
-    }
-private:
-    /**
-     * A proxy to the comparator service that we are wrapping.
-     */
-    ServiceLocatorParamsComparePrx mCompare;
-};
-
 /** Paramater type for Locator, to allow use of ResponseCollector. */
 typedef std::pair<bool, ServiceManagementImplPtr> LocateParam;
 
@@ -274,7 +192,7 @@ private:
 };
 typedef IceUtil::Handle<LocateCallback> LocateCallbackPtr;
 
-} // end of ServiceLocatorManagementImplNS namespace 
+} // end of ServiceLocatorManagementImplNS namespace
 
 using namespace ServiceLocatorManagementImplNS;
 IsSupportedCallback::~IsSupportedCallback()
@@ -311,7 +229,7 @@ public:
      * A map of all the comparators that have been added by external components.
      * The key is the unique identifier that they add themselves with.
      */
-    std::map<std::string, ServiceLocatorComparator> mCompares;
+    std::map<std::string, ServiceLocatorParamsComparePrx> mCompares;
 
     /**
      * A vector of all the services that have been added.
@@ -428,10 +346,9 @@ void ServiceLocatorManagementImpl::addCompare(const string& guid,
 {
     lg(Info) << "addCompare(" << guid << ')';
     boost::unique_lock<boost::shared_mutex> lock(mImpl->mLock);
-    ServiceLocatorComparator newComparator(service);
 
-    pair<map<string, ServiceLocatorComparator>::iterator, bool> insertPair = mImpl->mCompares.insert(pair<string, ServiceLocatorComparator>(guid,
-            newComparator));
+    pair<map<string, ServiceLocatorParamsComparePrx>::iterator, bool> insertPair =
+        mImpl->mCompares.insert(make_pair(guid, service));
 
     if (insertPair.second == false)
     {
@@ -467,10 +384,10 @@ void ServiceLocatorManagementImpl::isSupported(const string& compareGuid,
     const ServiceLocatorParamsPtr& params,
     const IsSupportedCallbackPtr& callback)
 {
-    /* You'll note there is no lock here. This is because we already have a lock in the locate or locateAll
-     * functions.
+    /* You'll note there is no lock here. This is because we already have a lock
+     * in the locate or locateAll functions.
      */
-    map<string, ServiceLocatorComparator>::iterator comparator =
+    map<string, ServiceLocatorParamsComparePrx>::iterator comparator =
         mImpl->mCompares.find(compareGuid);
 
     if (comparator == mImpl->mCompares.end())
@@ -479,13 +396,40 @@ void ServiceLocatorManagementImpl::isSupported(const string& compareGuid,
         return;
     }
 
-    (comparator->second).isSupported(params, callback);
+    Ice::CallbackPtr iceCallback = Ice::newCallback(this,
+        &ServiceLocatorManagementImpl::finish_isSupported);
+    (comparator->second)->begin_isSupported(params, iceCallback, callback);
+}
+
+void ServiceLocatorManagementImpl::finish_isSupported(
+    const Ice::AsyncResultPtr& r)
+{
+    ServiceLocatorParamsComparePrx compare =
+        ServiceLocatorParamsComparePrx::uncheckedCast(r->getProxy());
+    IsSupportedCallbackPtr callback =
+        IsSupportedCallbackPtr::dynamicCast(r->getCookie());
+
+    try
+    {
+        callback->result(compare->end_isSupported(r));
+    }
+    catch (const std::exception& e)
+    {
+        lg(Error) << "Error communicating with comparator: " << e.what();
+        callback->result(false);
+    }
+    catch(...)
+    {
+        lg(Error) << "Error communicating with comparator";
+        callback->result(false);
+    }
 }
 
 /**
  * Implementation of the removeService method as defined in service_locator.ice
  */
-void ServiceLocatorManagementImpl::removeService(const ServiceManagementImplPtr& service)
+void ServiceLocatorManagementImpl::removeService(
+    const ServiceManagementImplPtr& service)
 {
     lg(Info) << "removeService(" << service->getGuid() << ')';
     boost::unique_lock<boost::shared_mutex> lock(mImpl->mLock);
diff --git a/src/ServiceLocatorManagement.h b/src/ServiceLocatorManagement.h
index 0261939..6d9f65d 100644
--- a/src/ServiceLocatorManagement.h
+++ b/src/ServiceLocatorManagement.h
@@ -43,7 +43,7 @@ class ServiceLocatorManagementImplPriv;
  * Callback interface for ServiceLocatorManagementImpl::isSupported.
  * @see ServiceLocatorManagementImpl::isSupported
  */
-class IsSupportedCallback : public IceUtil::Shared
+class IsSupportedCallback : public Ice::LocalObject
 {
 public:
     virtual ~IsSupportedCallback();
@@ -94,6 +94,9 @@ private:
      * Private implementation data for ServiceLocatorManagementImpl.
      */
     boost::shared_ptr<ServiceLocatorManagementImplPriv> mImpl;
+
+    /** Ice callback function */
+    void finish_isSupported(const Ice::AsyncResultPtr& r);
 };
 
 } /* end of ServiceDiscovery */

commit fac42e6c1a9dd61fe5d1fa28e137141d56443a1c
Author: David M. Lee <dlee at digium.com>
Date:   Thu Jan 6 12:42:50 2011 -0600

    Refactored CountedIsSupported to use ResponseCollector.

diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index 678bbd0..7e3f108 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -25,11 +25,13 @@
 #include "ServiceLocatorManagement.h"
 #include "ServiceManagement.h"
 #include "logger.h"
+#include "ResponseCollector.h"
 
 using namespace std;
 using namespace AsteriskSCF::Core::Discovery::V1;
 using namespace AsteriskSCF::ServiceDiscovery;
 using namespace AsteriskSCF::System::Logging;
+using namespace AsteriskSCF;
 
 namespace
 {
@@ -181,31 +183,39 @@ ServiceManagementPrx ServiceManagementImpl::getServiceManagementPrx()
  * invocations to a single callback.  If any callbacks are true, true is
  * called back.  If none are true, false is called back.
  */
-class CountedIsSupported : public IsSupportedCallback
+class CountedIsSupported :
+    public IsSupportedCallback,
+    public ResponseCollector<bool>
 {
 public:
-    CountedIsSupported(const IsSupportedCallbackPtr& callback, int numVotes) :
-        callback(callback),
-        numVotes(numVotes)
+    CountedIsSupported(const IsSupportedCallbackPtr& callback, size_t numVotes) :
+        callback(callback)
     {
-        if (numVotes == 0)
-        {
-            callback->result(false);
-            this->callback = 0;
-        }
+        init(numVotes);
     }
 
-    void result(bool result)
+    void result(bool result) { invoke(result); }
+
+protected:
+    void processInvocation(bool result)
     {
-        boost::lock_guard<boost::mutex> guard(mLock);
         // any affirmitive vote means success
-        if (result && callback)
+        if (result)
         {
-            callback->result(true);
-            callback = 0;
+            boost::lock_guard<boost::mutex> guard(mMutex);
+            if (callback)
+            {
+                callback->result(true);
+                callback = 0;
+            }
         }
+    }
+
+    void processCompletion()
+    {
+        boost::lock_guard<boost::mutex> guard(mMutex);
         // all negative votes means failure
-        if (--numVotes == 0 && callback)
+        if (callback)
         {
             callback->result(false);
             callback = 0;
@@ -213,9 +223,7 @@ public:
     }
 
 private:
-    boost::mutex mLock;
     IsSupportedCallbackPtr callback;
-    int numVotes;
 };
 
 /**

commit e27fb2812f4e636c49ea171bf07c55cb4ff76b38
Author: David M. Lee <dlee at digium.com>
Date:   Thu Jan 6 10:41:23 2011 -0600

    Refactored LocateOneCollector and LocateAllCollector.

diff --git a/src/ServiceLocatorManagement.cpp b/src/ServiceLocatorManagement.cpp
index 29ca8b8..3253736 100644
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@ -25,12 +25,14 @@
 #include "ServiceLocatorManagement.h"
 #include "ServiceManagement.h"
 #include "logger.h"
+#include "ResponseCollector.h"
 
 using namespace std;
 using namespace AsteriskSCF::System::Discovery;
 using namespace AsteriskSCF::System::Logging;
 using namespace AsteriskSCF::Core::Discovery::V1;
 using namespace AsteriskSCF::ServiceDiscovery;
+using namespace AsteriskSCF;
 
 //
 // Normally would use an anonymous namespace here, but there is a compiler
@@ -123,28 +125,26 @@ private:
     ServiceLocatorParamsComparePrx mCompare;
 };
 
+/** Paramater type for Locator, to allow use of ResponseCollector. */
+typedef std::pair<bool, ServiceManagementImplPtr> LocateParam;
+
 /**
  * Interface that the LocateCallback uses for collecting information on which
  * services are supported and which are not.
  */
-class LocateCollector : public IceUtil::Shared
+class LocateCollector :
+    public IceUtil::Shared,
+    public ResponseCollector<const LocateParam&>
+
 {
-public:
-    virtual ~LocateCollector() {}
-    /**
-     * Invoked for every service in ServiceLocatorManagementImplPriv::mServices.
-     * @param management Pointer to the management object
-     * @param supported resultsf from ServiceManagementImplPtr::isSupported
-     */
-    virtual void isSupported(const ServiceManagementImplPtr& management,
-        bool supported) = 0;
 };
 
 /**
  * A LocateCollector which forwards the first supported service to the
  * given Ice callback.
  */
-class LocateOneCollector : public LocateCollector
+class LocateOneCollector :
+    public LocateCollector
 {
 public:
     /**
@@ -152,32 +152,31 @@ public:
      * @param cb Ice callback.
      * @param numVotes The number of times isSupported will be called.
      */
-    LocateOneCollector(const AMD_ServiceLocator_locatePtr& cb, int numVotes) :
-        mCallback(cb), mNumVotes(numVotes)
+    LocateOneCollector(const AMD_ServiceLocator_locatePtr& cb, size_t numVotes) :
+        mCallback(cb)
     {
-        assert(mNumVotes >= 0);
-        if (mNumVotes == 0)
-        {
-            ServiceNotFound e;
-            mCallback->ice_exception(e);
-            mCallback = 0;
-        }
+        init(numVotes);
     }
 
-    void isSupported(const ServiceManagementImplPtr& management, bool supported)
+protected:
+    void processInvocation(const LocateParam& p)
     {
-        boost::lock_guard<boost::mutex> guard(mLock);
-
-        if (supported && mCallback)
+        if (p.first)
         {
-            mCallback->ice_response(management->getService());
-            // clear the mCallback pointer so we only answer once
-            mCallback = 0;
+            boost::lock_guard<boost::mutex> guard(mMutex);
+            if (mCallback)
+            {
+                mCallback->ice_response(p.second->getService());
+                // clear the mCallback pointer so we only answer once
+                mCallback = 0;
+            }
         }
+    }
 
-        assert(mNumVotes > 0); // isSupported was called too many times
-
-        if (--mNumVotes == 0 && mCallback)
+    void processCompletion()
+    {
+        boost::lock_guard<boost::mutex> guard(mMutex);
+        if (mCallback)
         {
             ServiceNotFound e;
             mCallback->ice_exception(e);
@@ -187,18 +186,16 @@ public:
     }
 
 private:
-    boost::mutex mLock;
     /** Ice callback */
     AMD_ServiceLocator_locatePtr mCallback;
-    /** The number of times isSupported will be called. */
-    int mNumVotes;
 };
 
 /**
  * A LocateCollector which makes a list of all supported services to
  * forward to the given Ice callback.
  */
-class LocateAllCollector : public LocateCollector
+class LocateAllCollector :
+    public LocateCollector
 {
 public:
     /**
@@ -206,50 +203,42 @@ public:
      * @param cb Ice callback.
      * @param numVotes The number of times isSupported will be called.
      */
-    LocateAllCollector(const AMD_ServiceLocator_locateAllPtr& cb, int numVotes) :
-        mCallback(cb), mNumVotes(numVotes)
+    LocateAllCollector(const AMD_ServiceLocator_locateAllPtr& cb, size_t numVotes)
+        :
+        mCallback(cb)
+    {
+        init(numVotes);
+    }
+
+protected:
+    void processInvocation(const LocateParam& p)
     {
-        assert(mNumVotes >= 0);
-        if (mNumVotes == 0)
+        if (p.first)
         {
-            ServiceNotFound e;
-            mCallback->ice_exception(e);
-            mCallback = 0;
+            boost::lock_guard<boost::mutex> guard(mMutex);
+            mValue.push_back(p.second->getService());
         }
     }
 
-    void isSupported(const ServiceManagementImplPtr& management, bool supported)
+    void processCompletion()
     {
-        boost::lock_guard<boost::mutex> guard(mLock);
-
-        if (supported)
+        // the votes are in!
+        if (!mValue.empty())
         {
-            mValue.push_back(management->getService());
+            mCallback->ice_response(mValue);
         }
-
-        if (--mNumVotes == 0 && mCallback)
+        else
         {
-            // the votes are in!
-            if (!mValue.empty())
-            {
-                mCallback->ice_response(mValue);
-            }
-            else
-            {
-                ServiceNotFound e;
-                mCallback->ice_exception(e);
-            }
-            // we're done with the callback
-            mCallback = 0;
+            ServiceNotFound e;
+            mCallback->ice_exception(e);
         }
+        // we're done with the callback
+        mCallback = 0;
     }
 
 private:
-    boost::mutex mLock;
     /** Ice callback */
     AMD_ServiceLocator_locateAllPtr mCallback;
-    /** The number of times isSupported will be called. */
-    int mNumVotes;
     /** Collected results */
     Ice::ObjectProxySeq mValue;
 };
@@ -276,7 +265,7 @@ public:
     {
         // delegation to thread safe object
         // no lock needed
-        collector->isSupported(management, result);
+        collector->invoke(LocateParam(result, management));
         collector = 0;
     }
 private:

commit ce4af3b38292f34cf39ead9b498cd23c065e6ee3
Author: David M. Lee <dlee at digium.com>
Date:   Thu Jan 6 10:28:43 2011 -0600

    Adding AmiCollector to include path.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2d9a45b..113a797 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -30,6 +30,7 @@ if(NOT logger_dir)
 endif()
 include_directories(${logger_dir}/common)
 include_directories(${logger_dir}/client/src)
+include_directories(${utils_dir}/AmiCollector/src)
 
 asterisk_scf_component_build_icebox(service_locator)
 target_link_libraries(service_locator logging-client)

commit bd71b67137f32cd4c50f4dec70f2d5b4a4a29f3d
Author: David M. Lee <dlee at digium.com>
Date:   Tue Dec 14 14:47:31 2010 -0600

    Proxy was accidentally double registered.
    
    Since we're sharing the service locator between test suites, this caused some
    other tests to fail.

diff --git a/test/TestComparatorBlocking.cpp b/test/TestComparatorBlocking.cpp
index 627db32..dc679ac 100644
--- a/test/TestComparatorBlocking.cpp
+++ b/test/TestComparatorBlocking.cpp
@@ -132,7 +132,7 @@ class Fixture
 {
 public:
     Fixture() :
-        // create an register the blocker
+        // create and register the blocker
         blockerCommunicator(Ice::initialize()),
         blocker(new BlockingComparator()),
         blockerAdapter(
@@ -199,11 +199,6 @@ BOOST_FIXTURE_TEST_SUITE(TestComparatorBlocking, Fixture)
  */
 BOOST_AUTO_TEST_CASE(testNonBlocking)
 {
-    ServiceManagementPrx proxy = management->addService(management, "self");
-    ServiceLocatorParamsPtr params = new ServiceLocatorParams();
-    params->category = "self";
-    proxy->addLocatorParams(params, "blocker");
-
     // unblock, to test our test logic
     blocker->unblock();
 

commit 2a87802d144b3b22356ca3f684db6577df43b365
Author: David M. Lee <dlee at digium.com>
Date:   Tue Dec 14 14:47:01 2010 -0600

    Waaaaay too many debug statements.

diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index a389ef0..678bbd0 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -191,7 +191,6 @@ public:
         if (numVotes == 0)
         {
             callback->result(false);
-            lg(Debug) << "  ...isSupported() = false\n";
             this->callback = 0;
         }
     }
@@ -202,14 +201,12 @@ public:
         // any affirmitive vote means success
         if (result && callback)
         {
-            lg(Debug) << "  ...isSupported() = true\n";
             callback->result(true);
             callback = 0;
         }
         // all negative votes means failure
         if (--numVotes == 0 && callback)
         {
-            lg(Debug) << "  ...isSupported() = false\n";
             callback->result(false);
             callback = 0;
         }
@@ -232,14 +229,12 @@ private:
 void ServiceManagementImpl::isSupported(const ServiceLocatorParamsPtr& params, const IsSupportedCallbackPtr& callback)
 {
     boost::shared_lock<boost::shared_mutex> lock(mImpl->mLock);
-    lg(Debug) << "isSupported(" << params->category << ")\n";
 
     /* If this service is suspended we can just skip the entire check and
      * return false now, easy as pie
      */
     if (mImpl->mSuspended)
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = false\n";
         callback->result(false);
         return;
     }
@@ -271,19 +266,16 @@ const std::string& ServiceManagementImpl::getGuid() const
  */
 void ServiceLocatorParamsSpec::isSupported(const ServiceLocatorParamsPtr& params, const IsSupportedCallbackPtr& callback)
 {
-    lg(Debug) << "isSupported(" << params->category << ")\n";
     // If no category is passed to us then the component doing the locate wants
     // everything/anything, so give it to them
     if (params->category.empty())
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = true\n";
         callback->result(true);
     }
     /* This is just a simple comparison that acts as a preliminary, and
      * perhaps final, check */
     else if (mParams->category != params->category)
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = false\n";
         callback->result(false);
     }
     /* If a comparator was provided then yield to it for a final yay or nay */

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


-- 
asterisk-scf/integration/servicediscovery.git



More information about the asterisk-scf-commits mailing list