[asterisk-scf-commits] asterisk-scf/release/servicediscovery.git branch "master" updated.

Commits to the Asterisk SCF project code repositories asterisk-scf-commits at lists.digium.com
Mon Aug 8 03:10:47 CDT 2011


branch "master" has been updated
       via  8b7006f218db702c9afcefefc892ea55f11e0e78 (commit)
      from  8f135cc47b45a9b3099b5a71c6c2a05b0a3b4836 (commit)

Summary of changes:
 src/ServiceLocatorManagement.cpp |   11 +++-
 src/ServiceManagement.cpp        |  149 +++++++++++++++++++++++++++-----------
 test/TestComparatorBlocking.cpp  |    2 +
 test/TestServiceLocator.cpp      |   39 ++++++-----
 4 files changed, 140 insertions(+), 61 deletions(-)


- Log -----------------------------------------------------------------
commit 8b7006f218db702c9afcefefc892ea55f11e0e78
Author: Ken Hunt <ken.hunt at digium.com>
Date:   Tue Jun 21 11:45:58 2011 -0500

    Expanded ServiceLocatorParams.

diff --git a/src/ServiceLocatorManagement.cpp b/src/ServiceLocatorManagement.cpp
index 28168cd..0cfd407 100644
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@ -370,10 +370,17 @@ ServiceLocatorManagementImpl::ServiceLocatorManagementImpl(
 {
 }
 
+static string debugPrintParams(const ServiceLocatorParamsPtr& params)
+{
+    string result;
+    result = "(category=" + params->category + ", service=" + params->service + ", id=" + params->id + ")";
+    return result;
+}
+
 void ServiceLocatorManagementImpl::locate(const AMD_ServiceLocator_locatePtr& cb,
     const ServiceLocatorParamsPtr& params)
 {
-    lg(Debug) << "locate(" << params->category << ')';
+    lg(Debug) << "locate(" << debugPrintParams(params);
     boost::shared_lock<boost::shared_mutex> lock(mImpl->mLock);
 
     LocateCollectorPtr collector = new LocateOneCollector(cb,
@@ -393,7 +400,7 @@ void ServiceLocatorManagementImpl::locateAll(
     const AMD_ServiceLocator_locateAllPtr& cb,
     const ServiceLocatorParamsPtr& params)
 {
-    lg(Debug) << "locateAll(" << params->category << ')';
+    lg(Debug) << "locateAll(" << debugPrintParams(params);
     boost::shared_lock<boost::shared_mutex> lock(mImpl->mLock);
 
     LocateCollectorPtr collector = new LocateAllCollector(cb,
diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index 00397f1..ab08c00 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -56,24 +56,24 @@ public:
     ServiceLocatorParamsSpec(const ServiceLocatorParamsPtr& params,
         const std::string& compareGuid,
         ServiceLocatorManagementImplPtr management,
-	ServiceLocatorServiceStateItemPtr serviceState)
+        ServiceLocatorServiceStateItemPtr serviceState)
         :
-	mStateItem(new ServiceLocatorParamsStateItem()),
+        mStateItem(new ServiceLocatorParamsStateItem()),
         mManagement(management)
     {
-	mStateItem->key = IceUtil::generateUUID();
-	mStateItem->serviceKey = serviceState->key;
-	mStateItem->params = params;
-	mStateItem->compareGuid = compareGuid;
-	mManagement->replicateState(mStateItem);
+        mStateItem->key = IceUtil::generateUUID();
+        mStateItem->serviceKey = serviceState->key;
+        mStateItem->params = params;
+        mStateItem->compareGuid = compareGuid;
+        mManagement->replicateState(mStateItem);
     }
 
     ~ServiceLocatorParamsSpec()
     {
-	mManagement->removeState(mStateItem);
+        mManagement->removeState(mStateItem);
     }
 
-    void isSupported(const AsteriskSCF::Core::Discovery::V1::ServiceLocatorParamsPtr&, const IsSupportedCallbackPtr&);
+    bool isSupported(const AsteriskSCF::Core::Discovery::V1::ServiceLocatorParamsPtr&, const IsSupportedCallbackPtr&);
 
 private:
     /**
@@ -96,20 +96,20 @@ public:
     ServiceManagementImplPriv(ServiceManagementImpl* impl, ServiceLocatorManagementImplPtr management,
         const Ice::ObjectPrx& service, const Ice::ObjectAdapterPtr& adapter, const AsteriskSCF::System::Discovery::EventsPrx& serviceDiscoveryTopic,
         const string& guid, const Ice::Identity& identity) :
-	mStateItem(new ServiceLocatorServiceStateItem()),
+        mStateItem(new ServiceLocatorServiceStateItem()),
         mManagement(management), mAdapter(adapter), mLocatorTopic(serviceDiscoveryTopic)
     {
-	mStateItem->key = IceUtil::generateUUID();
-	mStateItem->suspended = false;
-	mStateItem->service = service;
-	mStateItem->guid = guid;
-	mManagementPrx = ServiceManagementPrx::uncheckedCast(mAdapter->add(impl, identity));
-	mStateItem->managementIdentity = identity;
+        mStateItem->key = IceUtil::generateUUID();
+        mStateItem->suspended = false;
+        mStateItem->service = service;
+        mStateItem->guid = guid;
+        mManagementPrx = ServiceManagementPrx::uncheckedCast(mAdapter->add(impl, identity));
+        mStateItem->managementIdentity = identity;
         if (mLocatorTopic)
         {
             mLocatorTopic->serviceRegistered(guid);
         }
-	mManagement->replicateState(mStateItem);
+        mManagement->replicateState(mStateItem);
     }
 
     /**
@@ -231,6 +231,13 @@ private:
     IsSupportedCallbackPtr mCallback;
 };
 
+static string debugPrintParams(const ServiceLocatorParamsPtr& params)
+{
+    string result;
+    result = "(category=" + params->category + ", service=" + params->service + ", id=" + params->id + ")";
+    return result;
+}
+
 /**
  * An internal function which is called by the service locator to perform a
  * locator parameters comparison at the service level.
@@ -248,7 +255,7 @@ void ServiceManagementImpl::isSupported(const ServiceLocatorParamsPtr& params, c
      */
     if (mImpl->mStateItem->suspended)
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = false (suspended)\n";
+        lg(Debug) << "  ...isSupported" << debugPrintParams(params) + " = false (suspended)\n";
         callback->result(false);
         return;
     }
@@ -261,7 +268,12 @@ void ServiceManagementImpl::isSupported(const ServiceLocatorParamsPtr& params, c
          spec != mImpl->mSupportedLocatorParams.end();
          ++spec)
     {
-        (*spec)->isSupported(params, myCallback);
+        if ((*spec)->isSupported(params, myCallback))
+        {
+            // If we get here, a match was found without needing to call an external comparator. 
+            // We are done. 
+            break;
+        }
     }
 }
 
@@ -277,33 +289,73 @@ const std::string& ServiceManagementImpl::getGuid() const
  * @param params A concrete class containing parameters describing the service
  *               that is trying to be found.
  * @param callback Callback to asynchronously rx the results.
+ *
+ * @return True only if we determined a match without any need for calling a custom comparator. 
+ *         Otherwise, the callback may be accumulating the results, or it's not a match. 
  */
-void ServiceLocatorParamsSpec::isSupported(const ServiceLocatorParamsPtr& params, const IsSupportedCallbackPtr& callback)
+bool ServiceLocatorParamsSpec::isSupported(const ServiceLocatorParamsPtr& params, const IsSupportedCallbackPtr& callback)
 {
-    // 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())
+    // Does the component doing the locate 
+    // want to filter based on category. 
+    if (!params->category.empty())
+    {
+        // Is this the wrong category?
+        if (mStateItem->params->category != params->category)
+        {
+            lg(Debug) << "  ...isSupported" << debugPrintParams(params) + " = false (different categories)\n";
+
+            callback->result(false);
+            return false;
+        }
+    }
+
+    // Does the component doing the locate 
+    // want all services in the category? 
+    if (params->service.empty())
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = true (empty category)\n";
+        // If a comparator was provided then yield to it. 
+        if (!mStateItem->compareGuid.empty())
+        {
+            mManagement->isSupported(mStateItem->compareGuid, params, callback);
+            return false;
+        }
+
+        // Ignore the id and treat this as a wildcard search. 
         callback->result(true);
+        return true;
     }
-    /* This is just a simple comparison that acts as a preliminary, and
-     * perhaps final, check */
-    else if (mStateItem->params->category != params->category)
+
+    // Wrong service?
+    if (mStateItem->params->service != params->service)
     {
-        lg(Debug) << "  ...isSupported(" << params->category << ") = false (different categories)\n";
+        lg(Debug) << "  ...isSupported" << debugPrintParams(params) + " = false (different services)\n";
         callback->result(false);
+        return false;
     }
-    /* If a comparator was provided then yield to it for a final yay or nay */
-    else if (!mStateItem->compareGuid.empty())
+
+    // Is this query for a specific instance?
+    if (!params->id.empty())
     {
-        mManagement->isSupported(mStateItem->compareGuid, params, callback);
+        // Wrong instance?
+        if (mStateItem->params->id != params->id)
+        {
+            lg(Debug) << "  ...isSupported" << debugPrintParams(params) + " = false (different id)\n";
+            callback->result(false);
+            return false;
+        }
     }
-    /* category matches, no comparator to turn us down.  it's a match. */
-    else
+
+    // If a comparator was provided then yield to it. 
+    if (!mStateItem->compareGuid.empty())
     {
-        callback->result(true);
+        mManagement->isSupported(mStateItem->compareGuid, params, callback);
+        return false;
     }
+
+    // If we get here we have a match on service and id. 
+    // (and category, if one was passed in.) 
+    callback->result(true);
+    return true;
 }
 
 /**
@@ -329,7 +381,7 @@ void ServiceManagementImpl::suspend(const Ice::Current&)
     {
         lg(Info) << "Suspending " << mImpl->mStateItem->guid << " " << mImpl->mStateItem->service->ice_toString();
         mImpl->mStateItem->suspended = true;
-	mImpl->mManagement->replicateState(mImpl->mStateItem);
+        mImpl->mManagement->replicateState(mImpl->mStateItem);
     }
 
     if (mImpl->mLocatorTopic)
@@ -349,7 +401,7 @@ void ServiceManagementImpl::unsuspend(const Ice::Current&)
     {
         lg(Info) << "Un-suspending " << mImpl->mStateItem->guid << " " << mImpl->mStateItem->service->ice_toString();
         mImpl->mStateItem->suspended = false;
-	mImpl->mManagement->replicateState(mImpl->mStateItem);
+        mImpl->mManagement->replicateState(mImpl->mStateItem);
     }
 
     if (mImpl->mLocatorTopic)
@@ -379,17 +431,30 @@ void ServiceManagementImpl::unregister(const Ice::Current&)
     /* You'll notice no lock here. That's because we aren't actually modifying any internal state that should
      * be protected, and if we did lock here there is a chance for a deadlock which is super sad.
      */
-    lg(Info) << "Un-register " << mImpl->mStateItem->guid << " " << mImpl->mStateItem->service->ice_toString();
+    try
+    {
+        lg(Info) << "Un-register " << mImpl->mStateItem->guid << " " << mImpl->mStateItem->service->ice_toString();
 
-    mImpl->mManagement->removeState(mImpl->mStateItem);
+        mImpl->mManagement->removeState(mImpl->mStateItem);
 
-    mImpl->mAdapter->remove(mImpl->mManagementPrx->ice_getIdentity());
+        mImpl->mAdapter->remove(mImpl->mManagementPrx->ice_getIdentity());
 
-    mImpl->mManagement->removeService(this);
+        mImpl->mManagement->removeService(this);
 
-    if (mImpl->mLocatorTopic)
+        if (mImpl->mLocatorTopic)
+        {
+            mImpl->mLocatorTopic->serviceUnregistered(mImpl->mStateItem->guid);
+        }
+    }
+    catch(const std::exception& e)
+    {
+        lg(Error) << BOOST_CURRENT_FUNCTION << " : " << e.what();
+	throw;
+    }
+    catch(...)
     {
-        mImpl->mLocatorTopic->serviceUnregistered(mImpl->mStateItem->guid);
+        lg(Error) << BOOST_CURRENT_FUNCTION << " : " << "Unknown exception.";
+	throw;
     }
 }
 
diff --git a/test/TestComparatorBlocking.cpp b/test/TestComparatorBlocking.cpp
index c10b0da..c8cc599 100644
--- a/test/TestComparatorBlocking.cpp
+++ b/test/TestComparatorBlocking.cpp
@@ -207,6 +207,7 @@ BOOST_AUTO_TEST_CASE(testNonBlocking)
 
     ServiceLocatorParamsPtr dne = new ServiceLocatorParams();
     dne->category = "dne";
+    dne->service = "dne.com";
 
     try
     {
@@ -226,6 +227,7 @@ BOOST_AUTO_TEST_CASE(testBlocking)
     // try to locate something else while blocker is blocking
     ServiceLocatorParamsPtr dne = new ServiceLocatorParams();
     dne->category = "dne";
+    dne->service = "dne.com";
 
     BlockingCallbackPtr callback = new BlockingCallback();
 
diff --git a/test/TestServiceLocator.cpp b/test/TestServiceLocator.cpp
index 8016b04..d9d6ef4 100644
--- a/test/TestServiceLocator.cpp
+++ b/test/TestServiceLocator.cpp
@@ -91,17 +91,18 @@ public:
     ServiceLocatorParamsComparePrx foundCompare;
 
     /* Common implementation for locating a service */
-    bool findService(const string& category)
+    bool findService(const string& category, const string& service)
     {
         bool found = true;
 
         try
         {
-            /* Setup our parameters, for now we are simply using test as the category */
+            // Setup our parameters.
             ServiceLocatorParamsPtr params = new ServiceLocatorParams;
             params->category = category;
+            params->service = service;
 
-            /* Actually do the locate request */
+            // Actually do the locate request 
             foundCompare = ServiceLocatorParamsComparePrx::uncheckedCast(discovery->locate(params));
 
             /* If we get here we didn't get an exception back telling us no service found... which is wrong! */
@@ -121,7 +122,7 @@ public:
     }
 
     /* Common implementation for finding multiple services */
-    bool findServices(const string& category, unsigned int expectedNumOfResults)
+    bool findServices(const string& category, const string& service, unsigned int expectedNumOfResults)
     {
         bool found = true;
 
@@ -129,6 +130,7 @@ public:
         {
             ServiceLocatorParamsPtr params = new ServiceLocatorParams;
             params->category = category;
+            params->service = service;
 
             Ice::ObjectProxySeq services = discovery->locateAll(params);
 
@@ -241,7 +243,7 @@ BOOST_GLOBAL_FIXTURE(GlobalIceFixture);
  */
 BOOST_AUTO_TEST_CASE(ServiceNotFoundBeforeAdd)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(!found);
 }
@@ -251,7 +253,7 @@ BOOST_AUTO_TEST_CASE(ServiceNotFoundBeforeAdd)
  */
 BOOST_AUTO_TEST_CASE(ServicesNotFoundBeforeAdd)
 {
-    bool found = testbed.findServices("test", 1);
+    bool found = testbed.findServices("test", "default", 1);
 
     BOOST_CHECK(!found);
 }
@@ -289,7 +291,7 @@ BOOST_AUTO_TEST_CASE(AddService)
  */
 BOOST_AUTO_TEST_CASE(ServiceNotFoundAfterAdd)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(!found);
 }
@@ -299,7 +301,7 @@ BOOST_AUTO_TEST_CASE(ServiceNotFoundAfterAdd)
  */
 BOOST_AUTO_TEST_CASE(ServicesNotFoundAfterAdd)
 {
-    bool found = testbed.findServices("test", 0);
+    bool found = testbed.findServices("test", "default", 0);
 
     BOOST_CHECK(!found);
 }
@@ -315,6 +317,7 @@ BOOST_AUTO_TEST_CASE(AddLocatorParamsWithoutCompareService)
     {
         ServiceLocatorParamsPtr params = new ServiceLocatorParams;
         params->category = "test";
+        params->service = "default";
 
         testbed.compareManagement->addLocatorParams(params, "");
 
@@ -337,7 +340,7 @@ BOOST_AUTO_TEST_CASE(AddLocatorParamsWithoutCompareService)
  */
 BOOST_AUTO_TEST_CASE(FindServiceWithoutCompareService)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(found);
 }
@@ -347,7 +350,7 @@ BOOST_AUTO_TEST_CASE(FindServiceWithoutCompareService)
  */
 BOOST_AUTO_TEST_CASE(FindServicesWithoutCompareService)
 {
-    bool found = testbed.findServices("test", 1);
+    bool found = testbed.findServices("test", "default", 1);
 
     BOOST_CHECK(found);
 }
@@ -448,6 +451,7 @@ BOOST_AUTO_TEST_CASE(AddLocatorParamsWithCompareService)
     {
         ServiceLocatorParamsPtr params = new ServiceLocatorParams;
         params->category = "test2";
+        params->service = "default";
 
         testbed.compareManagement->addLocatorParams(params, "testcompare");
 
@@ -470,7 +474,7 @@ BOOST_AUTO_TEST_CASE(AddLocatorParamsWithCompareService)
  */
 BOOST_AUTO_TEST_CASE(FindServiceWithCompareService)
 {
-    bool found = testbed.findService("test2");
+    bool found = testbed.findService("test2", "default");
 
     BOOST_CHECK(found);
 }
@@ -513,10 +517,11 @@ BOOST_AUTO_TEST_CASE(FindMultipleServices)
     ServiceManagementPrx compareManagement = testbed.management->addService(testbed.compare, "testcompare2");
 
     params->category = "test";
+    params->service = "default";
 
     compareManagement->addLocatorParams(params, "");
 
-    bool found = testbed.findServices("test", 2);
+    bool found = testbed.findServices("test", "", 2);
 
     compareManagement->unregister();
 
@@ -535,7 +540,7 @@ BOOST_AUTO_TEST_CASE(FindMultipleServicesUsingEmptyCategory)
 
     compareManagement->addLocatorParams(params, "");
 
-    bool found = testbed.findServices("", 2);
+    bool found = testbed.findServices("", "", 2);
 
     compareManagement->unregister();
 
@@ -571,7 +576,7 @@ BOOST_AUTO_TEST_CASE(ServiceSuspend)
  */
 BOOST_AUTO_TEST_CASE(FindServiceAfterSuspend)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(!found);
 }
@@ -605,7 +610,7 @@ BOOST_AUTO_TEST_CASE(ServiceUnsuspend)
  */
 BOOST_AUTO_TEST_CASE(FindServiceAfterUnsuspend)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(found);
 }
@@ -696,7 +701,7 @@ BOOST_AUTO_TEST_CASE(RemoveAlreadyRemovedCompareService)
  */
 BOOST_AUTO_TEST_CASE(FindServiceAfterCompareServiceRemoved)
 {
-    bool found = testbed.findService("test2");
+    bool found = testbed.findService("test2", "default");
 
     BOOST_CHECK(!found);
 }
@@ -730,7 +735,7 @@ BOOST_AUTO_TEST_CASE(ServiceUnregister)
  */
 BOOST_AUTO_TEST_CASE(FindServiceAfterUnregister)
 {
-    bool found = testbed.findService("test");
+    bool found = testbed.findService("test", "default");
 
     BOOST_CHECK(!found);
 }

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


-- 
asterisk-scf/release/servicediscovery.git



More information about the asterisk-scf-commits mailing list