From 6f3f774f0ea4e3de4db4233b5c752e4fae313976 Mon Sep 17 00:00:00 2001 From: martintmk Date: Thu, 16 Mar 2023 16:06:53 +0100 Subject: [PATCH 01/12] Introduce IResilienceStrategyBuilder --- .../Builder/ResilienceStrategyBuilderTests.cs | 202 ++++++++++++++++++ .../Builder/DelegatingStrategyWrapper.cs | 21 ++ .../Builder/IResilienceStrategyBuilder.cs | 31 +++ .../Builder/ResilienceStrategyBuilder.cs | 84 ++++++++ .../ResilienceStrategyBuilderContext.cs | 35 +++ .../ResilienceStrategyBuilderExtensions.cs | 22 ++ .../ResilienceStrategyBuilderOptions.cs | 15 ++ .../Builder/ResilienceStrategyOptions.cs | 21 ++ src/Polly.Core/Polly.Core.csproj | 1 + 9 files changed, 432 insertions(+) create mode 100644 src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs create mode 100644 src/Polly.Core/Builder/DelegatingStrategyWrapper.cs create mode 100644 src/Polly.Core/Builder/IResilienceStrategyBuilder.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilder.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilderContext.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyOptions.cs diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs new file mode 100644 index 00000000000..726ea37208d --- /dev/null +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -0,0 +1,202 @@ +using System; +using System.ComponentModel.DataAnnotations; +using FluentAssertions; +using Polly.Builder; +using Polly.Core.Tests.Utils; +using Xunit; + +namespace Polly.Core.Tests.Builder; + +public class ResilienceStrategyBuilderTests +{ + [Fact] + public void AddStrategy_Single_Ok() + { + // arrange + var executions = new List(); + var builder = new ResilienceStrategyBuilder(); + var first = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(1), + After = (_, _) => executions.Add(3), + }; + + builder.AddStrategy(first); + + // act + var strategy = builder.Build(); + + // assert + strategy.Execute(_ => executions.Add(2)); + + executions.Should().BeInAscendingOrder(); + executions.Should().HaveCount(3); + } + + [Fact] + public void AddStrategy_Multiple_Ok() + { + // arrange + var executions = new List(); + var builder = new ResilienceStrategyBuilder(); + var first = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(1), + After = (_, _) => executions.Add(7), + }; + var second = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(2), + After = (_, _) => executions.Add(6), + }; + var third = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(3), + After = (_, _) => executions.Add(5), + }; + + builder.AddStrategy(first); + builder.AddStrategy(second); + builder.AddStrategy(third); + + // act + var strategy = builder.Build(); + + // assert + strategy.Execute(_ => executions.Add(4)); + + executions.Should().BeInAscendingOrder(); + executions.Should().HaveCount(7); + } + + [Fact] + public void Build_Empty_ReturnsNullResilienceStrategy() + { + new ResilienceStrategyBuilder().Build().Should().BeSameAs(NullResilienceStrategy.Instance); + } + + [Fact] + public void Options_SetNull_Throws() + { + var builder = new ResilienceStrategyBuilder(); + + builder.Invoking(b => b.Options = null!).Should().Throw(); + } + + [Fact] + public void Build_InvalidBuilderOptions_Throw() + { + var builder = new ResilienceStrategyBuilder(); + builder.Options.BuilderName = null!; + + builder.Invoking(b => b.Build()).Should().Throw(); + } + + [Fact] + public void AddStrategy_InvalidOptions_Throws() + { + var builder = new ResilienceStrategyBuilder(); + + builder + .Invoking(b => b.AddStrategy(NullResilienceStrategy.Instance, new ResilienceStrategyOptions { StrategyName = null!, StrategyType = null! })) + .Should() + .Throw() + .WithMessage("The StrategyName field is required."); + } + + [Fact] + public void AddStrategy_NullFactory_Throws() + { + var builder = new ResilienceStrategyBuilder(); + + builder + .Invoking(b => b.AddStrategy(null!)) + .Should() + .Throw() + .And.ParamName + .Should() + .Be("factory"); + } + + [Fact] + public void AddStrategy_CombinePipelines_Ok() + { + // arrange + var executions = new List(); + var first = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(1), + After = (_, _) => executions.Add(7), + }; + var second = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(2), + After = (_, _) => executions.Add(6), + }; + + var pipeline1 = new ResilienceStrategyBuilder().AddStrategy(first).AddStrategy(second).Build(); + + var third = new TestResilienceStrategy + { + Before = (_, _) => executions.Add(3), + After = (_, _) => executions.Add(5), + }; + var pipeline2 = new ResilienceStrategyBuilder().AddStrategy(third).Build(); + + // act + var strategy = new ResilienceStrategyBuilder().AddStrategy(pipeline1).AddStrategy(pipeline2).Build(); + + // assert + strategy.Execute(_ => executions.Add(4)); + + executions.Should().BeInAscendingOrder(); + executions.Should().HaveCount(7); + } + + [Fact] + public void BuildStrategy_EnsureCorrectContext() + { + // arrange + bool verified1 = false; + bool verified2 = false; + + var builder = new ResilienceStrategyBuilder + { + Options = new ResilienceStrategyBuilderOptions + { + BuilderName = "builder-name" + } + }; + + builder.AddStrategy( + context => + { + context.BuilderName.Should().Be("builder-name"); + context.StrategyName.Should().Be("strategy-name"); + context.StrategyType.Should().Be("strategy-type"); + verified1 = true; + + return NullResilienceStrategy.Instance; + }, + new ResilienceStrategyOptions { StrategyName = "strategy-name", StrategyType = "strategy-type" }); + + builder.AddStrategy( + context => + { + context.BuilderName.Should().Be("builder-name"); + context.StrategyName.Should().Be("strategy-name-2"); + context.StrategyType.Should().Be("strategy-type-2"); + verified2 = true; + + return NullResilienceStrategy.Instance; + }, + new ResilienceStrategyOptions { StrategyName = "strategy-name-2", StrategyType = "strategy-type-2" }); + + // act + builder.Build(); + + // assert + verified1.Should().BeTrue(); + verified2.Should().BeTrue(); + } +} diff --git a/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs b/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs new file mode 100644 index 00000000000..c5c1d8981bc --- /dev/null +++ b/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs @@ -0,0 +1,21 @@ +using System; + +namespace Polly.Builder; + +/// +/// A wrapper that converts a into a . +/// +internal sealed class DelegatingStrategyWrapper : DelegatingResilienceStrategy +{ + private readonly IResilienceStrategy _strategy; + + public DelegatingStrategyWrapper(IResilienceStrategy strategy) => _strategy = strategy; + + protected override ValueTask ExecuteCoreAsync(Func> callback, ResilienceContext context, TState state) + { + return _strategy.ExecuteAsync( + static (context, state) => state.Next.ExecuteAsync(state.callback, context, state.state), + context, + (Next, callback, state)); + } +} diff --git a/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs new file mode 100644 index 00000000000..04f64a68ad4 --- /dev/null +++ b/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs @@ -0,0 +1,31 @@ +namespace Polly.Builder; + +/// +/// A builder that is used to create an instance of . +/// +/// +/// The builder supports chaining multiple strategies into a pipeline of strategies. +/// The resulting instance of created by the call will execute the strategies in the same order they were added to the builder. +/// The order of the strategies is important. +/// +public interface IResilienceStrategyBuilder +{ + /// + /// Gets or sets the builder options. + /// + ResilienceStrategyBuilderOptions Options { get; set; } + + /// + /// Adds a strategy to the builder. + /// + /// The factory that creates a resilience strategy. + /// The options associated with the strategy. If none are provided the default instance of is created. + /// The same builder instance. + IResilienceStrategyBuilder AddStrategy(Func factory, ResilienceStrategyOptions? options = null); + + /// + /// Builds the resilience strategy. + /// + /// An instance of . + IResilienceStrategy Build(); +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs new file mode 100644 index 00000000000..33512a79358 --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -0,0 +1,84 @@ +using System.ComponentModel.DataAnnotations; + +namespace Polly.Builder; + +/// +public class ResilienceStrategyBuilder : IResilienceStrategyBuilder +{ + private readonly List _entries = new(); + private ResilienceStrategyBuilderOptions _options = new(); + + /// + public ResilienceStrategyBuilderOptions Options + { + get => _options; + set => _options = Guard.NotNull(value); + } + + /// + public IResilienceStrategyBuilder AddStrategy(Func factory, ResilienceStrategyOptions? options = null) + { + Guard.NotNull(factory); + + if (options is not null) + { + Validator.ValidateObject(options, new ValidationContext(options), validateAllProperties: true); + } + + _entries.Add(new Entry(factory, options ?? new ResilienceStrategyOptions())); + + return this; + } + + /// + public IResilienceStrategy Build() + { + Validator.ValidateObject(Options, new ValidationContext(Options), validateAllProperties: true); + + if (_entries.Count == 0) + { + return NullResilienceStrategy.Instance; + } + + var strategies = new List(_entries.Count); + + foreach (var entry in _entries) + { + var context = new ResilienceStrategyBuilderContext( + builderName: Options.BuilderName, + strategyName: entry.Properties.StrategyName, + strategyType: entry.Properties.StrategyType); + + var strategy = entry.Factory(context); + + if (strategy is DelegatingResilienceStrategy delegatingStrategy) + { + strategies.Add(delegatingStrategy); + } + else + { + strategies.Add(new DelegatingStrategyWrapper(strategy)); + } + } + + for (var i = 0; i < strategies.Count - 1; i++) + { + strategies[i].Next = strategies[i + 1]; + } + + return new DelegatingStrategyWrapper(strategies[0]); + } + + private sealed class Entry + { + public Entry(Func factory, ResilienceStrategyOptions properties) + { + Factory = factory; + Properties = properties; + } + + public Func Factory { get; } + + public ResilienceStrategyOptions Properties { get; } + } +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderContext.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderContext.cs new file mode 100644 index 00000000000..ed9bcc18c6f --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilderContext.cs @@ -0,0 +1,35 @@ +namespace Polly.Builder; + +/// +/// The context used for building an individual resilience strategy. +/// +public class ResilienceStrategyBuilderContext +{ + /// + /// Initializes a new instance of the class. + /// + /// The name of the builder. + /// The strategy name. + /// The strategy type. + public ResilienceStrategyBuilderContext(string builderName, string strategyName, string strategyType) + { + BuilderName = Guard.NotNull(builderName); + StrategyName = Guard.NotNull(strategyName); + StrategyType = Guard.NotNull(strategyType); + } + + /// + /// Gets the name of the builder. + /// + public string BuilderName { get; } + + /// + /// Gets the name of the strategy. + /// + public string StrategyName { get; } + + /// + /// Gets the type of the strategy. + /// + public string StrategyType { get; } +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs new file mode 100644 index 00000000000..93f1e0541a8 --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs @@ -0,0 +1,22 @@ +namespace Polly.Builder; + +/// +/// The extensions for the . +/// +public static class ResilienceStrategyBuilderExtensions +{ + /// + /// Adds an already created strategy instance to the builder. + /// + /// The builder instance. + /// The strategy instance. + /// The options associated with the strategy. If none are provided the default instance of is created. + /// The same builder instance. + public static IResilienceStrategyBuilder AddStrategy(this IResilienceStrategyBuilder builder, IResilienceStrategy strategy, ResilienceStrategyOptions? options = null) + { + Guard.NotNull(builder); + Guard.NotNull(strategy); + + return builder.AddStrategy(_ => strategy, options); + } +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs new file mode 100644 index 00000000000..8755ea47023 --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs @@ -0,0 +1,15 @@ +using System.ComponentModel.DataAnnotations; + +namespace Polly.Builder; + +/// +/// The builder options used by . +/// +public class ResilienceStrategyBuilderOptions +{ + /// + /// Gets or sets the name of the builder. + /// + [Required(AllowEmptyStrings = true)] + public string BuilderName { get; set; } = string.Empty; +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyOptions.cs new file mode 100644 index 00000000000..353135252d6 --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyOptions.cs @@ -0,0 +1,21 @@ +using System.ComponentModel.DataAnnotations; + +namespace Polly.Builder; + +/// +/// The options associated with the . +/// +public class ResilienceStrategyOptions +{ + /// + /// Gets or sets the name of the strategy. + /// + [Required(AllowEmptyStrings = true)] + public string StrategyName { get; set; } = string.Empty; + + /// + /// Gets or sets the type of the strategy. + /// + [Required(AllowEmptyStrings = true)] + public string StrategyType { get; set; } = string.Empty; +} diff --git a/src/Polly.Core/Polly.Core.csproj b/src/Polly.Core/Polly.Core.csproj index 7dce169e674..8abfbb4006f 100644 --- a/src/Polly.Core/Polly.Core.csproj +++ b/src/Polly.Core/Polly.Core.csproj @@ -21,6 +21,7 @@ + From ecde5dd58da73eb1d2ec59199d12fb68e796c635 Mon Sep 17 00:00:00 2001 From: martintmk Date: Thu, 16 Mar 2023 19:03:11 +0100 Subject: [PATCH 02/12] Cleanup --- .../Builder/IResilienceStrategyBuilder.cs | 31 -------------- .../Builder/ResilienceStrategyBuilder.cs | 42 ++++++++++++++++--- .../ResilienceStrategyBuilderExtensions.cs | 22 ---------- .../ResilienceStrategyBuilderOptions.cs | 2 +- 4 files changed, 37 insertions(+), 60 deletions(-) delete mode 100644 src/Polly.Core/Builder/IResilienceStrategyBuilder.cs delete mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs diff --git a/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs deleted file mode 100644 index 04f64a68ad4..00000000000 --- a/src/Polly.Core/Builder/IResilienceStrategyBuilder.cs +++ /dev/null @@ -1,31 +0,0 @@ -namespace Polly.Builder; - -/// -/// A builder that is used to create an instance of . -/// -/// -/// The builder supports chaining multiple strategies into a pipeline of strategies. -/// The resulting instance of created by the call will execute the strategies in the same order they were added to the builder. -/// The order of the strategies is important. -/// -public interface IResilienceStrategyBuilder -{ - /// - /// Gets or sets the builder options. - /// - ResilienceStrategyBuilderOptions Options { get; set; } - - /// - /// Adds a strategy to the builder. - /// - /// The factory that creates a resilience strategy. - /// The options associated with the strategy. If none are provided the default instance of is created. - /// The same builder instance. - IResilienceStrategyBuilder AddStrategy(Func factory, ResilienceStrategyOptions? options = null); - - /// - /// Builds the resilience strategy. - /// - /// An instance of . - IResilienceStrategy Build(); -} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 33512a79358..9e139a4f44f 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -2,21 +2,48 @@ namespace Polly.Builder; -/// -public class ResilienceStrategyBuilder : IResilienceStrategyBuilder +/// +/// A builder that is used to create an instance of . +/// +/// +/// The builder supports chaining multiple strategies into a pipeline of strategies. +/// The resulting instance of created by the call will execute the strategies in the same order they were added to the builder. +/// The order of the strategies is important. +/// +public class ResilienceStrategyBuilder { private readonly List _entries = new(); private ResilienceStrategyBuilderOptions _options = new(); - /// + /// + /// Gets or sets the builder options. + /// public ResilienceStrategyBuilderOptions Options { get => _options; set => _options = Guard.NotNull(value); } - /// - public IResilienceStrategyBuilder AddStrategy(Func factory, ResilienceStrategyOptions? options = null) + /// + /// Adds an already created strategy instance to the builder. + /// + /// The strategy instance. + /// The options associated with the strategy. If none are provided the default instance of is created. + /// The same builder instance. + public ResilienceStrategyBuilder AddStrategy(IResilienceStrategy strategy, ResilienceStrategyOptions? options = null) + { + Guard.NotNull(strategy); + + return AddStrategy(_ => strategy, options); + } + + /// + /// Adds a strategy to the builder. + /// + /// The factory that creates a resilience strategy. + /// The options associated with the strategy. If none are provided the default instance of is created. + /// The same builder instance. + public ResilienceStrategyBuilder AddStrategy(Func factory, ResilienceStrategyOptions? options = null) { Guard.NotNull(factory); @@ -30,7 +57,10 @@ public IResilienceStrategyBuilder AddStrategy(Func + /// + /// Builds the resilience strategy. + /// + /// An instance of . public IResilienceStrategy Build() { Validator.ValidateObject(Options, new ValidationContext(Options), validateAllProperties: true); diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs deleted file mode 100644 index 93f1e0541a8..00000000000 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilderExtensions.cs +++ /dev/null @@ -1,22 +0,0 @@ -namespace Polly.Builder; - -/// -/// The extensions for the . -/// -public static class ResilienceStrategyBuilderExtensions -{ - /// - /// Adds an already created strategy instance to the builder. - /// - /// The builder instance. - /// The strategy instance. - /// The options associated with the strategy. If none are provided the default instance of is created. - /// The same builder instance. - public static IResilienceStrategyBuilder AddStrategy(this IResilienceStrategyBuilder builder, IResilienceStrategy strategy, ResilienceStrategyOptions? options = null) - { - Guard.NotNull(builder); - Guard.NotNull(strategy); - - return builder.AddStrategy(_ => strategy, options); - } -} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs index 8755ea47023..eef4013dd28 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs @@ -3,7 +3,7 @@ namespace Polly.Builder; /// -/// The builder options used by . +/// The builder options used by . /// public class ResilienceStrategyBuilderOptions { From 56fdb7f0e5e5c53c6e6258fd0386dbf463b330f8 Mon Sep 17 00:00:00 2001 From: martintmk Date: Thu, 16 Mar 2023 21:17:12 +0100 Subject: [PATCH 03/12] tests and mutations --- .../ResilienceStrategyBuilderOptionsTests.cs | 15 +++++++++++ .../Builder/ResilienceStrategyBuilderTests.cs | 22 ++++++++++++--- .../Builder/ResilienceStrategyOptionsTests.cs | 16 +++++++++++ .../Builder/ResilienceStrategyBuilder.cs | 6 ++--- src/Polly.Core/Utils/ValidationHelper.cs | 27 +++++++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs create mode 100644 src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs create mode 100644 src/Polly.Core/Utils/ValidationHelper.cs diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs new file mode 100644 index 00000000000..93636c3f386 --- /dev/null +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs @@ -0,0 +1,15 @@ +using FluentAssertions; +using Polly.Builder; +using Xunit; + +namespace Polly.Core.Tests.Builder; +public class ResilienceStrategyBuilderOptionsTests +{ + [Fact] + public void Ctor_EnsureDefaults() + { + var options = new ResilienceStrategyBuilderOptions(); + + options.BuilderName.Should().Be(""); + } +} diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index 726ea37208d..4d60b4e05bb 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -89,7 +89,16 @@ public void Build_InvalidBuilderOptions_Throw() var builder = new ResilienceStrategyBuilder(); builder.Options.BuilderName = null!; - builder.Invoking(b => b.Build()).Should().Throw(); + builder.Invoking(b => b.Build()) + .Should() + .Throw() + .WithMessage( +""" +The 'ResilienceStrategyBuilderOptions' options are not valid. + +Validation Errors: +The BuilderName field is required. +"""); } [Fact] @@ -101,7 +110,14 @@ public void AddStrategy_InvalidOptions_Throws() .Invoking(b => b.AddStrategy(NullResilienceStrategy.Instance, new ResilienceStrategyOptions { StrategyName = null!, StrategyType = null! })) .Should() .Throw() - .WithMessage("The StrategyName field is required."); + .WithMessage( +""" +The 'ResilienceStrategyOptions' options are not valid. + +Validation Errors: +The StrategyName field is required. +The StrategyType field is required. +"""); } [Fact] @@ -110,7 +126,7 @@ public void AddStrategy_NullFactory_Throws() var builder = new ResilienceStrategyBuilder(); builder - .Invoking(b => b.AddStrategy(null!)) + .Invoking(b => b.AddStrategy((Func)null!)) .Should() .Throw() .And.ParamName diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs new file mode 100644 index 00000000000..1149db233c3 --- /dev/null +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs @@ -0,0 +1,16 @@ +using FluentAssertions; +using Polly.Builder; +using Xunit; + +namespace Polly.Core.Tests.Builder; +public class ResilienceStrategyOptionsTests +{ + [Fact] + public void Ctor_EnsureDefaults() + { + var options = new ResilienceStrategyOptions(); + + options.StrategyType.Should().Be(""); + options.StrategyName.Should().Be(""); + } +} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 9e139a4f44f..2ebf9a70dd3 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -1,5 +1,3 @@ -using System.ComponentModel.DataAnnotations; - namespace Polly.Builder; /// @@ -49,7 +47,7 @@ public ResilienceStrategyBuilder AddStrategy(FuncAn instance of . public IResilienceStrategy Build() { - Validator.ValidateObject(Options, new ValidationContext(Options), validateAllProperties: true); + ValidationHelper.ValidateObject(Options, $"The '{nameof(ResilienceStrategyBuilderOptions)}' options are not valid."); if (_entries.Count == 0) { diff --git a/src/Polly.Core/Utils/ValidationHelper.cs b/src/Polly.Core/Utils/ValidationHelper.cs new file mode 100644 index 00000000000..9556e9e6421 --- /dev/null +++ b/src/Polly.Core/Utils/ValidationHelper.cs @@ -0,0 +1,27 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Text; + +namespace Polly.Utils; + +internal static class ValidationHelper +{ + public static void ValidateObject(object instance, string mainMessage) + { + var errors = new List(); + + if (!Validator.TryValidateObject(instance, new ValidationContext(instance), errors)) + { + StringBuilder stringBuilder = new StringBuilder(mainMessage); + stringBuilder.AppendLine(); + + stringBuilder.AppendLine("Validation Errors:"); + foreach (var error in errors) + { + stringBuilder.AppendLine(error.ErrorMessage); + } + + throw new ValidationException(stringBuilder.ToString()); + } + } +} From b02fa956e13064b5e2aa93f9510b225b42b1c2f3 Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 08:06:53 +0100 Subject: [PATCH 04/12] Cleanup --- .../Builder/ResilienceStrategyBuilderTests.cs | 63 ++++++++++++++++++- .../ResilienceStrategyPipelineTests.cs | 50 +++++++++++++++ .../DelegatingResilienceStrategyTests.cs | 2 +- .../Builder/ResilienceStrategyBuilder.cs | 6 +- .../Builder/ResilienceStrategyPipeline.cs | 50 +++++++++++++++ .../DelegatingResilienceStrategy.cs | 10 +-- 6 files changed, 172 insertions(+), 9 deletions(-) create mode 100644 src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs create mode 100644 src/Polly.Core/Builder/ResilienceStrategyPipeline.cs diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index 4d60b4e05bb..d09bff39944 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -28,7 +28,7 @@ public void AddStrategy_Single_Ok() // assert strategy.Execute(_ => executions.Add(2)); - + strategy.Should().BeOfType(); executions.Should().BeInAscendingOrder(); executions.Should().HaveCount(3); } @@ -59,6 +59,47 @@ public void AddStrategy_Multiple_Ok() builder.AddStrategy(second); builder.AddStrategy(third); + // act + var strategy = builder.Build(); + strategy + .Should() + .BeOfType() + .Subject + .Strategies.Should().HaveCount(3); + + // assert + strategy.Execute(_ => executions.Add(4)); + + executions.Should().BeInAscendingOrder(); + executions.Should().HaveCount(7); + } + + [Fact] + public void AddStrategy_MultipleNonDelegating_Ok() + { + // arrange + var executions = new List(); + var builder = new ResilienceStrategyBuilder(); + var first = new Strategy + { + Before = () => executions.Add(1), + After = () => executions.Add(7), + }; + var second = new Strategy + { + Before = () => executions.Add(2), + After = () => executions.Add(6), + }; + var third = new Strategy + { + Before = () => executions.Add(3), + After = () => executions.Add(5), + }; + + builder.AddStrategy(first); + builder.AddStrategy(second); + builder.AddStrategy(third); + // act var strategy = builder.Build(); @@ -215,4 +256,24 @@ public void BuildStrategy_EnsureCorrectContext() verified1.Should().BeTrue(); verified2.Should().BeTrue(); } + + private class Strategy : IResilienceStrategy + { + public Action? Before { get; set; } + + public Action? After { get; set; } + + async ValueTask IResilienceStrategy.ExecuteInternalAsync(Func> callback, ResilienceContext context, TState state) + { + try + { + Before?.Invoke(); + return await callback(context, state); + } + finally + { + After?.Invoke(); + } + } + } } diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs new file mode 100644 index 00000000000..49762f19e57 --- /dev/null +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs @@ -0,0 +1,50 @@ +using System; +using FluentAssertions; +using Polly.Builder; +using Polly.Core.Tests.Utils; +using Xunit; + +namespace Polly.Core.Tests.Builder; + +public class ResilienceStrategyPipelineTests +{ + [Fact] + public void CreateAndFreezeStrategies_ArgValidation() + { + Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(null!)); + Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(Array.Empty())); + Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(new DelegatingResilienceStrategy[] { new TestResilienceStrategy() })); + } + + [Fact] + public void CreateAndFreezeStrategies_EnsureStrategiesLinked() + { + var s1 = new TestResilienceStrategy(); + var s2 = new TestResilienceStrategy(); + var s3 = new TestResilienceStrategy(); + + var pipeline = ResilienceStrategyPipeline.CreateAndFreezeStrategies(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[] + { + new TestResilienceStrategy(), + new TestResilienceStrategy(), + new TestResilienceStrategy(), + }; + + var pipeline = ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); + + foreach (var s in strategies) + { + Assert.Throws(() => s.Next = NullResilienceStrategy.Instance); + } + } +} diff --git a/src/Polly.Core.Tests/DelegatingResilienceStrategyTests.cs b/src/Polly.Core.Tests/DelegatingResilienceStrategyTests.cs index 84d7cce8116..58ffc9ec52d 100644 --- a/src/Polly.Core.Tests/DelegatingResilienceStrategyTests.cs +++ b/src/Polly.Core.Tests/DelegatingResilienceStrategyTests.cs @@ -37,7 +37,7 @@ public void Next_ChangeAfterExecuted_Throws() .Invoking(s => s.Next = NullResilienceStrategy.Instance) .Should() .Throw() - .WithMessage("The delegating resilience strategy has already been executed and changing the value of 'Next' property is not allowed."); + .WithMessage("The delegating resilience strategy is already frozen and changing the value of 'Next' property is not allowed."); } [Fact] diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 2ebf9a70dd3..5a425e7f242 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -89,12 +89,12 @@ public IResilienceStrategy Build() } } - for (var i = 0; i < strategies.Count - 1; i++) + if (strategies.Count == 1) { - strategies[i].Next = strategies[i + 1]; + return strategies[0]; } - return new DelegatingStrategyWrapper(strategies[0]); + return ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); } private sealed class Entry diff --git a/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs new file mode 100644 index 00000000000..1c801ee9de7 --- /dev/null +++ b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs @@ -0,0 +1,50 @@ +using System; + +namespace Polly.Builder; + +/// +/// A pipeline of strategies. +/// +internal sealed class ResilienceStrategyPipeline : DelegatingResilienceStrategy +{ + private readonly IResilienceStrategy _strategy; + + public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList strategies) + { + Guard.NotNull(strategies); + + if (strategies.Count < 2) + { + throw new ArgumentException("The pipeline must contain at least two strategies.", nameof(strategies)); + } + + for (var i = 0; i < strategies.Count - 1; i++) + { + strategies[i].Next = strategies[i + 1]; + } + + // now, freeze the strategies so any further modifications are not allowed + foreach (var strategy in strategies) + { + strategy.Freeze(); + } + + return new ResilienceStrategyPipeline(strategies); + } + + private ResilienceStrategyPipeline(IReadOnlyList strategies) + { + Strategies = strategies; + _strategy = strategies[0]; + } + + public IReadOnlyList Strategies { get; } + + protected override ValueTask ExecuteCoreAsync(Func> callback, ResilienceContext context, TState state) + { + return _strategy.ExecuteAsync( + static (context, state) => state.Next.ExecuteAsync(state.callback, context, state.state), + context, + (Next, callback, state)); + } +} diff --git a/src/Polly.Core/DelegatingResilienceStrategy.cs b/src/Polly.Core/DelegatingResilienceStrategy.cs index f1f8cdfd2bf..16d743ef9e6 100644 --- a/src/Polly.Core/DelegatingResilienceStrategy.cs +++ b/src/Polly.Core/DelegatingResilienceStrategy.cs @@ -5,7 +5,7 @@ namespace Polly; /// public class DelegatingResilienceStrategy : IResilienceStrategy { - private bool _executed; + private bool _frozen; private IResilienceStrategy _next = NullResilienceStrategy.Instance; /// @@ -26,9 +26,9 @@ public IResilienceStrategy Next { Guard.NotNull(value); - if (_executed) + if (_frozen) { - throw new InvalidOperationException($"The delegating resilience strategy has already been executed and changing the value of '{nameof(Next)}' property is not allowed."); + throw new InvalidOperationException($"The delegating resilience strategy is already frozen and changing the value of '{nameof(Next)}' property is not allowed."); } _next = value; @@ -64,8 +64,10 @@ protected virtual ValueTask ExecuteCoreAsync( ResilienceContext context, TState state) { - _executed = true; + _frozen = true; return Next.ExecuteInternalAsync(callback, context, state); } + + internal void Freeze() => _frozen = true; } From 5cf9ab4c5ef584487b74fa7b29e61bfb8814b50f Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 08:12:14 +0100 Subject: [PATCH 05/12] Cleanup --- .../Builder/ResilienceStrategyBuilder.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 5a425e7f242..4324919b843 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -68,16 +68,16 @@ public IResilienceStrategy Build() return NullResilienceStrategy.Instance; } + if (_entries.Count == 1) + { + return CreateResilienceStrategy(_entries[0]); + } + var strategies = new List(_entries.Count); foreach (var entry in _entries) { - var context = new ResilienceStrategyBuilderContext( - builderName: Options.BuilderName, - strategyName: entry.Properties.StrategyName, - strategyType: entry.Properties.StrategyType); - - var strategy = entry.Factory(context); + var strategy = CreateResilienceStrategy(entry); if (strategy is DelegatingResilienceStrategy delegatingStrategy) { @@ -89,14 +89,19 @@ public IResilienceStrategy Build() } } - if (strategies.Count == 1) - { - return strategies[0]; - } - return ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); } + private IResilienceStrategy CreateResilienceStrategy(Entry entry) + { + var context = new ResilienceStrategyBuilderContext( + builderName: Options.BuilderName, + strategyName: entry.Properties.StrategyName, + strategyType: entry.Properties.StrategyType); + + return entry.Factory(context); + } + private sealed class Entry { public Entry(Func factory, ResilienceStrategyOptions properties) From 2e21e23cd8c2b9ced2c7a600beba695d8d70299d Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 08:28:17 +0100 Subject: [PATCH 06/12] Cleanup --- .../Builder/ResilienceStrategyBuilder.cs | 16 +------------- .../Builder/ResilienceStrategyPipeline.cs | 22 ++++++++++++++----- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 4324919b843..600f770e72b 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -73,21 +73,7 @@ public IResilienceStrategy Build() return CreateResilienceStrategy(_entries[0]); } - var strategies = new List(_entries.Count); - - foreach (var entry in _entries) - { - var strategy = CreateResilienceStrategy(entry); - - if (strategy is DelegatingResilienceStrategy delegatingStrategy) - { - strategies.Add(delegatingStrategy); - } - else - { - strategies.Add(new DelegatingStrategyWrapper(strategy)); - } - } + var strategies = _entries.Select(CreateResilienceStrategy).ToList(); return ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); } diff --git a/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs index 1c801ee9de7..0cc6686ef99 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs @@ -9,7 +9,7 @@ internal sealed class ResilienceStrategyPipeline : DelegatingResilienceStrategy { private readonly IResilienceStrategy _strategy; - public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList strategies) + public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList strategies) { Guard.NotNull(strategies); @@ -18,18 +18,30 @@ public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList throw new ArgumentException("The pipeline must contain at least two strategies.", nameof(strategies)); } - for (var i = 0; i < strategies.Count - 1; i++) + var delegatingStrategies = strategies.Select(strategy => { - strategies[i].Next = strategies[i + 1]; + if (strategy is DelegatingResilienceStrategy delegatingStrategy) + { + return delegatingStrategy; + } + else + { + return new DelegatingStrategyWrapper(strategy); + } + }).ToList(); + + 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 strategies) + foreach (var strategy in delegatingStrategies) { strategy.Freeze(); } - return new ResilienceStrategyPipeline(strategies); + return new ResilienceStrategyPipeline(delegatingStrategies); } private ResilienceStrategyPipeline(IReadOnlyList strategies) From 687a5bcde650896b04c0a20ac60a7cac0fa6f297 Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 08:47:08 +0100 Subject: [PATCH 07/12] Cleanup --- .../Builder/ResilienceStrategyBuilderTests.cs | 19 +++++++- .../ResilienceStrategyPipelineTests.cs | 43 ++++++++++++++++--- .../Builder/DelegatingStrategyWrapper.cs | 21 --------- .../Builder/ResilienceStrategyBuilder.cs | 2 +- .../Builder/ResilienceStrategyPipeline.cs | 41 +++++++++++++++--- 5 files changed, 90 insertions(+), 36 deletions(-) delete mode 100644 src/Polly.Core/Builder/DelegatingStrategyWrapper.cs diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index d09bff39944..d63331b7536 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -74,6 +74,21 @@ public void AddStrategy_Multiple_Ok() executions.Should().HaveCount(7); } + [Fact] + public void AddStrategy_Duplicate_Throws() + { + // arrange + var executions = new List(); + var builder = new ResilienceStrategyBuilder() + .AddStrategy(NullResilienceStrategy.Instance) + .AddStrategy(NullResilienceStrategy.Instance); + + builder.Invoking(b => b.Build()) + .Should() + .Throw() + .WithMessage("The resilience pipeline must contain unique resilience strategies."); + } + [Fact] public void AddStrategy_MultipleNonDelegating_Ok() { @@ -233,7 +248,7 @@ public void BuildStrategy_EnsureCorrectContext() context.StrategyType.Should().Be("strategy-type"); verified1 = true; - return NullResilienceStrategy.Instance; + return new TestResilienceStrategy(); }, new ResilienceStrategyOptions { StrategyName = "strategy-name", StrategyType = "strategy-type" }); @@ -245,7 +260,7 @@ public void BuildStrategy_EnsureCorrectContext() context.StrategyType.Should().Be("strategy-type-2"); verified2 = true; - return NullResilienceStrategy.Instance; + return new TestResilienceStrategy(); }, new ResilienceStrategyOptions { StrategyName = "strategy-name-2", StrategyType = "strategy-type-2" }); diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs index 49762f19e57..31086d86f5a 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyPipelineTests.cs @@ -11,9 +11,14 @@ public class ResilienceStrategyPipelineTests [Fact] public void CreateAndFreezeStrategies_ArgValidation() { - Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(null!)); - Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(Array.Empty())); - Assert.Throws(() => ResilienceStrategyPipeline.CreateAndFreezeStrategies(new DelegatingResilienceStrategy[] { new TestResilienceStrategy() })); + Assert.Throws(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(null!)); + Assert.Throws(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(Array.Empty())); + Assert.Throws(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new IResilienceStrategy[] { new TestResilienceStrategy() })); + Assert.Throws(() => ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new IResilienceStrategy[] + { + NullResilienceStrategy.Instance, + NullResilienceStrategy.Instance + })); } [Fact] @@ -23,7 +28,7 @@ public void CreateAndFreezeStrategies_EnsureStrategiesLinked() var s2 = new TestResilienceStrategy(); var s3 = new TestResilienceStrategy(); - var pipeline = ResilienceStrategyPipeline.CreateAndFreezeStrategies(new[] { s1, s2, s3 }); + var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(new[] { s1, s2, s3 }); s1.Next.Should().Be(s2); s2.Next.Should().Be(s3); @@ -40,11 +45,39 @@ public void Create_EnsureStrategiesFrozen() new TestResilienceStrategy(), }; - var pipeline = ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); + var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies); foreach (var s in strategies) { Assert.Throws(() => s.Next = NullResilienceStrategy.Instance); } } + + [Fact] + public void Create_EnsureOriginalStrategiesPreserved() + { + var strategies = new IResilienceStrategy[] + { + new TestResilienceStrategy(), + new Strategy(), + new TestResilienceStrategy(), + }; + + var pipeline = ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies); + + for (int i = 0; i < strategies.Length; i++) + { + pipeline.Strategies[i].Should().BeSameAs(strategies[i]); + } + + pipeline.Strategies.SequenceEqual(strategies).Should().BeTrue(); + } + + private class Strategy : IResilienceStrategy + { + ValueTask IResilienceStrategy.ExecuteInternalAsync(Func> callback, ResilienceContext context, TState state) + { + throw new NotSupportedException(); + } + } } diff --git a/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs b/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs deleted file mode 100644 index c5c1d8981bc..00000000000 --- a/src/Polly.Core/Builder/DelegatingStrategyWrapper.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System; - -namespace Polly.Builder; - -/// -/// A wrapper that converts a into a . -/// -internal sealed class DelegatingStrategyWrapper : DelegatingResilienceStrategy -{ - private readonly IResilienceStrategy _strategy; - - public DelegatingStrategyWrapper(IResilienceStrategy strategy) => _strategy = strategy; - - protected override ValueTask ExecuteCoreAsync(Func> callback, ResilienceContext context, TState state) - { - return _strategy.ExecuteAsync( - static (context, state) => state.Next.ExecuteAsync(state.callback, context, state.state), - context, - (Next, callback, state)); - } -} diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 600f770e72b..f9e26287cfd 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -75,7 +75,7 @@ public IResilienceStrategy Build() var strategies = _entries.Select(CreateResilienceStrategy).ToList(); - return ResilienceStrategyPipeline.CreateAndFreezeStrategies(strategies); + return ResilienceStrategyPipeline.CreatePipelineAndFreezeStrategies(strategies); } private IResilienceStrategy CreateResilienceStrategy(Entry entry) diff --git a/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs index 0cc6686ef99..439857c52cf 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyPipeline.cs @@ -7,15 +7,24 @@ namespace Polly.Builder; /// internal sealed class ResilienceStrategyPipeline : DelegatingResilienceStrategy { - private readonly IResilienceStrategy _strategy; + private readonly IResilienceStrategy _pipeline; - public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList strategies) + public static ResilienceStrategyPipeline CreatePipelineAndFreezeStrategies(IReadOnlyList strategies) { Guard.NotNull(strategies); if (strategies.Count < 2) { - throw new ArgumentException("The pipeline must contain at least two strategies.", nameof(strategies)); +#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 => @@ -41,22 +50,40 @@ public static ResilienceStrategyPipeline CreateAndFreezeStrategies(IReadOnlyList strategy.Freeze(); } - return new ResilienceStrategyPipeline(delegatingStrategies); + return new ResilienceStrategyPipeline(delegatingStrategies[0], strategies); } - private ResilienceStrategyPipeline(IReadOnlyList strategies) + private ResilienceStrategyPipeline(IResilienceStrategy pipeline, IReadOnlyList strategies) { Strategies = strategies; - _strategy = strategies[0]; + _pipeline = pipeline; } public IReadOnlyList Strategies { get; } protected override ValueTask ExecuteCoreAsync(Func> callback, ResilienceContext context, TState state) { - return _strategy.ExecuteAsync( + return _pipeline.ExecuteAsync( static (context, state) => state.Next.ExecuteAsync(state.callback, context, state.state), context, (Next, callback, state)); } + + /// + /// A wrapper that converts a into a . + /// + private sealed class DelegatingStrategyWrapper : DelegatingResilienceStrategy + { + private readonly IResilienceStrategy _strategy; + + public DelegatingStrategyWrapper(IResilienceStrategy strategy) => _strategy = strategy; + + protected override ValueTask ExecuteCoreAsync(Func> callback, ResilienceContext context, TState state) + { + return _strategy.ExecuteAsync( + static (context, state) => state.Next.ExecuteAsync(state.callback, context, state.state), + context, + (Next, callback, state)); + } + } } From 81e47805c3c073c3d4e8d159e64ee573d3733f1c Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 09:49:41 +0100 Subject: [PATCH 08/12] PR comments --- src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs | 1 + src/Polly.Core/Builder/ResilienceStrategyOptions.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs index eef4013dd28..654e4d5ba37 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs @@ -10,6 +10,7 @@ public class ResilienceStrategyBuilderOptions /// /// Gets or sets the name of the builder. /// + /// This property is also included in the telemetry that is produced by the individual resilience strategies. [Required(AllowEmptyStrings = true)] public string BuilderName { get; set; } = string.Empty; } diff --git a/src/Polly.Core/Builder/ResilienceStrategyOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyOptions.cs index 353135252d6..81eceba15c7 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyOptions.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyOptions.cs @@ -10,12 +10,14 @@ public class ResilienceStrategyOptions /// /// Gets or sets the name of the strategy. /// + /// This property is also included in the telemetry that is produced by the individual resilience strategies. [Required(AllowEmptyStrings = true)] public string StrategyName { get; set; } = string.Empty; /// /// Gets or sets the type of the strategy. /// + /// This property is also included in the telemetry that is produced by the individual resilience strategies. [Required(AllowEmptyStrings = true)] public string StrategyType { get; set; } = string.Empty; } From 3a8d2e1e0fb78f0a50830b417f073c5749c8d91e Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 10:01:11 +0100 Subject: [PATCH 09/12] Add protection against modifications --- .../Builder/ResilienceStrategyBuilderTests.cs | 12 ++++++++++++ src/Polly.Core/Builder/ResilienceStrategyBuilder.cs | 10 +++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index d63331b7536..db2c01fd821 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -131,6 +131,18 @@ public void Build_Empty_ReturnsNullResilienceStrategy() new ResilienceStrategyBuilder().Build().Should().BeSameAs(NullResilienceStrategy.Instance); } + [Fact] + public void AddStrategy_AfterUsed_Throws() + { + var builder = new ResilienceStrategyBuilder(); + + builder + .Invoking(b => b.AddStrategy(NullResilienceStrategy.Instance)) + .Should() + .Throw() + .WithMessage("Unable to add any more resilience strategies to the builder after it has been used to build a strategy."); + } + [Fact] public void Options_SetNull_Throws() { diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index f9e26287cfd..1af83e8df09 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -12,7 +12,8 @@ public class ResilienceStrategyBuilder { private readonly List _entries = new(); private ResilienceStrategyBuilderOptions _options = new(); - + private bool _used; + /// /// Gets or sets the builder options. /// @@ -50,6 +51,11 @@ public ResilienceStrategyBuilder AddStrategy(Func Date: Fri, 17 Mar 2023 10:01:42 +0100 Subject: [PATCH 10/12] fixes --- src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index db2c01fd821..51220908308 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -136,6 +136,8 @@ public void AddStrategy_AfterUsed_Throws() { var builder = new ResilienceStrategyBuilder(); + builder.Build(); + builder .Invoking(b => b.AddStrategy(NullResilienceStrategy.Instance)) .Should() From 89cad1761a16c7105cd05a258cf3027cb8b26b22 Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 10:09:49 +0100 Subject: [PATCH 11/12] Cleanup --- src/Polly.Core/Builder/ResilienceStrategyBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index 1af83e8df09..ff0073ad15f 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -13,7 +13,7 @@ public class ResilienceStrategyBuilder private readonly List _entries = new(); private ResilienceStrategyBuilderOptions _options = new(); private bool _used; - + /// /// Gets or sets the builder options. /// From 14f907056084f6fcb72d79e2712b1fec1aaa63a9 Mon Sep 17 00:00:00 2001 From: martintmk Date: Fri, 17 Mar 2023 16:11:37 +0100 Subject: [PATCH 12/12] PR comments --- .../Builder/ResilienceStrategyBuilderOptionsTests.cs | 1 + src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs | 2 +- src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs | 1 + src/Polly.Core/Builder/ResilienceStrategyBuilder.cs | 2 +- src/Polly.Core/Utils/ValidationHelper.cs | 2 +- 5 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs index 93636c3f386..4f2b13f74f9 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs @@ -3,6 +3,7 @@ using Xunit; namespace Polly.Core.Tests.Builder; + public class ResilienceStrategyBuilderOptionsTests { [Fact] diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index 51220908308..f991df6c060 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -142,7 +142,7 @@ public void AddStrategy_AfterUsed_Throws() .Invoking(b => b.AddStrategy(NullResilienceStrategy.Instance)) .Should() .Throw() - .WithMessage("Unable to add any more resilience strategies to the builder after it has been used to build a strategy."); + .WithMessage("Cannot add any more resilience strategies to the builder after it has been used to build a strategy once."); } [Fact] diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs index 1149db233c3..f8870527190 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyOptionsTests.cs @@ -3,6 +3,7 @@ using Xunit; namespace Polly.Core.Tests.Builder; + public class ResilienceStrategyOptionsTests { [Fact] diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index ff0073ad15f..aca88357ec2 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -53,7 +53,7 @@ public ResilienceStrategyBuilder AddStrategy(Func