Group Data Provider Listener: Code review changes.

project-chip · Dec 14, 2021 · f739bac · f739bac
1 parent 42f5add
commit f739bac
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 22 deletions.
diff --git 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.
          *
@@ -286,7 +286,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:
@@ -297,15 +297,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
@@ -708,7 +708,6 @@ CHIP_ERROR GroupDataProviderImpl::SetGroupInfoAt(chip::FabricIndex fabric_index,
 
     FabricData fabric(fabric_index);
     GroupData group;
-    bool new_group = false;
 
     // Load fabric, defaults to zero
     fabric.Load(mStorage);
@@ -717,9 +716,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)
@@ -824,7 +823,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;
 }
 
@@ -1139,10 +1141,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
@@ -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;