Address review comment: reduce duplication in read client iteration m…

…ethods.
project-chip · Sep 1, 2022 · cb7ac0c · cb7ac0c
1 parent e39968d
commit cb7ac0c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 27 deletions.
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -258,38 +258,20 @@ CHIP_ERROR InteractionModelEngine::ShutdownSubscription(const ScopedNodeId & aPe
 
 void InteractionModelEngine::ShutdownSubscriptions(FabricIndex aFabricIndex, NodeId aPeerNodeId)
 {
-    // This is assuming that ReadClient::Close will not affect any other
-    // ReadClients in the list.
-    for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
-    {
-        // Grab the next client now, because we might be about to delete readClient.
-        auto * nextClient = readClient->GetNextClient();
-        if (readClient->IsSubscriptionType() && readClient->GetFabricIndex() == aFabricIndex &&
-            readClient->GetPeerNodeId() == aPeerNodeId)
-        {
-            readClient->Close(CHIP_NO_ERROR);
-        }
-        readClient = nextClient;
-    }
+    ShutdownMatchingSubscriptions(MakeOptional(aFabricIndex), MakeOptional(aPeerNodeId));
 }
-
 void InteractionModelEngine::ShutdownSubscriptions(FabricIndex aFabricIndex)
 {
-    // This is assuming that ReadClient::Close will not affect any other
-    // ReadClients in the list.
-    for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
-    {
-        // Grab the next client now, because we might be about to delete readClient.
-        auto * nextClient = readClient->GetNextClient();
-        if (readClient->IsSubscriptionType() && readClient->GetFabricIndex() == aFabricIndex)
-        {
-            readClient->Close(CHIP_NO_ERROR);
-        }
-        readClient = nextClient;
-    }
+    ShutdownMatchingSubscriptions(MakeOptional(aFabricIndex));
 }
 
 void InteractionModelEngine::ShutdownAllSubscriptions()
+{
+    ShutdownMatchingSubscriptions();
+}
+
+void InteractionModelEngine::ShutdownMatchingSubscriptions(const Optional<FabricIndex> & aFabricIndex,
+                                                           const Optional<NodeId> & aPeerNodeId)
 {
     // This is assuming that ReadClient::Close will not affect any other
     // ReadClients in the list.
@@ -299,7 +281,12 @@ void InteractionModelEngine::ShutdownAllSubscriptions()
         auto * nextClient = readClient->GetNextClient();
         if (readClient->IsSubscriptionType())
         {
-            readClient->Close(CHIP_NO_ERROR);
+            bool fabricMatches = !aFabricIndex.HasValue() || (aFabricIndex.Value() == readClient->GetFabricIndex());
+            bool nodeIdMatches = !aPeerNodeId.HasValue() || (aPeerNodeId.Value() == readClient->GetPeerNodeId());
+            if (fabricMatches && nodeIdMatches)
+            {
+                readClient->Close(CHIP_NO_ERROR);
+            }
         }
         readClient = nextClient;
     }

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -509,6 +509,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      */
     Status EnsureResourceForRead(FabricIndex aFabricIndex, size_t aRequestedAttributePathCount, size_t aRequestedEventPathCount);
 
+    /**
+     * Helper for various ShutdownSubscriptions functions.  The subscriptions
+     * that match all the provided arguments will be shut down.
+     */
+    void ShutdownMatchingSubscriptions(const Optional<FabricIndex> & aFabricIndex = NullOptional,
+                                       const Optional<NodeId> & aPeerNodeId       = NullOptional);
+
     template <typename T, size_t N>
     void ReleasePool(ObjectList<T> *& aObjectList, ObjectPool<ObjectList<T>, N> & aObjectPool);
     template <typename T, size_t N>