diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs index d4cb7cfed2..e06c9e1e69 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using Microsoft.AspNet.Mvc.Description; +using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.Routing; using Microsoft.Framework.Logging; @@ -141,6 +142,21 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels controllerModel.ApiExplorer.GroupName = apiGroupName.GroupName; } + // Controllers can implement action filter and result filter interfaces. We add + // a special delegating filter implementation to the pipeline to handle it. + // + // This is needed because filters are instantiated before the controller. + if (typeof(IAsyncActionFilter).GetTypeInfo().IsAssignableFrom(typeInfo) || + typeof(IActionFilter).GetTypeInfo().IsAssignableFrom(typeInfo)) + { + controllerModel.Filters.Add(new ControllerActionFilter()); + } + if (typeof(IAsyncResultFilter).GetTypeInfo().IsAssignableFrom(typeInfo) || + typeof(IResultFilter).GetTypeInfo().IsAssignableFrom(typeInfo)) + { + controllerModel.Filters.Add(new ControllerResultFilter()); + } + return controllerModel; } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index e4e4b51226..e5ad680fa9 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -16,7 +16,7 @@ using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc { - public class Controller : IActionFilter, IAsyncActionFilter, IOrderedFilter, IDisposable + public class Controller : IActionFilter, IAsyncActionFilter, IDisposable { private DynamicViewData _viewBag; private ViewDataDictionary _viewData; @@ -137,15 +137,6 @@ namespace Microsoft.AspNet.Mvc } } - int IOrderedFilter.Order - { - get - { - // Controller-filter methods run farthest the action by default. - return int.MinValue; - } - } - /// /// Creates a object that renders a view to the response. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerActionFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerActionFilter.cs new file mode 100644 index 0000000000..d62c4d4131 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerActionFilter.cs @@ -0,0 +1,52 @@ +// 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. + +using System; +using System.Threading.Tasks; +using Microsoft.AspNet.Mvc.Core; + +namespace Microsoft.AspNet.Mvc.Filters +{ + /// + /// A filter implementation which delegates to the controller for action filter interfaces. + /// + public class ControllerActionFilter : IAsyncActionFilter, IOrderedFilter + { + // Controller-filter methods run farthest from the action by default. + /// + public int Order { get; set; } = int.MinValue; + + /// + public async Task OnActionExecutionAsync( + [NotNull] ActionExecutingContext context, + [NotNull] ActionExecutionDelegate next) + { + var controller = context.Controller; + if (controller == null) + { + throw new InvalidOperationException(Resources.FormatPropertyOfTypeCannotBeNull( + nameof(context.Controller), + nameof(ActionExecutingContext))); + } + + IAsyncActionFilter asyncActionFilter; + IActionFilter actionFilter; + if ((asyncActionFilter = controller as IAsyncActionFilter) != null) + { + await asyncActionFilter.OnActionExecutionAsync(context, next); + } + else if ((actionFilter = controller as IActionFilter) != null) + { + actionFilter.OnActionExecuting(context); + if (context.Result == null) + { + actionFilter.OnActionExecuted(await next()); + } + } + else + { + await next(); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerResultFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerResultFilter.cs new file mode 100644 index 0000000000..3e18e0e587 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/ControllerResultFilter.cs @@ -0,0 +1,52 @@ +// 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. + +using System; +using System.Threading.Tasks; +using Microsoft.AspNet.Mvc.Core; + +namespace Microsoft.AspNet.Mvc.Filters +{ + /// + /// A filter implementation which delegates to the controller for result filter interfaces. + /// + public class ControllerResultFilter : IAsyncResultFilter, IOrderedFilter + { + // Controller-filter methods run farthest from the result by default. + /// + public int Order { get; set; } = int.MinValue; + + /// + public async Task OnResultExecutionAsync( + [NotNull] ResultExecutingContext context, + [NotNull] ResultExecutionDelegate next) + { + var controller = context.Controller; + if (controller == null) + { + throw new InvalidOperationException(Resources.FormatPropertyOfTypeCannotBeNull( + nameof(context.Controller), + nameof(ResultExecutingContext))); + } + + IAsyncResultFilter asyncResultFilter; + IResultFilter resultFilter; + if ((asyncResultFilter = controller as IAsyncResultFilter) != null) + { + await asyncResultFilter.OnResultExecutionAsync(context, next); + } + else if ((resultFilter = controller as IResultFilter) != null) + { + resultFilter.OnResultExecuting(context); + if (!context.Cancel) + { + resultFilter.OnResultExecuted(await next()); + } + } + else + { + await next(); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs index 24bbe8e273..70934c4911 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs @@ -32,12 +32,6 @@ namespace Microsoft.AspNet.Mvc.Filters } } - var controllerFilter = context.ActionContext.Controller as IFilter; - if (controllerFilter != null) - { - InsertControllerAsFilter(context, controllerFilter); - } - if (callNext != null) { callNext(); @@ -73,28 +67,6 @@ namespace Microsoft.AspNet.Mvc.Filters } } - private void InsertControllerAsFilter(FilterProviderContext context, IFilter controllerFilter) - { - var descriptor = new FilterDescriptor(controllerFilter, FilterScope.Controller); - var item = new FilterItem(descriptor, controllerFilter); - - // BinarySearch will return the index of where the item _should_be_ in the list. - // - // If index > 0: - // Other items in the list have the same order and scope - the item was 'found'. - // - // If index < 0: - // No other items in the list have the same order and scope - the item was not 'found' - // Index will be the bitwise compliment of of the 'right' location. - var index = context.Results.BinarySearch(item, FilterItemOrderComparer.Comparer); - if (index < 0) - { - index = ~index; - } - - context.Results.Insert(index, item); - } - private void ApplyFilterToContainer(object actualFilter, IFilter filterMetadata) { Debug.Assert(actualFilter != null, "actualFilter should not be null"); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs index 87481ef450..cb37c4aea9 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs @@ -2,8 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Reflection; +using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.ApplicationModels.DefaultControllerModelBuilderTestControllers; using Xunit; +using System; +using System.Threading.Tasks; namespace Microsoft.AspNet.Mvc.ApplicationModels { @@ -163,6 +166,67 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels Assert.True(isController); } + [Fact] + public void BuildControllerModel_DerivedFromControllerClass_HasFilter() + { + // Arrange + var builder = new AccessibleControllerModelBuilder(); + var typeInfo = typeof(StoreController).GetTypeInfo(); + + // Act + var model = builder.BuildControllerModel(typeInfo); + + // Assert + var filter = Assert.Single(model.Filters); + Assert.IsType(filter); + } + + // This class has a filter attribute, but doesn't implement any filter interfaces, + // so ControllerFilter is not present. + [Fact] + public void BuildControllerModel_ClassWithoutFilterInterfaces_HasNoControllerFilter() + { + // Arrange + var builder = new AccessibleControllerModelBuilder(); + var typeInfo = typeof(NoFiltersController).GetTypeInfo(); + + // Act + var model = builder.BuildControllerModel(typeInfo); + + // Assert + var filter = Assert.Single(model.Filters); + Assert.IsType(filter); + } + + [Fact] + public void BuildControllerModel_ClassWithFilterInterfaces_HasFilter() + { + // Arrange + var builder = new AccessibleControllerModelBuilder(); + var typeInfo = typeof(SomeFiltersController).GetTypeInfo(); + + // Act + var model = builder.BuildControllerModel(typeInfo); + + // Assert + Assert.Single(model.Filters, f => f is ControllerActionFilter); + Assert.Single(model.Filters, f => f is ControllerResultFilter); + } + + [Fact] + public void BuildControllerModel_ClassWithFilterInterfaces_UnsupportedType() + { + // Arrange + var builder = new AccessibleControllerModelBuilder(); + var typeInfo = typeof(UnsupportedFiltersController).GetTypeInfo(); + + // Act + var model = builder.BuildControllerModel(typeInfo); + + // Assert + Assert.Empty(model.Filters); + } + private class AccessibleControllerModelBuilder : DefaultControllerModelBuilder { public AccessibleControllerModelBuilder() @@ -220,4 +284,45 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels.DefaultControllerModelBuilderTe public class PocoController { } + + [Produces("application/json")] + public class NoFiltersController + { + } + + public class SomeFiltersController : IAsyncActionFilter, IResultFilter + { + public Task OnActionExecutionAsync( + [NotNull] ActionExecutingContext context, + [NotNull] ActionExecutionDelegate next) + { + return null; + } + + public void OnResultExecuted([NotNull] ResultExecutedContext context) + { + } + + public void OnResultExecuting([NotNull ]ResultExecutingContext context) + { + } + } + + public class UnsupportedFiltersController : IExceptionFilter, IAuthorizationFilter, IAsyncResourceFilter + { + public void OnAuthorization([NotNull]AuthorizationContext context) + { + throw new NotImplementedException(); + } + + public void OnException([NotNull]ExceptionContext context) + { + throw new NotImplementedException(); + } + + public Task OnResourceExecutionAsync([NotNull]ResourceExecutingContext context, [NotNull]ResourceExecutionDelegate next) + { + throw new NotImplementedException(); + } + } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/DefaultFilterProviderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/DefaultFilterProviderTest.cs index 8b37076e78..b30b13543a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/DefaultFilterProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/DefaultFilterProviderTest.cs @@ -129,74 +129,6 @@ namespace Microsoft.AspNet.Mvc.Filters Assert.Equal(0, item.Descriptor.Order); } - [Fact] - public void DefaultFilterProvider_InsertsController_DefaultOrder() - { - // Arrange - var filter1 = new Mock(); - filter1.SetupGet(f => f.Order).Returns(int.MinValue); - - var filter2 = new Mock(); - filter2.SetupGet(f => f.Order).Returns(int.MinValue); - - var context = CreateFilterContext(new List() - { - new FilterItem(new FilterDescriptor(filter1.Object, FilterScope.Global)), - new FilterItem(new FilterDescriptor(filter2.Object, FilterScope.Action)), - }); - - var controller = new Controller(); - context.ActionContext.Controller = controller; - - var provider = CreateProvider(); - - // Act - provider.Invoke(context, () => { }); - var results = context.Results; - - // Assert - var controllerItem = results[1]; - Assert.Same(controller, controllerItem.Filter); - Assert.Same(controller, controllerItem.Descriptor.Filter); - Assert.Equal(FilterScope.Controller, controllerItem.Descriptor.Scope); - Assert.Equal(int.MinValue, controllerItem.Descriptor.Order); - } - - [Fact] - public void DefaultFilterProvider_InsertsController_CustomOrder() - { - // Arrange - var filter1 = new Mock(); - filter1.SetupGet(f => f.Order).Returns(0); - - var filter2 = new Mock(); - filter2.SetupGet(f => f.Order).Returns(int.MaxValue); - - var context = CreateFilterContext(new List() - { - new FilterItem(new FilterDescriptor(filter1.Object, FilterScope.Global)), - new FilterItem(new FilterDescriptor(filter2.Object, FilterScope.Action)), - }); - - var controller = new Mock(); - controller.SetupGet(f => f.Order).Returns(17); - - context.ActionContext.Controller = controller.Object; - - var provider = CreateProvider(); - - // Act - provider.Invoke(context, () => { }); - var results = context.Results; - - // Assert - var controllerItem = results[1]; - Assert.Same(controller.Object, controllerItem.Filter); - Assert.Same(controller.Object, controllerItem.Descriptor.Filter); - Assert.Equal(FilterScope.Controller, controllerItem.Descriptor.Scope); - Assert.Equal(17, controllerItem.Descriptor.Order); - } - private DefaultFilterProvider CreateProvider() { var services = new ServiceContainer(); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/FiltersTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/FiltersTest.cs index 27ef0e4dcd..6923c888e8 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/FiltersTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/FiltersTest.cs @@ -18,6 +18,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests private readonly IServiceProvider _services = TestHelper.CreateServices("FiltersWebSite"); private readonly Action _app = new FiltersWebSite.Startup().Configure; + // A controller can only be an action filter and result filter, so we don't have entries + // for the other filter types implemented by the controller. [Fact] public async Task ListAllFilters() { @@ -27,11 +29,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var expected = new string[] { - "Controller Override - OnAuthorization", "Global Authorization Filter - OnAuthorization", "On Controller Authorization Filter - OnAuthorization", "Authorize Filter On Action - OnAuthorization", - "Controller Override Resource Filter - OnResourceExecuting", "Global Resource Filter - OnResourceExecuting", "Controller Resource Filter - OnResourceExecuting", "Action Resource Filter - OnResourceExecuting", @@ -55,7 +55,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests "Action Resource Filter - OnResourceExecuted", "Controller Resource Filter - OnResourceExecuted", "Global Resource Filter - OnResourceExecuted", - "Controller Override Resource Filter - OnResourceExecuted", }; // Act @@ -336,7 +335,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("GlobalExceptionFilter.OnException", await response.Content.ReadAsStringAsync()); } - // Controller Override, Action, Controller, and a Global Exception filters are present. + // Action, Controller, and a Global Exception filters are present. // Verifies they are executed in the above mentioned order. [Fact] public async Task ExceptionFilter_Scope() @@ -351,7 +350,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal( - "OnException implemented in Controller, " + "GlobalExceptionFilter.OnException, " + "ControllerExceptionFilter.OnException, " + "Action Exception Filter", diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/Logging/LoggingStartupTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/Logging/LoggingStartupTest.cs index dbb285ee25..f4bfe81024 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/Logging/LoggingStartupTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/Logging/LoggingStartupTest.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using LoggingWebSite; using LoggingWebSite.Controllers; using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.TestHost; using Newtonsoft.Json; @@ -79,10 +80,12 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Empty(controller.ApiExplorer.IsVisible); Assert.Empty(controller.ApiExplorer.GroupName.ToString()); Assert.Empty(controller.Attributes); - Assert.Empty(controller.Filters); Assert.Empty(controller.ActionConstraints); Assert.Empty(controller.RouteConstraints); Assert.Empty(controller.AttributeRoutes); + + var filter = Assert.Single(controller.Filters); + Assert.Equal(typeof(ControllerActionFilter).AssemblyQualifiedName, (string)filter.FilterMetadataType); } [Fact] @@ -96,7 +99,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests dynamic action = logs.First().State; Assert.Equal("Index", action.Name.ToString()); Assert.Empty(action.Parameters); - Assert.Empty(action.FilterDescriptors); Assert.Equal("action", action.RouteConstraints[0].RouteKey.ToString()); Assert.Equal("controller", action.RouteConstraints[1].RouteKey.ToString()); Assert.Empty(action.RouteValueDefaults); @@ -104,6 +106,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Empty(action.HttpMethods.ToString()); Assert.Empty(action.Properties); Assert.Equal("Home", action.ControllerName.ToString()); + + var filter = Assert.Single(action.FilterDescriptors).Filter; + Assert.Equal(typeof(ControllerActionFilter).AssemblyQualifiedName, (string)filter.FilterMetadataType); } private async Task> GetLogsByDataTypeAsync() diff --git a/test/WebSites/FiltersWebSite/Controllers/ExceptionOrderController.cs b/test/WebSites/FiltersWebSite/Controllers/ExceptionOrderController.cs index ef64272c82..79489f15e0 100644 --- a/test/WebSites/FiltersWebSite/Controllers/ExceptionOrderController.cs +++ b/test/WebSites/FiltersWebSite/Controllers/ExceptionOrderController.cs @@ -7,20 +7,12 @@ using Microsoft.AspNet.Mvc; namespace FiltersWebSite { [ControllerExceptionFilter] - public class ExceptionOrderController : Controller, IExceptionFilter + public class ExceptionOrderController : Controller { [HandleInvalidOperationExceptionFilter] public string GetError(string error) { throw new InvalidOperationException(error); } - - public void OnException(ExceptionContext context) - { - if (context.Exception.GetType() == typeof(InvalidOperationException)) - { - context.Result = Helpers.GetContentResult(context.Result, "OnException implemented in Controller"); - } - } } } \ No newline at end of file diff --git a/test/WebSites/FiltersWebSite/Controllers/JsonOnlyController.cs b/test/WebSites/FiltersWebSite/Controllers/JsonOnlyController.cs index 5b549c1cff..a5800b93d1 100644 --- a/test/WebSites/FiltersWebSite/Controllers/JsonOnlyController.cs +++ b/test/WebSites/FiltersWebSite/Controllers/JsonOnlyController.cs @@ -1,13 +1,15 @@ // 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. +using System; using System.Linq; using Microsoft.AspNet.Mvc; namespace FiltersWebSite.Controllers { + [JsonOnly] [Route("Json")] - public class JsonOnlyController : Controller, IResourceFilter + public class JsonOnlyController : Controller { [HttpPost] public string Post([FromBody] DummyClass dummy) @@ -15,16 +17,19 @@ namespace FiltersWebSite.Controllers return (dummy?.SampleInt ?? 0).ToString(); } - public void OnResourceExecuted(ResourceExecutedContext context) + private class JsonOnlyAttribute : Attribute, IResourceFilter { - } + public void OnResourceExecuted(ResourceExecutedContext context) + { + } - public void OnResourceExecuting(ResourceExecutingContext context) - { - var jsonFormatter = context.InputFormatters.OfType().Single(); + public void OnResourceExecuting(ResourceExecutingContext context) + { + var jsonFormatter = context.InputFormatters.OfType().Single(); - context.InputFormatters.Clear(); - context.InputFormatters.Add(jsonFormatter); + context.InputFormatters.Clear(); + context.InputFormatters.Add(jsonFormatter); + } } } } \ No newline at end of file diff --git a/test/WebSites/FiltersWebSite/Controllers/ProductsController.cs b/test/WebSites/FiltersWebSite/Controllers/ProductsController.cs index a039fefa6c..7fa13d9cab 100644 --- a/test/WebSites/FiltersWebSite/Controllers/ProductsController.cs +++ b/test/WebSites/FiltersWebSite/Controllers/ProductsController.cs @@ -11,7 +11,7 @@ namespace FiltersWebSite [ControllerActionFilter] [ControllerAuthorizationFilter] [TracingResourceFilter("Controller Resource Filter")] - public class ProductsController : Controller, IResultFilter, IAuthorizationFilter, IResourceFilter + public class ProductsController : Controller, IResultFilter { [PassThroughResultFilter] [PassThroughActionFilter] @@ -44,24 +44,5 @@ namespace FiltersWebSite { context.HttpContext.Response.Headers.Append("filters", "Controller Override - OnResultExecuting"); } - - public void OnAuthorization(AuthorizationContext context) - { - context.HttpContext.Response.Headers.Append("filters", "Controller Override - OnAuthorization"); - } - - public void OnResourceExecuting(ResourceExecutingContext context) - { - context.HttpContext.Response.Headers.Append( - "filters", - "Controller Override Resource Filter - OnResourceExecuting"); - } - - public void OnResourceExecuted(ResourceExecutedContext context) - { - context.HttpContext.Response.Headers.Append( - "filters", - "Controller Override Resource Filter - OnResourceExecuted"); - } } } \ No newline at end of file