diff --git a/src/Polly.Core/Registry/ConfigureBuilderContext.cs b/src/Polly.Core/Registry/ConfigureBuilderContext.cs index 50a00c1125b..4c4ab402264 100644 --- a/src/Polly.Core/Registry/ConfigureBuilderContext.cs +++ b/src/Polly.Core/Registry/ConfigureBuilderContext.cs @@ -1,3 +1,5 @@ +using System.ComponentModel; + namespace Polly.Registry; /// @@ -29,20 +31,21 @@ internal ConfigureBuilderContext(TKey strategyKey, string builderName, string st /// public string StrategyKeyString { get; } - internal Func? ReloadTokenProducer { get; private set; } + internal Func>? ReloadTokenProducer { get; private set; } /// /// Enables dynamic reloading of the strategy retrieved from . /// - /// The producer of that is triggered when change occurs. + /// The producer of that is triggered when change occurs. /// - /// The should always return a new instance when invoked otherwise - /// the reload infrastructure will stop listening for changes. + /// The should always return function that returns a new instance when invoked otherwise + /// the reload infrastructure will stop listening for changes. The is called only once for each streategy. /// - public void EnableReloads(Func reloadTokenProducer) + [EditorBrowsable(EditorBrowsableState.Never)] + public void EnableReloads(Func> tokenProducerFactory) { - Guard.NotNull(reloadTokenProducer); + Guard.NotNull(tokenProducerFactory); - ReloadTokenProducer = reloadTokenProducer; + ReloadTokenProducer = tokenProducerFactory; } } diff --git a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs index df58391bb7a..96cfb7caf12 100644 --- a/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs +++ b/src/Polly.Core/Registry/ResilienceStrategyRegistry.cs @@ -230,7 +230,7 @@ private static ResilienceStrategy CreateStrategy( return new ReloadableResilienceStrategy( strategy, - context.ReloadTokenProducer, + context.ReloadTokenProducer(), () => factory().BuildStrategy(), TelemetryUtil.CreateTelemetry( diagnosticSource, diff --git a/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs b/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs index 032604df498..4836eaeaa55 100644 --- a/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs +++ b/src/Polly.Extensions/DependencyInjection/AddResilienceStrategyContext.cs @@ -1,3 +1,4 @@ +using System; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Polly.Extensions.Utils; @@ -17,8 +18,14 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext registryCont RegistryContext = registryContext; ServiceProvider = serviceProvider; StrategyKey = registryContext.StrategyKey; + BuilderName = registryContext.BuilderName; } + /// + /// Gets the strategy key for the strategy being created. + /// + public string BuilderName { get; } + /// /// Gets the strategy key for the strategy being created. /// @@ -32,7 +39,7 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext registryCont /// /// Gets the context that is used by the registry. /// - public ConfigureBuilderContext RegistryContext { get; } + internal ConfigureBuilderContext RegistryContext { get; } /// /// Enables dynamic reloading of the resilience strategy whenever the options are changed. @@ -48,10 +55,9 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext registryCont /// public void EnableReloads(string? name = null) { - var registry = ServiceProvider.GetRequiredService>(); - var helper = registry.Get(StrategyKey, name); + var monitor = ServiceProvider.GetRequiredService>(); - RegistryContext.EnableReloads(helper.GetCancellationToken); + RegistryContext.EnableReloads(() => new OptionsReloadHelper(monitor, name ?? Options.DefaultName).GetCancellationToken); } /// diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index dcb8648a63a..bfa35f079e3 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Options; using Polly.Extensions.DependencyInjection; using Polly.Extensions.Telemetry; -using Polly.Extensions.Utils; using Polly.Registry; using Polly.Utils; @@ -174,7 +173,6 @@ private static IServiceCollection AddResilienceStrategyRegistry(this IServ services.Add(RegistryMarker.ServiceDescriptor); services.AddResilienceStrategyBuilder(); services.AddResilienceStrategyRegistry(); - services.TryAddSingleton>(); services.TryAddSingleton(serviceProvider => { diff --git a/src/Polly.Extensions/Utils/OptionsReloadHelper.cs b/src/Polly.Extensions/Utils/OptionsReloadHelper.cs index 26560ecefc9..c865f97b537 100644 --- a/src/Polly.Extensions/Utils/OptionsReloadHelper.cs +++ b/src/Polly.Extensions/Utils/OptionsReloadHelper.cs @@ -2,12 +2,14 @@ namespace Polly.Extensions.Utils; -internal sealed class OptionsReloadHelper : IDisposable +#pragma warning disable CA1001 // we can get away of not disposing this class because it's active for a lifetime of app +#pragma warning disable S2931 + +internal sealed class OptionsReloadHelper { - private readonly IDisposable? _listener; private CancellationTokenSource _cancellation = new(); - public OptionsReloadHelper(IOptionsMonitor monitor, string name) => _listener = monitor.OnChange((_, changedNamed) => + public OptionsReloadHelper(IOptionsMonitor monitor, string name) => monitor.OnChange((_, changedNamed) => { if (name == changedNamed) { @@ -17,12 +19,6 @@ public OptionsReloadHelper(IOptionsMonitor monitor, string name) => _listener public CancellationToken GetCancellationToken() => _cancellation.Token; - public void Dispose() - { - _cancellation.Dispose(); - _listener?.Dispose(); - } - private void HandleChange() { var oldCancellation = _cancellation; diff --git a/src/Polly.Extensions/Utils/OptionsReloadHelperRegistry.cs b/src/Polly.Extensions/Utils/OptionsReloadHelperRegistry.cs deleted file mode 100644 index e0270649d50..00000000000 --- a/src/Polly.Extensions/Utils/OptionsReloadHelperRegistry.cs +++ /dev/null @@ -1,34 +0,0 @@ -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; - -namespace Polly.Extensions.Utils; - -internal sealed class OptionsReloadHelperRegistry : IDisposable -{ - private readonly IServiceProvider _serviceProvider; - private readonly ConcurrentDictionary<(Type optionsType, TKey key, string? name), object> _helpers = new(); - - public OptionsReloadHelperRegistry(IServiceProvider serviceProvider) => _serviceProvider = serviceProvider; - - public int Count => _helpers.Count; - - public OptionsReloadHelper Get(TKey key, string? name) - { - name ??= Options.DefaultName; - - return (OptionsReloadHelper)_helpers.GetOrAdd((typeof(TOptions), key, name), _ => - { - return new OptionsReloadHelper(_serviceProvider.GetRequiredService>(), name); - }); - } - - public void Dispose() - { - foreach (var helper in _helpers.Values.OfType()) - { - helper.Dispose(); - } - - _helpers.Clear(); - } -} diff --git a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs index 0f141b11d50..2a95b2b95c6 100644 --- a/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResilienceStrategyRegistryTests.cs @@ -368,7 +368,7 @@ public void EnableReloads_Ok() registry.TryAddBuilder("dummy", (builder, context) => { // this call enables dynamic reloads for the dummy strategy - context.EnableReloads(() => changeSource.Token); + context.EnableReloads(() => () => changeSource.Token); builder.AddRetry(new RetryStrategyOptions { @@ -404,7 +404,7 @@ public void EnableReloads_Generic_Ok() registry.TryAddBuilder("dummy", (builder, context) => { // this call enables dynamic reloads for the dummy strategy - context.EnableReloads(() => changeSource.Token); + context.EnableReloads(() => () => changeSource.Token); builder.AddRetry(new RetryStrategyOptions { diff --git a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs index b693221931c..8caae3b2343 100644 --- a/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs +++ b/test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs @@ -97,6 +97,7 @@ public void AddResilienceStrategy_EnsureContextFilled(bool generic) { context.RegistryContext.Should().NotBeNull(); context.StrategyKey.Should().Be(Key); + context.BuilderName.Should().Be(Key); builder.Should().NotBeNull(); context.ServiceProvider.Should().NotBeNull(); builder.AddStrategy(new TestStrategy()); diff --git a/test/Polly.Extensions.Tests/ReloadableResilienceStrategyTests.cs b/test/Polly.Extensions.Tests/ReloadableResilienceStrategyTests.cs index 2d477ec4b19..f5fbc022471 100644 --- a/test/Polly.Extensions.Tests/ReloadableResilienceStrategyTests.cs +++ b/test/Polly.Extensions.Tests/ReloadableResilienceStrategyTests.cs @@ -1,8 +1,5 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; -using Moq; -using Polly.Extensions.Utils; using Polly.Registry; namespace Polly.Extensions.Tests; @@ -43,7 +40,6 @@ public void AddResilienceStrategy_EnsureReloadable(string? name) var serviceProvider = services.BuildServiceProvider(); var strategy = serviceProvider.GetRequiredService>().GetStrategy("my-strategy"); var context = ResilienceContext.Get(); - var registry = serviceProvider.GetRequiredService>(); // initial strategy.Execute(_ => "dummy", context); @@ -56,38 +52,6 @@ public void AddResilienceStrategy_EnsureReloadable(string? name) strategy.Execute(_ => "dummy", context); context.Properties.GetValue(TagKey, string.Empty).Should().Be($"reload-{i}"); } - - registry.Count.Should().Be(1); - serviceProvider.Dispose(); - registry.Count.Should().Be(0); - } - - [Fact] - public void OptionsReloadHelperRegistry_EnsureTracksDifferentHelpers() - { - var services = new ServiceCollection().AddResilienceStrategy("dummy", builder => { }); - var serviceProvider = services.BuildServiceProvider(); - var registry = serviceProvider.GetRequiredService>(); - - registry.Get("A", null); - registry.Get("A", "dummy"); - registry.Get("B", null); - registry.Get("B", "dummy"); - - registry.Count.Should().Be(4); - - registry.Dispose(); - registry.Count.Should().Be(0); - } - - [Fact] - public void OptionsReloadHelper_Dispose_Ok() - { - var monitor = new Mock>(); - - using var helper = new OptionsReloadHelper(monitor.Object, string.Empty); - - helper.Invoking(h => h.Dispose()).Should().NotThrow(); } public class ReloadableStrategy : ResilienceStrategy diff --git a/test/Polly.Extensions.Tests/Utils/OptionsReloadHelperTests.cs b/test/Polly.Extensions.Tests/Utils/OptionsReloadHelperTests.cs index 693adc0eda5..5f24dad197d 100644 --- a/test/Polly.Extensions.Tests/Utils/OptionsReloadHelperTests.cs +++ b/test/Polly.Extensions.Tests/Utils/OptionsReloadHelperTests.cs @@ -15,7 +15,7 @@ public void Ctor_NamedOptions() .Setup(m => m.OnChange(It.IsAny>())) .Returns(() => Mock.Of()); - using var helper = new OptionsReloadHelper(monitor.Object, "name"); + var helper = new OptionsReloadHelper(monitor.Object, "name"); monitor.VerifyAll(); }