huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Changing when controllers are created 

This change moves controller creation to the stage immediately before
model binding. The controller will be disposed/released before Resource
Filters run their 'OnResourceExecuted' method. Previously the controller's
lifetime surrounded all filter invocation.

Additionally, the Controller property is now gone from ActionContext, and
is moved to the 4 filter contexts that should have access to it
Action*Context and Result*Context.
This commit is contained in:
Ryan Nowak 2015-01-15 15:37:01 -08:00
parent e1069dbf65
commit 0c5a702245
15 changed files with 184 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Microsoft.AspNet.Mvc.Core/ActionContext.cs
View File

@ -14,7 +14,6 @@ namespace Microsoft.AspNet.Mvc
: this(actionContext.HttpContext, actionContext.RouteData, actionContext.ActionDescriptor)
{
ModelState = actionContext.ModelState;
Controller = actionContext.Controller;
}
public ActionContext([NotNull] RouteContext routeContext, [NotNull] ActionDescriptor actionDescriptor)
@ -39,10 +38,5 @@ namespace Microsoft.AspNet.Mvc
public ModelStateDictionary ModelState { get; private set; }
public ActionDescriptor ActionDescriptor { get; private set; }
/// <summary>
/// The controller is available only after the controller factory runs.
/// </summary>
public object Controller { get; set; }
}
}

19
src/Microsoft.AspNet.Mvc.Core/ControllerActionInvoker.cs
View File

@ -50,21 +50,16 @@ namespace Microsoft.AspNet.Mvc
}
}
public async override Task InvokeAsync()
protected override object CreateInstance()
{
// The binding context is used in activation
Debug.Assert(ActionBindingContext != null);
var controller = _controllerFactory.CreateController(ActionContext);
return _controllerFactory.CreateController(ActionContext);
}
try
{
ActionContext.Controller = controller;
await base.InvokeAsync();
}
finally
{
_controllerFactory.ReleaseController(ActionContext.Controller);
}
protected override void ReleaseInstance(object instance)
{
_controllerFactory.ReleaseController(instance);
}
protected override async Task<IActionResult> InvokeActionAsync(ActionExecutingContext actionExecutingContext)
@ -72,7 +67,7 @@ namespace Microsoft.AspNet.Mvc
var actionMethodInfo = _descriptor.MethodInfo;
var actionReturnValue = await ControllerActionExecutor.ExecuteAsync(
actionMethodInfo,
ActionContext.Controller,
actionExecutingContext.Controller,
actionExecutingContext.ActionArguments);
var actionResult = CreateActionResult(

1
src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs
View File

@ -37,7 +37,6 @@ namespace Microsoft.AspNet.Mvc
_serviceProvider,
actionDescriptor.ControllerTypeInfo.AsType());
actionContext.Controller = controller;
_controllerActivator.Activate(controller, actionContext);
return controller;

84
src/Microsoft.AspNet.Mvc.Core/FilterActionInvoker.cs
View File

@ -55,8 +55,6 @@ namespace Microsoft.AspNet.Mvc
_modelValidatorProviderProvider = modelValidatorProviderProvider;
_valueProviderFactoryProvider = valueProviderFactoryProvider;
_actionBindingContextAccessor = actionBindingContextAccessor;
ActionBindingContext = new ActionBindingContext();
}
protected ActionContext ActionContext { get; private set; }
@ -73,6 +71,21 @@ namespace Microsoft.AspNet.Mvc
}
}
protected object Instance { get; private set; }
/// <summary>
/// Called to create an instance of an object which will act as the reciever of the action invocation.
/// </summary>
/// <returns>The constructed instance or <c>null</c>.</returns>
protected abstract object CreateInstance();
/// <summary>
/// Called to create an instance of an object which will act as the reciever of the action invocation.
/// </summary>
/// <param name="instance">The instance to release.</param>
/// <remarks>This method will not be called if <see cref="CreateInstance"/> returns <c>null</c>.</remarks>
protected abstract void ReleaseInstance(object instance);
protected abstract Task<IActionResult> InvokeActionAsync(ActionExecutingContext actionExecutingContext);
protected abstract Task<IDictionary<string, object>> GetActionArgumentsAsync(
@ -95,7 +108,20 @@ namespace Microsoft.AspNet.Mvc
return;
}
await InvokeAllResourceFiltersAsync();
try
{
await InvokeAllResourceFiltersAsync();
}
finally
{
// Release the instance after all filters have run. We don't need to surround
// Authorizations filters because the instance will be created much later than
// that.
if (Instance != null)
{
ReleaseInstance(Instance);
}
}
// We've reached the end of resource filters. If there's an unhandled exception on the context then
// it should be thrown and middleware has a chance to handle it.
@ -207,7 +233,7 @@ namespace Microsoft.AspNet.Mvc
if (item.FilterAsync != null)
{
await item.FilterAsync.OnResourceExecutionAsync(
_resourceExecutingContext,
_resourceExecutingContext,
InvokeResourceFilterAsync);
if (_resourceExecutedContext == null)
@ -384,23 +410,30 @@ namespace Microsoft.AspNet.Mvc
Debug.Assert(_resourceExecutingContext != null);
Debug.Assert(ActionBindingContext != null);
ActionBindingContext = new ActionBindingContext();
ActionBindingContext.InputFormatters = _resourceExecutingContext.InputFormatters;
ActionBindingContext.ModelBinder = new CompositeModelBinder(_resourceExecutingContext.ModelBinders);
ActionBindingContext.ValidatorProvider = new CompositeModelValidatorProvider(
_resourceExecutingContext.ValidatorProviders);
var valueProviderFactoryContext = new ValueProviderFactoryContext(
ActionContext.HttpContext,
ActionContext.HttpContext,
ActionContext.RouteData.Values);
ActionBindingContext.ValueProvider = CompositeValueProvider.Create(
_resourceExecutingContext.ValueProviderFactories,
valueProviderFactoryContext);
Instance = CreateInstance();
var arguments = await GetActionArgumentsAsync(ActionContext, ActionBindingContext);
_actionExecutingContext = new ActionExecutingContext(ActionContext, _filters, arguments);
_actionExecutingContext = new ActionExecutingContext(
ActionContext,
_filters,
arguments,
Instance);
await InvokeActionFilterAsync();
}
@ -429,7 +462,10 @@ namespace Microsoft.AspNet.Mvc
if (_actionExecutedContext == null)
{
// If we get here then the filter didn't call 'next' indicating a short circuit
_actionExecutedContext = new ActionExecutedContext(_actionExecutingContext, _filters)
_actionExecutedContext = new ActionExecutedContext(
_actionExecutingContext,
_filters,
Instance)
{
Canceled = true,
Result = _actionExecutingContext.Result,
@ -443,7 +479,10 @@ namespace Microsoft.AspNet.Mvc
if (_actionExecutingContext.Result != null)
{
// Short-circuited by setting a result.
_actionExecutedContext = new ActionExecutedContext(_actionExecutingContext, _filters)
_actionExecutedContext = new ActionExecutedContext(
_actionExecutingContext,
_filters,
Instance)
{
Canceled = true,
Result = _actionExecutingContext.Result,
@ -457,7 +496,10 @@ namespace Microsoft.AspNet.Mvc
else
{
// All action filters have run, execute the action method.
_actionExecutedContext = new ActionExecutedContext(_actionExecutingContext, _filters)
_actionExecutedContext = new ActionExecutedContext(
_actionExecutingContext,
_filters,
Instance)
{
Result = await InvokeActionAsync(_actionExecutingContext),
};
@ -466,7 +508,10 @@ namespace Microsoft.AspNet.Mvc
catch (Exception exception)
{
// Exceptions thrown by the action method OR filters bubble back up through ActionExcecutedContext.
_actionExecutedContext = new ActionExecutedContext(_actionExecutingContext, _filters)
_actionExecutedContext = new ActionExecutedContext(
_actionExecutingContext,
_filters,
Instance)
{
ExceptionDispatchInfo = ExceptionDispatchInfo.Capture(exception)
};
@ -478,7 +523,7 @@ namespace Microsoft.AspNet.Mvc
{
_cursor.SetStage(FilterStage.ResultFilters);
_resultExecutingContext = new ResultExecutingContext(ActionContext, _filters, result);
_resultExecutingContext = new ResultExecutingContext(ActionContext, _filters, result, Instance);
await InvokeResultFilterAsync();
Debug.Assert(_resultExecutingContext != null);
@ -525,7 +570,8 @@ namespace Microsoft.AspNet.Mvc
_resultExecutedContext = new ResultExecutedContext(
_resultExecutingContext,
_filters,
_resultExecutingContext.Result)
_resultExecutingContext.Result,
Instance)
{
Canceled = true,
};
@ -536,7 +582,8 @@ namespace Microsoft.AspNet.Mvc
_resultExecutedContext = new ResultExecutedContext(
_resultExecutingContext,
_filters,
_resultExecutingContext.Result)
_resultExecutingContext.Result,
Instance)
{
Canceled = true,
};
@ -552,7 +599,8 @@ namespace Microsoft.AspNet.Mvc
_resultExecutedContext = new ResultExecutedContext(
_resultExecutingContext,
_filters,
_resultExecutingContext.Result)
_resultExecutingContext.Result,
Instance)
{
Canceled = true,
};
@ -570,7 +618,8 @@ namespace Microsoft.AspNet.Mvc
_resultExecutedContext = new ResultExecutedContext(
_resultExecutingContext,
_filters,
_resultExecutingContext.Result);
_resultExecutingContext.Result,
Instance);
}
}
catch (Exception exception)
@ -578,7 +627,8 @@ namespace Microsoft.AspNet.Mvc
_resultExecutedContext = new ResultExecutedContext(
_resultExecutingContext,
_filters,
_resultExecutingContext.Result)
_resultExecutingContext.Result,
Instance)
{
ExceptionDispatchInfo = ExceptionDispatchInfo.Capture(exception)
};

6
src/Microsoft.AspNet.Mvc.Core/Filters/ActionExecutedContext.cs
View File

@ -14,13 +14,17 @@ namespace Microsoft.AspNet.Mvc
public ActionExecutedContext(
[NotNull] ActionContext actionContext,
[NotNull] IList<IFilter> filters)
[NotNull] IList<IFilter> filters,
object controller)
: base(actionContext, filters)
{
Controller = controller;
}
public virtual bool Canceled { get; set; }
public virtual object Controller { get; }
public virtual Exception Exception
{
get

8
src/Microsoft.AspNet.Mvc.Core/Filters/ActionExecutingContext.cs
View File

@ -10,14 +10,18 @@ namespace Microsoft.AspNet.Mvc
public ActionExecutingContext(
[NotNull] ActionContext actionContext,
[NotNull] IList<IFilter> filters,
[NotNull] IDictionary<string, object> actionArguments)
[NotNull] IDictionary<string, object> actionArguments,
object controller)
: base(actionContext, filters)
{
ActionArguments = actionArguments;
Controller = controller;
}
public virtual IActionResult Result { get; set; }
public virtual IDictionary<string, object> ActionArguments { get; private set; }
public virtual IDictionary<string, object> ActionArguments { get; }
public virtual object Controller { get; }
}
}

6
src/Microsoft.AspNet.Mvc.Core/Filters/ResultExecutedContext.cs
View File

@ -15,14 +15,18 @@ namespace Microsoft.AspNet.Mvc
public ResultExecutedContext(
[NotNull] ActionContext actionContext,
[NotNull] IList<IFilter> filters,
[NotNull] IActionResult result)
[NotNull] IActionResult result,
object controller)
: base(actionContext, filters)
{
Result = result;
Controller = controller;
}
public virtual bool Canceled { get; set; }
public virtual object Controller { get; }
public virtual Exception Exception
{
get

6
src/Microsoft.AspNet.Mvc.Core/Filters/ResultExecutingContext.cs
View File

@ -10,12 +10,16 @@ namespace Microsoft.AspNet.Mvc
public ResultExecutingContext(
[NotNull] ActionContext actionContext,
[NotNull] IList<IFilter> filters,
[NotNull] IActionResult result)
[NotNull] IActionResult result,
object controller)
: base(actionContext, filters)
{
Result = result;
Controller = controller;
}
public virtual object Controller { get; }
public virtual IActionResult Result { get; set; }
public virtual bool Cancel { get; set; }

63
test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs
View File

@ -296,6 +296,7 @@ namespace Microsoft.AspNet.Mvc
// Assert
filter1.Verify(f => f.OnAuthorization(It.IsAny<AuthorizationContext>()), Times.Once());
Assert.False(invoker.ControllerFactory.CreateCalled);
}
[Fact]
@ -325,6 +326,8 @@ namespace Microsoft.AspNet.Mvc
filter1.Verify(
f => f.OnAuthorizationAsync(It.IsAny<AuthorizationContext>()),
Times.Once());
Assert.False(invoker.ControllerFactory.CreateCalled);
}
[Fact]
@ -1735,6 +1738,7 @@ namespace Microsoft.AspNet.Mvc
// Assert
Assert.Same(expected.Object, context.Result);
Assert.True(context.Canceled);
Assert.False(invoker.ControllerFactory.CreateCalled);
}
[Fact]
@ -1786,6 +1790,7 @@ namespace Microsoft.AspNet.Mvc
// Assert
Assert.Same(expected.Object, context.Result);
Assert.True(context.Canceled);
Assert.False(invoker.ControllerFactory.CreateCalled);
}
[Fact]
@ -1847,6 +1852,8 @@ namespace Microsoft.AspNet.Mvc
resourceFilter.Verify(
f => f.OnResourceExecutionAsync(It.IsAny<ResourceExecutingContext>(), It.IsAny<ResourceExecutionDelegate>()),
Times.Never());
Assert.False(invoker.ControllerFactory.CreateCalled);
}
[Fact]
@ -1969,10 +1976,6 @@ namespace Microsoft.AspNet.Mvc
routeData: new RouteData(),
actionDescriptor: actionDescriptor);
var controllerFactory = new Mock<IControllerFactory>();
controllerFactory.Setup(c => c.CreateController(It.IsAny<ActionContext>())).Returns(this);
controllerFactory.Setup(m => m.ReleaseController(this)).Verifiable();
var filterProvider = new Mock<INestedProviderManager<FilterProviderContext>>(MockBehavior.Strict);
filterProvider
.Setup(fp => fp.Invoke(It.IsAny<FilterProviderContext>()))
@ -1985,7 +1988,7 @@ namespace Microsoft.AspNet.Mvc
var invoker = new TestControllerActionInvoker(
actionContext,
filterProvider.Object,
controllerFactory,
new MockControllerFactory(this),
actionDescriptor,
inputFormattersProvider.Object,
Mock.Of<IControllerActionArgumentBinder>(),
@ -1997,8 +2000,6 @@ namespace Microsoft.AspNet.Mvc
return invoker;
}
[Fact]
public async Task Invoke_UsesDefaultValuesIfNotBound()
{
@ -2097,14 +2098,47 @@ namespace Microsoft.AspNet.Mvc
}
}
public class TestControllerActionInvoker : ControllerActionInvoker
private class MockControllerFactory : IControllerFactory
{
private Mock<IControllerFactory> _factoryMock;
private object _controller;
public MockControllerFactory(object controller)
{
_controller = controller;
}
public bool CreateCalled { get; private set; }
public bool ReleaseCalled { get; private set; }
public object CreateController(ActionContext actionContext)
{
CreateCalled = true;
return _controller;
}
public void ReleaseController(object controller)
{
Assert.NotNull(controller);
Assert.Same(_controller, controller);
ReleaseCalled = true;
}
public void Verify()
{
if (CreateCalled && !ReleaseCalled)
{
Assert.False(true, "ReleaseController should have been called.");
}
}
}
private class TestControllerActionInvoker : ControllerActionInvoker
{
public TestControllerActionInvoker(
ActionContext actionContext,
INestedProviderManager<FilterProviderContext> filterProvider,
Mock<IControllerFactory> controllerFactoryMock,
MockControllerFactory controllerFactory,
ControllerActionDescriptor descriptor,
IInputFormattersProvider inputFormattersProvider,
IControllerActionArgumentBinder controllerActionArgumentBinder,
@ -2115,7 +2149,7 @@ namespace Microsoft.AspNet.Mvc
: base(
actionContext,
filterProvider,
controllerFactoryMock.Object,
controllerFactory,
descriptor,
inputFormattersProvider,
controllerActionArgumentBinder,
@ -2124,14 +2158,17 @@ namespace Microsoft.AspNet.Mvc
valueProviderFactoryProvider,
actionBindingContext)
{
_factoryMock = controllerFactoryMock;
ControllerFactory = controllerFactory;
}
public MockControllerFactory ControllerFactory { get; }
public async override Task InvokeAsync()
{
await base.InvokeAsync();
_factoryMock.Verify();
// Make sure that the controller was disposed in every test that creates ones.
ControllerFactory.Verify();
}
}
}

25
test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerActivatorTest.cs
View File

@ -29,10 +29,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
.Returns(services);
var routeContext = new RouteContext(httpContext.Object);
var controller = new TestController();
var context = new ActionContext(routeContext, new ActionDescriptor())
{
Controller = controller
};
var context = new ActionContext(routeContext, new ActionDescriptor());
var activator = new DefaultControllerActivator();
// Act
@ -55,10 +52,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
.Returns(services);
var routeContext = new RouteContext(httpContext.Object);
var controller = new TestController();
var context = new ActionContext(routeContext, new ActionDescriptor())
{
Controller = controller
};
var context = new ActionContext(routeContext, new ActionDescriptor());
var activator = new DefaultControllerActivator();
// Act
@ -83,10 +77,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
var routeContext = new RouteContext(httpContext.Object);
var controller = new TestController();
var context = new ActionContext(routeContext, new ActionDescriptor())
{
Controller = controller
};
var context = new ActionContext(routeContext, new ActionDescriptor());
var activator = new DefaultControllerActivator();
@ -109,10 +100,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
.Returns(services);
var routeContext = new RouteContext(httpContext.Object);
var controller = new TestController();
var context = new ActionContext(routeContext, new ActionDescriptor())
{
Controller = controller
};
var context = new ActionContext(routeContext, new ActionDescriptor());
var activator = new DefaultControllerActivator();
// Act
@ -135,10 +123,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
.Returns(services);
var routeContext = new RouteContext(httpContext.Object);
var controller = new TestController();
var context = new ActionContext(routeContext, new ActionDescriptor())
{
Controller = controller
};
var context = new ActionContext(routeContext, new ActionDescriptor());
var activator = new DefaultControllerActivator();
// Act

10
test/Microsoft.AspNet.Mvc.Core.Test/Filters/ActionFilterAttributeTests.cs
View File

@ -227,12 +227,13 @@ namespace Microsoft.AspNet.Mvc.Test
return new ActionExecutingContext(
CreateActionContext(),
new IFilter[] { filter, },
new Dictionary<string, object>());
new Dictionary<string, object>(),
controller: new object());
}
private static ActionExecutedContext CreateActionExecutedContext(ActionExecutingContext context)
{
return new ActionExecutedContext(context, context.Filters)
return new ActionExecutedContext(context, context.Filters, context.Controller)
{
Result = context.Result,
};
@ -243,12 +244,13 @@ namespace Microsoft.AspNet.Mvc.Test
return new ResultExecutingContext(
CreateActionContext(),
new IFilter[] { filter, },
new NoOpResult());
new NoOpResult(),
controller: new object());
}
private static ResultExecutedContext CreateResultExecutedContext(ResultExecutingContext context)
{
return new ResultExecutedContext(context, context.Filters, context.Result);
return new ResultExecutedContext(context, context.Filters, context.Result, context.Controller);
}
private static ActionContext CreateActionContext()

5
test/Microsoft.AspNet.Mvc.Core.Test/Filters/ProducesAttributeTests.cs
View File

@ -62,7 +62,7 @@ namespace Microsoft.AspNet.Mvc.Test
private static ResultExecutedContext CreateResultExecutedContext(ResultExecutingContext context)
{
return new ResultExecutedContext(context, context.Filters, context.Result);
return new ResultExecutedContext(context, context.Filters, context.Result, context.Controller);
}
private static ResultExecutingContext CreateResultExecutingContext(IFilter filter)
@ -70,7 +70,8 @@ namespace Microsoft.AspNet.Mvc.Test
return new ResultExecutingContext(
CreateActionContext(),
new IFilter[] { filter, },
new ObjectResult("Some Value"));
new ObjectResult("Some Value"),
controller: new object());
}
private static ActionContext CreateActionContext()

21
test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs
View File

@ -190,12 +190,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
})
.Returns(Task.FromResult(result: false));
var actionContext = new ActionContext(
new RouteContext(Mock.Of<HttpContext>()),
actionDescriptor)
{
Controller = Mock.Of<object>(),
};
var actionContext = new ActionContext(new RouteContext(Mock.Of<HttpContext>()), actionDescriptor);
var actionBindingContext = new ActionBindingContext()
{
@ -243,12 +238,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
})
.Returns(Task.FromResult(result: true));
var actionContext = new ActionContext(
new RouteContext(Mock.Of<HttpContext>()),
actionDescriptor)
{
Controller = Mock.Of<object>(),
};
var actionContext = new ActionContext(new RouteContext(Mock.Of<HttpContext>()), actionDescriptor);
var actionBindingContext = new ActionBindingContext()
{
@ -303,12 +293,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test
})
.Returns(Task.FromResult(result: true));
var actionContext = new ActionContext(
new RouteContext(Mock.Of<HttpContext>()),
actionDescriptor)
{
Controller = Mock.Of<object>(),
};
var actionContext = new ActionContext(new RouteContext(Mock.Of<HttpContext>()), actionDescriptor);
var actionBindingContext = new ActionBindingContext()
{

3
test/Microsoft.AspNet.Mvc.Core.Test/ResponseCacheAttributeTest.cs
View File

@ -236,7 +236,8 @@ namespace Microsoft.AspNet.Mvc
return new ActionExecutingContext(
new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()),
filters ?? new List<IFilter>(),
new Dictionary<string, object>());
new Dictionary<string, object>(),
new object());
}
}
}

31
test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpResponseExceptionActionFilterTest.cs
View File

@ -34,12 +34,17 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim
{
// Arrange
var filter = new HttpResponseExceptionActionFilter();
var context = new ActionExecutingContext(new ActionContext(
new DefaultHttpContext(),
new RouteData(),
actionDescriptor: Mock.Of<ActionDescriptor>()),
filters: Mock.Of<IList<IFilter>>(),
actionArguments: new Dictionary<string, object>());
var actionContext = new ActionContext(
new DefaultHttpContext(),
new RouteData(),
Mock.Of<ActionDescriptor>());
var context = new ActionExecutingContext(
actionContext,
filters: new List<IFilter>(),
actionArguments: new Dictionary<string, object>(),
controller: new object());
// Act
filter.OnActionExecuting(context);
@ -56,12 +61,16 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim
var httpContext = new DefaultHttpContext();
httpContext.Request.Method = "GET";
var actionContext = new ActionContext(
httpContext,
new RouteData(),
Mock.Of<ActionDescriptor>());
var context = new ActionExecutedContext(
new ActionContext(
httpContext,
new RouteData(),
actionDescriptor: Mock.Of<ActionDescriptor>()),
filters: null);
actionContext,
filters: new List<IFilter>(),
controller: new object());
context.Exception = new HttpResponseException(HttpStatusCode.BadRequest);
// Act