From 488bc103fb6f8126aa528a52efd904f19c0d0183 Mon Sep 17 00:00:00 2001 From: martintmk <103487740+martintmk@users.noreply.github.com> Date: Fri, 16 Jun 2023 15:44:55 +0200 Subject: [PATCH] Enhance `OnHedgingArguments` (#1314) --- .../Hedging/Controller/HedgingHandler.cs | 7 ++- .../Hedging/Controller/TaskExecution.cs | 5 +- .../HedgingActionGeneratorArguments.cs | 11 +++- .../Hedging/HedgingResilienceStrategy.cs | 2 +- .../Hedging/HedgingStrategyOptions.TResult.cs | 2 +- src/Polly.Core/Hedging/OnHedgingArguments.cs | 6 +- .../HedgingExecutionContextTests.cs | 6 +- .../Hedging/Controller/TaskExecutionTests.cs | 4 +- .../HedgingActionGeneratorArgumentsTests.cs | 5 +- .../Hedging/HedgingActions.cs | 4 +- .../Hedging/HedgingHandlerTests.cs | 8 +-- ...esilienceStrategyBuilderExtensionsTests.cs | 2 +- .../Hedging/HedgingResilienceStrategyTests.cs | 60 +++++++++++++++---- .../Hedging/HedgingStrategyOptionsTests.cs | 2 +- 14 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/Polly.Core/Hedging/Controller/HedgingHandler.cs b/src/Polly.Core/Hedging/Controller/HedgingHandler.cs index 60e031b4ee8..b80abbe9972 100644 --- a/src/Polly.Core/Hedging/Controller/HedgingHandler.cs +++ b/src/Polly.Core/Hedging/Controller/HedgingHandler.cs @@ -16,7 +16,8 @@ internal sealed record class HedgingHandler( if (IsGeneric) { var copiedArgs = new HedgingActionGeneratorArguments( - args.Context, + args.PrimaryContext, + args.ActionContext, args.Attempt, (Func>>)(object)args.Callback); @@ -29,7 +30,7 @@ internal sealed record class HedgingHandler( private Func>>? CreateNonGenericAction(HedgingActionGeneratorArguments args) { var generator = (Func, Func>>?>)(object)ActionGenerator; - var action = generator(new HedgingActionGeneratorArguments(args.Context, args.Attempt, async context => + var action = generator(new HedgingActionGeneratorArguments(args.PrimaryContext, args.ActionContext, args.Attempt, async context => { var outcome = await args.Callback(context).ConfigureAwait(context.ContinueOnCapturedContext); return outcome.AsOutcome(); @@ -42,7 +43,7 @@ internal sealed record class HedgingHandler( return async () => { - var outcome = await action().ConfigureAwait(args.Context.ContinueOnCapturedContext); + var outcome = await action().ConfigureAwait(args.ActionContext.ContinueOnCapturedContext); return outcome.AsOutcome(); }; } diff --git a/src/Polly.Core/Hedging/Controller/TaskExecution.cs b/src/Polly.Core/Hedging/Controller/TaskExecution.cs index 8162a655cb4..2ec0476865d 100644 --- a/src/Polly.Core/Hedging/Controller/TaskExecution.cs +++ b/src/Polly.Core/Hedging/Controller/TaskExecution.cs @@ -100,7 +100,7 @@ public async ValueTask InitializeAsync( try { - action = _handler.GenerateAction(CreateArguments(primaryCallback, state, attempt)); + action = _handler.GenerateAction(CreateArguments(primaryCallback, snapshot.Context, state, attempt)); if (action == null) { await ResetAsync().ConfigureAwait(false); @@ -125,8 +125,9 @@ public async ValueTask InitializeAsync( private HedgingActionGeneratorArguments CreateArguments( Func>> primaryCallback, + ResilienceContext primaryContext, TState state, - int attempt) => new(Context, attempt, (context) => primaryCallback(context, state)); + int attempt) => new(primaryContext, Context, attempt, (context) => primaryCallback(context, state)); public async ValueTask ResetAsync() { diff --git a/src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs b/src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs index 7b49c1577bb..de2626fd0bb 100644 --- a/src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs +++ b/src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs @@ -4,10 +4,17 @@ namespace Polly.Hedging; /// Represents arguments used in the hedging resilience strategy. /// /// The type of the result. -/// The context associated with the execution of a user-provided callback. +/// The primary resilience context. +/// +/// The context that will be passed to action generated by . +/// This context is cloned from . /// The zero-based hedging attempt number. /// The callback passed to hedging strategy. /// /// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility. /// -public readonly record struct HedgingActionGeneratorArguments(ResilienceContext Context, int Attempt, Func>> Callback); +public readonly record struct HedgingActionGeneratorArguments( + ResilienceContext PrimaryContext, + ResilienceContext ActionContext, + int Attempt, + Func>> Callback); diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs index 5800ff3632b..90c442783f1 100644 --- a/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs +++ b/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs @@ -84,7 +84,7 @@ protected internal override async ValueTask> ExecuteCoreAsync(context, outcome, new OnHedgingArguments(hedgingContext.LoadedTasks - 1)); + var onHedgingArgs = new OutcomeArguments(context, outcome, new OnHedgingArguments(context, hedgingContext.LoadedTasks - 1)); _telemetry.Report(HedgingConstants.OnHedgingEventName, onHedgingArgs); if (OnHedging is not null) diff --git a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs index 44ec334f2b6..2f2657381f0 100644 --- a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs +++ b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs @@ -61,7 +61,7 @@ public class HedgingStrategyOptions : ResilienceStrategyOptions { return async () => { - return await args.Callback(args.Context).ConfigureAwait(args.Context.ContinueOnCapturedContext); + return await args.Callback(args.ActionContext).ConfigureAwait(args.ActionContext.ContinueOnCapturedContext); }; }; diff --git a/src/Polly.Core/Hedging/OnHedgingArguments.cs b/src/Polly.Core/Hedging/OnHedgingArguments.cs index 044550f8129..199911a93af 100644 --- a/src/Polly.Core/Hedging/OnHedgingArguments.cs +++ b/src/Polly.Core/Hedging/OnHedgingArguments.cs @@ -3,8 +3,6 @@ namespace Polly.Hedging; /// /// Represents arguments used by the on-hedging event. /// +/// The context associated with the execution of a user-provided callback. /// The zero-based hedging attempt number. -/// -/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility. -/// -public readonly record struct OnHedgingArguments(int Attempt); +public record OnHedgingArguments(ResilienceContext Context, int Attempt); diff --git a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs index 7778d146314..65833262be4 100644 --- a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs +++ b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs @@ -456,12 +456,12 @@ private void ConfigureSecondaryTasks(params TimeSpan[] delays) return null; } - args.Context.AddResilienceEvent(new ResilienceEvent("dummy-event")); + args.ActionContext.AddResilienceEvent(new ResilienceEvent("dummy-event")); return async () => { - args.Context.Properties.Set(new ResiliencePropertyKey(attempt.ToString(CultureInfo.InvariantCulture)), attempt); - await _timeProvider.Delay(delays[attempt], args.Context.CancellationToken); + args.ActionContext.Properties.Set(new ResiliencePropertyKey(attempt.ToString(CultureInfo.InvariantCulture)), attempt); + await _timeProvider.Delay(delays[attempt], args.ActionContext.CancellationToken); return new DisposableResult(delays[attempt].ToString()).AsOutcome(); }; }; diff --git a/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs b/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs index 77af1a6ee5a..63d005a329d 100644 --- a/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs +++ b/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs @@ -74,7 +74,7 @@ public async Task Initialize_Secondary_Ok(string value, bool handled) var execution = Create(); Generator = args => { - AssertSecondaryContext(args.Context, execution); + AssertSecondaryContext(args.ActionContext, execution); args.Attempt.Should().Be(4); return () => new DisposableResult { Name = value }.AsOutcomeAsync(); }; @@ -158,7 +158,7 @@ public async Task Initialize_Cancelled_EnsureRespected(bool primary) { return async () => { - await _timeProvider.Delay(TimeSpan.FromDays(1), args.Context.CancellationToken); + await _timeProvider.Delay(TimeSpan.FromDays(1), args.ActionContext.CancellationToken); return new DisposableResult { Name = Handled }.AsOutcome(); }; }; diff --git a/test/Polly.Core.Tests/Hedging/HedgingActionGeneratorArgumentsTests.cs b/test/Polly.Core.Tests/Hedging/HedgingActionGeneratorArgumentsTests.cs index 531a964d04f..c934e5748c8 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingActionGeneratorArgumentsTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingActionGeneratorArgumentsTests.cs @@ -7,9 +7,10 @@ public class HedgingActionGeneratorArgumentsTests [Fact] public void Ctor_Ok() { - var args = new HedgingActionGeneratorArguments(ResilienceContext.Get(), 5, _ => "dummy".AsOutcomeAsync()); + var args = new HedgingActionGeneratorArguments(ResilienceContext.Get(), ResilienceContext.Get(), 5, _ => "dummy".AsOutcomeAsync()); - args.Context.Should().NotBeNull(); + args.PrimaryContext.Should().NotBeNull(); + args.ActionContext.Should().NotBeNull(); args.Attempt.Should().Be(5); args.Callback.Should().NotBeNull(); } diff --git a/test/Polly.Core.Tests/Hedging/HedgingActions.cs b/test/Polly.Core.Tests/Hedging/HedgingActions.cs index 0367152c579..08342c4c598 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingActions.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingActions.cs @@ -24,7 +24,7 @@ public HedgingActions(TimeProvider timeProvider) { return async () => { - return await Functions[args.Attempt - 1]!(args.Context); + return await Functions[args.Attempt - 1]!(args.ActionContext); }; } @@ -58,7 +58,7 @@ private async ValueTask> GetOranges(ResilienceContext context) public static Func, Func>>?> GetGenerator(Func>> task) { - return args => () => task(args.Context); + return args => () => task(args.ActionContext); } public int MaxHedgedTasks => Functions.Count + 1; diff --git a/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs b/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs index ebceaf3bab1..45bc207478a 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs @@ -15,7 +15,7 @@ public async Task GenerateAction_Generic_Ok() args => () => "ok".AsOutcomeAsync(), true); - var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), 0, _ => "primary".AsOutcomeAsync()))!; + var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), ResilienceContext.Get(), 0, _ => "primary".AsOutcomeAsync()))!; var res = await action(); res.Result.Should().Be("ok"); @@ -39,7 +39,7 @@ public async Task GenerateAction_NonGeneric_Ok(bool nullAction) }, false); - var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), 0, _ => "primary".AsOutcomeAsync()))!; + var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), ResilienceContext.Get(), 0, _ => "primary".AsOutcomeAsync()))!; if (nullAction) { action.Should().BeNull(); @@ -56,10 +56,10 @@ public async Task GenerateAction_NonGeneric_FromCallback() { var handler = new HedgingHandler( PredicateInvoker.Create(args => PredicateResult.True, false)!, - args => () => args.Callback(args.Context), + args => () => args.Callback(args.ActionContext), false); - var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), 0, _ => "callback".AsOutcomeAsync()))!; + var action = handler.GenerateAction(new HedgingActionGeneratorArguments(ResilienceContext.Get(), ResilienceContext.Get(), 0, _ => "callback".AsOutcomeAsync()))!; var res = await action(); res.Result.Should().Be("callback"); } diff --git a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs index 7965b49c736..8e6051b0903 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs @@ -68,7 +68,7 @@ public async Task AddHedging_IntegrationTest() { return async () => { - await Task.Delay(25, args.Context.CancellationToken); + await Task.Delay(25, args.ActionContext.CancellationToken); if (args.Attempt == 3) { diff --git a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs index 6afea31d832..ce007592b16 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs @@ -120,6 +120,42 @@ public async Task ExecuteAsync_ShouldReturnAnyPossibleResult() result.Should().Be("Oranges"); } + [Fact] + public async Task ExecuteAsync_EnsurePrimaryContextFlows() + { + var primaryContext = ResilienceContext.Get(); + var attempts = 0; + var key = new ResiliencePropertyKey("primary-key"); + + _options.MaxHedgedAttempts = 4; + _options.OnHedging = args => + { + args.Context.Should().Be(primaryContext); + + if (args.Arguments.Attempt == 0) + { + args.Context.Properties.Set(key, "dummy"); + } + + attempts++; + + return default; + }; + + ConfigureHedging(args => + { + args.PrimaryContext.Properties.GetValue(key, string.Empty).Should().Be("dummy"); + args.PrimaryContext.Should().Be(primaryContext); + return () => Failure.AsOutcomeAsync(); + }); + + var strategy = Create(); + var result = await strategy.ExecuteAsync(_ => new ValueTask(Failure), primaryContext); + + attempts.Should().Be(4); + primaryContext.Properties.GetValue(key, string.Empty).Should().Be("dummy"); + } + [Fact] public async void ExecuteAsync_EnsureHedgedTasksCancelled_Ok() { @@ -256,12 +292,12 @@ public async Task ExecuteAsync_EveryHedgedTaskShouldHaveDifferentContexts() { return async () => { - tokenHashCodes.Add(args.Context.CancellationToken.GetHashCode()); - args.Context.CancellationToken.CanBeCanceled.Should().BeTrue(); - args.Context.Properties.GetValue(beforeKey, "wrong").Should().Be("before"); - contexts.Add(args.Context); + tokenHashCodes.Add(args.ActionContext.CancellationToken.GetHashCode()); + args.ActionContext.CancellationToken.CanBeCanceled.Should().BeTrue(); + args.ActionContext.Properties.GetValue(beforeKey, "wrong").Should().Be("before"); + contexts.Add(args.ActionContext); await Task.Yield(); - args.Context.Properties.Set(afterKey, "after"); + args.ActionContext.Properties.Set(afterKey, "after"); return "secondary".AsOutcome(); }; }); @@ -327,10 +363,10 @@ public async Task ExecuteAsync_EnsurePropertiesConsistency(bool primaryFails) { return async () => { - contexts.Add(args.Context); - args.Context.Properties.GetValue(primaryKey, string.Empty).Should().Be("primary"); - args.Context.Properties.Set(secondaryKey, "secondary"); - await _timeProvider.Delay(TimeSpan.FromHours(1), args.Context.CancellationToken); + contexts.Add(args.ActionContext); + args.ActionContext.Properties.GetValue(primaryKey, string.Empty).Should().Be("primary"); + args.ActionContext.Properties.Set(secondaryKey, "secondary"); + await _timeProvider.Delay(TimeSpan.FromHours(1), args.ActionContext.CancellationToken); return (primaryFails ? Success : Failure).AsOutcome(); }; }); @@ -415,9 +451,9 @@ public async Task ExecuteAsync_Secondary_CustomPropertiesAvailable() { return () => { - args.Context.Properties.TryGetValue(key2, out var val).Should().BeTrue(); + args.ActionContext.Properties.TryGetValue(key2, out var val).Should().BeTrue(); val.Should().Be("my-value-2"); - args.Context.Properties.Set(key, "my-value"); + args.ActionContext.Properties.Set(key, "my-value"); return Success.AsOutcomeAsync(); }; }); @@ -820,7 +856,7 @@ private void ConfigureHedging() private void ConfigureHedging(Func>> background) { - ConfigureHedging(args => () => background(args.Context)); + ConfigureHedging(args => () => background(args.ActionContext)); } private void ConfigureHedging(Func, Func>>?> generator) diff --git a/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs b/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs index c7a4a3ba50e..9f9e53e443f 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs @@ -18,7 +18,7 @@ public async Task Ctor_EnsureDefaults() options.MaxHedgedAttempts.Should().Be(2); options.OnHedging.Should().BeNull(); - var action = options.HedgingActionGenerator(new HedgingActionGeneratorArguments(ResilienceContext.Get(), 1, c => 99.AsOutcomeAsync()))!; + var action = options.HedgingActionGenerator(new HedgingActionGeneratorArguments(ResilienceContext.Get(), ResilienceContext.Get(), 1, c => 99.AsOutcomeAsync()))!; action.Should().NotBeNull(); (await action()).Result.Should().Be(99); }