Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix finally in empty chains of responsibility #29

Merged
merged 12 commits into from
Nov 10, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
.Finally(input => Task.FromResult(input));

var resultTask = responsibilityChain.Execute(" Test\nwith spaces\n and new lines \n ");
var result = resultTask.Result;

Check warning on line 212 in src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs

View workflow job for this annotation

GitHub Actions / build

Test methods should not use blocking task operations, as they can cause deadlocks. Use an async test method and await instead. (https://xunit.net/xunit.analyzers/rules/xUnit1031)

Check warning on line 212 in src/PipelineNet.Tests/ChainsOfResponsibility/AsyncResponsibilityChainTests.cs

View workflow job for this annotation

GitHub Actions / build

Test methods should not use blocking task operations, as they can cause deadlocks. Use an async test method and await instead. (https://xunit.net/xunit.analyzers/rules/xUnit1031)

Assert.Equal("Test with spaces and new lines", result);
}
Expand Down Expand Up @@ -271,5 +271,18 @@
// The 'FinallyThrowIfCancellationRequested' should throw 'OperationCanceledException'.
await Assert.ThrowsAsync<OperationCanceledException>(() => responsibilityChain.Execute(exception, cancellationToken));
}

[Fact]
public async Task Execute_EmptyChainOfMiddlewareWithFinally_FinallyIsExecuted()
{
var responsibilityChain = new AsyncResponsibilityChain<Exception, bool>(new ActivatorMiddlewareResolver())
.Finally<FinallyThrow>();

// Creates an ArgumentNullException.
var exception = new ArgumentNullException();

// The 'FinallyThrow' should throw 'InvalidOperationException'.
await Assert.ThrowsAsync<InvalidOperationException>(() => responsibilityChain.Execute(exception));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,17 @@ public void Execute_ChainOfMiddlewareWithFinally_FinallyIsExecuted()
Assert.Throws<InvalidOperationException>(() => responsibilityChain.Execute(exception));
}

[Fact]
public void Execute_EmptyChainOfMiddlewareWithFinally_FinallyIsExecuted()
{
var responsibilityChain = new ResponsibilityChain<Exception, bool>(new ActivatorMiddlewareResolver())
.Finally<FinallyThrow>();

// Creates an ArgumentNullException.
var exception = new ArgumentNullException();

// The 'FinallyThrow' should throw 'InvalidOperationException'.
Assert.Throws<InvalidOperationException>(() => responsibilityChain.Execute(exception));
}
}
}
1 change: 0 additions & 1 deletion src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
// 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
Expand All @@ -132,7 +131,7 @@
Gender = Gender.Other
};

pipeline.Execute(personModel);

Check warning on line 134 in src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs

View workflow job for this annotation

GitHub Actions / build

Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.

Check warning on line 134 in src/PipelineNet.Tests/Pipelines/AsyncPipelineTests.cs

View workflow job for this annotation

GitHub Actions / build

Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.

// Check if the level of 'personModel' is 4, which is configured by 'PersonWithGenderProperty' middleware.
Assert.Equal(4, personModel.Level);
Expand Down
1 change: 0 additions & 1 deletion src/PipelineNet.Tests/Pipelines/PipelineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/PipelineNet/AsyncBaseMiddlewareFlow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ internal AsyncBaseMiddlewareFlow(IMiddlewareResolver middlewareResolver)
/// </summary>
private static readonly TypeInfo CancellableMiddlewareTypeInfo = typeof(TCancellableMiddleware).GetTypeInfo();


/// <summary>
/// Adds a new middleware type to the internal list of types.
/// Middleware will be executed in the same order they are added.
Expand Down
37 changes: 35 additions & 2 deletions src/PipelineNet/ChainsOfResponsibility/AsyncResponsibilityChain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ public class AsyncResponsibilityChain<TParameter, TReturn> : AsyncBaseMiddleware
/// </summary>
private static readonly TypeInfo CancellableFinallyTypeInfo = typeof(ICancellableAsyncFinally<TParameter, TReturn>).GetTypeInfo();

/// <summary>
/// Stores the shared instance of <see cref="DefaultFinally"/>.
/// </summary>
private static readonly IAsyncFinally<TParameter, TReturn> DefaultFinallyInstance = new DefaultFinally();

private Type _finallyType;
private Func<TParameter, Task<TReturn>> _finallyFunc;
Expand Down Expand Up @@ -92,7 +96,30 @@ public async Task<TReturn> Execute(TParameter parameter) =>
public async Task<TReturn> Execute(TParameter parameter, CancellationToken cancellationToken)
{
if (MiddlewareTypes.Count == 0)
return default(TReturn);
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this get merged, the behavior change should be noted in release notes or somewhere else.

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<TParameter, Task<TReturn>> next = null;
Expand Down Expand Up @@ -123,7 +150,7 @@ public async Task<TReturn> 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);
}
}

Expand Down Expand Up @@ -235,5 +262,11 @@ public IAsyncResponsibilityChain<TParameter, TReturn> Finally(Func<TParameter, T
this._finallyFunc = finallyFunc;
return this;
}

private class DefaultFinally : IAsyncFinally<TParameter, TReturn>
{
public async Task<TReturn> Finally(TParameter parameter) =>
await Task.FromResult(default(TReturn)).ConfigureAwait(false);
}
}
}
38 changes: 36 additions & 2 deletions src/PipelineNet/ChainsOfResponsibility/ResponsibilityChain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public class ResponsibilityChain<TParameter, TReturn> : BaseMiddlewareFlow<IMidd
/// </summary>
private static readonly TypeInfo FinallyTypeInfo = typeof(IFinally<TParameter, TReturn>).GetTypeInfo();

/// <summary>
/// Stores the shared instance of <see cref="DefaultFinally"/>.
/// </summary>
private static readonly IFinally<TParameter, TReturn> DefaultFinallyInstance = new DefaultFinally();

private Type _finallyType;
private Func<TParameter, TReturn> _finallyFunc;

Expand Down Expand Up @@ -113,7 +118,30 @@ public IResponsibilityChain<TParameter, TReturn> 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<TParameter, TReturn> next = null;
Expand Down Expand Up @@ -144,7 +172,7 @@ public TReturn Execute(TParameter parameter)
}
else
{
next = (p) => default(TReturn);
next = (p) => DefaultFinallyInstance.Finally(p);
}
}

Expand Down Expand Up @@ -172,5 +200,11 @@ private static TReturn RunFinally(MiddlewareResolverResult finallyResolverResult
var @finally = (IFinally<TParameter, TReturn>)finallyResolverResult.Middleware;
return @finally.Finally(parameter);
}

private class DefaultFinally : IFinally<TParameter, TReturn>
{
public TReturn Finally(TParameter parameter) =>
default(TReturn);
}
}
}