From a43043d7352a534c8f36345e8d5feb01431e2a13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20St=C4=99pie=C5=84?= <62397363+mariusz96@users.noreply.github.com> Date: Sun, 10 Nov 2024 15:51:28 +0100 Subject: [PATCH] Fix finally in empty chains of responsibility (#29) * replace IsDisposable with Dispose * don't check the type of TFinally twice * extract methods * update variable names * udpate PipelineNet.ServiceProvider csproj * udpate readme * extract locals * fix bug in async middleware disposal * fix test name * execute finally for empty chains of responsibility * extract default finally classes --- .../AsyncResponsibilityChainTests.cs | 13 +++++++ .../ResponsibilityChainTests.cs | 12 ++++++ .../Pipelines/AsyncPipelineTests.cs | 1 - .../Pipelines/PipelineTests.cs | 1 - src/PipelineNet/AsyncBaseMiddlewareFlow.cs | 1 - .../AsyncResponsibilityChain.cs | 37 +++++++++++++++++- .../ResponsibilityChain.cs | 38 ++++++++++++++++++- 7 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs b/src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs index 38f1f72..ffc310d 100644 --- a/src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs +++ b/src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs @@ -271,5 +271,18 @@ public async Task Execute_ChainOfMiddlewareWithCancellableFinally_CancellableFin // The 'FinallyThrowIfCancellationRequested' should throw 'OperationCanceledException'. await Assert.ThrowsAsync(() => responsibilityChain.Execute(exception, cancellationToken)); } + + [Fact] + public async Task Execute_EmptyChainOfMiddlewareWithFinally_FinallyIsExecuted() + { + var responsibilityChain = new AsyncResponsibilityChain(new ActivatorMiddlewareResolver()) + .Finally(); + + // Creates an ArgumentNullException. + var exception = new ArgumentNullException(); + + // The 'FinallyThrow' should throw 'InvalidOperationException'. + await Assert.ThrowsAsync(() => responsibilityChain.Execute(exception)); + } } } diff --git a/src/PipelineNet.Tests/ChainsOfResponsibility/ResponsibilityChainTests.cs b/src/PipelineNet.Tests/ChainsOfResponsibility/ResponsibilityChainTests.cs index 2fd40f6..d42f853 100644 --- a/src/PipelineNet.Tests/ChainsOfResponsibility/ResponsibilityChainTests.cs +++ b/src/PipelineNet.Tests/ChainsOfResponsibility/ResponsibilityChainTests.cs @@ -169,5 +169,17 @@ public void Execute_ChainOfMiddlewareWithFinally_FinallyIsExecuted() Assert.Throws(() => responsibilityChain.Execute(exception)); } + [Fact] + public void Execute_EmptyChainOfMiddlewareWithFinally_FinallyIsExecuted() + { + var responsibilityChain = new ResponsibilityChain(new ActivatorMiddlewareResolver()) + .Finally(); + + // Creates an ArgumentNullException. + var exception = new ArgumentNullException(); + + // The 'FinallyThrow' should throw 'InvalidOperationException'. + Assert.Throws(() => responsibilityChain.Execute(exception)); + } } } diff --git a/src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs b/src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs index 321a129..32b2049 100644 --- a/src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs +++ b/src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs @@ -123,7 +123,6 @@ public async Task Execute_RunSamePipelineTwice_SuccessfullyExecute() // Check if the level of 'personModel' is 3, which is configured by 'PersonWithEmailName' middleware. Assert.Equal(3, personModel.Level); - // Creates a new instance with a 'Gender' property. The 'PersonWithGenderProperty' // middleware should be the last one to be executed. personModel = new PersonModel diff --git a/src/PipelineNet.Tests/Pipelines/PipelineTests.cs b/src/PipelineNet.Tests/Pipelines/PipelineTests.cs index 2a05850..6823084 100644 --- a/src/PipelineNet.Tests/Pipelines/PipelineTests.cs +++ b/src/PipelineNet.Tests/Pipelines/PipelineTests.cs @@ -110,7 +110,6 @@ public void Execute_RunSamePipelineTwice_SuccessfullyExecute() // Check if the level of 'personModel' is 3, which is configured by 'PersonWithEmailName' middleware. Assert.Equal(3, personModel.Level); - // Creates a new instance with a 'Gender' property. The 'PersonWithGenderProperty' // middleware should be the last one to be executed. personModel = new PersonModel diff --git a/src/PipelineNet/AsyncBaseMiddlewareFlow.cs b/src/PipelineNet/AsyncBaseMiddlewareFlow.cs index 518bea7..4fa8ce4 100644 --- a/src/PipelineNet/AsyncBaseMiddlewareFlow.cs +++ b/src/PipelineNet/AsyncBaseMiddlewareFlow.cs @@ -40,7 +40,6 @@ internal AsyncBaseMiddlewareFlow(IMiddlewareResolver middlewareResolver) /// private static readonly TypeInfo CancellableMiddlewareTypeInfo = typeof(TCancellableMiddleware).GetTypeInfo(); - /// /// Adds a new middleware type to the internal list of types. /// Middleware will be executed in the same order they are added. diff --git a/src/PipelineNet/ChainsOfResponsibility/AsyncResponsibilityChain.cs b/src/PipelineNet/ChainsOfResponsibility/AsyncResponsibilityChain.cs index 78ba345..56257fb 100644 --- a/src/PipelineNet/ChainsOfResponsibility/AsyncResponsibilityChain.cs +++ b/src/PipelineNet/ChainsOfResponsibility/AsyncResponsibilityChain.cs @@ -26,6 +26,10 @@ public class AsyncResponsibilityChain : AsyncBaseMiddleware /// private static readonly TypeInfo CancellableFinallyTypeInfo = typeof(ICancellableAsyncFinally).GetTypeInfo(); + /// + /// Stores the shared instance of . + /// + private static readonly IAsyncFinally DefaultFinallyInstance = new DefaultFinally(); private Type _finallyType; private Func> _finallyFunc; @@ -92,7 +96,30 @@ public async Task Execute(TParameter parameter) => public async Task Execute(TParameter parameter, CancellationToken cancellationToken) { if (MiddlewareTypes.Count == 0) - return default(TReturn); + { + MiddlewareResolverResult finallyResolverResult = null; + try + { + if (_finallyType != null) + { + finallyResolverResult = MiddlewareResolver.Resolve(_finallyType); + EnsureMiddlewareNotNull(finallyResolverResult, _finallyType); + return await RunFinallyAsync(finallyResolverResult, parameter, cancellationToken).ConfigureAwait(false); + } + else if (_finallyFunc != null) + { + return await _finallyFunc(parameter).ConfigureAwait(false); + } + else + { + return await DefaultFinallyInstance.Finally(parameter).ConfigureAwait(false); + } + } + finally + { + await DisposeMiddlewareAsync(finallyResolverResult).ConfigureAwait(false); + } + } int index = 0; Func> next = null; @@ -123,7 +150,7 @@ public async Task Execute(TParameter parameter, CancellationToken cance } else { - next = async (p) => await Task.FromResult(default(TReturn)).ConfigureAwait(false); + next = async (p) => await DefaultFinallyInstance.Finally(p).ConfigureAwait(false); } } @@ -235,5 +262,11 @@ public IAsyncResponsibilityChain Finally(Func + { + public async Task Finally(TParameter parameter) => + await Task.FromResult(default(TReturn)).ConfigureAwait(false); + } } } diff --git a/src/PipelineNet/ChainsOfResponsibility/ResponsibilityChain.cs b/src/PipelineNet/ChainsOfResponsibility/ResponsibilityChain.cs index ee912d2..bb1e132 100644 --- a/src/PipelineNet/ChainsOfResponsibility/ResponsibilityChain.cs +++ b/src/PipelineNet/ChainsOfResponsibility/ResponsibilityChain.cs @@ -19,6 +19,11 @@ public class ResponsibilityChain : BaseMiddlewareFlow private static readonly TypeInfo FinallyTypeInfo = typeof(IFinally).GetTypeInfo(); + /// + /// Stores the shared instance of . + /// + private static readonly IFinally DefaultFinallyInstance = new DefaultFinally(); + private Type _finallyType; private Func _finallyFunc; @@ -113,7 +118,30 @@ public IResponsibilityChain Chain(Type middlewareType) public TReturn Execute(TParameter parameter) { if (MiddlewareTypes.Count == 0) - return default(TReturn); + { + MiddlewareResolverResult finallyResolverResult = null; + try + { + if (_finallyType != null) + { + finallyResolverResult = MiddlewareResolver.Resolve(_finallyType); + EnsureMiddlewareNotNull(finallyResolverResult, _finallyType); + return RunFinally(finallyResolverResult, parameter); + } + else if (_finallyFunc != null) + { + return _finallyFunc(parameter); + } + else + { + return DefaultFinallyInstance.Finally(parameter); + } + } + finally + { + DisposeMiddleware(finallyResolverResult); + } + } int index = 0; Func next = null; @@ -144,7 +172,7 @@ public TReturn Execute(TParameter parameter) } else { - next = (p) => default(TReturn); + next = (p) => DefaultFinallyInstance.Finally(p); } } @@ -172,5 +200,11 @@ private static TReturn RunFinally(MiddlewareResolverResult finallyResolverResult var @finally = (IFinally)finallyResolverResult.Middleware; return @finally.Finally(parameter); } + + private class DefaultFinally : IFinally + { + public TReturn Finally(TParameter parameter) => + default(TReturn); + } } }