From f695c9236e2e591ceb697ab45ccc3559ddf4f6e4 Mon Sep 17 00:00:00 2001
From: Martin Tomka <martintomka@microsoft.com>
Date: Thu, 15 Jun 2023 06:54:42 +0200
Subject: [PATCH] Drop redundant validation of resilience strategy options

---
 ...akerResilienceStrategyBuilderExtensions.cs |  4 ----
 ...backResilienceStrategyBuilderExtensions.cs |  2 --
 ...gingResilienceStrategyBuilderExtensions.cs |  2 --
 .../ResilienceStrategyBuilderBase.cs          |  2 +-
 ...etryResilienceStrategyBuilderExtensions.cs |  2 --
 ...eoutResilienceStrategyBuilderExtensions.cs |  2 --
 src/Polly.Core/Utils/TypeNameFormatter.cs     | 24 +++++++++++++++++++
 .../Polly.RateLimiting.csproj                 |  1 -
 ...iterResilienceStrategyBuilderExtensions.cs |  2 --
 ...itBreakerResilienceStrategyBuilderTests.cs | 12 ++++------
 ...esilienceStrategyBuilderExtensionsTests.cs |  6 ++---
 ...esilienceStrategyBuilderExtensionsTests.cs |  9 +++----
 .../ResilienceStrategyBuilderTests.cs         |  2 +-
 ...esilienceStrategyBuilderExtensionsTests.cs |  6 ++---
 ...esilienceStrategyBuilderExtensionsTests.cs |  2 +-
 .../Utils/TypeNameFormatterTests.cs           | 15 ++++++++++++
 ...esilienceStrategyBuilderExtensionsTests.cs |  4 ++--
 17 files changed, 55 insertions(+), 42 deletions(-)
 create mode 100644 src/Polly.Core/Utils/TypeNameFormatter.cs
 create mode 100644 test/Polly.Core.Tests/Utils/TypeNameFormatterTests.cs

diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs
index 9f91f1bc45f..b9ead87a5a6 100644
--- 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
index 3ae51e0e949..7a6165cea45 100644
--- 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
index bc8b845f29b..4c43896da91 100644
--- 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
index 49a9f5a44c7..ed6e17f5c54 100644
--- 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
index 319af049767..d0c1b786880 100644
--- 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
index d57edb6568b..5dc00b14bc2 100644
--- 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
new file mode 100644
index 00000000000..6ec5f07fddd
--- /dev/null
+++ 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
index 309aa5624cb..adb54088e78 100644
--- 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
index 9a659a7b7b6..d8b49ba831d 100644
--- 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
index 219b05f6d29..ef77a83a16b 100644
--- 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
index 445cc45cd4f..48070144373 100644
--- 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
index baaa9fb1f34..7965b49c736 100644
--- 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
index 41f753022c9..48506ba1bfc 100644
--- 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
index 0a33f809fd3..bce599a018c 100644
--- 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
index 80b62eb982b..10e828e2904 100644
--- 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
new file mode 100644
index 00000000000..ee91f54bf80
--- /dev/null
+++ 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
index 68c1ab21154..f256d42866a 100644
--- 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.