From 40854a8ed18728b6804ab20225057aff01b2222f Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Wed, 17 Jan 2024 17:07:17 +0100 Subject: [PATCH] Do not encourage re-throwing the same exception instance --- src/Polly.Core/PublicAPI.Unshipped.txt | 2 -- .../Simmy/Fault/FaultChaosStrategy.cs | 11 ++------- .../Simmy/Fault/FaultStrategyOptions.cs | 14 ++++------- ...aultChaosPipelineBuilderExtensionsTests.cs | 5 ++-- .../Simmy/Fault/FaultChaosStrategyTests.cs | 12 +++++----- .../Simmy/Fault/FaultStrategyOptionsTests.cs | 24 +++++++++++++++++-- 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index 049a415793..97bb9afd8e 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -25,8 +25,6 @@ Polly.Simmy.Fault.FaultGeneratorArguments.Context.get -> Polly.ResilienceContext Polly.Simmy.Fault.FaultGeneratorArguments.FaultGeneratorArguments() -> void Polly.Simmy.Fault.FaultGeneratorArguments.FaultGeneratorArguments(Polly.ResilienceContext! context) -> void Polly.Simmy.Fault.FaultStrategyOptions -Polly.Simmy.Fault.FaultStrategyOptions.Fault.get -> System.Exception? -Polly.Simmy.Fault.FaultStrategyOptions.Fault.set -> void Polly.Simmy.Fault.FaultStrategyOptions.FaultGenerator.get -> System.Func>? Polly.Simmy.Fault.FaultStrategyOptions.FaultGenerator.set -> void Polly.Simmy.Fault.FaultStrategyOptions.FaultStrategyOptions() -> void diff --git a/src/Polly.Core/Simmy/Fault/FaultChaosStrategy.cs b/src/Polly.Core/Simmy/Fault/FaultChaosStrategy.cs index a394f05c23..a6af2f9a46 100644 --- a/src/Polly.Core/Simmy/Fault/FaultChaosStrategy.cs +++ b/src/Polly.Core/Simmy/Fault/FaultChaosStrategy.cs @@ -9,23 +9,16 @@ internal class FaultChaosStrategy : MonkeyStrategy public FaultChaosStrategy(FaultStrategyOptions options, ResilienceStrategyTelemetry telemetry) : base(options) { - if (options.Fault is null && options.FaultGenerator is null) - { - throw new InvalidOperationException("Either Fault or FaultGenerator is required."); - } - _telemetry = telemetry; - Fault = options.Fault; + OnFaultInjected = options.OnFaultInjected; - FaultGenerator = options.FaultGenerator is not null ? options.FaultGenerator : (_) => new(options.Fault); + FaultGenerator = options.FaultGenerator!; } public Func? OnFaultInjected { get; } public Func> FaultGenerator { get; } - public Exception? Fault { get; } - protected internal override async ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, diff --git a/src/Polly.Core/Simmy/Fault/FaultStrategyOptions.cs b/src/Polly.Core/Simmy/Fault/FaultStrategyOptions.cs index 23b4e2a0b3..d32f2b2e4a 100644 --- a/src/Polly.Core/Simmy/Fault/FaultStrategyOptions.cs +++ b/src/Polly.Core/Simmy/Fault/FaultStrategyOptions.cs @@ -1,4 +1,6 @@ -namespace Polly.Simmy.Fault; +using System.ComponentModel.DataAnnotations; + +namespace Polly.Simmy.Fault; /// /// Represents the options for the Fault chaos strategy. @@ -20,14 +22,6 @@ public class FaultStrategyOptions : MonkeyStrategyOptions /// Defaults to . Either or this property is required. /// When this property is the is used. /// + [Required] public Func>? FaultGenerator { get; set; } = default!; - - /// - /// Gets or sets the outcome to be injected for a given execution. - /// - /// - /// Defaults to . Either or this property is required. - /// When this property is the is used. - /// - public Exception? Fault { get; set; } } diff --git a/test/Polly.Core.Tests/Simmy/Fault/FaultChaosPipelineBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Simmy/Fault/FaultChaosPipelineBuilderExtensionsTests.cs index abb46fabae..6606de28cb 100644 --- a/test/Polly.Core.Tests/Simmy/Fault/FaultChaosPipelineBuilderExtensionsTests.cs +++ b/test/Polly.Core.Tests/Simmy/Fault/FaultChaosPipelineBuilderExtensionsTests.cs @@ -17,11 +17,10 @@ public class FaultChaosPipelineBuilderExtensionsTests InjectionRate = 0.6, Enabled = true, Randomizer = () => 0.5, - Fault = new InvalidOperationException("Dummy exception.") + FaultGenerator = _=> new ValueTask( new InvalidOperationException("Dummy exception.")) }); - AssertFaultStrategy(builder, true, 0.6) - .Fault.Should().BeOfType(typeof(InvalidOperationException)); + AssertFaultStrategy(builder, true, 0.6).FaultGenerator.Should().NotBeNull(); }, }; #pragma warning restore IDE0028 diff --git a/test/Polly.Core.Tests/Simmy/Fault/FaultChaosStrategyTests.cs b/test/Polly.Core.Tests/Simmy/Fault/FaultChaosStrategyTests.cs index 1a076c6791..4a80446c63 100644 --- a/test/Polly.Core.Tests/Simmy/Fault/FaultChaosStrategyTests.cs +++ b/test/Polly.Core.Tests/Simmy/Fault/FaultChaosStrategyTests.cs @@ -59,7 +59,7 @@ public void Given_not_enabled_should_not_inject_fault() InjectionRate = 0.6, Enabled = false, Randomizer = () => 0.5, - Fault = fault + FaultGenerator = _ => new ValueTask(fault) }; var sut = CreateSut(options); @@ -79,7 +79,7 @@ public async Task Given_not_enabled_should_not_inject_fault_and_return_outcome() InjectionRate = 0.6, Enabled = false, Randomizer = () => 0.5, - Fault = fault + FaultGenerator = _ => new ValueTask(fault) }; var sut = new ResiliencePipelineBuilder().AddChaosFault(options).Build(); @@ -106,7 +106,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_faul InjectionRate = 0.6, Enabled = true, Randomizer = () => 0.5, - Fault = fault, + FaultGenerator = _ => new ValueTask(fault), OnFaultInjected = args => { args.Context.Should().NotBeNull(); @@ -143,7 +143,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_faul InjectionRate = 0.6, Enabled = true, Randomizer = () => 0.5, - Fault = fault, + FaultGenerator = _ => new ValueTask(fault), OnFaultInjected = args => { args.Context.Should().NotBeNull(); @@ -181,7 +181,7 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_fa InjectionRate = 0.3, Enabled = true, Randomizer = () => 0.5, - Fault = fault + FaultGenerator = _ => new ValueTask(fault) }; var sut = CreateSut(options); @@ -232,7 +232,7 @@ public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running return new ValueTask(true); }, Randomizer = () => 0.5, - Fault = fault + FaultGenerator = _ => new ValueTask(fault) }; var sut = CreateSut(options); diff --git a/test/Polly.Core.Tests/Simmy/Fault/FaultStrategyOptionsTests.cs b/test/Polly.Core.Tests/Simmy/Fault/FaultStrategyOptionsTests.cs index ba211b4993..5ef2458692 100644 --- a/test/Polly.Core.Tests/Simmy/Fault/FaultStrategyOptionsTests.cs +++ b/test/Polly.Core.Tests/Simmy/Fault/FaultStrategyOptionsTests.cs @@ -1,5 +1,7 @@ -using Polly.Simmy; +using System.ComponentModel.DataAnnotations; +using Polly.Simmy; using Polly.Simmy.Fault; +using Polly.Utils; namespace Polly.Core.Tests.Simmy.Fault; @@ -14,8 +16,26 @@ public void Ctor_Ok() sut.EnabledGenerator.Should().BeNull(); sut.InjectionRate.Should().Be(MonkeyStrategyConstants.DefaultInjectionRate); sut.InjectionRateGenerator.Should().BeNull(); - sut.Fault.Should().BeNull(); sut.OnFaultInjected.Should().BeNull(); sut.FaultGenerator.Should().BeNull(); } + + [Fact] + public void InvalidOptions() + { + var options = new FaultStrategyOptions + { + FaultGenerator = null!, + }; + + options.Invoking(o => ValidationHelper.ValidateObject(new(o, "Invalid Options"))) + .Should() + .Throw() + .WithMessage(""" + Invalid Options + + Validation Errors: + The FaultGenerator field is required. + """); + } }