diff --git a/src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.cs b/src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.cs index ecc7b42d..1b5ffcdf 100644 --- a/src/core/StackExchange.Redis.Extensions.Core/Implementations/RedisDatabase.cs +++ b/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 SetCombineAndStoreAsync(SetOperation operation, string destina /// public async Task> SearchKeysAsync(string pattern) { - pattern = $"{keyPrefix}{pattern}"; + pattern = keyPrefix + pattern; var keys = new HashSet(); 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)) diff --git a/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerEnumerable.cs b/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerEnumerable.cs index ab410c77..59ffa646 100644 --- a/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerEnumerable.cs +++ b/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; /// /// The class that allows you to enumerate all the redis servers. /// +#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 { private readonly IConnectionMultiplexer multiplexer; @@ -36,25 +42,13 @@ public ServerEnumerable( /// Return the enumerator of the Redis servers /// public IEnumerator 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() { diff --git a/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs b/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs index 0f1b0bdf..1dc40a1a 100644 --- a/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs +++ b/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerIteratorFactory.cs @@ -10,6 +10,11 @@ namespace StackExchange.Redis.Extensions.Core.ServerIteration; /// /// The factory that allows you to enumerate all Redis servers. /// +#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 { /// @@ -21,34 +26,5 @@ public static class ServerIteratorFactory public static IEnumerable 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 TakeFirst(ServerEnumerable servers) - { - foreach (var server in servers) - { - yield return server; - yield break; - } - } + => new ServerSource(multiplexer).GetServers(serverEnumerationStrategy); } diff --git a/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerSource.cs b/src/core/StackExchange.Redis.Extensions.Core/ServerIteration/ServerSource.cs new file mode 100644 index 00000000..fb51faac --- /dev/null +++ b/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; + +/// +/// Represents a source of servers from a connection multiplexer. +/// +/// The connection multiplexer to retrieve servers from. +/// +/// DO NOT implement to avoid boxing struct enumerators and to keep allocation semantics explicit. +/// +internal readonly struct ServerSource(IConnectionMultiplexer multiplexer) +{ + private readonly EndPoint[] endPoints = multiplexer.GetEndPoints(); + + public IEnumerable GetServers(ServerEnumerationStrategy strategy) + => strategy.Mode == ModeOptions.Single + ? EnumerateCore(strategy).Take(1) + : EnumerateCore(strategy); + + private IEnumerable 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; + } + } +} diff --git a/tests/StackExchange.Redis.Extensions.Core.Tests/ServerSourceTests.cs b/tests/StackExchange.Redis.Extensions.Core.Tests/ServerSourceTests.cs new file mode 100644 index 00000000..72fde22e --- /dev/null +++ b/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()); + + _ = deferred.ToList(); + + multiplexer.Received().GetServer(Arg.Any()); + } + + private static (IConnectionMultiplexer Multiplexer, List Servers) BuildMultiplexer(int count) + { + var multiplexer = Substitute.For(); + var endPoints = new EndPoint[count]; + var servers = new List(count); + + for (var i = 0; i < count; i++) + { + var endPoint = new IPEndPoint(IPAddress.Loopback, 6379 + i); + endPoints[i] = endPoint; + + var server = Substitute.For(); + 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); + } +}