Refactor server iteration to ServerSource (reduce allocations, improve clarity)

src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.cs

@@ -2,7 +2,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Threading.Tasks;
@@ -506,11 +505,12 @@ public Task<long> SetCombineAndStoreAsync(SetOperation operation, string destina
     /// <inheritdoc/>
     public async Task<IEnumerable<string>> SearchKeysAsync(string pattern)
     {
-        pattern = $"{keyPrefix}{pattern}";
+        pattern = keyPrefix + pattern;
         var keys = new HashSet<string>();
         var hasPrefix = !string.IsNullOrEmpty(keyPrefix);
 
-        foreach (var server in ServerIteratorFactory.GetServers(connectionPoolManager.GetConnection(), serverEnumerationStrategy))
+        var serverSource = new ServerSource(connectionPoolManager.GetConnection());
+        foreach (var server in serverSource.GetServers(serverEnumerationStrategy))
         {
             // The prefix is stripped while filling: a lazy Select would re-allocate every substring on each enumeration.
             await foreach (var key in server.KeysAsync(dbNumber, pattern, 1000).ConfigureAwait(false))

src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerEnumerable.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Ugo Lattanzi.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
+using System;
 using System.Collections;
 using System.Collections.Generic;
 
@@ -10,6 +11,11 @@ namespace StackExchange.Redis.Extensions.Core.ServerIteration;
 /// <summary>
 /// The class that allows you to enumerate all the redis servers.
 /// </summary>
+#if NET5_0_OR_GREATER
+[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.", DiagnosticId = "SRE0002")]
+#else
+[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.")]
+#endif
 public class ServerEnumerable : IEnumerable<IServer>
 {
     private readonly IConnectionMultiplexer multiplexer;
@@ -36,25 +42,13 @@ public ServerEnumerable(
     /// Return the enumerator of the Redis servers
     /// </summary>
     public IEnumerator<IServer> GetEnumerator()
-    {
-        foreach (var endPoint in multiplexer.GetEndPoints())
-        {
-            var server = multiplexer.GetServer(endPoint);
-            if (targetRole == ServerEnumerationStrategy.TargetRoleOptions.PreferSlave)
-            {
-                if (!server.IsReplica)
-                    continue;
-            }
-
-            if (unreachableServerAction == ServerEnumerationStrategy.UnreachableServerActionOptions.IgnoreIfOtherAvailable)
-            {
-                if (!server.IsConnected || !server.Features.Scan)
-                    continue;
-            }
-
-            yield return server;
-        }
-    }
+        => new ServerSource(multiplexer).GetServers(new()
+        {
+            Mode = ServerEnumerationStrategy.ModeOptions.All,
+            TargetRole = targetRole,
+            UnreachableServerAction = unreachableServerAction
+        })
+            .GetEnumerator();
 
     IEnumerator IEnumerable.GetEnumerator()
     {

src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs

@@ -10,6 +10,11 @@ namespace StackExchange.Redis.Extensions.Core.ServerIteration;
 /// <summary>
 /// The factory that allows you to enumerate all Redis servers.
 /// </summary>
+#if NET5_0_OR_GREATER
+[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.", DiagnosticId = "SRE0002")]
+#else
+[Obsolete("This type is removed in v14. Fire an issue if you require similar functionality.")]
+#endif
 public static class ServerIteratorFactory
 {
     /// <summary>
@@ -21,34 +26,5 @@ public static class ServerIteratorFactory
     public static IEnumerable<IServer> GetServers(
         IConnectionMultiplexer multiplexer,
         ServerEnumerationStrategy serverEnumerationStrategy)
-    {
-        switch (serverEnumerationStrategy.Mode)
-        {
-            case ServerEnumerationStrategy.ModeOptions.All:
-                return new ServerEnumerable(
-                    multiplexer,
-                    serverEnumerationStrategy.TargetRole,
-                    serverEnumerationStrategy.UnreachableServerAction);
-
-            case ServerEnumerationStrategy.ModeOptions.Single:
-                var serversSingle = new ServerEnumerable(
-                    multiplexer,
-                    serverEnumerationStrategy.TargetRole,
-                    serverEnumerationStrategy.UnreachableServerAction);
-
-                return TakeFirst(serversSingle);
-
-            default:
-                throw new NotImplementedException();
-        }
-    }
-
-    private static IEnumerable<IServer> TakeFirst(ServerEnumerable servers)
-    {
-        foreach (var server in servers)
-        {
-            yield return server;
-            yield break;
-        }
-    }
+        => new ServerSource(multiplexer).GetServers(serverEnumerationStrategy);
 }

src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerSource.cs

@@ -0,0 +1,50 @@
+// Copyright (c) Ugo Lattanzi.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Linq;
+using System.Net;
+
+using StackExchange.Redis.Extensions.Core.Configuration;
+
+using static StackExchange.Redis.Extensions.Core.Configuration.ServerEnumerationStrategy;
+
+namespace StackExchange.Redis.Extensions.Core.ServerIteration;
+
+/// <summary>
+/// Represents a source of servers from a connection multiplexer.
+/// </summary>
+/// <param name="multiplexer">The connection multiplexer to retrieve servers from.</param>
+/// <remarks>
+/// DO NOT implement <see cref="IEnumerable{IServer}"/> to avoid boxing struct enumerators and to keep allocation semantics explicit.
+/// </remarks>
+internal readonly struct ServerSource(IConnectionMultiplexer multiplexer)
+{
+    private readonly EndPoint[] endPoints = multiplexer.GetEndPoints();
+
+    public IEnumerable<IServer> GetServers(ServerEnumerationStrategy strategy)
+        => strategy.Mode == ModeOptions.Single
+            ? EnumerateCore(strategy).Take(1)
+            : EnumerateCore(strategy);
+
+    private IEnumerable<IServer> EnumerateCore(ServerEnumerationStrategy strategy)
+    {
+        foreach (var endPoint in endPoints)
+        {
+            var server = multiplexer.GetServer(endPoint);
+
+            if (strategy.TargetRole == TargetRoleOptions.PreferSlave)
+            {
+                if (!server.IsReplica)
+                    continue;
+            }
+
+            if (strategy.UnreachableServerAction == UnreachableServerActionOptions.IgnoreIfOtherAvailable)
+            {
+                if (!server.IsConnected || !server.Features.Scan)
+                    continue;
+            }
+
+            yield return server;
+        }
+    }
+}

tests/StackExchange.Redis.Extensions.Core.Tests/ServerSourceTests.cs

@@ -0,0 +1,125 @@
+// Copyright (c) Ugo Lattanzi.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Net;
+
+using NSubstitute;
+
+using StackExchange.Redis.Extensions.Core.Configuration;
+using StackExchange.Redis.Extensions.Core.ServerIteration;
+
+using Xunit;
+
+namespace StackExchange.Redis.Extensions.Core.Tests;
+
+public class ServerSourceTests
+{
+    [Fact]
+    public void GetServers_SingleMode_ReturnsOnlyFirstServer()
+    {
+        var (multiplexer, servers) = BuildMultiplexer(3);
+        var strategy = new ServerEnumerationStrategy { Mode = ServerEnumerationStrategy.ModeOptions.Single };
+
+        var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
+
+        Assert.Single(result);
+        Assert.Same(servers[0], result[0]);
+    }
+
+    [Fact]
+    public void GetServers_AllMode_ReturnsEveryServer()
+    {
+        var (multiplexer, servers) = BuildMultiplexer(3);
+        var strategy = new ServerEnumerationStrategy { Mode = ServerEnumerationStrategy.ModeOptions.All };
+
+        var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
+
+        Assert.Equal(3, result.Count);
+        Assert.Equal(servers, result);
+    }
+
+    [Fact]
+    public void GetServers_PreferSlave_SkipsPrimaries()
+    {
+        var (multiplexer, servers) = BuildMultiplexer(3);
+        servers[1].IsReplica.Returns(true);
+
+        var strategy = new ServerEnumerationStrategy
+        {
+            Mode = ServerEnumerationStrategy.ModeOptions.All,
+            TargetRole = ServerEnumerationStrategy.TargetRoleOptions.PreferSlave
+        };
+
+        var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
+
+        Assert.Single(result);
+        Assert.Same(servers[1], result[0]);
+    }
+
+    [Fact]
+    public void GetServers_IgnoreIfOtherAvailable_SkipsDisconnectedServers()
+    {
+        var (multiplexer, servers) = BuildMultiplexer(3);
+        servers[0].IsConnected.Returns(false);
+
+        var strategy = new ServerEnumerationStrategy
+        {
+            Mode = ServerEnumerationStrategy.ModeOptions.Single,
+            UnreachableServerAction = ServerEnumerationStrategy.UnreachableServerActionOptions.IgnoreIfOtherAvailable
+        };
+
+        var result = new ServerSource(multiplexer).GetServers(strategy).ToList();
+
+        Assert.Single(result);
+        Assert.Same(servers[1], result[0]);
+    }
+
+    // Guards the deferred-execution contract: GetServers must not touch the multiplexer until the
+    // result is enumerated. An earlier revision evaluated the Single branch eagerly, which this catches.
+    [Theory]
+    [InlineData(ServerEnumerationStrategy.ModeOptions.Single)]
+    [InlineData(ServerEnumerationStrategy.ModeOptions.All)]
+    public void GetServers_IsLazy_DoesNotResolveServersUntilEnumerated(ServerEnumerationStrategy.ModeOptions mode)
+    {
+        var (multiplexer, _) = BuildMultiplexer(3);
+
+        var deferred = new ServerSource(multiplexer).GetServers(new() { Mode = mode });
+
+        multiplexer.DidNotReceive().GetServer(Arg.Any<EndPoint>());
+
+        _ = deferred.ToList();
+
+        multiplexer.Received().GetServer(Arg.Any<EndPoint>());
+    }
+
+    private static (IConnectionMultiplexer Multiplexer, List<IServer> Servers) BuildMultiplexer(int count)
+    {
+        var multiplexer = Substitute.For<IConnectionMultiplexer>();
+        var endPoints = new EndPoint[count];
+        var servers = new List<IServer>(count);
+
+        for (var i = 0; i < count; i++)
+        {
+            var endPoint = new IPEndPoint(IPAddress.Loopback, 6379 + i);
+            endPoints[i] = endPoint;
+
+            var server = Substitute.For<IServer>();
+            server.IsReplica.Returns(false);
+            server.IsConnected.Returns(true);
+
+            // SCAN landed in Redis 2.8; without a version the mock reports Features.Scan == false
+            // and the IgnoreIfOtherAvailable filter would discard every server.
+            server.Features.Returns(new RedisFeatures(new Version(2, 8)));
+
+            servers.Add(server);
+
+            multiplexer.GetServer(endPoint).Returns(server);
+        }
+
+        multiplexer.GetEndPoints().Returns(endPoints);
+
+        return (multiplexer, servers);
+    }
+}