diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h index 02f2230b1ddcdf..d3fbc57cf03576 100644 --- a/src/credentials/GroupDataProvider.h +++ b/src/credentials/GroupDataProvider.h @@ -144,10 +144,10 @@ class GroupDataProvider /** * Interface to listen for changes in the Group info. */ - class Listener + class GroupListener { public: - virtual ~Listener() = default; + virtual ~GroupListener() = default; /** * Callback invoked when a new group is added. * @@ -232,16 +232,16 @@ class GroupDataProvider // Iterators /** * Creates an iterator that may be used to obtain the list of groups associated with the given fabric. - * The number of concurrent instances of this iterator is limited. In order to release the allocated memory, - * the iterator's Release() method must be called after the iteration is finished. + * In order to release the allocated memory, the Release() method must be called after the iteration is finished. + * Modifying the group table during the iteration is currently not supported, and may yield unexpected behaviour. * @retval An instance of EndpointIterator on success * @retval nullptr if no iterator instances are available. */ virtual GroupInfoIterator * IterateGroupInfo(chip::FabricIndex fabric_index) = 0; /** * Creates an iterator that may be used to obtain the list of (group, endpoint) pairs associated with the given fabric. - * The number of concurrent instances of this iterator is limited. In order to release the allocated memory, - * the iterator's Release() method must be called after the iteration is finished. + * In order to release the allocated memory, the Release() method must be called after the iteration is finished. + * Modifying the group table during the iteration is currently not supported, and may yield unexpected behaviour. * @retval An instance of EndpointIterator on success * @retval nullptr if no iterator instances are available. */ @@ -257,8 +257,8 @@ class GroupDataProvider /** * Creates an iterator that may be used to obtain the list of (group, keyset) pairs associated with the given fabric. - * The number of concurrent instances of this iterator is limited. In order to release the allocated memory, - * the iterator's Release() method must be called after the iteration is finished. + * In order to release the allocated memory, the Release() method must be called after the iteration is finished. + * Modifying the keyset mappings during the iteration is currently not supported, and may yield unexpected behaviour. * @retval An instance of GroupKeyIterator on success * @retval nullptr if no iterator instances are available. */ @@ -273,8 +273,8 @@ class GroupDataProvider virtual CHIP_ERROR RemoveKeySet(chip::FabricIndex fabric_index, chip::KeysetId keyset_id) = 0; /** * Creates an iterator that may be used to obtain the list of key sets associated with the given fabric. - * The number of concurrent instances of this iterator is limited. In order to release the allocated memory, - * the iterator's Release() method must be called after the iteration is finished. + * In order to release the allocated memory, the Release() method must be called after the iteration is finished. + * Modifying the key sets table during the iteration is currently not supported, and may yield unexpected behaviour. * @retval An instance of KeySetIterator on success * @retval nullptr if no iterator instances are available. */ @@ -287,7 +287,7 @@ class GroupDataProvider virtual CHIP_ERROR Decrypt(PacketHeader packetHeader, PayloadHeader & payloadHeader, System::PacketBufferHandle & msg) = 0; // Listener - void SetListener(Listener * listener) { mListener = listener; }; + void SetListener(GroupListener * listener) { mListener = listener; }; void RemoveListener() { mListener = nullptr; }; protected: @@ -298,15 +298,7 @@ class GroupDataProvider mListener->OnGroupAdded(fabric_index, new_group); } } - void GroupRemoved(chip::FabricIndex fabric_index, const GroupInfo & old_group) - { - if (mListener) - { - mListener->OnGroupRemoved(fabric_index, old_group); - } - } - - Listener * mListener = nullptr; + GroupListener * mListener = nullptr; }; /** diff --git a/src/credentials/GroupDataProviderImpl.cpp b/src/credentials/GroupDataProviderImpl.cpp index 43d89c9d2fa7ed..8873a27c870621 100644 --- a/src/credentials/GroupDataProviderImpl.cpp +++ b/src/credentials/GroupDataProviderImpl.cpp @@ -749,7 +749,6 @@ CHIP_ERROR GroupDataProviderImpl::SetGroupInfoAt(chip::FabricIndex fabric_index, FabricData fabric(fabric_index); GroupData group; - bool new_group = false; // Load fabric, defaults to zero CHIP_ERROR err = fabric.Load(mStorage); @@ -759,9 +758,9 @@ CHIP_ERROR GroupDataProviderImpl::SetGroupInfoAt(chip::FabricIndex fabric_index, bool found = group.Find(mStorage, fabric, info.group_id); VerifyOrReturnError(!found || (group.index == index), CHIP_ERROR_DUPLICATE_KEY_ID); - found = group.Get(mStorage, fabric, index); - new_group = (group.group_id != info.group_id); - group.group_id = info.group_id; + found = group.Get(mStorage, fabric, index); + const bool new_group = (group.group_id != info.group_id); + group.group_id = info.group_id; group.SetName(info.name); if (found) @@ -866,7 +865,10 @@ CHIP_ERROR GroupDataProviderImpl::RemoveGroupInfoAt(chip::FabricIndex fabric_ind } // Update fabric info ReturnErrorOnFailure(fabric.Save(mStorage)); - GroupRemoved(fabric_index, group); + if (mListener) + { + mListener->OnGroupRemoved(fabric_index, group); + } return CHIP_NO_ERROR; } @@ -1182,10 +1184,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveEndpoints(chip::FabricIndex fabric_index size_t endpoint_index = 0; while (endpoint_index < group.endpoint_count) { - if (CHIP_NO_ERROR != endpoint.Load(mStorage)) - { - break; - } + ReturnErrorOnFailure(endpoint.Load(mStorage)); endpoint.Delete(mStorage); endpoint.id = endpoint.next; endpoint_index++; diff --git a/src/credentials/tests/TestGroupDataProvider.cpp b/src/credentials/tests/TestGroupDataProvider.cpp index 72e6e08af5ed8c..9d39efbafe8713 100644 --- a/src/credentials/tests/TestGroupDataProvider.cpp +++ b/src/credentials/tests/TestGroupDataProvider.cpp @@ -95,7 +95,7 @@ static KeySet kKeySet1(kKeysetId1, KeySet::SecurityPolicy::kLowLatency, 1); static KeySet kKeySet2(kKeysetId2, KeySet::SecurityPolicy::kLowLatency, 2); static KeySet kKeySet3(kKeysetId3, KeySet::SecurityPolicy::kStandard, 3); -class TestListener : public GroupDataProvider::Listener +class TestListener : public GroupDataProvider::GroupListener { public: chip::FabricIndex fabric_index = kUndefinedFabricIndex;