Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
martintmk committed Jun 16, 2023
1 parent 9c891ef commit 5961df9
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 31 deletions.
6 changes: 3 additions & 3 deletions src/Polly.Core/Hedging/Controller/HedgingHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal sealed record class HedgingHandler<T>(
{
var copiedArgs = new HedgingActionGeneratorArguments<T>(
args.PrimaryContext,
args.Context,
args.ActionContext,
args.Attempt,
(Func<ResilienceContext, ValueTask<Outcome<T>>>)(object)args.Callback);

Expand All @@ -30,7 +30,7 @@ internal sealed record class HedgingHandler<T>(
private Func<ValueTask<Outcome<TResult>>>? CreateNonGenericAction<TResult>(HedgingActionGeneratorArguments<TResult> args)
{
var generator = (Func<HedgingActionGeneratorArguments<object>, Func<ValueTask<Outcome<object>>>?>)(object)ActionGenerator;
var action = generator(new HedgingActionGeneratorArguments<object>(args.PrimaryContext, args.Context, args.Attempt, async context =>
var action = generator(new HedgingActionGeneratorArguments<object>(args.PrimaryContext, args.ActionContext, args.Attempt, async context =>
{
var outcome = await args.Callback(context).ConfigureAwait(context.ContinueOnCapturedContext);
return outcome.AsOutcome();
Expand All @@ -43,7 +43,7 @@ internal sealed record class HedgingHandler<T>(

return async () =>
{
var outcome = await action().ConfigureAwait(args.Context.ContinueOnCapturedContext);
var outcome = await action().ConfigureAwait(args.ActionContext.ContinueOnCapturedContext);
return outcome.AsOutcome<TResult>();
};
}
Expand Down
6 changes: 4 additions & 2 deletions src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ namespace Polly.Hedging;
/// </summary>
/// <typeparam name="TResult">The type of the result.</typeparam>
/// <param name="PrimaryContext">The primary resilience context.</param>
/// <param name="Context">The context associated with the execution of a user-provided callback that is cloned from the primary context.</param>
/// <param name="ActionContext">
/// The context that will be passed to action generated by <see cref="HedgingStrategyOptions{TResult}.HedgingActionGenerator"/>.
/// This context is cloned from <paramref name="PrimaryContext"/>.</param>
/// <param name="Attempt">The zero-based hedging attempt number.</param>
/// <param name="Callback">The callback passed to hedging strategy.</param>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct HedgingActionGeneratorArguments<TResult>(
ResilienceContext PrimaryContext,
ResilienceContext Context,
ResilienceContext ActionContext,
int Attempt,
Func<ResilienceContext, ValueTask<Outcome<TResult>>> Callback);
2 changes: 1 addition & 1 deletion src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
{
return async () =>
{
return await args.Callback(args.Context).ConfigureAwait(args.Context.ContinueOnCapturedContext);
return await args.Callback(args.ActionContext).ConfigureAwait(args.ActionContext.ContinueOnCapturedContext);
};
};

Expand Down
3 changes: 0 additions & 3 deletions src/Polly.Core/Hedging/OnHedgingArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,4 @@ namespace Polly.Hedging;
/// </summary>
/// <param name="Context">The context associated with the execution of a user-provided callback.</param>
/// <param name="Attempt">The zero-based hedging attempt number.</param>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public record OnHedgingArguments(ResilienceContext Context, int Attempt);
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(attempt.ToString(CultureInfo.InvariantCulture)), attempt);
await _timeProvider.Delay(delays[attempt], args.Context.CancellationToken);
args.ActionContext.Properties.Set(new ResiliencePropertyKey<int>(attempt.ToString(CultureInfo.InvariantCulture)), attempt);
await _timeProvider.Delay(delays[attempt], args.ActionContext.CancellationToken);
return new DisposableResult(delays[attempt].ToString()).AsOutcome();
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand Down Expand Up @@ -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();
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public void Ctor_Ok()
var args = new HedgingActionGeneratorArguments<string>(ResilienceContext.Get(), ResilienceContext.Get(), 5, _ => "dummy".AsOutcomeAsync());

args.PrimaryContext.Should().NotBeNull();
args.Context.Should().NotBeNull();
args.ActionContext.Should().NotBeNull();
args.Attempt.Should().Be(5);
args.Callback.Should().NotBeNull();
}
Expand Down
4 changes: 2 additions & 2 deletions test/Polly.Core.Tests/Hedging/HedgingActions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}

Expand Down Expand Up @@ -58,7 +58,7 @@ private async ValueTask<Outcome<string>> GetOranges(ResilienceContext context)

public static Func<HedgingActionGeneratorArguments<string>, Func<ValueTask<Outcome<string>>>?> GetGenerator(Func<ResilienceContext, ValueTask<Outcome<string>>> task)
{
return args => () => task(args.Context);
return args => () => task(args.ActionContext);
}

public int MaxHedgedTasks => Functions.Count + 1;
Expand Down
2 changes: 1 addition & 1 deletion test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public async Task GenerateAction_NonGeneric_FromCallback()
{
var handler = new HedgingHandler<object>(
PredicateInvoker<HandleHedgingArguments>.Create<object>(args => PredicateResult.True, false)!,
args => () => args.Callback(args.Context),
args => () => args.Callback(args.ActionContext),
false);

var action = handler.GenerateAction(new HedgingActionGeneratorArguments<string>(ResilienceContext.Get(), ResilienceContext.Get(), 0, _ => "callback".AsOutcomeAsync()))!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
24 changes: 12 additions & 12 deletions test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,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();
};
});
Expand Down Expand Up @@ -363,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();
};
});
Expand Down Expand Up @@ -451,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();
};
});
Expand Down Expand Up @@ -856,7 +856,7 @@ private void ConfigureHedging()

private void ConfigureHedging(Func<ResilienceContext, ValueTask<Outcome<string>>> background)
{
ConfigureHedging(args => () => background(args.Context));
ConfigureHedging(args => () => background(args.ActionContext));
}

private void ConfigureHedging(Func<HedgingActionGeneratorArguments<string>, Func<ValueTask<Outcome<string>>>?> generator)
Expand Down

0 comments on commit 5961df9

Please sign in to comment.