Drop DelegatingResilienceStrategy (#1063)

src/Polly.Core.Benchmarks/Benchmarks/PipelineBenchmark.cs

@@ -7,14 +7,12 @@ public class PipelineBenchmark
 {
     private object? _strategyV7;
     private object? _strategyV8;
-    private object? _strategyV8Delegating;
 
     [GlobalSetup]
     public void Setup()
     {
-        _strategyV7 = Helper.CreatePipeline(PollyVersion.V7, Components, delegating: false);
-        _strategyV8 = Helper.CreatePipeline(PollyVersion.V8, Components, delegating: false);
-        _strategyV8Delegating = Helper.CreatePipeline(PollyVersion.V8, Components, delegating: true);
+        _strategyV7 = Helper.CreatePipeline(PollyVersion.V7, Components);
+        _strategyV8 = Helper.CreatePipeline(PollyVersion.V8, Components);
     }
 
     [Params(1, 2, 5, 10)]
@@ -25,7 +23,4 @@ public void Setup()
 
     [Benchmark]
     public ValueTask ExecutePipeline_V8() => _strategyV8!.ExecuteAsync(PollyVersion.V8);
-
-    [Benchmark]
-    public ValueTask ExecutePipeline_V8Delegating() => _strategyV8Delegating!.ExecuteAsync(PollyVersion.V8);
 }

src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs

@@ -4,15 +4,15 @@ namespace Polly.Core.Benchmarks;
 
 internal static partial class Helper
 {
-    public static object CreatePipeline(PollyVersion technology, int count, bool delegating) => technology switch
+    public static object CreatePipeline(PollyVersion technology, int count) => technology switch
     {
         PollyVersion.V7 => count == 1 ? Policy.NoOpAsync<int>() : Policy.WrapAsync(Enumerable.Repeat(0, count).Select(_ => Policy.NoOpAsync<int>()).ToArray()),
 
         PollyVersion.V8 => CreateStrategy(builder =>
         {
             for (var i = 0; i < count; i++)
             {
-                builder.AddStrategy(delegating ? new EmptyDelegatingResilienceStrategy() : new EmptyResilienceStrategy());
+                builder.AddStrategy(new EmptyResilienceStrategy());
             }
         }),
         _ => throw new NotSupportedException()

src/Polly.Core.Benchmarks/README.md

@@ -12,20 +12,16 @@ LaunchCount=2  WarmupCount=10
 
 ## PIPELINES
 
-|                       Method | Components |        Mean |     Error |    StdDev |      Median | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
-|----------------------------- |----------- |------------:|----------:|----------:|------------:|------:|--------:|-------:|----------:|------------:|
-|           ExecutePipeline_V7 |          1 |    74.84 ns |  1.279 ns |  1.835 ns |    75.81 ns |  1.00 |    0.00 | 0.0362 |     304 B |        1.00 |
-|           ExecutePipeline_V8 |          1 |    72.61 ns |  0.584 ns |  0.819 ns |    72.28 ns |  0.97 |    0.02 | 0.0048 |      40 B |        0.13 |
-| ExecutePipeline_V8Delegating |          1 |    90.04 ns |  0.753 ns |  1.104 ns |    89.76 ns |  1.20 |    0.02 | 0.0048 |      40 B |        0.13 |
-|                              |            |             |           |           |             |       |         |        |           |             |
-|           ExecutePipeline_V7 |          2 |   156.67 ns |  2.810 ns |  4.119 ns |   154.12 ns |  1.00 |    0.00 | 0.0658 |     552 B |        1.00 |
-|           ExecutePipeline_V8 |          2 |   185.87 ns |  1.932 ns |  2.580 ns |   185.02 ns |  1.19 |    0.04 | 0.0048 |      40 B |        0.07 |
-| ExecutePipeline_V8Delegating |          2 |   127.92 ns |  1.264 ns |  1.772 ns |   129.00 ns |  0.82 |    0.02 | 0.0048 |      40 B |        0.07 |
-|                              |            |             |           |           |             |       |         |        |           |             |
-|           ExecutePipeline_V7 |          5 |   543.85 ns | 19.540 ns | 29.247 ns |   530.00 ns |  1.00 |    0.00 | 0.1545 |    1296 B |        1.00 |
-|           ExecutePipeline_V8 |          5 |   335.67 ns |  3.903 ns |  5.598 ns |   336.22 ns |  0.62 |    0.04 | 0.0048 |      40 B |        0.03 |
-| ExecutePipeline_V8Delegating |          5 |   189.99 ns |  4.045 ns |  5.929 ns |   189.05 ns |  0.35 |    0.02 | 0.0048 |      40 B |        0.03 |
-|                              |            |             |           |           |             |       |         |        |           |             |
-|           ExecutePipeline_V7 |         10 | 1,112.84 ns |  8.585 ns | 12.036 ns | 1,111.96 ns |  1.00 |    0.00 | 0.3014 |    2536 B |        1.00 |
-|           ExecutePipeline_V8 |         10 |   543.85 ns |  6.205 ns |  9.095 ns |   545.30 ns |  0.49 |    0.01 | 0.0048 |      40 B |        0.02 |
-| ExecutePipeline_V8Delegating |         10 |   258.11 ns |  0.881 ns |  1.318 ns |   258.30 ns |  0.23 |    0.00 | 0.0048 |      40 B |        0.02 |
+|             Method | Components |        Mean |     Error |    StdDev | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
+|------------------- |----------- |------------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
+| ExecutePipeline_V7 |          1 |    81.25 ns |  1.304 ns |  1.870 ns |  1.00 |    0.00 | 0.0362 |     304 B |        1.00 |
+| ExecutePipeline_V8 |          1 |    82.25 ns |  1.414 ns |  2.073 ns |  1.01 |    0.04 | 0.0048 |      40 B |        0.13 |
+|                    |            |             |           |           |       |         |        |           |             |
+| ExecutePipeline_V7 |          2 |   166.04 ns |  2.875 ns |  4.215 ns |  1.00 |    0.00 | 0.0658 |     552 B |        1.00 |
+| ExecutePipeline_V8 |          2 |   108.29 ns |  1.504 ns |  2.251 ns |  0.65 |    0.02 | 0.0048 |      40 B |        0.07 |
+|                    |            |             |           |           |       |         |        |           |             |
+| ExecutePipeline_V7 |          5 |   531.04 ns |  4.728 ns |  6.930 ns |  1.00 |    0.00 | 0.1545 |    1296 B |        1.00 |
+| ExecutePipeline_V8 |          5 |   245.50 ns |  2.344 ns |  3.509 ns |  0.46 |    0.01 | 0.0048 |      40 B |        0.03 |
+|                    |            |             |           |           |       |         |        |           |             |
+| ExecutePipeline_V7 |         10 | 1,128.82 ns | 10.838 ns | 15.886 ns |  1.00 |    0.00 | 0.3014 |    2536 B |        1.00 |
+| ExecutePipeline_V8 |         10 |   449.31 ns |  2.926 ns |  4.379 ns |  0.40 |    0.01 | 0.0048 |      40 B |        0.02 |

src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs

@@ -9,52 +9,40 @@ namespace Polly.Core.Tests.Builder;
 public class ResilienceStrategyPipelineTests
 {
     [Fact]
-    public void CreateAndFreezeStrategies_ArgValidation()
+    public void CreatePipeline_ArgValidation()
     {
-        Assert.Throws<ArgumentNullException>(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(null!));
-        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(Array.Empty<ResilienceStrategy>()));
-        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new ResilienceStrategy[] { new TestResilienceStrategy() }));
-        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new ResilienceStrategy[]
+        Assert.Throws<ArgumentNullException>(() => ResilienceStrategyPipeline.CreatePipeline(null!));
+        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipeline(Array.Empty<ResilienceStrategy>()));
+        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipeline(new ResilienceStrategy[] { new TestResilienceStrategy() }));
+        Assert.Throws<InvalidOperationException>(() => ResilienceStrategyPipeline.CreatePipeline(new ResilienceStrategy[]
         {
             NullResilienceStrategy.Instance,
             NullResilienceStrategy.Instance
         }));
     }
 
     [Fact]
-    public void CreateAndFreezeStrategies_EnsureStrategiesLinked()
+    public void CreatePipeline_EnsureOriginalStrategiesPreserved()
     {
-        var s1 = new TestResilienceStrategy();
-        var s2 = new TestResilienceStrategy();
-        var s3 = new TestResilienceStrategy();
-
-        var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new[] { s1, s2, s3 });
-
-        s1.Next.Should().Be(s2);
-        s2.Next.Should().Be(s3);
-        s3.Next.Should().Be(NullResilienceStrategy.Instance);
-    }
-
-    [Fact]
-    public void Create_EnsureStrategiesFrozen()
-    {
-        var strategies = new[]
+        var strategies = new ResilienceStrategy[]
         {
             new TestResilienceStrategy(),
-            new TestResilienceStrategy(),
+            new Strategy(),
             new TestResilienceStrategy(),
         };
 
-        var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies);
+        var pipeline = ResilienceStrategyPipeline.CreatePipeline(strategies);
 
-        foreach (var s in strategies)
+        for (int i = 0; i < strategies.Length; i++)
         {
-            Assert.Throws<InvalidOperationException>(() => s.Next = NullResilienceStrategy.Instance);
+            pipeline.Strategies[i].Should().BeSameAs(strategies[i]);
         }
+
+        pipeline.Strategies.SequenceEqual(strategies).Should().BeTrue();
     }
 
     [Fact]
-    public void Create_EnsureOriginalStrategiesPreserved()
+    public void CreatePipeline_EnsurePipelineReusableAcrossDifferentPipelines()
     {
         var strategies = new ResilienceStrategy[]
         {
@@ -63,14 +51,13 @@ public void Create_EnsureOriginalStrategiesPreserved()
             new TestResilienceStrategy(),
         };
 
-        var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies);
+        var pipeline = ResilienceStrategyPipeline.CreatePipeline(strategies);
 
-        for (int i = 0; i < strategies.Length; i++)
-        {
-            pipeline.Strategies[i].Should().BeSameAs(strategies[i]);
-        }
+        ResilienceStrategyPipeline.CreatePipeline(new ResilienceStrategy[] { NullResilienceStrategy.Instance, pipeline });
 
-        pipeline.Strategies.SequenceEqual(strategies).Should().BeTrue();
+        this.Invoking(_ => ResilienceStrategyPipeline.CreatePipeline(new ResilienceStrategy[] { NullResilienceStrategy.Instance, pipeline }))
+            .Should()
+            .NotThrow();
     }
 
     private class Strategy : ResilienceStrategy

src/Polly.Core.Tests/Utils/TestResilienceStrategy.cs

@@ -1,6 +1,6 @@
 namespace Polly.Core.Tests.Utils;
 
-public class TestResilienceStrategy : DelegatingResilienceStrategy
+public class TestResilienceStrategy : ResilienceStrategy
 {
     public Action<ResilienceContext, object?>? Before { get; set; }
 
@@ -19,7 +19,7 @@ protected internal override async ValueTask<TResult> ExecuteCoreAsync<TResult, T
                 await OnExecute(context, state);
             }
 
-            var result = await base.ExecuteCoreAsync(callback, context, state);
+            var result = await callback(context, state);
 
             After?.Invoke(result, null);
 

src/Polly.Core/Builder/ResilienceStrategyBuilder.cs

@@ -83,7 +83,7 @@ public ResilienceStrategy Build()
 
         var strategies = _entries.Select(CreateResilienceStrategy).ToList();
 
-        return ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies);
+        return ResilienceStrategyPipeline.CreatePipeline(strategies);
     }
 
     private ResilienceStrategy CreateResilienceStrategy(Entry entry)

src/Polly.Core/Builder/ResilienceStrategyPipeline.cs

@@ -1,55 +1,50 @@
 using System;
+using static System.Net.Mime.MediaTypeNames;
 
 namespace Polly.Builder;
 
+#pragma warning disable S2302 // "nameof" should be used
+
 /// <summary>
 /// A pipeline of strategies.
 /// </summary>
-internal sealed class ResilienceStrategyPipeline : DelegatingResilienceStrategy
+internal sealed class ResilienceStrategyPipeline : ResilienceStrategy
 {
     private readonly ResilienceStrategy _pipeline;
 
-    public static ResilienceStrategyPipeline CreatePipelineAndFreezeStrategies(IReadOnlyList<ResilienceStrategy> strategies)
+    public static ResilienceStrategyPipeline CreatePipeline(IReadOnlyList<ResilienceStrategy> strategies)
     {
         Guard.NotNull(strategies);
 
         if (strategies.Count < 2)
         {
-#pragma warning disable S2302 // "nameof" should be used
             throw new InvalidOperationException("The resilience pipeline must contain at least two resilience strategies.");
-#pragma warning restore S2302 // "nameof" should be used
         }
 
         if (strategies.Distinct().Count() != strategies.Count)
         {
-#pragma warning disable S2302 // "nameof" should be used
             throw new InvalidOperationException("The resilience pipeline must contain unique resilience strategies.");
-#pragma warning restore S2302 // "nameof" should be used
         }
 
-        var delegatingStrategies = strategies.Select(strategy =>
-        {
-            if (strategy is DelegatingResilienceStrategy delegatingStrategy)
-            {
-                return delegatingStrategy;
-            }
-            else
-            {
-                return new DelegatingStrategyWrapper(strategy);
-            }
-        }).ToList();
+        // convert all strategies to delegating ones (except the last one as it's not required)
+        var delegatingStrategies = strategies
+            .Take(strategies.Count - 1)
+            .Select(strategy => new DelegatingResilienceStrategy(strategy))
+            .ToList();
 
+#if NET6_0_OR_GREATER
+        // link the last one
+        delegatingStrategies[^1].Next = strategies[^1];
+#else
+        delegatingStrategies[delegatingStrategies.Count - 1].Next = strategies[strategies.Count - 1];
+#endif
+
+        // link the remaining ones
         for (var i = 0; i < delegatingStrategies.Count - 1; i++)
         {
             delegatingStrategies[i].Next = delegatingStrategies[i + 1];
         }
 
-        // now, freeze the strategies so any further modifications are not allowed
-        foreach (var strategy in delegatingStrategies)
-        {
-            strategy.Freeze();
-        }
-
         return new ResilienceStrategyPipeline(delegatingStrategies[0], strategies);
     }
 
@@ -63,25 +58,24 @@ private ResilienceStrategyPipeline(ResilienceStrategy pipeline, IReadOnlyList<Re
 
     protected internal override ValueTask<TResult> ExecuteCoreAsync<TResult, TState>(Func<ResilienceContext, TState, ValueTask<TResult>> callback, ResilienceContext context, TState state)
     {
-        return _pipeline.ExecuteCoreAsync(
-            static (context, state) => state.Next.ExecuteCoreAsync(state.callback, context, state.state),
-            context,
-            (Next, callback, state));
+        return _pipeline.ExecuteCoreAsync(callback, context, state);
     }
 
     /// <summary>
-    /// A wrapper that converts a <see cref="ResilienceStrategy"/> into a <see cref="DelegatingResilienceStrategy"/>.
+    /// A resilience strategy that delegates the execution to the next strategy in the chain.
     /// </summary>
-    private sealed class DelegatingStrategyWrapper : DelegatingResilienceStrategy
+    private sealed class DelegatingResilienceStrategy : ResilienceStrategy
     {
         private readonly ResilienceStrategy _strategy;
 
-        public DelegatingStrategyWrapper(ResilienceStrategy strategy) => _strategy = strategy;
+        public DelegatingResilienceStrategy(ResilienceStrategy strategy) => _strategy = strategy;
+
+        public ResilienceStrategy? Next { get; set; }
 
         protected internal override ValueTask<TResult> ExecuteCoreAsync<TResult, TState>(Func<ResilienceContext, TState, ValueTask<TResult>> callback, ResilienceContext context, TState state)
         {
             return _strategy.ExecuteCoreAsync(
-                static (context, state) => state.Next.ExecuteCoreAsync(state.callback, context, state.state),
+                static (context, state) => state.Next!.ExecuteCoreAsync(state.callback, context, state.state),
                 context,
                 (Next, callback, state));
         }