Persisting Group Data Provider implemented. (#12047)

* Group Data Provider implemented using the KeyValueStoreMgr utility.

* Group Data Provider implemented using PersistentStorageDelegate.

* Group Data Provider: Code review changes.

* Groups cluster tests enabled using TestPersistentStorageDelegate.

src/app/clusters/groups-server/groups-server.cpp

@@ -85,28 +85,6 @@ static bool GroupExists(FabricIndex fabricIndex, EndpointId endpointId, GroupId
     return provider->GroupMappingExists(fabricIndex, mapping);
 }
 
-static bool GroupFind(FabricIndex fabricIndex, EndpointId endpointId, GroupId groupId, CharSpan & name)
-{
-    GroupDataProvider * provider = GetGroupDataProvider();
-    VerifyOrReturnError(nullptr != provider, false);
-
-    auto * groupIt = provider->IterateGroupMappings(fabricIndex, endpointId);
-    VerifyOrReturnError(nullptr != groupIt, false);
-
-    GroupDataProvider::GroupMapping mapping;
-    bool found = false;
-    while (!found && groupIt->Next(mapping))
-    {
-        if (mapping.group == groupId)
-        {
-            name  = mapping.name;
-            found = true;
-        }
-    }
-    groupIt->Release();
-    return found;
-}
-
 static EmberAfStatus GroupAdd(FabricIndex fabricIndex, EndpointId endpointId, GroupId groupId, const CharSpan & groupName)
 {
     VerifyOrReturnError(IsFabricGroupId(groupId), EMBER_ZCL_STATUS_INVALID_VALUE);
@@ -217,22 +195,38 @@ bool emberAfGroupsClusterViewGroupCallback(app::CommandHandler * commandObj, con
     auto groupId     = commandData.groupId;
     auto endpointId  = commandPath.mEndpointId;
 
+    GroupDataProvider::GroupMapping mapping;
     EmberAfStatus status = EMBER_ZCL_STATUS_NOT_FOUND;
     CHIP_ERROR err       = CHIP_NO_ERROR;
-    CharSpan groupName;
+    size_t nameSize      = 0;
 
     if (emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST && emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST_REPLY)
     {
         return true;
     }
 
-    if (!IsFabricGroupId(groupId))
+    if (IsFabricGroupId(groupId))
     {
-        status = EMBER_ZCL_STATUS_INVALID_VALUE;
+        GroupDataProvider * provider = GetGroupDataProvider();
+        VerifyOrReturnError(nullptr != provider, false);
+
+        auto * groupIt = provider->IterateGroupMappings(fabricIndex, endpointId);
+        VerifyOrReturnError(nullptr != groupIt, false);
+
+        while (groupIt->Next(mapping))
+        {
+            if (mapping.group == groupId)
+            {
+                nameSize = strnlen(mapping.name, GroupDataProvider::GroupMapping::kGroupNameMax);
+                status   = EMBER_ZCL_STATUS_SUCCESS;
+                break;
+            }
+        }
+        groupIt->Release();
     }
-    else if (GroupFind(fabricIndex, endpointId, groupId, groupName))
+    else
     {
-        status = EMBER_ZCL_STATUS_SUCCESS;
+        status = EMBER_ZCL_STATUS_INVALID_VALUE;
     }
 
     {
@@ -243,7 +237,7 @@ bool emberAfGroupsClusterViewGroupCallback(app::CommandHandler * commandObj, con
         VerifyOrExit((writer = commandObj->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
         SuccessOrExit(err = writer->Put(TLV::ContextTag(0), status));
         SuccessOrExit(err = writer->Put(TLV::ContextTag(1), groupId));
-        SuccessOrExit(err = writer->PutString(TLV::ContextTag(2), groupName.data(), static_cast<uint32_t>(groupName.size())));
+        SuccessOrExit(err = writer->PutString(TLV::ContextTag(2), mapping.name, static_cast<uint32_t>(nameSize)));
         SuccessOrExit(err = commandObj->FinishCommand());
     }
 exit:

src/app/server/Server.cpp

@@ -93,6 +93,11 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
     err = mFabrics.Init(&mServerStorage);
     SuccessOrExit(err);
 
+    // Group data provider must be initialized after mServerStorage
+    err = mGroupsProvider.Init();
+    SuccessOrExit(err);
+    SetGroupDataProvider(&mGroupsProvider);
+
     // Init transport before operations with secure session mgr.
     err = mTransports.Init(
         UdpListenParameters(&DeviceLayer::InetLayer()).SetAddressType(IPAddressType::kIPv6).SetListenPort(mSecuredServicePort)

src/app/server/Server.h

@@ -21,7 +21,9 @@
 #include <app/server/AppDelegate.h>
 #include <app/server/CommissioningWindowManager.h>
 #include <credentials/FabricTable.h>
+#include <credentials/GroupDataProviderImpl.h>
 #include <inet/InetConfig.h>
+#include <lib/support/TestPersistentStorageDelegate.h>
 #include <messaging/ExchangeMgr.h>
 #include <platform/KeyValueStoreManager.h>
 #include <protocols/secure_channel/CASEServer.h>
@@ -90,7 +92,7 @@ class Server
     static Server & GetInstance() { return sServer; }
 
 private:
-    Server() : mCommissioningWindowManager(this) {}
+    Server() : mCommissioningWindowManager(this), mGroupsProvider(mGroupsStorage) {}
 
     static Server sServer;
 
@@ -145,9 +147,15 @@ class Server
     chip::Protocols::UserDirectedCommissioning::UserDirectedCommissioningClient gUDCClient;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
     SecurePairingUsingTestSecret mTestPairing;
+    CommissioningWindowManager mCommissioningWindowManager;
 
+    // Both PersistentStorageDelegate, and GroupDataProvider should be injected by the applications
+    // See: https://github.com/project-chip/connectedhomeip/issues/12276
     ServerStorageDelegate mServerStorage;
-    CommissioningWindowManager mCommissioningWindowManager;
+    // Currently, the GroupDataProvider cannot use KeyValueStoreMgr() due to
+    // (https://github.com/project-chip/connectedhomeip/issues/12174)
+    TestPersistentStorageDelegate mGroupsStorage;
+    Credentials::GroupDataProviderImpl mGroupsProvider;
 
     chip::OperationalDeviceProxy * mOperationalDeviceProxy = nullptr;
 

src/app/tests/suites/TestGroupsCluster.yaml

@@ -92,9 +92,20 @@ tests:
               - name: "groupId"
                 value: 0x1111
 
+    - label: "Get Group Membership 1 (all)"
+      command: "GetGroupMembership"
+      arguments:
+          values:
+              - name: "groupList"
+                value: []
+      response:
+          values:
+              - name: "capacity"
+                value: 0xff
+              - name: "groupList"
+                value: [0x01]
+
     - label: "Add Group 2 (new)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "AddGroup"
       arguments:
           values:
@@ -110,8 +121,6 @@ tests:
                 value: 0x1111
 
     - label: "View Group 2 (new)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "ViewGroup"
       arguments:
           values:
@@ -140,8 +149,6 @@ tests:
                 value: 0x7fff
 
     - label: "Add Group 3 (new)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "AddGroup"
       arguments:
           values:
@@ -172,8 +179,6 @@ tests:
                 value: "Group #1"
 
     - label: "View Group 2 (existing)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "ViewGroup"
       arguments:
           values:
@@ -188,39 +193,20 @@ tests:
               - name: "groupName"
                 value: "Group #2"
 
-    - label: "Get Group Membership 1"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
-      command: "GetGroupMembership"
-      arguments:
-          values:
-              - name: "groupList"
-                value: [0x01, 0x02, 0x03, 0x7fff]
-      response:
-          values:
-              - name: "capacity"
-                value: 0xff
-              - name: "groupList"
-                value: [0x01, 0x7fff]
-
     - label: "Get Group Membership 2"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "GetGroupMembership"
       arguments:
           values:
               - name: "groupList"
-                value: []
+                value: [0x02, 0x03, 0x7fff]
       response:
           values:
               - name: "capacity"
                 value: 0xff
               - name: "groupList"
-                value: [0x01, 0x1111, 0x7fff]
+                value: [0x7fff]
 
     - label: "View Group 3 (new)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "ViewGroup"
       arguments:
           values:
@@ -262,8 +248,6 @@ tests:
                 value: 0x04
 
     - label: "Remove Group 2 (existing)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "RemoveGroup"
       arguments:
           values:
@@ -305,8 +289,6 @@ tests:
                 value: 0x1111
 
     - label: "View Group 3 (not removed)"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "ViewGroup"
       arguments:
           values:
@@ -322,19 +304,17 @@ tests:
                 value: "Group #3"
 
     - label: "Get Group Membership 3"
-      # https://github.com/project-chip/connectedhomeip/issues/11312
-      disabled: true
       command: "GetGroupMembership"
       arguments:
           values:
               - name: "groupList"
-                value: [0x01, 0x02, 0x1111, 0x03, 0x7fff]
+                value: [0x01, 0x02, 0x1111, 0x03]
       response:
           values:
               - name: "capacity"
                 value: 0xff
               - name: "groupList"
-                value: [0x01, 0x7fff]
+                value: [0x01]
 
     - label: "Remove All"
       command: "RemoveAllGroups"
@@ -379,8 +359,6 @@ tests:
                 value: 0x7fff
 
     - label: "Get Group Membership 4"
-      # https://github.com/project-chip/connectedhomeip/issues/11495
-      disabled: true
       command: "GetGroupMembership"
       arguments:
           values:

src/credentials/BUILD.gn

@@ -37,11 +37,11 @@ static_library("credentials") {
     "FabricTable.h",
     "GenerateChipX509Cert.cpp",
     "GroupDataProvider.h",
+    "GroupDataProviderImpl.cpp",
     "examples/DefaultDeviceAttestationVerifier.cpp",
     "examples/DefaultDeviceAttestationVerifier.h",
     "examples/DeviceAttestationCredsExample.cpp",
     "examples/DeviceAttestationCredsExample.h",
-    "examples/GroupDataProviderExample.cpp",
   ]
 
   if (chip_with_se05x == 1) {