From c0165b4da73afb0dbad06b743c28d43daa605b9b Mon Sep 17 00:00:00 2001 From: coderhyme Date: Mon, 4 Jan 2016 22:12:27 -0800 Subject: [PATCH] Refactored 'RCSDiscoveryManager' The class is refactored for readability and better structure. Change-Id: Iefb4b71fb074358e833d779139fbb646c759f022 Signed-off-by: coderhyme Reviewed-on: https://gerrit.iotivity.org/gerrit/4751 Tested-by: jenkins-iotivity Reviewed-by: Uze Choi --- .../include/RCSDiscoveryManager.h | 4 +- .../primitiveResource/include/RCSAddressDetail.h | 2 + .../common/primitiveResource/src/RCSAddress.cpp | 5 + .../src/resourceClient/RCSDiscoveryManager.cpp | 8 +- .../src/resourceClient/RCSDiscoveryManagerImpl.cpp | 127 +++++++++++---------- .../src/resourceClient/RCSDiscoveryManagerImpl.h | 38 +++--- 6 files changed, 100 insertions(+), 84 deletions(-) diff --git a/service/resource-encapsulation/include/RCSDiscoveryManager.h b/service/resource-encapsulation/include/RCSDiscoveryManager.h index 31feefc..663e787 100644 --- a/service/resource-encapsulation/include/RCSDiscoveryManager.h +++ b/service/resource-encapsulation/include/RCSDiscoveryManager.h @@ -90,8 +90,8 @@ namespace OIC * * @see discoverResource */ - typedef std::function< void(std::shared_ptr< RCSRemoteResourceObject >) - > ResourceDiscoveredCallback; + typedef std::function< void(std::shared_ptr< RCSRemoteResourceObject >) > + ResourceDiscoveredCallback; /** * @return RCSDiscoveryManager instance. diff --git a/service/resource-encapsulation/src/common/primitiveResource/include/RCSAddressDetail.h b/service/resource-encapsulation/src/common/primitiveResource/include/RCSAddressDetail.h index 0281ee3..4a52eaf 100644 --- a/service/resource-encapsulation/src/common/primitiveResource/include/RCSAddressDetail.h +++ b/service/resource-encapsulation/src/common/primitiveResource/include/RCSAddressDetail.h @@ -39,6 +39,8 @@ namespace OIC const std::string& getAddress() const; + bool isMulticast() const; + private: std::string m_addr; }; diff --git a/service/resource-encapsulation/src/common/primitiveResource/src/RCSAddress.cpp b/service/resource-encapsulation/src/common/primitiveResource/src/RCSAddress.cpp index 3bc4021..8518ba1 100644 --- a/service/resource-encapsulation/src/common/primitiveResource/src/RCSAddress.cpp +++ b/service/resource-encapsulation/src/common/primitiveResource/src/RCSAddress.cpp @@ -67,5 +67,10 @@ namespace OIC return m_addr; } + bool RCSAddressDetail::isMulticast() const + { + return m_addr.empty(); + } + } } diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp index 328a964..eebbd83 100755 --- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp +++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp @@ -18,6 +18,7 @@ // //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= #include "RCSDiscoveryManager.h" + #include "RCSDiscoveryManagerImpl.h" namespace OIC @@ -25,18 +26,21 @@ namespace OIC namespace Service { RCSDiscoveryManager::DiscoveryTask::DiscoveryTask(unsigned int id) : - m_id { id } + m_id{ id } { } bool RCSDiscoveryManager::DiscoveryTask::isCanceled() { - return RCSDiscoveryManagerImpl::getInstance()->isCanceled(m_id); + return m_id == RCSDiscoveryManagerImpl::INVALID_ID; } void RCSDiscoveryManager::DiscoveryTask::cancel() { + if (isCanceled()) return; + RCSDiscoveryManagerImpl::getInstance()->cancel(m_id); + m_id = RCSDiscoveryManagerImpl::INVALID_ID; } RCSDiscoveryManager* RCSDiscoveryManager::getInstance() diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp index 70114ce..6ed5e90 100755 --- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp +++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp @@ -17,26 +17,35 @@ // limitations under the License. // //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= + #include "RCSDiscoveryManagerImpl.h" #include "OCPlatform.h" #include "PresenceSubscriber.h" #include "RCSAddressDetail.h" #include "RCSAddress.h" +#include "RCSRemoteResourceObject.h" namespace { constexpr unsigned int POLLING_INTERVAL_TIME = 60000; + + std::string makeResourceId(const std::shared_ptr< OIC::Service::PrimitiveResource >& resource) + { + return resource->getSid() + resource->getUri(); + } } namespace OIC { namespace Service { + constexpr RCSDiscoveryManagerImpl::ID RCSDiscoveryManagerImpl::INVALID_ID; + RCSDiscoveryManagerImpl::RCSDiscoveryManagerImpl() { - srand (time(NULL)); - requestMulticastPresence(); + subscribePresenceWithMuticast(); + m_timer.post(POLLING_INTERVAL_TIME, std::bind(&RCSDiscoveryManagerImpl::onPolling, this)); } @@ -47,17 +56,18 @@ namespace OIC } void RCSDiscoveryManagerImpl::onResourceFound(std::shared_ptr resource, - RCSDiscoveryManagerImpl::ID discoveryId, - const RCSDiscoveryManager::ResourceDiscoveredCallback& discoverCB) + ID discoveryId, const RCSDiscoveryManager::ResourceDiscoveredCallback& discoverCB) { { - std::lock_guard < std::mutex > lock(m_mutex); + std::lock_guard< std::mutex > lock(m_mutex); auto it = m_discoveryMap.find(discoveryId); if (it == m_discoveryMap.end()) return; if (it->second.isKnownResource(resource)) return; + + it->second.addKnownResource(resource); } - discoverCB(std::make_shared < RCSRemoteResourceObject > (resource)); + discoverCB(std::make_shared< RCSRemoteResourceObject > (resource)); } RCSDiscoveryManager::DiscoveryTask::Ptr RCSDiscoveryManagerImpl::startDiscovery( @@ -66,120 +76,119 @@ namespace OIC { if (!cb) { - throw RCSInvalidParameterException { "Callback is empty" }; + throw RCSInvalidParameterException{ "Callback is empty" }; } - ID discoveryId = createId(); - auto discoverCb = std::bind(&RCSDiscoveryManagerImpl::onResourceFound, this, - std::placeholders::_1, discoveryId, std::move(cb)); - DiscoveryRequestInfo discoveryInfo(RCSAddressDetail::getDetail(address)->getAddress(), - relativeUri, resourceType, std::move(discoverCb)); + const ID discoveryId = createId(); + + DiscoveryRequestInfo discoveryInfo(address, relativeUri, resourceType, + std::bind(&RCSDiscoveryManagerImpl::onResourceFound, this, + std::placeholders::_1, discoveryId, std::move(cb))); discoveryInfo.discover(); { - std::lock_guard < std::mutex > lock(m_mutex); + std::lock_guard< std::mutex > lock(m_mutex); m_discoveryMap.insert(std::make_pair(discoveryId, std::move(discoveryInfo))); } - return std::unique_ptr < RCSDiscoveryManager::DiscoveryTask> ( + return std::unique_ptr< RCSDiscoveryManager::DiscoveryTask >( new RCSDiscoveryManager::DiscoveryTask(discoveryId)); } - void RCSDiscoveryManagerImpl::requestMulticastPresence() + void RCSDiscoveryManagerImpl::subscribePresenceWithMuticast() { + using namespace std::placeholders; + constexpr char MULTICAST_PRESENCE_ADDRESS[] = "coap://" OC_MULTICAST_PREFIX; + OCDoHandle presenceHandle; subscribePresence(presenceHandle, MULTICAST_PRESENCE_ADDRESS, OCConnectivityType::CT_DEFAULT, - std::move(std::bind(&RCSDiscoveryManagerImpl::onPresence, this, - std::placeholders::_1, std::placeholders::_2, - std::placeholders::_3))); + std::bind(&RCSDiscoveryManagerImpl::onPresence, this, _1, _2, _3)); } void RCSDiscoveryManagerImpl::onPolling() { - std::lock_guard < std::mutex > lock(m_mutex); - - for (const auto& it : m_discoveryMap) { - it.second.discover(); + std::lock_guard< std::mutex > lock(m_mutex); + + for (const auto& it : m_discoveryMap) + { + it.second.discover(); + } } m_timer.post(POLLING_INTERVAL_TIME, std::bind(&RCSDiscoveryManagerImpl::onPolling, this)); } - void RCSDiscoveryManagerImpl::onPresence(OCStackResult ret, const unsigned int /*seq*/, + void RCSDiscoveryManagerImpl::onPresence(OCStackResult result, const unsigned int /*seq*/, const std::string& address) { - if (ret != OC_STACK_OK && ret != OC_STACK_RESOURCE_CREATED) return; + if (result != OC_STACK_OK && result != OC_STACK_RESOURCE_CREATED) return; - std::lock_guard < std::mutex > lock(m_mutex); + std::lock_guard< std::mutex > lock(m_mutex); for (const auto& it : m_discoveryMap) { - if (it.second.isMatchingAddress(address)) + if (it.second.isMatchedAddress(address)) { it.second.discover(); } } } - RCSDiscoveryManagerImpl::ID RCSDiscoveryManagerImpl::createId() + RCSDiscoveryManagerImpl::ID RCSDiscoveryManagerImpl::createId() const { - std::lock_guard < std::mutex > lock(m_mutex); - static ID s_uniqueId; + static ID s_nextId = INVALID_ID + 1; + + std::lock_guard< std::mutex > lock(m_mutex); - s_uniqueId++; - while (m_discoveryMap.find(s_uniqueId) != m_discoveryMap.end()) + while (s_nextId == INVALID_ID || m_discoveryMap.find(s_nextId) != m_discoveryMap.end()) { - s_uniqueId++; + ++s_nextId; } - return s_uniqueId; + + assert(s_nextId != INVALID_ID && "Invalid ID!"); + + return s_nextId++; } void RCSDiscoveryManagerImpl::cancel(ID id) { - std::lock_guard < std::mutex > lock(m_mutex); + std::lock_guard< std::mutex > lock(m_mutex); m_discoveryMap.erase(id); } - bool RCSDiscoveryManagerImpl::isCanceled(ID 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, + DiscoveryRequestInfo::DiscoveryRequestInfo(const RCSAddress& address, + const std::string& relativeUri, const std::string& resourceType, DiscoverCallback cb) : - m_address(address), m_relativeUri(relativeUri), m_resourceType(resourceType), - m_discoverCb(std::move(cb)) + m_address{ address }, + m_relativeUri{ relativeUri }, + m_resourceType{ resourceType }, + m_knownResourceIds{ }, + m_discoverCb{ std::move(cb) } { } void DiscoveryRequestInfo::discover() const { - OIC::Service::discoverResource(m_address, m_relativeUri + "?rt=" + m_resourceType, - OCConnectivityType::CT_DEFAULT, m_discoverCb); + discoverResource(m_address, m_relativeUri + "?rt=" + m_resourceType, m_discoverCb); } bool DiscoveryRequestInfo::isKnownResource( - const std::shared_ptr& resource) + const std::shared_ptr< PrimitiveResource >& resource) const { - std::string resourceId = resource->getSid() + resource->getUri(); - - auto it = std::find(m_receivedIds.begin(), m_receivedIds.end(), resourceId); + return m_knownResourceIds.find(makeResourceId(resource)) != m_knownResourceIds.end(); + } - if (it != m_receivedIds.end()) return true; - m_receivedIds.insert(resourceId); - return false; + void DiscoveryRequestInfo::addKnownResource( + const std::shared_ptr< PrimitiveResource >& resource) + { + m_knownResourceIds.insert(makeResourceId(resource)); } - bool DiscoveryRequestInfo::isMatchingAddress(const std::string& address) const + bool DiscoveryRequestInfo::isMatchedAddress(const std::string& address) const { - return m_address == RCSAddressDetail::getDetail(RCSAddress::multicast())->getAddress() - || m_address == address; + return RCSAddressDetail::getDetail(m_address)->isMulticast() || + RCSAddressDetail::getDetail(m_address)->getAddress() == address; } } } diff --git a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h index 904a654..0637aeb 100644 --- a/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h +++ b/service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h @@ -35,19 +35,15 @@ #include #include +#include "RCSAddress.h" #include "RCSDiscoveryManager.h" #include "ExpiryTimer.h" #include "PrimitiveResource.h" -#include "RCSRemoteResourceObject.h" namespace OIC { namespace Service { - class RCSDiscoveryManager; - class PrimitiveResource; - class RCSAddress; - /** * The class contains discovery request information * @@ -56,19 +52,20 @@ namespace OIC class DiscoveryRequestInfo { public: - DiscoveryRequestInfo(const std::string &, const std::string &, - const std::string &, DiscoverCallback); + DiscoveryRequestInfo(const RCSAddress&, const std::string&, const std::string&, + DiscoverCallback); public: void discover() const; - bool isKnownResource(const std::shared_ptr&); - bool isMatchingAddress(const std::string&) const; + bool isKnownResource(const std::shared_ptr< PrimitiveResource >&) const; + void addKnownResource(const std::shared_ptr< PrimitiveResource >&); + bool isMatchedAddress(const std::string&) const; private: - std::string m_address; + RCSAddress m_address; std::string m_relativeUri; std::string m_resourceType; - std::unordered_set m_receivedIds; + std::unordered_set< std::string > m_knownResourceIds; DiscoverCallback m_discoverCb; }; @@ -114,16 +111,12 @@ namespace OIC RCSDiscoveryManager::ResourceDiscoveredCallback cb); void cancel(ID); - bool isCanceled(ID); private: RCSDiscoveryManagerImpl(); ~RCSDiscoveryManagerImpl() = default; - /** - * Request presence by multicast - */ - void requestMulticastPresence(); + void subscribePresenceWithMuticast(); /** * Check duplicated callback and invoke callback when resource is discovered @@ -146,21 +139,24 @@ namespace OIC * Discover resource on all requests when supporting presence function resource * enter into network */ - void onPresence(OCStackResult ret, const unsigned int seq, const std::string& address); + void onPresence(OCStackResult, const unsigned int seq, const std::string& address); /** * Create unique id * * @return Returns the id */ - ID createId(); + ID createId() const; public: - ExpiryTimer m_timer; + constexpr static ID INVALID_ID = 0; private: - std::unordered_map m_discoveryMap; - std::mutex m_mutex; + ExpiryTimer m_timer; + + std::unordered_map< ID, DiscoveryRequestInfo > m_discoveryMap; + + mutable std::mutex m_mutex; }; } } -- 2.7.4