From 46dc47fd2a40d1c4a6b7406c54d06f3ce34a4234 Mon Sep 17 00:00:00 2001 From: martintmk <103487740+martintmk@users.noreply.github.com> Date: Thu, 17 Aug 2023 08:12:56 +0200 Subject: [PATCH] Drop unused ResiliencePipelineREgistri APIs (#1495) --- src/Polly.Core/PublicAPI.Unshipped.txt | 8 - .../ResiliencePipelineRegistry.TResult.cs | 8 - .../Registry/ResiliencePipelineRegistry.cs | 76 ------- .../PollyServiceCollectionExtensions.cs | 3 - .../ResiliencePipelineRegistryTests.cs | 193 ++++-------------- .../PollyServiceCollectionExtensionTests.cs | 6 +- ...uesTests.OverrideLibraryStrategies_1072.cs | 3 +- 7 files changed, 48 insertions(+), 249 deletions(-) diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index 5d0dae075af..0fe492f1c48 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -167,22 +167,14 @@ Polly.Registry.ConfigureBuilderContext.PipelineKey.get -> TKey Polly.Registry.ResiliencePipelineProvider Polly.Registry.ResiliencePipelineProvider.ResiliencePipelineProvider() -> void Polly.Registry.ResiliencePipelineRegistry -Polly.Registry.ResiliencePipelineRegistry.ClearPipelines() -> void -Polly.Registry.ResiliencePipelineRegistry.ClearPipelines() -> void Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action!>! configure) -> Polly.ResiliencePipeline! Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action! configure) -> Polly.ResiliencePipeline! Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action!, Polly.Registry.ConfigureBuilderContext!>! configure) -> Polly.ResiliencePipeline! Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action!>! configure) -> Polly.ResiliencePipeline! -Polly.Registry.ResiliencePipelineRegistry.RemoveBuilder(TKey key) -> bool -Polly.Registry.ResiliencePipelineRegistry.RemoveBuilder(TKey key) -> bool -Polly.Registry.ResiliencePipelineRegistry.RemovePipeline(TKey key) -> bool -Polly.Registry.ResiliencePipelineRegistry.RemovePipeline(TKey key) -> bool Polly.Registry.ResiliencePipelineRegistry.ResiliencePipelineRegistry() -> void Polly.Registry.ResiliencePipelineRegistry.ResiliencePipelineRegistry(Polly.Registry.ResiliencePipelineRegistryOptions! options) -> void Polly.Registry.ResiliencePipelineRegistry.TryAddBuilder(TKey key, System.Action!>! configure) -> bool Polly.Registry.ResiliencePipelineRegistry.TryAddBuilder(TKey key, System.Action!, Polly.Registry.ConfigureBuilderContext!>! configure) -> bool -Polly.Registry.ResiliencePipelineRegistry.TryAddPipeline(TKey key, Polly.ResiliencePipeline! pipeline) -> bool -Polly.Registry.ResiliencePipelineRegistry.TryAddPipeline(TKey key, Polly.ResiliencePipeline! pipeline) -> bool Polly.Registry.ResiliencePipelineRegistryOptions Polly.Registry.ResiliencePipelineRegistryOptions.BuilderComparer.get -> System.Collections.Generic.IEqualityComparer! Polly.Registry.ResiliencePipelineRegistryOptions.BuilderComparer.set -> void diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs index 1fe4849965d..4e3b5668429 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs @@ -28,10 +28,6 @@ public GenericRegistry( _instanceNameFormatter = instanceNameFormatter; } - public bool TryAdd(TKey key, ResiliencePipeline strategy) => _strategies.TryAdd(key, strategy); - - public bool Remove(TKey key) => _strategies.TryRemove(key, out _); - public bool TryGet(TKey key, [NotNullWhen(true)] out ResiliencePipeline? strategy) { if (_strategies.TryGetValue(key, out strategy)) @@ -65,9 +61,5 @@ public ResiliencePipeline GetOrAdd(TKey key, Action, ConfigureBuilderContext> configure) => _builders.TryAdd(key, configure); - - public bool RemoveBuilder(TKey key) => _builders.TryRemove(key, out _); - - public void Clear() => _strategies.Clear(); } } diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs index 6b4fe3de875..50a47749107 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs @@ -60,50 +60,6 @@ public ResiliencePipelineRegistry(ResiliencePipelineRegistryOptions option _pipelineComparer = options.PipelineComparer; } - /// - /// Tries to add an existing resilience pipeline to the registry. - /// - /// The key used to identify the resilience pipeline. - /// The resilience pipeline instance. - /// if the pipeline was added successfully, otherwise. - /// Thrown when is . - public bool TryAddPipeline(TKey key, ResiliencePipeline pipeline) - { - Guard.NotNull(pipeline); - - return _pipelines.TryAdd(key, pipeline); - } - - /// - /// Tries to add an existing generic resilience pipeline to the registry. - /// - /// The type of result that the resilience pipeline handles. - /// The key used to identify the resilience pipeline. - /// The resilience pipeline instance. - /// if the pipeline was added successfully, otherwise. - /// Thrown when is . - public bool TryAddPipeline(TKey key, ResiliencePipeline pipeline) - { - Guard.NotNull(pipeline); - - return GetGenericRegistry().TryAdd(key, pipeline); - } - - /// - /// Removes a resilience pipeline from the registry. - /// - /// The key used to identify the resilience pipeline. - /// if the pipeline was removed successfully, otherwise. - public bool RemovePipeline(TKey key) => _pipelines.TryRemove(key, out _); - - /// - /// Removes a generic resilience pipeline from the registry. - /// - /// The type of result that the resilience pipeline handles. - /// The key used to identify the resilience pipeline. - /// if the pipeline was removed successfully, otherwise. - public bool RemovePipeline(TKey key) => GetGenericRegistry().Remove(key); - /// public override bool TryGetPipeline(TKey key, [NotNullWhen(true)] out ResiliencePipeline? pipeline) { @@ -232,38 +188,6 @@ public bool TryAddBuilder(TKey key, Action().TryAddBuilder(key, configure); } - /// - /// Removes a resilience pipeline builder from the registry. - /// - /// The key used to identify the resilience pipeline builder. - /// if the builder was removed successfully, otherwise. - public bool RemoveBuilder(TKey key) => _builders.TryRemove(key, out _); - - /// - /// Removes a generic resilience pipeline builder from the registry. - /// - /// The type of result that the resilience pipeline handles. - /// The key used to identify the resilience pipeline builder. - /// if the builder was removed successfully, otherwise. - public bool RemoveBuilder(TKey key) => GetGenericRegistry().RemoveBuilder(key); - - /// - /// Clears all cached pipelines. - /// - /// - /// This method only clears the cached pipelines, the registered builders are kept unchanged. - /// - public void ClearPipelines() => _pipelines.Clear(); - - /// - /// Clears all cached generic pipelines. - /// - /// The type of result that the resilience pipeline handles. - /// - /// This method only clears the cached pipelines, the registered builders are kept unchanged. - /// - public void ClearPipelines() => GetGenericRegistry().Clear(); - private static PipelineComponent CreatePipelineComponent( Func activator, ConfigureBuilderContext context, diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index 302ad620355..be0826c845f 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -76,8 +76,6 @@ public static IServiceCollection AddResiliencePipeline( { options.Actions.Add((registry) => { - // the last added builder with the same key wins, this allows overriding the builders - registry.RemoveBuilder(key); registry.TryAddBuilder(key, (builder, context) => { configure(builder, new AddResiliencePipelineContext(context, serviceProvider)); @@ -148,7 +146,6 @@ public static IServiceCollection AddResiliencePipeline( options.Actions.Add((registry) => { // the last added builder with the same key wins, this allows overriding the builders - registry.RemoveBuilder(key); registry.TryAddBuilder(key, (builder, context) => { configure(builder, new AddResiliencePipelineContext(context, serviceProvider)); diff --git a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs index cde4c8fe910..f725f31dfaa 100644 --- a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs @@ -3,7 +3,6 @@ using Polly.Retry; using Polly.Testing; using Polly.Timeout; -using Polly.Utils; namespace Polly.Core.Tests.Registry; @@ -39,96 +38,6 @@ public void Ctor_InvalidOptions_Throws() .Throw(); } - [Fact] - public void ClearPipelines_Ok() - { - var registry = new ResiliencePipelineRegistry(); - - registry.TryAddBuilder("C", (b, _) => b.AddStrategy(new TestResilienceStrategy())); - - registry.TryAddPipeline("A", new TestResilienceStrategy().AsPipeline()); - registry.TryAddPipeline("B", new TestResilienceStrategy().AsPipeline()); - registry.TryAddPipeline("C", new TestResilienceStrategy().AsPipeline()); - - registry.ClearPipelines(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - registry.TryGetPipeline("B", out _).Should().BeFalse(); - registry.TryGetPipeline("C", out _).Should().BeTrue(); - } - - [Fact] - public void ClearPipelines_Generic_Ok() - { - var registry = new ResiliencePipelineRegistry(); - - registry.TryAddBuilder("C", (b, _) => b.AddStrategy(new TestResilienceStrategy())); - - registry.TryAddPipeline("A", new ResiliencePipeline(PipelineComponent.Null)); - registry.TryAddPipeline("B", new ResiliencePipeline(PipelineComponent.Null)); - registry.TryAddPipeline("C", new ResiliencePipeline(PipelineComponent.Null)); - - registry.ClearPipelines(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - registry.TryGetPipeline("B", out _).Should().BeFalse(); - registry.TryGetPipeline("C", out _).Should().BeTrue(); - } - - [Fact] - public void Remove_Ok() - { - var registry = new ResiliencePipelineRegistry(); - - registry.TryAddPipeline("A", new TestResilienceStrategy().AsPipeline()); - registry.TryAddPipeline("B", new TestResilienceStrategy().AsPipeline()); - - registry.RemovePipeline("A").Should().BeTrue(); - registry.RemovePipeline("A").Should().BeFalse(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - registry.TryGetPipeline("B", out _).Should().BeTrue(); - } - - [Fact] - public void Remove_Generic_Ok() - { - var registry = new ResiliencePipelineRegistry(); - - registry.TryAddPipeline("A", new ResiliencePipeline(PipelineComponent.Null)); - registry.TryAddPipeline("B", new ResiliencePipeline(PipelineComponent.Null)); - - registry.RemovePipeline("A").Should().BeTrue(); - registry.RemovePipeline("A").Should().BeFalse(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - registry.TryGetPipeline("B", out _).Should().BeTrue(); - } - - [Fact] - public void RemoveBuilder_Ok() - { - var registry = new ResiliencePipelineRegistry(); - registry.TryAddBuilder("A", (b, _) => b.AddStrategy(new TestResilienceStrategy())); - - registry.RemoveBuilder("A").Should().BeTrue(); - registry.RemoveBuilder("A").Should().BeFalse(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - } - - [Fact] - public void RemoveBuilder_Generic_Ok() - { - var registry = new ResiliencePipelineRegistry(); - registry.TryAddBuilder("A", (b, _) => b.AddStrategy(new TestResilienceStrategy())); - - registry.RemoveBuilder("A").Should().BeTrue(); - registry.RemoveBuilder("A").Should().BeFalse(); - - registry.TryGetPipeline("A", out _).Should().BeFalse(); - } - [Fact] public void GetPipeline_BuilderMultiInstance_EnsureMultipleInstances() { @@ -172,7 +81,7 @@ public void GetPipeline_GenericBuilderMultiInstance_EnsureMultipleInstances() } [Fact] - public void AddBuilder_GetPipeline_EnsureCalled() + public void TryAddBuilder_GetPipeline_EnsureCalled() { var activatorCalls = 0; _callback = _ => activatorCalls++; @@ -200,7 +109,7 @@ public void AddBuilder_GetPipeline_EnsureCalled() } [Fact] - public void AddBuilder_GenericGetPipeline_EnsureCalled() + public void TryAddBuilder_GenericGetPipeline_EnsureCalled() { var activatorCalls = 0; _callback = _ => activatorCalls++; @@ -228,7 +137,7 @@ public void AddBuilder_GenericGetPipeline_EnsureCalled() } [Fact] - public void AddBuilder_EnsurePipelineKey() + public void TryAddBuilder_EnsurePipelineKey() { _options.BuilderNameFormatter = k => k.BuilderName; _options.InstanceNameFormatter = k => k.InstanceName; @@ -250,8 +159,46 @@ public void AddBuilder_EnsurePipelineKey() called.Should().BeTrue(); } + [InlineData(false)] + [InlineData(true)] + [Theory] + public void TryAddBuilder_Twice_EnsureCorrectBehavior(bool generic) + { + var registry = new ResiliencePipelineRegistry(); + + var called1 = false; + var called2 = false; + + AddBuilder(() => called1 = true).Should().BeTrue(); + AddBuilder(() => called2 = true).Should().BeFalse(); + + if (generic) + { + registry.GetPipeline("A"); + } + else + { + registry.GetPipeline("A"); + } + + called1.Should().BeTrue(); + called2.Should().BeFalse(); + + bool AddBuilder(Action onCalled) + { + if (generic) + { + return registry!.TryAddBuilder("A", (_, _) => onCalled()); + } + else + { + return registry!.TryAddBuilder("A", (_, _) => onCalled()); + } + } + } + [Fact] - public void AddBuilder_MultipleGeneric_EnsureDistinctInstances() + public void TryAddBuilder_MultipleGeneric_EnsureDistinctInstances() { var registry = CreateRegistry(); registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => builder.AddStrategy(new TestResilienceStrategy())); @@ -262,7 +209,7 @@ public void AddBuilder_MultipleGeneric_EnsureDistinctInstances() } [Fact] - public void AddBuilder_Generic_EnsurePipelineKey() + public void TryAddBuilder_Generic_EnsurePipelineKey() { _options.BuilderNameFormatter = k => k.BuilderName; _options.InstanceNameFormatter = k => k.InstanceName; @@ -301,60 +248,6 @@ public void TryGet_GenericNoBuilder_Null() strategy.Should().BeNull(); } - [Fact] - public void TryGet_ExplicitPipelineAdded_Ok() - { - var expectedPipeline = new TestResilienceStrategy().AsPipeline(); - var registry = CreateRegistry(); - var key = StrategyId.Create("A", "Instance"); - registry.TryAddPipeline(key, expectedPipeline).Should().BeTrue(); - - registry.TryGetPipeline(key, out var strategy).Should().BeTrue(); - - strategy.Should().BeSameAs(expectedPipeline); - } - - [Fact] - public void TryGet_GenericExplicitPipelineAdded_Ok() - { - var expectedPipeline = new ResiliencePipeline(PipelineComponent.Null); - var registry = CreateRegistry(); - var key = StrategyId.Create("A", "Instance"); - registry.TryAddPipeline(key, expectedPipeline).Should().BeTrue(); - - registry.TryGetPipeline(key, out var strategy).Should().BeTrue(); - - strategy.Should().BeSameAs(expectedPipeline); - } - - [Fact] - public void TryAdd_Twice_SecondNotAdded() - { - var expectedPipeline = new TestResilienceStrategy().AsPipeline(); - var registry = CreateRegistry(); - var key = StrategyId.Create("A", "Instance"); - registry.TryAddPipeline(key, expectedPipeline).Should().BeTrue(); - - registry.TryAddPipeline(key, new TestResilienceStrategy().AsPipeline()).Should().BeFalse(); - - registry.TryGetPipeline(key, out var strategy).Should().BeTrue(); - strategy.Should().BeSameAs(expectedPipeline); - } - - [Fact] - public void TryAdd_GenericTwice_SecondNotAdded() - { - var expectedPipeline = new ResiliencePipeline(PipelineComponent.Null); - var registry = CreateRegistry(); - var key = StrategyId.Create("A", "Instance"); - registry.TryAddPipeline(key, expectedPipeline).Should().BeTrue(); - - registry.TryAddPipeline(key, new ResiliencePipeline(PipelineComponent.Null)).Should().BeFalse(); - - registry.TryGetPipeline(key, out var strategy).Should().BeTrue(); - strategy.Should().BeSameAs(expectedPipeline); - } - [Fact] public void EnableReloads_Ok() { diff --git a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs index 5187bf97fef..8d6e160d52d 100644 --- a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs +++ b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs @@ -197,7 +197,7 @@ public void AddResiliencePipeline_Single_Ok() [InlineData(true)] [InlineData(false)] [Theory] - public void AddResiliencePipeline_Twice_LastOneWins(bool generic) + public void AddResiliencePipeline_Twice_FirstOneWins(bool generic) { var firstCalled = false; var secondCalled = false; @@ -215,8 +215,8 @@ public void AddResiliencePipeline_Twice_LastOneWins(bool generic) CreateProvider().GetPipeline(Key); } - firstCalled.Should().BeFalse(); - secondCalled.Should().BeTrue(); + firstCalled.Should().BeTrue(); + secondCalled.Should().BeFalse(); } [Fact] diff --git a/test/Polly.Extensions.Tests/Issues/IssuesTests.OverrideLibraryStrategies_1072.cs b/test/Polly.Extensions.Tests/Issues/IssuesTests.OverrideLibraryStrategies_1072.cs index 01ddaddf36f..9e3e23fc6b8 100644 --- a/test/Polly.Extensions.Tests/Issues/IssuesTests.OverrideLibraryStrategies_1072.cs +++ b/test/Polly.Extensions.Tests/Issues/IssuesTests.OverrideLibraryStrategies_1072.cs @@ -15,7 +15,6 @@ public void OverrideLibraryStrategies_898(bool overrideStrategy) // arrange var services = new ServiceCollection(); var failFirstCall = true; - AddLibraryServices(services); if (overrideStrategy) { @@ -32,6 +31,8 @@ public void OverrideLibraryStrategies_898(bool overrideStrategy) })); } + AddLibraryServices(services); + var serviceProvider = services.BuildServiceProvider(); var api = serviceProvider.GetRequiredService();