21

If I create a BaseController in my Asp.Net Core 2.0 web application that capsulizes some of the common dependencies are they still necessary in the actual controllers.

For Example, the standard Account and Manage controllers in a default MVC 6 web application.

public class AccountController : Controller
{
    private readonly UserManager<ApplicationUser> _userManager;
    private readonly SignInManager<ApplicationUser> _signInManager;
    private readonly IEmailSender _emailSender;
    private readonly ILogger _logger;

    public AccountController(
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<AccountController> logger)
    {
        _userManager = userManager;
        _signInManager = signInManager;
        _emailSender = emailSender;
        _logger = logger;
    }
   //rest of code removed
}

public class ManageController : Controller
{
    private readonly UserManager<ApplicationUser> _userManager;
    private readonly SignInManager<ApplicationUser> _signInManager;
    private readonly IEmailSender _emailSender;
    private readonly ILogger _logger;
    private readonly UrlEncoder _urlEncoder;

    private const string AuthenicatorUriFormat = "otpauth://totp/{0}:{1}?secret={2}&issuer={0}&digits=6";

    public ManageController(
      UserManager<ApplicationUser> userManager,
      SignInManager<ApplicationUser> signInManager,
      IEmailSender emailSender,
      ILogger<ManageController> logger,
      UrlEncoder urlEncoder)
    {
        _userManager = userManager;
        _signInManager = signInManager;
        _emailSender = emailSender;
        _logger = logger;
        _urlEncoder = urlEncoder;
    }
    // rest of code removed
}

In the custom web application template I am building I refactor the Account Controller to three different Controllers, RegisterController(Which handles everything regarding a user registration), LoginController(which handles login and logout), and the balance into a third. I split the Manage Controller in two, a ManagePasswordController(everything related to passwords) and a UserManageController(everything else).

There is a lot of commonality in the DI requirements for each and I want to put them in a BaseController. To look something like this?

public abstract class BaseController : Controller
{
    private readonly IConfiguration _config;
    private readonly IEmailSender _emailSender;
    private readonly ILogger _logger;
    private readonly SignInManager<ApplicationUser> _signInManager;
    private readonly UserManager<ApplicationUser> _userManager;

     protected BaseController(IConfiguration iconfiguration,
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<ManageController> logger)
    {
        _config = iconfiguration;
        _userManager = userManager;
        _signInManager = signInManager;
        _emailSender = emailSender;
        _logger = logger;
    }
    //rest of code removed
}

But it seems like that accomplishes nothing? because it seems to me I still have to inject everything. I can't be right (I'm new to DI so clearly have no clue) but the BaseController should allow me to do NO DI that's common between BaseController and RegisterController. Am I wrong? How do I accomplish what I am trying to do?

public class RegisterController : BaseController
{
    private const string ConfirmedRegistration = "User created a new account with password.";

    private readonly UserManager<ApplicationUser> _userManager;
    private readonly SignInManager<ApplicationUser> _signInManager;
    private readonly IEmailSender _emailSender;
    private readonly ILogger _logger;
    private readonly IConfiguration _config;

     public RegisterController(
        IConfiguration config,
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<AccountController> logger) : base(config, userManager, signInManager, emailSender, logger)

    {
        _userManager = userManager;
        _signInManager = signInManager;
        _emailSender = emailSender;
        _logger = logger;
        _config = config;
    }
    //rest of code removed
}

Update

Per Sir Rufo's suggestion

public abstract class BaseController : Controller
{
    protected UserManager<ApplicationUser> UserManager { get; }
    protected SignInManager<ApplicationUser> SignInManager { get; }
    protected IConfiguration Config { get; }
    protected IEmailSender EmailSender { get; }
    protected ILogger AppLogger { get; }

    protected BaseController(IConfiguration iconfiguration,
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<ManageController> logger)
    {
        AppLogger = logger;
        EmailSender = emailSender;
        Config = iconfiguration;
        SignInManager = signInManager;
        UserManager = userManager; 
    }
}

And the inheriting controller

public class TestBaseController : BaseController
{

    public TestBaseController() : base()
    {

    }
}

This doesn't work. Resharper is telling me I have to add the parameters to the base constructor call in the TestBaseController constructor.

Also should BaseController be inheriting from Controller or ControllerBase in .Net Core 2.0?

17
  • 1
    If the BaseController requires you to provide all of them in the constructor call, then yes. But you could refactor the BaseController to not need all of those. Also, why save references in both the BaseController and it's subclasses? Commented Nov 18, 2017 at 22:03
  • @fredrik..Well the 5 controllers I mention in the post, all need those DI's. Is that all I need to do in the actual controllers? Just eliminate the private references and use the parameters passed in? i.e signInManager.IssignedIn, instead of _signInManager.IsSignedIn? Commented Nov 18, 2017 at 22:20
  • 1
    Publish the injected references via protected properties in the BaseController to have access to them from any derived class Commented Nov 18, 2017 at 22:25
  • @Sir Rufo..not sure what Publish means in this context. Example? Commented Nov 18, 2017 at 22:26
  • you know protected properties? Commented Nov 18, 2017 at 22:27

3 Answers 3

47

The Microsoft.AspNetCore.MVC.Controller class comes with the extension method

HttpContext.RequestServices.GetService<T>

Which can be used whenever the HttpContext is available in the pipeline (e.g. The HttpContext property will be Null if called from the controller's constructor)

Try this pattern

Note: make sure you include this directive using Microsoft.Extensions.DependencyInjection;

Base Controller

public abstract class BaseController<T> : Controller where T: BaseController<T>
{

    private ILogger<T> _logger;

    protected ILogger<T> Logger => _logger ?? (_logger = HttpContext.RequestServices.GetService<ILogger<T>>());

Child Controller

[Route("api/authors")]
public class AuthorsController : BaseController<AuthorsController>
{

    public AuthorsController(IAuthorRepository authorRepository)
    {
        _authorRepository = authorRepository;
    }

    [HttpGet("LogMessage")]
    public IActionResult LogMessage(string message)
    {
        Logger.LogInformation(message);

        return Ok($"The following message has been logged: '{message}'");
    }

Needless to say, remember to register your services in the Startup.cs --> ConfingureServices method

Sign up to request clarification or add additional context in comments.

2 Comments

I had to write using Microsoft.Extensions.DependencyInjection; to be able to use the generic GetService method.
This is the real code i need in base and derived controller! The feature is that the code doesn't pollute derived controller's ctor and stuff like dbContext, logger and other things put in BaseController.
8

There are very few good reasons to use a BaseController in MVC. A base controller in this scenario only adds more code to maintain, with no real benefit.

For true cross-cutting concerns, the most common way to handle them in MVC is to use global filters, although there are a few new options worth considering in MVC core.

However, your problem doesn't look like a cross-cutting concern so much as a violation of the Single Responsibility Principle. That is, having more than 3 injected dependencies is a code smell that your controller is doing too much. The most practical solution would be to Refactor to Aggregate Services.

In this case, I would argue that you have at least 1 implicit service that you need to make explicit - namely, UserManager and SignInManager should be wrapped into a service of its own. From there, you could potentially inject your other 3 dependencies into that service (depending on how they are used, of course). So, you could potentially whittle this down to a single dependency for both the AccountController and ManageController.

Some signs that a controller is doing too much:

  1. There are a lot of "helper" methods that contain business logic that are shared between actions.
  2. Action methods are doing more than simple HTTP request/response stuff. An action method should generally do nothing but call services that process input and/or produce output and return views and response codes.

In cases such as these, it is worth a look to see if you can move that logic into a service of their own and any shared logic into dependencies of that service, etc.

1 Comment

@NightOwl888...and yet the default web application for MVC 6 comes with an AccountController that has 4 injected dependencies and ManageController with 5 injected dependencies. This was the whole purpose of this exercise, to reduce the injected dependencies that were common, to one base class so the newly created controllers could just take them from the base class. I am not a professional programmer, just trying to learn Core 2.0 to build my own web app template as a learning mechanism. An example for " you have at least 1 implicit service that you need to make explicit ", would be helpful
1

Per suggestions from both Calc and Sir Rufo, this works.

 public abstract class BaseController : Controller
{
    protected UserManager<ApplicationUser> UserManager { get; }
    protected SignInManager<ApplicationUser> SignInManager { get; }
    protected IConfiguration Config { get; }
    protected IEmailSender EmailSender { get; }
    protected ILogger AppLogger { get; }

    protected BaseController(IConfiguration iconfiguration,
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<ManageController> logger)
    {
        AppLogger = logger;
        EmailSender = emailSender;
        Config = iconfiguration;
        SignInManager = signInManager;
        UserManager = userManager; 
    }

    protected BaseController()
    {
    }
}

The parameters still have to be injected into the inherited controller and passed to the base constructor

public class TestBaseController : BaseController
{
    public static IConfigurationRoot Configuration { get; set; }

    public TestBaseController(IConfiguration config,
        UserManager<ApplicationUser> userManager,
        SignInManager<ApplicationUser> signInManager,
        IEmailSender emailSender,
        ILogger<ManageController> logger) : base(config,userManager,signInManager,emailSender,logger)
    {
    }

    public string TestConfigGetter()
    {

        var t = Config["ConnectionStrings:DefaultConnection"];
        return t;
    }

    public class TestViewModel
    {
        public string ConnString { get; set; }
    }
    public IActionResult Index()
    {
        var tm = new TestViewModel { ConnString = TestConfigGetter() };
        return View(tm);
    }
}

So now all the injected objects will have instances.

Was hoping the final solution would not require injecting the commonly needed instances into each inherited controller, only any additional instance objects required for that specific controller. All I really solved from a code repeating aspect was the removal of the private fields in each Controller.

Still wondering if the BaseController should inherit from Controller or ControllerBase?

1 Comment

This is what we are trying to avoid.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.