modify timing issue and add test case for DiscoveryManagerUnitTest

- default waiting time is 3sec as each test.
- change discovery checking way from using sleep method to condition_variable.
: just unlock waiting mutex when is called callback function.
- request multiple discovery that have equal contents, and just one canceled.
  other request will be received callback function.

modify createId's logic for RCSDiscoveryManagerImpl.

modify Sconscript
- directory location for DiscoveryMaanger' unittest code is changed.

Change-Id: I3dfd5b1440959eac200746eba098ebd1ff93b69b
Signed-off-by: doil.kwon <doil.kwon@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/3249
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Madan Lanka <lanka.madan@samsung.com>
(cherry picked from commit 244d7b436e11f8705c79165ba67d26d6db3b6d0a)
Reviewed-on: https://gerrit.iotivity.org/gerrit/3275

diff --git a/service/resource-encapsulation/SConscript b/service/resource-encapsulation/SConscript
index cb38c26..4a21ac5 100644 (file)
--- a/service/resource-encapsulation/SConscript
+++ b/service/resource-encapsulation/SConscript
@@ -120,7 +120,6 @@ if target_os == 'linux':
     SConscript('unittests/SConscript')
     SConscript('src/resourceCache/unittests/SConscript')
     SConscript('src/resourceBroker/unittest/SConscript')
-    SConscript('src/resourceClient/unittests/SConscript')
 
 if target_os == 'android':
        SConscript('android/SConscript')

diff --git a/service/resource-encapsulation/include/RCSDiscoveryManager.h b/service/resource-encapsulation/include/RCSDiscoveryManager.h
index 773c114..29fc919 100644 (file)
--- a/service/resource-encapsulation/include/RCSDiscoveryManager.h
+++ b/service/resource-encapsulation/include/RCSDiscoveryManager.h
@@ -180,7 +180,7 @@ namespace OIC
 
             private:
                 RCSDiscoveryManager() = default;
-                ~RCSDiscoveryManager()= default;;
+                ~RCSDiscoveryManager()= default;
 
                 friend class DiscoveryTask;
         };

diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp
index 7ef459c..62b16b7 100755 (executable)
--- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp
+++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp
@@ -37,7 +37,7 @@ namespace OIC {
 
         bool RCSDiscoveryManager::DiscoveryTask::isCanceled()
         {
-            return false;
+            return RCSDiscoveryManagerImpl::getInstance()->isCanceled(m_id);
         }
 
         void RCSDiscoveryManager::DiscoveryTask::cancel()

diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp
index 8d2ea81..6dec042 100755 (executable)
--- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp
+++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp
@@ -28,7 +28,7 @@
 
 namespace
 {
-    constexpr unsigned int LIMITNUMBER = std::numeric_limits<unsigned int>::max();;
+    constexpr unsigned int LIMITNUMBER = std::numeric_limits<unsigned int>::max();
     constexpr unsigned int INTERVALTIME = 60000;
 }
 
@@ -50,7 +50,8 @@ namespace OIC
         }
 
         void RCSDiscoveryManagerImpl::onResourceFound(std::shared_ptr< PrimitiveResource > resource,
-                    RCSDiscoveryManagerImpl::ID discoveryId, const RCSDiscoveryManager::ResourceDiscoveredCallback& discoverCB)
+                    RCSDiscoveryManagerImpl::ID discoveryId,
+                    const RCSDiscoveryManager::ResourceDiscoveredCallback& discoverCB)
         {
             {
                 std::lock_guard<std::mutex> lock(m_mutex);
@@ -74,10 +75,11 @@ namespace OIC
             ID discoveryId = createId();
             auto discoverCb = std::bind(&RCSDiscoveryManagerImpl::onResourceFound, this,
                     std::placeholders::_1, discoveryId, std::move(cb));
-            DiscoverRequestInfo discoveryItem(RCSAddressDetail::getDetail(address)->getAddress(), relativeUri,
+            DiscoveryRequestInfo discoveryItem(RCSAddressDetail::getDetail(address)->getAddress(), relativeUri,
                     resourceType, std::move(discoverCb));
             discoveryItem.discoverRequest();
 
+            std::lock_guard<std::mutex> lock(m_mutex);
             m_discoveryMap.insert(std::make_pair(discoveryId, std::move(discoveryItem)));
 
             return std::unique_ptr<RCSDiscoveryManager::DiscoveryTask>(
@@ -128,7 +130,7 @@ namespace OIC
             {
                 throw RCSException { "Discovery request is full!" };
             }
-
+            s_uniqueId++;
             while(m_discoveryMap.find(s_uniqueId) != m_discoveryMap.end())
             {
                 s_uniqueId++;
@@ -142,17 +144,26 @@ namespace OIC
             m_discoveryMap.erase(id);
         }
 
-        DiscoverRequestInfo::DiscoverRequestInfo(const std::string &address, const std::string &relativeUri,
+        bool RCSDiscoveryManagerImpl::isCanceled(unsigned int id)
+        {
+            std::lock_guard<std::mutex> lock(m_mutex);
+            auto it = m_discoveryMap.find(id);
+            if(it == m_discoveryMap.end()) return true;
+
+            return false;
+        }
+
+        DiscoveryRequestInfo::DiscoveryRequestInfo(const std::string &address, const std::string &relativeUri,
                 const std::string &resourceType, DiscoverCallback cb) : m_address(address),
                         m_relativeUri(relativeUri), m_resourceType(resourceType), m_discoverCB(cb) {}
 
-        void DiscoverRequestInfo::discoverRequest() const
+        void DiscoveryRequestInfo::discoverRequest() const
         {
             OIC::Service::discoverResource(m_address, m_relativeUri + "?rt=" + m_resourceType,
                     OCConnectivityType::CT_DEFAULT, m_discoverCB);
         }
 
-        bool DiscoverRequestInfo::isKnownResource(const std::shared_ptr<PrimitiveResource>& resource)
+        bool DiscoveryRequestInfo::isKnownResource(const std::shared_ptr<PrimitiveResource>& resource)
         {
             std::string resourceId = resource->getSid() + resource->getUri();
 
@@ -163,7 +174,7 @@ namespace OIC
             return false;
         }
 
-        bool DiscoverRequestInfo::isMatchingAddress(const std::string& address) const
+        bool DiscoveryRequestInfo::isMatchingAddress(const std::string& address) const
         {
             return m_address == RCSAddressDetail::getDetail(RCSAddress::multicast())->getAddress()
                     || m_address == address;

diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h
index 7b1605c..5f3dabc 100644 (file)
--- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h
+++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h
@@ -35,8 +35,6 @@
 #include <mutex>
 #include <unordered_map>
 
-#include "octypes.h"
-
 #include "RCSDiscoveryManager.h"
 #include "ExpiryTimer.h"
 #include "PrimitiveResource.h"
@@ -55,10 +53,10 @@ namespace OIC
          *
          * @see RCSDiscoveryManager
          */
-        class DiscoverRequestInfo
+        class DiscoveryRequestInfo
         {
             public:
-                DiscoverRequestInfo(const std::string &, const std::string &,
+                DiscoveryRequestInfo(const std::string &, const std::string &,
                         const std::string &, DiscoverCallback);
 
             private:
@@ -128,6 +126,7 @@ namespace OIC
                         RCSDiscoveryManager::ResourceDiscoveredCallback cb);
 
                 void cancel(ID);
+                bool isCanceled(ID);
 
             private:
                 RCSDiscoveryManagerImpl();
@@ -175,7 +174,7 @@ namespace OIC
                 ExpiryTimer m_timer;
 
             private:
-                std::unordered_map<ID,DiscoverRequestInfo> m_discoveryMap;
+                std::unordered_map<ID,DiscoveryRequestInfo> m_discoveryMap;
                 std::mutex m_mutex;
         };
     }

diff --git a/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp b/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp
index 0cbd045..361bfc1 100644 (file)
--- a/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp
+++ b/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp
@@ -27,29 +27,39 @@
 
 #include "OCPlatform.h"
 
+#include <condition_variable>
+#include <mutex>
+
 using namespace OIC::Service;
 using namespace OC::OCPlatform;
 
 constexpr char RESOURCEURI[]{ "/a/TemperatureSensor" };
 constexpr char RESOURCETYPE[]{ "resource.type" };
 constexpr char RESOURCEINTERFACE[]{ "oic.if.baseline" };
-constexpr int DiscoveryTaskDELAYTIME = 7;
+constexpr int DEFAULT_DISCOVERYTASK_DELAYTIME = 3000;
+
+void resourceDiscoveredForCall(RCSRemoteResourceObject::Ptr) {}
+void resourceDiscoveredForNeverCall(RCSRemoteResourceObject::Ptr) {}
 
 class DiscoveryManagerTest: public TestWithMock
 {
 public:
 
-    RCSResourceObject::Ptr server;
-    RCSRemoteResourceObject::Ptr object;
-    std::unique_ptr<RCSDiscoveryManager::DiscoveryTask> discoveryTask;
-
+    typedef std::unique_ptr<RCSDiscoveryManager::DiscoveryTask> DiscoveryTaskPtr;
+    typedef std::function< void(std::shared_ptr< RCSRemoteResourceObject >) >
+                                       ResourceDiscoveredCallback;
 public:
 
-    void startDiscovery()
+    static DiscoveryTaskPtr discoverResource(ResourceDiscoveredCallback cb)
     {
         const std::string uri  = "/oic/res";
-        discoveryTask = RCSDiscoveryManager::getInstance()->discoverResourceByType(RCSAddress::multicast(),
-                 uri, RESOURCETYPE, &resourceDiscovered);
+        return RCSDiscoveryManager::getInstance()->discoverResourceByType(RCSAddress::multicast(),
+                 uri, RESOURCETYPE, cb);
+    }
+
+    void startDiscovery()
+    {
+        discoveryTask = discoverResource(resourceDiscoveredForCall);
     }
 
     void cancelDiscovery()
@@ -57,25 +67,42 @@ public:
         discoveryTask->cancel();
     }
 
+    bool isCanceled()
+    {
+        return discoveryTask->isCanceled();
+    }
+
     void createResource()
     {
         server = RCSResourceObject::Builder(RESOURCEURI, RESOURCETYPE, RESOURCEINTERFACE).build();
     }
 
-    void waitForDiscoveryTask()
+    void proceed()
+    {
+        cond.notify_all();
+    }
+
+    void waitForDiscoveryTask(int waitingTime = DEFAULT_DISCOVERYTASK_DELAYTIME)
     {
-        sleep(DiscoveryTaskDELAYTIME);
+        std::unique_lock< std::mutex > lock{ mutex };
+        cond.wait_for(lock, std::chrono::milliseconds{ waitingTime });
     }
 
-    static void resourceDiscovered(std::shared_ptr< RCSRemoteResourceObject >) {}
+private:
 
+    std::condition_variable cond;
+    std::mutex mutex;
+    RCSResourceObject::Ptr server;
+    RCSRemoteResourceObject::Ptr object;
+    DiscoveryTaskPtr discoveryTask;
 };
 
 TEST_F(DiscoveryManagerTest, resourceIsNotSupportedPresenceBeforeDiscovering)
 {
     createResource();
 
-    mocks.ExpectCallFunc(resourceDiscovered);
+    mocks.ExpectCallFunc(resourceDiscoveredForCall).Do(
+        [this](RCSRemoteResourceObject::Ptr){ proceed();});
 
     startDiscovery();
     waitForDiscoveryTask();
@@ -86,15 +113,18 @@ TEST_F(DiscoveryManagerTest, resourceIsSupportedPresenceBeforeDiscovering)
     startPresence(10);
     createResource();
 
-    mocks.ExpectCallFunc(resourceDiscovered);
+    mocks.ExpectCallFunc(resourceDiscoveredForCall).Do(
+        [this](RCSRemoteResourceObject::Ptr){ proceed();});
 
     startDiscovery();
     waitForDiscoveryTask();
+    stopPresence();
 }
 
 TEST_F(DiscoveryManagerTest, resourceIsNotSupportedPresenceAfterDiscovering)
 {
-    mocks.ExpectCallFunc(resourceDiscovered);
+    mocks.ExpectCallFunc(resourceDiscoveredForCall).Do(
+        [this](RCSRemoteResourceObject::Ptr){ proceed();});
 
     startDiscovery();
     createResource();
@@ -103,12 +133,14 @@ TEST_F(DiscoveryManagerTest, resourceIsNotSupportedPresenceAfterDiscovering)
 
 TEST_F(DiscoveryManagerTest, resourceIsSupportedPresenceAndAfterDiscovering)
 {
-    mocks.ExpectCallFunc(resourceDiscovered);
+    mocks.ExpectCallFunc(resourceDiscoveredForCall).Do(
+        [this](RCSRemoteResourceObject::Ptr){ proceed();});
 
     startPresence(10);
     startDiscovery();
     createResource();
     waitForDiscoveryTask();
+    stopPresence();
 }
 
 TEST_F(DiscoveryManagerTest, cancelDiscoveryTaskAfterDiscoveryResource)
@@ -116,11 +148,10 @@ TEST_F(DiscoveryManagerTest, cancelDiscoveryTaskAfterDiscoveryResource)
     startDiscovery();
     cancelDiscovery();
 
-    mocks.NeverCallFunc(resourceDiscovered);
+    mocks.NeverCallFunc(resourceDiscoveredForCall);
 
-    sleep(3);
+    waitForDiscoveryTask();
     createResource();
-
 }
 
 TEST_F(DiscoveryManagerTest, cancelDiscoveryTaskNotStartDiscoveryResource)
@@ -129,3 +160,39 @@ TEST_F(DiscoveryManagerTest, cancelDiscoveryTaskNotStartDiscoveryResource)
     cancelDiscovery();
     cancelDiscovery();
 }
+
+TEST_F(DiscoveryManagerTest, isCanceledAfterCancelDiscoveryTask)
+{
+    startDiscovery();
+    cancelDiscovery();
+
+    ASSERT_TRUE(isCanceled());
+}
+
+TEST_F(DiscoveryManagerTest, multipleDiscoveryRequestAndCancelJustOneDiscoveryRequest)
+{
+    DiscoveryTaskPtr canceledTask = discoverResource(resourceDiscoveredForCall);
+    DiscoveryTaskPtr notCanceledTask_1 = discoverResource(resourceDiscoveredForCall);
+    DiscoveryTaskPtr notCanceledTask_2 = discoverResource(resourceDiscoveredForCall);
+
+    canceledTask->cancel();
+
+    ASSERT_TRUE(canceledTask->isCanceled());
+    ASSERT_FALSE(notCanceledTask_1->isCanceled());
+    ASSERT_FALSE(notCanceledTask_2->isCanceled());
+}
+
+TEST_F(DiscoveryManagerTest, equalDiscoveryRequestsAndCancelJustOneRequest)
+{
+    mocks.ExpectCallFunc(resourceDiscoveredForCall).Do(
+        [this](RCSRemoteResourceObject::Ptr){ proceed();});
+
+    mocks.NeverCallFunc(resourceDiscoveredForNeverCall);
+
+    DiscoveryTaskPtr notCanceledTask = discoverResource(resourceDiscoveredForCall);
+    DiscoveryTaskPtr canceledTask = discoverResource(resourceDiscoveredForNeverCall);
+    canceledTask->cancel();
+
+    createResource();
+    waitForDiscoveryTask();
+}