Issue #592 - ActionResult is not being executed by ResultFilterAttribute
The issue here is actually different than described in the bug. ResultFilter should only short circuit when .Cancel is set to true. This is consistent with legacy MVC. Added tests for all of this stuff. There's already good test coverage for the invoker, what was missing was coverage for the attributes and for the methods on Controller. ExceptionFilterAttribute and AuthorizationFilterAttribute don't have short circuiting logic inside of them, so they are already covered by tests for the invoker.
This commit is contained in:
Ryan Nowak
parent
10285d7d39
commit
1bae3bcaa2
|
|
@ -24,7 +24,7 @@ namespace Microsoft.AspNet.Mvc
|
|||
[NotNull] ResultExecutionDelegate next)
|
||||
{
|
||||
OnResultExecuting(context);
|
||||
if (context.Result == null)
|
||||
if (!context.Cancel)
|
||||
{
|
||||
OnResultExecuted(await next());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,14 +3,18 @@
|
|||
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Threading.Tasks;
|
||||
using System.Reflection;
|
||||
using System.Text;
|
||||
using Microsoft.AspNet.Mvc.ModelBinding;
|
||||
using Microsoft.AspNet.Routing;
|
||||
using Microsoft.AspNet.Testing;
|
||||
#if NET45
|
||||
using Moq;
|
||||
#endif
|
||||
using Xunit;
|
||||
|
||||
namespace Microsoft.AspNet.Mvc.Core
|
||||
namespace Microsoft.AspNet.Mvc.Test
|
||||
{
|
||||
public class ControllerTests
|
||||
{
|
||||
|
|
@ -426,5 +430,23 @@ namespace Microsoft.AspNet.Mvc.Core
|
|||
};
|
||||
}
|
||||
}
|
||||
|
||||
// These tests share code with the ActionFilterAttribute tests because the various filter
|
||||
// implementations need to behave the same way.
|
||||
#if NET45
|
||||
[Fact]
|
||||
public async Task Controller_ActionFilter_SettingResult_ShortCircuits()
|
||||
{
|
||||
await ActionFilterAttributeTests.ActionFilter_SettingResult_ShortCircuits(
|
||||
new Mock<Controller>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Controller_ActionFilter_Calls_OnActionExecuted()
|
||||
{
|
||||
await ActionFilterAttributeTests.ActionFilter_Calls_OnActionExecuted(
|
||||
new Mock<Controller>());
|
||||
}
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,256 @@
|
|||
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
|
||||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||
|
||||
#if NET45
|
||||
using System.Collections.Generic;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.AspNet.Http;
|
||||
using Microsoft.AspNet.Routing;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
|
||||
namespace Microsoft.AspNet.Mvc.Test
|
||||
{
|
||||
public class ActionFilterAttributeTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task ActionFilterAttribute_ActionFilter_SettingResult_ShortCircuits()
|
||||
{
|
||||
await ActionFilter_SettingResult_ShortCircuits(new Mock<ActionFilterAttribute>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ActionAttributeFilter_ActionFilter_Calls_OnActionExecuted()
|
||||
{
|
||||
await ActionFilter_Calls_OnActionExecuted(new Mock<ActionFilterAttribute>());
|
||||
}
|
||||
|
||||
// This is used as a 'common' test method for ActionFilterAttribute and Controller
|
||||
public static async Task ActionFilter_Calls_OnActionExecuted(Mock mock)
|
||||
{
|
||||
// Arrange
|
||||
mock.As<IAsyncActionFilter>()
|
||||
.Setup(f => f.OnActionExecutionAsync(
|
||||
It.IsAny<ActionExecutingContext>(),
|
||||
It.IsAny<ActionExecutionDelegate>()))
|
||||
.CallBase();
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()))
|
||||
.Verifiable();
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Setup(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()))
|
||||
.Verifiable();
|
||||
|
||||
var context = CreateActionExecutingContext(mock.As<IFilter>().Object);
|
||||
var next = new ActionExecutionDelegate(() => Task.FromResult(CreateActionExecutedContext(context)));
|
||||
|
||||
// Act
|
||||
await mock.As<IAsyncActionFilter>().Object.OnActionExecutionAsync(context, next);
|
||||
|
||||
// Assert
|
||||
Assert.Null(context.Result);
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Verify(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()), Times.Once());
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Once());
|
||||
}
|
||||
|
||||
// This is used as a 'common' test method for ActionFilterAttribute and Controller
|
||||
public static async Task ActionFilter_SettingResult_ShortCircuits(Mock mock)
|
||||
{
|
||||
// Arrange
|
||||
mock.As<IAsyncActionFilter>()
|
||||
.Setup(f => f.OnActionExecutionAsync(
|
||||
It.IsAny<ActionExecutingContext>(),
|
||||
It.IsAny<ActionExecutionDelegate>()))
|
||||
.CallBase();
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Setup(f => f.OnActionExecuting(It.IsAny<ActionExecutingContext>()))
|
||||
.Callback<ActionExecutingContext>(c => c.Result = new NoOpResult());
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Setup(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()))
|
||||
.Verifiable();
|
||||
|
||||
var context = CreateActionExecutingContext(mock.As<IFilter>().Object);
|
||||
var next = new ActionExecutionDelegate(() => { throw null; }); // This won't run
|
||||
|
||||
// Act
|
||||
await mock.As<IAsyncActionFilter>().Object.OnActionExecutionAsync(context, next);
|
||||
|
||||
// Assert
|
||||
Assert.IsType<NoOpResult>(context.Result);
|
||||
|
||||
mock.As<IActionFilter>()
|
||||
.Verify(f => f.OnActionExecuted(It.IsAny<ActionExecutedContext>()), Times.Never());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ActionAttributeFilter_ResultFilter_Calls_OnResultExecuted()
|
||||
{
|
||||
await ResultFilter_Calls_OnResultExecuted(new Mock<ActionFilterAttribute>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ActionFilterAttribute_ResultFilter_SettingResult_DoesNotShortCircuit()
|
||||
{
|
||||
await ResultFilter_SettingResult_DoesNotShortCircuit(new Mock<ActionFilterAttribute>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ActionFilterAttribute_ResultFilter_SettingCancel_ShortCircuits()
|
||||
{
|
||||
await ResultFilter_SettingCancel_ShortCircuits(new Mock<ActionFilterAttribute>());
|
||||
}
|
||||
|
||||
// This is used as a 'common' test method for ActionFilterAttribute and ResultFilterAttribute
|
||||
public static async Task ResultFilter_Calls_OnResultExecuted(Mock mock)
|
||||
{
|
||||
// Arrange
|
||||
mock.As<IAsyncResultFilter>()
|
||||
.Setup(f => f.OnResultExecutionAsync(
|
||||
It.IsAny<ResultExecutingContext>(),
|
||||
It.IsAny<ResultExecutionDelegate>()))
|
||||
.CallBase();
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()))
|
||||
.Verifiable();
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()))
|
||||
.Verifiable();
|
||||
|
||||
var context = CreateResultExecutingContext(mock.As<IFilter>().Object);
|
||||
var next = new ResultExecutionDelegate(() => Task.FromResult(CreateResultExecutedContext(context)));
|
||||
|
||||
// Act
|
||||
await mock.As<IAsyncResultFilter>().Object.OnResultExecutionAsync(context, next);
|
||||
|
||||
// Assert
|
||||
Assert.False(context.Cancel);
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
|
||||
}
|
||||
|
||||
// This is used as a 'common' test method for ActionFilterAttribute and ResultFilterAttribute
|
||||
public static async Task ResultFilter_SettingResult_DoesNotShortCircuit(Mock mock)
|
||||
{
|
||||
// Arrange
|
||||
mock.As<IAsyncResultFilter>()
|
||||
.Setup(f => f.OnResultExecutionAsync(
|
||||
It.IsAny<ResultExecutingContext>(),
|
||||
It.IsAny<ResultExecutionDelegate>()))
|
||||
.CallBase();
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()))
|
||||
.Callback<ResultExecutingContext>(c => c.Result = new NoOpResult());
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()))
|
||||
.Verifiable();
|
||||
|
||||
var context = CreateResultExecutingContext(mock.As<IFilter>().Object);
|
||||
var next = new ResultExecutionDelegate(() => Task.FromResult(CreateResultExecutedContext(context)));
|
||||
|
||||
// Act
|
||||
await mock.As<IAsyncResultFilter>().Object.OnResultExecutionAsync(context, next);
|
||||
|
||||
// Assert
|
||||
Assert.False(context.Cancel);
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Once());
|
||||
}
|
||||
|
||||
// This is used as a 'common' test method for ActionFilterAttribute and ResultFilterAttribute
|
||||
public static async Task ResultFilter_SettingCancel_ShortCircuits(Mock mock)
|
||||
{
|
||||
// Arrange
|
||||
mock.As<IAsyncResultFilter>()
|
||||
.Setup(f => f.OnResultExecutionAsync(
|
||||
It.IsAny<ResultExecutingContext>(),
|
||||
It.IsAny<ResultExecutionDelegate>()))
|
||||
.CallBase();
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()))
|
||||
.Callback<ResultExecutingContext>(c => c.Cancel = true);
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Setup(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()))
|
||||
.Verifiable();
|
||||
|
||||
var context = CreateResultExecutingContext(mock.As<IFilter>().Object);
|
||||
var next = new ResultExecutionDelegate(() => { throw null; }); // This won't run
|
||||
|
||||
// Act
|
||||
await mock.As<IAsyncResultFilter>().Object.OnResultExecutionAsync(context, next);
|
||||
|
||||
// Assert
|
||||
Assert.True(context.Cancel);
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuting(It.IsAny<ResultExecutingContext>()), Times.Once());
|
||||
|
||||
mock.As<IResultFilter>()
|
||||
.Verify(f => f.OnResultExecuted(It.IsAny<ResultExecutedContext>()), Times.Never());
|
||||
}
|
||||
|
||||
private static ActionExecutingContext CreateActionExecutingContext(IFilter filter)
|
||||
{
|
||||
return new ActionExecutingContext(
|
||||
CreateActionContext(),
|
||||
new IFilter[] { filter, },
|
||||
new Dictionary<string, object>());
|
||||
}
|
||||
|
||||
private static ActionExecutedContext CreateActionExecutedContext(ActionExecutingContext context)
|
||||
{
|
||||
return new ActionExecutedContext(context, context.Filters)
|
||||
{
|
||||
Result = context.Result,
|
||||
};
|
||||
}
|
||||
|
||||
private static ResultExecutingContext CreateResultExecutingContext(IFilter filter)
|
||||
{
|
||||
return new ResultExecutingContext(
|
||||
CreateActionContext(),
|
||||
new IFilter[] { filter, },
|
||||
new NoOpResult());
|
||||
}
|
||||
|
||||
private static ResultExecutedContext CreateResultExecutedContext(ResultExecutingContext context)
|
||||
{
|
||||
return new ResultExecutedContext(context, context.Filters, context.Result);
|
||||
}
|
||||
|
||||
private static ActionContext CreateActionContext()
|
||||
{
|
||||
return new ActionContext(Mock.Of<HttpContext>(), new RouteData(), new ActionDescriptor());
|
||||
}
|
||||
|
||||
private class NoOpResult : IActionResult
|
||||
{
|
||||
public Task ExecuteResultAsync(ActionContext context)
|
||||
{
|
||||
return Task.FromResult<object>(null);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
|
||||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||
|
||||
#if NET45
|
||||
using System.Threading.Tasks;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
|
||||
namespace Microsoft.AspNet.Mvc.Test
|
||||
{
|
||||
// These tests share code with the ActionFilterAttribute tests because the IAsyncResultFilter
|
||||
// implementations need to behave the same way.
|
||||
public class ResultFilterAttributeTest
|
||||
{
|
||||
[Fact]
|
||||
public async Task ResultFilterAttribute_ResultFilter_Calls_OnResultExecuted()
|
||||
{
|
||||
await ActionFilterAttributeTests.ResultFilter_Calls_OnResultExecuted(
|
||||
new Mock<ResultFilterAttribute>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResultFilterAttribute_ResultFilter_SettingResult_DoesNotShortCircuit()
|
||||
{
|
||||
await ActionFilterAttributeTests.ResultFilter_SettingResult_DoesNotShortCircuit(
|
||||
new Mock<ResultFilterAttribute>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResultFilterAttribute_ResultFilter_SettingCancel_ShortCircuits()
|
||||
{
|
||||
await ActionFilterAttributeTests.ResultFilter_SettingCancel_ShortCircuits(
|
||||
new Mock<ResultFilterAttribute>());
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
|
@ -34,6 +34,7 @@
|
|||
<Compile Include="AntiXsrf\AntiForgeryTokenSerializerTest.cs" />
|
||||
<Compile Include="AntiXsrf\ITokenProvider.cs" />
|
||||
<Compile Include="AntiXsrf\ValidateAntiForgeryTokenAttributeTest.cs" />
|
||||
<Compile Include="Filters\ActionFilterAttributeTests.cs" />
|
||||
<Compile Include="Filters\AuthorizeAttributeTests.cs" />
|
||||
<Compile Include="Filters\AuthorizeAttributeTestsBase.cs" />
|
||||
<Compile Include="AntiXsrf\AntiForgeryTokenStoreTest.cs" />
|
||||
|
|
@ -47,6 +48,7 @@
|
|||
<Compile Include="DefaultActionSelectorTest.cs" />
|
||||
<Compile Include="DefaultControllerAssemblyProviderTests.cs" />
|
||||
<Compile Include="DefaultControllerFactoryTest.cs" />
|
||||
<Compile Include="Filters\ResultFilterAttributeTest.cs" />
|
||||
<Compile Include="JsonResultTest.cs" />
|
||||
<Compile Include="Properties\AssemblyInfo.cs" />
|
||||
<Compile Include="PropertyHelperTest.cs" />
|
||||
|
|
|
|||
Loading…
Reference in New Issue