Create instance of interface with dependency injection

Question

I have a base controller and before every page load I want to get the current user. I originally had a constructor in my BaseController that looked like this

public BaseController(ISystemUserCommand command)
{
    _systemUserCommand = command
}

The problem with this then is that every controller that inherits from the BaseController would have to contain the ISystemUserCommand in its constructor, which I don't think would be good.

Instead I tried to create just an instance of the service class (shown below - it's the commented line under var sid...) but I need to pass in user service. How would I pass in the user service here or is this a bad way of doing it?

public abstract class BaseController : Controller
{
    public SystemUserViewModel CurrentUser { get; set; }

    private readonly ISystemUserCommand _systemUserCommand;

    public SystemUserViewModel GetCurrentUser()
    {
        if (HttpContext == null || HttpContext.User == null) return null;
        if (CurrentUser != null) return CurrentUser;

        var sid = System.Web.HttpContext.Current.Request.LogonUserIdentity.User.ToString();

        //var command = new SystemUserCommand();

        CurrentUser = _systemUserCommand.GetUser(sid);

        return CurrentUser;
    }

    public void SetUserInformation(SystemUserViewModel currentUser)
    {
        ViewBag.UserId = currentUser.SystemUserId;
        ViewBag.FullName = string.Format("{0} {1}", currentUser.FirstName, currentUser.LastName);
        ViewBag.FirstName = currentUser.FirstName;
        ViewBag.LastName = currentUser.LastName;
        ViewBag.CurrentUser = currentUser;
    }

    protected override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        var currentUser = GetCurrentUser();

        if (currentUser != null)
        {
            if (currentUser.IsActive)
            {
                SetUserInformation(currentUser);
            }
            else
                filterContext.Result = RedirectToAction("denied", "unauthorized");
        }
        else
            filterContext.Result = RedirectToAction("denied", "unauthorized");

        base.OnActionExecuting(filterContext);
    }
}

public class SystemUserCommand : ISystemUserCommand
{
    private readonly ISystemUserBusiness _systemUserBusiness;

    public SystemUserCommand(ISystemUserBusiness systemUserBusiness)
    {
        _systemUserBusiness = systemUserBusiness;
    }

    ...
}

All of your issues stem from the fact that you are using a base controller for the singular purpose of sharing code. While there may be reasons for making a base controller in MVC, sharing code is certainly not one of them because it tightly couples the rest of your application to that base class. As was pointed out by Ilya, filters can be used as a loosely-coupled alternative. See [this answer](http://stackoverflow.com/a/6119341/181087) for a more complete explanation. — NightOwl888, Oct 30 '16 at 13:13

Sean Sobey · Answer 1 · 2016-10-28T18:27:44.450

2

You could use property injection instead of constructor injection, via the base class, eg using unity:

public abstract class BaseController : Controller
{
    [Dependency]
    public ISystemUserCommand SystemUserCommand { get; set; }
}

This would mean the interface reference is only on the base class.

See here for the full examples.

EDIT, Autofac example:

You don't need property attributes on the dependency,

public abstract class BaseController : Controller
{
    public ISystemUserCommand SystemUserCommand { get; set; }
}

Just to register the properites to auto resolve on the autofac builder:

builder.RegisterControllers(typeof(MvcApplication).Assembly).Where(t => t.IsAssignableFrom(typeof(BaseController))).PropertiesAutowired();

See autofac property injection here.

edited Oct 28 '16 at 18:27

answered Oct 28 '16 at 17:35

Sean Sobey

942
7
13

Do you have an example with AutoFac? – Andrew Oct 28 '16 at 17:44
I added an autofac example :) – Sean Sobey Oct 28 '16 at 17:50
Does this still allow constructor injection or does everything default to property injection? – Andrew Oct 28 '16 at 18:02
I added a way to scope the auto property injection and a link to the autofac dos as well. – Sean Sobey Oct 28 '16 at 18:28

Ilya Chernomordik · Accepted Answer · 2016-10-28T17:53:23.643

1

First of all, it does not seem a good idea to have OnActionExecuting override in the controller. You can use filters, that are specially designed for this purpose. And it seems that is the main reason you created the BaseController at all.

Regarding the problem with injecting the system command in all the required service, I would do so, but without inheriting from a base class, since I generally prefer aggregation to inheritance. That would mean that each controller that needs to work with the service will get it.

Another option that I have used few times to abstract some operations is to create a UserSerivce that will provide the required operations to the controllers. It will have ISystemUserCommand and HttpContext injected inside so that all of your controllers won't have to do the job. You can either use HttpContext.Current as static or abstract it away if you need testability.

Moreover I would not recommend property injection since it is more obscure than constructor injection that should be preferred if possible.

You can read more about filters here. Unfortunately if you use filters it's not that easy to inject in filters themselves and mostly done with property injection or ServiceLocator pattern (which is not good usually). It's possible to do better with some amount of voodoo though. I think that SimpleInjector has a lot of examples and tutorials on how to apply DI to filters in MVC, maybe they even have a nuget package now to ahieve that.

edited Oct 28 '16 at 17:53

answered Oct 28 '16 at 17:44

Ilya Chernomordik

27,817
27
121
207

My base controller does other stuff too, I just didn't include it. I put `OnActionExecuting` here because I want to populate viewbags for the layout page with user information. You think I should use a filter instead for that? – Andrew Oct 28 '16 at 17:49
If you show some common data on your page (like user info) I would rather create a special partial view that would be reused in MVC pages. This is very non-obvious operation and others might think it appears "magically" until they understand how it works. – Ilya Chernomordik Oct 28 '16 at 17:52
Hmm okay. So if I put someone's name and company in the navbar of the page I would move that to a partial view and then I'd create a filter to populate the partial? – Andrew Oct 28 '16 at 18:00
No, not quite. You would put that navbar to a partial view and you will inject your command/service in the controller that is responsible for that view. Then you would put your partial view in the required place in your pages. Filter you need to use for authrorization (I see you do RedirectToAction now). And you can inject the same UserService in the filter since you would need to do similar operation there (get the user). – Ilya Chernomordik Oct 28 '16 at 18:02
Okay, so as an example in my `HomeController` I would have `HomeController(ISystemUserCommand command)` and inside the constructor would I would call `GetUser(int id)` and populate the viewbags that way? – Andrew Oct 28 '16 at 18:04
I have mixed few things in here, sorry. Partial views don't have their own controllers. But you can use Authorize filter to make sure that you have a user in HttpContext.Current.User.Identity, then I think you can use it directly in the Page if you need. – Ilya Chernomordik Oct 28 '16 at 18:16
I don't think that'll work though. `HttpContext.Current.User.Identity` won't return information from my user table. What do you think about scoping the property injector how R2dical did in his answer? – Andrew Oct 28 '16 at 19:18
Actually, I could probably create a custom `Authorize` attribute and use property injection like this: http://docs.autofac.org/en/latest/integration/mvc.html. I could populate the user information in that attribute and remove the `OnActionExecuting` from the base controller – Andrew Oct 28 '16 at 19:21
Yes, that was part of what I was proposing :) – Ilya Chernomordik Oct 28 '16 at 19:31
Yeah sorry lol, I'm moving slow today. Do you think populating the user info in a viewbag would be bad? I can look to see if I can return an object from the `Authorize` attribute and maybe have a BaseViewModel that could be used in the Layout that would just have a single property for the current user – Andrew Oct 28 '16 at 19:45
When you override Authorize properly after the authentication you should be able to get the HttpContext.Current.Identity directly in your views and get that info in there. – Ilya Chernomordik Oct 28 '16 at 19:57
Yeah, but that won't have first/last name and company name. That'll come from the database. – Andrew Oct 28 '16 at 20:01
You can add whatever information you need if you inherit the corresponding classes, but I am afraid I don't remember exactly how to do that. It's some type of custom identity. – Ilya Chernomordik Oct 28 '16 at 20:12
Another simple option [ that is a bit beyond nice DI rules, but works for MVC case] that I have used before: create a tag helper that will render this view and use ServiceLocator to get user service (no way to inject in helpers). This is probably easiest solution. – Ilya Chernomordik Oct 28 '16 at 20:17
I created an extension method for `IIdentity` but can you access autofac in a static class? Static classes can't have constructors so I can't inject `IComponentContext` – Andrew Oct 31 '16 at 13:45
That's why I mentioned SeviceLocator anti-pattern. It should not be abused, but some cases involving framework derivatives like you have here we can do that in my opinion. You will have to have a public static reference to your container that extension can access. – Ilya Chernomordik Oct 31 '16 at 14:12

Create instance of interface with dependency injection

2 Answers2