From 2ff8a4bee43b02b9feb9e201b6b4f2fdbf2ec26b Mon Sep 17 00:00:00 2001 From: affonsov <67347924+affonsov@users.noreply.github.com> Date: Mon, 11 May 2026 18:04:38 -0700 Subject: [PATCH 01/16] C#: Support custom socket address resolution Port PRs valkey-io/valkey-glide#5328, #5876, #5890, #5891 to the C# client. Adds an optional AddressResolverDelegate callback to the connection configuration that intercepts and rewrites server host/port pairs before the client establishes connections. Useful for routing through proxies or custom DNS resolution. Implementation follows the direct FFI callback pattern (same as Go), not the socket listener/registry pattern used by Python and Node. Changes: - Update valkey-glide submodule to b4b34a1dc16d (includes core Rust infra from #5328 and FFI layer from #5876) - rust/src/ffi.rs: add address_resolver: None to ConnectionRequest - rust/src/lib.rs: add AddressResolverCallback type, FFIAddressResolver struct implementing redis::AddressResolver, update create_client with optional 5th address_resolver parameter - FFI.methods.cs: add 5th IntPtr addressResolverCallback param to both CreateClientFfi declarations - ConnectionConfiguration.cs: add AddressResolverDelegate public delegate, AddressResolver field, WithAddressResolver() builder method - BaseClient.cs: add AddressResolverAction delegate, _addressResolverDelegate field (GC-safe), wire up in CreateClient - AddressResolverTests.cs: 5 unit tests Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com> --- CHANGELOG.md | 1 + rust/src/ffi.rs | 1 + rust/src/lib.rs | 57 ++++++++++++++++++- sources/Valkey.Glide/Client/BaseClient.cs | 31 +++++++++- .../Valkey.Glide/ConnectionConfiguration.cs | 31 ++++++++++ sources/Valkey.Glide/Internals/FFI.methods.cs | 4 +- .../AddressResolverTests.cs | 56 ++++++++++++++++++ valkey-glide | 2 +- 8 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 tests/Valkey.Glide.UnitTests/AddressResolverTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 16b0d2d15..ca90d23f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changes - Add client-side caching support with TTL-based expiration, LRU/LFU eviction policies, and cache metrics (#330) +- Add custom socket address resolution support via `AddressResolverDelegate` callback (#392) ## 0.10.0 diff --git a/rust/src/ffi.rs b/rust/src/ffi.rs index a070fef7e..d20056771 100644 --- a/rust/src/ffi.rs +++ b/rust/src/ffi.rs @@ -358,6 +358,7 @@ pub(crate) unsafe fn create_connection_request( periodic_checks: None, inflight_requests_limit: None, node_discovery_mode: glide_core::client::NodeDiscoveryMode::default(), + address_resolver: None, }) } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 4e9f1f6fd..23acacc7e 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -97,6 +97,55 @@ pub type FailureCallback = unsafe extern "C-unwind" fn( error_type: RequestErrorType, ) -> (); +/// Callback for resolving server addresses before connection. +/// Returns the resolved port (0 on error), and writes the resolved host into `resolved_host_buf`. +/// `resolved_host_len` is set to the number of bytes written. +pub type AddressResolverCallback = unsafe extern "C-unwind" fn( + host: *const u8, + host_len: usize, + port: u16, + resolved_host_buf: *mut u8, + resolved_host_buf_len: usize, + resolved_host_len: *mut usize, +) -> u16; + +struct FFIAddressResolver { + callback: AddressResolverCallback, +} + +unsafe impl Send for FFIAddressResolver {} +unsafe impl Sync for FFIAddressResolver {} + +impl std::fmt::Debug for FFIAddressResolver { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "FFIAddressResolver") + } +} + +impl redis::AddressResolver for FFIAddressResolver { + fn resolve(&self, host: &str, port: u16) -> (String, u16) { + let mut buf = vec![0u8; 1024]; + let mut len: usize = 0; + let resolved_port = unsafe { + (self.callback)( + host.as_ptr(), + host.len(), + port, + buf.as_mut_ptr(), + buf.len(), + &mut len, + ) + }; + if resolved_port == 0 || len == 0 || len > buf.len() { + return (host.to_string(), port); + } + match std::str::from_utf8(&buf[..len]) { + Ok(h) => (h.to_string(), resolved_port), + Err(_) => (host.to_string(), port), + } + } +} + struct CommandExecutionCore { client: GlideClient, success_callback: SuccessCallback, @@ -160,6 +209,7 @@ pub unsafe extern "C-unwind" fn create_client( success_callback: SuccessCallback, failure_callback: FailureCallback, #[allow(unused_variables)] pubsub_callback: Option, + address_resolver: Option, ) { let mut panic_guard = PanicGuard { panicked: true, @@ -167,7 +217,7 @@ pub unsafe extern "C-unwind" fn create_client( callback_index: 0, }; - let request = match unsafe { create_connection_request(config) } { + let mut request = match unsafe { create_connection_request(config) } { Ok(req) => req, Err(err) => { panic_guard.panicked = false; @@ -178,6 +228,11 @@ pub unsafe extern "C-unwind" fn create_client( } }; + // Set address resolver if provided + if let Some(cb) = address_resolver { + request.address_resolver = Some(std::sync::Arc::new(FFIAddressResolver { callback: cb })); + } + let runtime = Builder::new_multi_thread() .enable_all() .worker_threads(10) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index 75bb6e802..233b270dd 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -201,7 +201,27 @@ protected static async Task CreateClient(BaseClientConfiguration config, F } Message message = client.MessageContainer.GetMessageForCall(); - CreateClientFfi(request.ToPtr(), successCallbackPointer, failureCallbackPointer, pubsubCallbackPointer); + + IntPtr addressResolverPointer = IntPtr.Zero; + if (config.Request.AddressResolver != null) + { + client._addressResolverDelegate = (hostPtr, hostLen, port, bufPtr, bufLen, outLen) => + { + string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen) ?? string.Empty; + var (resolvedHost, resolvedPort) = config.Request.AddressResolver(host, port); + if (resolvedPort <= 0 || string.IsNullOrEmpty(resolvedHost)) + return 0; + byte[] bytes = System.Text.Encoding.UTF8.GetBytes(resolvedHost); + if (bytes.Length > (int)bufLen) + return 0; + Marshal.Copy(bytes, 0, bufPtr, bytes.Length); + unsafe { *(UIntPtr*)outLen = (UIntPtr)bytes.Length; } + return (ushort)resolvedPort; + }; + addressResolverPointer = Marshal.GetFunctionPointerForDelegate(client._addressResolverDelegate); + } + + CreateClientFfi(request.ToPtr(), successCallbackPointer, failureCallbackPointer, pubsubCallbackPointer, addressResolverPointer); client.ClientPointer = await message; // This will throw an error thru failure callback if any if (client.ClientPointer == IntPtr.Zero) @@ -626,6 +646,12 @@ private delegate void PubSubAction( ulong channelLen, IntPtr patternPtr, ulong patternLen); + + [UnmanagedFunctionPointer(CallingConvention.Cdecl)] + private delegate ushort AddressResolverAction( + IntPtr host, UIntPtr hostLen, ushort port, + IntPtr resolvedHostBuf, UIntPtr resolvedHostBufLen, + UIntPtr resolvedHostLen); #endregion private methods #region private fields @@ -642,6 +668,9 @@ private delegate void PubSubAction( /// and held in order to prevent the cost of marshalling on each function call. private readonly PubSubAction _pubsubCallbackDelegate; + /// Held to prevent the delegate being garbage collected. + private AddressResolverAction? _addressResolverDelegate; + private readonly object _lock = new(); private string _clientInfo = ""; // used to distinguish and identify clients during tests diff --git a/sources/Valkey.Glide/ConnectionConfiguration.cs b/sources/Valkey.Glide/ConnectionConfiguration.cs index 1913f3091..6088826f1 100644 --- a/sources/Valkey.Glide/ConnectionConfiguration.cs +++ b/sources/Valkey.Glide/ConnectionConfiguration.cs @@ -20,6 +20,15 @@ public abstract class ConnectionConfiguration /// public static readonly long CertificateMaxSize = 10 * 1024 * 1024; // 10 MB + /// + /// Callback for resolving server addresses before connection. + /// Allows intercepting and rewriting host/port pairs before the client connects. + /// + /// The configured host. + /// The configured port. + /// The resolved (host, port) to use for the actual connection. + public delegate (string host, int port) AddressResolverDelegate(string host, int port); + #region Structs and Enums definitions internal record ConnectionConfig @@ -43,6 +52,7 @@ internal record ConnectionConfig public CompressionConfig? CompressionConfig; public bool ReadOnly; public ClientSideCacheConfig? ClientSideCacheConfig; + public AddressResolverDelegate? AddressResolver; internal FFI.ConnectionConfig ToFfi() => new( @@ -877,6 +887,27 @@ public T WithClientSideCache(ClientSideCacheConfig clientSideCacheConfig) #endregion + #region Address Resolver + + /// + /// Optional callback for resolving server addresses before connection. + /// When set, the callback is invoked for each server address and can return a different host/port. + /// + public AddressResolverDelegate? AddressResolver + { + get => Config.AddressResolver; + set => Config.AddressResolver = value; + } + + /// + public T WithAddressResolver(AddressResolverDelegate addressResolver) + { + AddressResolver = addressResolver; + return (T)this; + } + + #endregion + internal ConnectionConfig Build() => Config; } diff --git a/sources/Valkey.Glide/Internals/FFI.methods.cs b/sources/Valkey.Glide/Internals/FFI.methods.cs index aebf0aacc..c4bb61f19 100644 --- a/sources/Valkey.Glide/Internals/FFI.methods.cs +++ b/sources/Valkey.Glide/Internals/FFI.methods.cs @@ -49,7 +49,7 @@ internal delegate void PubSubMessageCallback( [LibraryImport("libglide_rs", EntryPoint = "create_client")] [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] - public static partial void CreateClientFfi(IntPtr config, IntPtr successCallback, IntPtr failureCallback, IntPtr pubsubCallback); + public static partial void CreateClientFfi(IntPtr config, IntPtr successCallback, IntPtr failureCallback, IntPtr pubsubCallback, IntPtr addressResolverCallback); [LibraryImport("libglide_rs", EntryPoint = "close_client")] [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] @@ -135,7 +135,7 @@ public static partial void ScriptInvokeFfi( public static extern void FreeString(IntPtr strPtr); [DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "create_client")] - public static extern void CreateClientFfi(IntPtr config, IntPtr successCallback, IntPtr failureCallback, IntPtr pubsubCallback); + public static extern void CreateClientFfi(IntPtr config, IntPtr successCallback, IntPtr failureCallback, IntPtr pubsubCallback, IntPtr addressResolverCallback); [DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "close_client")] public static extern void CloseClientFfi(IntPtr client); diff --git a/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs b/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs new file mode 100644 index 000000000..af02b4ffb --- /dev/null +++ b/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs @@ -0,0 +1,56 @@ +// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 + +using static Valkey.Glide.ConnectionConfiguration; + +namespace Valkey.Glide.UnitTests; + +public class AddressResolverTests +{ + [Fact] + public void StandaloneBuilder_WithAddressResolver_SetsResolver() + { + static (string, int) Resolver(string host, int port) => ("resolved-host", 9999); + var config = new StandaloneClientConfigurationBuilder() + .WithAddressResolver(Resolver) + .Build(); + Assert.NotNull(config.Request.AddressResolver); + } + + [Fact] + public void ClusterBuilder_WithAddressResolver_SetsResolver() + { + static (string, int) Resolver(string host, int port) => (host, port); + var config = new ClusterClientConfigurationBuilder() + .WithAddressResolver(Resolver) + .Build(); + Assert.NotNull(config.Request.AddressResolver); + } + + [Fact] + public void AddressResolver_DefaultIsNull() + { + var config = new StandaloneClientConfigurationBuilder().Build(); + Assert.Null(config.Request.AddressResolver); + } + + [Fact] + public void AddressResolver_InvokesCallback() + { + static (string, int) Resolver(string host, int port) => ("proxy.example.com", 7000); + var config = new StandaloneClientConfigurationBuilder() + .WithAddressResolver(Resolver) + .Build(); + var (resolvedHost, resolvedPort) = config.Request.AddressResolver!("localhost", 6379); + Assert.Equal("proxy.example.com", resolvedHost); + Assert.Equal(7000, resolvedPort); + } + + [Fact] + public void AddressResolver_NullIsValid() + { + var builder = new StandaloneClientConfigurationBuilder(); + builder.AddressResolver = null; + var config = builder.Build(); + Assert.Null(config.Request.AddressResolver); + } +} diff --git a/valkey-glide b/valkey-glide index bf74288bf..b4b34a1dc 160000 --- a/valkey-glide +++ b/valkey-glide @@ -1 +1 @@ -Subproject commit bf74288bfa51c02b67904e43b1e20be61a29a09b +Subproject commit b4b34a1dc16d97a3fae32fc19bc1d42b3ec2f224 From 00e7a85368d8a27af5a458e41e03136e9c7560dd Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 17:19:01 -0700 Subject: [PATCH 02/16] Port type of `port` parameter from `int` to `ushort` in delegate and callback, and refactor adddress resolver tests Signed-off-by: currantw --- sources/Valkey.Glide/Client/BaseClient.cs | 2 +- .../Valkey.Glide/ConnectionConfiguration.cs | 2 +- .../AddressResolverTests.cs | 65 +++++++++---------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index 233b270dd..bb123576b 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -216,7 +216,7 @@ protected static async Task CreateClient(BaseClientConfiguration config, F return 0; Marshal.Copy(bytes, 0, bufPtr, bytes.Length); unsafe { *(UIntPtr*)outLen = (UIntPtr)bytes.Length; } - return (ushort)resolvedPort; + return resolvedPort; }; addressResolverPointer = Marshal.GetFunctionPointerForDelegate(client._addressResolverDelegate); } diff --git a/sources/Valkey.Glide/ConnectionConfiguration.cs b/sources/Valkey.Glide/ConnectionConfiguration.cs index 6088826f1..f9f4d0435 100644 --- a/sources/Valkey.Glide/ConnectionConfiguration.cs +++ b/sources/Valkey.Glide/ConnectionConfiguration.cs @@ -27,7 +27,7 @@ public abstract class ConnectionConfiguration /// The configured host. /// The configured port. /// The resolved (host, port) to use for the actual connection. - public delegate (string host, int port) AddressResolverDelegate(string host, int port); + public delegate (string host, ushort port) AddressResolverDelegate(string host, ushort port); #region Structs and Enums definitions diff --git a/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs b/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs index af02b4ffb..8cd7f26f5 100644 --- a/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs +++ b/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs @@ -6,51 +6,48 @@ namespace Valkey.Glide.UnitTests; public class AddressResolverTests { + #region Constants + + private static readonly (string, ushort) Resolved = ("resolved-host", 9999); + private static readonly AddressResolverDelegate Resolver = (host, port) => Resolved; + + #endregion + + #region Tests + [Fact] - public void StandaloneBuilder_WithAddressResolver_SetsResolver() + public void WithAddressResolver_Standalone_SetsResolver() { - static (string, int) Resolver(string host, int port) => ("resolved-host", 9999); - var config = new StandaloneClientConfigurationBuilder() - .WithAddressResolver(Resolver) - .Build(); - Assert.NotNull(config.Request.AddressResolver); + var config = new StandaloneClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); + Assert.Equal(Resolver, config.Request.AddressResolver); + Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); + } [Fact] - public void ClusterBuilder_WithAddressResolver_SetsResolver() + public void WithAddressResolver_Cluster_SetsResolver() { - static (string, int) Resolver(string host, int port) => (host, port); - var config = new ClusterClientConfigurationBuilder() - .WithAddressResolver(Resolver) - .Build(); - Assert.NotNull(config.Request.AddressResolver); + var config = new ClusterClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); + Assert.Equal(Resolver, config.Request.AddressResolver); + Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); + } [Fact] - public void AddressResolver_DefaultIsNull() - { - var config = new StandaloneClientConfigurationBuilder().Build(); - Assert.Null(config.Request.AddressResolver); - } + public void AddressResolver_Standalone_DefaultIsNull() + => Assert.Null(new StandaloneClientConfigurationBuilder().Build().Request.AddressResolver); [Fact] - public void AddressResolver_InvokesCallback() - { - static (string, int) Resolver(string host, int port) => ("proxy.example.com", 7000); - var config = new StandaloneClientConfigurationBuilder() - .WithAddressResolver(Resolver) - .Build(); - var (resolvedHost, resolvedPort) = config.Request.AddressResolver!("localhost", 6379); - Assert.Equal("proxy.example.com", resolvedHost); - Assert.Equal(7000, resolvedPort); - } + public void AddressResolver_Cluster_DefaultIsNull() + => Assert.Null(new ClusterClientConfigurationBuilder().Build().Request.AddressResolver); [Fact] - public void AddressResolver_NullIsValid() - { - var builder = new StandaloneClientConfigurationBuilder(); - builder.AddressResolver = null; - var config = builder.Build(); - Assert.Null(config.Request.AddressResolver); - } + public void AddressResolver_Standalone_SetToNull() + => Assert.Null(new StandaloneClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); + + [Fact] + public void AddressResolver_Cluster_SetToNull() + => Assert.Null(new ClusterClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); + + #endregion } From da12b9daf70a079e57365eb805319e1003f662da Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 17:35:02 -0700 Subject: [PATCH 03/16] Address testing review comments. Signed-off-by: currantw --- .../AddressResolverTests.cs | 53 ------------------- .../ConnectionConfigurationTests.cs | 39 ++++++++++++++ 2 files changed, 39 insertions(+), 53 deletions(-) delete mode 100644 tests/Valkey.Glide.UnitTests/AddressResolverTests.cs diff --git a/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs b/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs deleted file mode 100644 index 8cd7f26f5..000000000 --- a/tests/Valkey.Glide.UnitTests/AddressResolverTests.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 - -using static Valkey.Glide.ConnectionConfiguration; - -namespace Valkey.Glide.UnitTests; - -public class AddressResolverTests -{ - #region Constants - - private static readonly (string, ushort) Resolved = ("resolved-host", 9999); - private static readonly AddressResolverDelegate Resolver = (host, port) => Resolved; - - #endregion - - #region Tests - - [Fact] - public void WithAddressResolver_Standalone_SetsResolver() - { - var config = new StandaloneClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); - Assert.Equal(Resolver, config.Request.AddressResolver); - Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); - - } - - [Fact] - public void WithAddressResolver_Cluster_SetsResolver() - { - var config = new ClusterClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); - Assert.Equal(Resolver, config.Request.AddressResolver); - Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); - - } - - [Fact] - public void AddressResolver_Standalone_DefaultIsNull() - => Assert.Null(new StandaloneClientConfigurationBuilder().Build().Request.AddressResolver); - - [Fact] - public void AddressResolver_Cluster_DefaultIsNull() - => Assert.Null(new ClusterClientConfigurationBuilder().Build().Request.AddressResolver); - - [Fact] - public void AddressResolver_Standalone_SetToNull() - => Assert.Null(new StandaloneClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); - - [Fact] - public void AddressResolver_Cluster_SetToNull() - => Assert.Null(new ClusterClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); - - #endregion -} diff --git a/tests/Valkey.Glide.UnitTests/ConnectionConfigurationTests.cs b/tests/Valkey.Glide.UnitTests/ConnectionConfigurationTests.cs index 20737a0af..eb2453c41 100644 --- a/tests/Valkey.Glide.UnitTests/ConnectionConfigurationTests.cs +++ b/tests/Valkey.Glide.UnitTests/ConnectionConfigurationTests.cs @@ -27,6 +27,10 @@ public class ConnectionConfigurationTests // IAM auth constants. private const uint RefreshInterval = 300; + // Address resolver constants. + private static readonly (string, ushort) Resolved = ("resolved-host", 9999); + private static readonly AddressResolverDelegate Resolver = (host, port) => Resolved; + #endregion #region Authentication & Credentials Tests @@ -672,6 +676,41 @@ public void WithConnectionRetryStrategy_Cluster_UintParams_WithJitter() Assert.Equal(JitterPercent, strategy.JitterPercent); } + #endregion + #region Address Resolver Tests + + [Fact] + public void WithAddressResolver_Standalone_SetsResolver() + { + var config = new StandaloneClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); + Assert.Equal(Resolver, config.Request.AddressResolver); + Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); + } + + [Fact] + public void WithAddressResolver_Cluster_SetsResolver() + { + var config = new ClusterClientConfigurationBuilder().WithAddressResolver(Resolver).Build(); + Assert.Equal(Resolver, config.Request.AddressResolver); + Assert.Equal(Resolved, config.Request.AddressResolver!("localhost", 6379)); + } + + [Fact] + public void AddressResolver_Standalone_NotSet_IsNull() + => Assert.Null(new StandaloneClientConfigurationBuilder().Build().Request.AddressResolver); + + [Fact] + public void AddressResolver_Cluster_NotSet_IsNull() + => Assert.Null(new ClusterClientConfigurationBuilder().Build().Request.AddressResolver); + + [Fact] + public void AddressResolver_Standalone_SetToNull_IsNull() + => Assert.Null(new StandaloneClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); + + [Fact] + public void AddressResolver_Cluster_SetToNull_IsNull() + => Assert.Null(new ClusterClientConfigurationBuilder { AddressResolver = null }.Build().Request.AddressResolver); + #endregion #region Helpers From 6635ebc19509de5ce563b1bb5d145dd82ba85bc8 Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 17:55:59 -0700 Subject: [PATCH 04/16] Fix Taskfile filter error Signed-off-by: currantw --- Taskfile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Taskfile.yml b/Taskfile.yml index a4b825b96..d914be06f 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -15,7 +15,7 @@ vars: # Filter tests by class or method name. # Usage: `task test:unit filter=MyTestClass` FILTER: '{{.filter | default ""}}' - TEST_FILTER: "{{if .FILTER}}--filter FullyQualifiedName~{{.FILTER}}{{end}}" + TEST_FILTER: '{{with .filter}}--filter FullyQualifiedName~{{.}}{{end}}' tasks: clean: From 52102e284bae049af827422328b9b4b057a50e38 Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 18:48:15 -0700 Subject: [PATCH 05/16] Simplify Taskfile Signed-off-by: currantw --- Taskfile.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index d914be06f..c4a2aa4da 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -12,11 +12,6 @@ vars: TARGET: '{{.target | default "all"}}' BUILD_TARGET: '{{if eq .TARGET "lib"}}sources/Valkey.Glide/{{end}}' - # Filter tests by class or method name. - # Usage: `task test:unit filter=MyTestClass` - FILTER: '{{.filter | default ""}}' - TEST_FILTER: '{{with .filter}}--filter FullyQualifiedName~{{.}}{{end}}' - tasks: clean: desc: Clean test results and reports directories @@ -36,20 +31,24 @@ tasks: test:unit: desc: Run unit tests only deps: [clean] + vars: + FILTER: '{{.filter | default ""}}' cmds: - dotnet test {{.UNIT_TEST_PROJECT}} --configuration Release --verbosity normal - {{.TEST_FILTER}} + {{if .FILTER}}--filter FullyQualifiedName~{{.FILTER}}{{end}} test:integration: desc: Run integration tests only deps: [clean] + vars: + FILTER: '{{.filter | default ""}}' cmds: - dotnet test {{.INTEGRATION_TEST_PROJECT}} --configuration Release --verbosity normal - {{.TEST_FILTER}} + {{if .FILTER}}--filter FullyQualifiedName~{{.FILTER}}{{end}} test:coverage:unit: desc: Run unit tests with coverage collection From 7df76ba62a7ebda0bbe68277e3507859590bf362 Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 18:48:29 -0700 Subject: [PATCH 06/16] Add integration tests Signed-off-by: currantw --- .../AddressResolverTests.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs diff --git a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs new file mode 100644 index 000000000..9ce9034e9 --- /dev/null +++ b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs @@ -0,0 +1,81 @@ +// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 + +using Valkey.Glide.TestUtils; + +using static Valkey.Glide.ConnectionConfiguration; +using static Valkey.Glide.TestUtils.Client; +using static Valkey.Glide.TestUtils.Data; + +namespace Valkey.Glide.IntegrationTests; + +/// +/// Integration tests for the address resolver callback. +/// +public class AddressResolverTests +{ + #region Tests + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_IdentityResolver_IsInvokedAndConnects(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + int invocationCount = 0; + (string, ushort) Resolver(string host, ushort port) + { + _ = Interlocked.Increment(ref invocationCount); + return (host, port); + } + + using var client = await BuildClient( + useCluster, + host: address.Host, + port: address.Port, + resolver: Resolver); + + await AssertConnected(client); + Assert.True(invocationCount > 0); + } + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_ResolvesAddress_ConnectsSuccessfully(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + using var client = await BuildClient( + useCluster: false, + host: "nonexistent.invalid", + port: 0001, + resolver: (_, _) => (address.Host, address.Port)); + + await AssertConnected(client); + } + + #endregion + #region Helpers + + /// + /// Builds a client configured with the given host, port, and address resolver. + /// + private static async Task BuildClient( + bool useCluster, string host, ushort port, AddressResolverDelegate resolver) + => useCluster + ? await GlideClusterClient.CreateClient( + new ClusterClientConfigurationBuilder() + .WithAddress(host, port) + .WithRequestTimeout(TestConfiguration.DEFAULT_TIMEOUT) + .WithTls(TestConfiguration.TLS) + .WithAddressResolver(resolver) + .Build()) + : await GlideClient.CreateClient( + new StandaloneClientConfigurationBuilder() + .WithAddress(host, port) + .WithRequestTimeout(TestConfiguration.DEFAULT_TIMEOUT) + .WithTls(TestConfiguration.TLS) + .WithAddressResolver(resolver) + .Build()); + + #endregion +} From cdf5290c5ae633371c51c06e4df60e69c2f059b9 Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 25 May 2026 21:18:53 -0700 Subject: [PATCH 07/16] Add integration tests Signed-off-by: currantw --- .../AddressResolverTests.cs | 60 +------- tests/Valkey.Glide.TestUtils/Client.cs | 14 ++ tests/Valkey.Glide.TestUtils/Config.cs | 130 ++++++++++++++++++ tests/Valkey.Glide.TestUtils/Server.cs | 72 ++-------- 4 files changed, 160 insertions(+), 116 deletions(-) create mode 100644 tests/Valkey.Glide.TestUtils/Config.cs diff --git a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs index 9ce9034e9..599da123b 100644 --- a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs @@ -2,8 +2,8 @@ using Valkey.Glide.TestUtils; -using static Valkey.Glide.ConnectionConfiguration; using static Valkey.Glide.TestUtils.Client; +using static Valkey.Glide.TestUtils.Config; using static Valkey.Glide.TestUtils.Data; namespace Valkey.Glide.IntegrationTests; @@ -13,69 +13,23 @@ namespace Valkey.Glide.IntegrationTests; /// public class AddressResolverTests { - #region Tests - [Theory] [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] public async Task AddressResolver_IdentityResolver_IsInvokedAndConnects(bool useCluster) { var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; - int invocationCount = 0; + int count = 0; (string, ushort) Resolver(string host, ushort port) { - _ = Interlocked.Increment(ref invocationCount); - return (host, port); + _ = Interlocked.Increment(ref count); + return (address.Host, address.Port); } - using var client = await BuildClient( - useCluster, - host: address.Host, - port: address.Port, - resolver: Resolver); + var config = BuildConfig(useCluster, [address], TestConfiguration.TLS, addressResolver: Resolver); + await using var client = await CreateClient(config); await AssertConnected(client); - Assert.True(invocationCount > 0); + Assert.True(count > 0); } - - [Theory] - [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] - public async Task AddressResolver_ResolvesAddress_ConnectsSuccessfully(bool useCluster) - { - var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; - - using var client = await BuildClient( - useCluster: false, - host: "nonexistent.invalid", - port: 0001, - resolver: (_, _) => (address.Host, address.Port)); - - await AssertConnected(client); - } - - #endregion - #region Helpers - - /// - /// Builds a client configured with the given host, port, and address resolver. - /// - private static async Task BuildClient( - bool useCluster, string host, ushort port, AddressResolverDelegate resolver) - => useCluster - ? await GlideClusterClient.CreateClient( - new ClusterClientConfigurationBuilder() - .WithAddress(host, port) - .WithRequestTimeout(TestConfiguration.DEFAULT_TIMEOUT) - .WithTls(TestConfiguration.TLS) - .WithAddressResolver(resolver) - .Build()) - : await GlideClient.CreateClient( - new StandaloneClientConfigurationBuilder() - .WithAddress(host, port) - .WithRequestTimeout(TestConfiguration.DEFAULT_TIMEOUT) - .WithTls(TestConfiguration.TLS) - .WithAddressResolver(resolver) - .Build()); - - #endregion } diff --git a/tests/Valkey.Glide.TestUtils/Client.cs b/tests/Valkey.Glide.TestUtils/Client.cs index e760f5ac6..311c99f2f 100644 --- a/tests/Valkey.Glide.TestUtils/Client.cs +++ b/tests/Valkey.Glide.TestUtils/Client.cs @@ -1,5 +1,7 @@ // Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 +using static Valkey.Glide.ConnectionConfiguration; + namespace Valkey.Glide.TestUtils; /// @@ -105,4 +107,16 @@ client is GlideClient standaloneClient string versionLine = lines.FirstOrDefault(static l => l.Contains("valkey_version")) ?? lines.First(static l => l.Contains("redis_version")); return new(versionLine.Split(':')[1]); } + + /// + /// Creates a client for the given configuration. + /// + /// The client configuration. + public static async Task CreateClient(BaseClientConfiguration config) + => config switch + { + StandaloneClientConfiguration standalone => await GlideClient.CreateClient(standalone), + ClusterClientConfiguration cluster => await GlideClusterClient.CreateClient(cluster), + _ => throw new ArgumentException($"Unknown configuration type: {config.GetType().Name}") + }; } diff --git a/tests/Valkey.Glide.TestUtils/Config.cs b/tests/Valkey.Glide.TestUtils/Config.cs new file mode 100644 index 000000000..225fbb0aa --- /dev/null +++ b/tests/Valkey.Glide.TestUtils/Config.cs @@ -0,0 +1,130 @@ +// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 + +using static Valkey.Glide.ConnectionConfiguration; + +namespace Valkey.Glide.TestUtils; + +/// +/// Test utilities for building client configurations. +/// +public static class Config +{ + #region Constants + + /// + /// Timeout for client connection and reconnection attempts. + /// Use a longer timeout to allow for slower connections in CI environments. + /// + private static readonly TimeSpan ConnectionTimeout = TimeSpan.FromSeconds(30); + + /// + /// Retry strategy for client connections. + /// Allow retries for transient connection timeouts in CI environments. + /// + private static readonly RetryStrategy RetryStrategy = new(numberOfRetries: 5, factor: 100, exponentBase: 2); + + #endregion + #region Public Methods + + /// + /// Builds a standalone client configuration builder with the given addresses and options. + /// + public static StandaloneClientConfigurationBuilder BuildStandaloneConfig( + IEnumerable
addresses, + bool useTls = false, + TimeSpan? connectionTimeout = null, + RetryStrategy? retryStrategy = null, + byte[]? trustedCertificate = null, + string? password = null, + AddressResolverDelegate? addressResolver = null) + { + StandaloneClientConfigurationBuilder builder = new() + { + UseTls = useTls, + ConnectionTimeout = connectionTimeout ?? ConnectionTimeout, + ConnectionRetryStrategy = retryStrategy ?? RetryStrategy, + }; + + foreach ((string host, ushort port) in addresses) + { + _ = builder.WithAddress(host, port); + } + + if (trustedCertificate is not null) + { + _ = builder.WithTrustedCertificate(trustedCertificate); + } + + if (password is not null) + { + _ = builder.WithAuthentication(password); + } + + if (addressResolver is not null) + { + _ = builder.WithAddressResolver(addressResolver); + } + + return builder; + } + + /// + /// Builds a cluster client configuration builder with the given addresses and options. + /// + public static ClusterClientConfigurationBuilder BuildClusterConfig( + IEnumerable
addresses, + bool useTls = false, + TimeSpan? connectionTimeout = null, + RetryStrategy? retryStrategy = null, + byte[]? trustedCertificate = null, + string? password = null, + AddressResolverDelegate? addressResolver = null) + { + ClusterClientConfigurationBuilder builder = new() + { + UseTls = useTls, + ConnectionTimeout = connectionTimeout ?? ConnectionTimeout, + ConnectionRetryStrategy = retryStrategy ?? RetryStrategy, + }; + + foreach ((string host, ushort port) in addresses) + { + _ = builder.WithAddress(host, port); + } + + if (trustedCertificate is not null) + { + _ = builder.WithTrustedCertificate(trustedCertificate); + } + + if (password is not null) + { + _ = builder.WithAuthentication(password); + } + + if (addressResolver is not null) + { + _ = builder.WithAddressResolver(addressResolver); + } + + return builder; + } + + /// + /// Builds a client configuration for the given mode, addresses, and options. + /// + public static BaseClientConfiguration BuildConfig( + bool clusterMode, + IEnumerable
addresses, + bool useTls = false, + TimeSpan? connectionTimeout = null, + RetryStrategy? retryStrategy = null, + byte[]? trustedCertificate = null, + string? password = null, + AddressResolverDelegate? addressResolver = null) + => clusterMode + ? BuildClusterConfig(addresses, useTls, connectionTimeout, retryStrategy, trustedCertificate, password, addressResolver).Build() + : BuildStandaloneConfig(addresses, useTls, connectionTimeout, retryStrategy, trustedCertificate, password, addressResolver).Build(); + + #endregion +} diff --git a/tests/Valkey.Glide.TestUtils/Server.cs b/tests/Valkey.Glide.TestUtils/Server.cs index 3f2e72d17..f339a02a9 100644 --- a/tests/Valkey.Glide.TestUtils/Server.cs +++ b/tests/Valkey.Glide.TestUtils/Server.cs @@ -11,18 +11,6 @@ public abstract class Server : IDisposable { #region Constants - /// - /// Timeout for client connection and reconnection attempts. - /// Use a longer timeout to allow for slower connections in CI environments. - /// - protected static readonly TimeSpan ConnectionTimeout = TimeSpan.FromSeconds(30); - - /// - /// Retry strategy for client connections. - /// Allow retries for transient connection timeouts in CI environments. - /// - protected static readonly RetryStrategy RetryStrategy = new(numberOfRetries: 5, factor: 100, exponentBase: 2); - /// /// Custom command arguments to kill all normal clients. /// @@ -37,7 +25,7 @@ public abstract class Server : IDisposable private readonly string _name = $"Server_{Guid.NewGuid():N}"; /// - /// Indicates whether the server has been stopped. + /// Whether the server has been stopped. /// See . /// private bool _disposed = false; @@ -148,31 +136,10 @@ public sealed class ClusterServer(bool useTls = false) : Server(useClusterMode: /// Builds and returns a cluster client configuration builder for this server. ///
public ClusterClientConfigurationBuilder CreateConfigBuilder() - { - ClusterClientConfigurationBuilder configBuilder = new() - { - UseTls = UseTls, - ConnectionTimeout = ConnectionTimeout, - ConnectionRetryStrategy = RetryStrategy - }; - - if (UseTls) - { - _ = configBuilder.WithTrustedCertificate(CertificateData!); - } - - if (_password is not null) - { - _ = configBuilder.WithAuthentication(_password); - } - - foreach ((string host, ushort port) in Addresses) - { - _ = configBuilder.WithAddress(host, port); - } - - return configBuilder; - } + => Config.BuildClusterConfig(Addresses, + useTls: UseTls, + trustedCertificate: UseTls ? CertificateData : null, + password: _password); /// public override async Task CreateClientAsync() @@ -218,31 +185,10 @@ public sealed class StandaloneServer(bool useTls = false) : Server(useClusterMod /// Builds and returns a standalone client configuration builder for this server. /// public StandaloneClientConfigurationBuilder CreateConfigBuilder() - { - StandaloneClientConfigurationBuilder configBuilder = new() - { - UseTls = UseTls, - ConnectionTimeout = ConnectionTimeout, - ConnectionRetryStrategy = RetryStrategy - }; - - if (UseTls) - { - _ = configBuilder.WithTrustedCertificate(CertificateData!); - } - - if (_password is not null) - { - _ = configBuilder.WithAuthentication(_password); - } - - foreach ((string host, ushort port) in Addresses) - { - _ = configBuilder.WithAddress(host, port); - } - - return configBuilder; - } + => Config.BuildStandaloneConfig(Addresses, + useTls: UseTls, + trustedCertificate: UseTls ? CertificateData : null, + password: _password); /// public override async Task CreateClientAsync() From 35d5bebe19bad84bfbf90b6d759a02d72be515c2 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 07:56:26 -0700 Subject: [PATCH 08/16] Simplify Server test utils Signed-off-by: currantw --- .../AddressResolverTests.cs | 2 +- .../ClusterClientTests.cs | 7 ++-- .../Valkey.Glide.IntegrationTests/DnsTests.cs | 8 +--- .../StackExchange/GenericCommandsTests.cs | 9 ++--- .../StackExchange/ServerManagementTests.cs | 4 +- .../StackExchange/ValkeyServerTests.cs | 2 +- .../StandaloneClientTests.cs | 11 ++---- .../Valkey.Glide.IntegrationTests/TlsTests.cs | 38 +++++++++---------- tests/Valkey.Glide.TestUtils/Config.cs | 38 ++++++++----------- tests/Valkey.Glide.TestUtils/Server.cs | 12 +++--- 10 files changed, 58 insertions(+), 73 deletions(-) diff --git a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs index 599da123b..6566c520d 100644 --- a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs @@ -26,7 +26,7 @@ public async Task AddressResolver_IdentityResolver_IsInvokedAndConnects(bool use return (address.Host, address.Port); } - var config = BuildConfig(useCluster, [address], TestConfiguration.TLS, addressResolver: Resolver); + var config = BuildConfig(useCluster, address, useTls: TestConfiguration.TLS, addressResolver: Resolver); await using var client = await CreateClient(config); await AssertConnected(client); diff --git a/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs b/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs index d1f60d44d..2fdc9aa9c 100644 --- a/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs @@ -627,11 +627,10 @@ public async Task EagerConnect() public async Task Connect_WithIpAddress_Succeeds(string address) { using var server = new ClusterServer(useTls: false); - var port = server.Addresses.First().Port; - var configBuilder = new ConnectionConfiguration.ClusterClientConfigurationBuilder() - .WithAddress(address, port); + var builder = new ConnectionConfiguration.ClusterClientConfigurationBuilder() + .WithAddress(address, server.Address.Port); - await using var client = await GlideClusterClient.CreateClient(configBuilder.Build()); + await using var client = await GlideClusterClient.CreateClient(builder.Build()); await Client.AssertConnected(client); } } diff --git a/tests/Valkey.Glide.IntegrationTests/DnsTests.cs b/tests/Valkey.Glide.IntegrationTests/DnsTests.cs index 7b4cb761e..6d72682ed 100644 --- a/tests/Valkey.Glide.IntegrationTests/DnsTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/DnsTests.cs @@ -88,10 +88,8 @@ private async Task BuildClient(bool useCluster, bool useTls, string if (useCluster) { var server = useTls ? fixture.TlsClusterServer! : fixture.ClusterServer!; - var port = server.Addresses.First().Port; - var builder = new ClusterClientConfigurationBuilder() - .WithAddress(host, port); + .WithAddress(host, server.Address.Port); if (useTls) { @@ -105,10 +103,8 @@ private async Task BuildClient(bool useCluster, bool useTls, string else { var server = useTls ? fixture.TlsStandaloneServer! : fixture.StandaloneServer!; - var port = server.Addresses.First().Port; - var builder = new StandaloneClientConfigurationBuilder() - .WithAddress(host, port); + .WithAddress(host, server.Address.Port); if (useTls) { diff --git a/tests/Valkey.Glide.IntegrationTests/StackExchange/GenericCommandsTests.cs b/tests/Valkey.Glide.IntegrationTests/StackExchange/GenericCommandsTests.cs index 0df5299a2..ba2115822 100644 --- a/tests/Valkey.Glide.IntegrationTests/StackExchange/GenericCommandsTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/StackExchange/GenericCommandsTests.cs @@ -794,15 +794,14 @@ public class GenericCommandsFixture : IDisposable { private readonly StandaloneServer _standaloneServer; private readonly ConnectionMultiplexer _connection; - private readonly GlideClient _client; public IDatabase Database { get; } - public GlideClient Client => _client; + public GlideClient Client { get; } public GenericCommandsFixture() { _standaloneServer = new(); - var (host, port) = _standaloneServer.Addresses.First(); + var (host, port) = _standaloneServer.Address; ConfigurationOptions config = new(); config.EndPoints.Add(host, port); @@ -814,12 +813,12 @@ public GenericCommandsFixture() var glideConfig = new StandaloneClientConfigurationBuilder() .WithAddress(host, port) .Build(); - _client = GlideClient.CreateClient(glideConfig).GetAwaiter().GetResult(); + Client = GlideClient.CreateClient(glideConfig).GetAwaiter().GetResult(); } public void Dispose() { - _client.Dispose(); + Client.Dispose(); _connection.Dispose(); _standaloneServer.Dispose(); } diff --git a/tests/Valkey.Glide.IntegrationTests/StackExchange/ServerManagementTests.cs b/tests/Valkey.Glide.IntegrationTests/StackExchange/ServerManagementTests.cs index 1b2ffa704..e15f522a3 100644 --- a/tests/Valkey.Glide.IntegrationTests/StackExchange/ServerManagementTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/StackExchange/ServerManagementTests.cs @@ -96,8 +96,8 @@ public class ServerManagementFixture : IDisposable public ServerManagementFixture() { ConfigurationOptions config = new(); - (string host, ushort port) = _standaloneServer.Addresses.First(); - config.EndPoints.Add(host, port); + var address = _standaloneServer.Address; + config.EndPoints.Add(address.Host, address.Port); ConnectionMultiplexer conn = ConnectionMultiplexer.Connect(config); Server = conn.GetServers().First(); } diff --git a/tests/Valkey.Glide.IntegrationTests/StackExchange/ValkeyServerTests.cs b/tests/Valkey.Glide.IntegrationTests/StackExchange/ValkeyServerTests.cs index 1721eb7fd..a0f040f5f 100644 --- a/tests/Valkey.Glide.IntegrationTests/StackExchange/ValkeyServerTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/StackExchange/ValkeyServerTests.cs @@ -165,7 +165,7 @@ public class ValkeyServerFixture : IDisposable public ValkeyServerFixture() { _standaloneServer = new(); - var (host, port) = _standaloneServer.Addresses.First(); + var (host, port) = _standaloneServer.Address; ConfigurationOptions config = new(); config.EndPoints.Add(host, port); diff --git a/tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs b/tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs index 35a7a9827..1eff5a724 100644 --- a/tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs @@ -353,10 +353,8 @@ public async Task ConfigSetAsync_SetsConfiguration(GlideClient client) [Theory(DisableDiscoveryEnumeration = true)] [MemberData(nameof(Config.TestStandaloneClients), MemberType = typeof(TestConfiguration))] public async Task ConfigResetStatisticsAsync_ResetsStats(GlideClient client) - { // This should not throw - await client.ConfigResetStatisticsAsync(); - } + => await client.ConfigResetStatisticsAsync(); [Theory(DisableDiscoveryEnumeration = true)] [MemberData(nameof(Config.TestStandaloneClients), MemberType = typeof(TestConfiguration))] @@ -492,11 +490,10 @@ public async Task EagerConnect() public async Task Connect_WithIpAddress_Succeeds(string address) { using var server = new StandaloneServer(useTls: false); - var port = server.Addresses.First().Port; - var configBuilder = new ConnectionConfiguration.StandaloneClientConfigurationBuilder() - .WithAddress(address, port); + var builder = new ConnectionConfiguration.StandaloneClientConfigurationBuilder() + .WithAddress(address, server.Address.Port); - await using var client = await GlideClient.CreateClient(configBuilder.Build()); + await using var client = await GlideClient.CreateClient(builder.Build()); await AssertConnected(client); } } diff --git a/tests/Valkey.Glide.IntegrationTests/TlsTests.cs b/tests/Valkey.Glide.IntegrationTests/TlsTests.cs index c5e18ae84..dcdd2a3e8 100644 --- a/tests/Valkey.Glide.IntegrationTests/TlsTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/TlsTests.cs @@ -23,7 +23,7 @@ public class TlsTests(ServerFixture serverFixture, TlsServerFixture tlsServerFix public async Task Cluster_WithCertificateData_NotTrusted_Throws() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -39,7 +39,7 @@ public async Task Cluster_WithCertificateData_NotTrusted_Throws() public async Task Cluster_WithCertificateData_Malformed_Throws() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -60,7 +60,7 @@ public void Cluster_WithCertificatePath_NotFound_Throws() public async Task Cluster_NoCertificate_TlsServer_Throws() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -75,7 +75,7 @@ public async Task Cluster_NoCertificate_TlsServer_Throws() public async Task Cluster_WithCertificate_NonTlsServer_Throws() { var server = serverFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -90,7 +90,7 @@ public async Task Cluster_WithCertificate_NonTlsServer_Throws() public async Task Cluster_WithCertificateData_Trusted_Succeeds() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -106,7 +106,7 @@ public async Task Cluster_WithCertificateData_Trusted_Succeeds() public async Task Cluster_WithCertificatePath_Trusted_Succeeds() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -122,7 +122,7 @@ public async Task Cluster_WithCertificatePath_Trusted_Succeeds() public async Task Cluster_WithInsecureTls_WithTlsServer_Succeeds() { var server = tlsServerFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -138,7 +138,7 @@ public async Task Cluster_WithInsecureTls_WithTlsServer_Succeeds() public async Task Cluster_WithInsecureTls_WithNonTlsServer_Throws() { var server = serverFixture.ClusterServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new ClusterClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -155,10 +155,9 @@ public async Task Cluster_WithInsecureTls_WithNonTlsServer_Throws() public async Task Cluster_WithIpAddress_Succeeds(string address) { var server = tlsServerFixture.ClusterServer; - var port = server.Addresses.First().Port; var config = new ClusterClientConfigurationBuilder() - .WithAddress(address, port) + .WithAddress(address, server.Address.Port) .WithTls() .WithTrustedCertificate(server.CertificateData!) .Build(); @@ -174,7 +173,7 @@ public async Task Cluster_WithIpAddress_Succeeds(string address) public async Task Standalone_WithCertificateData_NotTrusted_Throws() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -190,7 +189,7 @@ public async Task Standalone_WithCertificateData_NotTrusted_Throws() public async Task Standalone_WithCertificateData_Malformed_Throws() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -211,7 +210,7 @@ public void Standalone_WithCertificatePath_InvalidThrows() public async Task Standalone_NoCertificate_Throws() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -226,7 +225,7 @@ public async Task Standalone_NoCertificate_Throws() public async Task Standalone_WithCertificate_NonTlsServer_Throws() { var server = serverFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -242,7 +241,7 @@ public async Task Standalone_WithCertificate_NonTlsServer_Throws() public async Task Standalone_WithCertificateData_Trusted_Succeeds() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -258,7 +257,7 @@ public async Task Standalone_WithCertificateData_Trusted_Succeeds() public async Task Standalone_WithCertificatePath_Trusted_Succeeds() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -274,7 +273,7 @@ public async Task Standalone_WithCertificatePath_Trusted_Succeeds() public async Task Standalone_WithInsecureTls_WithTlsServer_Succeeds() { var server = tlsServerFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -290,7 +289,7 @@ public async Task Standalone_WithInsecureTls_WithTlsServer_Succeeds() public async Task Standalone_WithInsecureTls_WithNonTlsServer_Throws() { var server = serverFixture.StandaloneServer; - var address = server.Addresses.First(); + var address = server.Address; var config = new StandaloneClientConfigurationBuilder() .WithAddress(address.Host, address.Port) @@ -307,10 +306,9 @@ public async Task Standalone_WithInsecureTls_WithNonTlsServer_Throws() public async Task Standalone_WithIpAddress_Succeeds(string address) { var server = tlsServerFixture.StandaloneServer; - var port = server.Addresses.First().Port; var config = new StandaloneClientConfigurationBuilder() - .WithAddress(address, port) + .WithAddress(address, server.Address.Port) .WithTls() .WithTrustedCertificate(server.CertificateData!) .Build(); diff --git a/tests/Valkey.Glide.TestUtils/Config.cs b/tests/Valkey.Glide.TestUtils/Config.cs index 225fbb0aa..961059db9 100644 --- a/tests/Valkey.Glide.TestUtils/Config.cs +++ b/tests/Valkey.Glide.TestUtils/Config.cs @@ -27,16 +27,16 @@ public static class Config #region Public Methods /// - /// Builds a standalone client configuration builder with the given addresses and options. + /// Builds a standalone client configuration builder with the given address and options. /// public static StandaloneClientConfigurationBuilder BuildStandaloneConfig( - IEnumerable
addresses, + Address address, + AddressResolverDelegate? addressResolver = null, bool useTls = false, TimeSpan? connectionTimeout = null, RetryStrategy? retryStrategy = null, byte[]? trustedCertificate = null, - string? password = null, - AddressResolverDelegate? addressResolver = null) + string? password = null) { StandaloneClientConfigurationBuilder builder = new() { @@ -45,10 +45,7 @@ public static StandaloneClientConfigurationBuilder BuildStandaloneConfig( ConnectionRetryStrategy = retryStrategy ?? RetryStrategy, }; - foreach ((string host, ushort port) in addresses) - { - _ = builder.WithAddress(host, port); - } + _ = builder.WithAddress(address.Host, address.Port); if (trustedCertificate is not null) { @@ -69,16 +66,16 @@ public static StandaloneClientConfigurationBuilder BuildStandaloneConfig( } /// - /// Builds a cluster client configuration builder with the given addresses and options. + /// Builds a cluster client configuration builder with the given address and options. /// public static ClusterClientConfigurationBuilder BuildClusterConfig( - IEnumerable
addresses, + Address address, + AddressResolverDelegate? addressResolver = null, bool useTls = false, TimeSpan? connectionTimeout = null, RetryStrategy? retryStrategy = null, byte[]? trustedCertificate = null, - string? password = null, - AddressResolverDelegate? addressResolver = null) + string? password = null) { ClusterClientConfigurationBuilder builder = new() { @@ -87,10 +84,7 @@ public static ClusterClientConfigurationBuilder BuildClusterConfig( ConnectionRetryStrategy = retryStrategy ?? RetryStrategy, }; - foreach ((string host, ushort port) in addresses) - { - _ = builder.WithAddress(host, port); - } + _ = builder.WithAddress(address.Host, address.Port); if (trustedCertificate is not null) { @@ -111,20 +105,20 @@ public static ClusterClientConfigurationBuilder BuildClusterConfig( } /// - /// Builds a client configuration for the given mode, addresses, and options. + /// Builds a client configuration for the given mode, address, and options. /// public static BaseClientConfiguration BuildConfig( bool clusterMode, - IEnumerable
addresses, + Address address, + AddressResolverDelegate? addressResolver = null, bool useTls = false, TimeSpan? connectionTimeout = null, RetryStrategy? retryStrategy = null, byte[]? trustedCertificate = null, - string? password = null, - AddressResolverDelegate? addressResolver = null) + string? password = null) => clusterMode - ? BuildClusterConfig(addresses, useTls, connectionTimeout, retryStrategy, trustedCertificate, password, addressResolver).Build() - : BuildStandaloneConfig(addresses, useTls, connectionTimeout, retryStrategy, trustedCertificate, password, addressResolver).Build(); + ? BuildClusterConfig(address, addressResolver, useTls, connectionTimeout, retryStrategy, trustedCertificate, password).Build() + : BuildStandaloneConfig(address, addressResolver, useTls, connectionTimeout, retryStrategy, trustedCertificate, password).Build(); #endregion } diff --git a/tests/Valkey.Glide.TestUtils/Server.cs b/tests/Valkey.Glide.TestUtils/Server.cs index f339a02a9..5371d8276 100644 --- a/tests/Valkey.Glide.TestUtils/Server.cs +++ b/tests/Valkey.Glide.TestUtils/Server.cs @@ -39,9 +39,9 @@ public abstract class Server : IDisposable #region Public Properties /// - /// Addresses of the server instances. + /// Address of the server. /// - public IList
Addresses { get; init; } + public Address Address { get; init; } /// /// Indicates whether the server uses TLS. @@ -69,7 +69,7 @@ public abstract class Server : IDisposable protected Server(bool useClusterMode, bool useTls) { UseTls = useTls; - Addresses = ServerManager.StartServer(_name, useClusterMode: useClusterMode, useTls: UseTls); + Address = ServerManager.StartServer(_name, useClusterMode: useClusterMode, useTls: UseTls).First(); if (UseTls) { @@ -136,7 +136,8 @@ public sealed class ClusterServer(bool useTls = false) : Server(useClusterMode: /// Builds and returns a cluster client configuration builder for this server. /// public ClusterClientConfigurationBuilder CreateConfigBuilder() - => Config.BuildClusterConfig(Addresses, + => Config.BuildClusterConfig( + address: Address, useTls: UseTls, trustedCertificate: UseTls ? CertificateData : null, password: _password); @@ -185,7 +186,8 @@ public sealed class StandaloneServer(bool useTls = false) : Server(useClusterMod /// Builds and returns a standalone client configuration builder for this server. /// public StandaloneClientConfigurationBuilder CreateConfigBuilder() - => Config.BuildStandaloneConfig(Addresses, + => Config.BuildStandaloneConfig( + address: Address, useTls: UseTls, trustedCertificate: UseTls ? CertificateData : null, password: _password); From 00c9b2007cda6a116865cd550edf3a11e3c0a321 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:16:49 -0700 Subject: [PATCH 09/16] fix(client): Add try/catch in address resolver FFI callback - Wrap user-provided AddressResolver delegate in try/catch to prevent process crash if the delegate throws an exception - Log the exception at Error level for debuggability - Return 0 to signal failure to Rust, which falls back to the original address (consistent with Java client behavior) - Add integration test verifying fallback behavior when resolver throws Signed-off-by: currantw --- sources/Valkey.Glide/Client/BaseClient.cs | 33 ++++++++++++++----- .../AddressResolverTests.cs | 14 +++++++- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index bb123576b..c5945b286 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -207,16 +207,31 @@ protected static async Task CreateClient(BaseClientConfiguration config, F { client._addressResolverDelegate = (hostPtr, hostLen, port, bufPtr, bufLen, outLen) => { - string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen) ?? string.Empty; - var (resolvedHost, resolvedPort) = config.Request.AddressResolver(host, port); - if (resolvedPort <= 0 || string.IsNullOrEmpty(resolvedHost)) - return 0; - byte[] bytes = System.Text.Encoding.UTF8.GetBytes(resolvedHost); - if (bytes.Length > (int)bufLen) + try + { + string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen) ?? string.Empty; + var (resolvedHost, resolvedPort) = config.Request.AddressResolver(host, port); + if (resolvedPort <= 0 || string.IsNullOrEmpty(resolvedHost)) + { + return 0; + } + + byte[] bytes = System.Text.Encoding.UTF8.GetBytes(resolvedHost); + if (bytes.Length > (int)bufLen) + { + return 0; + } + + Marshal.Copy(bytes, 0, bufPtr, bytes.Length); + unsafe { *(UIntPtr*)outLen = (UIntPtr)bytes.Length; } + return resolvedPort; + } + catch (Exception ex) + { + Logger.Log(Level.Error, "AddressResolver", + $"Address resolver callback threw an exception: {ex.Message}", ex); return 0; - Marshal.Copy(bytes, 0, bufPtr, bytes.Length); - unsafe { *(UIntPtr*)outLen = (UIntPtr)bytes.Length; } - return resolvedPort; + } }; addressResolverPointer = Marshal.GetFunctionPointerForDelegate(client._addressResolverDelegate); } diff --git a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs index 6566c520d..a5928bad4 100644 --- a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs @@ -26,10 +26,22 @@ public async Task AddressResolver_IdentityResolver_IsInvokedAndConnects(bool use return (address.Host, address.Port); } - var config = BuildConfig(useCluster, address, useTls: TestConfiguration.TLS, addressResolver: Resolver); + var config = BuildConfig(useCluster, address, addressResolver: Resolver); await using var client = await CreateClient(config); await AssertConnected(client); Assert.True(count > 0); } + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_ThrowsException_FallsBackToOriginalAddress(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + var config = BuildConfig(useCluster, address, addressResolver: (_, _) => throw new InvalidOperationException("Resolution failed!")); + await using var client = await CreateClient(config); + + await AssertConnected(client); + } } From 2d7b7addfefc891c39c716bd23d2be0bb710da6e Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:23:41 -0700 Subject: [PATCH 10/16] Better align documentation with Java Signed-off-by: currantw --- .../Valkey.Glide/ConnectionConfiguration.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sources/Valkey.Glide/ConnectionConfiguration.cs b/sources/Valkey.Glide/ConnectionConfiguration.cs index f9f4d0435..6fd660c23 100644 --- a/sources/Valkey.Glide/ConnectionConfiguration.cs +++ b/sources/Valkey.Glide/ConnectionConfiguration.cs @@ -21,12 +21,21 @@ public abstract class ConnectionConfiguration public static readonly long CertificateMaxSize = 10 * 1024 * 1024; // 10 MB /// - /// Callback for resolving server addresses before connection. - /// Allows intercepting and rewriting host/port pairs before the client connects. + /// A callback for resolving server addresses before connection. /// - /// The configured host. - /// The configured port. + /// The configured host name or IP address. + /// The configured port number. /// The resolved (host, port) to use for the actual connection. + /// + /// The resolver must be thread-safe and should avoid blocking operations, as it is + /// called synchronously during the connection process. + /// + /// If the resolver throws an exception, the client falls back to the original + /// (unresolved) address and logs the exception at . + /// + /// The resolver is invoked once per address at initial connection time. It is not + /// invoked on subsequent reconnection attempts. + /// public delegate (string host, ushort port) AddressResolverDelegate(string host, ushort port); #region Structs and Enums definitions @@ -862,7 +871,6 @@ public T WithCompression(CompressionConfig compressionConfig) } #endregion - #region Client-Side Cache /// @@ -886,13 +894,13 @@ public T WithClientSideCache(ClientSideCacheConfig clientSideCacheConfig) } #endregion - #region Address Resolver /// /// Optional callback for resolving server addresses before connection. /// When set, the callback is invoked for each server address and can return a different host/port. /// + /// public AddressResolverDelegate? AddressResolver { get => Config.AddressResolver; From 7bdd2886c9f69eba1a4c09aa5851e759d872be9b Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:36:16 -0700 Subject: [PATCH 11/16] Add more detailed error handling in `BaseClient` and add corresponding integration tests. Signed-off-by: currantw --- sources/Valkey.Glide/Client/BaseClient.cs | 21 ++++++++--- .../AddressResolverTests.cs | 36 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index c5945b286..d6a034380 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -209,27 +209,40 @@ protected static async Task CreateClient(BaseClientConfiguration config, F { try { - string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen) ?? string.Empty; + string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen)!; var (resolvedHost, resolvedPort) = config.Request.AddressResolver(host, port); - if (resolvedPort <= 0 || string.IsNullOrEmpty(resolvedHost)) + + if (string.IsNullOrEmpty(resolvedHost)) { + var msg = $"Address resolver returned an empty or null host for {host}:{port}"; + Logger.Log(Level.Error, "AddressResolver", msg); return 0; } byte[] bytes = System.Text.Encoding.UTF8.GetBytes(resolvedHost); if (bytes.Length > (int)bufLen) { + var msg = $"Resolved host length ({bytes.Length} bytes) exceeds maximum length ({bufLen} bytes) for {host}:{port}"; + Logger.Log(Level.Error, "AddressResolver", msg); + return 0; + } + + if (resolvedPort <= 0) + { + var msg = $"Address resolver returned an invalid port ({resolvedPort}) for {host}:{port}"; + Logger.Log(Level.Error, "AddressResolver", msg); return 0; } Marshal.Copy(bytes, 0, bufPtr, bytes.Length); unsafe { *(UIntPtr*)outLen = (UIntPtr)bytes.Length; } + return resolvedPort; } catch (Exception ex) { - Logger.Log(Level.Error, "AddressResolver", - $"Address resolver callback threw an exception: {ex.Message}", ex); + var msg = $"Address resolver callback threw an exception: {ex.Message}"; + Logger.Log(Level.Error, "AddressResolver", msg, ex); return 0; } }; diff --git a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs index a5928bad4..1d150c0ab 100644 --- a/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs +++ b/tests/Valkey.Glide.IntegrationTests/AddressResolverTests.cs @@ -44,4 +44,40 @@ public async Task AddressResolver_ThrowsException_FallsBackToOriginalAddress(boo await AssertConnected(client); } + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_ReturnsEmptyHost_FallsBackToOriginalAddress(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + var config = BuildConfig(useCluster, address, addressResolver: (_, _) => ("", address.Port)); + await using var client = await CreateClient(config); + + await AssertConnected(client); + } + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_ReturnsPortZero_FallsBackToOriginalAddress(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + var config = BuildConfig(useCluster, address, addressResolver: (_, _) => (address.Host, 0)); + await using var client = await CreateClient(config); + + await AssertConnected(client); + } + + [Theory] + [MemberData(nameof(ClusterMode), MemberType = typeof(Data))] + public async Task AddressResolver_ReturnsOversizedHost_FallsBackToOriginalAddress(bool useCluster) + { + var address = useCluster ? TestConfiguration.CLUSTER_ADDRESS : TestConfiguration.STANDALONE_ADDRESS; + + var config = BuildConfig(useCluster, address, addressResolver: (_, _) => (new string('a', 2048), address.Port)); + await using var client = await CreateClient(config); + + await AssertConnected(client); + } } From 2c54a1ab85152001a96ea4307945eaaff1c3090f Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:40:42 -0700 Subject: [PATCH 12/16] Add some extra documentation Signed-off-by: currantw --- rust/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 23acacc7e..d297efb61 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -123,6 +123,8 @@ impl std::fmt::Debug for FFIAddressResolver { } impl redis::AddressResolver for FFIAddressResolver { + /// Resolves the given host and port by invoking the C# callback. + /// On error, falls back to the original `(host, port)` unchanged. fn resolve(&self, host: &str, port: u16) -> (String, u16) { let mut buf = vec![0u8; 1024]; let mut len: usize = 0; @@ -202,6 +204,7 @@ impl Drop for PanicGuard { /// See the safety documentation of [`SuccessCallback`] and [`FailureCallback`]. /// * `pubsub_callback` is an optional callback. When provided, it must be a valid function pointer. /// See the safety documentation in the FFI module for PubSubCallback. +/// * `address_resolver` is an optional callback. When provided, it must be a valid function pointer. #[allow(rustdoc::private_intra_doc_links)] #[unsafe(no_mangle)] pub unsafe extern "C-unwind" fn create_client( From d460414368249240ec01f0e1e2178b01fac4df3b Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:56:58 -0700 Subject: [PATCH 13/16] docs(resolver): Improve address resolver documentation and code quality Rust: - Extract MAX_RESOLVED_HOST_LEN constant (1024 bytes) - Use stack allocation instead of heap for resolve buffer - Add full doc comments on AddressResolverCallback matching SuccessCallback/FailureCallback style - Add doc comment on FFIAddressResolver explaining newtype pattern - Add SAFETY comment on unsafe impl Send + Sync - Document fallback behavior on resolve() method - Add address_resolver to create_client safety docs C#: - Expand AddressResolverDelegate XML docs (use cases, thread safety, error behavior, reconnection note) - Add tag on AddressResolver property - Extract addressResolver local variable to avoid capturing config.Request - Add comment on unsafe pointer write (outLen always valid from Rust) - Add comment explaining why _addressResolverDelegate is not readonly - Log specific error messages for each failure case (empty host, oversized host, invalid port) Signed-off-by: currantw --- rust/src/lib.rs | 30 ++++++++++++++++++++--- sources/Valkey.Glide/Client/BaseClient.cs | 4 ++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d297efb61..a5222207b 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -98,8 +98,27 @@ pub type FailureCallback = unsafe extern "C-unwind" fn( ) -> (); /// Callback for resolving server addresses before connection. -/// Returns the resolved port (0 on error), and writes the resolved host into `resolved_host_buf`. -/// `resolved_host_len` is set to the number of bytes written. +/// +/// Invoked synchronously during connection setup to translate a configured (host, port) pair +/// into the actual address to connect to. The callback writes the resolved hostname into the +/// provided buffer and returns the resolved port. +/// +/// # Arguments +/// * `host` is a pointer to the UTF-8 encoded hostname to resolve. +/// * `host_len` is the length of `host` in bytes. +/// * `port` is the configured port number. +/// * `resolved_host_buf` is a caller-allocated buffer to write the resolved hostname into. +/// * `resolved_host_buf_len` is the size of `resolved_host_buf` in bytes. +/// * `resolved_host_len` is an out-parameter set to the number of bytes written to `resolved_host_buf`. +/// +/// # Returns +/// The resolved port number, or `0` to signal an error (in which case the original address is used). +/// +/// # Safety +/// * `host` must be a valid pointer to `host_len` bytes of UTF-8 data. +/// * `resolved_host_buf` must be a valid pointer to a writable buffer of at least `resolved_host_buf_len` bytes. +/// * `resolved_host_len` must be a valid pointer to a writable `usize`. +/// * The callback must not store any of the provided pointers beyond the duration of the call. pub type AddressResolverCallback = unsafe extern "C-unwind" fn( host: *const u8, host_len: usize, @@ -109,6 +128,10 @@ pub type AddressResolverCallback = unsafe extern "C-unwind" fn( resolved_host_len: *mut usize, ) -> u16; +/// Maximum resolved hostname length in bytes. +const MAX_RESOLVED_HOST_LEN: usize = 1024; + +/// Implements the [`redis::AddressResolver`] trait. struct FFIAddressResolver { callback: AddressResolverCallback, } @@ -124,9 +147,10 @@ impl std::fmt::Debug for FFIAddressResolver { impl redis::AddressResolver for FFIAddressResolver { /// Resolves the given host and port by invoking the C# callback. + /// /// On error, falls back to the original `(host, port)` unchanged. fn resolve(&self, host: &str, port: u16) -> (String, u16) { - let mut buf = vec![0u8; 1024]; + let mut buf = [0u8; MAX_RESOLVED_HOST_LEN]; let mut len: usize = 0; let resolved_port = unsafe { (self.callback)( diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index d6a034380..5afd50ca7 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -205,12 +205,14 @@ protected static async Task CreateClient(BaseClientConfiguration config, F IntPtr addressResolverPointer = IntPtr.Zero; if (config.Request.AddressResolver != null) { + var addressResolver = config.Request.AddressResolver; + client._addressResolverDelegate = (hostPtr, hostLen, port, bufPtr, bufLen, outLen) => { try { string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen)!; - var (resolvedHost, resolvedPort) = config.Request.AddressResolver(host, port); + var (resolvedHost, resolvedPort) = addressResolver(host, port); if (string.IsNullOrEmpty(resolvedHost)) { From 93428ac5faa928ba7f577122bfcbac3a3a16f2b3 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 08:58:56 -0700 Subject: [PATCH 14/16] Fix lint error Signed-off-by: currantw --- sources/Valkey.Glide/Client/BaseClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index 5afd50ca7..3e4c4d270 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -211,7 +211,7 @@ protected static async Task CreateClient(BaseClientConfiguration config, F { try { - string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen)!; + string host = Marshal.PtrToStringUTF8(hostPtr, (int)hostLen); var (resolvedHost, resolvedPort) = addressResolver(host, port); if (string.IsNullOrEmpty(resolvedHost)) From e0a4e46b27ec0a6bd10174a41068c7e00aa4aab0 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 11:16:05 -0700 Subject: [PATCH 15/16] Try to reduce test flakiness Signed-off-by: currantw --- .../PubSubMemoryLeakFixValidationTests.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/Valkey.Glide.UnitTests/PubSubMemoryLeakFixValidationTests.cs b/tests/Valkey.Glide.UnitTests/PubSubMemoryLeakFixValidationTests.cs index 17761de2a..f49b6d04c 100644 --- a/tests/Valkey.Glide.UnitTests/PubSubMemoryLeakFixValidationTests.cs +++ b/tests/Valkey.Glide.UnitTests/PubSubMemoryLeakFixValidationTests.cs @@ -17,12 +17,8 @@ public void MarshalPubSubMessage_ProcessMultipleMessages_NoMemoryLeak() // Arrange const int messageCount = 10_000; - // Force initial GC to get baseline - GC.Collect(); - GC.WaitForPendingFinalizers(); - GC.Collect(); - - long initialMemory = GC.GetTotalMemory(false); + // Force initial GC to get a stable baseline + long initialMemory = GC.GetTotalMemory(forceFullCollection: true); // Act: Process multiple messages for (int i = 0; i < messageCount; i++) @@ -33,12 +29,8 @@ public void MarshalPubSubMessage_ProcessMultipleMessages_NoMemoryLeak() ProcessSingleMessage(message, channel, null); } - // Force GC to clean up any leaked memory - GC.Collect(); - GC.WaitForPendingFinalizers(); - GC.Collect(); - - long finalMemory = GC.GetTotalMemory(false); + // Force GC to clean up any leaked memory. + long finalMemory = GC.GetTotalMemory(forceFullCollection: true); long memoryGrowth = finalMemory - initialMemory; Console.WriteLine($"Processed {messageCount:N0} messages"); From f82a0e8c42dc4c6e6277a402a41eb7dd6c0c695a Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 26 May 2026 11:16:51 -0700 Subject: [PATCH 16/16] Simplify resolvedPort error condition Signed-off-by: currantw --- sources/Valkey.Glide/Client/BaseClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/Valkey.Glide/Client/BaseClient.cs b/sources/Valkey.Glide/Client/BaseClient.cs index 3e4c4d270..62124d35b 100644 --- a/sources/Valkey.Glide/Client/BaseClient.cs +++ b/sources/Valkey.Glide/Client/BaseClient.cs @@ -229,7 +229,7 @@ protected static async Task CreateClient(BaseClientConfiguration config, F return 0; } - if (resolvedPort <= 0) + if (resolvedPort == 0) { var msg = $"Address resolver returned an invalid port ({resolvedPort}) for {host}:{port}"; Logger.Log(Level.Error, "AddressResolver", msg);