From 3534ae380f2b47b2daf84a4c4b27a1ca4e665476 Mon Sep 17 00:00:00 2001 From: Stuart Turner Date: Wed, 7 Oct 2026 09:10:24 -0500 Subject: [PATCH] Fix return behavior in failure case --- .editorconfig | 5 +- .../ApplicationCache.cs | 9 ++- .../ApplicationCacheTests.cs | 61 +++++++++++++++++++ .../DelayGetValueCache.cs | 6 -- .../FailingLoadCache.cs | 48 +++++++++++++++ .../GetValue.cs | 15 ----- .../GetValueCache.cs | 19 ++++-- 7 files changed, 133 insertions(+), 30 deletions(-) create mode 100644 tests/Immediate.Cache.FunctionalTests/FailingLoadCache.cs delete mode 100644 tests/Immediate.Cache.FunctionalTests/GetValue.cs diff --git a/.editorconfig b/.editorconfig index d16aa5c..d41d425 100644 --- a/.editorconfig +++ b/.editorconfig @@ -260,8 +260,8 @@ csharp_style_prefer_top_level_statements = true:silent csharp_style_prefer_tuple_swap = true:suggestion csharp_style_prefer_utf8_string_literals = true:suggestion csharp_style_throw_expression = true:warning -csharp_style_unused_value_assignment_preference = discard_variable:warning -csharp_style_unused_value_expression_statement_preference = discard_variable:warning +csharp_style_unused_value_assignment_preference = discard_variable:suggestion +csharp_style_unused_value_expression_statement_preference = discard_variable:suggestion # New Lines csharp_style_allow_embedded_statements_on_same_line_experimental = true:silent @@ -321,6 +321,7 @@ dotnet_diagnostic.CA2007.severity = error # CA2007: Consider calli [tests/**.cs] +dotnet_diagnostic.CA1062.severity = none # CA1062: Validate arguments of public methods dotnet_diagnostic.CA1707.severity = none # CA1707: Identifiers should not contain underscores dotnet_diagnostic.CA1724.severity = none # CA1724: Type names should not match namespaces dotnet_diagnostic.CA1822.severity = none # CA1822: Mark members as static diff --git a/src/Immediate.Cache.Shared/ApplicationCache.cs b/src/Immediate.Cache.Shared/ApplicationCache.cs index 319e218..8e2ba62 100644 --- a/src/Immediate.Cache.Shared/ApplicationCache.cs +++ b/src/Immediate.Cache.Shared/ApplicationCache.cs @@ -224,9 +224,16 @@ private async Task RunHandler(CancellationTokenSource tokenSource) { lock (_lock) { + // if token source has been cancelled (aka `RemoveValue` or `SetValue`) + // then finish the loop and quite or try again if (!tokenSource.IsCancellationRequested) + { _responseSource?.SetException(ex); - return; + + // only return if we have an exception and that is the final value + // of this handler + return; + } } } diff --git a/tests/Immediate.Cache.FunctionalTests/ApplicationCacheTests.cs b/tests/Immediate.Cache.FunctionalTests/ApplicationCacheTests.cs index 8d06170..a5574c8 100644 --- a/tests/Immediate.Cache.FunctionalTests/ApplicationCacheTests.cs +++ b/tests/Immediate.Cache.FunctionalTests/ApplicationCacheTests.cs @@ -421,4 +421,65 @@ public async Task TransformWorksWhenMultipleSimultaneous() Assert.Equal(12, transformedResponse2.Value); Assert.Equal(2, transformation2.TimesExecuted); } + + [Fact] + public async Task GetValueWorksWhenRepeatedCallAfterFail() + { + var query = new FailingLoad.Query { Key = "Load fails" }; + + var cache = _serviceProvider.GetRequiredService(); + var waiting = cache.GetValue(query, TestContext.Current.CancellationToken); + + await query.Started.Task; + + query.Release.SetResult(); + + // caller waiting during the load + await ValidateFailingLoad(waiting); + + // caller arriving afterwards + await ValidateFailingLoad(cache.GetValue(query, TestContext.Current.CancellationToken)); + + cache.RemoveValue(query); + + // caller after another RemoveValue + await ValidateFailingLoad(cache.GetValue(query, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task GetValueWorksWhenRepeatedCallAfterFailAndRemove() + { + var query = new FailingLoad.Query { Key = "Load fails after RemoveValue" }; + + var cache = _serviceProvider.GetRequiredService(); + var waiting = cache.GetValue(query, TestContext.Current.CancellationToken); + + await query.Started.Task; + + cache.RemoveValue(query); + + query.Release.SetResult(); + + // caller waiting during the load + await ValidateFailingLoad(waiting); + + // caller arriving afterwards + await ValidateFailingLoad(cache.GetValue(query, TestContext.Current.CancellationToken)); + + cache.RemoveValue(query); + + // caller after another RemoveValue + await ValidateFailingLoad(cache.GetValue(query, TestContext.Current.CancellationToken)); + } + + private static async Task ValidateFailingLoad(ValueTask task) + { + var ex = await Assert.ThrowsAsync( + async () => await task + .AsTask() + .WaitAsync(TimeSpan.FromMinutes(3), TestContext.Current.CancellationToken) + ); + + Assert.Equal(FailingLoad.FailingLoadMessage, ex.Message); + } } diff --git a/tests/Immediate.Cache.FunctionalTests/DelayGetValueCache.cs b/tests/Immediate.Cache.FunctionalTests/DelayGetValueCache.cs index 1478dce..1ac21f7 100644 --- a/tests/Immediate.Cache.FunctionalTests/DelayGetValueCache.cs +++ b/tests/Immediate.Cache.FunctionalTests/DelayGetValueCache.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using Immediate.Cache.Shared; namespace Immediate.Cache.FunctionalTests; @@ -6,11 +5,6 @@ namespace Immediate.Cache.FunctionalTests; [CacheFor] public sealed partial class DelayGetValueCache { - [SuppressMessage( - "Design", - "CA1062:Validate arguments of public methods", - Justification = "Not a public method" - )] protected override string TransformKey(DelayGetValue.Query request) => $"DelayGetValue(query: {request.Value})"; diff --git a/tests/Immediate.Cache.FunctionalTests/FailingLoadCache.cs b/tests/Immediate.Cache.FunctionalTests/FailingLoadCache.cs new file mode 100644 index 0000000..3924b29 --- /dev/null +++ b/tests/Immediate.Cache.FunctionalTests/FailingLoadCache.cs @@ -0,0 +1,48 @@ +using Immediate.Cache.Shared; +using Immediate.Handlers.Shared; + +namespace Immediate.Cache.FunctionalTests; + +[Handler] +public sealed partial class FailingLoad +{ + public const string FailingLoadMessage = "load failed"; + + public sealed class Query + { + public required string Key { get; init; } + public TaskCompletionSource Started { get; } = new(); + public TaskCompletionSource Release { get; } = new(); + } + + private async ValueTask HandleAsync(Query query, CancellationToken _) + { + query.Started.TrySetResult(); + + // Stands in for work that fails without observing the token, such as a database query whose rows changed underneath it. + await query.Release.Task; + + throw new FailingLoadException(FailingLoadMessage); + } + + public sealed class FailingLoadException : Exception + { + public FailingLoadException() + { + } + + public FailingLoadException(string message) : base(message) + { + } + + public FailingLoadException(string message, Exception innerException) : base(message, innerException) + { + } + } +} + +[CacheFor] +public sealed partial class FailingLoadCache +{ + protected override string TransformKey(FailingLoad.Query request) => request.Key; +} diff --git a/tests/Immediate.Cache.FunctionalTests/GetValue.cs b/tests/Immediate.Cache.FunctionalTests/GetValue.cs deleted file mode 100644 index 1fefe51..0000000 --- a/tests/Immediate.Cache.FunctionalTests/GetValue.cs +++ /dev/null @@ -1,15 +0,0 @@ -using Immediate.Handlers.Shared; - -namespace Immediate.Cache.FunctionalTests; - -[Handler] -public sealed partial class GetValue -{ - public sealed record Query(int Value); - public sealed record Response(int Value, bool ExecutedHandler); - - private ValueTask HandleAsync( - Query query, - CancellationToken _ - ) => ValueTask.FromResult(new Response(query.Value, ExecutedHandler: true)); -} diff --git a/tests/Immediate.Cache.FunctionalTests/GetValueCache.cs b/tests/Immediate.Cache.FunctionalTests/GetValueCache.cs index cb0ccc5..0580faf 100644 --- a/tests/Immediate.Cache.FunctionalTests/GetValueCache.cs +++ b/tests/Immediate.Cache.FunctionalTests/GetValueCache.cs @@ -1,16 +1,23 @@ -using System.Diagnostics.CodeAnalysis; using Immediate.Cache.Shared; +using Immediate.Handlers.Shared; namespace Immediate.Cache.FunctionalTests; +[Handler] +public sealed partial class GetValue +{ + public sealed record Query(int Value); + public sealed record Response(int Value, bool ExecutedHandler); + + private ValueTask HandleAsync( + Query query, + CancellationToken _ + ) => ValueTask.FromResult(new Response(query.Value, ExecutedHandler: true)); +} + [CacheFor] public sealed partial class GetValueCache { - [SuppressMessage( - "Design", - "CA1062:Validate arguments of public methods", - Justification = "Not a public method" - )] protected override string TransformKey(GetValue.Query request) => $"GetValue(query: {request.Value})"; }