Property Injection in ASP.NET MVC with Ninject

I got a design challenge with asp.net mvc.  I want to keep track of the currently logged in user in the session because I don’t want to hit the database every time I need to get the username or id for a query.  And I have all my controllers inherit from a base controller named BaseController.  So, I added a CurrentUser property to the BaseController and I want it to automagically work without the derived controllers having to do anything.  Here is a class diagram to help clarify:

image

The CurrentUser Property needs to look something like this:

public User CurrentUser
{
    get
    {
        var key = "currentuser";
        if (Session[key] == null)
        {
            Session[key] = /*get user from database some how*/;
        }
        return (User) Session[key] ;
    }
}

This looks simple enough but it is not.  The reason it is not simple is because to get the current user I have to call Membership.Provider.GetUser from the BaseController.  The problem with that is that it creates a dependency on the MembershipProvider class which I don’t want to have, because it will make testing very hard.

One obvious solution was to add the MembershipProvider (which is an abstract class) to the BaseController’s construct and then pass a mocked instance during testing…  The problem with this design is that now my BaseController will be forced to have a parameterized constructor which means that I have to change the code in all the derived controllers to handle that and pass the appropriate instance of MembershipProvider.  Sounds like a code smell.

My solution was to create the MembershipProvider class using my IoC container – in this case, my Ninject Kernel.  This allows me to inject a SqlMembershipProvider in development and runtime and inject a mocked provider in testing.  So the final CurrentUser property looks like this:

public User CurrentUser
{
    get
    {
        var key = "currentuser";
        if (Session[key] == null)
        {
            var Provider
                = (MembershipProvider)
                    Kernel.Get(typeof(MembershipProvider));

            Session[key] = AppHelper.CreateUserFromMembershipUser
                            (Provider.GetUser(User.Identity.Name, true));
        }
        return (User) Session[key] ;
    }
}

If you have been paying attention, you are probably wondering  what is this “Kernel” thing.  Well Kernel is an instance of the Ninject Kernel which itself was injected into the BaseController class like this:

[Inject]
public IKernel Kernel { get; set; }

I could have done this differently.  For example, I could have injected the provider itself using property injection like this:

[Inject]
public MembershipProvider Provider { get; set; }

The only problem is that the provider isn’t needed by all the derived classed and I didn’t want to have a public property in the base class that would hardly be used anywhere else.   On the other hand Kernel could be globally used to instantiate an object.

What do you think?  Is this the way to do it?  Is there a better way?

Advertisements

0 thoughts on “Property Injection in ASP.NET MVC with Ninject

  1. For me the Kernel.Get's should be used as infrequently as possible. You say that you have a BaseController class, but only certain controllers will make use of the MembershipProvider property. Why not create a “SecuredController” that inherits from BaseController and adds the property? This way you're back to doing the injection at the IoC level and only doing it for the controllers that need it.

    Like

    • The SecuredController solution wouldn't work for me. Almost every controller needs to call the property CurrentUser which means every controller will inherit from SecuredController which defeats the point.Why are you opposed to Kernel.Get? It seems like I can easily achieve the goal I want without having to force every derived controller from calling the base controller consturctor with interfaces…I am trying to get away from doing thisclass DerviedClass : BaseClass public DerivedClass(IService1, IService2) : base(IBaseService1, IBaseService2)By using Kernel.Get in the base class then I eliminate the use of base(IBaseService1, IBaseService2) in the derived classes constructor… I hope that makes sense.

      Like

      • Regarding my opposition to Kernel.Get:From the testing angle I'd rather be able to pass my dependencies in via the constructor (or set via a property) than have to mock an instance of IKernel up.Think about how you're going to write tests for the controllers that use that property. With the code listed above you would have to either (a) mock the Kernel or (b) actually wire up DI for your tests.

        Like

      • I see your point… I am actually using DI in my tests, which I would haveto do anyway to inject constructor arguments. Actually my DI code in mytest project didn't change when I moved from constructor to property tokernel.get…At points, I feel like kernel.get is a good solution and at other points Ifeel it's not. I can't explain it :)…

        Like

      • Hmmm, I'm not sure why you would _have_ to use DI in your tests rather than creating your controllers and manually injecting your dependencies (which will likely be mocks or stubs).Btw, how deep can these threads go? =)

        Like

      • I don't have to use DI in my tests but I do because it is easier… I haveone method that just sets up my DI and all it is really doing is mocking allthe interfaces…I don't know how deep the threads go… But we can find out :)… Are youposting your comments on the site or are you just replying to the emails?

        Like

      • I was doing it via the website. But this time I'm doing it via email.My only concern about doing DI on your tests is that now your tests havedependencies that you can't see while reading the test.

        Like

      • Another valid point, but if I don't refactor the DI stuff then I will end upwith a lot of repetition in my unit tests and test fixtures. I might haveto post an example of how I am doing this, to get feedback from thecommunity. I might be experiencing tunnel vision, since I am the only onewriting, reviewing and refactoring the code…

        Like

  2. Regarding my opposition to Kernel.Get:From the testing angle I'd rather be able to pass my dependencies in via the constructor (or set via a property) than have to mock an instance of IKernel up.Think about how you're going to write tests for the controllers that use that property. With the code listed above you would have to either (a) mock the Kernel or (b) actually wire up DI for your tests.

    Like

  3. I see your point… I am actually using DI in my tests, which I would haveto do anyway to inject constructor arguments. Actually my DI code in mytest project didn't change when I moved from constructor to property tokernel.get…At points, I feel like kernel.get is a good solution and at other points Ifeel it's not. I can't explain it :)…

    Like

  4. Hmmm, I'm not sure why you would _have_ to use DI in your tests rather than creating your controllers and manually injecting your dependencies (which will likely be mocks or stubs).Btw, how deep can these threads go? =)

    Like

  5. I don't have to use DI in my tests but I do because it is easier… I haveone method that just sets up my DI and all it is really doing is mocking allthe interfaces…I don't know how deep the threads go… But we can find out :)… Are youposting your comments on the site or are you just replying to the emails?

    Like

  6. I was doing it via the website. But this time I'm doing it via email.My only concern about doing DI on your tests is that now your tests havedependencies that you can't see while reading the test.

    Like

  7. Another valid point, but if I don't refactor the DI stuff then I will end upwith a lot of repetition in my unit tests and test fixtures. I might haveto post an example of how I am doing this, to get feedback from thecommunity. I might be experiencing tunnel vision, since I am the only onewriting, reviewing and refactoring the code…

    Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s