From 29dd861af85b6807d39eeb31d331e697a20e26d8 Mon Sep 17 00:00:00 2001
From: Yunhan Wang <yunhanw@google.com>
Date: Wed, 18 May 2022 22:01:47 -0700
Subject: [PATCH] Fix clusterStateCache dataVersion bug

--if there exists any path in the request that references a specific attribute, remove affacted wildcard cluster attribute in requestPathSet used in cached
so that we would not cache the dataVersion  for all clusters affected by that attribute.
---
 src/app/AttributePathParams.h                | 3 ++-
 src/app/ClusterStateCache.cpp                | 5 -----
 src/controller/tests/data_model/TestRead.cpp | 9 +++------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h
index a2425e590143ad..effb97da71b981 100644
--- a/src/app/AttributePathParams.h
+++ b/src/app/AttributePathParams.h
@@ -100,7 +100,8 @@ struct AttributePathParams
     {
         VerifyOrReturnError(HasWildcardEndpointId() || other.HasWildcardEndpointId() || mEndpointId == other.mEndpointId, false);
         VerifyOrReturnError(HasWildcardClusterId() || other.HasWildcardClusterId() || mClusterId == other.mClusterId, false);
-        VerifyOrReturnError(HasWildcardAttributeId() || other.HasWildcardAttributeId() || mAttributeId == other.mAttributeId, false);
+        VerifyOrReturnError(HasWildcardAttributeId() || other.HasWildcardAttributeId() || mAttributeId == other.mAttributeId,
+                            false);
         return true;
     }
 
diff --git a/src/app/ClusterStateCache.cpp b/src/app/ClusterStateCache.cpp
index d6cbdaa4968ded..17fe89d603a273 100644
--- a/src/app/ClusterStateCache.cpp
+++ b/src/app/ClusterStateCache.cpp
@@ -470,20 +470,15 @@ CHIP_ERROR ClusterStateCache::OnUpdateDataVersionFilterList(DataVersionFilterIBs
         {
             if (attribute1.IsAttributePathOverlapped(*attribute2))
             {
-                ChipLogError(InteractionModel, "overlapped");
                 attribute2 = mRequestPathSet.erase(attribute2);
             }
             else
             {
-                ChipLogError(InteractionModel, "!overlapped");
                 ++attribute2;
             }
         }
     }
 
-    ChipLogError(InteractionModel, "debug how many %lu", mRequestPathSet.size());
-
-
     std::vector<std::pair<DataVersionFilter, size_t>> filterVector;
     GetSortedFilters(filterVector);
 
diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp
index 18d35f09a39c43..6a370b6c3f5a4f 100644
--- a/src/controller/tests/data_model/TestRead.cpp
+++ b/src/controller/tests/data_model/TestRead.cpp
@@ -384,7 +384,7 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit
 
     int testId = 0;
 
-    // Read of E2C3A1, E*C3A1(1 supported attribute in E1C3A1), E2C3A* (3 supported attributes)
+    // Read of E2C3A1(dedup), E*C3A1(E1C3A1 not exit, E2C3A1 exist), E2C3A* (5 supported attributes)
     // Expect no versions would be cached.
     {
         testId++;
@@ -410,14 +410,11 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit
         NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
         ctx.DrainAndServiceIO();
-        NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 4);
+        NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 6);
         NL_TEST_ASSERT(apSuite, !delegate.mReadError);
         Optional<DataVersion> version1;
-        NL_TEST_ASSERT(apSuite, cache.GetVersion(Test::kMockEndpoint1, Test::MockClusterId(3), version1) == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(apSuite, cache.GetVersion(Test::kMockEndpoint2, Test::MockClusterId(3), version1) == CHIP_NO_ERROR);
         NL_TEST_ASSERT(apSuite, !version1.HasValue());
-        Optional<DataVersion> version2;
-        NL_TEST_ASSERT(apSuite, cache.GetVersion(Test::kMockEndpoint2, Test::MockClusterId(3), version2) == CHIP_NO_ERROR);
-        NL_TEST_ASSERT(apSuite, !version2.HasValue());
         delegate.mNumAttributeResponse = 0;
     }