Skip to content

[DRAFT] Nullability fixes for chaos outcome strategy #2010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
peter-csala wants to merge 2 commits into from
Closed

[DRAFT] Nullability fixes for chaos outcome strategy #2010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68e9281
Nullability fixes for chaos outcome strategy
martintmk Feb 20, 2024
081b4b5
Fix broken tests
peter-csala Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Polly.Core/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.ChaosOutcomeStrategyOptions() -> void
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.OnOutcomeInjected.get -> System.Func<Polly.Simmy.Outcomes.OnOutcomeInjectedArguments<TResult>, System.Threading.Tasks.ValueTask>?
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.OnOutcomeInjected.set -> void
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.OutcomeGenerator.get -> System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>?>>!
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.OutcomeGenerator.get -> System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>?
Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>.OutcomeGenerator.set -> void
Polly.Simmy.Outcomes.OnOutcomeInjectedArguments<TResult>
Polly.Simmy.Outcomes.OnOutcomeInjectedArguments<TResult>.Context.get -> Polly.ResilienceContext!
Expand Down Expand Up @@ -542,7 +542,7 @@ static Polly.Simmy.ChaosLatencyPipelineBuilderExtensions.AddChaosLatency<TBuilde
static Polly.Simmy.ChaosOutcomePipelineBuilderExtensions.AddChaosOutcome<TResult>(this Polly.ResiliencePipelineBuilder<TResult>! builder, double injectionRate, System.Func<TResult?>! resultGenerator) -> Polly.ResiliencePipelineBuilder<TResult>!
static Polly.Simmy.ChaosOutcomePipelineBuilderExtensions.AddChaosOutcome<TResult>(this Polly.ResiliencePipelineBuilder<TResult>! builder, Polly.Simmy.Outcomes.ChaosOutcomeStrategyOptions<TResult>! options) -> Polly.ResiliencePipelineBuilder<TResult>!
static Polly.Simmy.Fault.FaultGenerator.implicit operator System.Func<Polly.Simmy.Fault.FaultGeneratorArguments, System.Threading.Tasks.ValueTask<System.Exception?>>!(Polly.Simmy.Fault.FaultGenerator! generator) -> System.Func<Polly.Simmy.Fault.FaultGeneratorArguments, System.Threading.Tasks.ValueTask<System.Exception?>>!
static Polly.Simmy.Outcomes.OutcomeGenerator<TResult>.implicit operator System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>?>>!(Polly.Simmy.Outcomes.OutcomeGenerator<TResult>! generator) -> System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>?>>!
static Polly.Simmy.Outcomes.OutcomeGenerator<TResult>.implicit operator System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>!(Polly.Simmy.Outcomes.OutcomeGenerator<TResult>! generator) -> System.Func<Polly.Simmy.Outcomes.OutcomeGeneratorArguments, System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>!
static Polly.TimeoutResiliencePipelineBuilderExtensions.AddTimeout<TBuilder>(this TBuilder! builder, Polly.Timeout.TimeoutStrategyOptions! options) -> TBuilder!
static Polly.TimeoutResiliencePipelineBuilderExtensions.AddTimeout<TBuilder>(this TBuilder! builder, System.TimeSpan timeout) -> TBuilder!
static readonly Polly.ResiliencePipeline.Empty -> Polly.ResiliencePipeline!
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Simmy/Fault/FaultGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,6 @@ public FaultGenerator AddException<TException>(int weight = DefaultWeight)

var generatorDelegate = generator._helper.CreateGenerator();

return args => new ValueTask<Exception?>(generatorDelegate(args.Context)!.Value.Exception);
return args => new ValueTask<Exception?>(generatorDelegate(args.Context).Exception);
}
}
2 changes: 1 addition & 1 deletion src/Polly.Core/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static class ChaosOutcomePipelineBuilderExtensions
InjectionRate = injectionRate,
OutcomeGenerator = (_) =>
{
return new ValueTask<Outcome<TResult>?>(Outcome.FromResult(resultGenerator()));
return new ValueTask<Outcome<TResult>>(Outcome.FromResult(resultGenerator()));
}
});
return builder;
Expand Down
8 changes: 4 additions & 4 deletions src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ public ChaosOutcomeStrategy(ChaosOutcomeStrategyOptions<T> options, ResilienceSt
{
_telemetry = telemetry;
OnOutcomeInjected = options.OnOutcomeInjected;
OutcomeGenerator = options.OutcomeGenerator;
OutcomeGenerator = options.OutcomeGenerator!;
}

public Func<OnOutcomeInjectedArguments<T>, ValueTask>? OnOutcomeInjected { get; }

public Func<OutcomeGeneratorArguments, ValueTask<Outcome<T>?>> OutcomeGenerator { get; }
public Func<OutcomeGeneratorArguments, ValueTask<Outcome<T>>> OutcomeGenerator { get; }

protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
{
Expand All @@ -25,15 +25,15 @@ protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func
if (await ShouldInjectAsync(context).ConfigureAwait(context.ContinueOnCapturedContext))
{
var outcome = await OutcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext);
var args = new OnOutcomeInjectedArguments<T>(context, outcome.Value);
var args = new OnOutcomeInjectedArguments<T>(context, outcome);
_telemetry.Report(new(ResilienceEventSeverity.Information, ChaosOutcomeConstants.OnOutcomeInjectedEvent), context, args);

if (OnOutcomeInjected is not null)
{
await OnOutcomeInjected(args).ConfigureAwait(context.ContinueOnCapturedContext);
}

return new Outcome<T>(outcome.Value.Result);
return new Outcome<T>(outcome.Result);
}

return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategyOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ public class ChaosOutcomeStrategyOptions<TResult> : ChaosStrategyOptions
/// Defaults to <see langword="null"/>. This property is required.
/// </value>
[Required]
public Func<OutcomeGeneratorArguments, ValueTask<Outcome<TResult>?>> OutcomeGenerator { get; set; } = default!;
public Func<OutcomeGeneratorArguments, ValueTask<Outcome<TResult>>>? OutcomeGenerator { get; set; }
}
4 changes: 2 additions & 2 deletions src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ public OutcomeGenerator<TResult> AddResult(Func<ResilienceContext, TResult> gene
/// </summary>
/// <param name="generator">The generator instance.</param>
[EditorBrowsable(EditorBrowsableState.Never)]
public static implicit operator Func<OutcomeGeneratorArguments, ValueTask<Outcome<TResult>?>>(OutcomeGenerator<TResult> generator)
public static implicit operator Func<OutcomeGeneratorArguments, ValueTask<Outcome<TResult>>>(OutcomeGenerator<TResult> generator)
{
Guard.NotNull(generator);

var generatorDelegate = generator._helper.CreateGenerator();

return args => new ValueTask<Outcome<TResult>?>(generatorDelegate(args.Context));
return args => new ValueTask<Outcome<TResult>>(generatorDelegate(args.Context));
}
}

7 changes: 4 additions & 3 deletions src/Polly.Core/Simmy/Utils/GeneratorHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ public void AddOutcome(Func<ResilienceContext, Outcome<TResult>> generator, int
_weights.Add(weight);
}

internal Func<ResilienceContext, Outcome<TResult>?> CreateGenerator()
internal Func<ResilienceContext, Outcome<TResult>> CreateGenerator()
{
if (_factories.Count == 0)
{
return _ => null;
throw new InvalidOperationException("No outcome generators have been added.");
}

var totalWeight = _totalWeight;
Expand All @@ -45,7 +45,8 @@ public void AddOutcome(Func<ResilienceContext, Outcome<TResult>> generator, int
}
}

return null;
// Should never reach here!
return default;
};
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/Snippets/Docs/Chaos.Outcome.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static void OutcomeUsage()
OutcomeGenerator = static args =>
{
var response = new HttpResponseMessage(HttpStatusCode.InternalServerError);
return new ValueTask<Outcome<HttpResponseMessage>?>(Outcome.FromResult(response));
return Outcome.FromResultAsValueTask(response);
},
InjectionRate = 0.1
})
Expand Down Expand Up @@ -99,13 +99,12 @@ public static void OutcomeGeneratorDelegates()
// The same behavior can be achieved with delegates
OutcomeGenerator = args =>
{
Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
Outcome<HttpResponseMessage> outcome = Random.Shared.Next(350) switch
{
< 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
< 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
< 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
< 350 => Outcome.FromException<HttpResponseMessage>(new TimeoutException()),
_ => null
_ => Outcome.FromException<HttpResponseMessage>(new TimeoutException())
};

return ValueTask.FromResult(outcome);
Expand Down
2 changes: 1 addition & 1 deletion test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class ChaosOutcomePipelineBuilderExtensionsTests
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<int>?>(Outcome.FromResult(100))
OutcomeGenerator = (_) => Outcome.FromResultAsValueTask(100)
});

AssertResultStrategy(builder, true, 0.6, new(100));
Expand Down
31 changes: 4 additions & 27 deletions test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void Given_not_enabled_should_not_inject_result()
InjectionRate = 0.6,
Enabled = false,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
OutcomeGenerator = (_) => Outcome.FromResultAsValueTask(fakeResult)
};

var sut = CreateSut(options);
Expand All @@ -81,7 +81,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_resu
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult)),
OutcomeGenerator = (_) => Outcome.FromResultAsValueTask(fakeResult),
OnOutcomeInjected = args =>
{
args.Context.Should().NotBeNull();
Expand Down Expand Up @@ -117,7 +117,7 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_re
InjectionRate = 0.3,
Enabled = false,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
OutcomeGenerator = (_) => Outcome.FromResultAsValueTask(fakeResult)
};

var sut = CreateSut(options);
Expand All @@ -132,29 +132,6 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_re
_onOutcomeInjectedExecuted.Should().BeFalse();
}

[Fact]
public async Task Given_enabled_and_randomly_within_threshold_should_inject_result_even_as_null()
{
Outcome<HttpStatusCode?>? nullOutcome = Outcome.FromResult<HttpStatusCode?>(null);
var options = new ChaosOutcomeStrategyOptions<HttpStatusCode?>
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode?>?>(nullOutcome)
};

var sut = CreateSut(options);
var response = await sut.ExecuteAsync<HttpStatusCode?>(async _ =>
{
_userDelegateExecuted = true;
return await Task.FromResult(HttpStatusCode.OK);
});

response.Should().Be(null);
_userDelegateExecuted.Should().BeFalse();
_onOutcomeInjectedExecuted.Should().BeFalse();
}

[Fact]
public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running_the_strategy()
{
Expand All @@ -168,7 +145,7 @@ public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running
cts.Cancel();
return new ValueTask<bool>(true);
},
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(HttpStatusCode.TooManyRequests))
OutcomeGenerator = (_) => Outcome.FromResultAsValueTask(HttpStatusCode.TooManyRequests)
};

var sut = CreateSut(options);
Expand Down
14 changes: 7 additions & 7 deletions test/Polly.Core.Tests/Simmy/Outcomes/OutcomeGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public void AddException_Generic_Ok()

generator.AddException<InvalidOperationException>();

Generate(generator)!.Value.Exception.Should().BeOfType<InvalidOperationException>();
Generate(generator).Exception.Should().BeOfType<InvalidOperationException>();
}

[Fact]
Expand All @@ -22,7 +22,7 @@ public void AddException_Factory_Ok()

generator.AddException(() => new InvalidOperationException());

Generate(generator)!.Value.Exception.Should().BeOfType<InvalidOperationException>();
Generate(generator).Exception.Should().BeOfType<InvalidOperationException>();
}

[Fact]
Expand All @@ -37,7 +37,7 @@ public void AddException_FactoryWithResilienceContext_Ok()
return new InvalidOperationException();
});

Generate(generator)!.Value.Exception.Should().BeOfType<InvalidOperationException>();
Generate(generator).Exception.Should().BeOfType<InvalidOperationException>();
}

[Fact]
Expand All @@ -50,7 +50,7 @@ public void AddResult_Factory_Ok()
return "dummy";
});

Generate(generator)!.Value.Result.Should().Be("dummy");
Generate(generator).Result.Should().Be("dummy");
}

[Fact]
Expand All @@ -65,12 +65,12 @@ public void AddResult_FactoryWithResilienceContext_Ok()
return "dummy";
});

Generate(generator)!.Value.Result.Should().Be("dummy");
Generate(generator).Result.Should().Be("dummy");
}

private static Outcome<string>? Generate(OutcomeGenerator<string> generator)
private static Outcome<string> Generate(OutcomeGenerator<string> generator)
{
Func<OutcomeGeneratorArguments, ValueTask<Outcome<string>?>> func = generator;
Func<OutcomeGeneratorArguments, ValueTask<Outcome<string>>> func = generator;

return func(new OutcomeGeneratorArguments(ResilienceContextPool.Shared.Get())).AsTask().Result;
}
Expand Down
17 changes: 10 additions & 7 deletions test/Polly.Core.Tests/Simmy/Utils/GeneratorHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ namespace Polly.Core.Tests.Simmy.Utils;
public class GeneratorHelperTests
{
[Fact]
public void CreateGenerator_NoGenerators_Ok()
public void CreateGenerator_NoGenerators_ThrowsInvalidOperationException()
{
var helper = new GeneratorHelper<int>(_ => 10);

helper.CreateGenerator()(ResilienceContextPool.Shared.Get()).Should().BeNull();
Action act = () => helper.CreateGenerator();
act.Should()
.Throw<InvalidOperationException>()
.WithMessage("No outcome generators have been added.");
}

[Fact]
Expand All @@ -32,25 +35,25 @@ public void AddOutcome_EnsureWeightRespected()
var generator = helper.CreateGenerator();

weight = 0;
generator(context)!.Value.Result.Should().Be(1);
generator(context).Result.Should().Be(1);
weight = 39;
generator(context)!.Value.Result.Should().Be(1);
generator(context).Result.Should().Be(1);

weight = 40;
generator(context)!.Value.Result.Should().Be(2);
generator(context).Result.Should().Be(2);

maxWeight.Should().Be(120);
}

[Fact]
public void Generator_OutsideRange_ReturnsNull()
public void Generator_OutsideRange_ReturnsOutcomeWithDefault()
{
var context = ResilienceContextPool.Shared.Get();
var helper = new GeneratorHelper<int>(_ => 1000);

helper.AddOutcome(_ => Outcome.FromResult(1), 40);

var generator = helper.CreateGenerator();
generator(context).Should().BeNull();
generator(context).Should().Be(Outcome.FromResult(default(int)));
}
}