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

Drop redundant validation of resilience strategy options #1299

Merged
merged 1 commit into from
Jun 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ public static ResilienceStrategyBuilder AddSimpleCircuitBreaker(this ResilienceS
private static TBuilder AddAdvancedCircuitBreakerCore<TBuilder, TResult>(this TBuilder builder, AdvancedCircuitBreakerStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
ValidationHelper.ValidateObject(options, "The advanced circuit breaker strategy options are invalid.");

builder.AddStrategy(
context =>
{
Expand All @@ -122,8 +120,6 @@ private static TBuilder AddAdvancedCircuitBreakerCore<TBuilder, TResult>(this TB
private static TBuilder AddSimpleCircuitBreakerCore<TBuilder, TResult>(this TBuilder builder, SimpleCircuitBreakerStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
ValidationHelper.ValidateObject(options, "The circuit breaker strategy options are invalid.");

builder.AddStrategy(context => CreateStrategy(context, options, new ConsecutiveFailuresCircuitBehavior(options.FailureThreshold)), options);
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ internal static ResilienceStrategyBuilder AddFallback(this ResilienceStrategyBui

internal static void AddFallbackCore<TResult>(this ResilienceStrategyBuilderBase builder, FallbackStrategyOptions<TResult> options)
{
ValidationHelper.ValidateObject(options, "The fallback strategy options are invalid.");

builder.AddStrategy(context =>
{
var handler = new FallbackHandler<TResult>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ internal static ResilienceStrategyBuilder AddHedging(this ResilienceStrategyBuil

internal static void AddHedgingCore<TResult>(this ResilienceStrategyBuilderBase builder, HedgingStrategyOptions<TResult> options)
{
ValidationHelper.ValidateObject(options, "The hedging strategy options are invalid.");

builder.AddStrategy(context =>
{
var handler = new HedgingHandler<TResult>(
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/ResilienceStrategyBuilderBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void AddStrategy(Func<ResilienceStrategyBuilderContext, ResilienceStrateg
Guard.NotNull(factory);
Guard.NotNull(options);

ValidationHelper.ValidateObject(options, $"The '{nameof(ResilienceStrategyOptions)}' options are not valid.");
ValidationHelper.ValidateObject(options, $"The '{TypeNameFormatter.Format(options.GetType())}' are invalid.");

if (_used)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ public static ResilienceStrategyBuilder<TResult> AddRetry<TResult>(this Resilien
private static TBuilder AddRetryCore<TBuilder, TResult>(this TBuilder builder, RetryStrategyOptions<TResult> options)
where TBuilder : ResilienceStrategyBuilderBase
{
ValidationHelper.ValidateObject(options, "The retry strategy options are invalid.");

builder.AddStrategy(context =>
new RetryResilienceStrategy(
options.BaseDelay,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ public static TBuilder AddTimeout<TBuilder>(this TBuilder builder, TimeoutStrate
Guard.NotNull(builder);
Guard.NotNull(options);

ValidationHelper.ValidateObject(options, "The timeout strategy options are invalid.*");

builder.AddStrategy(context => new TimeoutResilienceStrategy(options, context.TimeProvider, context.Telemetry), options);
return builder;
}
Expand Down
24 changes: 24 additions & 0 deletions src/Polly.Core/Utils/TypeNameFormatter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System;

namespace Polly.Utils;

internal static class TypeNameFormatter
{
private const int GenericSuffixLength = 2;

public static string Format(Type type)
{
if (!type.IsGenericType)
{
return type.Name;
}

var args = type.GetGenericArguments();
if (args.Length != 1)
{
return type.Name;
}

return $"{type.Name.Substring(0, type.Name.Length - GenericSuffixLength)}<{Format(args[0])}>";
}
}
1 change: 0 additions & 1 deletion src/Polly.RateLimiting/Polly.RateLimiting.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
<ItemGroup>
<Using Include="Polly.Utils" />
<Compile Include="..\Polly.Core\Utils\Guard.cs" Link="Utils\Guard.cs" />
<Compile Include="..\Polly.Core\Utils\ValidationHelper.cs" Link="Utils\ValidationHelper.cs" />
<InternalsVisibleToTest Include="Polly.RateLimiting.Tests" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ public static TBuilder AddRateLimiter<TBuilder>(
Guard.NotNull(builder);
Guard.NotNull(options);

ValidationHelper.ValidateObject(options, "The rate limiter strategy options are invalid.");

builder.AddStrategy(context => new RateLimiterResilienceStrategy(options.RateLimiter!, options.OnRejected, context.Telemetry), options);
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ public void AddCircuitBreaker_Validation()
new ResilienceStrategyBuilder<int>()
.Invoking(b => b.AddSimpleCircuitBreaker(new SimpleCircuitBreakerStrategyOptions<int> { BreakDuration = TimeSpan.MinValue }))
.Should()
.Throw<ValidationException>()
.WithMessage("The circuit breaker strategy options are invalid.*");
.Throw<ValidationException>();

new ResilienceStrategyBuilder()
.Invoking(b => b.AddSimpleCircuitBreaker(new SimpleCircuitBreakerStrategyOptions { BreakDuration = TimeSpan.MinValue }))
.Should()
.Throw<ValidationException>()
.WithMessage("The circuit breaker strategy options are invalid.*");
.Throw<ValidationException>();
}

[Fact]
Expand All @@ -77,14 +75,12 @@ public void AddAdvancedCircuitBreaker_Validation()
new ResilienceStrategyBuilder<int>()
.Invoking(b => b.AddAdvancedCircuitBreaker(new AdvancedCircuitBreakerStrategyOptions<int> { BreakDuration = TimeSpan.MinValue }))
.Should()
.Throw<ValidationException>()
.WithMessage("The advanced circuit breaker strategy options are invalid.*");
.Throw<ValidationException>();

new ResilienceStrategyBuilder()
.Invoking(b => b.AddAdvancedCircuitBreaker(new AdvancedCircuitBreakerStrategyOptions { BreakDuration = TimeSpan.MinValue }))
.Should()
.Throw<ValidationException>()
.WithMessage("The advanced circuit breaker strategy options are invalid.*");
.Throw<ValidationException>();
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ public void AddFallback_InvalidOptions_Throws()
new ResilienceStrategyBuilder()
.Invoking(b => b.AddFallback(new FallbackStrategyOptions()))
.Should()
.Throw<ValidationException>()
.WithMessage("The fallback strategy options are invalid.*");
.Throw<ValidationException>();
}

[Fact]
Expand All @@ -66,7 +65,6 @@ public void AddFallbackT_InvalidOptions_Throws()
new ResilienceStrategyBuilder<double>()
.Invoking(b => b.AddFallback(new FallbackStrategyOptions<double>()))
.Should()
.Throw<ValidationException>()
.WithMessage("The fallback strategy options are invalid.*");
.Throw<ValidationException>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ public void AddHedging_InvalidOptions_Throws()
_builder
.Invoking(b => b.AddHedging(new HedgingStrategyOptions { HedgingActionGenerator = null! }))
.Should()
.Throw<ValidationException>()
.WithMessage("The hedging strategy options are invalid.*");
.Throw<ValidationException>();
}

[Fact]
Expand All @@ -42,14 +41,12 @@ public void AddHedgingT_InvalidOptions_Throws()
_builder
.Invoking(b => b.AddHedging(new HedgingStrategyOptions { MaxHedgedAttempts = 1000 }))
.Should()
.Throw<ValidationException>()
.WithMessage("The hedging strategy options are invalid.*");
.Throw<ValidationException>();

_genericBuilder
.Invoking(b => b.AddHedging(new HedgingStrategyOptions<string> { ShouldHandle = null! }))
.Should()
.Throw<ValidationException>()
.WithMessage("The hedging strategy options are invalid.*");
.Throw<ValidationException>();
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion test/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public void AddStrategy_InvalidOptions_Throws()
.Throw<ValidationException>()
.WithMessage(
"""
The 'ResilienceStrategyOptions' options are not valid.
The 'InvalidResilienceStrategyOptions' are invalid.

Validation Errors:
The RequiredProperty field is required.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,11 @@ public void AddRetry_InvalidOptions_Throws()
new ResilienceStrategyBuilder()
.Invoking(b => b.AddRetry(new RetryStrategyOptions { ShouldRetry = null! }))
.Should()
.Throw<ValidationException>()
.WithMessage("The retry strategy options are invalid.*");
.Throw<ValidationException>();

new ResilienceStrategyBuilder<int>()
.Invoking(b => b.AddRetry(new RetryStrategyOptions<int> { ShouldRetry = null! }))
.Should()
.Throw<ValidationException>()
.WithMessage("The retry strategy options are invalid.*");
.Throw<ValidationException>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void AddTimeout_InvalidOptions_Throws()
new ResilienceStrategyBuilder()
.Invoking(b => b.AddTimeout(new TimeoutStrategyOptions { Timeout = TimeSpan.Zero }))
.Should()
.Throw<ValidationException>().WithMessage("The timeout strategy options are invalid.*");
.Throw<ValidationException>();
}

private static TimeSpan GetTimeout(TimeoutResilienceStrategy strategy)
Expand Down
15 changes: 15 additions & 0 deletions test/Polly.Core.Tests/Utils/TypeNameFormatterTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.Collections.Generic;
using Polly.Utils;

namespace Polly.Core.Tests.Utils;

public class TypeNameFormatterTests
{
[Fact]
public void AsString_Ok()
{
Polly.Utils.TypeNameFormatter.Format(typeof(string)).Should().Be("String");
Polly.Utils.TypeNameFormatter.Format(typeof(List<string>)).Should().Be("List<String>");
Polly.Utils.TypeNameFormatter.Format(typeof(KeyValuePair<string, string>)).Should().Be("KeyValuePair`2");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void AddRateLimiter_InvalidOptions_Throws()
.Should()
.Throw<ValidationException>()
.WithMessage("""
The rate limiter strategy options are invalid.
The 'RateLimiterStrategyOptions' are invalid.

Validation Errors:
The RateLimiter field is required.
Expand All @@ -89,7 +89,7 @@ public void AddGenericRateLimiter_InvalidOptions_Throws()
.Should()
.Throw<ValidationException>()
.WithMessage("""
The rate limiter strategy options are invalid.
The 'RateLimiterStrategyOptions' are invalid.

Validation Errors:
The RateLimiter field is required.
Expand Down