From dbb678911bc020bf9365f1223ca7ca51acf34153 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Thu, 30 Jul 2026 02:03:08 +0300 Subject: [PATCH] Make AddHealthieRedis one method, because two could not be told apart AddHealthieRedis("localhost:6379") did not register a Redis connection. It registered "localhost:6379" as a key prefix. There were two overloads: one taking a configuration string and an optional prefix, one taking just an optional prefix. Given a single string argument C# prefers the second -- no optional parameter left unfilled -- so the connection string silently became the prefix, no IConnectionMultiplexer was registered, and the failure surfaced later as a missing service. That is the exact call in the package README, so the documented usage was broken. Nothing caught it because every test built RedisStateProvider directly. The provider was well covered and its registration was not covered at all. One method now, with configuration optional: pass it to have the connection opened, leave it out to use the one the application already registers. There is nothing left to resolve between. Three tests go through AddHealthieRedis rather than around it -- with a configuration string, with an existing connection, and with only a named prefix -- and the first of those fails against the two-overload version. Found by building the packages into a local feed and consuming them from a project outside the repository, which is the only thing that exercises what dotnet add package actually hands someone. --- src/Healthie.StateProviding.Redis/README.md | 2 + .../StartupExtensions.cs | 56 ++++++------- .../RedisStateProviderTests.cs | 83 +++++++++++++++++++ 3 files changed, 110 insertions(+), 31 deletions(-) diff --git a/src/Healthie.StateProviding.Redis/README.md b/src/Healthie.StateProviding.Redis/README.md index 3c78786..ee58a6c 100644 --- a/src/Healthie.StateProviding.Redis/README.md +++ b/src/Healthie.StateProviding.Redis/README.md @@ -28,6 +28,8 @@ If your application already registers an `IConnectionMultiplexer` — for its ow builder.Services.AddHealthieRedis(); // uses the registered IConnectionMultiplexer ``` +Both shapes are the same method: pass a configuration string to have the connection opened for you, or leave it out to use the registered one. Name the argument when you only want a different prefix — `AddHealthieRedis(keyPrefix: "myapp:health:")`. + Every key is prefixed, `healthie:state:` by default, so the provider stays out of the way of whatever else lives on that server. Pass your own as the last argument. ## Optimistic concurrency diff --git a/src/Healthie.StateProviding.Redis/StartupExtensions.cs b/src/Healthie.StateProviding.Redis/StartupExtensions.cs index 6d85055..15af3fc 100644 --- a/src/Healthie.StateProviding.Redis/StartupExtensions.cs +++ b/src/Healthie.StateProviding.Redis/StartupExtensions.cs @@ -18,53 +18,47 @@ public static class StartupExtensions /// /// The service collection to register with. /// - /// A StackExchange.Redis configuration string, such as localhost:6379. + /// A StackExchange.Redis configuration string, such as localhost:6379. Leave it out to use + /// an the application already registers -- for its own cache, + /// or through AddStackExchangeRedisCache. Sharing that one is the better option where it + /// exists: a second connection to the same server buys nothing. /// /// Prefixed to every key the provider owns. /// The service collection for fluent chaining. /// - /// Opens the connection here, once, and keeps it: an is - /// designed to be shared for the life of the application and is expensive to build per call. - /// Registered as a singleton so the container disposes it on shutdown. - /// - public static IServiceCollection AddHealthieRedis( - this IServiceCollection services, - string configuration, - string keyPrefix = DefaultKeyPrefix) - { - ArgumentNullException.ThrowIfNull(services); - ArgumentException.ThrowIfNullOrWhiteSpace(configuration); - - services.TryAddSingleton(_ => ConnectionMultiplexer.Connect(configuration)); - - return services.AddHealthieRedis(keyPrefix); - } - - /// - /// Registers a state provider backed by a Redis connection the application already owns. - /// - /// The service collection to register with. - /// Prefixed to every key the provider owns. - /// The service collection for fluent chaining. - /// /// - /// For an application that already registers an -- for its - /// own cache, or through AddStackExchangeRedisCache. Sharing it is the point: a second - /// connection to the same server buys nothing. + /// One method rather than one per case, because two overloads separated only by what a + /// means cannot be told apart at the call site. C# resolved + /// AddHealthieRedis("localhost:6379") to the overload whose single string was the key + /// prefix, so the connection string quietly became a prefix, no connection was registered, and + /// the failure arrived later as a missing service -- from the very call the README documents. + /// + /// + /// The connection is opened once and kept: an is built to be + /// shared for the life of the application and is expensive to create per call. It goes in with + /// TryAdd, so an application that already has one keeps it. /// /// - /// Registered with AddSingleton rather than TryAddSingleton on purpose: the - /// built-in in-memory provider registers with TryAdd, so whichever call comes first, this - /// one wins. + /// The provider is registered with AddSingleton rather than TryAddSingleton on + /// purpose: the built-in in-memory provider registers with TryAdd, so whichever call comes + /// first, this one wins. /// /// public static IServiceCollection AddHealthieRedis( this IServiceCollection services, + string? configuration = null, string keyPrefix = DefaultKeyPrefix) { ArgumentNullException.ThrowIfNull(services); ArgumentException.ThrowIfNullOrWhiteSpace(keyPrefix); + if (configuration is not null) + { + ArgumentException.ThrowIfNullOrWhiteSpace(configuration); + + services.TryAddSingleton(_ => ConnectionMultiplexer.Connect(configuration)); + } + return services.AddSingleton(provider => new RedisStateProvider(provider.GetRequiredService(), keyPrefix)); } diff --git a/tests/Healthie.Tests.Unit/RedisStateProviderTests.cs b/tests/Healthie.Tests.Unit/RedisStateProviderTests.cs index d05ce18..13a7258 100644 --- a/tests/Healthie.Tests.Unit/RedisStateProviderTests.cs +++ b/tests/Healthie.Tests.Unit/RedisStateProviderTests.cs @@ -1,7 +1,9 @@ using Healthie.Abstractions.Enums; using Healthie.Abstractions.Models; using Healthie.Abstractions.StateProviding; +using Healthie.DependencyInjection; using Healthie.StateProviding.Redis; +using Microsoft.Extensions.DependencyInjection; using StackExchange.Redis; using Testcontainers.Redis; @@ -61,6 +63,87 @@ private bool Unavailable() return false; } + /// + /// Through AddHealthieRedis, not by constructing the provider. Nothing did that before, + /// which is how a registration that never registered the connection shipped: the documented call + /// bound to the overload whose one string was the key prefix. + /// + [Fact] + public async Task AddHealthieRedis_WithAConfigurationString_ResolvesAWorkingProvider() + { + if (Unavailable()) + { + return; + } + + var services = new ServiceCollection(); + services.AddHealthie(typeof(RedisStateProviderTests).Assembly); + services.AddHealthieRedis(_redis!.GetConnectionString()); + + await using var host = services.BuildServiceProvider(); + + var provider = host.GetRequiredService(); + + Assert.IsType(provider); + + await provider.SetStateAsync("via-di", new PulseCheckerState(PulseInterval.Every30Seconds), Ct); + Assert.Equal( + PulseInterval.Every30Seconds, + (await provider.GetStateAsync("via-di", Ct))!.Interval); + } + + /// + /// The other shape: the application owns the connection and Healthie shares it. + /// + [Fact] + public async Task AddHealthieRedis_WithAnExistingConnection_SharesIt() + { + if (Unavailable()) + { + return; + } + + var services = new ServiceCollection(); + services.AddHealthie(typeof(RedisStateProviderTests).Assembly); + services.AddSingleton(_connection!); + services.AddHealthieRedis(); + + await using var host = services.BuildServiceProvider(); + + await host.GetRequiredService() + .SetStateAsync("shared", new PulseCheckerState(), Ct); + + // Read back through the prefix the registration defaults to, over the same connection. + Assert.NotNull(await Provider(Healthie.StateProviding.Redis.StartupExtensions.DefaultKeyPrefix) + .GetStateAsync("shared", Ct)); + } + + /// + /// A prefix is still a prefix when it is the only thing passed, and it must be named to be that. + /// + [Fact] + public async Task AddHealthieRedis_WithOnlyAPrefix_UsesTheRegisteredConnection() + { + if (Unavailable()) + { + return; + } + + var services = new ServiceCollection(); + services.AddHealthie(typeof(RedisStateProviderTests).Assembly); + services.AddSingleton(_connection!); + services.AddHealthieRedis(keyPrefix: "named-prefix:"); + + await using var host = services.BuildServiceProvider(); + + await host.GetRequiredService() + .SetStateAsync("k", new PulseCheckerState(), Ct); + + // Written under the prefix that was asked for, and nowhere else. + Assert.NotNull(await Provider("named-prefix:").GetStateAsync("k", Ct)); + Assert.Null(await Provider().GetStateAsync("k", Ct)); + } + [Fact] public async Task StateSurvivesARoundTrip() {