From 3245209c1723ca0b9ef81b10f8faae133a731ffb Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Fri, 15 Dec 2023 13:59:31 -0800 Subject: [PATCH] Improve ICDClientStorage (#30931) --- .../icd/client/DefaultICDClientStorage.cpp | 17 --------- src/app/icd/client/DefaultICDClientStorage.h | 22 +++++++++--- src/app/icd/client/ICDClientStorage.h | 35 +++---------------- src/app/tests/TestDefaultICDClientStorage.cpp | 4 --- 4 files changed, 22 insertions(+), 56 deletions(-) diff --git a/src/app/icd/client/DefaultICDClientStorage.cpp b/src/app/icd/client/DefaultICDClientStorage.cpp index 79713fc6be2c15..c2c6cdfe95142e 100644 --- a/src/app/icd/client/DefaultICDClientStorage.cpp +++ b/src/app/icd/client/DefaultICDClientStorage.cpp @@ -410,23 +410,6 @@ CHIP_ERROR DefaultICDClientStorage::UpdateEntryCountForFabric(FabricIndex fabric backingBuffer.Get(), static_cast(len)); } -CHIP_ERROR DefaultICDClientStorage::GetEntry(const ScopedNodeId & peerNode, ICDClientInfo & clientInfo) -{ - size_t clientInfoSize = 0; - std::vector 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; diff --git a/src/app/icd/client/DefaultICDClientStorage.h b/src/app/icd/client/DefaultICDClientStorage.h index c98d058fbba3e8..0d398469a30ff4 100644 --- a/src/app/icd/client/DefaultICDClientStorage.h +++ b/src/app/icd/client/DefaultICDClientStorage.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -50,11 +51,18 @@ namespace app { class DefaultICDClientStorage : public ICDClientStorage { public: + using ICDClientInfoIterator = CommonIterator; + 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; diff --git a/src/app/icd/client/ICDClientStorage.h b/src/app/icd/client/ICDClientStorage.h index 369926c3394853..d4727ffac6030e 100644 --- a/src/app/icd/client/ICDClientStorage.h +++ b/src/app/icd/client/ICDClientStorage.h @@ -24,7 +24,6 @@ #include #include #include -#include #include namespace chip { @@ -37,17 +36,8 @@ namespace app { class ICDClientStorage { public: - using ICDClientInfoIterator = CommonIterator; - 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,20 +56,13 @@ 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. @@ -87,16 +70,6 @@ class ICDClientStorage */ 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, diff --git a/src/app/tests/TestDefaultICDClientStorage.cpp b/src/app/tests/TestDefaultICDClientStorage.cpp index 730747232455c6..9bb11ccd734476 100644 --- a/src/app/tests/TestDefaultICDClientStorage.cpp +++ b/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