From 244d7b436e11f8705c79165ba67d26d6db3b6d0a Mon Sep 17 00:00:00 2001 From: "doil.kwon" Date: Tue, 29 Sep 2015 15:59:05 +0900 Subject: [PATCH] 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/3249 Tested-by: jenkins-iotivity Reviewed-by: Madan Lanka --- service/resource-encapsulation/SConscript | 1 - .../include/RCSDiscoveryManager.h | 2 +- .../src/resourceClient/RCSDiscoveryManager.cpp | 2 +- .../src/resourceClient/RCSDiscoveryManagerImpl.cpp | 27 ++++-- .../src/resourceClient/RCSDiscoveryManagerImpl.h | 9 +- .../unittests/DiscoveryManagerTest.cpp | 103 +++++++++++++++++---- 6 files changed, 110 insertions(+), 34 deletions(-) diff --git a/service/resource-encapsulation/SConscript b/service/resource-encapsulation/SConscript index cb38c26..4a21ac5 100644 --- 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 --- 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 --- 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 --- 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::max();; + constexpr unsigned int LIMITNUMBER = std::numeric_limits::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 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 lock(m_mutex); m_discoveryMap.insert(std::make_pair(discoveryId, std::move(discoveryItem))); return std::unique_ptr( @@ -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 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& resource) + bool DiscoveryRequestInfo::isKnownResource(const std::shared_ptr& 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 --- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h +++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h @@ -35,8 +35,6 @@ #include #include -#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 m_discoveryMap; + std::unordered_map 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 --- a/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp +++ b/service/resource-encapsulation/unittests/DiscoveryManagerTest.cpp @@ -27,29 +27,39 @@ #include "OCPlatform.h" +#include +#include + 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 discoveryTask; - + typedef std::unique_ptr 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(); +} -- 2.7.4