Cleanup OutcomeResilienceStrategy

App-vNext · Jul 26, 2023 · faa77a2 · faa77a2
1 parent 0cebeea
commit faa77a2
Show file tree

Hide file tree

Showing 15 changed files with 65 additions and 109 deletions.
diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs
@@ -9,9 +9,7 @@ public CircuitBreakerResilienceStrategy(
         Func<OutcomeArguments<T, CircuitBreakerPredicateArguments>, ValueTask<bool>> handler,
         CircuitStateController<T> controller,
         CircuitBreakerStateProvider? stateProvider,
-        CircuitBreakerManualControl? manualControl,
-        bool isGeneric)
-        : base(isGeneric)
+        CircuitBreakerManualControl? manualControl)
     {
         _handler = handler;
         _controller = controller;
@@ -23,7 +21,7 @@ public CircuitBreakerResilienceStrategy(
             _controller.Dispose);
     }
 
-    protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
     {
         if (await _controller.OnActionPreExecuteAsync(context).ConfigureAwait(context.ContinueOnCapturedContext) is Outcome<T> outcome)
         {

diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs
@@ -156,8 +156,7 @@ private static TBuilder AddSimpleCircuitBreakerCore<TBuilder, TResult>(this TBui
             options.ShouldHandle!,
             controller,
             options.StateProvider,
-            options.ManualControl,
-            context.IsGenericBuilder);
+            options.ManualControl);
     }
 }
 
diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs
@@ -10,15 +10,14 @@ internal sealed class FallbackResilienceStrategy<T> : OutcomeResilienceStrategy<
     private readonly Func<OutcomeArguments<T, OnFallbackArguments>, ValueTask>? _onFallback;
     private readonly ResilienceStrategyTelemetry _telemetry;
 
-    public FallbackResilienceStrategy(FallbackHandler<T> handler, Func<OutcomeArguments<T, OnFallbackArguments>, ValueTask>? onFallback, ResilienceStrategyTelemetry telemetry, bool isGeneric)
-        : base(isGeneric)
+    public FallbackResilienceStrategy(FallbackHandler<T> handler, Func<OutcomeArguments<T, OnFallbackArguments>, ValueTask>? onFallback, ResilienceStrategyTelemetry telemetry)
     {
         _handler = handler;
         _onFallback = onFallback;
         _telemetry = telemetry;
     }
 
-    protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
     {
         var outcome = await ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
         var handleFallbackArgs = new OutcomeArguments<T, FallbackPredicateArguments>(context, outcome, default);

diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs
@@ -62,8 +62,7 @@ internal static ResilienceStrategyBuilder AddFallback(this ResilienceStrategyBui
             return new FallbackResilienceStrategy<TResult>(
                 handler,
                 options.OnFallback,
-                context.Telemetry,
-                context.IsGenericBuilder);
+                context.Telemetry);
         },
         options);
     }

diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs
@@ -17,9 +17,7 @@ public HedgingResilienceStrategy(
         Func<OutcomeArguments<T, OnHedgingArguments>, ValueTask>? onHedging,
         Func<HedgingDelayArguments, ValueTask<TimeSpan>>? hedgingDelayGenerator,
         TimeProvider timeProvider,
-        ResilienceStrategyTelemetry telemetry,
-        bool isGeneric)
-        : base(isGeneric)
+        ResilienceStrategyTelemetry telemetry)
     {
         HedgingDelay = hedgingDelay;
         MaxHedgedAttempts = maxHedgedAttempts;
@@ -43,7 +41,7 @@ public HedgingResilienceStrategy(
     public Func<OutcomeArguments<T, OnHedgingArguments>, ValueTask>? OnHedging { get; }
 
     [ExcludeFromCodeCoverage] // coverlet issue
-    protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(
+    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(
         Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback,
         ResilienceContext context,
         TState state)

diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategyBuilderExtensions.cs
@@ -67,8 +67,7 @@ internal static ResilienceStrategyBuilder AddHedging(this ResilienceStrategyBuil
                 options.OnHedging,
                 options.HedgingDelayGenerator,
                 context.TimeProvider,
-                context.Telemetry,
-                context.IsGenericBuilder);
+                context.Telemetry);
         },
         options);
     }

diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs
@@ -11,11 +11,9 @@ internal sealed class RetryResilienceStrategy<T> : OutcomeResilienceStrategy<T>
 
     public RetryResilienceStrategy(
         RetryStrategyOptions<T> options,
-        bool isGeneric,
         TimeProvider timeProvider,
         ResilienceStrategyTelemetry telemetry,
         Func<double> randomizer)
-        : base(isGeneric)
     {
         ShouldHandle = options.ShouldHandle;
         BaseDelay = options.BaseDelay;
@@ -41,7 +39,7 @@ public RetryResilienceStrategy(
 
     public Func<OutcomeArguments<T, OnRetryArguments>, ValueTask>? OnRetry { get; }
 
-    protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+    protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
     {
         double retryState = 0;
 

diff --git a/src/Polly.Core/Retry/RetryResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Retry/RetryResilienceStrategyBuilderExtensions.cs
@@ -55,7 +55,6 @@ public static ResilienceStrategyBuilder<TResult> AddRetry<TResult>(this Resilien
         builder.AddStrategy(context =>
             new RetryResilienceStrategy<TResult>(
                 options,
-                context.IsGenericBuilder,
                 context.TimeProvider,
                 context.Telemetry,
                 context.Randomizer),

diff --git a/src/Polly.Core/Utils/OutcomeResilienceStrategy.cs b/src/Polly.Core/Utils/OutcomeResilienceStrategy.cs
@@ -10,71 +10,41 @@
 /// </remarks>
 internal abstract class OutcomeResilienceStrategy<T> : ResilienceStrategy
 {
-    private readonly bool _isGeneric;
-
-    protected OutcomeResilienceStrategy(bool isGeneric)
+    protected OutcomeResilienceStrategy()
     {
-        if (!isGeneric && typeof(T) != typeof(object))
-        {
-            throw new NotSupportedException("For non-generic strategies the generic parameter should be of type 'object'.");
-        }
-
-        _isGeneric = isGeneric;
     }
 
     protected internal sealed override ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState>(
         Func<ResilienceContext, TState, ValueTask<Outcome<TResult>>> callback,
         ResilienceContext context,
         TState state)
     {
-        if (_isGeneric)
+        // Check if we can cast directly, thus saving some cycles and improving the performance
+        if (context.ResultType == typeof(T))
         {
-            if (typeof(TResult) != typeof(T))
-            {
-                return callback(context, state);
-            }
-
             // cast is safe here, because TResult and T are the same type
             var callbackCasted = (Func<ResilienceContext, TState, ValueTask<Outcome<T>>>)(object)callback;
-            var valueTask = ExecuteCallbackAsync(callbackCasted, context, state);
+            var valueTask = ExecuteCore(callbackCasted, context, state);
 
-            return ConvertValueTask<TResult>(valueTask, context);
+            return TaskHelper.ConvertValueTask<T, TResult>(valueTask, context);
         }
         else
         {
-            var valueTask = ExecuteCallbackAsync(
+            var valueTask = ExecuteCore(
                 static async (context, state) =>
                 {
                     var outcome = await state.callback(context, state.state).ConfigureAwait(context.ContinueOnCapturedContext);
-
-                    // cast the outcome to "object" based on (T)
                     return outcome.AsOutcome<T>();
                 },
                 context,
                 (callback, state));
 
-            return ConvertValueTask<TResult>(valueTask, context);
+            return TaskHelper.ConvertValueTask<T, TResult>(valueTask, context);
         }
     }
 
-    protected abstract ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(
+    protected abstract ValueTask<Outcome<T>> ExecuteCore<TState>(
         Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback,
         ResilienceContext context,
         TState state);
-
-    private static ValueTask<Outcome<TResult>> ConvertValueTask<TResult>(ValueTask<Outcome<T>> valueTask, ResilienceContext resilienceContext)
-    {
-        if (valueTask.IsCompletedSuccessfully)
-        {
-            return new ValueTask<Outcome<TResult>>(valueTask.Result.AsOutcome<TResult>());
-        }
-
-        return ConvertValueTaskAsync(valueTask, resilienceContext);
-
-        static async ValueTask<Outcome<TResult>> ConvertValueTaskAsync(ValueTask<Outcome<T>> valueTask, ResilienceContext resilienceContext)
-        {
-            var outcome = await valueTask.ConfigureAwait(resilienceContext.ContinueOnCapturedContext);
-            return outcome.AsOutcome<TResult>();
-        }
-    }
 }
diff --git a/src/Polly.Core/Utils/TaskHelper.cs b/src/Polly.Core/Utils/TaskHelper.cs
@@ -0,0 +1,22 @@
+using System.Threading.Tasks;
+
+namespace Polly.Utils;
+
+internal static class TaskHelper
+{
+    public static ValueTask<Outcome<TTo>> ConvertValueTask<TFrom, TTo>(ValueTask<Outcome<TFrom>> valueTask, ResilienceContext resilienceContext)
+    {
+        if (valueTask.IsCompletedSuccessfully)
+        {
+            return new ValueTask<Outcome<TTo>>(valueTask.Result.AsOutcome<TTo>());
+        }
+
+        return ConvertValueTaskAsync(valueTask, resilienceContext);
+
+        static async ValueTask<Outcome<TTo>> ConvertValueTaskAsync(ValueTask<Outcome<TFrom>> valueTask, ResilienceContext resilienceContext)
+        {
+            var outcome = await valueTask.ConfigureAwait(resilienceContext.ContinueOnCapturedContext);
+            return outcome.AsOutcome<TTo>();
+        }
+    }
+}
diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs
@@ -127,11 +127,8 @@ public void Execute_Ok()
         _options.ShouldHandle = _ => PredicateResult.False;
         _behavior.Setup(v => v.OnActionSuccess(CircuitState.Closed));
 
-        Create().Invoking(s => s.Execute(_ => { })).Should().NotThrow();
+        Create().Invoking(s => s.Execute(_ => 0)).Should().NotThrow();
     }
 
-    private CircuitBreakerResilienceStrategy<int> Create()
-    {
-        return new(_options.ShouldHandle!, _controller, _options.StateProvider, _options.ManualControl, true);
-    }
+    private CircuitBreakerResilienceStrategy<int> Create() => new(_options.ShouldHandle!, _controller, _options.StateProvider, _options.ManualControl);
 }
diff --git a/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs b/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs
@@ -18,16 +18,6 @@ public void Ctor_Ok()
         Create().Should().NotBeNull();
     }
 
-    [Fact]
-    public void DoesntHandle_Skips()
-    {
-        SetHandler(_ => true, () => Outcome.FromResult("dummy"), true);
-
-        Create().Execute(() => -1).Should().Be(-1);
-
-        _args.Should().BeEmpty();
-    }
-
     [Fact]
     public void Handle_Result_Ok()
     {
@@ -70,7 +60,7 @@ public void Handle_UnhandledException_Ok()
         _options.OnFallback = _ => { called = true; return default; };
         SetHandler(outcome => outcome.Exception is InvalidOperationException, () => { fallbackActionCalled = true; return Outcome.FromResult("secondary"); });
 
-        Create().Invoking(s => s.Execute<int>(_ => throw new ArgumentException())).Should().Throw<ArgumentException>();
+        Create().Invoking(s => s.Execute<string>(_ => throw new ArgumentException())).Should().Throw<ArgumentException>();
 
         _args.Should().BeEmpty();
         called.Should().BeFalse();
@@ -103,6 +93,5 @@ private void SetHandler(
     private FallbackResilienceStrategy<string> Create() => new(
         _handler!,
         _options.OnFallback,
-        _telemetry,
-        true);
+        _telemetry);
 }
diff --git a/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs b/test/Polly.Core.Tests/Hedging/HedgingResilienceStrategyTests.cs
@@ -54,15 +54,6 @@ public void Ctor_EnsureDefaults()
         strategy.HedgingHandler.Should().NotBeNull();
     }
 
-    [Fact]
-    public void Execute_Skipped_Ok()
-    {
-        ConfigureHedging();
-        var strategy = Create();
-
-        strategy.Execute(_ => 10).Should().Be(10);
-    }
-
     [Fact]
     public async Task Execute_CancellationRequested_Throws()
     {
@@ -973,6 +964,5 @@ private HedgingResilienceStrategy<T> Create<T>(
         onHedging,
         _options.HedgingDelayGenerator,
         _timeProvider,
-        _telemetry,
-        true);
+        _telemetry);
 }
diff --git a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs
@@ -352,7 +352,6 @@ private async ValueTask<int> ExecuteAndAdvance(RetryResilienceStrategy<object> s
 
     private RetryResilienceStrategy<object> CreateSut(TimeProvider? timeProvider = null) =>
         new(_options,
-            false,
             timeProvider ?? _timeProvider,
             _telemetry,
             () => 1.0);

diff --git a/test/Polly.Core.Tests/Utils/OutcomeResilienceStrategyTests.cs b/test/Polly.Core.Tests/Utils/OutcomeResilienceStrategyTests.cs
@@ -7,15 +7,7 @@ public class OutcomeResilienceStrategyTests
     [Fact]
     public void Ctor_Ok()
     {
-        new Strategy<string>(args => { }, true).Should().NotBeNull();
-        new Strategy<object>(args => { }, false).Should().NotBeNull();
-        new Strategy<object>(args => { }, true).Should().NotBeNull();
-    }
-
-    [Fact]
-    public void Ctor_InvalidArgs_Throws()
-    {
-        this.Invoking(_ => new Strategy<string>(_ => { }, false)).Should().Throw<NotSupportedException>();
+        new Strategy<string>(args => { }).Should().NotBeNull();
     }
 
     [Fact]
@@ -26,8 +18,7 @@ public void Execute_NonGeneric_Ok()
         var strategy = new Strategy<object>(outcome =>
         {
             values.Add(outcome.Result);
-        },
-        false);
+        });
 
         strategy.Execute(args => "dummy");
         strategy.Execute(args => 0);
@@ -48,31 +39,40 @@ public void Execute_Generic_Ok()
         var strategy = new Strategy<string>(outcome =>
         {
             values.Add(outcome.Result);
-        },
-        true);
+        });
 
         strategy.Execute(args => "dummy");
-        strategy.Execute(args => 0);
-        strategy.Execute<object?>(args => null);
-        strategy.Execute(args => true);
 
         values.Should().HaveCount(1);
         values[0].Should().Be("dummy");
     }
 
+    [Fact]
+    public void Pipeline_TypeCheck_Ok()
+    {
+        var called = false;
+        var strategy = new Strategy<object>(o =>
+        {
+            o.Result.Should().Be(-1);
+            called = true;
+        });
+
+        strategy.Execute(() => -1);
+
+        called.Should().BeTrue();
+    }
+
     private class Strategy<T> : OutcomeResilienceStrategy<T>
     {
         private readonly Action<Outcome<T>> _onOutcome;
 
-        public Strategy(Action<Outcome<T>> onOutcome, bool isGeneric)
-            : base(isGeneric) => _onOutcome = onOutcome;
+        public Strategy(Action<Outcome<T>> onOutcome) => _onOutcome = onOutcome;
 
-        protected override async ValueTask<Outcome<T>> ExecuteCallbackAsync<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
+        protected override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
         {
             var outcome = await callback(context, state);
             _onOutcome(outcome);
             return outcome;
         }
     }
-
 }