Check if a duplicated provider is present before loading into RAM (#37643)

* [Silabs] Check if a duplicated provider is present before loading it to RAM

* [Silabs] Added change requested by bzbarsky-apple

* Restyled by clang-format

* Fixed codereview request

* Fixed unit test issues

* Restyled by clang-format

* Make ProviderAlreadyInList public instead of private

* Update src/app/clusters/ota-requestor/DefaultOTARequestorStorage.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Should return false if the check is made on an empty list

* Fixed unit test, removed GetListSize and put back ProviderAlreadyInList as a private member

* Fixed unit test

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

Author: 3 people

src/app/clusters/ota-requestor/DefaultOTARequestorStorage.cpp

@@ -59,6 +59,24 @@ CHIP_ERROR DefaultOTARequestorStorage::StoreDefaultProviders(const ProviderLocat
                                                static_cast<uint16_t>(writer.GetLengthWritten()));
 }
 
+bool DefaultOTARequestorStorage::ProviderAlreadyInList(ProviderLocationList & listProviders, ProviderLocationType provider)
+{
+    auto iterator = listProviders.Begin();
+
+    while (iterator.Next())
+    {
+        ProviderLocationType pl = iterator.GetValue();
+
+        if ((pl.providerNodeID == provider.providerNodeID) && (pl.fabricIndex == provider.fabricIndex) &&
+            (pl.endpoint == provider.endpoint))
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 CHIP_ERROR DefaultOTARequestorStorage::LoadDefaultProviders(ProviderLocationList & providers)
 {
     uint8_t buffer[kProviderListMaxSerializedSize];
@@ -77,7 +95,10 @@ CHIP_ERROR DefaultOTARequestorStorage::LoadDefaultProviders(ProviderLocationList
     {
         ProviderLocationType provider;
         ReturnErrorOnFailure(provider.Decode(reader));
-        providers.Add(provider);
+        if (!ProviderAlreadyInList(providers, provider))
+        {
+            providers.Add(provider);
+        }
     }
 
     ReturnErrorOnFailure(reader.ExitContainer(outerType));

src/app/clusters/ota-requestor/DefaultOTARequestorStorage.h

@@ -49,6 +49,10 @@ class DefaultOTARequestorStorage : public OTARequestorStorage
     CHIP_ERROR ClearTargetVersion() override;
 
 private:
+    /**
+     * Check whether a provider is already present in a list of providers.
+     */
+    bool ProviderAlreadyInList(ProviderLocationList & listProviders, ProviderLocationType provider);
     CHIP_ERROR Load(const char * key, MutableByteSpan & buffer);
     PersistentStorageDelegate * mPersistentStorage = nullptr;
 };

src/app/tests/TestDefaultOTARequestorStorage.cpp

@@ -97,6 +97,48 @@ TEST(TestDefaultOTARequestorStorage, TestDefaultProvidersEmpty)
     EXPECT_FALSE(providers.Begin().Next());
 }
 
+TEST(TestDefaultOTARequestorStorage, TestDefaultProvidersDuplicated)
+{
+    TestPersistentStorageDelegate persistentStorage;
+    DefaultOTARequestorStorage otaStorage;
+    otaStorage.Init(persistentStorage);
+
+    const auto makeProvider = [](FabricIndex fabric, NodeId nodeId, EndpointId endpointId) {
+        OTARequestorStorage::ProviderLocationType provider;
+        provider.fabricIndex    = fabric;
+        provider.providerNodeID = nodeId;
+        provider.endpoint       = endpointId;
+        return provider;
+    };
+
+    ProviderLocationList providers = {};
+
+    EXPECT_EQ(CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND, otaStorage.LoadDefaultProviders(providers));
+    auto iterator = providers.Begin();
+    EXPECT_EQ(false, iterator.Next());
+
+    for (uint8_t i = 0; i < CHIP_CONFIG_MAX_FABRICS; i++)
+    {
+        EXPECT_EQ(CHIP_NO_ERROR, providers.Add(makeProvider(FabricIndex(1), NodeId(0x11111111), EndpointId(1))));
+    }
+
+    EXPECT_EQ(CHIP_NO_ERROR, otaStorage.StoreDefaultProviders(providers));
+
+    providers = {};
+    EXPECT_EQ(CHIP_NO_ERROR, otaStorage.LoadDefaultProviders(providers));
+
+    iterator = providers.Begin();
+
+    // Check provider #1
+    EXPECT_EQ(true, iterator.Next());
+    OTARequestorStorage::ProviderLocationType provider1 = iterator.GetValue();
+    EXPECT_EQ(FabricIndex(1), provider1.fabricIndex);
+    EXPECT_EQ(NodeId(0x11111111), provider1.providerNodeID);
+    EXPECT_EQ(EndpointId(1), provider1.endpoint);
+
+    EXPECT_EQ(false, iterator.Next());
+}
+
 TEST(TestDefaultOTARequestorStorage, TestCurrentProviderLocation)
 {
     TestPersistentStorageDelegate persistentStorage;