From 2e6d7a02a77e17c19a652695bb5cedc57a5d049d Mon Sep 17 00:00:00 2001 From: martintmk <103487740+martintmk@users.noreply.github.com> Date: Thu, 13 Apr 2023 17:19:09 +0200 Subject: [PATCH] Drop ResilienceStrategyBuilderOptions (#1117) --- .../ResilienceStrategyBuilderOptionsTests.cs | 41 ------------ .../Builder/ResilienceStrategyBuilderTests.cs | 52 +++++++-------- .../ResilienceStrategyRegistryTests.cs | 2 +- .../Builder/ResilienceStrategyBuilder.cs | 46 +++++++++---- .../ResilienceStrategyBuilderOptions.cs | 66 ------------------- .../PollyServiceCollectionExtensionTests.cs | 17 +---- .../PollyServiceCollectionExtensions.cs | 25 ++----- 7 files changed, 69 insertions(+), 180 deletions(-) delete mode 100644 src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs delete mode 100644 src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs deleted file mode 100644 index 1b7b1952816..00000000000 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderOptionsTests.cs +++ /dev/null @@ -1,41 +0,0 @@ -using Moq; -using Polly.Builder; -using Polly.Telemetry; -using Polly.Utils; - -namespace Polly.Core.Tests.Builder; - -public class ResilienceStrategyBuilderOptionsTests -{ - [Fact] - public void Ctor_EnsureDefaults() - { - var options = new ResilienceStrategyBuilderOptions(); - - options.BuilderName.Should().Be(""); - options.Properties.Should().NotBeNull(); - options.TimeProvider.Should().Be(TimeProvider.System); - options.TelemetryFactory.Should().Be(NullResilienceTelemetryFactory.Instance); - } - - [Fact] - public void Ctor_Copy_EnsureCopied() - { - var options = new ResilienceStrategyBuilderOptions - { - BuilderName = "test", - TelemetryFactory = Mock.Of(), - TimeProvider = new FakeTimeProvider().Object - }; - - options.Properties.Set(new ResiliencePropertyKey("A"), 1); - options.Properties.Set(new ResiliencePropertyKey("B"), 2); - - var other = new ResilienceStrategyBuilderOptions(options); - - other.BuilderName.Should().Be("test"); - other.TelemetryFactory.Should().BeSameAs(options.TelemetryFactory); - other.TimeProvider.Should().BeSameAs(options.TimeProvider); - other.Properties.Should().BeEquivalentTo(options.Properties); - } -} diff --git a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs index f51353a6081..20c68234bca 100644 --- a/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs @@ -2,11 +2,23 @@ using Moq; using Polly.Builder; using Polly.Telemetry; +using Polly.Utils; namespace Polly.Core.Tests.Builder; public class ResilienceStrategyBuilderTests { + [Fact] + public void Ctor_EnsureDefaults() + { + var builder = new ResilienceStrategyBuilder(); + + builder.BuilderName.Should().Be(""); + builder.Properties.Should().NotBeNull(); + builder.TimeProvider.Should().Be(TimeProvider.System); + builder.TelemetryFactory.Should().Be(NullResilienceTelemetryFactory.Instance); + } + [Fact] public void AddStrategy_Single_Ok() { @@ -143,26 +155,20 @@ public void AddStrategy_AfterUsed_Throws() .WithMessage("Cannot add any more resilience strategies to the builder after it has been used to build a strategy once."); } - [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!; + var builder = new ResilienceStrategyBuilder + { + BuilderName = null! + }; builder.Invoking(b => b.Build()) .Should() .Throw() .WithMessage( """ -The 'ResilienceStrategyBuilderOptions' options are not valid. +The 'ResilienceStrategyBuilder' configuration is invalid. Validation Errors: The BuilderName field is required. @@ -246,11 +252,8 @@ public void BuildStrategy_EnsureCorrectContext() var builder = new ResilienceStrategyBuilder { - Options = new ResilienceStrategyBuilderOptions - { - BuilderName = "builder-name", - TimeProvider = new FakeTimeProvider().Object - } + BuilderName = "builder-name", + TimeProvider = new FakeTimeProvider().Object }; builder.AddStrategy( @@ -259,10 +262,10 @@ public void BuildStrategy_EnsureCorrectContext() context.BuilderName.Should().Be("builder-name"); context.StrategyName.Should().Be("strategy-name"); context.StrategyType.Should().Be("strategy-type"); - context.BuilderProperties.Should().BeSameAs(builder.Options.Properties); + context.BuilderProperties.Should().BeSameAs(builder.Properties); context.Telemetry.Should().NotBeNull(); context.Telemetry.Should().Be(NullResilienceTelemetry.Instance); - context.TimeProvider.Should().Be(builder.Options.TimeProvider); + context.TimeProvider.Should().Be(builder.TimeProvider); verified1 = true; return new TestResilienceStrategy(); @@ -275,10 +278,10 @@ public void BuildStrategy_EnsureCorrectContext() context.BuilderName.Should().Be("builder-name"); context.StrategyName.Should().Be("strategy-name-2"); context.StrategyType.Should().Be("strategy-type-2"); - context.BuilderProperties.Should().BeSameAs(builder.Options.Properties); + context.BuilderProperties.Should().BeSameAs(builder.Properties); context.Telemetry.Should().NotBeNull(); context.Telemetry.Should().Be(NullResilienceTelemetry.Instance); - context.TimeProvider.Should().Be(builder.Options.TimeProvider); + context.TimeProvider.Should().Be(builder.TimeProvider); verified2 = true; return new TestResilienceStrategy(); @@ -300,11 +303,8 @@ public void BuildStrategy_EnsureTelemetryFactoryInvoked() var factory = new Mock(MockBehavior.Strict); var builder = new ResilienceStrategyBuilder { - Options = new ResilienceStrategyBuilderOptions - { - BuilderName = "builder-name", - TelemetryFactory = factory.Object - }, + BuilderName = "builder-name", + TelemetryFactory = factory.Object }; factory @@ -315,7 +315,7 @@ public void BuildStrategy_EnsureTelemetryFactoryInvoked() context.BuilderName.Should().Be("builder-name"); context.StrategyName.Should().Be("strategy-name"); context.StrategyType.Should().Be("strategy-type"); - context.BuilderProperties.Should().BeSameAs(builder.Options.Properties); + context.BuilderProperties.Should().BeSameAs(builder.Properties); }); builder.AddStrategy( diff --git a/src/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/src/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index 754956a6305..daf13abb9e2 100644 --- a/src/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/src/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -99,7 +99,7 @@ public void AddBuilder_GetStrategy_EnsureCalled() registry.TryAddBuilder(StrategyId.Create("A"), (key, builder) => { builder.AddStrategy(new TestResilienceStrategy()); - builder.Options.Properties.Set(StrategyId.ResilienceKey, key); + builder.Properties.Set(StrategyId.ResilienceKey, key); called++; }); diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs index a10040e40e6..717b450775a 100644 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs +++ b/src/Polly.Core/Builder/ResilienceStrategyBuilder.cs @@ -1,3 +1,4 @@ +using System.ComponentModel.DataAnnotations; using Polly.Telemetry; namespace Polly.Builder; @@ -13,17 +14,34 @@ namespace Polly.Builder; public class ResilienceStrategyBuilder { private readonly List _entries = new(); - private ResilienceStrategyBuilderOptions _options = new(); private bool _used; /// - /// Gets or sets the builder options. + /// Gets or sets the name of the builder. /// - public ResilienceStrategyBuilderOptions Options - { - get => _options; - set => _options = Guard.NotNull(value); - } + /// 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; + + /// + /// Gets the custom properties attached to builder options. + /// + public ResilienceProperties Properties { get; } = new(); + + /// + /// Gets or sets an instance of . + /// + [Required] + public ResilienceTelemetryFactory TelemetryFactory { get; set; } = NullResilienceTelemetryFactory.Instance; + + /// + /// Gets or sets a that is used by strategies that work with time. + /// + /// + /// This property is internal until we switch to official System.TimeProvider. + /// + [Required] + internal TimeProvider TimeProvider { get; set; } = TimeProvider.System; /// /// Adds an already created strategy instance to the builder. @@ -69,7 +87,7 @@ public ResilienceStrategyBuilder AddStrategy(FuncAn instance of . public ResilienceStrategy Build() { - ValidationHelper.ValidateObject(Options, $"The '{nameof(ResilienceStrategyBuilderOptions)}' options are not valid."); + ValidationHelper.ValidateObject(this, $"The '{nameof(ResilienceStrategyBuilder)}' configuration is invalid."); _used = true; @@ -92,20 +110,20 @@ private ResilienceStrategy CreateResilienceStrategy(Entry entry) { var telemetryContext = new ResilienceTelemetryFactoryContext { - BuilderName = Options.BuilderName, - BuilderProperties = Options.Properties, + BuilderName = BuilderName, + BuilderProperties = Properties, StrategyName = entry.Properties.StrategyName, StrategyType = entry.Properties.StrategyType }; var context = new ResilienceStrategyBuilderContext { - BuilderName = Options.BuilderName, - BuilderProperties = Options.Properties, + BuilderName = BuilderName, + BuilderProperties = Properties, StrategyName = entry.Properties.StrategyName, StrategyType = entry.Properties.StrategyType, - Telemetry = Options.TelemetryFactory.Create(telemetryContext), - TimeProvider = Options.TimeProvider + Telemetry = TelemetryFactory.Create(telemetryContext), + TimeProvider = TimeProvider }; return entry.Factory(context); diff --git a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs b/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs deleted file mode 100644 index c0eabecc027..00000000000 --- a/src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs +++ /dev/null @@ -1,66 +0,0 @@ -using System.ComponentModel; -using System.ComponentModel.DataAnnotations; -using Polly.Telemetry; - -namespace Polly.Builder; - -/// -/// The builder options used by . -/// -public class ResilienceStrategyBuilderOptions -{ - /// - /// Initializes a new instance of the class. - /// - public ResilienceStrategyBuilderOptions() - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The options to copy the values from. - [EditorBrowsable(EditorBrowsableState.Never)] - public ResilienceStrategyBuilderOptions(ResilienceStrategyBuilderOptions other) - { - Guard.NotNull(other); - - BuilderName = other.BuilderName; - TelemetryFactory = other.TelemetryFactory; - TimeProvider = other.TimeProvider; - - IDictionary props = Properties; - - foreach (KeyValuePair pair in other.Properties) - { - props.Add(pair.Key, pair.Value); - } - } - - /// - /// 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; - - /// - /// Gets the custom properties attached to builder options. - /// - public ResilienceProperties Properties { get; } = new(); - - /// - /// Gets or sets an instance of . - /// - [Required] - public ResilienceTelemetryFactory TelemetryFactory { get; set; } = NullResilienceTelemetryFactory.Instance; - - /// - /// Gets or sets a that is used by strategies that work with time. - /// - /// - /// This property is internal until we switch to official System.TimeProvider. - /// - [Required] - internal TimeProvider TimeProvider { get; set; } = TimeProvider.System; -} diff --git a/src/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs b/src/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs index ddb785911d5..6369c2cb2ec 100644 --- a/src/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs +++ b/src/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs @@ -70,29 +70,18 @@ public void AddResilienceStrategy_EnsureContextFilled() } [Fact] - public void AddResilienceStrategy_EnsureResilienceStrategyBuilderOptionsApplied() + public void AddResilienceStrategy_EnsureResilienceStrategyBuilderResolvedCorrectly() { var telemetry = Mock.Of(); var telemetryFactory = Mock.Of(v => v.Create(It.IsAny()) == telemetry); var asserted = false; var key = new ResiliencePropertyKey("A"); - ResilienceStrategyBuilderOptions? globalOptions = null; - _services.Configure(options => - { - options.BuilderName = "dummy"; - options.TelemetryFactory = telemetryFactory; - options.Properties.Set(key, 123); - globalOptions = options; - }); + _services.AddSingleton(telemetryFactory); AddResilienceStrategy(Key, context => { - context.BuilderProperties.Should().NotBeSameAs(globalOptions!.Properties); - context.BuilderName.Should().Be("dummy"); context.Telemetry.Should().Be(telemetry); - context.BuilderProperties.TryGetValue(key, out var val).Should().BeTrue(); - val.Should().Be(123); asserted = true; }); @@ -166,7 +155,7 @@ public void AddResilienceStrategy_CustomTelemetryFactory_EnsureUsed() Key, context => { - context.Builder.Options.TelemetryFactory.Should().Be(factory.Object); + context.Builder.TelemetryFactory.Should().Be(factory.Object); context.Builder.AddStrategy(context => { context.Telemetry.Should().Be(telemetry.Object); diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index 295b68cf843..28315a7f286 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -95,27 +95,16 @@ private static IServiceCollection AddResilienceStrategyRegistry(this IServ private static void AddResilienceStrategyBuilder(this IServiceCollection services) { - services - .AddOptions() - .PostConfigure((options, serviceProvider) => - { - if (options.TelemetryFactory == NullResilienceTelemetryFactory.Instance && - serviceProvider.GetService() is ResilienceTelemetryFactory factory) - { - options.TelemetryFactory = factory; - } - - options.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider); - }); - services.TryAddTransient(serviceProvider => { - var globalOptions = serviceProvider.GetRequiredService>().Value; - - return new ResilienceStrategyBuilder + var builder = new ResilienceStrategyBuilder(); + if (serviceProvider.GetService() is ResilienceTelemetryFactory factory) { - Options = new ResilienceStrategyBuilderOptions(globalOptions) - }; + builder.TelemetryFactory = factory; + } + + builder.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider); + return builder; }); }