From cb7ac0c5adb04bba461a45e4109c0526bd8dd72d Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 1 Sep 2022 10:14:44 -0400
Subject: [PATCH] Address review comment: reduce duplication in read client
 iteration methods.

---
 src/app/InteractionModelEngine.cpp | 41 ++++++++++--------------------
 src/app/InteractionModelEngine.h   |  7 +++++
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
index 9888395e562ad3..3f88c40f5f9910 100644
--- 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
index f1547341a7ade4..0bd2c6738a3818 100644
--- 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>