59

In my ASP.Net Core MVC 6 solution I have two sets of controllers. One set contains the webpages with their regular views. Another set contains the API controllers.

To avoid duplicating db logic the web controllers are using the API controllers. Currently I am creating an instance of the required controller manually by handing it a DbContext as constructor argument. This is the DbContext given to web controller by dependency injection.

But whenever I add another constructor parameter to the API controller I need to modify all web controllers that use this API controller.

How can I use the dependency injection system builtin to ASP.Net 5 to create an instance of the required API controller for me? Then it would fill in the required constructor parameters automatically.

One solution could be to move the db logic from the API controllers to a separate layer and call that from both API and web controllers. This would not solve my problem since the new layer would still need the same parameters and I'm not fan of the unnecessary wiring.

Another solution would be to have the web controllers access the API through a web call, but that just adds complexity to the app.

Today I am doing this:

public IActionResult Index()
{
    using (var foobarController = new Areas.Api.Controllers.FoobarController(
        // All of these has to be in the constructor of this controller so they can be passed on to the ctor of api controller
        _dbContext, _appEnvironment, 
        _userManager, _roleManager, 
        _emailSender, _smsSender))
    {
        var model = new IndexViewModel();
        model.Foo = foobarController.List(new FoobarRequest() { Foo = true, Bar = false });
        model.Bar = foobarController.List(new FoobarRequest() { Foo = false, Bar = true });
        return View(model);
    }
}

And I am hoping for something like this: (This example does not work.)

using (var foobarController = CallContextServiceLocator.Locator.ServiceProvider.GetService<Areas.Api.Controllers.FoobarController>())
{
    var model = new IndexViewModel();
    model.Foo = foobarController.List(new FoobarRequest() { Foo = true, Bar = false });
    model.Bar = foobarController.List(new FoobarRequest() { Foo = false, Bar = true });
    return View(model);
}
Tedd Hansen
  • 12,074
  • 14
  • 61
  • 97
  • 6
    As some people mentioned here, this a bad practice and it may lead to some unwanted/unpredicted bugs; e.g., HttpContext would be null. I recommend reconsidering your logic. You might "_not be a fan of unnecessary wiring_", but this is **NECESSARY** wiring! – Mojtaba Oct 15 '20 at 08:55
  • 2
    A ton of extra code will probably also lead to some unwanted/unpredictable bugs, especially when any small change must propagate through a hierarchy of highly unnecessary plumbing. It is in no way necessary. We are allowed be pragmatic about code even though theoretical what ifs dictate an elaborate scheme to solve a simple problem. – Tedd Hansen Oct 16 '20 at 13:33
  • 3
    Tedd, I don't agree with your concern on duplicating code, because the code is part of a different structural layer, that's separated purposely. You wouldn't call steel that is on one pillar of a bridge, a duplicate of steel on the other pillar. Nor would you combine the two pillars to be more efficient, considering they are both structurally necessary. So your argument is then left with "it's a lot to manage" but again, it's proven itself necessary at least in my projects, for any application that's more complex than a simple grid, but I'm not sure its on-topic, you have to inject regardless. – Dan Chase Jul 05 '21 at 15:17
  • I have worked on a few projects over the past decades, I have seen some ... lets call it less pragmatic stuff. I think what we can/should agree upon is that it is not right to claim there is only a single way everything must be solved because one (of many) design principles dictates it. Nobody would react if these were nicely layered classes, but since they are controllers then suddenly its a horror show with the what ifs. And you build a bridge by structural integrity at lowest possible cost, not layers for the sake of principles. – Tedd Hansen Jul 06 '21 at 14:03
  • If you are familiar with the class you'll be working with (for example b/c you wrote it yourself) there's no reason not to get it injected from services. – gjvdkamp Jan 08 '22 at 19:59

6 Answers6

46

How can I use the dependency injection system builtin to ASP.Net 5 to create an instance of the required API controller for me?

In your Startup.cs can tell the MVC to register all your controllers as services.

services.AddMvc().AddControllersAsServices();

Then you can simply inject the desired controller in your other controller via the DI mechanism and invoke its action method.

Felix K.
  • 14,171
  • 9
  • 58
  • 72
  • 8
    In my case, the properties on the Controller, like HttpContext and UrlHelper were null, which does not seem ok. – Mladen B. Apr 17 '19 at 13:37
  • In my case I do `return otherController.OtherAction()`, and it's still invoking the current controller View. In my case it's crashing because Model is null and current view requires Model. – drizin May 29 '21 at 23:33
39

Don't do it. Move that logic to another component that gets shared between the 2 controllers. The controller is dispatched to by the framework as a result of an HTTP call, its not your public API surface. In general, your controllers should be used as a the place where the HTTP request is transformed into business objects. Operations on those objects should be delegate to another layer (especially if it needs to be used from more than one place in your application).

davidfowl
  • 37,120
  • 7
  • 93
  • 103
  • 2
    That does not solve anything at all. To quote myself: "One solution could be to move the db logic from the API controllers to a separate layer and call that from both API and web controllers. This would not solve my problem since the new layer would still need the same parameters and I'm not fan of the unnecessary wiring." – Tedd Hansen Jan 25 '16 at 14:42
  • 5
    Use the dependency injection system to get an instance of that thing. I don't see why that's a problem. You can solve anything in programming with some indirection :D – davidfowl Jan 25 '16 at 15:53
  • @davidfowl How about plugin system where you don't have access to type at compile time? Let's say I am loading controller from an external assembly. – Akash Kava Oct 22 '18 at 19:12
  • Use the plugin system directly instead of using MVC as your communication mechanism between plugins – davidfowl Apr 08 '19 at 16:30
  • I've got into this situation where two controllers had to do similar things and the solution as @davidfowl said was a shared function on some other class created for that propose – CREM Apr 08 '19 at 16:43
  • @davidfowl Could you please add more explanation (with possible references) why do you think that is a bad practice? – Mojtaba May 02 '19 at 10:17
  • 3
    This is probably the "right" answer, although the answers showing how it *could* be done are helpful for understanding how the DI system works (and/or doesn't) with Controllers. – hemp May 22 '19 at 17:41
19

To be able to use a controller from another controller you need to:

  1. Register the controller in Startup.cs ConfigureServices: services.AddTransient <Areas.Api.Controllers.FoobarController, Areas.Api.Controllers.FoobarController>();
  2. You must pass the controller you want to access as a ctor parameter into the main controller.

If you need to access local properties in the controller such as User or Url there are two ways to do this.

The first way is to use DI to get an instance of IHttpContextAccessor to access User and IUrlHelper to access Url objects:

public class FoobarController : Controller
{
    private readonly ApplicationDbContext _dbContext;
    private readonly IHttpContextAccessor _httpContextAccessor;
    private readonly IUrlHelper _urlHelper;
    public FoobarController(ApplicationDbContext dbContext, IHttpContextAccessor httpContextAccessor, IUrlHelper _urlHelper, [...])
    {
         _dbContext = dbContext;
         _httpContextAccessor = httpContextAccessor;
         _urlHelper = urlHelper;
    }

    public FoobarResponse List(FoobarRequest request)
    {
        var userId = _httpContextAccessor.HttpContext.User.GetUserId();
        var response = new FoobarResponse();
        response.List = _dbContext.Foobars.Where(f => f.UserId == userId).ToList();
        response.Thumb = 
        return response;
    }
}

The second way is to set it in the calling controller:

public class HomeController : Controller
{
    private Areas.Api.Controllers.FoobarController _foobarController;
    public HomeController(Areas.Api.Controllers.FoobarController foobarController)
    {
        _foobarController = foobarController;
    }

    private void InitControllers()
    {
        // We can't set this at Ctor because we don't have our local copy yet
        // Access to Url 
        _foobarController.Url = Url;
        // Access to User
        _foobarController.ActionContext = ActionContext;
        // For more references see https://github.com/aspnet/Mvc/blob/6.0.0-rc1/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs
        // Note: This will change in RC2
    }

    public IActionResult Index()
    {
        InitControllers();

        var model = new IndexViewModel();
        model.Foo = _foobarController.List(new FoobarRequest() { Foo = true, Bar = false });
        model.Bar = _foobarController.List(new FoobarRequest() { Foo = false, Bar = true });
        return View(model);
    }
}

The source code for ASP.Net Core MVC6 RC1 Controller can be found here. It is however undergoing heavy rewrite for RC2 and with it the properties that has to be copied to get access to User and Url will change.

Tedd Hansen
  • 12,074
  • 14
  • 61
  • 97
  • I would like to note that in RC2 this seems to be solved differently. I haven't looked closely at the code, but it looks like there might be a more "official" way to do it in the future. See `CreateController` on https://github.com/aspnet/Mvc/blob/2b0bea675ec55dc752aacd47223376045a9be1ab/src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerFactory.cs – Tedd Hansen Jan 28 '16 at 08:57
  • I hope registering in services using Scoped instead of transient is ok. That is what I did. – Post Impatica Dec 08 '22 at 15:53
1

@B12Toaster is correct for MVC but if you only use ApiController you should do it like this:

services.AddControllers().AddControllersAsServices();
Ogglas
  • 62,132
  • 37
  • 328
  • 418
0

Why would your new layer need wiring up? Why not take in an object into both controllers and call a method on that object. The DI container could resolve the dependencies of this new object without duplicated wiring couldn't it?

ie you could have this:

public class MvcController
{
    SharedComponent sharedComponent;
    public MvcController(SharedComponent sharedComponent)
    {
        this.sharedComponent = sharedComponent;
    }
    public IActionResult Index()
    {
        var model = new IndexViewModel();
        model.Foo = shredComponent.List(new FoobarRequest() { Foo = true, Bar = false });
        model.Bar = shredComponent.List(new FoobarRequest() { Foo = false, Bar = true });
        return View(model);   
    }
}

//Repeat this for the API controller

public class SharedComponent
{
    public SharedComponent(DBContext dbContext, AppEnvironment appEnvironment, UserManager userManager, RoleManager roleManager, 
        EmailSender emailSender, SmsSender smsSender)
    {
       ...Store in fields for later usage
    }
}
Tedd Hansen
  • 12,074
  • 14
  • 61
  • 97
Sam Holder
  • 32,535
  • 13
  • 101
  • 181
  • 1
    For every new method in the SharedComponent I would have to wire up a mirrored call from ApiController. In practice duplicating all signatures and return values and having two sets to maintain. That is extra wiring which seems unnecessary when the class is already there. This is a preference and some people are fine with that. – Tedd Hansen Jan 28 '16 at 08:54
0

I'd have to agree with others that injecting the controller may not be the best route. Mostly because it marries the business logic with ASP.Net instead of treating it like an IO device like, in my opinion, it should be.

Let's say we have an interface that looks like this:

public interface ICalculator {
    int Add(int left, int right);
}

and we have an implementation that stores the business logic:

public class MyCalculator : ICalculator {
    public int Add(int left, int right) => left + right;
}

This implementation can be used as a background service, within the same process as a WPF application, or as an ASP.NET WebAPI controller. It would look something like this:

[ApiController]
[Route("api/{controller}")]
public void CalculatorController : Controller, ICalculator {
    private readonly ICalculator _calculator;

    public CalculatorController(ICalculator calc) => _calculator = calc;

    [Route("Add")]
    public int Add(int left, int right) => _calculator.Add(left, right);
}

If that controller has a dependency on a repository you can inject that interface too. Personally I like defining a collection of repositories (like IUserRepository for example) and injecting only what is needed instead of the entire DbContext.

public CalculatorController(ICalculator calculator, IDbContext db) { }

There's nothing wrong with a controller depending on more than just the thing it is decorating. Just make sure you have a set of tests that assert various things. For example you could assert that when a particular controller method is called the particular method on the other interface is also called.

Personally I find this approach a better fit. It's okay to use certain technologies but they should be kept at arm's length from the business rules. A developer should be able to take the business rules that govern a particular part of the code and switch from a WCF service to ASP.NET WebAPI trivially.

I've personally been a part of a couple projects where we had to switch from one database technology to another (SQL Server to CouchDB) and one where our micro-services needed to be running as restful Web API services instead of Windows services. If you architect things this way those types of projects become relatively trivial compared to how things are normally composed.

Logan K
  • 324
  • 2
  • 8
  • I don't get the difference, aren't you still injecting either way? I understand the structural recommendation, but everyone here seems to have a different approach at doing the same thing (injecting) – Dan Chase Jul 05 '21 at 15:12
  • @DanChase I agree. Ultimately it is the same suggestion of "inject your rules into the controller." I was going more into detail on why, in my opinion, you inject certain things and not other things. I don't like the way it feels having one ApiController being injected into a different one and instead injecting what rules the one controller needs (and maybe the two controllers depend on some of the same rules). – Logan K Jul 07 '21 at 02:54