Fix #2151 - Part 4 remove [Activate] support from controllers.

This change completely removes [Activate]. In a controller, you should
constructor injection or [FromServices] to access services.

To access context items (ActionContext, ActionBindingContext, root
ViewDataDictionary) you should use the respected attribute class.

We'd like to consider streamlining this further in the future by getting
down to a single injectable context for controllers, but for now this will
have to do.
This commit is contained in:
Ryan Nowak 2015-05-21 15:02:54 -07:00
parent af5322e2ce
commit 144c1d3cf4
14 changed files with 147 additions and 156 deletions

View File

@ -9,8 +9,12 @@ namespace MvcSample.Web
[Route("ApiExplorer")]
public class ApiExplorerController : Controller
{
[Activate]
public IApiDescriptionGroupCollectionProvider Provider { get; set; }
public ApiExplorerController(IApiDescriptionGroupCollectionProvider provider)
{
Provider = provider;
}
public IApiDescriptionGroupCollectionProvider Provider { get; }
[HttpGet]
public IActionResult All()

View File

@ -12,11 +12,10 @@ namespace MvcSample.Web.RandomNameSpace
{
private User _user = new User() { Name = "User Name", Address = "Home Address" };
[Activate]
public HttpResponse Response
{
get; set;
}
[ActionContext]
public ActionContext ActionContext { get; set; }
public HttpResponse Response => ActionContext.HttpContext.Response;
public string Index()
{

View File

@ -0,0 +1,17 @@
// 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;
namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Specifies that a controller property should be set with the current
/// <see cref="ActionBindingContext"/> when creating the controller. The property must have a public
/// set method.
/// </summary>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class ActionBindingContextAttribute : Attribute
{
}
}

View File

@ -0,0 +1,17 @@
// 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;
namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Specifies that a controller property should be set with the current
/// <see cref="ActionContext"/> when creating the controller. The property must have a public
/// set method.
/// </summary>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class ActionContextAttribute : Attribute
{
}
}

View File

@ -1,16 +0,0 @@
// 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;
namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Specifies that a property or parameter value should be initialized via the dependency injection
/// container for activated types.
/// </summary>
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false)]
public sealed class ActivateAttribute : Attribute
{
}
}

View File

@ -101,7 +101,7 @@ namespace Microsoft.AspNet.Mvc
/// <see cref="IControllerActivator"/> activates this property while activating controllers. If user codes
/// directly instantiate controllers, the getter returns an empty <see cref="Mvc.ActionContext"/>.
/// </remarks>
[Activate]
[ActionContext]
public ActionContext ActionContext
{
get
@ -124,7 +124,7 @@ namespace Microsoft.AspNet.Mvc
/// <summary>
/// Gets or sets the <see cref="ActionBindingContext"/>.
/// </summary>
[Activate]
[ActionBindingContext]
public ActionBindingContext BindingContext { get; set; }
/// <summary>
@ -161,7 +161,7 @@ namespace Microsoft.AspNet.Mvc
/// However, when controllers are directly instantiated in user codes, this property is initialized with
/// <see cref="EmptyModelMetadataProvider"/>.
/// </remarks>
[Activate]
[ViewDataDictionary]
public ViewDataDictionary ViewData
{
get

View File

@ -6,7 +6,6 @@ using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Mvc.Core;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.Framework.DependencyInjection;
@ -20,10 +19,8 @@ namespace Microsoft.AspNet.Mvc
public class DefaultControllerFactory : IControllerFactory
{
private readonly IControllerActivator _controllerActivator;
private readonly IDictionary<Type, Func<ActionContext, object>> _valueAccessorLookup;
private readonly ConcurrentDictionary<Type, PropertyActivator<ActionContext>[]> _activateActions;
private readonly Func<Type, PropertyActivator<ActionContext>[]> _getPropertiesToActivate;
private readonly Func<Type, Func<ActionContext, object>> _getRequiredService = GetRequiredService;
/// <summary>
/// Initializes a new instance of <see cref="DefaultControllerFactory"/>.
@ -33,7 +30,7 @@ namespace Microsoft.AspNet.Mvc
public DefaultControllerFactory(IControllerActivator controllerActivator)
{
_controllerActivator = controllerActivator;
_valueAccessorLookup = CreateValueAccessorLookup();
_activateActions = new ConcurrentDictionary<Type, PropertyActivator<ActionContext>[]>();
_getPropertiesToActivate = GetPropertiesToActivate;
}
@ -98,8 +95,9 @@ namespace Microsoft.AspNet.Mvc
protected virtual void ActivateProperties([NotNull] object controller, [NotNull] ActionContext context)
{
var controllerType = controller.GetType();
var propertiesToActivate = _activateActions.GetOrAdd(controllerType,
_getPropertiesToActivate);
var propertiesToActivate = _activateActions.GetOrAdd(
controllerType,
_getPropertiesToActivate);
for (var i = 0; i < propertiesToActivate.Length; i++)
{
@ -108,79 +106,40 @@ namespace Microsoft.AspNet.Mvc
}
}
/// <summary>
/// Returns a <see cref="IDictionary{TKey, TValue}"/> of property types to delegates used to activate
/// controller properties annotated with <see cref="ActivateAttribute"/>.
/// </summary>
/// <returns>A dictionary containing the property type to activator delegate mapping.</returns>
/// <remarks>Override this method to provide custom activation behavior for controller properties
/// annotated with <see cref="ActivateAttribute"/>.</remarks>
protected virtual IDictionary<Type, Func<ActionContext, object>> CreateValueAccessorLookup()
{
var dictionary = new Dictionary<Type, Func<ActionContext, object>>
{
{ typeof(ActionContext), (context) => context },
{ typeof(HttpContext), (context) => context.HttpContext },
{ typeof(HttpRequest), (context) => context.HttpContext.Request },
{ typeof(HttpResponse), (context) => context.HttpContext.Response },
{
typeof(ViewDataDictionary),
(context) =>
{
var serviceProvider = context.HttpContext.RequestServices;
return new ViewDataDictionary(
serviceProvider.GetRequiredService<IModelMetadataProvider>(),
context.ModelState);
}
},
{
typeof(ActionBindingContext),
(context) =>
{
var serviceProvider = context.HttpContext.RequestServices;
var accessor = serviceProvider.GetRequiredService<IScopedInstance<ActionBindingContext>>();
return accessor.Value;
}
}
};
return dictionary;
}
private PropertyActivator<ActionContext>[] GetPropertiesToActivate(Type type)
{
var activatorsForActivateProperties = PropertyActivator<ActionContext>.GetPropertiesToActivate(
type,
typeof(ActivateAttribute),
CreateActivateInfo);
return activatorsForActivateProperties;
IEnumerable<PropertyActivator<ActionContext>> activators;
activators = PropertyActivator<ActionContext>.GetPropertiesToActivate(
type,
typeof(ActionContextAttribute),
p => new PropertyActivator<ActionContext>(p, c => c));
activators = activators.Concat(PropertyActivator<ActionContext>.GetPropertiesToActivate(
type,
typeof(ActionBindingContextAttribute),
p => new PropertyActivator<ActionContext>(p, GetActionBindingContext)));
activators = activators.Concat(PropertyActivator<ActionContext>.GetPropertiesToActivate(
type,
typeof(ViewDataDictionaryAttribute),
p => new PropertyActivator<ActionContext>(p, GetViewDataDictionary)));
return activators.ToArray();
}
private PropertyActivator<ActionContext> CreateActivateInfo(
PropertyInfo property)
private static ActionBindingContext GetActionBindingContext(ActionContext context)
{
Func<ActionContext, object> valueAccessor;
if (!_valueAccessorLookup.TryGetValue(property.PropertyType, out valueAccessor))
{
var message = Resources.FormatControllerFactory_PropertyCannotBeActivated(
property.Name,
property.DeclaringType.FullName);
throw new InvalidOperationException(message);
}
return new PropertyActivator<ActionContext>(property, valueAccessor);
var serviceProvider = context.HttpContext.RequestServices;
var accessor = serviceProvider.GetRequiredService<IScopedInstance<ActionBindingContext>>();
return accessor.Value;
}
private PropertyActivator<ActionContext> CreateFromServicesInfo(
PropertyInfo property)
private static ViewDataDictionary GetViewDataDictionary(ActionContext context)
{
var valueAccessor = _getRequiredService(property.PropertyType);
return new PropertyActivator<ActionContext>(property, valueAccessor);
}
private static Func<ActionContext, object> GetRequiredService(Type propertyType)
{
return actionContext => actionContext.HttpContext.RequestServices.GetRequiredService(propertyType);
var serviceProvider = context.HttpContext.RequestServices;
return new ViewDataDictionary(
serviceProvider.GetRequiredService<IModelMetadataProvider>(),
context.ModelState);
}
}
}

View File

@ -0,0 +1,17 @@
// 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;
namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Specifies that a controller property should be set with the current
/// <see cref="ViewDataDictionary"/> when creating the controller. The property must have a public
/// set method.
/// </summary>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class ViewDataDictionaryAttribute : Attribute
{
}
}

View File

@ -27,14 +27,14 @@ namespace System.Web.Http
/// Gets the action context.
/// </summary>
/// <remarks>The setter is intended for unit testing purposes only.</remarks>
[Activate]
[ActionContext]
public ActionContext ActionContext { get; set; }
/// <summary>
/// Gets the <see cref="ActionBindingContext"/>.
/// </summary>
/// <remarks>The setter is intended for unit testing purposes only.</remarks>
[Activate]
[ActionBindingContext]
public ActionBindingContext BindingContext { get; set; }
/// <summary>

View File

@ -72,7 +72,7 @@ namespace Microsoft.AspNet.Mvc.Core
// Arrange
var actionDescriptor = new ControllerActionDescriptor
{
ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo()
ControllerTypeInfo = typeof(ControllerWithAttributes).GetTypeInfo()
};
var services = GetServices();
var httpContext = new DefaultHttpContext
@ -86,9 +86,8 @@ namespace Microsoft.AspNet.Mvc.Core
var result = factory.CreateController(context);
// Assert
var controller = Assert.IsType<ControllerWithActivateAndFromServices>(result);
var controller = Assert.IsType<ControllerWithAttributes>(result);
Assert.Same(context, controller.ActionContext);
Assert.Same(httpContext, controller.HttpContext);
}
[Fact]
@ -97,7 +96,7 @@ namespace Microsoft.AspNet.Mvc.Core
// Arrange
var actionDescriptor = new ControllerActionDescriptor
{
ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo()
ControllerTypeInfo = typeof(ControllerWithAttributes).GetTypeInfo()
};
var services = GetServices();
@ -112,8 +111,8 @@ namespace Microsoft.AspNet.Mvc.Core
var result = factory.CreateController(context);
// Assert
var controller = Assert.IsType<ControllerWithActivateAndFromServices>(result);
Assert.NotNull(controller.GetViewData());
var controller = Assert.IsType<ControllerWithAttributes>(result);
Assert.NotNull(controller.ViewData);
}
[Fact]
@ -122,7 +121,7 @@ namespace Microsoft.AspNet.Mvc.Core
// Arrange
var actionDescriptor = new ControllerActionDescriptor
{
ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo()
ControllerTypeInfo = typeof(ControllerWithAttributes).GetTypeInfo()
};
var bindingContext = new ActionBindingContext();
@ -139,17 +138,17 @@ namespace Microsoft.AspNet.Mvc.Core
var result = factory.CreateController(context);
// Assert
var controller = Assert.IsType<ControllerWithActivateAndFromServices>(result);
var controller = Assert.IsType<ControllerWithAttributes>(result);
Assert.Same(bindingContext, controller.BindingContext);
}
[Fact]
public void CreateController_IgnoresPropertiesThatAreNotDecoratedWithActivateAttribute()
public void CreateController_IgnoresPropertiesThatAreNotDecoratedWithAttribute()
{
// Arrange
var actionDescriptor = new ControllerActionDescriptor
{
ControllerTypeInfo = typeof(ControllerWithActivateAndFromServices).GetTypeInfo()
ControllerTypeInfo = typeof(ControllerWithoutAttributes).GetTypeInfo()
};
var services = GetServices();
var httpContext = new DefaultHttpContext
@ -163,8 +162,8 @@ namespace Microsoft.AspNet.Mvc.Core
var result = factory.CreateController(context);
// Assert
var controller = Assert.IsType<ControllerWithActivateAndFromServices>(result);
Assert.Null(controller.Response);
var controller = Assert.IsType<ControllerWithoutAttributes>(result);
Assert.Null(controller.ActionContext);
}
[Fact]
@ -185,8 +184,10 @@ namespace Microsoft.AspNet.Mvc.Core
// Act and Assert
var exception = Assert.Throws<InvalidOperationException>(() => factory.CreateController(context));
Assert.Equal("The property 'Service' on controller '" + typeof(ControllerThatCannotBeActivated) +
"' cannot be activated.", exception.Message);
Assert.Equal(
$"Unable to resolve service for type '{typeof(TestService).FullName}' while attempting to activate " +
$"'{typeof(ControllerThatCannotBeActivated).FullName}'.",
exception.Message);
}
[Theory]
@ -211,8 +212,9 @@ namespace Microsoft.AspNet.Mvc.Core
// Act and Assert
var exception = Assert.Throws<InvalidOperationException>(() => factory.CreateController(context));
Assert.Equal("The type '" + type.FullName + "' cannot be activated by '" + typeof(DefaultControllerFactory) +
"' because it is either a value type, an interface, an abstract class or an open generic type.",
Assert.Equal(
$"The type '{type.FullName}' cannot be activated by '{typeof(DefaultControllerFactory).FullName}' " +
"because it is either a value type, an interface, an abstract class or an open generic type.",
exception.Message);
}
@ -250,8 +252,6 @@ namespace Microsoft.AspNet.Mvc.Core
.Returns(Mock.Of<IUrlHelper>());
services.Setup(s => s.GetService(typeof(IModelMetadataProvider)))
.Returns(metadataProvider);
services.Setup(s => s.GetService(typeof(TestService)))
.Returns(new TestService());
services.Setup(s => s.GetService(typeof(IObjectModelValidator)))
.Returns(new DefaultObjectValidator(new IExcludeTypeValidationFilter[0], metadataProvider));
services
@ -262,40 +262,25 @@ namespace Microsoft.AspNet.Mvc.Core
return services.Object;
}
private class ControllerWithActivateAndFromServices
private class ControllerWithoutAttributes
{
[Activate]
public ActionContext ActionContext { get; set; }
[Activate]
public ActionBindingContext BindingContext { get; set; }
[Activate]
public HttpContext HttpContext { get; set; }
public ViewDataDictionary ViewData { get; set; }
}
[Activate]
protected HttpRequest Request { get; set; }
private class ControllerWithAttributes
{
[ActionContext]
public ActionContext ActionContext { get; set; }
[Activate]
private ViewDataDictionary ViewData { get; set; }
[ActionBindingContext]
public ActionBindingContext BindingContext { get; set; }
[FromServices]
public IUrlHelper Helper { get; set; }
[FromServices]
public TestService TestService { get; set; }
public HttpResponse Response { get; set; }
public ViewDataDictionary GetViewData()
{
return ViewData;
}
public HttpRequest GetHttpRequest()
{
return Request;
}
[ViewDataDictionary]
public ViewDataDictionary ViewData { get; set; }
}
private class MyController : Controller
@ -310,8 +295,12 @@ namespace Microsoft.AspNet.Mvc.Core
private class ControllerThatCannotBeActivated
{
[Activate]
public TestService Service { get; set; }
public ControllerThatCannotBeActivated(TestService service)
{
Service = service;
}
public TestService Service { get; }
}
private class TestService

View File

@ -23,8 +23,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
var client = server.CreateClient();
var expectedMessage = "The property 'Service' on controller 'ActivatorWebSite.CannotBeActivatedController' " +
"cannot be activated.";
var expectedMessage =
"Unable to resolve service for type 'ActivatorWebSite.CannotBeActivatedController+FakeType' while " +
"attempting to activate 'ActivatorWebSite.CannotBeActivatedController'.";
// Act & Assert
var response = await client.GetAsync("http://localhost/CannotBeActivated/Index");

View File

@ -7,15 +7,16 @@ namespace ActivatorWebSite
{
public class CannotBeActivatedController
{
[Activate]
private FakeType Service { get; set; }
public CannotBeActivatedController(FakeType service)
{
}
public IActionResult Index()
{
return new NoContentResult();
}
private sealed class FakeType
public sealed class FakeType
{
}
}

View File

@ -11,11 +11,12 @@ namespace ActivatorWebSite
[FromServices]
public MyService Service { get; set; }
[Activate]
public HttpRequest Request { get; set; }
[ActionContext]
public ActionContext ActionContext { get; set; }
[Activate]
public HttpResponse Response { get; set; }
public HttpRequest Request => ActionContext.HttpContext.Request;
public HttpResponse Response => ActionContext.HttpContext.Response;
public IActionResult Index()
{

View File

@ -19,8 +19,10 @@ namespace ControllersFromServicesClassLibrary
private QueryValueService QueryService { get; }
[Activate]
public HttpRequest Request { get; set; }
[ActionContext]
public ActionContext ActionContext { get; set; }
public HttpRequest Request => ActionContext.HttpContext.Request;
[HttpGet("/constructorinjection")]
public IActionResult Index()