Simplify handling of reloads (#1374)

src/Polly.Core/Registry/ConfigureBuilderContext.cs

@@ -1,3 +1,5 @@
+using System.ComponentModel;
+
 namespace Polly.Registry;
 
 /// <summary>
@@ -29,20 +31,21 @@ internal ConfigureBuilderContext(TKey strategyKey, string builderName, string st
     /// </summary>
     public string StrategyKeyString { get; }
 
-    internal Func<CancellationToken>? ReloadTokenProducer { get; private set; }
+    internal Func<Func<CancellationToken>>? ReloadTokenProducer { get; private set; }
 
     /// <summary>
     /// Enables dynamic reloading of the strategy retrieved from <see cref="ResilienceStrategyRegistry{TKey}"/>.
     /// </summary>
-    /// <param name="reloadTokenProducer">The producer of <see cref="CancellationToken"/> that is triggered when change occurs.</param>
+    /// <param name="tokenProducerFactory">The producer of <see cref="CancellationToken"/> that is triggered when change occurs.</param>
     /// <remarks>
-    /// The <paramref name="reloadTokenProducer"/> should always return a new <see cref="CancellationToken"/> instance when invoked otherwise
-    /// the reload infrastructure will stop listening for changes.
+    /// The <paramref name="tokenProducerFactory"/> should always return function that returns a new <see cref="CancellationToken"/> instance when invoked otherwise
+    /// the reload infrastructure will stop listening for changes. The <paramref name="tokenProducerFactory"/> is called only once for each streategy.
     /// </remarks>
-    public void EnableReloads(Func<CancellationToken> reloadTokenProducer)
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void EnableReloads(Func<Func<CancellationToken>> tokenProducerFactory)
     {
-        Guard.NotNull(reloadTokenProducer);
+        Guard.NotNull(tokenProducerFactory);
 
-        ReloadTokenProducer = reloadTokenProducer;
+        ReloadTokenProducer = tokenProducerFactory;
     }
 }

src/Polly.Core/Registry/ResilienceStrategyRegistry.cs

@@ -230,7 +230,7 @@ private static ResilienceStrategy CreateStrategy<TBuilder>(
 
         return new ReloadableResilienceStrategy(
             strategy,
-            context.ReloadTokenProducer,
+            context.ReloadTokenProducer(),
             () => factory().BuildStrategy(),
             TelemetryUtil.CreateTelemetry(
                 diagnosticSource,

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<TKey> registryCont
         RegistryContext = registryContext;
         ServiceProvider = serviceProvider;
         StrategyKey = registryContext.StrategyKey;
+        BuilderName = registryContext.BuilderName;
     }
 
+    /// <summary>
+    /// Gets the strategy key for the strategy being created.
+    /// </summary>
+    public string BuilderName { get; }
+
     /// <summary>
     /// Gets the strategy key for the strategy being created.
     /// </summary>
@@ -32,7 +39,7 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext<TKey> registryCont
     /// <summary>
     /// Gets the context that is used by the registry.
     /// </summary>
-    public ConfigureBuilderContext<TKey> RegistryContext { get; }
+    internal ConfigureBuilderContext<TKey> RegistryContext { get; }
 
     /// <summary>
     /// Enables dynamic reloading of the resilience strategy whenever the <typeparamref name="TOptions"/> options are changed.
@@ -48,10 +55,9 @@ internal AddResilienceStrategyContext(ConfigureBuilderContext<TKey> registryCont
     /// </remarks>
     public void EnableReloads<TOptions>(string? name = null)
     {
-        var registry = ServiceProvider.GetRequiredService<OptionsReloadHelperRegistry<TKey>>();
-        var helper = registry.Get<TOptions>(StrategyKey, name);
+        var monitor = ServiceProvider.GetRequiredService<IOptionsMonitor<TOptions>>();
 
-        RegistryContext.EnableReloads(helper.GetCancellationToken);
+        RegistryContext.EnableReloads(() => new OptionsReloadHelper<TOptions>(monitor, name ?? Options.DefaultName).GetCancellationToken);
     }
 
     /// <summary>

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<TKey>(this IServ
         services.Add(RegistryMarker<TKey>.ServiceDescriptor);
         services.AddResilienceStrategyBuilder();
         services.AddResilienceStrategyRegistry<TKey>();
-        services.TryAddSingleton<OptionsReloadHelperRegistry<TKey>>();
 
         services.TryAddSingleton(serviceProvider =>
         {

src/Polly.Extensions/Utils/OptionsReloadHelper.cs

@@ -2,12 +2,14 @@
 
 namespace Polly.Extensions.Utils;
 
-internal sealed class OptionsReloadHelper<T> : 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<T>
 {
-    private readonly IDisposable? _listener;
     private CancellationTokenSource _cancellation = new();
 
-    public OptionsReloadHelper(IOptionsMonitor<T> monitor, string name) => _listener = monitor.OnChange((_, changedNamed) =>
+    public OptionsReloadHelper(IOptionsMonitor<T> monitor, string name) => monitor.OnChange((_, changedNamed) =>
     {
         if (name == changedNamed)
         {
@@ -17,12 +19,6 @@ public OptionsReloadHelper(IOptionsMonitor<T> monitor, string name) => _listener
 
     public CancellationToken GetCancellationToken() => _cancellation.Token;
 
-    public void Dispose()
-    {
-        _cancellation.Dispose();
-        _listener?.Dispose();
-    }
-
     private void HandleChange()
     {
         var oldCancellation = _cancellation;

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<string>("dummy", (builder, context) =>
         {
             // this call enables dynamic reloads for the dummy strategy
-            context.EnableReloads(() => changeSource.Token);
+            context.EnableReloads(() => () => changeSource.Token);
 
             builder.AddRetry(new RetryStrategyOptions<string>
             {

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());

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<ResilienceStrategyProvider<string>>().GetStrategy("my-strategy");
         var context = ResilienceContext.Get();
-        var registry = serviceProvider.GetRequiredService<OptionsReloadHelperRegistry<string>>();
 
         // 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<OptionsReloadHelperRegistry<string>>();
-
-        registry.Get<ReloadableStrategy>("A", null);
-        registry.Get<ReloadableStrategy>("A", "dummy");
-        registry.Get<ReloadableStrategy>("B", null);
-        registry.Get<ReloadableStrategy>("B", "dummy");
-
-        registry.Count.Should().Be(4);
-
-        registry.Dispose();
-        registry.Count.Should().Be(0);
-    }
-
-    [Fact]
-    public void OptionsReloadHelper_Dispose_Ok()
-    {
-        var monitor = new Mock<IOptionsMonitor<ReloadableStrategyOptions>>();
-
-        using var helper = new OptionsReloadHelper<ReloadableStrategyOptions>(monitor.Object, string.Empty);
-
-        helper.Invoking(h => h.Dispose()).Should().NotThrow();
     }
 
     public class ReloadableStrategy : ResilienceStrategy

test/Polly.Extensions.Tests/Utils/OptionsReloadHelperTests.cs

@@ -15,7 +15,7 @@ public void Ctor_NamedOptions()
             .Setup(m => m.OnChange(It.IsAny<Action<string, string?>>()))
             .Returns(() => Mock.Of<IDisposable>());
 
-        using var helper = new OptionsReloadHelper<string>(monitor.Object, "name");
+        var helper = new OptionsReloadHelper<string>(monitor.Object, "name");
 
         monitor.VerifyAll();
     }