diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs index 627553faea1..a05a0edbfa6 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs @@ -5,7 +5,7 @@ namespace Polly.Registry; public sealed partial class ResiliencePipelineRegistry : ResiliencePipelineProvider where TKey : notnull { - private sealed class GenericRegistry : IDisposable, IAsyncDisposable + private sealed class GenericRegistry : IAsyncDisposable { private readonly Func> _activator; private readonly ConcurrentDictionary, ConfigureBuilderContext>> _builders; @@ -64,16 +64,6 @@ public ResiliencePipeline GetOrAdd(TKey key, Action, ConfigureBuilderContext> configure) => _builders.TryAdd(key, configure); - public void Dispose() - { - foreach (var strategy in _pipelines.Values) - { - strategy.DisposeHelper.ForceDispose(); - } - - _pipelines.Clear(); - } - public async ValueTask DisposeAsync() { foreach (var strategy in _pipelines.Values) diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs index 27f26fcc0e8..2017894c9ef 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs @@ -218,19 +218,7 @@ public bool TryAddBuilder(TKey key, Action - public void Dispose() - { - _disposed = true; - - var pipelines = _pipelines.Values.ToList(); - _pipelines.Clear(); - - var registries = _genericRegistry.Values.Cast().ToList(); - _genericRegistry.Clear(); - - pipelines.ForEach(p => p.DisposeHelper.ForceDispose()); - registries.ForEach(p => p.Dispose()); - } + public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult(); /// /// Disposes all resources that are held by the resilience pipelines created by this builder. diff --git a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs index 85548f1f70a..e5181d53c5f 100644 --- a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs +++ b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs @@ -6,28 +6,17 @@ internal abstract class BridgeComponentBase : PipelineComponent protected BridgeComponentBase(object strategy) => _strategy = strategy; - public override void Dispose() - { - if (_strategy is IDisposable disposable) - { - disposable.Dispose(); - } - else if (_strategy is IAsyncDisposable asyncDisposable) - { - asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); - } - } - public override ValueTask DisposeAsync() { if (_strategy is IAsyncDisposable asyncDisposable) { return asyncDisposable.DisposeAsync(); } - else + else if (_strategy is IDisposable disposable) { - Dispose(); - return default; + disposable.Dispose(); } + + return default; } } diff --git a/src/Polly.Core/Utils/Pipeline/ComponentDisposeHelper.cs b/src/Polly.Core/Utils/Pipeline/ComponentDisposeHelper.cs index e32a71b4e67..0be0cbf5775 100644 --- a/src/Polly.Core/Utils/Pipeline/ComponentDisposeHelper.cs +++ b/src/Polly.Core/Utils/Pipeline/ComponentDisposeHelper.cs @@ -1,6 +1,6 @@ namespace Polly.Utils.Pipeline; -internal sealed class ComponentDisposeHelper : IDisposable, IAsyncDisposable +internal sealed class ComponentDisposeHelper : IAsyncDisposable { private readonly PipelineComponent _component; private readonly DisposeBehavior _disposeBehavior; @@ -12,14 +12,6 @@ public ComponentDisposeHelper(PipelineComponent component, DisposeBehavior dispo _disposeBehavior = disposeBehavior; } - public void Dispose() - { - if (EnsureDisposable()) - { - ForceDispose(); - } - } - public ValueTask DisposeAsync() { if (EnsureDisposable()) @@ -38,14 +30,6 @@ public void EnsureNotDisposed() } } - public void ForceDispose() - { - _disposed = true; -#pragma warning disable S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods - _component.Dispose(); -#pragma warning restore S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods - } - public ValueTask ForceDisposeAsync() { _disposed = true; diff --git a/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs b/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs index e8a205af4c5..61162a94528 100644 --- a/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs +++ b/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs @@ -12,13 +12,6 @@ public ComponentWithDisposeCallbacks(PipelineComponent component, List c internal PipelineComponent Component { get; } - public override void Dispose() - { - ExecuteCallbacks(); - - Component.Dispose(); - } - public override ValueTask DisposeAsync() { ExecuteCallbacks(); diff --git a/src/Polly.Core/Utils/Pipeline/CompositeComponent.cs b/src/Polly.Core/Utils/Pipeline/CompositeComponent.cs index 8ec9515df29..abaa8692f4f 100644 --- a/src/Polly.Core/Utils/Pipeline/CompositeComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/CompositeComponent.cs @@ -61,14 +61,6 @@ public static PipelineComponent Create( public IReadOnlyList Components { get; } - public override void Dispose() - { - foreach (var component in Components) - { - component.Dispose(); - } - } - public override async ValueTask DisposeAsync() { foreach (var component in Components) @@ -155,10 +147,6 @@ internal override ValueTask> ExecuteCore( (Next, callback, state)); } - public override void Dispose() - { - } - public override ValueTask DisposeAsync() => default; } } diff --git a/src/Polly.Core/Utils/Pipeline/ExternalComponent.cs b/src/Polly.Core/Utils/Pipeline/ExternalComponent.cs index d4d6732f586..75e7e375a30 100644 --- a/src/Polly.Core/Utils/Pipeline/ExternalComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/ExternalComponent.cs @@ -15,11 +15,6 @@ internal override ValueTask> ExecuteCore( ResilienceContext context, TState state) => Component.ExecuteCore(callback, context, state); - public override void Dispose() - { - // don't dispose component that is external - } - public override ValueTask DisposeAsync() { // don't dispose component that is external diff --git a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs index d790f58ac52..09993a66712 100644 --- a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs @@ -6,7 +6,7 @@ /// /// The component of the pipeline can be either a strategy, a generic strategy or a whole pipeline. /// -internal abstract class PipelineComponent : IDisposable, IAsyncDisposable +internal abstract class PipelineComponent : IAsyncDisposable { public static PipelineComponent Empty { get; } = new NullComponent(); @@ -33,8 +33,6 @@ internal Outcome ExecuteCoreSync( (callback, state)).GetResult(); } - public abstract void Dispose(); - public abstract ValueTask DisposeAsync(); private class NullComponent : PipelineComponent @@ -42,10 +40,6 @@ private class NullComponent : PipelineComponent internal override ValueTask> ExecuteCore(Func>> callback, ResilienceContext context, TState state) => callback(context, state); - public override void Dispose() - { - } - public override ValueTask DisposeAsync() => default; } } diff --git a/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs b/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs index 3305d4fdc0f..ca88256e9e7 100644 --- a/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs @@ -8,6 +8,8 @@ internal sealed class ReloadableComponent : PipelineComponent { public const string ReloadFailedEvent = "ReloadFailed"; + public const string DisposeFailedEvent = "DisposeFailed"; + public const string OnReloadEvent = "OnReload"; private readonly Func _factory; @@ -37,12 +39,6 @@ internal override ValueTask> ExecuteCore( return Component.ExecuteCore(callback, context, state); } - public override void Dispose() - { - DisposeRegistration(); - Component.Dispose(); - } - public override ValueTask DisposeAsync() { DisposeRegistration(); @@ -60,14 +56,12 @@ private void TryRegisterOnReload() _registration = _tokenSource.Token.Register(() => { var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); - PipelineComponent previousComponent = Component; + var previousComponent = Component; try { _telemetry.Report(new(ResilienceEventSeverity.Information, OnReloadEvent), context, new OnReloadArguments()); (Component, _reloadTokens) = _factory(); - - previousComponent.Dispose(); } catch (Exception e) { @@ -78,9 +72,27 @@ private void TryRegisterOnReload() DisposeRegistration(); TryRegisterOnReload(); + + _ = DisposeDiscardedComponentSafeAsync(previousComponent); }); } + private async Task DisposeDiscardedComponentSafeAsync(PipelineComponent component) + { + var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: false); + + try + { + await component.DisposeAsync().ConfigureAwait(false); + } + catch (Exception e) + { + _telemetry.Report(new(ResilienceEventSeverity.Error, DisposeFailedEvent), context, Outcome.FromException(e), new DisposedFailedArguments(e)); + } + + ResilienceContextPool.Shared.Return(context); + } + #pragma warning disable S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods private void DisposeRegistration() { @@ -91,6 +103,8 @@ private void DisposeRegistration() internal record ReloadFailedArguments(Exception Exception); + internal record DisposedFailedArguments(Exception Exception); + internal record OnReloadArguments(); internal record Entry(PipelineComponent Component, List ReloadTokens); diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs index c9bb40cb017..a081a36655b 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs @@ -131,12 +131,10 @@ public async Task AddCircuitBreakers_WithIsolatedManualControl_ShouldBeIsolated( strategy2.Execute(() => { }); } - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(false, true)] - [InlineData(true, true)] + [InlineData(false)] + [InlineData(true)] [Theory] - public async Task DisposePipeline_EnsureCircuitBreakerDisposed(bool isAsync, bool attachManualControl) + public async Task DisposePipeline_EnsureCircuitBreakerDisposed(bool attachManualControl) { var manualControl = attachManualControl ? new CircuitBreakerManualControl() : null; var pipeline = new ResiliencePipelineBuilder() @@ -153,14 +151,7 @@ public async Task DisposePipeline_EnsureCircuitBreakerDisposed(bool isAsync, boo var strategy = (ResilienceStrategy)pipeline.GetPipelineDescriptor().FirstStrategy.StrategyInstance; - if (isAsync) - { - await pipeline.DisposeHelper.DisposeAsync(); - } - else - { - pipeline.DisposeHelper.Dispose(); - } + await pipeline.DisposeHelper.DisposeAsync(); strategy.AsPipeline().Invoking(s => s.Execute(() => 1)).Should().Throw(); diff --git a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs index ea89af60a89..547f1111cb7 100644 --- a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs @@ -424,22 +424,14 @@ public async Task Dispose_EnsureDisposed(bool isAsync) pipeline4.Invoking(p => p.Execute(() => "dummy")).Should().Throw(); } - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task DisposePipeline_NotAllowed(bool isAsync) + [Fact] + public async Task DisposePipeline_NotAllowed() { using var registry = CreateRegistry(); var pipeline = registry.GetOrAddPipeline(StrategyId.Create("A"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); - if (isAsync) - { - await pipeline.Invoking(p => p.DisposeHelper.DisposeAsync().AsTask()).Should().ThrowAsync(); - } - else - { - pipeline.Invoking(p => p.DisposeHelper.Dispose()).Should().Throw(); - } + await pipeline.Invoking(p => p.DisposeHelper.DisposeAsync().AsTask()).Should().ThrowAsync(); + } [InlineData(true)] diff --git a/test/Polly.Core.Tests/ResiliencePipelineBuilderTests.cs b/test/Polly.Core.Tests/ResiliencePipelineBuilderTests.cs index 942e56bd4d0..db0d9799558 100644 --- a/test/Polly.Core.Tests/ResiliencePipelineBuilderTests.cs +++ b/test/Polly.Core.Tests/ResiliencePipelineBuilderTests.cs @@ -232,10 +232,8 @@ public void AddPipeline_NullFactory_Throws() .Be("factory"); } - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task AddPipeline_EnsureNotDisposed(bool isAsync) + [Fact] + public async Task AddPipeline_EnsureNotDisposed() { var externalComponent = Substitute.For(); var externalBuilder = new ResiliencePipelineBuilder(); @@ -249,24 +247,13 @@ public async Task AddPipeline_EnsureNotDisposed(bool isAsync) .AddPipelineComponent(_ => internalComponent, new TestResilienceStrategyOptions()); var pipeline = builder.Build(); - if (isAsync) - { - await pipeline.DisposeHelper.DisposeAsync(); - await externalComponent.Received(0).DisposeAsync(); - await internalComponent.Received(1).DisposeAsync(); - } - else - { - pipeline.DisposeHelper.Dispose(); - externalComponent.Received(0).Dispose(); - internalComponent.Received(1).Dispose(); - } + await pipeline.DisposeHelper.DisposeAsync(); + await externalComponent.Received(0).DisposeAsync(); + await internalComponent.Received(1).DisposeAsync(); } - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task AddPipeline_Generic_EnsureNotDisposed(bool isAsync) + [Fact] + public async Task AddPipeline_Generic_EnsureNotDisposed() { var externalComponent = Substitute.For(); var externalBuilder = new ResiliencePipelineBuilder(); @@ -282,18 +269,9 @@ public async Task AddPipeline_Generic_EnsureNotDisposed(bool isAsync) pipeline.Execute(_ => string.Empty); - if (isAsync) - { - await pipeline.DisposeHelper.DisposeAsync(); - await externalComponent.Received(0).DisposeAsync(); - await internalComponent.Received(1).DisposeAsync(); - } - else - { - pipeline.DisposeHelper.Dispose(); - externalComponent.Received(0).Dispose(); - internalComponent.Received(1).Dispose(); - } + await pipeline.DisposeHelper.DisposeAsync(); + await externalComponent.Received(0).DisposeAsync(); + await internalComponent.Received(1).DisposeAsync(); } [Fact] diff --git a/test/Polly.Core.Tests/ResiliencePipelineTests.cs b/test/Polly.Core.Tests/ResiliencePipelineTests.cs index 616d6728128..a213563c4bf 100644 --- a/test/Polly.Core.Tests/ResiliencePipelineTests.cs +++ b/test/Polly.Core.Tests/ResiliencePipelineTests.cs @@ -11,10 +11,8 @@ public partial class ResiliencePipelineTests public static readonly CancellationToken CancellationToken = new CancellationTokenSource().Token; [Fact] - public async Task Dispose_NullPipeline_OK() + public async Task DisposeAsync_NullPipeline_OK() { - ResiliencePipeline.Empty.DisposeHelper.Dispose(); - ResiliencePipeline.Empty.DisposeHelper.Dispose(); await ResiliencePipeline.Empty.DisposeHelper.DisposeAsync(); await ResiliencePipeline.Empty.DisposeHelper.DisposeAsync(); @@ -22,10 +20,8 @@ public async Task Dispose_NullPipeline_OK() } [Fact] - public async Task Dispose_NullGenericPipeline_OK() + public async Task DisposeAsync_NullGenericPipeline_OK() { - ResiliencePipeline.Empty.DisposeHelper.Dispose(); - ResiliencePipeline.Empty.DisposeHelper.Dispose(); await ResiliencePipeline.Empty.DisposeHelper.DisposeAsync(); await ResiliencePipeline.Empty.DisposeHelper.DisposeAsync(); @@ -33,35 +29,17 @@ public async Task Dispose_NullGenericPipeline_OK() } [Fact] - public async Task Dispose_Reject_Throws() + public async Task DisposeAsync_Reject_Throws() { var component = Substitute.For(); var pipeline = new ResiliencePipeline(component, DisposeBehavior.Reject); - pipeline.Invoking(p => p.DisposeHelper.Dispose()) - .Should() - .Throw() - .WithMessage("Disposing this resilience pipeline is not allowed because it is owned by the pipeline registry."); - (await pipeline.Invoking(p => p.DisposeHelper.DisposeAsync().AsTask()) .Should() .ThrowAsync()) .WithMessage("Disposing this resilience pipeline is not allowed because it is owned by the pipeline registry."); } - [Fact] - public void Dispose_Allowed_Disposed() - { - var component = Substitute.For(); - var pipeline = new ResiliencePipeline(component, DisposeBehavior.Allow); - pipeline.DisposeHelper.Dispose(); - pipeline.DisposeHelper.Dispose(); - - pipeline.Invoking(p => p.Execute(() => { })).Should().Throw(); - - component.Received(1).Dispose(); - } - [Fact] public async Task DisposeAsync_Allowed_Disposed() { @@ -83,9 +61,9 @@ public void Null_Ok() } [Fact] - public void DebuggerProxy_Ok() + public async Task DebuggerProxy_Ok() { - using var pipeline = (CompositeComponent)PipelineComponentFactory.CreateComposite(new[] + await using var pipeline = (CompositeComponent)PipelineComponentFactory.CreateComposite(new[] { Substitute.For(), Substitute.For(), diff --git a/test/Polly.Core.Tests/Utils/Pipeline/BridgePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/BridgePipelineComponentTests.cs index 7769c6ff0ba..fd7c91d2545 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/BridgePipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/BridgePipelineComponentTests.cs @@ -66,45 +66,34 @@ public void Pipeline_TypeCheck_Ok() } #pragma warning disable S1944 // Invalid casts should be avoided - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task Dispose_EnsureStrategyDisposed(bool isAsync) + [Fact] + public async Task Dispose_EnsureStrategyDisposed() { var strategy = Substitute.For(); - await Dispose(PipelineComponentFactory.FromStrategy(strategy), isAsync); + await Dispose(PipelineComponentFactory.FromStrategy(strategy)); ((IDisposable)strategy).Received(1).Dispose(); strategy = Substitute.For(); - await Dispose(PipelineComponentFactory.FromStrategy(strategy), isAsync); + await Dispose(PipelineComponentFactory.FromStrategy(strategy)); await ((IAsyncDisposable)strategy).Received(1).DisposeAsync(); } - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task Dispose_Generic_EnsureStrategyDisposed(bool isAsync) + [Fact] + public async Task Dispose_Generic_EnsureStrategyDisposed() { var strategy = Substitute.For, IDisposable>(); - await Dispose(PipelineComponentFactory.FromStrategy(strategy), isAsync); + await Dispose(PipelineComponentFactory.FromStrategy(strategy)); ((IDisposable)strategy).Received(1).Dispose(); strategy = Substitute.For, IAsyncDisposable>(); - await Dispose(PipelineComponentFactory.FromStrategy(strategy), isAsync); + await Dispose(PipelineComponentFactory.FromStrategy(strategy)); await ((IAsyncDisposable)strategy).Received(1).DisposeAsync(); } #pragma warning restore S1944 // Invalid casts should be avoided - private static async Task Dispose(PipelineComponent component, bool isAsync) + private static async Task Dispose(PipelineComponent component) { - if (isAsync) - { - await component.DisposeAsync(); - } - else - { - component.Dispose(); - } + await component.DisposeAsync(); } private class Strategy : ResilienceStrategy diff --git a/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs index a745a54de48..7502ebad7dd 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs @@ -5,10 +5,8 @@ namespace Polly.Core.Tests.Utils.Pipeline; public class ComponentWithDisposeCallbacksTests { - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task Dispose_Ok(bool isAsync) + [Fact] + public async Task Dispose_Ok() { // Arrange var called1 = 0; @@ -23,20 +21,9 @@ public async Task Dispose_Ok(bool isAsync) var sut = new ComponentWithDisposeCallbacks(component, callbacks); // Act - if (isAsync) - { - await sut.DisposeAsync(); - await sut.DisposeAsync(); - await component.Received(2).DisposeAsync(); - } - else - { - sut.Dispose(); -#pragma warning disable S3966 // Objects should not be disposed more than once - sut.Dispose(); -#pragma warning restore S3966 // Objects should not be disposed more than once - component.Received(2).Dispose(); - } + await sut.DisposeAsync(); + await sut.DisposeAsync(); + await component.Received(2).DisposeAsync(); // Assert callbacks.Should().BeEmpty(); diff --git a/test/Polly.Core.Tests/Utils/Pipeline/CompositePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/CompositePipelineComponentTests.cs index 77529774d78..82b8c9dd8e3 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/CompositePipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/CompositePipelineComponentTests.cs @@ -126,21 +126,6 @@ public void ExecutePipeline_EnsureTelemetryArgumentsReported() _listener.GetArgs().Should().HaveCount(1); } - [Fact] - public void Dispose_EnsureInnerComponentsDisposed() - { - var a = Substitute.For(); - var b = Substitute.For(); - - var composite = CreateSut(new[] { a, b }); - - composite.FirstComponent.Dispose(); - composite.Dispose(); - - a.Received(1).Dispose(); - b.Received(1).Dispose(); - } - [Fact] public async Task DisposeAsync_EnsureInnerComponentsDisposed() { diff --git a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentTests.cs index 41d4f01d1c2..72c8198ba76 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentTests.cs @@ -9,7 +9,6 @@ public class PipelineComponentTests public async Task Dispose_Ok() { PipelineComponent.Empty.Should().NotBeNull(); - PipelineComponent.Empty.Dispose(); await PipelineComponent.Empty.DisposeAsync(); } } diff --git a/test/Polly.Core.Tests/Utils/Pipeline/ReloadablePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/ReloadablePipelineComponentTests.cs index 883e97cc9d1..6eeca2a04df 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/ReloadablePipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/ReloadablePipelineComponentTests.cs @@ -6,41 +6,29 @@ namespace Polly.Core.Tests.Utils.Pipeline; public class ReloadablePipelineComponentTests : IDisposable { - private readonly List> _events = new(); + private readonly FakeTelemetryListener _listener; private readonly ResilienceStrategyTelemetry _telemetry; + private readonly List _synchronousFlags = new(); private CancellationTokenSource _cancellationTokenSource; public ReloadablePipelineComponentTests() { - _telemetry = TestUtilities.CreateResilienceTelemetry(args => - { - args.Context.IsSynchronous.Should().BeTrue(); - args.Context.IsVoid.Should().BeTrue(); - _events.Add(args); - }); - + _listener = new FakeTelemetryListener(args => _synchronousFlags.Add(args.Context.IsSynchronous)); + _telemetry = TestUtilities.CreateResilienceTelemetry(_listener); _cancellationTokenSource = new CancellationTokenSource(); } [Fact] - public void Ctor_Ok() + public async Task Ctor_Ok() { var component = Substitute.For(); - using var sut = CreateSut(component); + await using var sut = CreateSut(component); sut.Component.Should().Be(component); ReloadableComponent.ReloadFailedEvent.Should().Be("ReloadFailed"); } - [Fact] - public void Dispose_ComponentDisposed() - { - var component = Substitute.For(); - CreateSut(component).Dispose(); - component.Received(1).Dispose(); - } - [Fact] public async Task DisposeAsync_ComponentDisposed() { @@ -50,58 +38,94 @@ public async Task DisposeAsync_ComponentDisposed() } [Fact] - public void ChangeTriggered_StrategyReloaded() + public async Task ChangeTriggered_StrategyReloaded() { var component = Substitute.For(); - using var sut = CreateSut(component); + await using var sut = CreateSut(component); sut.Component.Should().Be(component); _cancellationTokenSource.Cancel(); sut.Component.Should().NotBe(component); - _events.Where(e => e.Event.EventName == "ReloadFailed").Should().HaveCount(0); - _events.Where(e => e.Event.EventName == "OnReload").Should().HaveCount(1); + _listener.Events.Where(e => e.Event.EventName == "ReloadFailed").Should().HaveCount(0); + _listener.Events.Where(e => e.Event.EventName == "OnReload").Should().HaveCount(1); } [Fact] - public void ChangeTriggered_EnsureOldStrategyDisposed() + public async Task ChangeTriggered_EnsureOldStrategyDisposed() { var component = Substitute.For(); - using var sut = CreateSut(component, () => new(Substitute.For(), new List())); + await using var sut = CreateSut(component, () => new(Substitute.For(), new List())); for (var i = 0; i < 10; i++) { var src = _cancellationTokenSource; _cancellationTokenSource = new CancellationTokenSource(); src.Cancel(); - component.Received(1).Dispose(); - sut.Component.Received(0).Dispose(); + await component.Received(1).DisposeAsync(); + await sut.Component.Received(0).DisposeAsync(); } } [Fact] - public void ChangeTriggered_FactoryError_LastStrategyUsedAndErrorReported() + public async Task ChangeTriggered_FactoryError_LastStrategyUsedAndErrorReported() { var component = Substitute.For(); - using var sut = CreateSut(component, () => throw new InvalidOperationException()); + await using var sut = CreateSut(component, () => throw new InvalidOperationException()); _cancellationTokenSource.Cancel(); sut.Component.Should().Be(component); - _events.Should().HaveCount(2); + _listener.Events.Should().HaveCount(2); - _events[0] + _listener.Events[0] .Arguments .Should() .BeOfType(); - var args = _events[1] + var args = _listener.Events[1] .Arguments .Should() .BeOfType() .Subject; args.Exception.Should().BeOfType(); + + _synchronousFlags.Should().AllSatisfy(f => f.Should().BeTrue()); + } + + [Fact] + public async Task DisposeError_EnsureReported() + { + var component = Substitute.For(); +#pragma warning disable CA2012 // Use ValueTasks correctly + component + .When(c => c.DisposeAsync()) + .Do(_ => throw new InvalidOperationException()); +#pragma warning restore CA2012 // Use ValueTasks correctly + + await using var sut = CreateSut(component); + + _cancellationTokenSource.Cancel(); + + _listener.Events.Should().HaveCount(2); + + _listener.Events[0] + .Arguments + .Should() + .BeOfType(); + + var args = _listener.Events[1] + .Arguments + .Should() + .BeOfType() + .Subject; + + args.Exception.Should().BeOfType(); + + _synchronousFlags.Should().HaveCount(2); + _synchronousFlags[0].Should().BeTrue(); + _synchronousFlags[1].Should().BeFalse(); } private ReloadableComponent CreateSut(PipelineComponent? initial = null, Func? factory = null) diff --git a/test/Polly.Extensions.Tests/DisposablePipelineTests.cs b/test/Polly.Extensions.Tests/DisposablePipelineTests.cs index 91b116a1aa3..985fd1f2ecb 100644 --- a/test/Polly.Extensions.Tests/DisposablePipelineTests.cs +++ b/test/Polly.Extensions.Tests/DisposablePipelineTests.cs @@ -44,7 +44,7 @@ public void DisposePipeline_EnsureLinkedResourcesDisposedToo() } [Fact] - public void DisposePipeline_EnsureExternalPipelineNotDisposed() + public async Task DisposePipeline_EnsureExternalPipelineNotDisposed() { var registry1 = new ResiliencePipelineRegistry(); var pipeline1 = registry1.GetOrAddPipeline("my-pipeline", builder => builder.AddConcurrencyLimiter(1)); @@ -56,13 +56,13 @@ public void DisposePipeline_EnsureExternalPipelineNotDisposed() .AddPipeline(pipeline2)); pipeline3.Execute(() => string.Empty); - registry2.Dispose(); + await registry2.DisposeAsync(); pipeline3.Invoking(p => p.Execute(() => string.Empty)).Should().Throw(); pipeline1.Execute(() => { }); pipeline2.Execute(() => string.Empty); - registry1.Dispose(); + await registry1.DisposeAsync(); pipeline1.Invoking(p => p.Execute(() => { })).Should().Throw(); pipeline2.Invoking(p => p.Execute(() => string.Empty)).Should().Throw(); diff --git a/test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTEsts.cs b/test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTEsts.cs index a362da31640..047b7da20ee 100644 --- a/test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTEsts.cs +++ b/test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTEsts.cs @@ -8,7 +8,7 @@ namespace Polly.Extensions.Tests.Registry; public class ConfigureBuilderContextExtensionsTests { [Fact] - public void ConfigureReloads_MonitorRegistrationReturnsNull_DoesNotThrow() + public async Task ConfigureReloads_MonitorRegistrationReturnsNull_DoesNotThrow() { var monitor = Substitute.For>(); monitor.OnChange(default!).ReturnsNullForAnyArgs(); @@ -22,7 +22,7 @@ public void ConfigureReloads_MonitorRegistrationReturnsNull_DoesNotThrow() context.EnableReloads(monitor); }); - registry.Dispose(); + await registry.DisposeAsync(); listener.Events.Should().BeEmpty(); } diff --git a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs index 33b53caf8b9..9cf0f4f404a 100644 --- a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs +++ b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs @@ -146,24 +146,15 @@ public void AddRateLimiter_Options_Ok() .BeOfType(); } - [InlineData(true)] - [InlineData(false)] - [Theory] - public async Task DisposeRegistry_EnsureRateLimiterDisposed(bool isAsync) + [Fact] + public async Task DisposeRegistry_EnsureRateLimiterDisposed() { var registry = new ResiliencePipelineRegistry(); var pipeline = registry.GetOrAddPipeline("limiter", p => p.AddRateLimiter(new RateLimiterStrategyOptions())); var strategy = (RateLimiterResilienceStrategy)pipeline.GetPipelineDescriptor().FirstStrategy.StrategyInstance; - if (isAsync) - { - await registry.DisposeAsync(); - } - else - { - registry.Dispose(); - } + await registry.DisposeAsync(); strategy.AsPipeline().Invoking(p => p.Execute(() => { })).Should().Throw(); } diff --git a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs index be04526b1cb..900e35d3261 100644 --- a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs +++ b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs @@ -109,11 +109,11 @@ public void GetPipelineDescriptor_SingleStrategy_Ok() } [Fact] - public void GetPipelineDescriptor_Reloadable_Ok() + public async Task GetPipelineDescriptor_Reloadable_Ok() { // arrange using var source = new CancellationTokenSource(); - using var registry = new ResiliencePipelineRegistry(); + await using var registry = new ResiliencePipelineRegistry(); var strategy = registry.GetOrAddPipeline("dummy", (builder, context) => { context.OnPipelineDisposed(() => { });