From 20e4b70b73fcaed74caaccb61bac05084c2a2740 Mon Sep 17 00:00:00 2001 From: avlcodemonkey Date: Sat, 3 Oct 2026 17:29:45 -0400 Subject: [PATCH 1/2] Improve caching behavior. --- README.md | 41 ++-- .../ExtensionsTests.cs | 4 +- .../FeatureDefinitionRefreshServiceTests.cs | 223 ++++++++++++++++++ .../HttpFeatureFlagClientTests.cs | 215 ----------------- src/FeatureFlags.Client/Extensions.cs | 5 +- .../FeatureDefinitionRefreshService.cs | 164 +++++++++++++ .../FeatureFlags.Client.csproj | 2 +- .../HttpFeatureFlagClient.cs | 62 +---- src/FeatureFlags.Client/IFeatureFlagClient.cs | 11 +- 9 files changed, 432 insertions(+), 295 deletions(-) create mode 100644 src/FeatureFlags.Client.Tests/FeatureDefinitionRefreshServiceTests.cs delete mode 100644 src/FeatureFlags.Client.Tests/HttpFeatureFlagClientTests.cs create mode 100644 src/FeatureFlags.Client/FeatureDefinitionRefreshService.cs diff --git a/README.md b/README.md index ee65b7c..6d34851 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,9 @@ Get started at https://featureflags.app, or if you want details first and vibes ## What This Library Does - Registers feature management services in ASP.NET Core via a single `AddFeatureFlags()` call. -- Fetches feature definitions from our API using an API key header (`x-api-key`). +- Fetches feature definitions from our API using an API key header (`x-api-key`), in a background service, and keeps the last-known-good copy if a refresh fails. - Exposes `IFeatureManager`/`IFeatureManagerSnapshot` usage patterns you already know from `Microsoft.FeatureManagement`. -- Includes a deterministic percentage filter (`Acmi.FeatureFlags.ConsistentPercentage`) and targeting support. +- Includes a deterministic percentage filter (`FeatureFlags.ConsistentPercentage`) and targeting support. ## Package And Runtime @@ -108,12 +108,15 @@ Also works with the normal ASP.NET Core feature management integrations: ## Cache Behavior -`HttpFeatureFlagClient` caches all retrieved definitions in memory under a single cache entry. +`AddFeatureFlags()` registers a hosted background service (`FeatureDefinitionRefreshService`) that keeps an in-memory snapshot of all feature definitions. -- First request fetches from remote API. -- Subsequent requests read from cache until expiration. -- Expiration defaults to 15 minutes. -- You can evict cache manually via `IFeatureFlagClient.ClearCache()`. +- Definitions are fetched at startup. Startup waits up to 5 seconds for the first fetch, then continues without it. +- They are refreshed every `CacheExpirationInMinutes` (default: `15`). +- Flag checks read the current snapshot. Evaluation never waits on an HTTP call. +- The snapshot is swapped atomically after each successful refresh. +- If a refresh fails (timeout, network error, 5xx, 401/403), the last-known-good snapshot stays in place and the next refresh is retried on the next tick. +- `IFeatureFlagClient.ClearCache()` requests an immediate background refresh. The old snapshot stays in place until that refresh succeeds. The method name is kept for compatibility. +- Flag changes reach your app within your refresh interval (15 minutes by default). Use a shorter interval for apps that rely on kill-switch flags. Example: @@ -128,7 +131,7 @@ public class AdminController : Controller { [HttpPost] public IActionResult RefreshFlags() { _featureFlagClient.ClearCache(); - return Ok(new { message = "Feature flag cache cleared." }); + return Accepted(new { message = "Feature flag refresh requested." }); } } ``` @@ -155,13 +158,13 @@ Use this filter when you want stable rollout behavior for authenticated users in ## Failure Semantics -When remote API calls fail: +When a refresh fails: -- Client logs an error. -- `GetAllFeatureDefinitionsAsync()` returns an empty list. -- `GetFeatureDefinitionByNameAsync()` returns `null`. +- The client logs a warning with the reason. During a long outage it logs the first failure and then at most once an hour. +- The last-known-good definitions keep being served, and the refresh is retried on the next tick. +- A log message is written when refreshes recover. -In practice this means feature checks degrade to "off" unless your app defines alternate behavior. This is generally safer than throwing exceptions into request pipelines and setting your pager on fire. +**Cold start with the API down:** if the app starts while FeatureFlags.app is unreachable (or the API key is invalid), no snapshot exists yet. `GetAllFeatureDefinitionsAsync()` returns an empty list, `GetFeatureDefinitionByNameAsync()` returns `null`, and all flags evaluate off until the first successful fetch. Nothing is thrown into the request pipeline. ## Common Issues And Fixes @@ -182,13 +185,13 @@ Fix: Possible causes: - API key invalid or missing permissions. -- API unavailable (client degrades to empty definitions). +- The API was unreachable the whole time since the app started, so no definitions have been loaded (see "Cold start" above). - Flag name mismatch (`"NewDashboard"` vs `"NewDashbaord"`, yes this typo happens a lot). Fix: - Verify API key and endpoint. -- Check app logs for "Error fetching feature definitions". +- Check app logs for "Failed to refresh feature definitions". - Centralize flag names in constants to avoid string-literal drift. ### 3. Rollout percentages look random per request @@ -202,16 +205,16 @@ Fix: - Ensure authenticated identity with stable `User.Identity.Name`. - If anonymous traffic dominates, choose filter strategy accordingly. -### 4. Flag updates not visible immediately +### 4. Flag updates are not visible right away Cause: -- Cached definitions not yet expired. +- Definitions refresh in the background every `CacheExpirationInMinutes` (15 by default), so a change shows up within that interval. Fix: -- Lower `CacheExpirationInMinutes` for development. -- Call `IFeatureFlagClient.ClearCache()` after admin updates when immediate refresh is required. +- Lower `CacheExpirationInMinutes` for development, or for apps that rely on kill-switch flags. +- Call `IFeatureFlagClient.ClearCache()` to request an immediate background refresh. It returns right away, and the new values appear once the refresh completes. ## Local Validation diff --git a/src/FeatureFlags.Client.Tests/ExtensionsTests.cs b/src/FeatureFlags.Client.Tests/ExtensionsTests.cs index f1ae01c..633a6cf 100644 --- a/src/FeatureFlags.Client.Tests/ExtensionsTests.cs +++ b/src/FeatureFlags.Client.Tests/ExtensionsTests.cs @@ -1,4 +1,3 @@ -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -60,7 +59,8 @@ public void AddFeatureFlags_RegistersServicesAndReturnsBuilder() { Assert.Same(builderMock.Object, result); Assert.Contains(services, s => s.ServiceType == typeof(IFeatureFlagClient)); Assert.Contains(services, s => s.ServiceType == typeof(IFeatureDefinitionProvider)); - Assert.Contains(services, s => s.ServiceType == typeof(IMemoryCache)); + Assert.Contains(services, s => s.ServiceType == typeof(FeatureDefinitionRefreshService)); + Assert.Contains(services, s => s.ServiceType == typeof(IHostedService) && s.ImplementationFactory is not null); Assert.Contains(services, s => s.ServiceType == typeof(IHttpClientFactory)); // Build the service provider and get the factory diff --git a/src/FeatureFlags.Client.Tests/FeatureDefinitionRefreshServiceTests.cs b/src/FeatureFlags.Client.Tests/FeatureDefinitionRefreshServiceTests.cs new file mode 100644 index 0000000..b205c78 --- /dev/null +++ b/src/FeatureFlags.Client.Tests/FeatureDefinitionRefreshServiceTests.cs @@ -0,0 +1,223 @@ +using System.Net; +using System.Net.Http.Json; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Moq; + +namespace Acmi.FeatureFlags.Client.Tests; + +public class FeatureDefinitionRefreshServiceTests { + private readonly Mock> _LoggerMock = new(); + + private sealed class StubHandler(Func> respond) : HttpMessageHandler { + public int Calls; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + Interlocked.Increment(ref Calls); + return respond(cancellationToken); + } + } + + private sealed class ManualTimeProvider : TimeProvider { + private long _Timestamp; + public override long GetTimestamp() => Interlocked.Read(ref _Timestamp); + public override long TimestampFrequency => TimeSpan.TicksPerSecond; + public void Advance(TimeSpan by) => Interlocked.Add(ref _Timestamp, by.Ticks); + } + + private static HttpResponseMessage Ok(params string[] names) + => new(HttpStatusCode.OK) { Content = JsonContent.Create(names.Select(n => new CustomFeatureDefinition { Name = n }).ToList()) }; + + private static HttpResponseMessage Status(HttpStatusCode code) => new(code); + + private FeatureDefinitionRefreshService CreateService(StubHandler handler, TimeProvider? timeProvider = null, string minutes = "15") { + var factory = new Mock(); + factory.Setup(f => f.CreateClient(Constants.HttpClientName)).Returns(() => new HttpClient(handler, disposeHandler: false) { BaseAddress = new Uri("http://localhost/") }); + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { { "FeatureFlags:CacheExpirationInMinutes", minutes } }) + .Build(); + return new FeatureDefinitionRefreshService(factory.Object, configuration, _LoggerMock.Object, timeProvider); + } + + private static async Task WaitUntilAsync(Func condition) { + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!condition()) { + Assert.True(DateTime.UtcNow < deadline, "Timed out waiting for condition"); + await Task.Delay(10, TestContext.Current.CancellationToken); + } + } + + private void VerifyWarnings(Times times) + => _LoggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), times); + + [Fact] + public async Task RefreshAsync_Success_ReplacesSnapshot() { + var responses = new Queue([Ok("A"), Ok("B", "C")]); + var service = CreateService(new StubHandler(_ => Task.FromResult(responses.Dequeue()))); + + Assert.True(await service.RefreshAsync(TestContext.Current.CancellationToken)); + Assert.Equal(["A"], service.GetDefinitions().Select(d => d.Name)); + + Assert.True(await service.RefreshAsync(TestContext.Current.CancellationToken)); + Assert.Equal(["B", "C"], service.GetDefinitions().Select(d => d.Name)); + Assert.Null(service.GetDefinition("A")); + Assert.NotNull(service.GetDefinition("c")); + } + + [Theory] + [InlineData(HttpStatusCode.InternalServerError)] + [InlineData(HttpStatusCode.Unauthorized)] + [InlineData(HttpStatusCode.Forbidden)] + public async Task RefreshAsync_BadStatus_KeepsPreviousSnapshot(HttpStatusCode failure) { + var responses = new Queue([Ok("A"), Status(failure)]); + var service = CreateService(new StubHandler(_ => Task.FromResult(responses.Dequeue()))); + await service.RefreshAsync(TestContext.Current.CancellationToken); + + Assert.False(await service.RefreshAsync(TestContext.Current.CancellationToken)); + + Assert.Equal(["A"], service.GetDefinitions().Select(d => d.Name)); + VerifyWarnings(Times.Once()); + } + + [Fact] + public async Task RefreshAsync_NetworkError_KeepsPreviousSnapshot() { + var calls = 0; + var service = CreateService(new StubHandler(_ => ++calls == 1 ? Task.FromResult(Ok("A")) : throw new HttpRequestException("boom"))); + await service.RefreshAsync(TestContext.Current.CancellationToken); + + Assert.False(await service.RefreshAsync(TestContext.Current.CancellationToken)); + + Assert.Equal(["A"], service.GetDefinitions().Select(d => d.Name)); + } + + [Fact] + public async Task RefreshAsync_RecoversAfterOutage() { + var responses = new Queue([Ok("A"), Status(HttpStatusCode.ServiceUnavailable), Status(HttpStatusCode.ServiceUnavailable), Ok("A", "B")]); + var service = CreateService(new StubHandler(_ => Task.FromResult(responses.Dequeue()))); + + await service.RefreshAsync(TestContext.Current.CancellationToken); + await service.RefreshAsync(TestContext.Current.CancellationToken); + await service.RefreshAsync(TestContext.Current.CancellationToken); + Assert.Single(service.GetDefinitions()); + + Assert.True(await service.RefreshAsync(TestContext.Current.CancellationToken)); + Assert.Equal(["A", "B"], service.GetDefinitions().Select(d => d.Name)); + } + + [Fact] + public async Task RefreshAsync_RepeatedFailures_LogsFirstThenThrottles() { + var time = new ManualTimeProvider(); + var service = CreateService(new StubHandler(_ => Task.FromResult(Status(HttpStatusCode.BadGateway))), time); + + await service.RefreshAsync(TestContext.Current.CancellationToken); + await service.RefreshAsync(TestContext.Current.CancellationToken); + await service.RefreshAsync(TestContext.Current.CancellationToken); + VerifyWarnings(Times.Once()); + + time.Advance(TimeSpan.FromHours(1)); + await service.RefreshAsync(TestContext.Current.CancellationToken); + VerifyWarnings(Times.Exactly(2)); + } + + [Fact] + public async Task ColdStart_ApiDown_EvaluatesOffWithoutThrowing() { + var service = CreateService(new StubHandler(_ => throw new HttpRequestException("down"))); + var client = new HttpFeatureFlagClient(service); + + Assert.False(await service.RefreshAsync(TestContext.Current.CancellationToken)); + + Assert.False(service.HasSnapshot); + Assert.Empty(await client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken)); + Assert.Null(await client.GetFeatureDefinitionByNameAsync("A", TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task Reads_DoNotBlockOnInFlightRefresh() { + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var calls = 0; + var handler = new StubHandler(_ => ++calls == 1 ? Task.FromResult(Ok("A")) : gate.Task); + var service = CreateService(handler); + var client = new HttpFeatureFlagClient(service); + await service.RefreshAsync(TestContext.Current.CancellationToken); + + var inFlight = service.RefreshAsync(TestContext.Current.CancellationToken); + await WaitUntilAsync(() => handler.Calls == 2); + + var read = client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken); + Assert.True(read.IsCompletedSuccessfully); + Assert.Equal(["A"], (await read).Select(d => d.Name)); + Assert.False(inFlight.IsCompleted); + + gate.SetResult(Ok("B")); + await inFlight; + Assert.Equal(["B"], service.GetDefinitions().Select(d => d.Name)); + } + + [Fact] + public async Task HostedService_RefreshesAtStartup_AndStopsCleanly() { + var handler = new StubHandler(_ => Task.FromResult(Ok("A"))); + var service = CreateService(handler); + + await service.StartAsync(TestContext.Current.CancellationToken); + + Assert.True(service.HasSnapshot); + await service.StopAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, handler.Calls); + } + + [Fact] + public async Task HostedService_RefreshesOnInterval() { + var handler = new StubHandler(_ => Task.FromResult(Ok("A"))); + var service = CreateService(handler, minutes: "0.0002"); + + await service.StartAsync(TestContext.Current.CancellationToken); + await WaitUntilAsync(() => handler.Calls >= 3); + await service.StopAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task ClearCache_TriggersImmediateRefresh() { + var calls = 0; + var handler = new StubHandler(_ => Task.FromResult(++calls == 1 ? Ok("A") : Ok("B"))); + var service = CreateService(handler); + var client = new HttpFeatureFlagClient(service); + await service.StartAsync(TestContext.Current.CancellationToken); + + Assert.True(client.ClearCache()); + await WaitUntilAsync(() => service.GetDefinition("B") is not null); + + Assert.Equal(["B"], service.GetDefinitions().Select(d => d.Name)); + await service.StopAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task ClearCache_DuringOutage_KeepsOldValues() { + var calls = 0; + var handler = new StubHandler(_ => Task.FromResult(++calls == 1 ? Ok("A") : Status(HttpStatusCode.InternalServerError))); + var service = CreateService(handler); + var client = new HttpFeatureFlagClient(service); + await service.StartAsync(TestContext.Current.CancellationToken); + + Assert.True(client.ClearCache()); + await WaitUntilAsync(() => handler.Calls >= 2); + + var definitions = await client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken); + Assert.Equal(["A"], definitions.Select(d => d.Name)); + await service.StopAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task StartAsync_DoesNotHangWhenApiNeverResponds() { + var handler = new StubHandler(async ct => { + await Task.Delay(Timeout.Infinite, ct); + return Ok(); + }); + var service = CreateService(handler); + + var started = service.StartAsync(TestContext.Current.CancellationToken); + await started.WaitAsync(TimeSpan.FromSeconds(15), TestContext.Current.CancellationToken); + + Assert.False(service.HasSnapshot); + await service.StopAsync(TestContext.Current.CancellationToken); + } +} diff --git a/src/FeatureFlags.Client.Tests/HttpFeatureFlagClientTests.cs b/src/FeatureFlags.Client.Tests/HttpFeatureFlagClientTests.cs deleted file mode 100644 index 955b9db..0000000 --- a/src/FeatureFlags.Client.Tests/HttpFeatureFlagClientTests.cs +++ /dev/null @@ -1,215 +0,0 @@ -using System.Net; -using System.Net.Http.Json; -using Microsoft.Extensions.Caching.Memory; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Logging; -using Moq; -using Moq.Protected; - -namespace Acmi.FeatureFlags.Client.Tests; - -public class HttpFeatureFlagClientTests { - [Fact] - public async Task GetAllFeatureDefinitionsAsync_ReturnsFeatureDefinitions_UsesCache() { - // Arrange - var customFeatures = new List - { - new() { Name = "FeatureA" }, - new() { Name = "FeatureB" } - }; - var handlerMock = new Mock(); - handlerMock.Protected() - .Setup>("SendAsync", - ItExpr.Is(req => req.RequestUri!.ToString().EndsWith("features")), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage { - StatusCode = HttpStatusCode.OK, - Content = JsonContent.Create(customFeatures) - }); - - var httpClient = new HttpClient(handlerMock.Object) { BaseAddress = new Uri("http://localhost/") }; - var httpClientFactoryMock = new Mock(); - httpClientFactoryMock.Setup(f => f.CreateClient(Constants.HttpClientName)).Returns(httpClient); - - var loggerMock = new Mock>(); - var configurationManager = new ConfigurationManager(); - configurationManager.AddInMemoryCollection(new Dictionary { - { "FeatureFlags:CacheExpirationInMinutes", "15" } - }); - - var memoryCache = new MemoryCache(new MemoryCacheOptions()); - var client = new HttpFeatureFlagClient(httpClientFactoryMock.Object, configurationManager, memoryCache, loggerMock.Object); - - // Act - var result1 = await client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken); - var result2 = await client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken); // Should hit cache - - // Assert - Assert.NotNull(result1); - Assert.Equal(2, result1.Count); - Assert.Contains(result1, f => f.Name == "FeatureA"); - Assert.Contains(result1, f => f.Name == "FeatureB"); - Assert.Equal(result1, result2); // Cached result should be the same - handlerMock.Protected().Verify("SendAsync", Times.Once(), - ItExpr.Is(req => req.RequestUri!.ToString().EndsWith("features")), - ItExpr.IsAny()); - } - - [Fact] - public async Task GetAllFeatureDefinitionsAsync_HandlesApiError_LogsAndReturnsEmpty() { - // Arrange - var handlerMock = new Mock(); - handlerMock.Protected() - .Setup>("SendAsync", - ItExpr.IsAny(), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage { - StatusCode = HttpStatusCode.InternalServerError - }); - - var httpClient = new HttpClient(handlerMock.Object) { BaseAddress = new Uri("http://localhost/") }; - var httpClientFactoryMock = new Mock(); - httpClientFactoryMock.Setup(f => f.CreateClient(Constants.HttpClientName)).Returns(httpClient); - - var loggerMock = new Mock>(); - var configurationManager = new ConfigurationManager(); - configurationManager.AddInMemoryCollection(new Dictionary { - { "FeatureFlags:CacheExpirationInMinutes", "15" } - }); - - var memoryCache = new MemoryCache(new MemoryCacheOptions()); - var client = new HttpFeatureFlagClient(httpClientFactoryMock.Object, configurationManager, memoryCache, loggerMock.Object); - - // Act - var result = await client.GetAllFeatureDefinitionsAsync(TestContext.Current.CancellationToken); - - // Assert - Assert.NotNull(result); - Assert.Empty(result); - loggerMock.Verify( - l => l.Log( - LogLevel.Error, - It.IsAny(), - It.Is((v, t) => v.ToString()!.Contains("Error fetching feature definitions")), - It.IsAny(), - It.IsAny>() - ), - Times.AtLeastOnce() - ); - handlerMock.Protected().Verify("SendAsync", Times.Once(), - ItExpr.Is(req => req.RequestUri!.ToString().EndsWith("features")), - ItExpr.IsAny()); - } - - [Fact] - public async Task GetFeatureDefinitionByNameAsync_ReturnsFeatureDefinition_FromCache() { - // Arrange - var customFeatures = new List - { - new() { Name = "FeatureX" }, - new() { Name = "FeatureY" } - }; - var handlerMock = new Mock(); - handlerMock.Protected() - .Setup>("SendAsync", - ItExpr.IsAny(), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage { - StatusCode = HttpStatusCode.OK, - Content = JsonContent.Create(customFeatures) - }); - - var httpClient = new HttpClient(handlerMock.Object) { BaseAddress = new Uri("http://localhost/") }; - var httpClientFactoryMock = new Mock(); - httpClientFactoryMock.Setup(f => f.CreateClient(Constants.HttpClientName)).Returns(httpClient); - - var loggerMock = new Mock>(); - var configurationManager = new ConfigurationManager(); - configurationManager.AddInMemoryCollection(new Dictionary { - { "FeatureFlags:CacheExpirationInMinutes", "15" } - }); - - var memoryCache = new MemoryCache(new MemoryCacheOptions()); - var client = new HttpFeatureFlagClient(httpClientFactoryMock.Object, configurationManager, memoryCache, loggerMock.Object); - - // Act - var result = await client.GetFeatureDefinitionByNameAsync("FeatureX", TestContext.Current.CancellationToken); - var result2 = await client.GetFeatureDefinitionByNameAsync("FeatureX", TestContext.Current.CancellationToken); // should hit cache - - // Assert - Assert.NotNull(result); - Assert.Equal("FeatureX", result.Name); - Assert.Equal(result, result2); - handlerMock.Protected().Verify("SendAsync", Times.Once(), - ItExpr.Is(req => req.RequestUri!.ToString().EndsWith("features")), - ItExpr.IsAny()); - } - - [Fact] - public async Task GetFeatureDefinitionByNameAsync_HandlesApiError_LogsAndReturnsNull() { - // Arrange - var handlerMock = new Mock(); - handlerMock.Protected() - .Setup>("SendAsync", - ItExpr.IsAny(), - ItExpr.IsAny()) - .ReturnsAsync(new HttpResponseMessage { - StatusCode = HttpStatusCode.InternalServerError - }); - - var httpClient = new HttpClient(handlerMock.Object) { BaseAddress = new Uri("http://localhost/") }; - var httpClientFactoryMock = new Mock(); - httpClientFactoryMock.Setup(f => f.CreateClient(Constants.HttpClientName)).Returns(httpClient); - - var loggerMock = new Mock>(); - var configurationManager = new ConfigurationManager(); - configurationManager.AddInMemoryCollection(new Dictionary { - { "FeatureFlags:CacheExpirationInMinutes", "15" } - }); - - var memoryCache = new MemoryCache(new MemoryCacheOptions()); - var client = new HttpFeatureFlagClient(httpClientFactoryMock.Object, configurationManager, memoryCache, loggerMock.Object); - - // Act - var result = await client.GetFeatureDefinitionByNameAsync("MissingFeature", TestContext.Current.CancellationToken); - - // Assert - Assert.Null(result); - loggerMock.Verify( - l => l.Log( - LogLevel.Error, - It.IsAny(), - It.Is((v, t) => v.ToString()!.Contains("Error fetching feature definitions") || v.ToString()!.Contains("Error getting feature definition")), - It.IsAny(), - It.IsAny>() - ), - Times.AtLeastOnce() - ); - handlerMock.Protected().Verify("SendAsync", Times.Once(), - ItExpr.Is(req => req.RequestUri!.ToString().EndsWith("features")), - ItExpr.IsAny()); - } - - [Fact] - public void ClearCache_ClearsCache() { - // Arrange - var httpClientFactoryMock = new Mock(); - var loggerMock = new Mock>(); - var configurationManager = new ConfigurationManager(); - configurationManager.AddInMemoryCollection(new Dictionary { - { "FeatureFlags:CacheExpirationInMinutes", "15" } - }); - var memoryCache = new MemoryCache(new MemoryCacheOptions()); - memoryCache.Set(Constants.FeatureDefinitionsCacheKey, ""); // Ensure cache entry exists - - // Act - var result1 = memoryCache.TryGetValue(Constants.FeatureDefinitionsCacheKey, out _); - var client = new HttpFeatureFlagClient(httpClientFactoryMock.Object, configurationManager, memoryCache, loggerMock.Object); - client.ClearCache(); - var result2 = memoryCache.TryGetValue(Constants.FeatureDefinitionsCacheKey, out _); - - // Assert - Assert.True(result1); - Assert.False(result2); - } -} diff --git a/src/FeatureFlags.Client/Extensions.cs b/src/FeatureFlags.Client/Extensions.cs index 858dd62..780b510 100644 --- a/src/FeatureFlags.Client/Extensions.cs +++ b/src/FeatureFlags.Client/Extensions.cs @@ -16,7 +16,7 @@ public static class Extensions { /// configuration (using the keys FeatureFlags:ApiBaseEndpoint and FeatureFlags:ApiKey, respectively). /// If either value is missing or invalid, an is thrown. The method registers an /// HTTP client with the specified base address and authorization header, as well as the required services for - /// feature flag management, including memory caching and scoped feature management services. + /// feature flag management, including a background service that refreshes definitions and scoped feature management services. /// The used to configure the application. /// The instance, allowing for method chaining. /// Thrown if configuration value for FeatureFlags:ApiBaseEndpoint or FeatureFlags:ApiKey is null, empty, or whitespace. @@ -40,7 +40,8 @@ public static IHostApplicationBuilder AddFeatureFlags(this IHostApplicationBuild // Register the feature management services builder.Services - .AddMemoryCache() + .AddSingleton() + .AddHostedService(sp => sp.GetRequiredService()) .AddScoped() .AddScoped() .AddScopedFeatureManagement() diff --git a/src/FeatureFlags.Client/FeatureDefinitionRefreshService.cs b/src/FeatureFlags.Client/FeatureDefinitionRefreshService.cs new file mode 100644 index 0000000..6e17d17 --- /dev/null +++ b/src/FeatureFlags.Client/FeatureDefinitionRefreshService.cs @@ -0,0 +1,164 @@ +using System.Collections.Frozen; +using System.Net.Http.Json; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.FeatureManagement; + +namespace Acmi.FeatureFlags.Client; + +/// +/// Hosted service that keeps an in-memory snapshot of feature definitions up to date. +/// +/// +/// Definitions are fetched at startup and then every FeatureFlags:CacheExpirationInMinutes (default 15). +/// Readers use the current snapshot and never wait on HTTP. A failed refresh keeps the last-known-good snapshot. +/// If no refresh has ever succeeded (for example the API is down at startup), there is no snapshot and flags evaluate off. +/// +public sealed class FeatureDefinitionRefreshService(IHttpClientFactory httpClientFactory, IConfiguration configuration, ILogger logger, TimeProvider? timeProvider = null) : BackgroundService { + private const double _DefaultRefreshMinutes = 15; + private static readonly TimeSpan _StartupWait = TimeSpan.FromSeconds(5); + private static readonly TimeSpan _RequestTimeout = TimeSpan.FromSeconds(30); + private static readonly TimeSpan _RepeatWarningInterval = TimeSpan.FromHours(1); + + private readonly IHttpClientFactory _HttpClientFactory = httpClientFactory; + private readonly IConfiguration _Configuration = configuration; + private readonly ILogger _Logger = logger; + private readonly TimeProvider _TimeProvider = timeProvider ?? TimeProvider.System; + private readonly SemaphoreSlim _RefreshSignal = new(0, 1); + private readonly TaskCompletionSource _InitialRefreshCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + private Snapshot? _Snapshot; + private int _ConsecutiveFailures; + private long _LastWarningTimestamp; + + /// + /// Gets whether at least one refresh has succeeded. + /// + public bool HasSnapshot => Volatile.Read(ref _Snapshot) is not null; + + /// + /// Gets the definitions in the current snapshot, or an empty list if no refresh has succeeded yet. + /// + public IReadOnlyList GetDefinitions() => Volatile.Read(ref _Snapshot)?.Definitions ?? []; + + /// + /// Gets a definition from the current snapshot by name (case-insensitive), or null if it isn't found. + /// + public FeatureDefinition? GetDefinition(string name) + => Volatile.Read(ref _Snapshot)?.ByName.GetValueOrDefault(name); + + /// + /// Asks the background loop to refresh immediately. The current snapshot stays in place until the refresh succeeds. + /// + public void RequestRefresh() { + try { + _RefreshSignal.Release(); + } catch (SemaphoreFullException) { + // a refresh is already requested + } + } + + /// + public override async Task StartAsync(CancellationToken cancellationToken) { + await base.StartAsync(cancellationToken); + + // give the first fetch a short chance to finish so flags are available as soon as the app starts taking requests, + // but never hold up startup for long when the API is slow or down + try { + await _InitialRefreshCompleted.Task.WaitAsync(_StartupWait, cancellationToken); + } catch (TimeoutException) { + // continue starting; the refresh keeps running in the background + } + } + + /// + protected override async Task ExecuteAsync(CancellationToken stoppingToken) { + try { + await RefreshAsync(stoppingToken); + _InitialRefreshCompleted.TrySetResult(); + + while (!stoppingToken.IsCancellationRequested) { + await _RefreshSignal.WaitAsync(RefreshInterval, stoppingToken); + await RefreshAsync(stoppingToken); + } + } catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) { + // shutting down + } finally { + _InitialRefreshCompleted.TrySetResult(); + } + } + + /// + /// Fetches definitions and swaps the snapshot on success. On failure the previous snapshot is kept. + /// + /// True if the refresh succeeded, else false. + public async Task RefreshAsync(CancellationToken cancellationToken = default) { + try { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeout.CancelAfter(_RequestTimeout); + + var httpClient = _HttpClientFactory.CreateClient(Constants.HttpClientName); + using var response = await httpClient.GetAsync("features", timeout.Token); + response.EnsureSuccessStatusCode(); + + var featureFlags = await response.Content.ReadFromJsonAsync>(timeout.Token) ?? []; + Volatile.Write(ref _Snapshot, new Snapshot(featureFlags.Select(FeatureDefinitionMapper.ToFeatureDefinition).ToArray())); + + if (Interlocked.Exchange(ref _ConsecutiveFailures, 0) > 0) { + _Logger.LogInformation("Feature definition refresh recovered"); + } + return true; + } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { + throw; + } catch (Exception ex) { + LogRefreshFailure(ex); + return false; + } + } + + private TimeSpan RefreshInterval { + get { + var minutes = _Configuration.GetValue("FeatureFlags:CacheExpirationInMinutes", _DefaultRefreshMinutes); + return TimeSpan.FromMinutes(minutes > 0 ? minutes : _DefaultRefreshMinutes); + } + } + + // log the first failure, then at most once per hour while the outage continues + private void LogRefreshFailure(Exception ex) { + var failures = Interlocked.Increment(ref _ConsecutiveFailures); + var now = _TimeProvider.GetTimestamp(); + if (failures > 1 && _TimeProvider.GetElapsedTime(Volatile.Read(ref _LastWarningTimestamp), now) < _RepeatWarningInterval) { + return; + } + Volatile.Write(ref _LastWarningTimestamp, now); + + var reason = ex is OperationCanceledException ? "request timed out" : ex.Message; + _Logger.LogWarning( + "Failed to refresh feature definitions ({Reason}); {State}. Consecutive failures: {Failures}", + reason, + HasSnapshot ? "keeping last-known-good definitions" : "no definitions loaded yet, all flags evaluate off", + failures); + } + + /// + public override void Dispose() { + _RefreshSignal.Dispose(); + base.Dispose(); + } + + // immutable once built; swapped atomically + private sealed class Snapshot { + public Snapshot(FeatureDefinition[] definitions) { + Definitions = definitions; + var byName = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var definition in definitions) { + byName.TryAdd(definition.Name, definition); + } + ByName = byName.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + } + + public IReadOnlyList Definitions { get; } + public FrozenDictionary ByName { get; } + } +} diff --git a/src/FeatureFlags.Client/FeatureFlags.Client.csproj b/src/FeatureFlags.Client/FeatureFlags.Client.csproj index ba283ba..8c3f92c 100644 --- a/src/FeatureFlags.Client/FeatureFlags.Client.csproj +++ b/src/FeatureFlags.Client/FeatureFlags.Client.csproj @@ -5,7 +5,7 @@ enable Acmi.FeatureFlags.Client Acmi.FeatureFlags.Client - 1.0.0.38 + 1.1.0.0 True true true diff --git a/src/FeatureFlags.Client/HttpFeatureFlagClient.cs b/src/FeatureFlags.Client/HttpFeatureFlagClient.cs index 7ea864d..266c986 100644 --- a/src/FeatureFlags.Client/HttpFeatureFlagClient.cs +++ b/src/FeatureFlags.Client/HttpFeatureFlagClient.cs @@ -1,67 +1,25 @@ -using System.Net.Http.Json; -using Microsoft.Extensions.Caching.Memory; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Logging; using Microsoft.FeatureManagement; namespace Acmi.FeatureFlags.Client; /// -public class HttpFeatureFlagClient(IHttpClientFactory httpClientFactory, IConfiguration configuration, IMemoryCache memoryCache, ILogger logger) : IFeatureFlagClient { - private readonly IHttpClientFactory _HttpClientFactory = httpClientFactory; - private readonly IConfiguration _Configuration = configuration; - private readonly IMemoryCache _MemoryCache = memoryCache; - private readonly ILogger _Logger = logger; +/// +/// Reads from the in-memory snapshot maintained by ; it never makes an HTTP call itself. +/// +public class HttpFeatureFlagClient(FeatureDefinitionRefreshService refreshService) : IFeatureFlagClient { + private readonly FeatureDefinitionRefreshService _RefreshService = refreshService; /// - public async Task> GetAllFeatureDefinitionsAsync(CancellationToken cancellationToken = default) { - try { - var definitions = await _MemoryCache.GetOrCreateAsync(Constants.FeatureDefinitionsCacheKey, async entry => { - entry.AbsoluteExpirationRelativeToNow = CacheTimeSpan; - return await FetchFeatureDefinitionsAsync(cancellationToken); - }); - - return definitions ?? []; - } catch (Exception ex) { - _Logger.LogError(ex, "Error getting feature definitions"); - return []; - } - } + public Task> GetAllFeatureDefinitionsAsync(CancellationToken cancellationToken = default) + => Task.FromResult(_RefreshService.GetDefinitions().ToList()); /// - public async Task GetFeatureDefinitionByNameAsync(string name, CancellationToken cancellationToken = default) { - try { - var definitions = await _MemoryCache.GetOrCreateAsync(Constants.FeatureDefinitionsCacheKey, async entry => { - entry.AbsoluteExpirationRelativeToNow = CacheTimeSpan; - return await FetchFeatureDefinitionsAsync(cancellationToken); - }); - return definitions?.FirstOrDefault(x => x.Name.Equals(name, StringComparison.OrdinalIgnoreCase)); - } catch (Exception ex) { - _Logger.LogError(ex, "Error getting feature definition for '{Name}'", name); - return null; - } - } + public Task GetFeatureDefinitionByNameAsync(string name, CancellationToken cancellationToken = default) + => Task.FromResult(_RefreshService.GetDefinition(name)); /// public bool ClearCache() { - _MemoryCache.Remove(Constants.FeatureDefinitionsCacheKey); + _RefreshService.RequestRefresh(); return true; } - - private TimeSpan? _CacheTimeSpan; - private TimeSpan CacheTimeSpan => _CacheTimeSpan ??= TimeSpan.FromMinutes(_Configuration.GetValue("FeatureFlags:CacheExpirationInMinutes", 15)); - - private async Task> FetchFeatureDefinitionsAsync(CancellationToken cancellationToken = default) { - try { - var httpClient = _HttpClientFactory.CreateClient(Constants.HttpClientName); - using var response = await httpClient.GetAsync("features", cancellationToken); - response.EnsureSuccessStatusCode(); - - var featureFlags = await response.Content.ReadFromJsonAsync>(cancellationToken) ?? []; - return featureFlags.Select(FeatureDefinitionMapper.ToFeatureDefinition).ToList(); - } catch (Exception ex) { - _Logger.LogError(ex, "Error fetching feature definitions"); - return []; - } - } } diff --git a/src/FeatureFlags.Client/IFeatureFlagClient.cs b/src/FeatureFlags.Client/IFeatureFlagClient.cs index f083624..13b3f87 100644 --- a/src/FeatureFlags.Client/IFeatureFlagClient.cs +++ b/src/FeatureFlags.Client/IFeatureFlagClient.cs @@ -7,14 +7,15 @@ namespace Acmi.FeatureFlags.Client; /// public interface IFeatureFlagClient { /// - /// Gets all feature definitions from the remote service. + /// Gets all feature definitions from the current in-memory snapshot. Never waits on HTTP. + /// Returns an empty list if no refresh has succeeded yet. /// /// that can be used to cancel the operation. Default value is . /// Task> GetAllFeatureDefinitionsAsync(CancellationToken cancellationToken = default); /// - /// Get a feature definition by its name from the remote service. + /// Gets a feature definition by its name from the current in-memory snapshot. Never waits on HTTP. /// /// Name of feature. /// that can be used to cancel the operation. Default value is . @@ -22,8 +23,10 @@ public interface IFeatureFlagClient { Task GetFeatureDefinitionByNameAsync(string name, CancellationToken cancellationToken = default); /// - /// Clear the cache of feature definitions. + /// Requests an immediate refresh of feature definitions from the remote service. + /// The refresh runs in the background. The existing snapshot stays in place until it succeeds, + /// so if the service is unreachable the previous definitions keep being used. /// - /// True if successful, else false. + /// True once the refresh has been requested. bool ClearCache(); } From a2856369cfcf497e385a6f89a3477a27de84d717 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:41:11 +0000 Subject: [PATCH 2/2] Clarify feature flag refresh cadence docs Co-authored-by: avlcodemonkey <6305631+avlcodemonkey@users.noreply.github.com> --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6d34851..8de2605 100644 --- a/README.md +++ b/README.md @@ -111,12 +111,12 @@ Also works with the normal ASP.NET Core feature management integrations: `AddFeatureFlags()` registers a hosted background service (`FeatureDefinitionRefreshService`) that keeps an in-memory snapshot of all feature definitions. - Definitions are fetched at startup. Startup waits up to 5 seconds for the first fetch, then continues without it. -- They are refreshed every `CacheExpirationInMinutes` (default: `15`). +- After each refresh completes, the next periodic refresh starts after `CacheExpirationInMinutes` (default: `15`); requests time out after 30 seconds. - Flag checks read the current snapshot. Evaluation never waits on an HTTP call. - The snapshot is swapped atomically after each successful refresh. - If a refresh fails (timeout, network error, 5xx, 401/403), the last-known-good snapshot stays in place and the next refresh is retried on the next tick. - `IFeatureFlagClient.ClearCache()` requests an immediate background refresh. The old snapshot stays in place until that refresh succeeds. The method name is kept for compatibility. -- Flag changes reach your app within your refresh interval (15 minutes by default). Use a shorter interval for apps that rely on kill-switch flags. +- Flag changes typically reach your app within the configured interval plus the time taken by the next refresh (up to 30 seconds), assuming the API responds successfully. Use a shorter interval for apps that rely on kill-switch flags. Example: @@ -209,7 +209,7 @@ Fix: Cause: -- Definitions refresh in the background every `CacheExpirationInMinutes` (15 by default), so a change shows up within that interval. +- After each refresh completes, the next periodic refresh starts after `CacheExpirationInMinutes` (15 by default). A change may take that interval plus the time taken by the next refresh (up to 30 seconds) to show up. Fix: