[Fixes #5175] Async resource filters' short circuited result getting executed more than once.

This commit is contained in:
Kiran Challa 2016-08-23 14:02:14 -07:00
parent 7036e2b0f5
commit ece8f33a65
2 changed files with 117 additions and 49 deletions

View File

@ -357,7 +357,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
goto case State.ExceptionBegin; goto case State.ExceptionBegin;
} }
} }
case State.ResourceAsyncBegin: case State.ResourceAsyncBegin:
{ {
Debug.Assert(state != null); Debug.Assert(state != null);
@ -392,13 +392,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Canceled = true, Canceled = true,
Result = _resourceExecutingContext.Result, Result = _resourceExecutingContext.Result,
}; };
}
_diagnosticSource.AfterOnResourceExecution(_resourceExecutedContext, filter); _diagnosticSource.AfterOnResourceExecution(_resourceExecutedContext, filter);
if (_resourceExecutingContext.Result != null) // A filter could complete a Task without setting a result
{ if (_resourceExecutingContext.Result != null)
goto case State.ResourceShortCircuit; {
goto case State.ResourceShortCircuit;
}
} }
goto case State.ResourceEnd; goto case State.ResourceEnd;
@ -1222,7 +1223,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
arguments, arguments,
controller); controller);
logger.ActionMethodExecuting(controllerContext, orderedArguments); logger.ActionMethodExecuting(controllerContext, orderedArguments);
var returnType = executor.MethodReturnType; var returnType = executor.MethodReturnType;
if (returnType == typeof(void)) if (returnType == typeof(void))
{ {

View File

@ -82,31 +82,41 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{ {
// Arrange // Arrange
Exception exception = null; Exception exception = null;
IActionResult result = null; IActionResult resultFromAction = null;
var expected = new Mock<IActionResult>(MockBehavior.Strict);
expected
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
var filter = new Mock<IExceptionFilter>(MockBehavior.Strict); var filter1 = new Mock<IExceptionFilter>(MockBehavior.Strict);
filter filter1
.Setup(f => f.OnException(It.IsAny<ExceptionContext>()))
.Verifiable();
var filter2 = new Mock<IExceptionFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnException(It.IsAny<ExceptionContext>())) .Setup(f => f.OnException(It.IsAny<ExceptionContext>()))
.Callback<ExceptionContext>(context => .Callback<ExceptionContext>(context =>
{ {
exception = context.Exception; exception = context.Exception;
result = context.Result; resultFromAction = context.Result;
// Handle the exception // Handle the exception
context.Result = new EmptyResult(); context.Result = expected.Object;
}) })
.Verifiable(); .Verifiable();
var invoker = CreateInvoker(filter.Object, actionThrows: true); var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object }, actionThrows: true);
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
filter.Verify(f => f.OnException(It.IsAny<ExceptionContext>()), Times.Once()); expected.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
filter2.Verify(f => f.OnException(It.IsAny<ExceptionContext>()), Times.Once());
Assert.Same(_actionException, exception); Assert.Same(_actionException, exception);
Assert.Null(result); Assert.Null(resultFromAction);
} }
[Fact] [Fact]
@ -114,34 +124,45 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{ {
// Arrange // Arrange
Exception exception = null; Exception exception = null;
IActionResult result = null; IActionResult resultFromAction = null;
var expected = new Mock<IActionResult>(MockBehavior.Strict);
expected
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
var filter = new Mock<IAsyncExceptionFilter>(MockBehavior.Strict); var filter1 = new Mock<IAsyncExceptionFilter>(MockBehavior.Strict);
filter filter1
.Setup(f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()))
.Returns<ExceptionContext>((context) => Task.FromResult(true))
.Verifiable();
var filter2 = new Mock<IAsyncExceptionFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnExceptionAsync(It.IsAny<ExceptionContext>())) .Setup(f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()))
.Callback<ExceptionContext>(context => .Callback<ExceptionContext>(context =>
{ {
exception = context.Exception; exception = context.Exception;
result = context.Result; resultFromAction = context.Result;
// Handle the exception // Handle the exception
context.Result = new EmptyResult(); context.Result = expected.Object;
}) })
.Returns<ExceptionContext>((context) => Task.FromResult(true)) .Returns<ExceptionContext>((context) => Task.FromResult(true))
.Verifiable(); .Verifiable();
var invoker = CreateInvoker(filter.Object, actionThrows: true); var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object }, actionThrows: true);
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
filter.Verify( expected.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
filter2.Verify(
f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()), f => f.OnExceptionAsync(It.IsAny<ExceptionContext>()),
Times.Once()); Times.Once());
Assert.Same(_actionException, exception); Assert.Same(_actionException, exception);
Assert.Null(result); Assert.Null(resultFromAction);
} }
[Fact] [Fact]
@ -340,22 +361,33 @@ namespace Microsoft.AspNetCore.Mvc.Internal
public async Task InvokeAction_InvokesAuthorizationFilter_ShortCircuit() public async Task InvokeAction_InvokesAuthorizationFilter_ShortCircuit()
{ {
// Arrange // Arrange
var challenge = new Mock<IActionResult>(MockBehavior.Loose).Object; var challenge = new Mock<IActionResult>(MockBehavior.Strict);
challenge
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
var filter1 = new Mock<IAuthorizationFilter>(MockBehavior.Strict); var filter1 = new Mock<IAuthorizationFilter>(MockBehavior.Strict);
filter1 filter1
.Setup(f => f.OnAuthorization(It.IsAny<AuthorizationFilterContext>())) .Setup(f => f.OnAuthorization(It.IsAny<AuthorizationFilterContext>()))
.Callback<AuthorizationFilterContext>(c => c.Result = challenge) .Callback<AuthorizationFilterContext>(c => Task.FromResult(true))
.Verifiable(); .Verifiable();
var filter2 = new Mock<IAuthorizationFilter>(MockBehavior.Strict); var filter2 = new Mock<IAuthorizationFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnAuthorization(It.IsAny<AuthorizationFilterContext>()))
.Callback<AuthorizationFilterContext>(c => c.Result = challenge.Object)
.Verifiable();
var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object }); var filter3 = new Mock<IAuthorizationFilter>(MockBehavior.Strict);
var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object, filter3.Object });
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
challenge.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
filter1.Verify(f => f.OnAuthorization(It.IsAny<AuthorizationFilterContext>()), Times.Once()); filter1.Verify(f => f.OnAuthorization(It.IsAny<AuthorizationFilterContext>()), Times.Once());
Assert.False(invoker.ControllerFactory.CreateCalled); Assert.False(invoker.ControllerFactory.CreateCalled);
} }
@ -364,26 +396,39 @@ namespace Microsoft.AspNetCore.Mvc.Internal
public async Task InvokeAction_InvokesAsyncAuthorizationFilter_ShortCircuit() public async Task InvokeAction_InvokesAsyncAuthorizationFilter_ShortCircuit()
{ {
// Arrange // Arrange
var challenge = new Mock<IActionResult>(MockBehavior.Loose).Object; var challenge = new Mock<IActionResult>(MockBehavior.Strict);
challenge
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
var filter1 = new Mock<IAsyncAuthorizationFilter>(MockBehavior.Strict); var filter1 = new Mock<IAsyncAuthorizationFilter>(MockBehavior.Strict);
filter1 filter1
.Setup(f => f.OnAuthorizationAsync(It.IsAny<AuthorizationFilterContext>())) .Setup(f => f.OnAuthorizationAsync(It.IsAny<AuthorizationFilterContext>()))
.Returns<AuthorizationFilterContext>((context) => .Returns<AuthorizationFilterContext>((context) =>
{ {
context.Result = challenge;
return Task.FromResult(true); return Task.FromResult(true);
}) })
.Verifiable(); .Verifiable();
var filter2 = new Mock<IAuthorizationFilter>(MockBehavior.Strict); var filter2 = new Mock<IAsyncAuthorizationFilter>(MockBehavior.Strict);
filter2
.Setup(f => f.OnAuthorizationAsync(It.IsAny<AuthorizationFilterContext>()))
.Returns<AuthorizationFilterContext>((context) =>
{
context.Result = challenge.Object;
return Task.FromResult(true);
});
var invoker = CreateInvoker(new IFilterMetadata[] { filter1.Object, filter2.Object }); var filter3 = new Mock<IAuthorizationFilter>(MockBehavior.Strict);
var invoker = CreateInvoker(new IFilterMetadata[] { filter1.Object, filter2.Object, filter3.Object });
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
challenge.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
filter1.Verify( filter1.Verify(
f => f.OnAuthorizationAsync(It.IsAny<AuthorizationFilterContext>()), f => f.OnAuthorizationAsync(It.IsAny<AuthorizationFilterContext>()),
Times.Once()); Times.Once());
@ -392,7 +437,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
} }
[Fact] [Fact]
public async Task InvokeAction_ExceptionInAuthorizationFilterCannotBeHandledByOtherFilters() public async Task InvokeAction_ExceptionInAuthorizationFilter_CannotBeHandledByOtherFilters()
{ {
// Arrange // Arrange
var expected = new InvalidCastException(); var expected = new InvalidCastException();
@ -524,7 +569,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal
public async Task InvokeAction_InvokesActionFilter_ShortCircuit() public async Task InvokeAction_InvokesActionFilter_ShortCircuit()
{ {
// Arrange // Arrange
var result = new EmptyResult(); var result = new Mock<IActionResult>(MockBehavior.Strict);
result
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
ActionExecutedContext context = null; ActionExecutedContext context = null;
@ -538,7 +587,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
var actionFilter2 = new Mock<IActionFilter>(MockBehavior.Strict); var actionFilter2 = new Mock<IActionFilter>(MockBehavior.Strict);
actionFilter2 actionFilter2
.Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>())) .Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()))
.Callback<ActionExecutingContext>(c => c.Result = result) .Callback<ActionExecutingContext>(c => c.Result = result.Object)
.Verifiable(); .Verifiable();
var actionFilter3 = new Mock<IActionFilter>(MockBehavior.Strict); var actionFilter3 = new Mock<IActionFilter>(MockBehavior.Strict);
@ -559,23 +608,29 @@ namespace Microsoft.AspNetCore.Mvc.Internal
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
result.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
actionFilter1.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once()); actionFilter1.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once());
actionFilter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once()); actionFilter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once());
actionFilter2.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once()); actionFilter2.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once());
actionFilter2.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Never());
resultFilter.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once()); resultFilter.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
resultFilter.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once()); resultFilter.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
Assert.True(context.Canceled); Assert.True(context.Canceled);
Assert.Same(context.Result, result); Assert.Same(context.Result, result.Object);
} }
[Fact] [Fact]
public async Task InvokeAction_InvokesAsyncActionFilter_ShortCircuit_WithResult() public async Task InvokeAction_InvokesAsyncActionFilter_ShortCircuit_WithResult()
{ {
// Arrange // Arrange
var result = new EmptyResult(); var result = new Mock<IActionResult>(MockBehavior.Strict);
result
.Setup(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()))
.Returns(Task.FromResult(true))
.Verifiable();
ActionExecutedContext context = null; ActionExecutedContext context = null;
@ -592,29 +647,34 @@ namespace Microsoft.AspNetCore.Mvc.Internal
.Returns<ActionExecutingContext, ActionExecutionDelegate>((c, next) => .Returns<ActionExecutingContext, ActionExecutionDelegate>((c, next) =>
{ {
// Notice we're not calling next // Notice we're not calling next
c.Result = result; c.Result = result.Object;
return Task.FromResult(true); return Task.FromResult(true);
}) })
.Verifiable(); .Verifiable();
var actionFilter3 = new Mock<IActionFilter>(MockBehavior.Strict); var actionFilter3 = new Mock<IActionFilter>(MockBehavior.Strict);
var resultFilter = new Mock<IResultFilter>(MockBehavior.Strict); var resultFilter1 = new Mock<IResultFilter>(MockBehavior.Strict);
resultFilter.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>())).Verifiable(); resultFilter1.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>())).Verifiable();
resultFilter.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>())).Verifiable(); resultFilter1.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>())).Verifiable();
var resultFilter2 = new Mock<IResultFilter>(MockBehavior.Strict);
resultFilter2.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>())).Verifiable();
resultFilter2.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>())).Verifiable();
var invoker = CreateInvoker(new IFilterMetadata[] var invoker = CreateInvoker(new IFilterMetadata[]
{ {
actionFilter1.Object, actionFilter1.Object,
actionFilter2.Object, actionFilter2.Object,
actionFilter3.Object, actionFilter3.Object,
resultFilter.Object, resultFilter1.Object,
resultFilter2.Object,
}); });
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
result.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
actionFilter1.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once()); actionFilter1.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once());
actionFilter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once()); actionFilter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once());
@ -622,11 +682,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
f => f.OnActionExecutionAsync(It.IsAny<ActionExecutingContext>(), It.IsAny<ActionExecutionDelegate>()), f => f.OnActionExecutionAsync(It.IsAny<ActionExecutingContext>(), It.IsAny<ActionExecutionDelegate>()),
Times.Once()); Times.Once());
resultFilter.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once()); resultFilter1.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
resultFilter.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once()); resultFilter1.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
resultFilter2.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
resultFilter2.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
Assert.True(context.Canceled); Assert.True(context.Canceled);
Assert.Same(context.Result, result); Assert.Same(context.Result, result.Object);
} }
[Fact] [Fact]
@ -766,10 +828,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal
.Verifiable(); .Verifiable();
var filter2 = new Mock<IActionFilter>(MockBehavior.Strict); var filter2 = new Mock<IActionFilter>(MockBehavior.Strict);
filter2.Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>())).Verifiable();
filter2 filter2
.Setup(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>())) .Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()))
.Callback<ActionExecutedContext>(c => { throw exception; }) .Callback<ActionExecutingContext>(c => { throw exception; })
.Verifiable(); .Verifiable();
var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object }); var invoker = CreateInvoker(new[] { filter1.Object, filter2.Object });
@ -782,6 +843,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
filter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once()); filter1.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once());
filter2.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once()); filter2.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once());
filter2.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Never());
Assert.Same(exception, context.Exception); Assert.Same(exception, context.Exception);
Assert.Null(context.Result); Assert.Null(context.Result);
@ -858,7 +920,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal
resultFilter.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>())).Verifiable(); resultFilter.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>())).Verifiable();
resultFilter.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>())).Verifiable(); resultFilter.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>())).Verifiable();
var invoker = CreateInvoker(new IFilterMetadata[] { actionFilter.Object, resultFilter.Object }, actionThrows: true); var invoker = CreateInvoker(
new IFilterMetadata[] { actionFilter.Object, resultFilter.Object },
actionThrows: true);
// Act // Act
await invoker.InvokeAsync(); await invoker.InvokeAsync();
@ -913,7 +977,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
} }
[Fact] [Fact]
public async Task InvokeAction_InvokesResultFilter_ShortCircuit() public async Task InvokeAction_InvokesResultFilter_ShortCircuit_WithCancel()
{ {
// Arrange // Arrange
ResultExecutedContext context = null; ResultExecutedContext context = null;
@ -947,6 +1011,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
filter1.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once()); filter1.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
filter2.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once()); filter2.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
filter2.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Never());
Assert.True(context.Canceled); Assert.True(context.Canceled);
} }
@ -1797,6 +1862,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
expected.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
Assert.Same(expected.Object, context.Result); Assert.Same(expected.Object, context.Result);
Assert.True(context.Canceled); Assert.True(context.Canceled);
Assert.False(invoker.ControllerFactory.CreateCalled); Assert.False(invoker.ControllerFactory.CreateCalled);
@ -1899,6 +1965,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
await invoker.InvokeAsync(); await invoker.InvokeAsync();
// Assert // Assert
expected.Verify(r => r.ExecuteResultAsync(It.IsAny<ActionContext>()), Times.Once());
Assert.Same(expected.Object, context.Result); Assert.Same(expected.Object, context.Result);
Assert.True(context.Canceled); Assert.True(context.Canceled);
Assert.False(invoker.ControllerFactory.CreateCalled); Assert.False(invoker.ControllerFactory.CreateCalled);
@ -2751,7 +2818,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Assert.NotNull(listener.AfterAction?.ActionDescriptor); Assert.NotNull(listener.AfterAction?.ActionDescriptor);
Assert.NotNull(listener.AfterAction?.HttpContext); Assert.NotNull(listener.AfterAction?.HttpContext);
} }
public async Task InvokeAction_ExceptionBubbling_AsyncActionFilter_To_ResourceFilter() public async Task InvokeAction_ExceptionBubbling_AsyncActionFilter_To_ResourceFilter()
{ {
// Arrange // Arrange