Simplify the creation of outcome invokers (#1237)

* Simplify the creation of outcome invokers * fixes * fixes * new tests * new tests
App-vNext · May 30, 2023 · 141bd0a · 141bd0a
1 parent 63853e2
commit 141bd0a
Show file tree

Hide file tree

Showing 21 changed files with 164 additions and 61 deletions.
diff --git a/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs b/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs
@@ -134,6 +134,6 @@ public void Execute_Ok()
 
     private CircuitBreakerResilienceStrategy Create()
     {
-        return new(PredicateInvoker<CircuitBreakerPredicateArguments>.NonGeneric(_options.ShouldHandle!), _controller, _options.StateProvider, _options.ManualControl);
+        return new(PredicateInvoker<CircuitBreakerPredicateArguments>.Create(_options.ShouldHandle!, false)!, _controller, _options.StateProvider, _options.ManualControl);
     }
 }
diff --git a/src/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs b/src/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs
@@ -435,8 +435,8 @@ private async Task OpenCircuit(CircuitStateController controller, Outcome<int>?
 
     private CircuitStateController CreateController() => new(
         _options.BreakDuration,
-        EventInvoker<OnCircuitOpenedArguments>.NonGeneric(_options.OnOpened),
-        EventInvoker<OnCircuitClosedArguments>.NonGeneric(_options.OnClosed),
+        EventInvoker<OnCircuitOpenedArguments>.Create(_options.OnOpened, false),
+        EventInvoker<OnCircuitClosedArguments>.Create(_options.OnClosed, false),
         _options.OnHalfOpened,
         _circuitBehavior.Object,
         _timeProvider.Object,

diff --git a/src/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs b/src/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs
@@ -130,6 +130,6 @@ public void Handle_UnhandledResult_Ok()
 
     private FallbackResilienceStrategy Create() => new(
         _options.Handler.CreateHandler(),
-        EventInvoker<OnFallbackArguments>.NonGeneric(_options.OnFallback),
+        EventInvoker<OnFallbackArguments>.Create(_options.OnFallback, false),
         _telemetry);
 }
diff --git a/src/Polly.Core.Tests/GenericResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/GenericResilienceStrategyBuilderTests.cs
@@ -15,6 +15,13 @@ public void Ctor_EnsureDefaults()
         _builder.Properties.Should().NotBeNull();
         _builder.TimeProvider.Should().Be(TimeProvider.System);
         _builder.OnCreatingStrategy.Should().BeNull();
+        _builder.Builder.IsGenericBuilder.Should().BeTrue();
+    }
+
+    [Fact]
+    public void CopyCtor_Ok()
+    {
+        new ResilienceStrategyBuilder<string>(new ResilienceStrategyBuilder()).Builder.IsGenericBuilder.Should().BeTrue();
     }
 
     [Fact]

diff --git a/src/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs b/src/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs
@@ -888,7 +888,7 @@ private void ConfigureHedging(TimeSpan delay) => ConfigureHedging(args => async
         _options.HedgingDelay,
         _options.MaxHedgedAttempts,
         _options.Handler.CreateHandler(),
-        EventInvoker<OnHedgingArguments>.NonGeneric(_options.OnHedging),
+        EventInvoker<OnHedgingArguments>.Create(_options.OnHedging, false),
         _options.HedgingDelayGenerator,
         _timeProvider,
         _telemetry);

diff --git a/src/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs
@@ -15,6 +15,7 @@ public void Ctor_EnsureDefaults()
         builder.BuilderName.Should().Be("");
         builder.Properties.Should().NotBeNull();
         builder.TimeProvider.Should().Be(TimeProvider.System);
+        builder.IsGenericBuilder.Should().BeFalse();
     }
 
     [Fact]
@@ -250,12 +251,14 @@ public void BuildStrategy_EnsureCorrectContext()
         var builder = new ResilienceStrategyBuilder
         {
             BuilderName = "builder-name",
-            TimeProvider = new FakeTimeProvider().Object
+            TimeProvider = new FakeTimeProvider().Object,
+            IsGenericBuilder = true
         };
 
         builder.AddStrategy(
             context =>
             {
+                context.IsGenericBuilder.Should().BeTrue();
                 context.BuilderName.Should().Be("builder-name");
                 context.StrategyName.Should().Be("strategy-name");
                 context.StrategyType.Should().Be("Test");

diff --git a/src/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs b/src/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs
@@ -232,9 +232,9 @@ private RetryResilienceStrategy CreateSut()
             _options.BaseDelay,
             _options.BackoffType,
             _options.RetryCount,
-            PredicateInvoker<ShouldRetryArguments>.NonGeneric(_options.ShouldRetry!),
-            EventInvoker<OnRetryArguments>.NonGeneric(_options.OnRetry),
-            GeneratorInvoker<RetryDelayArguments, TimeSpan>.NonGeneric(_options.RetryDelayGenerator),
+            PredicateInvoker<ShouldRetryArguments>.Create(_options.ShouldRetry!, false)!,
+            EventInvoker<OnRetryArguments>.Create(_options.OnRetry, false),
+            GeneratorInvoker<RetryDelayArguments, TimeSpan>.Create(_options.RetryDelayGenerator, TimeSpan.MinValue, false),
             _timeProvider.Object,
             _telemetry,
             RandomUtil.Instance);

diff --git a/src/Polly.Core.Tests/Strategy/EventInvokerTests.cs b/src/Polly.Core.Tests/Strategy/EventInvokerTests.cs
@@ -9,22 +9,23 @@ public class EventInvokerTests
     [Fact]
     public void NullCallback_Ok()
     {
-        EventInvoker<TestArguments>.NonGeneric(null).Should().BeNull();
-        EventInvoker<TestArguments>.Generic<string>(null).Should().BeNull();
+        EventInvoker<TestArguments>.Create<string>(null, isGeneric: true).Should().BeNull();
+        EventInvoker<TestArguments>.Create<object>(null, isGeneric: true).Should().BeNull();
     }
 
     [Fact]
     public async Task HandleAsync_NonGeneric_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
         var called = false;
-        var invoker = EventInvoker<TestArguments>.NonGeneric((outcome, args) =>
+        var invoker = EventInvoker<TestArguments>.Create<object>((outcome, args) =>
         {
             outcome.Result.Should().Be(10);
             args.Context.Should().NotBeNull();
             called = true;
             return default;
-        })!;
+        },
+        false)!;
 
         await invoker.HandleAsync(new Outcome<int>(10), args);
         called.Should().Be(true);
@@ -35,13 +36,14 @@ public async Task HandleAsync_Generic_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
         var called = false;
-        var invoker = EventInvoker<TestArguments>.Generic<int>((outcome, args) =>
+        var invoker = EventInvoker<TestArguments>.Create<int>((outcome, args) =>
         {
             args.Context.Should().NotBeNull();
             outcome.Result.Should().Be(10);
             called = true;
             return default;
-        })!;
+        },
+        true)!;
 
         await invoker.HandleAsync(new Outcome<int>(10), args);
         called.Should().Be(true);
@@ -50,4 +52,17 @@ public async Task HandleAsync_Generic_Ok()
         await invoker.HandleAsync(new Outcome<string>("dummy"), args);
         called.Should().Be(false);
     }
+
+    [Fact]
+    public async Task HandleAsync_GenericObject_Ok()
+    {
+        var called = false;
+        var args = new TestArguments(ResilienceContext.Get());
+        var invoker = EventInvoker<TestArguments>.Create<object>((_, _) => { called = true; return default; }, true);
+        await invoker!.HandleAsync(new Outcome<string>("dummy"), args);
+        called.Should().BeFalse();
+
+        await invoker!.HandleAsync(new Outcome<object>("dummy"), args);
+        called.Should().BeTrue();
+    }
 }
diff --git a/src/Polly.Core.Tests/Strategy/GeneratorInvokerTests.cs b/src/Polly.Core.Tests/Strategy/GeneratorInvokerTests.cs
@@ -8,21 +8,23 @@ public class GeneratorInvokerTests
     [Fact]
     public void NullCallback_Ok()
     {
-        GeneratorInvoker<TestArguments, string>.NonGeneric(null).Should().BeNull();
-        GeneratorInvoker<TestArguments, string>.Generic<string>(null, "default").Should().BeNull();
+        GeneratorInvoker<TestArguments, string>.Create<string>(null, "default", false).Should().BeNull();
+        GeneratorInvoker<TestArguments, string>.Create<string>(null, "default", true).Should().BeNull();
     }
 
     [Fact]
     public async Task HandleAsync_NonGeneric_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
-        var invoker = GeneratorInvoker<TestArguments, string>.NonGeneric((outcome, args) =>
+        var invoker = GeneratorInvoker<TestArguments, string>.Create<object>((outcome, args) =>
         {
             args.Context.Should().NotBeNull();
             outcome.Result.Should().Be(10);
 
             return new ValueTask<string>("generated-value");
-        })!;
+        },
+        "default",
+        false)!;
 
         (await invoker.HandleAsync(new Outcome<int>(10), args)).Should().Be("generated-value");
     }
@@ -31,16 +33,30 @@ public async Task HandleAsync_NonGeneric_Ok()
     public async Task HandleAsync_Generic_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
-        var invoker = GeneratorInvoker<TestArguments, string>.Generic<int>((outcome, args) =>
+        var invoker = GeneratorInvoker<TestArguments, string>.Create<int>((outcome, args) =>
         {
             args.Context.Should().NotBeNull();
             outcome.Result.Should().Be(10);
 
             return new ValueTask<string>("generated-value");
         },
-        "default")!;
+        "default",
+        true)!;
 
         (await invoker.HandleAsync(new Outcome<int>(10), args)).Should().Be("generated-value");
         (await invoker.HandleAsync(new Outcome<string>("dummy"), args)).Should().Be("default");
+
+        invoker = GeneratorInvoker<TestArguments, string>.Create<object>((_, _) => new ValueTask<string>("dummy"), "default", true);
+        (await invoker!.HandleAsync(new Outcome<string>("dummy"), args)).Should().Be("default");
+        (await invoker!.HandleAsync(new Outcome<object>("dummy"), args)).Should().Be("dummy");
+    }
+
+    [Fact]
+    public async Task HandleAsync_GenericObject_Ok()
+    {
+        var args = new TestArguments(ResilienceContext.Get());
+        var invoker = GeneratorInvoker<TestArguments, string>.Create<object>((_, _) => new ValueTask<string>("dummy"), "default", true);
+        (await invoker!.HandleAsync(new Outcome<string>("dummy"), args)).Should().Be("default");
+        (await invoker!.HandleAsync(new Outcome<object>("dummy"), args)).Should().Be("dummy");
     }
 }
diff --git a/src/Polly.Core.Tests/Strategy/PredicateInvokerTests.cs b/src/Polly.Core.Tests/Strategy/PredicateInvokerTests.cs
@@ -6,20 +6,28 @@ namespace Polly.Core.Tests.Strategy;
 
 public class PredicateInvokerTests
 {
+    [Fact]
+    public void NullCallback_Ok()
+    {
+        PredicateInvoker<TestArguments>.Create<string>(null, isGeneric: true).Should().BeNull();
+        PredicateInvoker<TestArguments>.Create<object>(null, isGeneric: true).Should().BeNull();
+    }
+
     [Fact]
     public async Task HandleAsync_NonGeneric_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
         var called = false;
-        var invoker = PredicateInvoker<TestArguments>.NonGeneric((outcome, _) =>
+        var invoker = PredicateInvoker<TestArguments>.Create<object>((outcome, _) =>
         {
             outcome.Result.Should().Be(10);
             args.Context.Should().NotBeNull();
             called = true;
             return new ValueTask<bool>(true);
-        });
+        },
+        false);
 
-        (await invoker.HandleAsync(new Outcome<int>(10), args)).Should().Be(true);
+        (await invoker!.HandleAsync(new Outcome<int>(10), args)).Should().Be(true);
         called.Should().Be(true);
     }
 
@@ -28,19 +36,29 @@ public async Task HandleAsync_Generic_Ok()
     {
         var args = new TestArguments(ResilienceContext.Get());
         var called = false;
-        var invoker = PredicateInvoker<TestArguments>.Generic<int>((outcome, args) =>
+        var invoker = PredicateInvoker<TestArguments>.Create<int>((outcome, args) =>
         {
             outcome.Result.Should().Be(10);
             args.Context.Should().NotBeNull();
             called = true;
             return new ValueTask<bool>(true);
-        });
+        },
+        true);
 
-        (await invoker.HandleAsync(new Outcome<int>(10), args)).Should().Be(true);
+        (await invoker!.HandleAsync(new Outcome<int>(10), args)).Should().Be(true);
         called.Should().Be(true);
 
         called = false;
         (await invoker.HandleAsync(new Outcome<string>("dummy"), args)).Should().Be(false);
         called.Should().Be(false);
     }
+
+    [Fact]
+    public async Task HandleAsync_GenericObject_Ok()
+    {
+        var args = new TestArguments(ResilienceContext.Get());
+        var invoker = PredicateInvoker<TestArguments>.Create<object>((_, _) => PredicateResult.True, true);
+        (await invoker!.HandleAsync(new Outcome<string>("dummy"), args)).Should().BeFalse();
+        (await invoker!.HandleAsync(new Outcome<object>("dummy"), args)).Should().BeTrue();
+    }
 }
diff --git a/src/Polly.Core.Tests/Strategy/ResilienceStrategyBuilderContextTests.cs b/src/Polly.Core.Tests/Strategy/ResilienceStrategyBuilderContextTests.cs
@@ -10,8 +10,9 @@ public void Ctor_EnsureDefaults()
     {
         var properties = new ResilienceProperties();
         var timeProvider = new FakeTimeProvider();
-        var context = new ResilienceStrategyBuilderContext("builder-name", properties, "strategy-name", "strategy-type", timeProvider.Object);
+        var context = new ResilienceStrategyBuilderContext("builder-name", properties, "strategy-name", "strategy-type", timeProvider.Object, true);
 
+        context.IsGenericBuilder.Should().BeTrue();
         context.BuilderName.Should().Be("builder-name");
         context.BuilderProperties.Should().BeSameAs(properties);
         context.StrategyName.Should().Be("strategy-name");

diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs
@@ -132,15 +132,15 @@ internal static CircuitBreakerResilienceStrategy CreateStrategy(ResilienceStrate
     {
         var controller = new CircuitStateController(
             options.BreakDuration,
-            EventInvoker<OnCircuitOpenedArguments>.NonGeneric(options.OnOpened),
-            EventInvoker<OnCircuitClosedArguments>.NonGeneric(options.OnClosed),
+            context.CreateInvoker(options.OnOpened),
+            context.CreateInvoker(options.OnClosed),
             options.OnHalfOpened,
             behavior,
             context.TimeProvider,
             context.Telemetry);
 
         return new CircuitBreakerResilienceStrategy(
-            PredicateInvoker<CircuitBreakerPredicateArguments>.NonGeneric(options.ShouldHandle!),
+            context.CreateInvoker(options.ShouldHandle)!,
             controller,
             options.StateProvider,
             options.ManualControl);
@@ -150,15 +150,15 @@ internal static CircuitBreakerResilienceStrategy CreateStrategy<TResult>(Resilie
     {
         var controller = new CircuitStateController(
             options.BreakDuration,
-            EventInvoker<OnCircuitOpenedArguments>.Generic(options.OnOpened),
-            EventInvoker<OnCircuitClosedArguments>.Generic(options.OnClosed),
+            context.CreateInvoker(options.OnOpened),
+            context.CreateInvoker(options.OnClosed),
             options.OnHalfOpened,
             behavior,
             context.TimeProvider,
             context.Telemetry);
 
         return new CircuitBreakerResilienceStrategy(
-            PredicateInvoker<CircuitBreakerPredicateArguments>.Generic(options.ShouldHandle!),
+            context.CreateInvoker(options.ShouldHandle)!,
             controller,
             options.StateProvider,
             options.ManualControl);

diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs
@@ -68,7 +68,7 @@ public static ResilienceStrategyBuilder<TResult> AddFallback<TResult>(this Resil
         return builder.AddStrategy(context =>
             new FallbackResilienceStrategy(
                 handler,
-                EventInvoker<OnFallbackArguments>.Generic(options.OnFallback),
+                context.CreateInvoker(options.OnFallback),
                 context.Telemetry),
             options);
     }
@@ -91,7 +91,7 @@ internal static ResilienceStrategyBuilder AddFallback(this ResilienceStrategyBui
         return builder.AddStrategy(context =>
             new FallbackResilienceStrategy(
                 options.Handler.CreateHandler(),
-                EventInvoker<OnFallbackArguments>.NonGeneric(options.OnFallback),
+                context.CreateInvoker(options.OnFallback),
                 context.Telemetry),
             options);
     }

diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs
@@ -38,7 +38,7 @@ public static ResilienceStrategyBuilder<TResult> AddHedging<TResult>(this Resili
                 options.HedgingDelay,
                 options.MaxHedgedAttempts,
                 handler,
-                EventInvoker<OnHedgingArguments>.Generic(options.OnHedging),
+                context.CreateInvoker(options.OnHedging),
                 options.HedgingDelayGenerator,
                 context.TimeProvider,
                 context.Telemetry),
@@ -65,7 +65,7 @@ internal static ResilienceStrategyBuilder AddHedging(this ResilienceStrategyBuil
                 options.HedgingDelay,
                 options.MaxHedgedAttempts,
                 options.Handler.CreateHandler(),
-                EventInvoker<OnHedgingArguments>.NonGeneric(options.OnHedging),
+                context.CreateInvoker(options.OnHedging),
                 options.HedgingDelayGenerator,
                 context.TimeProvider,
                 context.Telemetry),

diff --git a/src/Polly.Core/ResilienceStrategyBuilder.TResult.cs b/src/Polly.Core/ResilienceStrategyBuilder.TResult.cs
@@ -17,9 +17,16 @@ public class ResilienceStrategyBuilder<TResult>
     /// <summary>
     /// Initializes a new instance of the <see cref="ResilienceStrategyBuilder{TResult}"/> class.
     /// </summary>
-    public ResilienceStrategyBuilder() => Builder = new();
+    public ResilienceStrategyBuilder() => Builder = new()
+    {
+        IsGenericBuilder = true
+    };
 
-    internal ResilienceStrategyBuilder(ResilienceStrategyBuilder builder) => Builder = builder;
+    internal ResilienceStrategyBuilder(ResilienceStrategyBuilder builder)
+    {
+        Builder = builder;
+        Builder.IsGenericBuilder = true;
+    }
 
     /// <summary>
     /// Gets or sets the name of the builder.

diff --git a/src/Polly.Core/ResilienceStrategyBuilder.cs b/src/Polly.Core/ResilienceStrategyBuilder.cs
@@ -42,6 +42,8 @@ public class ResilienceStrategyBuilder
     /// </summary>
     internal Action<IList<ResilienceStrategy>>? OnCreatingStrategy { get; set; }
 
+    internal bool IsGenericBuilder { get; set; }
+
     /// <summary>
     /// Adds an already created strategy instance to the builder.
     /// </summary>
@@ -116,7 +118,8 @@ private ResilienceStrategy CreateResilienceStrategy(Entry entry)
             builderProperties: Properties,
             strategyName: entry.Properties.StrategyName,
             strategyType: entry.Properties.StrategyType,
-            timeProvider: TimeProvider);
+            timeProvider: TimeProvider,
+            IsGenericBuilder);
 
         return entry.Factory(context);
     }