From 79c7b4a356fed39a016961fbbe4b736892482f81 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Tue, 4 Jul 2023 14:40:43 +0200 Subject: [PATCH 1/4] `ResilienceStrategyRegistry` API improvements --- .../ResilienceStrategyRegistry.TResult.cs | 28 ++++--- .../Registry/ResilienceStrategyRegistry.cs | 81 ++++++++++++++++--- .../AddResilienceStrategyContext.cs | 9 +-- .../PollyServiceCollectionExtensions.cs | 25 +++++- src/Polly.Extensions/Polly.Extensions.csproj | 2 +- .../ConfigureBuilderContextExtensions.cs | 36 +++++++++ .../ResilienceStrategyRegistryTests.cs | 27 +++++++ .../PollyServiceCollectionExtensionTests.cs | 10 +++ 8 files changed, 183 insertions(+), 35 deletions(-) create mode 100644 src/Polly.Extensions/Registry/ConfigureBuilderContextExtensions.cs diff --git a/src/Polly.Core/Registry/ResilienceStrategyRegistry.TResult.cs b/src/Polly.Core/Registry/ResilienceStrategyRegistry.TResult.cs index e23d364089d..7ff833b2c41 100644 --- a/src/Polly.Core/Registry/ResilienceStrategyRegistry.TResult.cs +++ b/src/Polly.Core/Registry/ResilienceStrategyRegistry.TResult.cs @@ -41,18 +41,7 @@ public bool TryGet(TKey key, [NotNullWhen(true)] out ResilienceStrategy if (_builders.TryGetValue(key, out var configure)) { - var context = new ConfigureBuilderContext(key, _builderNameFormatter(key), _strategyKeyFormatter(key)); - -#if NETCOREAPP3_0_OR_GREATER - strategy = _strategies.GetOrAdd(key, static (_, factory) => - { - return new ResilienceStrategy(CreateStrategy(factory.instance._activator, factory.context, factory.configure)); - }, - (instance: this, context, configure)); -#else - strategy = _strategies.GetOrAdd(key, _ => new ResilienceStrategy(CreateStrategy(_activator, context, configure))); -#endif - + strategy = GetOrAdd(key, configure); return true; } @@ -60,6 +49,21 @@ public bool TryGet(TKey key, [NotNullWhen(true)] out ResilienceStrategy return false; } + public ResilienceStrategy GetOrAdd(TKey key, Action, ConfigureBuilderContext> configure) + { + var context = new ConfigureBuilderContext(key, _builderNameFormatter(key), _strategyKeyFormatter(key)); + +#if NETCOREAPP3_0_OR_GREATER + return _strategies.GetOrAdd(key, static (_, factory) => + { + return new ResilienceStrategy(CreateStrategy(factory.instance._activator, factory.context, factory.configure)); + }, + (instance: this, context, configure)); +#else + return _strategies.GetOrAdd(key, _ => new ResilienceStrategy(CreateStrategy(_activator, context, configure))); +#endif + } + public bool TryAddBuilder(TKey key, Action, ConfigureBuilderContext> configure) => _builders.TryAdd(key, configure); public bool RemoveBuilder(TKey key) => _builders.TryRemove(key, out _); diff --git a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs index 96cfb7caf12..221a5c018d3 100644 --- a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs +++ b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs @@ -118,17 +118,7 @@ public override bool TryGetStrategy(TKey key, [NotNullWhen(true)] out Resilience if (_builders.TryGetValue(key, out var configure)) { - var context = new ConfigureBuilderContext(key, _builderNameFormatter(key), _strategyKeyFormatter(key)); - -#if NETCOREAPP3_0_OR_GREATER - strategy = _strategies.GetOrAdd(key, static (_, factory) => - { - return CreateStrategy(factory.instance._activator, factory.context, factory.configure); - }, - (instance: this, context, configure)); -#else - strategy = _strategies.GetOrAdd(key, _ => CreateStrategy(_activator, context, configure)); -#endif + strategy = GetOrAddStrategy(key, configure); return true; } @@ -136,6 +126,75 @@ public override bool TryGetStrategy(TKey key, [NotNullWhen(true)] out Resilience return false; } + /// + /// Gets existing strategy or creates a new one using the callback. + /// + /// The key used to identify the resilience strategy. + /// The callback that configures the strategy builder. + /// An instance of strategy. + public ResilienceStrategy GetOrAddStrategy(TKey key, Action configure) + { + Guard.NotNull(configure); + + return GetOrAddStrategy(key, (builder, _) => configure(builder)); + } + + /// + /// Gets existing strategy or creates a new one using the callback. + /// + /// The key used to identify the resilience strategy. + /// The callback that configures the strategy builder. + /// An instance of strategy. + public ResilienceStrategy GetOrAddStrategy(TKey key, Action> configure) + { + Guard.NotNull(configure); + + if (_strategies.TryGetValue(key, out var strategy)) + { + return strategy; + } + + var context = new ConfigureBuilderContext(key, _builderNameFormatter(key), _strategyKeyFormatter(key)); + +#if NETCOREAPP3_0_OR_GREATER + return _strategies.GetOrAdd(key, static (_, factory) => + { + return CreateStrategy(factory.instance._activator, factory.context, factory.configure); + }, + (instance: this, context, configure)); +#else + return _strategies.GetOrAdd(key, _ => CreateStrategy(_activator, context, configure)); +#endif + } + + /// + /// Gets existing strategy or creates a new one using the callback. + /// + /// The type of result that the resilience strategy handles. + /// The key used to identify the resilience strategy. + /// The callback that configures the strategy builder. + /// An instance of strategy. + public ResilienceStrategy GetOrAddStrategy(TKey key, Action> configure) + { + Guard.NotNull(configure); + + return GetOrAddStrategy(key, (builder, _) => configure(builder)); + } + + /// + /// Gets existing strategy or creates a new one using the callback. + /// + /// The type of result that the resilience strategy handles. + /// The key used to identify the resilience strategy. + /// The callback that configures the strategy builder. + /// An instance of strategy. + public ResilienceStrategy GetOrAddStrategy(TKey key, Action, ConfigureBuilderContext> configure) + { + Guard.NotNull(configure); + + return GetGenericRegistry().GetOrAdd(key, configure); + } + /// /// Tries to add a resilience strategy builder to the registry. /// diff --git a/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs b/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs index 4836eaeaa55..3792108b1ab 100644 --- a/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs +++ b/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs @@ -1,7 +1,7 @@ using System; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using Polly.Extensions.Utils; +using Polly.Extensions.Registry; using Polly.Registry; namespace Polly.Extensions.DependencyInjection; @@ -53,12 +53,7 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext registryCont /// You can listen for changes only for single options. If you call this method multiple times, the preceding calls are ignored and only the last one wins. /// /// - public void EnableReloads(string? name = null) - { - var monitor = ServiceProvider.GetRequiredService>(); - - RegistryContext.EnableReloads(() => new OptionsReloadHelper(monitor, name ?? Options.DefaultName).GetCancellationToken); - } + public void EnableReloads(string? name = null) => RegistryContext.EnableReloads(ServiceProvider.GetRequiredService>(), name); /// /// Gets the options identified by . diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index bfa35f079e3..8c5b9ab7c4d 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -85,7 +85,7 @@ public static IServiceCollection AddResilienceStrategy( }); }); - return AddResilienceStrategyRegistry(services); + return AddResilienceStrategy(services); } /// @@ -156,12 +156,29 @@ public static IServiceCollection AddResilienceStrategy( }); }); - return AddResilienceStrategyRegistry(services); + return AddResilienceStrategy(services); } - private static IServiceCollection AddResilienceStrategyRegistry(this IServiceCollection services) + /// + /// Adds the infrastructure that allows configuring and retrieving resilience strategies using the key. + /// + /// The type of the key used to identify the resilience strategy. + /// The to add the resilience strategy to. + /// The updated with additional services added. + /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// + /// You can retrieve the strategy registry by resolving the + /// or class from the dependency injection container. + /// + /// This call enables the telemetry for al resilience strategies created using . + /// + /// + /// Thrown when is . + public static IServiceCollection AddResilienceStrategy(this IServiceCollection services) where TKey : notnull { + Guard.NotNull(services); + // check marker to ensure the APIs bellow are called only once for each TKey type // this prevents polluting the service collection with unnecessary Configure calls if (services.Contains(RegistryMarker.ServiceDescriptor)) @@ -172,7 +189,7 @@ private static IServiceCollection AddResilienceStrategyRegistry(this IServ services.AddOptions(); services.Add(RegistryMarker.ServiceDescriptor); services.AddResilienceStrategyBuilder(); - services.AddResilienceStrategyRegistry(); + services.AddResilienceStrategy(); services.TryAddSingleton(serviceProvider => { diff --git a/src/Polly.Extensions/Polly.Extensions.csproj b/src/Polly.Extensions/Polly.Extensions.csproj index 945084a8f63..0c9fbbe7862 100644 --- a/src/Polly.Extensions/Polly.Extensions.csproj +++ b/src/Polly.Extensions/Polly.Extensions.csproj @@ -29,6 +29,6 @@ - + diff --git a/src/Polly.Extensions/Registry/ConfigureBuilderContextExtensions.cs b/src/Polly.Extensions/Registry/ConfigureBuilderContextExtensions.cs new file mode 100644 index 00000000000..bdb257c379b --- /dev/null +++ b/src/Polly.Extensions/Registry/ConfigureBuilderContextExtensions.cs @@ -0,0 +1,36 @@ +using Microsoft.Extensions.Options; +using Polly.Extensions.Utils; +using Polly.Registry; +using Polly.Utils; + +namespace Polly.Extensions.Registry; + +/// +/// Extensions for . +/// +public static class ConfigureBuilderContextExtensions +{ + /// + /// Enables dynamic reloading of the resilience strategy whenever the options are changed. + /// + /// The type of the key used to identify the resilience strategy. + /// The options type to listen to. + /// The builder context. + /// The options monitor. + /// The named options, if any. + /// + /// You can decide based on the to listen for changes in global options or named options. + /// If is then the global options are listened to. + /// + /// You can listen for changes only for single options. If you call this method multiple times, the preceding calls are ignored and only the last one wins. + /// + /// + public static void EnableReloads(this ConfigureBuilderContext context, IOptionsMonitor optionsMonitor, string? name = null) + where TKey : notnull + { + Guard.NotNull(context); + Guard.NotNull(optionsMonitor); + + context.EnableReloads(() => new OptionsReloadHelper(optionsMonitor, name ?? Options.DefaultName).GetCancellationToken); + } +} diff --git a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index 2a95b2b95c6..5efa9982313 100644 --- a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -3,6 +3,7 @@ using Polly.Registry; using Polly.Retry; using Polly.Telemetry; +using Polly.Timeout; namespace Polly.Core.Tests.Registry; @@ -429,5 +430,31 @@ public void EnableReloads_Generic_Ok() tries.Should().Be(retryCount + 1); } + [Fact] + public void GetOrAddStrategy_Ok() + { + var id = new StrategyId(typeof(string), "A"); + + var registry = CreateRegistry(); + var strategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + var otherStrategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + + strategy.Should().BeOfType(); + strategy.Should().BeSameAs(otherStrategy); + } + + [Fact] + public void GetOrAddStrategy_Generic_Ok() + { + var id = new StrategyId(typeof(string), "A"); + + var registry = CreateRegistry(); + var strategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + var otherStrategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + + strategy.Strategy.Should().BeOfType(); + strategy.Should().BeSameAs(otherStrategy); + } + private ResilienceStrategyRegistry CreateRegistry() => new(_options); } diff --git a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs index 8caae3b2343..5b51244ebf5 100644 --- a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs +++ b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs @@ -254,6 +254,16 @@ public void AddResilienceStrategy_Multiple_Ok() .HaveCount(30); } + [Fact] + public void AddResilienceStrategyInfra_Ok() + { + var provider = new ServiceCollection().AddResilienceStrategy().BuildServiceProvider(); + + provider.GetRequiredService>().Should().NotBeNull(); + provider.GetRequiredService>().Should().NotBeNull(); + provider.GetRequiredService().DiagnosticSource.Should().NotBeNull(); + } + private void AddResilienceStrategy(string key, Action? onBuilding = null) { _services.AddResilienceStrategy(key, builder => From 5ad72a2bf0b6625cad3eac3b41e6618796469f27 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Tue, 4 Jul 2023 15:00:25 +0200 Subject: [PATCH 2/4] improve tests --- .../Registry/ResilienceStrategyRegistryTests.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index 5efa9982313..80c0c24ae43 100644 --- a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -434,23 +434,26 @@ public void EnableReloads_Generic_Ok() public void GetOrAddStrategy_Ok() { var id = new StrategyId(typeof(string), "A"); + var called = 0; var registry = CreateRegistry(); - var strategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); - var otherStrategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + var strategy = registry.GetOrAddStrategy(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); + var otherStrategy = registry.GetOrAddStrategy(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); strategy.Should().BeOfType(); strategy.Should().BeSameAs(otherStrategy); + called.Should().Be(1); } [Fact] public void GetOrAddStrategy_Generic_Ok() { var id = new StrategyId(typeof(string), "A"); + var called = 0; var registry = CreateRegistry(); - var strategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); - var otherStrategy = registry.GetOrAddStrategy(id, builder => builder.AddTimeout(TimeSpan.FromSeconds(1))); + var strategy = registry.GetOrAddStrategy(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); + var otherStrategy = registry.GetOrAddStrategy(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); strategy.Strategy.Should().BeOfType(); strategy.Should().BeSameAs(otherStrategy); From 0d831b211ca06ff6e32602d1e7c74806ed1cdd0a Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Tue, 4 Jul 2023 15:17:14 +0200 Subject: [PATCH 3/4] PR comments --- .../DependencyInjection/PollyServiceCollectionExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index 8c5b9ab7c4d..2f4127270b1 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -170,7 +170,7 @@ public static IServiceCollection AddResilienceStrategy( /// You can retrieve the strategy registry by resolving the /// or class from the dependency injection container. /// - /// This call enables the telemetry for al resilience strategies created using . + /// This call enables telemetry for all resilience strategies created using . /// /// /// Thrown when is . From b936db207ebba509d6aef84e703b4db2aadf4907 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Tue, 4 Jul 2023 15:19:59 +0200 Subject: [PATCH 4/4] cleanup --- .../PollyServiceCollectionExtensions.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index 2f4127270b1..9fd93250483 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -25,13 +25,13 @@ public static class PollyServiceCollectionExtensions /// An action that configures the resilience strategy. /// The updated with the registered resilience strategy. /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// Thrown when or is . /// /// You can retrieve the registered strategy by resolving the class from the dependency injection container. /// /// This call enables the telemetry for the registered resilience strategy. /// /// - /// Thrown when or is . public static IServiceCollection AddResilienceStrategy( this IServiceCollection services, TKey key, @@ -54,13 +54,13 @@ public static IServiceCollection AddResilienceStrategy( /// An action that configures the resilience strategy. /// The updated with the registered resilience strategy. /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// Thrown when or is . /// /// You can retrieve the registered strategy by resolving the class from the dependency injection container. /// /// This call enables the telemetry for the registered resilience strategy. /// /// - /// Thrown when or is . public static IServiceCollection AddResilienceStrategy( this IServiceCollection services, TKey key, @@ -97,13 +97,13 @@ public static IServiceCollection AddResilienceStrategy( /// An action that configures the resilience strategy. /// The updated with the registered resilience strategy. /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// Thrown when or is . /// /// You can retrieve the registered strategy by resolving the class from the dependency injection container. /// /// This call enables the telemetry for the registered resilience strategy. /// /// - /// Thrown when or is . public static IServiceCollection AddResilienceStrategy( this IServiceCollection services, TKey key, @@ -125,13 +125,13 @@ public static IServiceCollection AddResilienceStrategy( /// An action that configures the resilience strategy. /// The updated with the registered resilience strategy. /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// Thrown when or is . /// /// You can retrieve the registered strategy by resolving the class from the dependency injection container. /// /// This call enables the telemetry for the registered resilience strategy. /// /// - /// Thrown when or is . public static IServiceCollection AddResilienceStrategy( this IServiceCollection services, TKey key, @@ -165,7 +165,7 @@ public static IServiceCollection AddResilienceStrategy( /// The type of the key used to identify the resilience strategy. /// The to add the resilience strategy to. /// The updated with additional services added. - /// Thrown if the resilience strategy builder with the provided key has already been added to the registry. + /// Thrown when is . /// /// You can retrieve the strategy registry by resolving the /// or class from the dependency injection container. @@ -173,7 +173,6 @@ public static IServiceCollection AddResilienceStrategy( /// This call enables telemetry for all resilience strategies created using . /// /// - /// Thrown when is . public static IServiceCollection AddResilienceStrategy(this IServiceCollection services) where TKey : notnull {