Skip to content
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.

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

The options that handle outcomes now have sensible defaults #1316

Merged
merged 1 commit into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,16 @@ public abstract class CircuitBreakerStrategyOptions<TResult> : ResilienceStrateg
/// Gets or sets the predicates for the circuit breaker.
/// </summary>
/// <remarks>
/// Defaults to <see langword="null"/>. This property is required.
/// Defaults to a delegate that handles circuit breaker on any exception except <see cref="OperationCanceledException"/>.
/// This property is required.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>>? ShouldHandle { get; set; }
public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = args => args.Exception switch
{
OperationCanceledException => PredicateResult.False,
Exception => PredicateResult.True,
_ => PredicateResult.False
};

/// <summary>
/// Gets or sets the event that is raised when the circuit resets to a <see cref="CircuitState.Closed"/> state.
Expand Down
10 changes: 8 additions & 2 deletions src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
/// Gets or sets the predicate that determines whether a hedging should be performed for a given result.
/// </summary>
/// <remarks>
/// This property is required. Defaults to <see langword="null"/>.
/// Defaults to a delegate that hedges on any exception except <see cref="OperationCanceledException"/>.
/// This property is required.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, HandleHedgingArguments>, ValueTask<bool>>? ShouldHandle { get; set; }
public Func<OutcomeArguments<TResult, HandleHedgingArguments>, ValueTask<bool>> ShouldHandle { get; set; } = args => args.Exception switch
{
OperationCanceledException => PredicateResult.False,
Exception => PredicateResult.True,
_ => PredicateResult.False
};

/// <summary>
/// Gets or sets the hedging action generator that creates hedged actions.
Expand Down
10 changes: 8 additions & 2 deletions src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,16 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
/// Gets or sets an outcome predicate that is used to register the predicates to determine if a retry should be performed.
/// </summary>
/// <remarks>
/// Defaults to <see langword="null"/>. This property is required.
/// Defaults to a delegate that retries on any exception except <see cref="OperationCanceledException"/>.
/// This property is required.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, ShouldRetryArguments>, ValueTask<bool>>? ShouldRetry { get; set; }
public Func<OutcomeArguments<TResult, ShouldRetryArguments>, ValueTask<bool>> ShouldRetry { get; set; } = args => args.Exception switch
{
OperationCanceledException => PredicateResult.False,
Exception => PredicateResult.True,
_ => PredicateResult.False
};

/// <summary>
/// Gets or sets the generator instance that is used to calculate the time between retries.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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<object>("dummy"), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<object>(new OperationCanceledException()), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<object>(new InvalidOperationException()), args))).Should().Be(true);
}

[Fact]
public void Ctor_Generic_Defaults()
{
Expand All @@ -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();

Expand All @@ -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.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,29 @@ 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();

// now set to min values
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<object>("dummy"), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<object>(new OperationCanceledException()), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<object>(new InvalidOperationException()), args))).Should().Be(true);
}

[Fact]
public void Ctor_Generic_Defaults()
{
Expand All @@ -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();

Expand Down
14 changes: 13 additions & 1 deletion test/Polly.Core.Tests/Hedging/HedgingStrategyOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public async Task Ctor_EnsureDefaults()
var options = new HedgingStrategyOptions<int>();

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);
Expand All @@ -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<int>();
var args = new HandleHedgingArguments();
var context = ResilienceContext.Get();

(await options.ShouldHandle(new(context, new Outcome<int>(0), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<int>(new OperationCanceledException()), args))).Should().Be(false);
(await options.ShouldHandle(new(context, new Outcome<int>(new InvalidOperationException()), args))).Should().Be(true);
}

[Fact]
public void Validation()
{
Expand Down
14 changes: 13 additions & 1 deletion test/Polly.Core.Tests/Retry/RetryStrategyOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public void Ctor_Ok()
var options = new RetryStrategyOptions<int>();

options.StrategyType.Should().Be("Retry");
options.ShouldRetry.Should().BeNull();
options.ShouldRetry.Should().NotBeNull();

options.RetryDelayGenerator.Should().BeNull();

Expand All @@ -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<int>();
var args = new ShouldRetryArguments(0);
var context = ResilienceContext.Get();

(await options.ShouldRetry(new(context, new Outcome<int>(0), args))).Should().Be(false);
(await options.ShouldRetry(new(context, new Outcome<int>(new OperationCanceledException()), args))).Should().Be(false);
(await options.ShouldRetry(new(context, new Outcome<int>(new InvalidOperationException()), args))).Should().Be(true);
}

[Fact]
public void InvalidOptions()
{
Expand Down