From 1a24e7f956b9865c01730367638d16c3a42d683e Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 30 Jul 2026 13:42:35 -0700 Subject: [PATCH 1/4] refactor(client): consolidate redundant command implementations into BaseClient (#462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move 19 connection/server management commands from abstract+override pattern to concrete implementations in BaseClient - Fix incorrect cluster routing: - SELECT: Random → AllNodes (#491) - CONFIG SET: AllPrimaries → AllNodes (#492) - CONFIG REWRITE: Random → AllNodes (#493) - CONFIG RESETSTAT: AllPrimaries → AllNodes (#493) - FUNCTION KILL: AllPrimaries → AllNodes (#494) - Remove isSingleValue parameter from ToClusterValue; inline MakeClusterValueHandler into Cmd.ToClusterValue() - Update GetServerVersionAsync to use default routing - Mark deprecated methods for removal in 2.0 (#495, #496): - ConfigGetAsync(ValkeyValue): use routed overload instead - FunctionListAsync(FunctionListOptions?): use routed overload instead - ScriptInvokeAsync(Script, ClusterScriptOptions): wrong design - ClusterScriptOptions: remove in favor of ScriptOptions - Delete GlideClient.ConnectionManagementCommands.cs Signed-off-by: Taylor Curran Signed-off-by: currantw --- CHANGELOG.md | 6 + ...BaseClient.ConnectionManagementCommands.cs | 31 +++-- .../BaseClient.ServerManagementCommands.cs | 31 +++-- ...lideClient.ConnectionManagementCommands.cs | 50 -------- .../GlideClient.ServerManagementCommands.cs | 38 ------ ...sterClient.ConnectionManagementCommands.cs | 51 +------- ...sterClient.ScriptingAndFunctionCommands.cs | 110 +++++----------- ...eClusterClient.ServerManagementCommands.cs | 121 ++++++------------ .../Valkey.Glide/Client/GlideClusterClient.cs | 8 +- ...sterClient.ScriptingAndFunctionCommands.cs | 4 + ...eClusterClient.ServerManagementCommands.cs | 2 + sources/Valkey.Glide/Internals/Cmd.cs | 16 +-- .../Internals/Request.ConnectionManagement.cs | 3 - .../Internals/ResponseConverters.cs | 16 --- .../scripting/ClusterScriptOptions.cs | 2 + .../ClusterClientTests.cs | 3 + .../ServerManagementCommandTests.cs | 3 + .../ClusterScriptOptionsTests.cs | 3 + .../PubSubFFIIntegrationTests.cs | 23 ---- .../PubSubThreadSafetyTests.cs | 23 ---- 20 files changed, 149 insertions(+), 395 deletions(-) delete mode 100644 sources/Valkey.Glide/Client/GlideClient.ConnectionManagementCommands.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index ce8785878..9385a1f8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `XREADGROUP` returning only the first field-value pair per stream entry (#430) +- Incorrect default routes: + - `SELECT` routed to Random instead of AllNodes in cluster mode (#491) + - `CONFIG SET` routed to AllPrimaries instead of AllNodes in cluster mode (#492) + - `CONFIG REWRITE` routed to Random instead of AllNodes in cluster mode (#493) + - `CONFIG RESETSTAT` routed to AllPrimaries instead of AllNodes in cluster mode (#493) + - `FUNCTION KILL` routed to AllPrimaries instead of AllNodes in cluster mode (#494) ### Added diff --git a/sources/Valkey.Glide/Client/BaseClient.ConnectionManagementCommands.cs b/sources/Valkey.Glide/Client/BaseClient.ConnectionManagementCommands.cs index 215c3a67e..0c8f52f2e 100644 --- a/sources/Valkey.Glide/Client/BaseClient.ConnectionManagementCommands.cs +++ b/sources/Valkey.Glide/Client/BaseClient.ConnectionManagementCommands.cs @@ -5,40 +5,49 @@ namespace Valkey.Glide; -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public abstract partial class BaseClient { /// - public abstract Task ClientGetNameAsync(); + public async Task ClientGetNameAsync() + => await Command(Request.ClientGetName()); /// - public abstract Task ClientIdAsync(); + public async Task ClientIdAsync() + => await Command(Request.ClientId()); /// - public abstract Task ClientPauseAsync(TimeSpan timeout); + public async Task ClientPauseAsync(TimeSpan timeout) + => _ = await Command(Request.ClientPause(timeout)); /// - public abstract Task ClientPauseWriteAsync(TimeSpan timeout); + public async Task ClientPauseWriteAsync(TimeSpan timeout) + => _ = await Command(Request.ClientPauseWrite(timeout)); /// - public abstract Task ClientTrackingInfoAsync(); + public async Task ClientTrackingInfoAsync() + => await Command(Request.ClientTrackingInfo()); /// - public abstract Task ClientUnpauseAsync(); + public async Task ClientUnpauseAsync() + => _ = await Command(Request.ClientUnpause()); /// - public abstract Task EchoAsync(ValkeyValue message); + public async Task EchoAsync(ValkeyValue message) + => await Command(Request.Echo(message)); /// - public abstract Task PingAsync(); + public async Task PingAsync() + => await Command(Request.Ping()); /// - public abstract Task PingAsync(ValkeyValue message); + public async Task PingAsync(ValkeyValue message) + => await Command(Request.Ping(message)); /// public async Task ResetAsync() => _ = await Command(Request.Reset()); /// - public abstract Task SelectAsync(long index); + public async Task SelectAsync(long index) + => _ = await Command(Request.Select(index)); } diff --git a/sources/Valkey.Glide/Client/BaseClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/BaseClient.ServerManagementCommands.cs index 33a72c49b..b7243b689 100644 --- a/sources/Valkey.Glide/Client/BaseClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/BaseClient.ServerManagementCommands.cs @@ -1,38 +1,45 @@ // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 using Valkey.Glide.Commands.Options; +using Valkey.Glide.Internals; namespace Valkey.Glide; -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public abstract partial class BaseClient { /// - public abstract Task[]> ConfigGetAsync(IEnumerable patterns); + public async Task[]> ConfigGetAsync(IEnumerable patterns) + => await Command(Request.ConfigGetAsync(patterns)); /// - public abstract Task ConfigSetAsync(IDictionary parameters); + public async Task ConfigSetAsync(IDictionary parameters) + => _ = await Command(Request.ConfigSetAsync(parameters)); /// - public abstract Task FlushAllDatabasesAsync(FlushMode mode); + public async Task FlushAllDatabasesAsync(FlushMode mode) + => _ = await Command(Request.FlushAllDatabasesAsync(mode)); /// - public abstract Task FlushDatabaseAsync(FlushMode mode); + public async Task FlushDatabaseAsync(FlushMode mode) + => _ = await Command(Request.FlushDatabaseAsync(mode)); /// - public abstract Task LatencyResetAsync(); + public async Task LatencyResetAsync() + => await Command(Request.LatencyResetAsync([])); /// - public abstract Task LatencyResetAsync(ValkeyValue @event); + public async Task LatencyResetAsync(ValkeyValue @event) + => await Command(Request.LatencyResetAsync([@event])); /// - public abstract Task LatencyResetAsync(IEnumerable events); - - // TODO #475: Add parameterless LolwutAsync() here + public async Task LatencyResetAsync(IEnumerable events) + => await Command(Request.LatencyResetAsync(events)); /// - public abstract Task LolwutAsync(LolwutOptions options); + public async Task LolwutAsync(LolwutOptions options) + => await Command(Request.LolwutAsync(options)); /// - public abstract Task SaveAsync(); + public async Task SaveAsync() + => _ = await Command(Request.SaveAsync()); } diff --git a/sources/Valkey.Glide/Client/GlideClient.ConnectionManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClient.ConnectionManagementCommands.cs deleted file mode 100644 index e907fd158..000000000 --- a/sources/Valkey.Glide/Client/GlideClient.ConnectionManagementCommands.cs +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 - -using Valkey.Glide.Commands; -using Valkey.Glide.Internals; - -namespace Valkey.Glide; - -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). -public partial class GlideClient -{ - /// - public override async Task ClientGetNameAsync() - => await Command(Request.ClientGetName()); - - /// - public override async Task ClientIdAsync() - => await Command(Request.ClientId()); - - /// - public override async Task ClientPauseAsync(TimeSpan timeout) - => _ = await Command(Request.ClientPause(timeout)); - - /// - public override async Task ClientPauseWriteAsync(TimeSpan timeout) - => _ = await Command(Request.ClientPauseWrite(timeout)); - - /// - public override async Task ClientTrackingInfoAsync() - => await Command(Request.ClientTrackingInfo()); - - /// - public override async Task ClientUnpauseAsync() - => _ = await Command(Request.ClientUnpause()); - - /// - public override async Task EchoAsync(ValkeyValue message) - => await Command(Request.Echo(message)); - - /// - public override async Task PingAsync() - => await Command(Request.Ping()); - - /// - public override async Task PingAsync(ValkeyValue message) - => await Command(Request.Ping(message)); - - /// - public override async Task SelectAsync(long index) - => _ = await Command(Request.Select(index)); -} diff --git a/sources/Valkey.Glide/Client/GlideClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClient.ServerManagementCommands.cs index 2f5511bf0..36be866b1 100644 --- a/sources/Valkey.Glide/Client/GlideClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClient.ServerManagementCommands.cs @@ -5,7 +5,6 @@ namespace Valkey.Glide; -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public partial class GlideClient { /// @@ -28,10 +27,6 @@ public async Task BgRewriteAofAsync() public async Task[]> ConfigGetAsync(ValkeyValue pattern = default) => await Command(Request.ConfigGetAsync(pattern)); - /// - public override async Task[]> ConfigGetAsync(IEnumerable patterns) - => await Command(Request.ConfigGetAsync(patterns)); - /// public async Task ConfigResetStatisticsAsync() => _ = await Command(Request.ConfigResetStatisticsAsync()); @@ -44,10 +39,6 @@ public async Task ConfigRewriteAsync() public async Task ConfigSetAsync(ValkeyValue setting, ValkeyValue value) => _ = await Command(Request.ConfigSetAsync(setting, value)); - /// - public override async Task ConfigSetAsync(IDictionary parameters) - => _ = await Command(Request.ConfigSetAsync(parameters)); - /// public async Task DatabaseSizeAsync() => await Command(Request.DatabaseSizeAsync()); @@ -64,18 +55,10 @@ public async Task FailoverAsync(FailoverOptions options) public async Task FlushAllDatabasesAsync() => _ = await Command(Request.FlushAllDatabasesAsync()); - /// - public override async Task FlushAllDatabasesAsync(FlushMode mode) - => _ = await Command(Request.FlushAllDatabasesAsync(mode)); - /// public async Task FlushDatabaseAsync() => _ = await Command(Request.FlushDatabaseAsync()); - /// - public override async Task FlushDatabaseAsync(FlushMode mode) - => _ = await Command(Request.FlushDatabaseAsync(mode)); - /// public async Task InfoAsync() => await InfoAsync([]); @@ -95,27 +78,10 @@ public async Task LatencyHistoryAsync(ValkeyValue @event) public async Task LatencyLatestAsync() => await Command(Request.LatencyLatestAsync()); - /// - public override async Task LatencyResetAsync() - => await Command(Request.LatencyResetAsync([])); - - /// - public override async Task LatencyResetAsync(ValkeyValue @event) - => await Command(Request.LatencyResetAsync([@event])); - - /// - public override async Task LatencyResetAsync(IEnumerable events) - => await Command(Request.LatencyResetAsync(events)); - /// - // TODO #475: Move to BaseClient. public async Task LolwutAsync() => await Command(Request.LolwutAsync()); - /// - public override async Task LolwutAsync(LolwutOptions options) - => await Command(Request.LolwutAsync(options)); - /// public async Task MemoryDoctorAsync() => await Command(Request.MemoryDoctorAsync()); @@ -140,10 +106,6 @@ public async Task ReplicaOfAsync(string host, int port) public async Task ReplicaOfNoOneAsync() => _ = await Command(Request.ReplicaOfNoOneAsync()); - /// - public override async Task SaveAsync() - => _ = await Command(Request.SaveAsync()); - /// public Task TimeAsync() => Command(Request.TimeAsync()); diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs index 43fcddee0..577e374d9 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs @@ -1,66 +1,27 @@ // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 -using Valkey.Glide.Commands; using Valkey.Glide.Internals; -using static Valkey.Glide.Route; - namespace Valkey.Glide; // TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public partial class GlideClusterClient { - /// - public override async Task ClientGetNameAsync() - => await Command(Request.ClientGetName(), Route.Random); - /// public async Task> ClientGetNameAsync(Route route) - => await Command(Request.ClientGetName(route), route); - - /// - public override async Task ClientIdAsync() - => await Command(Request.ClientId(), Route.Random); + => await Command(Request.ClientGetName().ToClusterValue(), route); /// public async Task> ClientIdAsync(Route route) - => await Command(Request.ClientId().ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task ClientPauseAsync(TimeSpan timeout) - => _ = await Command(Request.ClientPause(timeout), AllPrimaries); - - /// - public override async Task ClientPauseWriteAsync(TimeSpan timeout) - => _ = await Command(Request.ClientPauseWrite(timeout), AllPrimaries); - - /// - public override async Task ClientTrackingInfoAsync() - => await Command(Request.ClientTrackingInfo(), Route.Random); + => await Command(Request.ClientId().ToClusterValue(), route); /// public async Task> ClientTrackingInfoAsync(Route route) - => await Command(Request.ClientTrackingInfo().ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task ClientUnpauseAsync() - => _ = await Command(Request.ClientUnpause(), AllPrimaries); - - /// - public override async Task EchoAsync(ValkeyValue message) - => await Command(Request.Echo(message), Route.Random); + => await Command(Request.ClientTrackingInfo().ToClusterValue(), route); /// public async Task> EchoAsync(ValkeyValue message, Route route) - => await Command(Request.Echo(message).ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task PingAsync() - => await Command(Request.Ping(), AllPrimaries); - - /// - public override async Task PingAsync(ValkeyValue message) - => await Command(Request.Ping(message), AllPrimaries); + => await Command(Request.Echo(message).ToClusterValue(), route); /// public async Task PingAsync(Route route) @@ -69,8 +30,4 @@ public async Task PingAsync(Route route) /// public async Task PingAsync(ValkeyValue message, Route route) => await Command(Request.Ping(message), route); - - /// - public override async Task SelectAsync(long index) - => _ = await Command(Request.Select(index), Route.Random); } diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs index 0e0f78153..42e2a66cf 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs @@ -9,21 +9,15 @@ public sealed partial class GlideClusterClient // ===== Script Execution with Routing ===== /// + // TODO #496: Change return type to Task; remove AllPrimaries default. + [Obsolete("Return type will change to Task in 2.0. See #496.")] public async Task> ScriptInvokeAsync( Script script, ClusterScriptOptions options, CancellationToken cancellationToken = default) { - // Determine the route - use provided route or default to AllPrimaries Route route = options.Route ?? Route.AllPrimaries; - - // Determine if this is a single-node route - bool isSingleNode = route is Route.SingleNodeRoute; - - // Create the EVALSHA command with cluster value support - var cmd = Request.EvalShaAsync(script.Hash, null, options.Args).ToClusterValue(isSingleNode); - - return await Command(cmd, route); + return await Command(Request.EvalShaAsync(script.Hash, null, options.Args).ToClusterValue(), route); } // ===== Script Management with Routing ===== @@ -33,34 +27,26 @@ public async Task> ScriptExistsAsync( IEnumerable sha1Hashes, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.ScriptExistsAsync([.. sha1Hashes]).ToClusterValue(route), route); - } + => await Command(Request.ScriptExistsAsync([.. sha1Hashes]).ToClusterValue(), route); /// public async Task ScriptFlushAsync( Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.ScriptFlushAsync(), route); - } + => _ = await Command(Request.ScriptFlushAsync(), route); /// public async Task ScriptFlushAsync( FlushMode mode, Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.ScriptFlushAsync(mode), route); - } + => _ = await Command(Request.ScriptFlushAsync(mode), route); /// public async Task ScriptKillAsync( Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.ScriptKillAsync(), route); - } + => _ = await Command(Request.ScriptKillAsync(), route); // ===== Function Execution with Routing ===== @@ -69,9 +55,7 @@ public async Task> FCallAsync( string function, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FCallAsync(function, null, null).ToClusterValue(route), route); - } + => await Command(Request.FCallAsync(function, null, null).ToClusterValue(), route); /// public async Task> FCallAsync( @@ -79,18 +63,14 @@ public async Task> FCallAsync( IEnumerable args, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FCallAsync(function, null, [.. args]).ToClusterValue(route), route); - } + => await Command(Request.FCallAsync(function, null, [.. args]).ToClusterValue(), route); /// public async Task> FCallReadOnlyAsync( string function, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FCallReadOnlyAsync(function, null, null).ToClusterValue(route), route); - } + => await Command(Request.FCallReadOnlyAsync(function, null, null).ToClusterValue(), route); /// public async Task> FCallReadOnlyAsync( @@ -98,9 +78,7 @@ public async Task> FCallReadOnlyAsync( IEnumerable args, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FCallReadOnlyAsync(function, null, [.. args]).ToClusterValue(route), route); - } + => await Command(Request.FCallReadOnlyAsync(function, null, [.. args]).ToClusterValue(), route); // ===== Function Management with Routing ===== @@ -109,9 +87,7 @@ public async Task> FunctionLoadAsync( string libraryCode, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionLoadAsync(libraryCode, false).ToClusterValue(route), route); - } + => await Command(Request.FunctionLoadAsync(libraryCode, false).ToClusterValue(), route); /// public async Task> FunctionLoadAsync( @@ -119,120 +95,94 @@ public async Task> FunctionLoadAsync( bool replace, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionLoadAsync(libraryCode, replace).ToClusterValue(route), route); - } + => await Command(Request.FunctionLoadAsync(libraryCode, replace).ToClusterValue(), route); /// public async Task FunctionDeleteAsync( string libraryName, Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionDeleteAsync(libraryName), route); - } + => _ = await Command(Request.FunctionDeleteAsync(libraryName), route); /// public async Task FunctionFlushAsync( Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionFlushAsync(), route); - } + => _ = await Command(Request.FunctionFlushAsync(), route); /// public async Task FunctionFlushAsync( FlushMode mode, Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionFlushAsync(mode), route); - } + => _ = await Command(Request.FunctionFlushAsync(mode), route); /// public new async Task FunctionKillAsync( CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionKillAsync(), Route.AllPrimaries); - } + => _ = await Command(Request.FunctionKillAsync()); /// public async Task FunctionKillAsync( Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionKillAsync(), route); - } + => _ = await Command(Request.FunctionKillAsync(), route); // ===== Function Inspection with Routing ===== /// + // TODO #495: Remove method; consolidate single-value version into BaseClient. + [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. This method will be removed in 2.0.")] public async Task> FunctionListAsync( FunctionListOptions? options = null, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionListAsync(options).ToClusterValue(Route.AllPrimaries), Route.AllPrimaries); - } + => await Command(Request.FunctionListAsync(options).ToClusterValue(), Route.AllPrimaries); /// public async Task> FunctionListAsync( FunctionListOptions? options, Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionListAsync(options).ToClusterValue(route), route); - } + => await Command(Request.FunctionListAsync(options).ToClusterValue(), route); /// public async Task> FunctionStatsAsync( Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionStatsAsync().ToClusterValue(route), route); - } + => await Command(Request.FunctionStatsAsync().ToClusterValue(), route); // ===== Function Persistence with Routing ===== /// public new async Task FunctionDumpAsync( CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionDumpAsync()); - } + => await Command(Request.FunctionDumpAsync()); /// public async Task> FunctionDumpAsync( Route route, CancellationToken cancellationToken = default) - { - return await Command(Request.FunctionDumpAsync().ToClusterValue(route), route); - } + => await Command(Request.FunctionDumpAsync().ToClusterValue(), route); /// public new async Task FunctionRestoreAsync( byte[] payload, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionRestoreAsync(payload, null), Route.AllPrimaries); - } + => _ = await Command(Request.FunctionRestoreAsync(payload, null)); /// public async Task FunctionRestoreAsync( byte[] payload, Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionRestoreAsync(payload, null), route); - } + => _ = await Command(Request.FunctionRestoreAsync(payload, null), route); /// public new async Task FunctionRestoreAsync( byte[] payload, FunctionRestorePolicy policy, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionRestoreAsync(payload, policy), Route.AllPrimaries); - } + => _ = await Command(Request.FunctionRestoreAsync(payload, policy)); /// public async Task FunctionRestoreAsync( @@ -240,7 +190,5 @@ public async Task FunctionRestoreAsync( FunctionRestorePolicy policy, Route route, CancellationToken cancellationToken = default) - { - _ = await Command(Request.FunctionRestoreAsync(payload, policy), route); - } + => _ = await Command(Request.FunctionRestoreAsync(payload, policy), route); } diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs index 7f59397cd..a64a637ba 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs @@ -7,60 +7,57 @@ namespace Valkey.Glide; -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public partial class GlideClusterClient { /// public Task> BackgroundSaveAsync() - => BackgroundSaveAsync(AllPrimaries); + => Command(Request.BackgroundSaveAsync().ToClusterValue()); /// public Task> BackgroundSaveAsync(Route route) - => Command(Request.BackgroundSaveAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.BackgroundSaveAsync().ToClusterValue(), route); /// public Task> BackgroundSaveCancelAsync() - => BackgroundSaveCancelAsync(AllPrimaries); + => Command(Request.BackgroundSaveCancelAsync().ToClusterValue()); /// public Task> BackgroundSaveCancelAsync(Route route) - => Command(Request.BackgroundSaveCancelAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.BackgroundSaveCancelAsync().ToClusterValue(), route); /// public Task> BackgroundSaveScheduleAsync() - => BackgroundSaveScheduleAsync(AllPrimaries); + => Command(Request.BackgroundSaveScheduleAsync().ToClusterValue()); /// public Task> BackgroundSaveScheduleAsync(Route route) - => Command(Request.BackgroundSaveScheduleAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.BackgroundSaveScheduleAsync().ToClusterValue(), route); /// public Task> BgRewriteAofAsync() - => BgRewriteAofAsync(AllPrimaries); + => Command(Request.BgRewriteAofAsync().ToClusterValue()); /// public Task> BgRewriteAofAsync(Route route) - => Command(Request.BgRewriteAofAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.BgRewriteAofAsync().ToClusterValue(), route); + // TODO #495: Remove method; consolidate single-value version into BaseClient. /// + [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. This method will be removed.")] public async Task[]>> ConfigGetAsync(ValkeyValue pattern = default) - => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(false), AllPrimaries); + => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(), AllPrimaries); /// public async Task[]>> ConfigGetAsync(ValkeyValue pattern, Route route) - => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task[]> ConfigGetAsync(IEnumerable patterns) - => await Command(Request.ConfigGetAsync(patterns)); + => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(), route); /// public async Task[]>> ConfigGetAsync(IEnumerable patterns, Route route) - => await Command(Request.ConfigGetAsync(patterns).ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.ConfigGetAsync(patterns).ToClusterValue(), route); /// public async Task ConfigResetStatisticsAsync() - => _ = await Command(Request.ConfigResetStatisticsAsync(), AllPrimaries); + => _ = await Command(Request.ConfigResetStatisticsAsync()); /// public async Task ConfigResetStatisticsAsync(Route route) @@ -68,7 +65,7 @@ public async Task ConfigResetStatisticsAsync(Route route) /// public async Task ConfigRewriteAsync() - => _ = await Command(Request.ConfigRewriteAsync(), Route.Random); + => _ = await Command(Request.ConfigRewriteAsync()); /// public async Task ConfigRewriteAsync(Route route) @@ -76,38 +73,33 @@ public async Task ConfigRewriteAsync(Route route) /// public async Task ConfigSetAsync(ValkeyValue setting, ValkeyValue value) - => _ = await Command(Request.ConfigSetAsync(setting, value), AllPrimaries); + => _ = await Command(Request.ConfigSetAsync(setting, value)); /// public async Task ConfigSetAsync(ValkeyValue setting, ValkeyValue value, Route route) => _ = await Command(Request.ConfigSetAsync(setting, value), route); - /// - public override async Task ConfigSetAsync(IDictionary parameters) - => _ = await Command(Request.ConfigSetAsync(parameters), AllPrimaries); - /// public async Task ConfigSetAsync(IDictionary parameters, Route route) => _ = await Command(Request.ConfigSetAsync(parameters), route); /// public async Task DatabaseSizeAsync() - => await DatabaseSizeAsync(AllPrimaries); + { + ClusterValue result = await Command(Request.DatabaseSizeAsync().ToClusterValue()); + return result.HasMultiData ? result.MultiValue.Values.Sum() : result.SingleValue; + } /// public async Task DatabaseSizeAsync(Route route) { - ClusterValue result = await Command(Request.DatabaseSizeAsync().ToClusterValue(false), route); + ClusterValue result = await Command(Request.DatabaseSizeAsync().ToClusterValue(), route); return result.HasMultiData ? result.MultiValue.Values.Sum() : result.SingleValue; } /// public async Task FlushAllDatabasesAsync() - => await FlushAllDatabasesAsync(AllPrimaries); - - /// - public override async Task FlushAllDatabasesAsync(FlushMode mode) - => await FlushAllDatabasesAsync(mode, AllPrimaries); + => _ = await Command(Request.FlushAllDatabasesAsync()); /// public async Task FlushAllDatabasesAsync(Route route) @@ -119,11 +111,7 @@ public async Task FlushAllDatabasesAsync(FlushMode mode, Route route) /// public async Task FlushDatabaseAsync() - => await FlushDatabaseAsync(AllPrimaries); - - /// - public override async Task FlushDatabaseAsync(FlushMode mode) - => await FlushDatabaseAsync(mode, AllPrimaries); + => _ = await Command(Request.FlushDatabaseAsync()); /// public async Task FlushDatabaseAsync(Route route) @@ -145,51 +133,38 @@ public async Task> InfoAsync(IEnumerable public async Task> InfoAsync(IEnumerable sections, Route route) - => await Command(Request.Info([.. sections]).ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.Info([.. sections]).ToClusterValue(), route); /// public async Task> LastSaveAsync() { - var result = await Command(Request.LastSaveAsync().ToClusterValue(false), Route.Random); + var result = await Command(Request.LastSaveAsync().ToClusterValue()); if (result.HasMultiData) { return result.MultiValue; } - // If we got a single value, create a dictionary with a single entry return new Dictionary { ["single_node"] = result.SingleValue }; } /// public Task> LastSaveAsync(Route route) - => Command(Request.LastSaveAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.LastSaveAsync().ToClusterValue(), route); /// public async Task> LatencyHistoryAsync(ValkeyValue @event) - => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue(false), AllPrimaries); + => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue()); /// public async Task> LatencyHistoryAsync(ValkeyValue @event, Route route) - => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue(), route); /// public async Task> LatencyLatestAsync() - => await Command(Request.LatencyLatestAsync().ToClusterValue(false), AllPrimaries); + => await Command(Request.LatencyLatestAsync().ToClusterValue()); /// public async Task> LatencyLatestAsync(Route route) - => await Command(Request.LatencyLatestAsync().ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task LatencyResetAsync() - => await Command(Request.LatencyResetAsync([]), AllPrimaries); - - /// - public override async Task LatencyResetAsync(ValkeyValue @event) - => await Command(Request.LatencyResetAsync([@event]), AllPrimaries); - - /// - public override async Task LatencyResetAsync(IEnumerable events) - => await Command(Request.LatencyResetAsync(events), AllPrimaries); + => await Command(Request.LatencyLatestAsync().ToClusterValue(), route); /// public async Task LatencyResetAsync(Route route) @@ -203,52 +178,45 @@ public async Task LatencyResetAsync(ValkeyValue @event, Route route) public async Task LatencyResetAsync(IEnumerable events, Route route) => await Command(Request.LatencyResetAsync(events), route); - /// - // TODO #475: Move to BaseClient, return Task instead of Task>. [Obsolete("This method will be updated to return Task in future. Use LolwutAsync(Route.Random) instead")] public async Task> LolwutAsync() { - ClusterValue result = await Command(Request.LolwutAsync().ToClusterValue(false), Route.Random); + ClusterValue result = await Command(Request.LolwutAsync().ToClusterValue()); if (result.HasMultiData) { return result.MultiValue; } - // If we got a single value, create a dictionary with a single entry return new Dictionary { ["single_node"] = result.SingleValue }; } - /// - public override async Task LolwutAsync(LolwutOptions options) - => await Command(Request.LolwutAsync(options), Route.Random); - /// public async Task> LolwutAsync(Route route) - => await Command(Request.LolwutAsync().ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.LolwutAsync().ToClusterValue(), route); /// public async Task> LolwutAsync(LolwutOptions options, Route route) - => await Command(Request.LolwutAsync(options).ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.LolwutAsync(options).ToClusterValue(), route); /// public async Task> MemoryDoctorAsync() - => await Command(Request.MemoryDoctorAsync().ToClusterValue(false), AllPrimaries); + => await Command(Request.MemoryDoctorAsync().ToClusterValue()); /// public async Task> MemoryDoctorAsync(Route route) - => await Command(Request.MemoryDoctorAsync().ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.MemoryDoctorAsync().ToClusterValue(), route); /// public async Task> MemoryMallocStatsAsync() - => await Command(Request.MemoryMallocStatsAsync().ToClusterValue(false), AllPrimaries); + => await Command(Request.MemoryMallocStatsAsync().ToClusterValue()); /// public async Task> MemoryMallocStatsAsync(Route route) - => await Command(Request.MemoryMallocStatsAsync().ToClusterValue(route is SingleNodeRoute), route); + => await Command(Request.MemoryMallocStatsAsync().ToClusterValue(), route); /// public async Task MemoryPurgeAsync() - => _ = await Command(Request.MemoryPurgeAsync(), AllPrimaries); + => _ = await Command(Request.MemoryPurgeAsync()); /// public async Task MemoryPurgeAsync(Route route) @@ -256,15 +224,11 @@ public async Task MemoryPurgeAsync(Route route) /// public async Task> MemoryStatsAsync() - => await Command(Request.MemoryStatsAsync().ToClusterValue(false), AllPrimaries); + => await Command(Request.MemoryStatsAsync().ToClusterValue()); /// public async Task> MemoryStatsAsync(Route route) - => await Command(Request.MemoryStatsAsync().ToClusterValue(route is SingleNodeRoute), route); - - /// - public override async Task SaveAsync() - => _ = await Command(Request.SaveAsync(), AllPrimaries); + => await Command(Request.MemoryStatsAsync().ToClusterValue(), route); /// public async Task SaveAsync(Route route) @@ -273,16 +237,15 @@ public async Task SaveAsync(Route route) /// public async Task> TimeAsync() { - var result = await Command(Request.TimeAsync().ToClusterValue(false), Route.Random); + var result = await Command(Request.TimeAsync().ToClusterValue()); if (result.HasMultiData) { return result.MultiValue; } - // If we got a single value, create a dictionary with a single entry return new Dictionary { ["single_node"] = result.SingleValue }; } /// public Task> TimeAsync(Route route) - => Command(Request.TimeAsync().ToClusterValue(route is SingleNodeRoute), route); + => Command(Request.TimeAsync().ToClusterValue(), route); } diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.cs b/sources/Valkey.Glide/Client/GlideClusterClient.cs index 5a350378e..2ae3e755b 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.cs @@ -52,6 +52,7 @@ public static async Task CreateClient(ClusterClientConfigura ? throw new RequestException("Retry strategy is not supported for atomic batches (transactions).") : await Batch(batch, raiseOnError, options); + /// protected override async Task GetServerVersionAsync() { @@ -59,8 +60,11 @@ protected override async Task GetServerVersionAsync() { try { - var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]).ToClusterValue(true), Route.Random); - _serverVersion = ParseServerVersion(infoResponse.SingleValue) ?? DefaultServerVersion; + var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]).ToClusterValue()); + string infoString = infoResponse.HasMultiData + ? infoResponse.MultiValue.Values.First() + : infoResponse.SingleValue; + _serverVersion = ParseServerVersion(infoString) ?? DefaultServerVersion; } catch { diff --git a/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs b/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs index 8440a9b4a..7d1db3bf8 100644 --- a/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs +++ b/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs @@ -38,6 +38,8 @@ public partial interface IGlideClusterClient /// /// /// + // TODO #496: Change return type to Task; remove AllPrimaries default. + [Obsolete("Return type will change to Task. See #496.")] Task> ScriptInvokeAsync( Script script, ClusterScriptOptions options, @@ -379,6 +381,8 @@ Task FunctionKillAsync( /// /// /// + // TODO #495: Remove method; consolidate single-value version into BaseClient. + [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. This method will be removed in 2.0.")] Task> FunctionListAsync( FunctionListOptions? options = null, CancellationToken cancellationToken = default); diff --git a/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs index fc64c30b2..0b49077ba 100644 --- a/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs @@ -190,6 +190,8 @@ public partial interface IGlideClusterClient /// /// /// + // TODO #495: Remove method; consolidate single-value version into BaseClient. + [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. This method will be removed in 2.0.")] Task[]>> ConfigGetAsync(ValkeyValue pattern = default); /// diff --git a/sources/Valkey.Glide/Internals/Cmd.cs b/sources/Valkey.Glide/Internals/Cmd.cs index 7485713f2..0cd15de0b 100644 --- a/sources/Valkey.Glide/Internals/Cmd.cs +++ b/sources/Valkey.Glide/Internals/Cmd.cs @@ -77,16 +77,12 @@ public Cmd, Dictionary> ToMultiNodeVa /// /// Convert a command to one which handles a . /// - /// Whether current command call returns a single value. - public Cmd> ToClusterValue(bool isSingleValue) - => new(Request, ArgsArray.Args, IsNullable, ResponseConverters.MakeClusterValueHandler(Converter, isSingleValue), AllowConverterToHandleNull); - - /// - /// Convert a command to one which handles a . - /// - /// The route to determine if this is a single-node operation. - public Cmd> ToClusterValue(Route route) - => ToClusterValue(route is Route.SingleNodeRoute); + public Cmd> ToClusterValue() + => new(Request, ArgsArray.Args, IsNullable, value => value is Dictionary dict + ? ClusterValue.OfMultiValue(dict.ConvertValues(Converter)) + : value is Dictionary stringDict + ? ClusterValue.OfMultiValue(stringDict.ConvertValues(Converter)) + : ClusterValue.OfSingleValue(Converter((R)value)), AllowConverterToHandleNull); /// /// Get full command line including command name. diff --git a/sources/Valkey.Glide/Internals/Request.ConnectionManagement.cs b/sources/Valkey.Glide/Internals/Request.ConnectionManagement.cs index c64bbf88b..2bce3d0d8 100644 --- a/sources/Valkey.Glide/Internals/Request.ConnectionManagement.cs +++ b/sources/Valkey.Glide/Internals/Request.ConnectionManagement.cs @@ -17,9 +17,6 @@ internal partial class Request public static Cmd ClientGetName() => ToValkeyValue(RequestType.ClientGetName, [], isNullable: true); - public static Cmd> ClientGetName(Route route) - => ClientGetName().ToClusterValue(route); - public static Cmd ClientId() => Simple(RequestType.ClientId, []); diff --git a/sources/Valkey.Glide/Internals/ResponseConverters.cs b/sources/Valkey.Glide/Internals/ResponseConverters.cs index 3f904f5df..5a70f3bd6 100644 --- a/sources/Valkey.Glide/Internals/ResponseConverters.cs +++ b/sources/Valkey.Glide/Internals/ResponseConverters.cs @@ -13,22 +13,6 @@ internal class ResponseConverters ? ClusterValue.OfSingleValue(data) : ClusterValue.OfMultiValue((Dictionary)data)); - /// - /// Process and convert a server response that may be a multi-node response. - /// - /// GLIDE's return type per node. - /// Command's return type. - /// Function to convert to . - /// Whether current command call returns a single value. - public static Func> MakeClusterValueHandler(Func converter, bool isSingleValue) - => isSingleValue - ? value => ClusterValue.OfSingleValue(converter((R)value)) - : value => value is Dictionary dict - ? ClusterValue.OfMultiValue(dict.ConvertValues(converter)) - : value is Dictionary stringDict - ? ClusterValue.OfMultiValue(stringDict.ConvertValues(converter)) - : ClusterValue.OfSingleValue(converter((R)value)); // In case the nodes combine multiple results to a single result - /// /// Process and convert a cluster multi-node response. /// diff --git a/sources/Valkey.Glide/scripting/ClusterScriptOptions.cs b/sources/Valkey.Glide/scripting/ClusterScriptOptions.cs index dc2f41d6d..6f7eae1c9 100644 --- a/sources/Valkey.Glide/scripting/ClusterScriptOptions.cs +++ b/sources/Valkey.Glide/scripting/ClusterScriptOptions.cs @@ -5,6 +5,8 @@ namespace Valkey.Glide; /// /// Options for cluster script execution with routing support. /// +[Obsolete("This class will be removed and replaced with use of ScriptOptions. See #496.")] +// TODO #496: Remove class; use BaseClient.ScriptInvokeAsync(Script, ScriptOptions) for key-based routing. public sealed class ClusterScriptOptions { /// diff --git a/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs b/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs index 36ffa80cf..53c38c1b7 100644 --- a/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs @@ -266,6 +266,8 @@ public async Task TestEcho_BinaryData_WithRoute(GlideClusterClient client) [MemberData(nameof(Config.TestClusterClients), MemberType = typeof(TestConfiguration))] public async Task ConfigGetAsync_ReturnsConfigurationPerNode(GlideClusterClient client) { + // TODO #495: Update for single node method. +#pragma warning disable CS0618 // Test getting all configuration from all nodes var allConfig = await client.ConfigGetAsync("*"); Assert.True(allConfig.HasMultiData); @@ -338,6 +340,7 @@ public async Task ConfigGetAsync_ReturnsConfigurationPerNode(GlideClusterClient { Assert.Empty(nodeConfig); } +#pragma warning restore CS0618 } [Theory(DisableDiscoveryEnumeration = true)] diff --git a/tests/Valkey.Glide.IntegrationTests/ServerManagementCommandTests.cs b/tests/Valkey.Glide.IntegrationTests/ServerManagementCommandTests.cs index 67cecfce4..fd2eb864c 100644 --- a/tests/Valkey.Glide.IntegrationTests/ServerManagementCommandTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/ServerManagementCommandTests.cs @@ -822,7 +822,10 @@ private async Task TriggerLatencySpikeAsync(bool isCluster) KeyValuePair[] prevConfigs; if (isCluster) { + // TODO #495: Update to new method and remove pragma +#pragma warning disable CS0618 var prev = await ClusterClient.ConfigGetAsync(latencyThresholdParam); +#pragma warning restore CS0618 prevConfigs = prev.HasSingleData ? prev.SingleValue : prev.MultiValue.Values.First(); } else diff --git a/tests/Valkey.Glide.UnitTests/ClusterScriptOptionsTests.cs b/tests/Valkey.Glide.UnitTests/ClusterScriptOptionsTests.cs index 330073521..6242b5f14 100644 --- a/tests/Valkey.Glide.UnitTests/ClusterScriptOptionsTests.cs +++ b/tests/Valkey.Glide.UnitTests/ClusterScriptOptionsTests.cs @@ -1,5 +1,8 @@ // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 +// TODO #496: Remove test file when ClusterScriptOptions is deleted. +#pragma warning disable CS0618 + namespace Valkey.Glide.UnitTests; public class ClusterScriptOptionsTests diff --git a/tests/Valkey.Glide.UnitTests/PubSubFFIIntegrationTests.cs b/tests/Valkey.Glide.UnitTests/PubSubFFIIntegrationTests.cs index bcf489062..b269422e6 100644 --- a/tests/Valkey.Glide.UnitTests/PubSubFFIIntegrationTests.cs +++ b/tests/Valkey.Glide.UnitTests/PubSubFFIIntegrationTests.cs @@ -1,7 +1,5 @@ // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 -using Valkey.Glide.Commands.Options; - namespace Valkey.Glide.UnitTests; public class PubSubFFIIntegrationTests @@ -208,26 +206,5 @@ protected override Task GetServerVersionAsync() // Mock implementations internal override void HandlePubSubMessage(PubSubMessage message) { } - - // Mock abstract methods. - public override Task ClientPauseAsync(TimeSpan timeout) => Task.CompletedTask; - public override Task ClientPauseWriteAsync(TimeSpan timeout) => Task.CompletedTask; - public override Task ClientUnpauseAsync() => Task.CompletedTask; - public override Task ConfigSetAsync(IDictionary parameters) => Task.CompletedTask; - public override Task FlushAllDatabasesAsync(FlushMode mode) => Task.CompletedTask; - public override Task FlushDatabaseAsync(FlushMode mode) => Task.CompletedTask; - public override Task SaveAsync() => Task.CompletedTask; - public override Task SelectAsync(long index) => Task.CompletedTask; - public override Task ClientTrackingInfoAsync() => Task.FromResult(new ClientTrackingInfo { Flags = new HashSet(), Redirect = -1, Prefixes = new HashSet() }); - public override Task[]> ConfigGetAsync(IEnumerable patterns) => Task.FromResult(Array.Empty>()); - public override Task ClientIdAsync() => Task.FromResult(0L); - public override Task LatencyResetAsync() => Task.FromResult(0L); - public override Task LatencyResetAsync(IEnumerable events) => Task.FromResult(0L); - public override Task LatencyResetAsync(ValkeyValue eventName) => Task.FromResult(0L); - public override Task LolwutAsync(LolwutOptions options) => Task.FromResult(string.Empty); - public override Task ClientGetNameAsync() => Task.FromResult(ValkeyValue.Null); - public override Task EchoAsync(ValkeyValue message) => Task.FromResult(message); - public override Task PingAsync() => Task.FromResult((ValkeyValue)"PONG"); - public override Task PingAsync(ValkeyValue message) => Task.FromResult(message); } } diff --git a/tests/Valkey.Glide.UnitTests/PubSubThreadSafetyTests.cs b/tests/Valkey.Glide.UnitTests/PubSubThreadSafetyTests.cs index 7bc7ab303..c99916e7f 100644 --- a/tests/Valkey.Glide.UnitTests/PubSubThreadSafetyTests.cs +++ b/tests/Valkey.Glide.UnitTests/PubSubThreadSafetyTests.cs @@ -2,8 +2,6 @@ using System.Collections.Concurrent; -using Valkey.Glide.Commands.Options; - namespace Valkey.Glide.UnitTests; /// @@ -227,26 +225,5 @@ protected override Task GetServerVersionAsync() _serverVersion = new Version(7, 2, 0); return Task.FromResult(_serverVersion); } - - // Mock abstract methods. - public override Task ClientPauseAsync(TimeSpan timeout) => Task.CompletedTask; - public override Task ClientPauseWriteAsync(TimeSpan timeout) => Task.CompletedTask; - public override Task ClientUnpauseAsync() => Task.CompletedTask; - public override Task ConfigSetAsync(IDictionary parameters) => Task.CompletedTask; - public override Task FlushAllDatabasesAsync(FlushMode mode) => Task.CompletedTask; - public override Task FlushDatabaseAsync(FlushMode mode) => Task.CompletedTask; - public override Task SaveAsync() => Task.CompletedTask; - public override Task SelectAsync(long index) => Task.CompletedTask; - public override Task ClientTrackingInfoAsync() => Task.FromResult(new ClientTrackingInfo { Flags = new HashSet(), Redirect = -1, Prefixes = new HashSet() }); - public override Task[]> ConfigGetAsync(IEnumerable patterns) => Task.FromResult(Array.Empty>()); - public override Task ClientIdAsync() => Task.FromResult(0L); - public override Task LatencyResetAsync() => Task.FromResult(0L); - public override Task LatencyResetAsync(IEnumerable events) => Task.FromResult(0L); - public override Task LatencyResetAsync(ValkeyValue @event) => Task.FromResult(0L); - public override Task LolwutAsync(LolwutOptions options) => Task.FromResult(string.Empty); - public override Task ClientGetNameAsync() => Task.FromResult(ValkeyValue.Null); - public override Task EchoAsync(ValkeyValue message) => Task.FromResult(message); - public override Task PingAsync() => Task.FromResult((ValkeyValue)"PONG"); - public override Task PingAsync(ValkeyValue message) => Task.FromResult(message); } } From 308de388e6682482c79eeb347c1c729becf5ed10 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 30 Jul 2026 19:33:35 -0700 Subject: [PATCH 2/4] Update Obselete messages for consistency Signed-off-by: currantw --- ...sterClient.ScriptingAndFunctionCommands.cs | 4 ++-- ...eClusterClient.ServerManagementCommands.cs | 22 +++++-------------- ...sterClient.ScriptingAndFunctionCommands.cs | 2 +- ...eClusterClient.ServerManagementCommands.cs | 2 +- 4 files changed, 9 insertions(+), 21 deletions(-) diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs index 42e2a66cf..87214312b 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs @@ -10,7 +10,7 @@ public sealed partial class GlideClusterClient /// // TODO #496: Change return type to Task; remove AllPrimaries default. - [Obsolete("Return type will change to Task in 2.0. See #496.")] + [Obsolete("Return type will change to Task. See #496.")] public async Task> ScriptInvokeAsync( Script script, ClusterScriptOptions options, @@ -132,7 +132,7 @@ public async Task FunctionKillAsync( /// // TODO #495: Remove method; consolidate single-value version into BaseClient. - [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. This method will be removed in 2.0.")] + [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. See #495.")] public async Task> FunctionListAsync( FunctionListOptions? options = null, CancellationToken cancellationToken = default) diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs index a64a637ba..967520029 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs @@ -43,7 +43,7 @@ public Task> BgRewriteAofAsync(Route route) // TODO #495: Remove method; consolidate single-value version into BaseClient. /// - [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. This method will be removed.")] + [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. See #495.")] public async Task[]>> ConfigGetAsync(ValkeyValue pattern = default) => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(), AllPrimaries); @@ -139,11 +139,7 @@ public async Task> InfoAsync(IEnumerable> LastSaveAsync() { var result = await Command(Request.LastSaveAsync().ToClusterValue()); - if (result.HasMultiData) - { - return result.MultiValue; - } - return new Dictionary { ["single_node"] = result.SingleValue }; + return result.HasMultiData ? result.MultiValue : new Dictionary { ["single_node"] = result.SingleValue }; } /// @@ -182,12 +178,8 @@ public async Task LatencyResetAsync(IEnumerable events, Route [Obsolete("This method will be updated to return Task in future. Use LolwutAsync(Route.Random) instead")] public async Task> LolwutAsync() { - ClusterValue result = await Command(Request.LolwutAsync().ToClusterValue()); - if (result.HasMultiData) - { - return result.MultiValue; - } - return new Dictionary { ["single_node"] = result.SingleValue }; + var result = await Command(Request.LolwutAsync().ToClusterValue()); + return result.HasMultiData ? result.MultiValue : new Dictionary { ["single_node"] = result.SingleValue }; } /// @@ -238,11 +230,7 @@ public async Task SaveAsync(Route route) public async Task> TimeAsync() { var result = await Command(Request.TimeAsync().ToClusterValue()); - if (result.HasMultiData) - { - return result.MultiValue; - } - return new Dictionary { ["single_node"] = result.SingleValue }; + return result.HasMultiData ? result.MultiValue : new Dictionary { ["single_node"] = result.SingleValue }; } /// diff --git a/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs b/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs index 7d1db3bf8..4e31e3cb0 100644 --- a/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs +++ b/sources/Valkey.Glide/Client/IGlideClusterClient.ScriptingAndFunctionCommands.cs @@ -382,7 +382,7 @@ Task FunctionKillAsync( /// /// // TODO #495: Remove method; consolidate single-value version into BaseClient. - [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. This method will be removed in 2.0.")] + [Obsolete("Use FunctionListAsync(FunctionListOptions?, Route) instead. See #495.")] Task> FunctionListAsync( FunctionListOptions? options = null, CancellationToken cancellationToken = default); diff --git a/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs index 0b49077ba..3562e0b6b 100644 --- a/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/IGlideClusterClient.ServerManagementCommands.cs @@ -191,7 +191,7 @@ public partial interface IGlideClusterClient /// /// // TODO #495: Remove method; consolidate single-value version into BaseClient. - [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. This method will be removed in 2.0.")] + [Obsolete("Use ConfigGetAsync(ValkeyValue, Route) instead. See #495.")] Task[]>> ConfigGetAsync(ValkeyValue pattern = default); /// From 39453e11a704d3a12419b236509d5273f4b3440f Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 30 Jul 2026 20:58:37 -0700 Subject: [PATCH 3/4] fix: restore route-aware ToClusterValue(Route) to handle single-node responses When a SingleNodeRoute is used, glide-core returns a raw value (not wrapped in a multi-node dictionary). Commands whose raw response type is itself a dictionary (e.g. MemoryStats) were incorrectly detected as multi-node responses. ToClusterValue(Route) skips dictionary detection for single-node routes. Signed-off-by: Taylor Curran Signed-off-by: currantw --- ...sterClient.ConnectionManagementCommands.cs | 8 ++--- ...sterClient.ScriptingAndFunctionCommands.cs | 22 ++++++------ ...eClusterClient.ServerManagementCommands.cs | 34 +++++++++---------- sources/Valkey.Glide/Internals/Cmd.cs | 8 +++++ 4 files changed, 40 insertions(+), 32 deletions(-) diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs index 577e374d9..f26aad6e7 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs @@ -9,19 +9,19 @@ public partial class GlideClusterClient { /// public async Task> ClientGetNameAsync(Route route) - => await Command(Request.ClientGetName().ToClusterValue(), route); + => await Command(Request.ClientGetName().ToClusterValue(route), route); /// public async Task> ClientIdAsync(Route route) - => await Command(Request.ClientId().ToClusterValue(), route); + => await Command(Request.ClientId().ToClusterValue(route), route); /// public async Task> ClientTrackingInfoAsync(Route route) - => await Command(Request.ClientTrackingInfo().ToClusterValue(), route); + => await Command(Request.ClientTrackingInfo().ToClusterValue(route), route); /// public async Task> EchoAsync(ValkeyValue message, Route route) - => await Command(Request.Echo(message).ToClusterValue(), route); + => await Command(Request.Echo(message).ToClusterValue(route), route); /// public async Task PingAsync(Route route) diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs index 87214312b..dea802386 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ScriptingAndFunctionCommands.cs @@ -17,7 +17,7 @@ public async Task> ScriptInvokeAsync( CancellationToken cancellationToken = default) { Route route = options.Route ?? Route.AllPrimaries; - return await Command(Request.EvalShaAsync(script.Hash, null, options.Args).ToClusterValue(), route); + return await Command(Request.EvalShaAsync(script.Hash, null, options.Args).ToClusterValue(route), route); } // ===== Script Management with Routing ===== @@ -27,7 +27,7 @@ public async Task> ScriptExistsAsync( IEnumerable sha1Hashes, Route route, CancellationToken cancellationToken = default) - => await Command(Request.ScriptExistsAsync([.. sha1Hashes]).ToClusterValue(), route); + => await Command(Request.ScriptExistsAsync([.. sha1Hashes]).ToClusterValue(route), route); /// public async Task ScriptFlushAsync( @@ -55,7 +55,7 @@ public async Task> FCallAsync( string function, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FCallAsync(function, null, null).ToClusterValue(), route); + => await Command(Request.FCallAsync(function, null, null).ToClusterValue(route), route); /// public async Task> FCallAsync( @@ -63,14 +63,14 @@ public async Task> FCallAsync( IEnumerable args, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FCallAsync(function, null, [.. args]).ToClusterValue(), route); + => await Command(Request.FCallAsync(function, null, [.. args]).ToClusterValue(route), route); /// public async Task> FCallReadOnlyAsync( string function, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FCallReadOnlyAsync(function, null, null).ToClusterValue(), route); + => await Command(Request.FCallReadOnlyAsync(function, null, null).ToClusterValue(route), route); /// public async Task> FCallReadOnlyAsync( @@ -78,7 +78,7 @@ public async Task> FCallReadOnlyAsync( IEnumerable args, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FCallReadOnlyAsync(function, null, [.. args]).ToClusterValue(), route); + => await Command(Request.FCallReadOnlyAsync(function, null, [.. args]).ToClusterValue(route), route); // ===== Function Management with Routing ===== @@ -87,7 +87,7 @@ public async Task> FunctionLoadAsync( string libraryCode, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FunctionLoadAsync(libraryCode, false).ToClusterValue(), route); + => await Command(Request.FunctionLoadAsync(libraryCode, false).ToClusterValue(route), route); /// public async Task> FunctionLoadAsync( @@ -95,7 +95,7 @@ public async Task> FunctionLoadAsync( bool replace, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FunctionLoadAsync(libraryCode, replace).ToClusterValue(), route); + => await Command(Request.FunctionLoadAsync(libraryCode, replace).ToClusterValue(route), route); /// public async Task FunctionDeleteAsync( @@ -143,13 +143,13 @@ public async Task> FunctionListAsync( FunctionListOptions? options, Route route, CancellationToken cancellationToken = default) - => await Command(Request.FunctionListAsync(options).ToClusterValue(), route); + => await Command(Request.FunctionListAsync(options).ToClusterValue(route), route); /// public async Task> FunctionStatsAsync( Route route, CancellationToken cancellationToken = default) - => await Command(Request.FunctionStatsAsync().ToClusterValue(), route); + => await Command(Request.FunctionStatsAsync().ToClusterValue(route), route); // ===== Function Persistence with Routing ===== @@ -162,7 +162,7 @@ public async Task> FunctionStatsAsync( public async Task> FunctionDumpAsync( Route route, CancellationToken cancellationToken = default) - => await Command(Request.FunctionDumpAsync().ToClusterValue(), route); + => await Command(Request.FunctionDumpAsync().ToClusterValue(route), route); /// public new async Task FunctionRestoreAsync( diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs index 967520029..422604317 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ServerManagementCommands.cs @@ -15,7 +15,7 @@ public Task> BackgroundSaveAsync() /// public Task> BackgroundSaveAsync(Route route) - => Command(Request.BackgroundSaveAsync().ToClusterValue(), route); + => Command(Request.BackgroundSaveAsync().ToClusterValue(route), route); /// public Task> BackgroundSaveCancelAsync() @@ -23,7 +23,7 @@ public Task> BackgroundSaveCancelAsync() /// public Task> BackgroundSaveCancelAsync(Route route) - => Command(Request.BackgroundSaveCancelAsync().ToClusterValue(), route); + => Command(Request.BackgroundSaveCancelAsync().ToClusterValue(route), route); /// public Task> BackgroundSaveScheduleAsync() @@ -31,7 +31,7 @@ public Task> BackgroundSaveScheduleAsync() /// public Task> BackgroundSaveScheduleAsync(Route route) - => Command(Request.BackgroundSaveScheduleAsync().ToClusterValue(), route); + => Command(Request.BackgroundSaveScheduleAsync().ToClusterValue(route), route); /// public Task> BgRewriteAofAsync() @@ -39,7 +39,7 @@ public Task> BgRewriteAofAsync() /// public Task> BgRewriteAofAsync(Route route) - => Command(Request.BgRewriteAofAsync().ToClusterValue(), route); + => Command(Request.BgRewriteAofAsync().ToClusterValue(route), route); // TODO #495: Remove method; consolidate single-value version into BaseClient. /// @@ -49,11 +49,11 @@ public async Task[]>> ConfigGetAsync(V /// public async Task[]>> ConfigGetAsync(ValkeyValue pattern, Route route) - => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(), route); + => await Command(Request.ConfigGetAsync(pattern).ToClusterValue(route), route); /// public async Task[]>> ConfigGetAsync(IEnumerable patterns, Route route) - => await Command(Request.ConfigGetAsync(patterns).ToClusterValue(), route); + => await Command(Request.ConfigGetAsync(patterns).ToClusterValue(route), route); /// public async Task ConfigResetStatisticsAsync() @@ -93,7 +93,7 @@ public async Task DatabaseSizeAsync() /// public async Task DatabaseSizeAsync(Route route) { - ClusterValue result = await Command(Request.DatabaseSizeAsync().ToClusterValue(), route); + ClusterValue result = await Command(Request.DatabaseSizeAsync().ToClusterValue(route), route); return result.HasMultiData ? result.MultiValue.Values.Sum() : result.SingleValue; } @@ -133,7 +133,7 @@ public async Task> InfoAsync(IEnumerable public async Task> InfoAsync(IEnumerable sections, Route route) - => await Command(Request.Info([.. sections]).ToClusterValue(), route); + => await Command(Request.Info([.. sections]).ToClusterValue(route), route); /// public async Task> LastSaveAsync() @@ -144,7 +144,7 @@ public async Task> LastSaveAsync() /// public Task> LastSaveAsync(Route route) - => Command(Request.LastSaveAsync().ToClusterValue(), route); + => Command(Request.LastSaveAsync().ToClusterValue(route), route); /// public async Task> LatencyHistoryAsync(ValkeyValue @event) @@ -152,7 +152,7 @@ public async Task> LatencyHistoryAsync(ValkeyValue /// public async Task> LatencyHistoryAsync(ValkeyValue @event, Route route) - => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue(), route); + => await Command(Request.LatencyHistoryAsync(@event).ToClusterValue(route), route); /// public async Task> LatencyLatestAsync() @@ -160,7 +160,7 @@ public async Task> LatencyLatestAsync() /// public async Task> LatencyLatestAsync(Route route) - => await Command(Request.LatencyLatestAsync().ToClusterValue(), route); + => await Command(Request.LatencyLatestAsync().ToClusterValue(route), route); /// public async Task LatencyResetAsync(Route route) @@ -184,11 +184,11 @@ public async Task> LolwutAsync() /// public async Task> LolwutAsync(Route route) - => await Command(Request.LolwutAsync().ToClusterValue(), route); + => await Command(Request.LolwutAsync().ToClusterValue(route), route); /// public async Task> LolwutAsync(LolwutOptions options, Route route) - => await Command(Request.LolwutAsync(options).ToClusterValue(), route); + => await Command(Request.LolwutAsync(options).ToClusterValue(route), route); /// public async Task> MemoryDoctorAsync() @@ -196,7 +196,7 @@ public async Task> MemoryDoctorAsync() /// public async Task> MemoryDoctorAsync(Route route) - => await Command(Request.MemoryDoctorAsync().ToClusterValue(), route); + => await Command(Request.MemoryDoctorAsync().ToClusterValue(route), route); /// public async Task> MemoryMallocStatsAsync() @@ -204,7 +204,7 @@ public async Task> MemoryMallocStatsAsync() /// public async Task> MemoryMallocStatsAsync(Route route) - => await Command(Request.MemoryMallocStatsAsync().ToClusterValue(), route); + => await Command(Request.MemoryMallocStatsAsync().ToClusterValue(route), route); /// public async Task MemoryPurgeAsync() @@ -220,7 +220,7 @@ public async Task> MemoryStatsAsync() /// public async Task> MemoryStatsAsync(Route route) - => await Command(Request.MemoryStatsAsync().ToClusterValue(), route); + => await Command(Request.MemoryStatsAsync().ToClusterValue(route), route); /// public async Task SaveAsync(Route route) @@ -235,5 +235,5 @@ public async Task> TimeAsync() /// public Task> TimeAsync(Route route) - => Command(Request.TimeAsync().ToClusterValue(), route); + => Command(Request.TimeAsync().ToClusterValue(route), route); } diff --git a/sources/Valkey.Glide/Internals/Cmd.cs b/sources/Valkey.Glide/Internals/Cmd.cs index 0cd15de0b..870f203ad 100644 --- a/sources/Valkey.Glide/Internals/Cmd.cs +++ b/sources/Valkey.Glide/Internals/Cmd.cs @@ -84,6 +84,14 @@ public Cmd> ToClusterValue() ? ClusterValue.OfMultiValue(stringDict.ConvertValues(Converter)) : ClusterValue.OfSingleValue(Converter((R)value)), AllowConverterToHandleNull); + /// + /// Convert a command to one which handles a for the given route. + /// + public Cmd> ToClusterValue(Route route) + => route is Route.SingleNodeRoute + ? new(Request, ArgsArray.Args, IsNullable, value => ClusterValue.OfSingleValue(Converter((R)value)), AllowConverterToHandleNull) + : ToClusterValue(); + /// /// Get full command line including command name. /// From 665926c897ed09ba353731d6c8350cabda54ce52 Mon Sep 17 00:00:00 2001 From: currantw Date: Fri, 31 Jul 2026 11:51:52 -0700 Subject: [PATCH 4/4] Address review comments Signed-off-by: currantw --- .../Client/GlideClusterClient.ConnectionManagementCommands.cs | 1 - sources/Valkey.Glide/Client/GlideClusterClient.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs index f26aad6e7..433114a88 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.ConnectionManagementCommands.cs @@ -4,7 +4,6 @@ namespace Valkey.Glide; -// TODO #462: Consolidate no-route overloads into BaseClient (glide-core default routing matches). public partial class GlideClusterClient { /// diff --git a/sources/Valkey.Glide/Client/GlideClusterClient.cs b/sources/Valkey.Glide/Client/GlideClusterClient.cs index 2ae3e755b..df28a5a74 100644 --- a/sources/Valkey.Glide/Client/GlideClusterClient.cs +++ b/sources/Valkey.Glide/Client/GlideClusterClient.cs @@ -52,7 +52,6 @@ public static async Task CreateClient(ClusterClientConfigura ? throw new RequestException("Retry strategy is not supported for atomic batches (transactions).") : await Batch(batch, raiseOnError, options); - /// protected override async Task GetServerVersionAsync() {