From 350c4ec4f6b377749e00b421ba5540badcb4c808 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 22 Dec 2017 10:52:52 -0800 Subject: [PATCH] Introduce IAlwaysRunResultFilter \ IAsyncAlwaysRunResultFilter (#7120) * Introduce IAlwaysRunResultFilter \ IAsyncAlwaysRunResultFilter Fixes #7105 --- .../Filters/ExceptionContext.cs | 2 +- .../Filters/IAlwaysRunResultFilter.cs | 24 + .../Filters/IAsyncAlwaysRunResultFilter.cs | 24 + .../Filters/IAsyncResultFilter.cs | 18 +- .../Filters/IResultFilter.cs | 18 +- .../Internal/ResourceInvoker.cs | 535 +++++++++--------- .../BasicTests.cs | 19 + .../CommonResourceInvokerTest.cs | 360 ++++++++++++ .../Controllers/FiltersController.cs | 22 + .../Filters/UnprocessableResultFilter.cs | 29 + 10 files changed, 795 insertions(+), 256 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAlwaysRunResultFilter.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncAlwaysRunResultFilter.cs create mode 100644 test/WebSites/BasicWebSite/Controllers/FiltersController.cs create mode 100644 test/WebSites/BasicWebSite/Filters/UnprocessableResultFilter.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/ExceptionContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/ExceptionContext.cs index 7732a60160..c0920f057b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/ExceptionContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/ExceptionContext.cs @@ -71,7 +71,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// /// Gets or sets an indication that the has been handled. /// - public virtual bool ExceptionHandled { get; set; } = false; + public virtual bool ExceptionHandled { get; set; } /// /// Gets or sets the . diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAlwaysRunResultFilter.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAlwaysRunResultFilter.cs new file mode 100644 index 0000000000..e119fd8d93 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAlwaysRunResultFilter.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.Filters +{ + /// + /// A filter that surrounds execution of all action results. + /// + /// + /// + /// The interface declares an implementation + /// that should run for all action results. . + /// + /// + /// and instances are not executed in cases where + /// an authorization filter or resource filter short-circuits the request to prevent execution of the action. + /// and implementations + /// are also not executed in cases where an exception filter handles an exception by producing an action result. + /// + /// + public interface IAlwaysRunResultFilter : IResultFilter + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncAlwaysRunResultFilter.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncAlwaysRunResultFilter.cs new file mode 100644 index 0000000000..f3c0c94dd8 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncAlwaysRunResultFilter.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.Filters +{ + /// + /// A filter that asynchronously surrounds execution of all action results. + /// + /// + /// + /// The interface declares an implementation + /// that should run for all action results. . + /// + /// + /// and instances are not executed in cases where + /// an authorization filter or resource filter short-circuits the request to prevent execution of the action. + /// and implementations + /// are also not executed in cases where an exception filter handles an exception by producing an action result. + /// + /// + public interface IAsyncAlwaysRunResultFilter : IAsyncResultFilter + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncResultFilter.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncResultFilter.cs index 64be384a8f..d6583cbc60 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncResultFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IAsyncResultFilter.cs @@ -6,8 +6,24 @@ using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.Filters { /// - /// A filter that asynchronously surrounds execution of the action result. + /// A filter that asynchronously surrounds execution of action results successfully returned from an action. /// + /// + /// + /// and implementations are executed around the action + /// result only when the action method (or action filters) complete successfully. + /// + /// + /// and instances are not executed in cases where + /// an authorization filter or resource filter short-circuits the request to prevent execution of the action. + /// . and implementations + /// are also not executed in cases where an exception filter handles an exception by producing an action result. + /// + /// + /// To create a result filter that surrounds the execution of all action results, implement + /// either the or the interface. + /// + /// public interface IAsyncResultFilter : IFilterMetadata { /// diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IResultFilter.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IResultFilter.cs index 151d2d2022..bb0ba46bbb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IResultFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/IResultFilter.cs @@ -4,8 +4,24 @@ namespace Microsoft.AspNetCore.Mvc.Filters { /// - /// A filter that surrounds execution of the action result. + /// A filter that surrounds execution of action results successfully returned from an action. /// + /// + /// + /// and implementations are executed around the action + /// result only when the action method (or action filters) complete successfully. + /// + /// + /// and instances are not executed in cases where + /// an authorization filter or resource filter short-circuits the request to prevent execution of the action. + /// . and implementations + /// are also not executed in cases where an exception filter handles an exception by producing an action result. + /// + /// + /// To create a result filter that surrounds the execution of all action results, implement + /// either the or the interface. + /// + /// public interface IResultFilter : IFilterMetadata { /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs index c8b26c0009..8b3a48ba0e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs @@ -41,38 +41,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal IFilterMetadata[] filters, IList valueProviderFactories) { + _diagnosticSource = diagnosticSource ?? throw new ArgumentNullException(nameof(diagnosticSource)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _actionContext = actionContext ?? throw new ArgumentNullException(nameof(actionContext)); - if (diagnosticSource == null) - { - throw new ArgumentNullException(nameof(diagnosticSource)); - } - - if (logger == null) - { - throw new ArgumentNullException(nameof(logger)); - } - - if (actionContext == null) - { - throw new ArgumentNullException(nameof(actionContext)); - } - - if (filters == null) - { - throw new ArgumentNullException(nameof(filters)); - } - - if (valueProviderFactories == null) - { - throw new ArgumentNullException(nameof(valueProviderFactories)); - } - - _diagnosticSource = diagnosticSource; - _logger = logger; - _actionContext = actionContext; - - _filters = filters; - _valueProviderFactories = valueProviderFactories; + _filters = filters ?? throw new ArgumentNullException(nameof(filters)); + _valueProviderFactories = valueProviderFactories ?? throw new ArgumentNullException(nameof(valueProviderFactories)); _cursor = new FilterCursor(filters); } @@ -289,13 +263,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal { Debug.Assert(state != null); Debug.Assert(_authorizationContext != null); + Debug.Assert(_authorizationContext.Result != null); _logger.AuthorizationFailure((IFilterMetadata)state); - // If an authorization filter short circuits, the result is the last thing we execute - // so just return that task instead of calling back into the state machine. + // This is a short-circuit - execute relevant result filters + result and complete this invocation. isCompleted = true; - return InvokeResultAsync(_authorizationContext.Result); + _result = _authorizationContext.Result; + return InvokeAlwaysRunResultFilters(); } case State.AuthorizationEnd: @@ -477,7 +452,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal _logger.ResourceFilterShortCircuited((IFilterMetadata)state); - var task = InvokeResultAsync(_resourceExecutingContext.Result); + _result = _resourceExecutingContext.Result; + var task = InvokeAlwaysRunResultFilters(); if (task.Status != TaskStatus.RanToCompletion) { next = State.ResourceEnd; @@ -662,13 +638,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal _exceptionContext.Result = new EmptyResult(); } - if (scope == Scope.Resource) - { - Debug.Assert(_exceptionContext.Result != null); - _result = _exceptionContext.Result; - } + _result = _exceptionContext.Result; - var task = InvokeResultAsync(_exceptionContext.Result); + var task = InvokeAlwaysRunResultFilters(); if (task.Status != TaskStatus.RanToCompletion) { next = State.ResourceInsideEnd; @@ -701,7 +673,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal Debug.Fail("unreachable"); } - goto case State.ResultBegin; + var task = InvokeResultFilters(); + if (task.Status != TaskStatus.RanToCompletion) + { + next = State.ResourceInsideEnd; + return task; + } + goto case State.ResourceInsideEnd; } case State.ActionBegin: @@ -727,217 +705,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal } Debug.Assert(scope == Scope.Invoker || scope == Scope.Resource); - goto case State.ResultBegin; - } - - case State.ResultBegin: - { - _cursor.Reset(); - goto case State.ResultNext; - } - - case State.ResultNext: - { - var current = _cursor.GetNextFilter(); - if (current.FilterAsync != null) - { - if (_resultExecutingContext == null) - { - _resultExecutingContext = new ResultExecutingContext(_actionContext, _filters, _result, _instance); - } - - state = current.FilterAsync; - goto case State.ResultAsyncBegin; - } - else if (current.Filter != null) - { - if (_resultExecutingContext == null) - { - _resultExecutingContext = new ResultExecutingContext(_actionContext, _filters, _result, _instance); - } - - state = current.Filter; - goto case State.ResultSyncBegin; - } - else - { - goto case State.ResultInside; - } - } - - case State.ResultAsyncBegin: - { - Debug.Assert(state != null); - Debug.Assert(_resultExecutingContext != null); - - var filter = (IAsyncResultFilter)state; - var resultExecutingContext = _resultExecutingContext; - - _diagnosticSource.BeforeOnResultExecution(resultExecutingContext, filter); - _logger.BeforeExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IAsyncResultFilter.OnResultExecutionAsync), - filter); - - var task = filter.OnResultExecutionAsync(resultExecutingContext, InvokeNextResultFilterAwaitedAsync); + var task = InvokeResultFilters(); if (task.Status != TaskStatus.RanToCompletion) { - next = State.ResultAsyncEnd; + next = State.ResourceInsideEnd; return task; } - - goto case State.ResultAsyncEnd; - } - - case State.ResultAsyncEnd: - { - Debug.Assert(state != null); - Debug.Assert(_resultExecutingContext != null); - - var filter = (IAsyncResultFilter)state; - var resultExecutingContext = _resultExecutingContext; - var resultExecutedContext = _resultExecutedContext; - - if (resultExecutedContext == null || resultExecutingContext.Cancel) - { - // Short-circuited by not calling next || Short-circuited by setting Cancel == true - _logger.ResultFilterShortCircuited(filter); - - _resultExecutedContext = new ResultExecutedContext( - _actionContext, - _filters, - resultExecutingContext.Result, - _instance) - { - Canceled = true, - }; - } - - _diagnosticSource.AfterOnResultExecution(_resultExecutedContext, filter); - _logger.AfterExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IAsyncResultFilter.OnResultExecutionAsync), - filter); - - goto case State.ResultEnd; - } - - case State.ResultSyncBegin: - { - Debug.Assert(state != null); - Debug.Assert(_resultExecutingContext != null); - - var filter = (IResultFilter)state; - var resultExecutingContext = _resultExecutingContext; - - _diagnosticSource.BeforeOnResultExecuting(resultExecutingContext, filter); - _logger.BeforeExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IResultFilter.OnResultExecuting), - filter); - - filter.OnResultExecuting(resultExecutingContext); - - _diagnosticSource.AfterOnResultExecuting(resultExecutingContext, filter); - _logger.AfterExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IResultFilter.OnResultExecuting), - filter); - - if (_resultExecutingContext.Cancel) - { - // Short-circuited by setting Cancel == true - _logger.ResultFilterShortCircuited(filter); - - _resultExecutedContext = new ResultExecutedContext( - resultExecutingContext, - _filters, - resultExecutingContext.Result, - _instance) - { - Canceled = true, - }; - - goto case State.ResultEnd; - } - - var task = InvokeNextResultFilterAsync(); - if (task.Status != TaskStatus.RanToCompletion) - { - next = State.ResultSyncEnd; - return task; - } - - goto case State.ResultSyncEnd; - } - - case State.ResultSyncEnd: - { - Debug.Assert(state != null); - Debug.Assert(_resultExecutingContext != null); - Debug.Assert(_resultExecutedContext != null); - - var filter = (IResultFilter)state; - var resultExecutedContext = _resultExecutedContext; - - _diagnosticSource.BeforeOnResultExecuted(resultExecutedContext, filter); - _logger.BeforeExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IResultFilter.OnResultExecuted), - filter); - - filter.OnResultExecuted(resultExecutedContext); - - _diagnosticSource.AfterOnResultExecuted(resultExecutedContext, filter); - _logger.AfterExecutingMethodOnFilter( - FilterTypeConstants.ResultFilter, - nameof(IResultFilter.OnResultExecuted), - filter); - - goto case State.ResultEnd; - } - - case State.ResultInside: - { - // If we executed result filters then we need to grab the result from there. - if (_resultExecutingContext != null) - { - _result = _resultExecutingContext.Result; - } - - if (_result == null) - { - // The empty result is always flowed back as the 'executed' result if we don't have one. - _result = new EmptyResult(); - } - - var task = InvokeResultAsync(_result); - if (task.Status != TaskStatus.RanToCompletion) - { - next = State.ResultEnd; - return task; - } - - goto case State.ResultEnd; - } - - case State.ResultEnd: - { - var result = _result; - - if (scope == Scope.Result) - { - if (_resultExecutedContext == null) - { - _resultExecutedContext = new ResultExecutedContext(_actionContext, _filters, result, _instance); - } - - isCompleted = true; - return Task.CompletedTask; - } - - Rethrow(_resultExecutedContext); - goto case State.ResourceInsideEnd; } @@ -1048,7 +821,260 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private async Task InvokeNextResultFilterAsync() + private async Task InvokeAlwaysRunResultFilters() + { + var next = State.ResultBegin; + var scope = Scope.Invoker; + var state = (object)null; + var isCompleted = false; + + while (!isCompleted) + { + await ResultNext(ref next, ref scope, ref state, ref isCompleted); + } + } + + private async Task InvokeResultFilters() + { + var next = State.ResultBegin; + var scope = Scope.Invoker; + var state = (object)null; + var isCompleted = false; + + while (!isCompleted) + { + await ResultNext(ref next, ref scope, ref state, ref isCompleted); + } + } + + private Task ResultNext(ref State next, ref Scope scope, ref object state, ref bool isCompleted) + where TFilter : class, IResultFilter + where TFilterAsync : class, IAsyncResultFilter + { + var resultFilterKind = typeof(TFilter) == typeof(IAlwaysRunResultFilter) ? + FilterTypeConstants.AlwaysRunResultFilter : + FilterTypeConstants.ResultFilter; + + switch (next) + { + case State.ResultBegin: + { + _cursor.Reset(); + goto case State.ResultNext; + } + + case State.ResultNext: + { + var current = _cursor.GetNextFilter(); + if (current.FilterAsync != null) + { + if (_resultExecutingContext == null) + { + _resultExecutingContext = new ResultExecutingContext(_actionContext, _filters, _result, _instance); + } + + state = current.FilterAsync; + goto case State.ResultAsyncBegin; + } + else if (current.Filter != null) + { + if (_resultExecutingContext == null) + { + _resultExecutingContext = new ResultExecutingContext(_actionContext, _filters, _result, _instance); + } + + state = current.Filter; + goto case State.ResultSyncBegin; + } + else + { + goto case State.ResultInside; + } + } + + case State.ResultAsyncBegin: + { + Debug.Assert(state != null); + Debug.Assert(_resultExecutingContext != null); + + var filter = (TFilterAsync)state; + var resultExecutingContext = _resultExecutingContext; + + _diagnosticSource.BeforeOnResultExecution(resultExecutingContext, filter); + _logger.BeforeExecutingMethodOnFilter( + resultFilterKind, + nameof(IAsyncResultFilter.OnResultExecutionAsync), + filter); + + var task = filter.OnResultExecutionAsync(resultExecutingContext, InvokeNextResultFilterAwaitedAsync); + if (task.Status != TaskStatus.RanToCompletion) + { + next = State.ResultAsyncEnd; + return task; + } + + goto case State.ResultAsyncEnd; + } + + case State.ResultAsyncEnd: + { + Debug.Assert(state != null); + Debug.Assert(_resultExecutingContext != null); + + var filter = (TFilterAsync)state; + var resultExecutingContext = _resultExecutingContext; + var resultExecutedContext = _resultExecutedContext; + + if (resultExecutedContext == null || resultExecutingContext.Cancel) + { + // Short-circuited by not calling next || Short-circuited by setting Cancel == true + _logger.ResultFilterShortCircuited(filter); + + _resultExecutedContext = new ResultExecutedContext( + _actionContext, + _filters, + resultExecutingContext.Result, + _instance) + { + Canceled = true, + }; + } + + _diagnosticSource.AfterOnResultExecution(_resultExecutedContext, filter); + _logger.AfterExecutingMethodOnFilter( + resultFilterKind, + nameof(IAsyncResultFilter.OnResultExecutionAsync), + filter); + + goto case State.ResultEnd; + } + + case State.ResultSyncBegin: + { + Debug.Assert(state != null); + Debug.Assert(_resultExecutingContext != null); + + var filter = (TFilter)state; + var resultExecutingContext = _resultExecutingContext; + + _diagnosticSource.BeforeOnResultExecuting(resultExecutingContext, filter); + _logger.BeforeExecutingMethodOnFilter( + resultFilterKind, + nameof(IResultFilter.OnResultExecuting), + filter); + + filter.OnResultExecuting(resultExecutingContext); + + _diagnosticSource.AfterOnResultExecuting(resultExecutingContext, filter); + _logger.AfterExecutingMethodOnFilter( + resultFilterKind, + nameof(IResultFilter.OnResultExecuting), + filter); + + if (_resultExecutingContext.Cancel) + { + // Short-circuited by setting Cancel == true + _logger.ResultFilterShortCircuited(filter); + + _resultExecutedContext = new ResultExecutedContext( + resultExecutingContext, + _filters, + resultExecutingContext.Result, + _instance) + { + Canceled = true, + }; + + goto case State.ResultEnd; + } + + var task = InvokeNextResultFilterAsync(); + if (task.Status != TaskStatus.RanToCompletion) + { + next = State.ResultSyncEnd; + return task; + } + + goto case State.ResultSyncEnd; + } + + case State.ResultSyncEnd: + { + Debug.Assert(state != null); + Debug.Assert(_resultExecutingContext != null); + Debug.Assert(_resultExecutedContext != null); + + var filter = (TFilter)state; + var resultExecutedContext = _resultExecutedContext; + + _diagnosticSource.BeforeOnResultExecuted(resultExecutedContext, filter); + _logger.BeforeExecutingMethodOnFilter( + resultFilterKind, + nameof(IResultFilter.OnResultExecuted), + filter); + + filter.OnResultExecuted(resultExecutedContext); + + _diagnosticSource.AfterOnResultExecuted(resultExecutedContext, filter); + _logger.AfterExecutingMethodOnFilter( + resultFilterKind, + nameof(IResultFilter.OnResultExecuted), + filter); + + goto case State.ResultEnd; + } + + case State.ResultInside: + { + // If we executed result filters then we need to grab the result from there. + if (_resultExecutingContext != null) + { + _result = _resultExecutingContext.Result; + } + + if (_result == null) + { + // The empty result is always flowed back as the 'executed' result if we don't have one. + _result = new EmptyResult(); + } + + var task = InvokeResultAsync(_result); + if (task.Status != TaskStatus.RanToCompletion) + { + next = State.ResultEnd; + return task; + } + + goto case State.ResultEnd; + } + + case State.ResultEnd: + { + var result = _result; + isCompleted = true; + + if (scope == Scope.Result) + { + if (_resultExecutedContext == null) + { + _resultExecutedContext = new ResultExecutedContext(_actionContext, _filters, result, _instance); + } + + return Task.CompletedTask; + } + + Rethrow(_resultExecutedContext); + return Task.CompletedTask; + } + + default: + throw new InvalidOperationException(); // Unreachable. + } + } + + private async Task InvokeNextResultFilterAsync() + where TFilter : class, IResultFilter + where TFilterAsync : class, IAsyncResultFilter { try { @@ -1058,7 +1084,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var isCompleted = false; while (!isCompleted) { - await Next(ref next, ref scope, ref state, ref isCompleted); + await ResultNext(ref next, ref scope, ref state, ref isCompleted); } } catch (Exception exception) @@ -1072,7 +1098,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal Debug.Assert(_resultExecutedContext != null); } - private async Task InvokeNextResultFilterAwaitedAsync() + private async Task InvokeNextResultFilterAwaitedAsync() + where TFilter : class, IResultFilter + where TFilterAsync : class, IAsyncResultFilter { Debug.Assert(_resultExecutingContext != null); if (_resultExecutingContext.Cancel) @@ -1088,7 +1116,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new InvalidOperationException(message); } - await InvokeNextResultFilterAsync(); + await InvokeNextResultFilterAsync(); Debug.Assert(_resultExecutedContext != null); return _resultExecutedContext; @@ -1221,6 +1249,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public const string ActionFilter = "Action Filter"; public const string ExceptionFilter = "Exception Filter"; public const string ResultFilter = "Result Filter"; + public const string AlwaysRunResultFilter = "Always Run Result Filter"; } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs index 34f3849ccc..f1958ed558 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs @@ -7,6 +7,7 @@ using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Reflection; +using System.Text; using System.Threading.Tasks; using BasicWebSite.Models; using Newtonsoft.Json; @@ -472,5 +473,23 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert Assert.Equal("true", response); } + + [Fact] + public async Task AlwaysRunResultFilters_CanRunWhenResourceFiltersShortCircuit() + { + // Arrange + var url = "Filters/AlwaysRunResultFiltersCanRunWhenResourceFilterShortCircuit"; + var request = new HttpRequestMessage(HttpMethod.Post, url) + { + Content = new StringContent("Test", Encoding.UTF8, "application/json"), + }; + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(422, (int)response.StatusCode); + Assert.Equal("Can't process this!", await response.Content.ReadAsStringAsync()); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.TestCommon/CommonResourceInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.TestCommon/CommonResourceInvokerTest.cs index ae17e2ec30..86d2f27071 100644 --- a/test/Microsoft.AspNetCore.Mvc.TestCommon/CommonResourceInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TestCommon/CommonResourceInvokerTest.cs @@ -556,6 +556,24 @@ namespace Microsoft.AspNetCore.Mvc result.Verify(r => r.ExecuteResultAsync(It.IsAny()), Times.Once()); } + [Fact] + public async Task InvokeAction_WithExceptionFilterInTheStack_InvokesResultFilter() + { + // Arrange + var exceptionFilter = new Mock(); + var resultFilter = new Mock(); + + var invoker = CreateInvoker( + new IFilterMetadata[] { exceptionFilter.Object, resultFilter.Object }); + + // Act + await invoker.InvokeAsync(); + + // Assert + exceptionFilter.Verify(f => f.OnException(It.IsAny()), Times.Never()); + resultFilter.Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); + } + [Fact] public async Task InvokeAction_InvokesAuthorizationFilter() { @@ -1713,6 +1731,348 @@ namespace Microsoft.AspNetCore.Mvc Times.Never()); } + [Fact] + public async Task InvokeAction_AuthorizationFilterShortCircuit_InvokesAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + resultFilter.Setup(f => f.OnResultExecuted(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + + var authorizationFilter = new Mock(MockBehavior.Strict); + authorizationFilter + .Setup(f => f.OnAuthorizationAsync(It.IsAny())) + .Returns((c) => + { + c.Result = Result; + return Task.CompletedTask; + }); + + var invoker = CreateInvoker(new IFilterMetadata[] { authorizationFilter.Object, resultFilter.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_AuthorizationFilterShortCircuit_InvokesAsyncAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => + { + Assert.Same(Result, c.Result); + return next(); + }) + .Verifiable(); + + var authorizationFilter = new Mock(MockBehavior.Strict); + authorizationFilter + .Setup(f => f.OnAuthorization(It.IsAny())) + .Callback((c) => + { + c.Result = Result; + }); + + var invoker = CreateInvoker(new IFilterMetadata[] { authorizationFilter.Object, resultFilter.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_AuthorizationFilterShortCircuit_DoesNotRunResultFilters() + { + // Arrange + var resultFilter1 = new Mock(MockBehavior.Strict); + resultFilter1.Setup(f => f.OnResultExecuting(It.IsAny())); + resultFilter1.Setup(f => f.OnResultExecuted(It.IsAny())); + + var resultFilter2 = new Mock(MockBehavior.Strict); + resultFilter2.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())); + + var resultFilter3 = new Mock(MockBehavior.Strict); + resultFilter3.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var authorizationFilter = new Mock(MockBehavior.Strict); + authorizationFilter + .Setup(f => f.OnAuthorization(It.IsAny())) + .Callback((c) => + { + c.Result = Result; + }); + + var invoker = CreateInvoker(new IFilterMetadata[] { authorizationFilter.Object, resultFilter1.Object, resultFilter2.Object, resultFilter3.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter1.Verify( + f => f.OnResultExecuting(It.IsAny()), + Times.Never()); + resultFilter1.Verify( + f => f.OnResultExecuted(It.IsAny()), + Times.Never()); + resultFilter2.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Never()); + resultFilter3.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + } + + [Fact] + public async Task InvokeAction_ResourceFilterShortCircuit_InvokesAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + resultFilter.Setup(f => f.OnResultExecuted(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + + var resourceFilter = new Mock(MockBehavior.Strict); + resourceFilter + .Setup(f => f.OnResourceExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => + { + c.Result = Result; + return Task.CompletedTask; + }); + + var invoker = CreateInvoker(new IFilterMetadata[] { resourceFilter.Object, resultFilter.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_ResourceFilterShortCircuit_InvokesAsyncAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => + { + Assert.Same(Result, c.Result); + return next(); + }) + .Verifiable(); + + var resourceFilter = new Mock(MockBehavior.Strict); + resourceFilter + .Setup(f => f.OnResourceExecuting(It.IsAny())) + .Callback(c => c.Result = Result); + + var invoker = CreateInvoker(new IFilterMetadata[] { resourceFilter.Object, resultFilter.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_ResourceFilterShortCircuit_DoesNotRunResultFilters() + { + // Arrange + var resultFilter1 = new Mock(MockBehavior.Strict); + resultFilter1.Setup(f => f.OnResultExecuting(It.IsAny())); + resultFilter1.Setup(f => f.OnResultExecuted(It.IsAny())); + + var resultFilter2 = new Mock(MockBehavior.Strict); + resultFilter2.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())); + + var resultFilter3 = new Mock(MockBehavior.Strict); + resultFilter3.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var resourceFilter = new Mock(MockBehavior.Strict); + resourceFilter + .Setup(f => f.OnResourceExecuting(It.IsAny())) + .Callback(c => c.Result = Result); + + var invoker = CreateInvoker(new IFilterMetadata[] { resourceFilter.Object, resultFilter1.Object, resultFilter2.Object, resultFilter3.Object, }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter1.Verify( + f => f.OnResultExecuting(It.IsAny()), + Times.Never()); + resultFilter1.Verify( + f => f.OnResultExecuted(It.IsAny()), + Times.Never()); + resultFilter2.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Never()); + resultFilter3.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + } + + [Fact] + public async Task InvokeAction_ExceptionFilterShortCircuit_InvokesAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + resultFilter.Setup(f => f.OnResultExecuted(It.IsAny())) + .Callback(c => Assert.Same(Result, c.Result)) + .Verifiable(); + + var exceptionFilter = new Mock(MockBehavior.Strict); + exceptionFilter + .Setup(f => f.OnExceptionAsync(It.IsAny())) + .Returns(c => + { + c.Result = Result; + return Task.CompletedTask; + }); + + var invoker = CreateInvoker(new IFilterMetadata[] { exceptionFilter.Object, resultFilter.Object, }, Exception); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_ExceptionFilterShortCircuit_InvokesAsyncAlwaysRunResultFilter() + { + // Arrange + var resultFilter = new Mock(MockBehavior.Strict); + resultFilter.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => + { + Assert.Same(Result, c.Result); + return next(); + }) + .Verifiable(); + + var exceptionFilter = new Mock(MockBehavior.Strict); + exceptionFilter + .Setup(f => f.OnException(It.IsAny())) + .Callback(c => c.Result = Result); + + var invoker = CreateInvoker(new IFilterMetadata[] { exceptionFilter.Object, resultFilter.Object, }, Exception); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter.Verify(); + } + + [Fact] + public async Task InvokeAction_ExceptionFilterShortCircuit_DoesNotRunResultFilters() + { + // Arrange + var resultFilter1 = new Mock(MockBehavior.Strict); + resultFilter1.Setup(f => f.OnResultExecuting(It.IsAny())); + resultFilter1.Setup(f => f.OnResultExecuted(It.IsAny())); + + var resultFilter2 = new Mock(MockBehavior.Strict); + resultFilter2.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())); + + var resultFilter3 = new Mock(MockBehavior.Strict); + resultFilter3.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var exceptionFilter = new Mock(MockBehavior.Strict); + exceptionFilter + .Setup(f => f.OnException(It.IsAny())) + .Callback(c => c.Result = Result); + + var invoker = CreateInvoker( + new IFilterMetadata[] { exceptionFilter.Object, resultFilter1.Object, resultFilter2.Object, resultFilter3.Object, }, + Exception); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter1.Verify( + f => f.OnResultExecuting(It.IsAny()), + Times.Never()); + resultFilter1.Verify( + f => f.OnResultExecuted(It.IsAny()), + Times.Never()); + resultFilter2.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Never()); + resultFilter3.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + } + + [Fact] + public async Task InvokeAction_AlwaysRunResultFiltersAndRunWithResultFilters() + { + // Arrange + var resultFilter1 = new Mock(MockBehavior.Strict); + resultFilter1 + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Verifiable(); + resultFilter1 + .Setup(f => f.OnResultExecuted(It.IsAny())) + .Verifiable(); + + var resultFilter2 = new Mock(MockBehavior.Strict); + resultFilter2 + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Verifiable(); + resultFilter2 + .Setup(f => f.OnResultExecuted(It.IsAny())) + .Verifiable(); + + var resultFilter3 = new Mock(MockBehavior.Strict); + resultFilter3.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => next()) + .Verifiable(); + + var resultFilter4 = new Mock(MockBehavior.Strict); + resultFilter4.Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((c, next) => next()) + .Verifiable(); + + var invoker = CreateInvoker( + new IFilterMetadata[] { resultFilter1.Object, resultFilter2.Object, resultFilter3.Object, resultFilter4.Object }); + + // Act + await invoker.InvokeAsync(); + + // Assert + resultFilter1.Verify(); + resultFilter2.Verify(); + resultFilter3.Verify(); + resultFilter4.Verify(); + } + public class TestResult : ActionResult { } diff --git a/test/WebSites/BasicWebSite/Controllers/FiltersController.cs b/test/WebSites/BasicWebSite/Controllers/FiltersController.cs new file mode 100644 index 0000000000..49c5009ef6 --- /dev/null +++ b/test/WebSites/BasicWebSite/Controllers/FiltersController.cs @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using BasicWebSite.Models; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Newtonsoft.Json.Serialization; + +namespace BasicWebSite.Controllers +{ + public class FiltersController : Controller + { + [HttpPost] + [Consumes("application/yaml")] + [UnprocessableResultFilter] + public IActionResult AlwaysRunResultFiltersCanRunWhenResourceFilterShortCircuit([FromBody] Product product) => + throw new Exception("Shouldn't be executed"); + } +} diff --git a/test/WebSites/BasicWebSite/Filters/UnprocessableResultFilter.cs b/test/WebSites/BasicWebSite/Filters/UnprocessableResultFilter.cs new file mode 100644 index 0000000000..ac85a6fc90 --- /dev/null +++ b/test/WebSites/BasicWebSite/Filters/UnprocessableResultFilter.cs @@ -0,0 +1,29 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace BasicWebSite +{ + public class UnprocessableResultFilter : Attribute, IAlwaysRunResultFilter + { + public void OnResultExecuted(ResultExecutedContext context) + { + } + + public void OnResultExecuting(ResultExecutingContext context) + { + if (context.Result is StatusCodeResult statusCodeResult && + statusCodeResult.StatusCode == 415) + { + context.Result = new ObjectResult("Can't process this!") + { + StatusCode = 422, + }; + } + } + } +}