Fix for #384 - And some other changes to controller as filter

This is a major change to how we handle the scenario where a controller is
a filter. We want to change the lifetime of the controller object, by
scoping it around action filters and result filters. This means that a
controller class can only implement action filters and result filters.

To implement #384 - we're creating a delegating filter class
'ControllerFilter' which will forward calls to the implementation of the
controller. This is discovered in the controller model and added to the
filter collection. This filter is removable as an opt-out of this feature.

The ControllerFilter only implements action filter and result filter, so
the new restriction about filter types on Controller is in place. A future
change will move the instantiation of the controller to after resource
filters.
This commit is contained in:
Ryan Nowak 2015-01-14 18:35:20 -08:00
parent 51e7812e7e
commit 12c2759cec
12 changed files with 251 additions and 150 deletions

View File

@ -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;
}

View File

@ -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;
}
}
/// <summary>
/// Creates a <see cref="ViewResult"/> object that renders a view to the response.
/// </summary>

View File

@ -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
{
/// <summary>
/// A filter implementation which delegates to the controller for action filter interfaces.
/// </summary>
public class ControllerActionFilter : IAsyncActionFilter, IOrderedFilter
{
// Controller-filter methods run farthest from the action by default.
/// <inheritdoc />
public int Order { get; set; } = int.MinValue;
/// <inheritdoc />
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();
}
}
}
}

View File

@ -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
{
/// <summary>
/// A filter implementation which delegates to the controller for result filter interfaces.
/// </summary>
public class ControllerResultFilter : IAsyncResultFilter, IOrderedFilter
{
// Controller-filter methods run farthest from the result by default.
/// <inheritdoc />
public int Order { get; set; } = int.MinValue;
/// <inheritdoc />
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();
}
}
}
}

View File

@ -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");

View File

@ -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<ControllerActionFilter>(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<ProducesAttribute>(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();
}
}
}

View File

@ -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<IOrderedFilter>();
filter1.SetupGet(f => f.Order).Returns(int.MinValue);
var filter2 = new Mock<IOrderedFilter>();
filter2.SetupGet(f => f.Order).Returns(int.MinValue);
var context = CreateFilterContext(new List<FilterItem>()
{
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<IOrderedFilter>();
filter1.SetupGet(f => f.Order).Returns(0);
var filter2 = new Mock<IOrderedFilter>();
filter2.SetupGet(f => f.Order).Returns(int.MaxValue);
var context = CreateFilterContext(new List<FilterItem>()
{
new FilterItem(new FilterDescriptor(filter1.Object, FilterScope.Global)),
new FilterItem(new FilterDescriptor(filter2.Object, FilterScope.Action)),
});
var controller = new Mock<IOrderedFilter>();
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();

View File

@ -18,6 +18,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
private readonly IServiceProvider _services = TestHelper.CreateServices("FiltersWebSite");
private readonly Action<IApplicationBuilder> _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",

View File

@ -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<IEnumerable<LogInfoDto>> GetLogsByDataTypeAsync<T>()

View File

@ -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");
}
}
}
}

View File

@ -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<JsonInputFormatter>().Single();
public void OnResourceExecuting(ResourceExecutingContext context)
{
var jsonFormatter = context.InputFormatters.OfType<JsonInputFormatter>().Single();
context.InputFormatters.Clear();
context.InputFormatters.Add(jsonFormatter);
context.InputFormatters.Clear();
context.InputFormatters.Add(jsonFormatter);
}
}
}
}

View File

@ -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");
}
}
}