From 6322b8dff3290e20dfe8cc02ca272b135723fda3 Mon Sep 17 00:00:00 2001
From: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>
Date: Mon, 15 Aug 2022 19:36:01 -0400
Subject: [PATCH] Fix a dependency cycle in DefaultStorageKeyAllocator.h
 (#21794)

* Fix a dependency cycle in DefaultStorageKeyAllocator.h

When using SDK with a build system that disallows build graph
dependencies and enforces explicit dependencies being always included,
the DefaultStorageKeyAllocator's dependency on IM-layer headers
and absence from BUILD.gn mean that a cycle that could have been
detected by GN is not detected in Matter SDK, but detected by
the build.

This PR:
- Adds DefaultStorageKeyAllocator.h to the correct dependency graph
- Removes a cycle from `app/ConcreteAttributePath.h`
- Fixes-up DefaultStorageKeyAllocator to work after the cycle is broken

Testing done:
- Cert tests pass
- Unit tests pass

* Restyle
---
 src/app/DefaultAttributePersistenceProvider.cpp |  6 ++++--
 src/lib/support/BUILD.gn                        |  1 +
 src/lib/support/DefaultStorageKeyAllocator.h    | 10 ++++++----
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/app/DefaultAttributePersistenceProvider.cpp b/src/app/DefaultAttributePersistenceProvider.cpp
index 9b7c3a4538912b..502b28e914d638 100644
--- a/src/app/DefaultAttributePersistenceProvider.cpp
+++ b/src/app/DefaultAttributePersistenceProvider.cpp
@@ -35,7 +35,8 @@ CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttribu
     {
         return CHIP_ERROR_BUFFER_TOO_SMALL;
     }
-    return mStorage->SyncSetKeyValue(key.AttributeValue(aPath), aValue.data(), static_cast<uint16_t>(aValue.size()));
+    return mStorage->SyncSetKeyValue(key.AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue.data(),
+                                     static_cast<uint16_t>(aValue.size()));
 }
 
 CHIP_ERROR DefaultAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & aPath,
@@ -45,7 +46,8 @@ CHIP_ERROR DefaultAttributePersistenceProvider::ReadValue(const ConcreteAttribut
 
     DefaultStorageKeyAllocator key;
     uint16_t size = static_cast<uint16_t>(min(aValue.size(), static_cast<size_t>(UINT16_MAX)));
-    ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.AttributeValue(aPath), aValue.data(), size));
+    ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId),
+                                                   aValue.data(), size));
     EmberAfAttributeType type = aMetadata->attributeType;
     if (emberAfIsStringAttributeType(type))
     {
diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
index d5e18e69f702f0..38903219b75ab1 100644
--- a/src/lib/support/BUILD.gn
+++ b/src/lib/support/BUILD.gn
@@ -100,6 +100,7 @@ static_library("support") {
     "CHIPPlatformMemory.h",
     "CodeUtils.h",
     "DLLUtil.h",
+    "DefaultStorageKeyAllocator.h",
     "Defer.h",
     "ErrorStr.cpp",
     "ErrorStr.h",
diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h
index 94b315b7e37c58..920e834bfdcd36 100644
--- a/src/lib/support/DefaultStorageKeyAllocator.h
+++ b/src/lib/support/DefaultStorageKeyAllocator.h
@@ -16,12 +16,14 @@
  */
 #pragma once
 
-#include <app/ConcreteAttributePath.h>
-#include <app/util/basic-types.h>
 #include <lib/core/CHIPPersistentStorageDelegate.h>
+#include <lib/core/DataModelTypes.h>
 #include <lib/core/GroupId.h>
 #include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 namespace chip {
@@ -93,11 +95,11 @@ class DefaultStorageKeyAllocator
     }
     const char * FabricKeyset(chip::FabricIndex fabric, uint16_t keyset) { return Format("f/%x/k/%x", fabric, keyset); }
 
-    const char * AttributeValue(const app::ConcreteAttributePath & aPath)
+    const char * AttributeValue(EndpointId endpointId, ClusterId clusterId, AttributeId attributeId)
     {
         // Needs at most 26 chars: 6 for "g/a///", 4 for the endpoint id, 8 each
         // for the cluster and attribute ids.
-        return Format("g/a/%x/%" PRIx32 "/%" PRIx32, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId);
+        return Format("g/a/%x/%" PRIx32 "/%" PRIx32, endpointId, clusterId, attributeId);
     }
 
     // TODO: Should store fabric-specific parts of the binding list under keys