diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/ResultFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/ResultFilterAttribute.cs index f88aed0c4d..7ba4d6b73b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/ResultFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/ResultFilterAttribute.cs @@ -24,7 +24,7 @@ namespace Microsoft.AspNet.Mvc [NotNull] ResultExecutionDelegate next) { OnResultExecuting(context); - if (context.Result == null) + if (!context.Cancel) { OnResultExecuted(await next()); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs index 2fbc6b6ceb..1c8fbc8b71 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs @@ -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()); + } + + [Fact] + public async Task Controller_ActionFilter_Calls_OnActionExecuted() + { + await ActionFilterAttributeTests.ActionFilter_Calls_OnActionExecuted( + new Mock()); + } +#endif } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ActionFilterAttributeTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ActionFilterAttributeTests.cs new file mode 100644 index 0000000000..cbf434b64a --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ActionFilterAttributeTests.cs @@ -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()); + } + + [Fact] + public async Task ActionAttributeFilter_ActionFilter_Calls_OnActionExecuted() + { + await ActionFilter_Calls_OnActionExecuted(new Mock()); + } + + // This is used as a 'common' test method for ActionFilterAttribute and Controller + public static async Task ActionFilter_Calls_OnActionExecuted(Mock mock) + { + // Arrange + mock.As() + .Setup(f => f.OnActionExecutionAsync( + It.IsAny(), + It.IsAny())) + .CallBase(); + + mock.As() + .Setup(f => f.OnActionExecuting(It.IsAny())) + .Verifiable(); + + mock.As() + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Verifiable(); + + var context = CreateActionExecutingContext(mock.As().Object); + var next = new ActionExecutionDelegate(() => Task.FromResult(CreateActionExecutedContext(context))); + + // Act + await mock.As().Object.OnActionExecutionAsync(context, next); + + // Assert + Assert.Null(context.Result); + + mock.As() + .Verify(f => f.OnActionExecuting(It.IsAny()), Times.Once()); + + mock.As() + .Verify(f => f.OnActionExecuted(It.IsAny()), 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() + .Setup(f => f.OnActionExecutionAsync( + It.IsAny(), + It.IsAny())) + .CallBase(); + + mock.As() + .Setup(f => f.OnActionExecuting(It.IsAny())) + .Callback(c => c.Result = new NoOpResult()); + + mock.As() + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Verifiable(); + + var context = CreateActionExecutingContext(mock.As().Object); + var next = new ActionExecutionDelegate(() => { throw null; }); // This won't run + + // Act + await mock.As().Object.OnActionExecutionAsync(context, next); + + // Assert + Assert.IsType(context.Result); + + mock.As() + .Verify(f => f.OnActionExecuted(It.IsAny()), Times.Never()); + } + + [Fact] + public async Task ActionAttributeFilter_ResultFilter_Calls_OnResultExecuted() + { + await ResultFilter_Calls_OnResultExecuted(new Mock()); + } + + [Fact] + public async Task ActionFilterAttribute_ResultFilter_SettingResult_DoesNotShortCircuit() + { + await ResultFilter_SettingResult_DoesNotShortCircuit(new Mock()); + } + + [Fact] + public async Task ActionFilterAttribute_ResultFilter_SettingCancel_ShortCircuits() + { + await ResultFilter_SettingCancel_ShortCircuits(new Mock()); + } + + // This is used as a 'common' test method for ActionFilterAttribute and ResultFilterAttribute + public static async Task ResultFilter_Calls_OnResultExecuted(Mock mock) + { + // Arrange + mock.As() + .Setup(f => f.OnResultExecutionAsync( + It.IsAny(), + It.IsAny())) + .CallBase(); + + mock.As() + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Verifiable(); + + mock.As() + .Setup(f => f.OnResultExecuted(It.IsAny())) + .Verifiable(); + + var context = CreateResultExecutingContext(mock.As().Object); + var next = new ResultExecutionDelegate(() => Task.FromResult(CreateResultExecutedContext(context))); + + // Act + await mock.As().Object.OnResultExecutionAsync(context, next); + + // Assert + Assert.False(context.Cancel); + + mock.As() + .Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); + + mock.As() + .Verify(f => f.OnResultExecuted(It.IsAny()), 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() + .Setup(f => f.OnResultExecutionAsync( + It.IsAny(), + It.IsAny())) + .CallBase(); + + mock.As() + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback(c => c.Result = new NoOpResult()); + + mock.As() + .Setup(f => f.OnResultExecuted(It.IsAny())) + .Verifiable(); + + var context = CreateResultExecutingContext(mock.As().Object); + var next = new ResultExecutionDelegate(() => Task.FromResult(CreateResultExecutedContext(context))); + + // Act + await mock.As().Object.OnResultExecutionAsync(context, next); + + // Assert + Assert.False(context.Cancel); + + mock.As() + .Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); + + mock.As() + .Verify(f => f.OnResultExecuted(It.IsAny()), 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() + .Setup(f => f.OnResultExecutionAsync( + It.IsAny(), + It.IsAny())) + .CallBase(); + + mock.As() + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback(c => c.Cancel = true); + + mock.As() + .Setup(f => f.OnResultExecuted(It.IsAny())) + .Verifiable(); + + var context = CreateResultExecutingContext(mock.As().Object); + var next = new ResultExecutionDelegate(() => { throw null; }); // This won't run + + // Act + await mock.As().Object.OnResultExecutionAsync(context, next); + + // Assert + Assert.True(context.Cancel); + + mock.As() + .Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); + + mock.As() + .Verify(f => f.OnResultExecuted(It.IsAny()), Times.Never()); + } + + private static ActionExecutingContext CreateActionExecutingContext(IFilter filter) + { + return new ActionExecutingContext( + CreateActionContext(), + new IFilter[] { filter, }, + new Dictionary()); + } + + 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(), new RouteData(), new ActionDescriptor()); + } + + private class NoOpResult : IActionResult + { + public Task ExecuteResultAsync(ActionContext context) + { + return Task.FromResult(null); + } + } + } +} +#endif \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResultFilterAttributeTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResultFilterAttributeTest.cs new file mode 100644 index 0000000000..7581379c5c --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/ResultFilterAttributeTest.cs @@ -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()); + } + + [Fact] + public async Task ResultFilterAttribute_ResultFilter_SettingResult_DoesNotShortCircuit() + { + await ActionFilterAttributeTests.ResultFilter_SettingResult_DoesNotShortCircuit( + new Mock()); + } + + [Fact] + public async Task ResultFilterAttribute_ResultFilter_SettingCancel_ShortCircuits() + { + await ActionFilterAttributeTests.ResultFilter_SettingCancel_ShortCircuits( + new Mock()); + } + } +} +#endif \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj index b6ab6319fb..3f18a38dcd 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj @@ -34,6 +34,7 @@ + @@ -47,6 +48,7 @@ +