Improve ICDClientStorage (#30931)

src/app/icd/client/DefaultICDClientStorage.cpp

@@ -410,23 +410,6 @@ CHIP_ERROR DefaultICDClientStorage::UpdateEntryCountForFabric(FabricIndex fabric
                                               backingBuffer.Get(), static_cast<uint16_t>(len));
 }
 
-CHIP_ERROR DefaultICDClientStorage::GetEntry(const ScopedNodeId & peerNode, ICDClientInfo & clientInfo)
-{
-    size_t clientInfoSize = 0;
-    std::vector<ICDClientInfo> clientInfoVector;
-    ReturnErrorOnFailure(Load(peerNode.GetFabricIndex(), clientInfoVector, clientInfoSize));
-    IgnoreUnusedVariable(clientInfoSize);
-    for (auto & info : clientInfoVector)
-    {
-        if (peerNode.GetNodeId() == info.peer_node.GetNodeId())
-        {
-            clientInfo = info;
-            return CHIP_NO_ERROR;
-        }
-    }
-    return CHIP_ERROR_NOT_FOUND;
-}
-
 CHIP_ERROR DefaultICDClientStorage::DeleteEntry(const ScopedNodeId & peerNode)
 {
     size_t clientInfoSize = 0;

src/app/icd/client/DefaultICDClientStorage.h

@@ -31,6 +31,7 @@
 #include <lib/core/DataModelTypes.h>
 #include <lib/core/ScopedNodeId.h>
 #include <lib/core/TLV.h>
+#include <lib/support/CommonIterator.h>
 #include <lib/support/Pool.h>
 #include <vector>
 
@@ -50,11 +51,18 @@ namespace app {
 class DefaultICDClientStorage : public ICDClientStorage
 {
 public:
+    using ICDClientInfoIterator = CommonIterator<ICDClientInfo>;
+
     static constexpr size_t kIteratorsMax = CHIP_CONFIG_MAX_ICD_CLIENTS_INFO_STORAGE_CONCURRENT_ITERATORS;
 
     CHIP_ERROR Init(PersistentStorageDelegate * clientInfoStore, Crypto::SymmetricKeystore * keyStore);
 
-    ICDClientInfoIterator * IterateICDClientInfo() override;
+    /**
+     * Iterate through persisted ICD Client Info
+     *
+     * @return A valid iterator on success. Use CommonIterator accessor to retrieve ICDClientInfo
+     */
+    ICDClientInfoIterator * IterateICDClientInfo();
 
     /**
      * When decrypting check-in messages, the system needs to iterate through all keys
@@ -75,11 +83,17 @@ class DefaultICDClientStorage : public ICDClientStorage
 
     CHIP_ERROR StoreEntry(const ICDClientInfo & clientInfo) override;
 
-    CHIP_ERROR GetEntry(const ScopedNodeId & peerNode, ICDClientInfo & clientInfo) override;
-
     CHIP_ERROR DeleteEntry(const ScopedNodeId & peerNode) override;
 
-    CHIP_ERROR DeleteAllEntries(FabricIndex fabricIndex) override;
+    /**
+     * Remove all ICDClient persistent information associated with the specified
+     * fabric index.  If no entries for the fabric index exist, this is a no-op
+     * and is considered successful.
+     * When the whole fabric is removed, all entries from persistent storage in current fabric index are removed.
+     *
+     * @param[in] fabricIndex the index of the fabric for which to remove ICDClient persistent information
+     */
+    CHIP_ERROR DeleteAllEntries(FabricIndex fabricIndex);
 
     CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo) override;
 

src/app/icd/client/ICDClientStorage.h

@@ -24,7 +24,6 @@
 #include <lib/core/DataModelTypes.h>
 #include <lib/core/ScopedNodeId.h>
 #include <lib/support/CodeUtils.h>
-#include <lib/support/CommonIterator.h>
 #include <stddef.h>
 
 namespace chip {
@@ -37,17 +36,8 @@ namespace app {
 class ICDClientStorage
 {
 public:
-    using ICDClientInfoIterator = CommonIterator<ICDClientInfo>;
-
     virtual ~ICDClientStorage() = default;
 
-    /**
-     * Iterate through persisted ICD Client Info
-     *
-     * @return A valid iterator on success. Use CommonIterator accessor to retrieve ICDClientInfo
-     */
-    virtual ICDClientInfoIterator * IterateICDClientInfo() = 0;
-
     /**
      * Called during ICD device registration in commissioning, commissioner/controller
      * provides raw key data, the shared aes key handle and hmac key handle in clientInfo are updated based upon raw key data
@@ -66,37 +56,20 @@ class ICDClientStorage
     virtual CHIP_ERROR StoreEntry(const ICDClientInfo & clientInfo) = 0;
 
     /**
-     * Remove ICD key from clientInfo when ICD registration fails
-     *
-     * @param[inout] clientInfo the updated ICD Client Info.
+     * This function removes the ICD key from the provided clientInfo object in the event
+     *  of a failed LIT ICD device registration attempt. If the key handle is not found within
+     *  the Keystore, the function will not perform any operation.
+     * @param[inout] clientInfo The ICD Client Info to update with uninitialized key handle if key is removed successfully.
      */
     virtual void RemoveKey(ICDClientInfo & clientInfo) = 0;
 
-    /**
-     * Get ICD ClientInfo from storage
-     * One user case is to retrieve UserActiveModeTriggerHint and inform how user how to wake up sleepy device.
-     * @param[in] peerNode scoped node with peer node id and fabric index
-     * @param[out] clientInfo the ICD Client Info.
-     */
-    virtual CHIP_ERROR GetEntry(const ScopedNodeId & peerNode, ICDClientInfo & clientInfo) = 0;
-
     /**
      * Delete ICD Client persistent information associated with the specified scoped node Id.
      * when ICD device is unpaired/removed, the corresponding entry in ICD storage is removed.
      * @param peerNode scoped node with peer node id and fabric index
      */
     virtual CHIP_ERROR DeleteEntry(const ScopedNodeId & peerNode) = 0;
 
-    /**
-     * Remove all ICDClient persistent information associated with the specified
-     * fabric index.  If no entries for the fabric index exist, this is a no-op
-     * and is considered successful.
-     * When the whole fabric is removed, all entries from persistent storage in current fabric index are removed.
-     *
-     * @param[in] fabricIndex the index of the fabric for which to remove ICDClient persistent information
-     */
-    virtual CHIP_ERROR DeleteAllEntries(FabricIndex fabricIndex) = 0;
-
     /**
      * Process received ICD Check-in message payload.  The implementation needs to parse the payload,
      * look for a key that allows successfully decrypting the payload, verify that the counter in the payload is valid,

src/app/tests/TestDefaultICDClientStorage.cpp

@@ -57,7 +57,6 @@ void TestClientInfoCount(nlTestSuite * apSuite, void * apContext)
     FabricIndex fabricId = 1;
     NodeId nodeId1       = 6666;
     NodeId nodeId2       = 6667;
-    NodeId unknownNodeId = 6668;
     TestPersistentStorageDelegate clientInfoStorage;
     TestSessionKeystoreImpl keystore;
 
@@ -90,9 +89,6 @@ void TestClientInfoCount(nlTestSuite * apSuite, void * apContext)
         NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
         ICDClientInfo clientInfo;
-        manager.GetEntry(clientInfo3.peer_node, clientInfo);
-        NL_TEST_ASSERT(apSuite, clientInfo.peer_node.GetNodeId() == nodeId1);
-        NL_TEST_ASSERT(apSuite, CHIP_ERROR_NOT_FOUND == manager.GetEntry(ScopedNodeId(unknownNodeId, fabricId), clientInfo));
         // Make sure iterator counts correctly
         auto * iterator = manager.IterateICDClientInfo();
         // same nodeId for clientInfo2 and clientInfo3, so the new one replace old one