Make reporting::Engine able to use DataModel::Provider for cluster version checks (project-chip#35863)

andy31415 · andreilitvin · restyled-commits · web-flow · commit c565622b721b · 2024-10-02T20:51:03.000Z
* Update repoting Engine to be able to use DataModel::Provider for version checks.

* Restyle

* Fix an include

* Fix an include

* Fix an include

* Restyled by clang-format

* Update src/app/reporting/Read-Ember.cpp

Co-authored-by: Boris Zbarsky &lt;bzbarsky@apple.com&gt;

---------

Co-authored-by: Andrei Litvin &lt;andreilitvin@google.com&gt;
Co-authored-by: Restyled.io &lt;commits@restyled.io&gt;
Co-authored-by: Boris Zbarsky &lt;bzbarsky@apple.com&gt;
diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp
@@ -23,8 +23,9 @@
  *
  */
 
-#include "app/data-model-provider/ActionReturnStatus.h"
+#include <app/data-model-provider/ActionReturnStatus.h>
 #include <app/icd/server/ICDServerConfig.h>
+#include <app/reporting/Read-Checked.h>
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
 #include <app/icd/server/ICDNotifier.h> // nogncheck
 #endif
@@ -72,8 +73,10 @@ bool Engine::IsClusterDataVersionMatch(const SingleLinkedListNode<DataVersionFil
         if (aPath.mEndpointId == filter->mValue.mEndpointId && aPath.mClusterId == filter->mValue.mClusterId)
         {
             existPathMatch = true;
-            if (!IsClusterDataVersionEqual(ConcreteClusterPath(filter->mValue.mEndpointId, filter->mValue.mClusterId),
-                                           filter->mValue.mDataVersion.Value()))
+
+            if (!Impl::IsClusterDataVersionEqualTo(mpImEngine->GetDataModelProvider(),
+                                                   ConcreteClusterPath(filter->mValue.mEndpointId, filter->mValue.mClusterId),
+                                                   filter->mValue.mDataVersion.Value()))
             {
                 existVersionMismatch = true;
             }
diff --git a/src/app/reporting/Read-Checked.cpp b/src/app/reporting/Read-Checked.cpp
@@ -14,13 +14,14 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
-#include "app/data-model-provider/ActionReturnStatus.h"
-#include "lib/support/StringBuilder.h"
 #include <app/reporting/Read-Checked.h>
 
+#include <app/data-model-provider/ActionReturnStatus.h>
 #include <app/reporting/Read-DataModel.h>
 #include <app/reporting/Read-Ember.h>
 #include <app/util/MatterCallbacks.h>
+#include <lib/support/CodeUtils.h>
+#include <lib/support/StringBuilder.h>
 
 namespace chip {
 namespace app {
@@ -160,6 +161,23 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
     return statusDm;
 }
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion)
+{
+    bool emberResult = EmberImpl::IsClusterDataVersionEqualTo(dataModel, path, dataVersion);
+    bool dmResult    = DataModelImpl::IsClusterDataVersionEqualTo(dataModel, path, dataVersion);
+
+    if (emberResult != dmResult)
+    {
+        ChipLogError(Test, "Different data model check result between ember (%s) and data model provider(%s)",
+                     emberResult ? "TRUE" : "FALSE", dmResult ? "TRUE" : "FALSE");
+#if CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE
+        chipDie();
+#endif
+    }
+
+    return dmResult;
+}
+
 } // namespace CheckedImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/reporting/Read-Checked.h b/src/app/reporting/Read-Checked.h
@@ -33,6 +33,8 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
                                                   AttributeReportIBs::Builder & reportBuilder,
                                                   const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState);
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion);
+
 } // namespace CheckedImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/reporting/Read-DataModel.cpp b/src/app/reporting/Read-DataModel.cpp
@@ -14,13 +14,14 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
-#include "app/data-model-provider/ActionReturnStatus.h"
-#include "lib/support/logging/TextOnlyLogging.h"
 #include <app/reporting/Read-DataModel.h>
 
 #include <app/AppConfig.h>
+#include <app/data-model-provider/ActionReturnStatus.h>
+#include <app/data-model-provider/MetadataTypes.h>
 #include <app/data-model-provider/Provider.h>
 #include <app/util/MatterCallbacks.h>
+#include <optional>
 
 namespace chip {
 namespace app {
@@ -101,6 +102,17 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
     return status;
 }
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion)
+{
+    std::optional<DataModel::ClusterInfo> info = dataModel->GetClusterInfo(path);
+    if (!info.has_value())
+    {
+        return false;
+    }
+
+    return (info->dataVersion == dataVersion);
+}
+
 } // namespace DataModelImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/reporting/Read-DataModel.h b/src/app/reporting/Read-DataModel.h
@@ -34,6 +34,8 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
                                                   AttributeReportIBs::Builder & reportBuilder,
                                                   const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState);
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion);
+
 } // namespace DataModelImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/reporting/Read-Ember.cpp b/src/app/reporting/Read-Ember.cpp
@@ -14,10 +14,10 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
-#include "app/data-model-provider/ActionReturnStatus.h"
 #include <app/reporting/Read-Ember.h>
 
 #include <app/AppConfig.h>
+#include <app/data-model-provider/ActionReturnStatus.h>
 #include <app/util/MatterCallbacks.h>
 #include <app/util/ember-compatibility-functions.h>
 
@@ -51,6 +51,11 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
     return CHIP_NO_ERROR;
 }
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion)
+{
+    return IsClusterDataVersionEqual(path, dataVersion);
+}
+
 } // namespace EmberImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/reporting/Read-Ember.h b/src/app/reporting/Read-Ember.h
@@ -18,10 +18,12 @@
 
 #include <access/SubjectDescriptor.h>
 #include <app/AttributeEncodeState.h>
+#include <app/ConcreteClusterPath.h>
 #include <app/MessageDef/AttributeReportIBs.h>
 #include <app/data-model-provider/ActionReturnStatus.h>
 #include <app/data-model-provider/Provider.h>
 #include <lib/core/CHIPError.h>
+#include <lib/core/DataModelTypes.h>
 
 namespace chip {
 namespace app {
@@ -33,6 +35,8 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
                                                   AttributeReportIBs::Builder & reportBuilder,
                                                   const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState);
 
+bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const ConcreteClusterPath & path, DataVersion dataVersion);
+
 } // namespace EmberImpl
 } // namespace reporting
 } // namespace app
diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
@@ -330,6 +330,7 @@ class TestReadInteraction : public chip::Test::AppContext
                                                           gCircularEventBuffer, logStorageResources, &mEventCounter);
         mOldProvider = InteractionModelEngine::GetInstance()->SetDataModelProvider(&TestImCustomDataModel::Instance());
         chip::Test::SetMockNodeConfig(TestMockNodeConfig());
+        chip::Test::SetVersionTo(chip::Test::kTestDataVersion1);
     }
     void TearDown() override
     {
diff --git a/src/app/util/mock/Functions.h b/src/app/util/mock/Functions.h
@@ -43,6 +43,9 @@ void BumpVersion();
 /// Sets GetVersion to return 0
 void ResetVersion();
 
+/// Force the global cluster version to a specific value
+void SetVersionTo(DataVersion version);
+
 /// Gets the current value for the version that will
 /// be returned by emberAfDataVersionStorage
 DataVersion GetVersion();
diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp
@@ -376,6 +376,11 @@ DataVersion GetVersion()
     return dataVersion;
 }
 
+void SetVersionTo(DataVersion version)
+{
+    dataVersion = version;
+}
+
 CHIP_ERROR ReadSingleMockClusterData(FabricIndex aAccessingFabricIndex, const ConcreteAttributePath & aPath,
                                      AttributeReportIBs::Builder & aAttributeReports, AttributeEncodeState * apEncoderState)
 {

Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,9 @@`
`23`	`23`	`*`
`24`	`24`	`*/`
`25`	`25`
`26`		`-#include "app/data-model-provider/ActionReturnStatus.h"`
	`26`	`+#include <app/data-model-provider/ActionReturnStatus.h>`
`27`	`27`	`#include <app/icd/server/ICDServerConfig.h>`
	`28`	`+#include <app/reporting/Read-Checked.h>`
`28`	`29`	`#if CHIP_CONFIG_ENABLE_ICD_SERVER`
`29`	`30`	`#include <app/icd/server/ICDNotifier.h> // nogncheck`
`30`	`31`	`#endif`
`@@ -72,8 +73,10 @@ bool Engine::IsClusterDataVersionMatch(const SingleLinkedListNode<DataVersionFil`
`72`	`73`	`if (aPath.mEndpointId == filter->mValue.mEndpointId && aPath.mClusterId == filter->mValue.mClusterId)`
`73`	`74`	`{`
`74`	`75`	`existPathMatch = true;`
`75`		`- if (!IsClusterDataVersionEqual(ConcreteClusterPath(filter->mValue.mEndpointId, filter->mValue.mClusterId),`
`76`		`- filter->mValue.mDataVersion.Value()))`
	`76`	`+`
	`77`	`+ if (!Impl::IsClusterDataVersionEqualTo(mpImEngine->GetDataModelProvider(),`
	`78`	`+ ConcreteClusterPath(filter->mValue.mEndpointId, filter->mValue.mClusterId),`
	`79`	`+ filter->mValue.mDataVersion.Value()))`
`77`	`80`	`{`
`78`	`81`	`existVersionMismatch = true;`
`79`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -330,6 +330,7 @@ class TestReadInteraction : public chip::Test::AppContext`
`330`	`330`	`gCircularEventBuffer, logStorageResources, &mEventCounter);`
`331`	`331`	`mOldProvider = InteractionModelEngine::GetInstance()->SetDataModelProvider(&TestImCustomDataModel::Instance());`
`332`	`332`	`chip::Test::SetMockNodeConfig(TestMockNodeConfig());`
	`333`	`+ chip::Test::SetVersionTo(chip::Test::kTestDataVersion1);`
`333`	`334`	`}`
`334`	`335`	`void TearDown() override`
`335`	`336`	`{`
Original file line number	Diff line number	Diff line change
`@@ -376,6 +376,11 @@ DataVersion GetVersion()`
`376`	`376`	`return dataVersion;`
`377`	`377`	`}`
`378`	`378`
	`379`	`+void SetVersionTo(DataVersion version)`
	`380`	`+{`
	`381`	`+ dataVersion = version;`
	`382`	`+}`
	`383`	`+`
`379`	`384`	`CHIP_ERROR ReadSingleMockClusterData(FabricIndex aAccessingFabricIndex, const ConcreteAttributePath & aPath,`
`380`	`385`	`AttributeReportIBs::Builder & aAttributeReports, AttributeEncodeState * apEncoderState)`
`381`	`386`	`{`