From f16ca6cdaf1fafb49e0141ca7ab65a94fa164faa Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Fri, 16 Jun 2023 16:00:08 +0200 Subject: [PATCH] The options that handle outcomes now have sensible defaults --- .../CircuitBreakerStrategyOptions.cs | 10 ++++++++-- .../Hedging/HedgingStrategyOptions.TResult.cs | 10 ++++++++-- .../Retry/RetryStrategyOptions.TResult.cs | 10 ++++++++-- .../AdvancedCircuitBreakerOptionsTests.cs | 18 ++++++++++++++---- .../SimpleCircuitBreakerOptionsTests.cs | 17 ++++++++++++++--- .../Hedging/HedgingStrategyOptionsTests.cs | 14 +++++++++++++- .../Retry/RetryStrategyOptionsTests.cs | 14 +++++++++++++- 7 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs index 10fd7e7788..4c0c539fe9 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs @@ -37,10 +37,16 @@ public abstract class CircuitBreakerStrategyOptions : ResilienceStrateg /// Gets or sets the predicates for the circuit breaker. /// /// - /// Defaults to . This property is required. + /// Defaults to a delegate that handles circuit breaker on any exception except . + /// This property is required. /// [Required] - public Func, ValueTask>? ShouldHandle { get; set; } + public Func, ValueTask> ShouldHandle { get; set; } = args => args.Exception switch + { + OperationCanceledException => PredicateResult.False, + Exception => PredicateResult.True, + _ => PredicateResult.False + }; /// /// Gets or sets the event that is raised when the circuit resets to a state. diff --git a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs index 44ec334f2b..a448243f5e 100644 --- a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs +++ b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs @@ -45,10 +45,16 @@ public class HedgingStrategyOptions : ResilienceStrategyOptions /// Gets or sets the predicate that determines whether a hedging should be performed for a given result. /// /// - /// This property is required. Defaults to . + /// Defaults to a delegate that hedges on any exception except . + /// This property is required. /// [Required] - public Func, ValueTask>? ShouldHandle { get; set; } + public Func, ValueTask> ShouldHandle { get; set; } = args => args.Exception switch + { + OperationCanceledException => PredicateResult.False, + Exception => PredicateResult.True, + _ => PredicateResult.False + }; /// /// Gets or sets the hedging action generator that creates hedged actions. diff --git a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs index a112556ef9..48ecd7c334 100644 --- a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs +++ b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs @@ -61,10 +61,16 @@ public class RetryStrategyOptions : ResilienceStrategyOptions /// Gets or sets an outcome predicate that is used to register the predicates to determine if a retry should be performed. /// /// - /// Defaults to . This property is required. + /// Defaults to a delegate that retries on any exception except . + /// This property is required. /// [Required] - public Func, ValueTask>? ShouldRetry { get; set; } + public Func, ValueTask> ShouldRetry { get; set; } = args => args.Exception switch + { + OperationCanceledException => PredicateResult.False, + Exception => PredicateResult.True, + _ => PredicateResult.False + }; /// /// Gets or sets the generator instance that is used to calculate the time between retries. diff --git a/test/Polly.Core.Tests/CircuitBreaker/AdvancedCircuitBreakerOptionsTests.cs b/test/Polly.Core.Tests/CircuitBreaker/AdvancedCircuitBreakerOptionsTests.cs index d6b5d99d79..2859a9c088 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/AdvancedCircuitBreakerOptionsTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/AdvancedCircuitBreakerOptionsTests.cs @@ -17,7 +17,7 @@ public void Ctor_Defaults() options.OnOpened.Should().BeNull(); options.OnClosed.Should().BeNull(); options.OnHalfOpened.Should().BeNull(); - options.ShouldHandle.Should().BeNull(); + options.ShouldHandle.Should().NotBeNull(); options.StrategyType.Should().Be("CircuitBreaker"); options.StrategyName.Should().BeNull(); @@ -27,10 +27,21 @@ public void Ctor_Defaults() options.MinimumThroughput = 2; options.SamplingDuration = TimeSpan.FromMilliseconds(500); - options.ShouldHandle = _ => PredicateResult.True; ValidationHelper.ValidateObject(options, "Dummy."); } + [Fact] + public async Task ShouldHandle_EnsureDefaults() + { + var options = new AdvancedCircuitBreakerStrategyOptions(); + var args = new CircuitBreakerPredicateArguments(); + var context = ResilienceContext.Get(); + + (await options.ShouldHandle(new(context, new Outcome("dummy"), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new OperationCanceledException()), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new InvalidOperationException()), args))).Should().Be(true); + } + [Fact] public void Ctor_Generic_Defaults() { @@ -43,7 +54,7 @@ public void Ctor_Generic_Defaults() options.OnOpened.Should().BeNull(); options.OnClosed.Should().BeNull(); options.OnHalfOpened.Should().BeNull(); - options.ShouldHandle.Should().BeNull(); + options.ShouldHandle.Should().NotBeNull(); options.StrategyType.Should().Be("CircuitBreaker"); options.StrategyName.Should().BeNull(); @@ -53,7 +64,6 @@ public void Ctor_Generic_Defaults() options.MinimumThroughput = 2; options.SamplingDuration = TimeSpan.FromMilliseconds(500); - options.ShouldHandle = _ => PredicateResult.True; ValidationHelper.ValidateObject(options, "Dummy."); } diff --git a/test/Polly.Core.Tests/CircuitBreaker/SimpleCircuitBreakerOptionsTests.cs b/test/Polly.Core.Tests/CircuitBreaker/SimpleCircuitBreakerOptionsTests.cs index b35968bade..d60f705098 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/SimpleCircuitBreakerOptionsTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/SimpleCircuitBreakerOptionsTests.cs @@ -16,7 +16,7 @@ public void Ctor_Defaults() options.OnOpened.Should().BeNull(); options.OnClosed.Should().BeNull(); options.OnHalfOpened.Should().BeNull(); - options.ShouldHandle.Should().BeNull(); + options.ShouldHandle.Should().NotBeNull(); options.StrategyType.Should().Be("CircuitBreaker"); options.StrategyName.Should().BeNull(); @@ -24,10 +24,21 @@ public void Ctor_Defaults() options.FailureThreshold = 1; options.BreakDuration = TimeSpan.FromMilliseconds(500); - options.ShouldHandle = _ => PredicateResult.True; ValidationHelper.ValidateObject(options, "Dummy."); } + [Fact] + public async Task ShouldHandle_EnsureDefaults() + { + var options = new SimpleCircuitBreakerStrategyOptions(); + var args = new CircuitBreakerPredicateArguments(); + var context = ResilienceContext.Get(); + + (await options.ShouldHandle(new(context, new Outcome("dummy"), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new OperationCanceledException()), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new InvalidOperationException()), args))).Should().Be(true); + } + [Fact] public void Ctor_Generic_Defaults() { @@ -38,7 +49,7 @@ public void Ctor_Generic_Defaults() options.OnOpened.Should().BeNull(); options.OnClosed.Should().BeNull(); options.OnHalfOpened.Should().BeNull(); - options.ShouldHandle.Should().BeNull(); + options.ShouldHandle.Should().NotBeNull(); options.StrategyType.Should().Be("CircuitBreaker"); options.StrategyName.Should().BeNull(); diff --git a/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs b/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs index c7a4a3ba50..59b4e5d601 100644 --- a/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs +++ b/test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs @@ -12,7 +12,7 @@ public async Task Ctor_EnsureDefaults() var options = new HedgingStrategyOptions(); options.StrategyType.Should().Be("Hedging"); - options.ShouldHandle.Should().BeNull(); + options.ShouldHandle.Should().NotBeNull(); options.HedgingActionGenerator.Should().NotBeNull(); options.HedgingDelay.Should().Be(TimeSpan.FromSeconds(2)); options.MaxHedgedAttempts.Should().Be(2); @@ -23,6 +23,18 @@ public async Task Ctor_EnsureDefaults() (await action()).Result.Should().Be(99); } + [Fact] + public async Task ShouldHandle_EnsureDefaults() + { + var options = new HedgingStrategyOptions(); + var args = new HandleHedgingArguments(); + var context = ResilienceContext.Get(); + + (await options.ShouldHandle(new(context, new Outcome(0), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new OperationCanceledException()), args))).Should().Be(false); + (await options.ShouldHandle(new(context, new Outcome(new InvalidOperationException()), args))).Should().Be(true); + } + [Fact] public void Validation() { diff --git a/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs b/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs index 544ec879af..7242cb10b6 100644 --- a/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs @@ -12,7 +12,7 @@ public void Ctor_Ok() var options = new RetryStrategyOptions(); options.StrategyType.Should().Be("Retry"); - options.ShouldRetry.Should().BeNull(); + options.ShouldRetry.Should().NotBeNull(); options.RetryDelayGenerator.Should().BeNull(); @@ -23,6 +23,18 @@ public void Ctor_Ok() options.BaseDelay.Should().Be(TimeSpan.FromSeconds(2)); } + [Fact] + public async Task ShouldHandle_EnsureDefaults() + { + var options = new RetryStrategyOptions(); + var args = new ShouldRetryArguments(0); + var context = ResilienceContext.Get(); + + (await options.ShouldRetry(new(context, new Outcome(0), args))).Should().Be(false); + (await options.ShouldRetry(new(context, new Outcome(new OperationCanceledException()), args))).Should().Be(false); + (await options.ShouldRetry(new(context, new Outcome(new InvalidOperationException()), args))).Should().Be(true); + } + [Fact] public void InvalidOptions() {