From f739baca7b8b048b6150aa82e634364722fc57e3 Mon Sep 17 00:00:00 2001
From: Ricardo Casallas <ricardo.casallas@silabs.com>
Date: Mon, 13 Dec 2021 11:48:45 -0500
Subject: [PATCH] Group Data Provider Listener: Code review changes.

---
 src/credentials/GroupDataProvider.h             | 16 ++++------------
 src/credentials/GroupDataProviderImpl.cpp       | 17 ++++++++---------
 src/credentials/tests/TestGroupDataProvider.cpp |  2 +-
 3 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h
index 36e7a30c51b4b0..fef7f776b99a82 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.
          *
@@ -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
index 382d8e76f6cb75..f0bf5fd67fc61e 100644
--- 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
index 99a6fc1edfff68..8746f65be7084f 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;