Drop redundant validation of resilience strategy options (#1299)

App-vNext · Jun 15, 2023 · c782d81 · c782d81
1 parent 833ad99
commit c782d81
Show file tree

Hide file tree

Showing 17 changed files with 55 additions and 42 deletions.
diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs
@@ -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 =>
             {
@@ -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;
     }

diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs
@@ -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>(

diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs
@@ -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>(

diff --git a/src/Polly.Core/ResilienceStrategyBuilderBase.cs b/src/Polly.Core/ResilienceStrategyBuilderBase.cs
@@ -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)
         {

diff --git a/src/Polly.Core/Retry/RetryResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Retry/RetryResilienceStrategyBuilderExtensions.cs
@@ -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,

diff --git a/src/Polly.Core/Timeout/TimeoutResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Timeout/TimeoutResilienceStrategyBuilderExtensions.cs
@@ -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;
     }

diff --git a/src/Polly.Core/Utils/TypeNameFormatter.cs b/src/Polly.Core/Utils/TypeNameFormatter.cs
@@ -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])}>";
+    }
+}
diff --git a/src/Polly.RateLimiting/Polly.RateLimiting.csproj b/src/Polly.RateLimiting/Polly.RateLimiting.csproj
@@ -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>
 

diff --git a/src/Polly.RateLimiting/RateLimiterResilienceStrategyBuilderExtensions.cs b/src/Polly.RateLimiting/RateLimiterResilienceStrategyBuilderExtensions.cs
@@ -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;
     }

diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs
@@ -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]
@@ -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]

diff --git a/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyBuilderExtensionsTests.cs
@@ -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]
@@ -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>();
     }
 }
diff --git a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyBuilderExtensionsTests.cs
@@ -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]
@@ -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]

diff --git a/test/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs b/test/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs
@@ -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.

diff --git a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyBuilderExtensionsTests.cs
@@ -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>();
     }
 }
diff --git a/test/Polly.Core.Tests/Timeout/TimeoutResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Timeout/TimeoutResilienceStrategyBuilderExtensionsTests.cs
@@ -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)

diff --git a/test/Polly.Core.Tests/Utils/TypeNameFormatterTests.cs b/test/Polly.Core.Tests/Utils/TypeNameFormatterTests.cs
@@ -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");
+    }
+}
diff --git a/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyBuilderExtensionsTests.cs b/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyBuilderExtensionsTests.cs
@@ -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.
@@ -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.