Refactored 'RCSDiscoveryManager'
authorcoderhyme <jhyo.kim@samsung.com>
Tue, 5 Jan 2016 06:12:27 +0000 (22:12 -0800)
committerUze Choi <uzchoi@samsung.com>
Mon, 11 Jan 2016 05:14:55 +0000 (05:14 +0000)
The class is refactored for readability and better structure.

Change-Id: Iefb4b71fb074358e833d779139fbb646c759f022
Signed-off-by: coderhyme <jhyo.kim@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/4751
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
service/resource-encapsulation/include/RCSDiscoveryManager.h
service/resource-encapsulation/src/common/primitiveResource/include/RCSAddressDetail.h
service/resource-encapsulation/src/common/primitiveResource/src/RCSAddress.cpp
service/resource-encapsulation/src/resourceClient/RCSDiscoveryManager.cpp
service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.cpp
service/resource-encapsulation/src/resourceClient/RCSDiscoveryManagerImpl.h

index 31feefc..663e787 100644 (file)
@@ -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.
index 0281ee3..4a52eaf 100644 (file)
@@ -39,6 +39,8 @@ namespace OIC
 
             const std::string& getAddress() const;
 
+            bool isMulticast() const;
+
         private:
             std::string m_addr;
         };
index 3bc4021..8518ba1 100644 (file)
@@ -67,5 +67,10 @@ namespace OIC
             return m_addr;
         }
 
+        bool RCSAddressDetail::isMulticast() const
+        {
+            return m_addr.empty();
+        }
+
     }
 }
index 328a964..eebbd83 100755 (executable)
@@ -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()
index 70114ce..6ed5e90 100755 (executable)
 // 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<PrimitiveResource> 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<PrimitiveResource>& 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;
         }
     }
 }
index 904a654..0637aeb 100644 (file)
 #include <unordered_map>
 #include <unordered_set>
 
+#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<PrimitiveResource>&);
-                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<std::string> 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<ID,DiscoveryRequestInfo> m_discoveryMap;
-                std::mutex m_mutex;
+                ExpiryTimer m_timer;
+
+                std::unordered_map< ID, DiscoveryRequestInfo > m_discoveryMap;
+
+                mutable std::mutex m_mutex;
         };
     }
 }