Split ServerClusterInterfaceRegistry into 2 classes to support both Single and Multi-endpoint paths (#40387)

* Split ServerClusterInterfaceRegistry into 2 classes to support both Single and Multi-endpoint use cases

* Restyled by clang-format

* Move dtor to base class

* Remove codegen comment; fix minor test issue

* Moved dtor back to the previous location

* Address Andrei's code review suggestions: better comments;rename AllClustersList class to ServerClusterInstances

* Split into separate classes

* Restyled by whitespace

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <commits@restyled.io>

Author: soares-sergio

src/app/server-cluster/BUILD.gn

@@ -43,6 +43,8 @@ source_set("registry") {
   sources = [
     "ServerClusterInterfaceRegistry.cpp",
     "ServerClusterInterfaceRegistry.h",
+    "SingleEndpointServerClusterRegistry.cpp",
+    "SingleEndpointServerClusterRegistry.h",
   ]
 
   public_deps = [

src/app/server-cluster/ServerClusterInterfaceRegistry.cpp

@@ -58,10 +58,6 @@ CHIP_ERROR ServerClusterInterfaceRegistry::Register(ServerClusterRegistration &
         // Double-checking for duplicates makes the checks O(n^2) on the total number of registered
         // items. We preserve this however we may want to make this optional at some point in time.
         VerifyOrReturnError(Get(path) == nullptr, CHIP_ERROR_DUPLICATE_KEY_ID);
-
-        // Codegen registry requirements (so that we can support endpoint unregistration): every
-        // path must belong to the same endpoint id.
-        VerifyOrReturnError(path.mEndpointId == paths[0].mEndpointId, CHIP_ERROR_INVALID_ARGUMENT);
     }
 
     if (mContext.has_value())
@@ -117,54 +113,6 @@ CHIP_ERROR ServerClusterInterfaceRegistry::Unregister(ServerClusterInterface * w
     return CHIP_ERROR_NOT_FOUND;
 }
 
-ServerClusterInterfaceRegistry::ClustersList ServerClusterInterfaceRegistry::ClustersOnEndpoint(EndpointId endpointId)
-{
-    return { mRegistrations, endpointId };
-}
-
-void ServerClusterInterfaceRegistry::UnregisterAllFromEndpoint(EndpointId endpointId)
-{
-    ServerClusterRegistration * prev    = nullptr;
-    ServerClusterRegistration * current = mRegistrations;
-    while (current != nullptr)
-    {
-        // Requirements for Paths:
-        //   - GetPaths() MUST be non-empty
-        //   - GetPaths() MUST belong to the same endpoint
-        // Loop below relies on that: if the endpoint matches, it can be removed
-        auto paths = current->serverClusterInterface->GetPaths();
-        if (paths.empty() || paths.front().mEndpointId == endpointId)
-        {
-            if (mCachedInterface == current->serverClusterInterface)
-            {
-                mCachedInterface = nullptr;
-            }
-            if (prev == nullptr)
-            {
-                mRegistrations = current->next;
-            }
-            else
-            {
-                prev->next = current->next;
-            }
-            ServerClusterRegistration * actual_next = current->next;
-
-            current->next = nullptr; // Make sure current does not look like part of a list.
-            if (mContext.has_value())
-            {
-                current->serverClusterInterface->Shutdown();
-            }
-
-            current = actual_next;
-        }
-        else
-        {
-            prev    = current;
-            current = current->next;
-        }
-    }
-}
-
 ServerClusterInterface * ServerClusterInterfaceRegistry::Get(const ConcreteClusterPath & clusterPath)
 {
     // Check the cache to speed things up
@@ -244,5 +192,10 @@ void ServerClusterInterfaceRegistry::ClearContext()
     mContext.reset();
 }
 
+ServerClusterInterfaceRegistry::ServerClusterInstances ServerClusterInterfaceRegistry::AllServerClusterInstances()
+{
+    return { mRegistrations };
+}
+
 } // namespace app
 } // namespace chip

src/app/server-cluster/ServerClusterInterfaceRegistry.h

@@ -23,6 +23,7 @@
 
 #include <cstdint>
 #include <new>
+#include <optional>
 
 namespace chip {
 namespace app {
@@ -132,82 +133,12 @@ struct LazyRegisteredServerCluster
 class ServerClusterInterfaceRegistry
 {
 public:
-    /// represents an iterable list of clusters
-    class ClustersList
-    {
-    public:
-        class Iterator
-        {
-        public:
-            Iterator(ServerClusterRegistration * interface, EndpointId endpoint) : mEndpointId(endpoint), mRegistration(interface)
-            {
-                if (mRegistration != nullptr)
-                {
-                    mSpan = interface->serverClusterInterface->GetPaths();
-                }
-                AdvanceUntilMatchingEndpoint();
-            }
-
-            Iterator & operator++()
-            {
-                if (!mSpan.empty())
-                {
-                    mSpan = mSpan.SubSpan(1);
-                }
-                AdvanceUntilMatchingEndpoint();
-                return *this;
-            }
-            bool operator==(const Iterator & other) const { return mRegistration == other.mRegistration; }
-            bool operator!=(const Iterator & other) const { return mRegistration != other.mRegistration; }
-            ClusterId operator*() { return mSpan.begin()->mClusterId; }
-
-        private:
-            const EndpointId mEndpointId;
-            ServerClusterRegistration * mRegistration;
-            Span<const ConcreteClusterPath> mSpan;
-
-            void AdvanceUntilMatchingEndpoint()
-            {
-                while (mRegistration != nullptr)
-                {
-                    if (mSpan.empty())
-                    {
-                        mRegistration = mRegistration->next;
-                        if (mRegistration != nullptr)
-                        {
-                            mSpan = mRegistration->serverClusterInterface->GetPaths();
-                        }
-                        continue;
-                    }
-                    if (mSpan.begin()->mEndpointId == mEndpointId)
-                    {
-                        return;
-                    }
-
-                    // need to keep searching
-                    mSpan = mSpan.SubSpan(1);
-                }
-            }
-        };
-
-        constexpr ClustersList(ServerClusterRegistration * start, EndpointId endpointId) : mEndpointId(endpointId), mStart(start) {}
-        Iterator begin() { return { mStart, mEndpointId }; }
-        Iterator end() { return { nullptr, mEndpointId }; }
-
-    private:
-        const EndpointId mEndpointId;
-        ServerClusterRegistration * mStart;
-    };
-
     ~ServerClusterInterfaceRegistry();
 
     /// Add the given entry to the registry.
-    /// NOTE the requirement of entries to be part of the same endpoint.
     ///
     /// Requirements:
     ///   - entry MUST NOT be part of any other registration
-    ///   - paths MUST be part of the same endpoint (requirement for codegen server cluster interface implementations)
-    ///
     ///   - LIFETIME of entry must outlive the Registry (or entry must be unregistered)
     ///
     /// There can be only a single registration for a given `endpointId/clusterId` path.
@@ -221,24 +152,52 @@ class ServerClusterInterfaceRegistry
     /// Return the interface registered for the given cluster path or nullptr if one does not exist
     ServerClusterInterface * Get(const ConcreteClusterPath & path);
 
-    /// Provides a list of clusters that are registered for the given endpoint.
-    ///
-    /// ClustersList points inside the internal registrations of the registry, so
-    /// the list is only valid as long as the registry is not modified.
-    ClustersList ClustersOnEndpoint(EndpointId endpointId);
-
-    /// Unregister all registrations for the given endpoint.
-    void UnregisterAllFromEndpoint(EndpointId endpointId);
-
     // Set up the underlying context for all clusters that are managed by this registry.
     //
-    // The values within context will be copied and used.
+    // The values within context will be moved and used as-is.
     CHIP_ERROR SetContext(ServerClusterContext && context);
 
     // Invalidates current context.
     void ClearContext();
 
-private:
+    // Represents an iterable list of all clusters registered in this registry.
+    // The list is only valid as long as the registry is not modified.
+    // The list is not guaranteed to be in any particular order.
+    class ServerClusterInstances
+    {
+    public:
+        class Iterator
+        {
+        public:
+            Iterator(ServerClusterRegistration * registration) : mRegistration(registration) {}
+
+            Iterator & operator++()
+            {
+                if (mRegistration)
+                {
+                    mRegistration = mRegistration->next;
+                }
+                return *this;
+            }
+            bool operator==(const Iterator & other) const { return mRegistration == other.mRegistration; }
+            bool operator!=(const Iterator & other) const { return mRegistration != other.mRegistration; }
+            ServerClusterInterface * operator*() { return mRegistration ? mRegistration->serverClusterInterface : nullptr; }
+
+        private:
+            ServerClusterRegistration * mRegistration;
+        };
+
+        constexpr ServerClusterInstances(ServerClusterRegistration * start) : mStart(start) {}
+        Iterator begin() { return { mStart }; }
+        Iterator end() { return { nullptr }; }
+
+    private:
+        ServerClusterRegistration * mStart;
+    };
+
+    ServerClusterInstances AllServerClusterInstances();
+
+protected:
     ServerClusterRegistration * mRegistrations = nullptr;
 
     // A one-element cache to speed up finding a cluster within an endpoint.

src/app/server-cluster/SingleEndpointServerClusterRegistry.cpp

@@ -0,0 +1,94 @@
+/*
+ *    Copyright (c) 2025 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+#include <app/server-cluster/SingleEndpointServerClusterRegistry.h>
+
+#include <app/ConcreteClusterPath.h>
+#include <app/server-cluster/ServerClusterInterface.h>
+#include <lib/core/CHIPError.h>
+#include <lib/core/DataModelTypes.h>
+#include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
+#include <optional>
+
+namespace chip {
+namespace app {
+
+CHIP_ERROR SingleEndpointServerClusterRegistry::Register(ServerClusterRegistration & entry)
+{
+    Span<const ConcreteClusterPath> paths = entry.serverClusterInterface->GetPaths();
+    VerifyOrReturnError(!paths.empty(), CHIP_ERROR_INVALID_ARGUMENT);
+
+    // Codegen registry requirements (so that we can support endpoint unregistration): every
+    // path must belong to the same endpoint id.
+    for (const ConcreteClusterPath & path : paths)
+    {
+        VerifyOrReturnError(path.mEndpointId == paths[0].mEndpointId, CHIP_ERROR_INVALID_ARGUMENT);
+    }
+
+    return ServerClusterInterfaceRegistry::Register(entry);
+}
+
+SingleEndpointServerClusterRegistry::ClustersList SingleEndpointServerClusterRegistry::ClustersOnEndpoint(EndpointId endpointId)
+{
+    return { mRegistrations, endpointId };
+}
+
+void SingleEndpointServerClusterRegistry::UnregisterAllFromEndpoint(EndpointId endpointId)
+{
+    ServerClusterRegistration * prev    = nullptr;
+    ServerClusterRegistration * current = mRegistrations;
+    while (current != nullptr)
+    {
+        // Requirements for Paths:
+        //   - GetPaths() MUST be non-empty
+        //   - GetPaths() MUST belong to the same endpoint
+        // Loop below relies on that: if the endpoint matches, it can be removed
+        auto paths = current->serverClusterInterface->GetPaths();
+        if (paths.empty() || paths.front().mEndpointId == endpointId)
+        {
+            if (mCachedInterface == current->serverClusterInterface)
+            {
+                mCachedInterface = nullptr;
+            }
+            if (prev == nullptr)
+            {
+                mRegistrations = current->next;
+            }
+            else
+            {
+                prev->next = current->next;
+            }
+            ServerClusterRegistration * actual_next = current->next;
+
+            current->next = nullptr; // Make sure current does not look like part of a list.
+            if (mContext.has_value())
+            {
+                current->serverClusterInterface->Shutdown();
+            }
+
+            current = actual_next;
+        }
+        else
+        {
+            prev    = current;
+            current = current->next;
+        }
+    }
+}
+
+} // namespace app
+} // namespace chip