Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

ControllerActivator should be able to use controllers registered as services #1707

Closed
danroth27 opened this issue Dec 11, 2014 · 6 comments
Closed

Comments

@danroth27
Copy link
Member

Something like:

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddMvc().WithControllersFromDI();
    }
}

This would replace the default activator with one that gets the controllers from DI.
This replaces #1700
CC @lodejard @davidfowl @halter73 @pranavkm @yishaigalatzer

@danroth27
Copy link
Member Author

We think we can do this without the additional extension method.

@danroth27 danroth27 modified the milestones: 6.0.0-rc1, 6.0.0-beta3 Jan 10, 2015
@pranavkm
Copy link
Contributor

Based on discussion with @lodejard \ @yishaigalatzer

  1. Move the code that currently exists in the DefaultControllerActivator into the DefaultControllerFactory.
  2. Change the signature of the IControllerActivator to have a method public object Activate(ActionContext context). When WithControllersFromDI is invoked, we'll replace the default TypeActivatedControllerActivator with ServiceBasedControlActivator that creates controllers from the service provider.
  3. Remove use of ITypeActivator in TypeActivatedControllerActivator and use the static activator method instead.

pranavkm added a commit that referenced this issue Jan 28, 2015
services

* Added WithControllersFromServiceProvider that replaces the default
  controller activator with a service based one.
* Move activation to DefaultControllerFactory
* Modify [Activate] behavior so that it no longer activates services. Use
  [FromService] attribute to hydrate services

Fixes #1707
@pranavkm
Copy link
Contributor

Additional requirements:

During controller discovery, controller types in the application that are not registered in the service container should not be considered as controllers. The intent is to

  1. Use registration as a way to determine which controllers are available for an instance of the Mvc application.
  2. Ensure that accessing controller that are not registered result in 404 and not a 500 (due to GetRequiredService throwing).

Implementation design:

  • Introduce a type (ServiceProviderControllerRegistration?) that allow registration of controller types. As part of registering a controller as a service, controllers must be additionally registered with this type.
  • Introduce IControllerProvider that will be consumed by the ControllerModelBuilder. In the default case, IControllerProvider will perform the scanning of controllers. This code exists in the DefaultControllerModelBuilder today.
  • ServiceBasedControllerProvider will be an additional implementation of IControllerProvider that will use the controller registration to produce a sequence of controllers that the application will use.

Usage:

services.AddMvc()
             .WithControllersFromDI()
             .RegisterController(typeof(FooController))
             .RegisterController<QuxController>()
             .RegisterControllers(new[] { typeof(BarController), typeof(BazController) });

pranavkm added a commit that referenced this issue Feb 5, 2015
services

* Added WithControllersFromServiceProvider that replaces the default
  controller activator with a service based one.
* Move activation to DefaultControllerFactory
* Modify [Activate] behavior so that it no longer activates services. Use
  [FromService] attribute to hydrate services

Fixes #1707
pranavkm added a commit that referenced this issue Feb 8, 2015
services

* Added WithControllersFromServiceProvider that replaces the default
  controller activator with a service based one.
* Move activation to DefaultControllerFactory
* Modify [Activate] behavior so that it no longer activates services. Use
  [FromService] attribute to hydrate services

Fixes #1707
pranavkm added a commit that referenced this issue Feb 10, 2015
services

* Added WithControllersFromServiceProvider that replaces the default
  controller activator with a service based one.
* Move activation to DefaultControllerFactory
* Modify [Activate] behavior so that it no longer activates services. Use
  [FromService] attribute to hydrate services

Fixes #1707
@avanderhoorn
Copy link
Member

Just wondering, why was this implemented the way that it is instead of tapping into the options system? Meaning why do we have FixedSetControllerTypeProvider which has its collection populated via WithControllersAsServices instead of it being like the other "Default" providers, and have a Controllers property on MvcOptions which it pulls in the values from. Where was the semantic line in the sand?

My only thought is that you were trying to make it more explicit and if it was on MvcOptions the property would sometimes be used and not others (depending on whether the FixedSetControllerTypeProvider was being used vs the DefaultControllerTypeProvider).

@yishaigalatzer
Copy link
Contributor

That's a good question. It's mostly because the controllers list is computed as late as possible, and options typically have specific types rather than providers. Honestly this could have gone both ways.

@avanderhoorn
Copy link
Member

@yishaigalatzer Thanks for filling me in. I was just confused when I saw two different strategies for doing very similar things.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants