Asp.Net Membership Provider’s Lifetime Considerations- Part 2

Previously I made a post about issues I encountered with the Asp.Net Membership Provider.

My fix was a bit short-sighted as it only fixed the issue in one location, when the account controller tries to perform operations. The problem with my solution is that it does not deal with all the areas where Asp.Net creates the membership provider itself, and keeps that instance running through the application lifetime.

This meant that all calls to the membership provider (e.g. calling Membership.GetUser()) were all using the same database session, even across multiple web requests. Not only did this cause hidden caching issues, this also mean that if the database connection was broken all calls to the membership provider would now exception out with the only way to fix it is to restart the Asp.Net instance.

In order to fix this I had to remove my database session creation out of my custom membership provider’s constructor, and instead retrieve a new database session from my IoC system inside each method.

This seems to not only fix my original issue, but several other “random” exceptions that I could not reproduce on a regular basis.

Beware of Asp.Net’s Membership Provider Lifetime, Entity Framework Caching, and Dependency Injection

I recently struggled while dealing with a bug I encountered in my Asp.Net MVC application, and I thought I would write about it to hopefully help someone else.

I have a method in my business layer that users use to change their password. After implementing unit tests to verify the method worked properly I implemented a call to it in my account controller. Everything seemed perfect at first, as I was able to change my password successfully and even validate that the user entered their correct current password, which is required to perform a password change.

However, when I logged out and tried to log back in using the new password the login attempt failed. Yet when I used my previous password I was able to log in! To make things even more confusing, since I require users to enter their current password to change their password I was able to confirm that the password change did actually take effect. Finally, when I made a debugging change and re-ran the app, the new password worked when logging in!

A few irritating hours later, I finally figured out what was wrong. It came down to the difference of lifetimes in my MVC application between different classes. My AccountMembershipService class, which was mostly based on the default code that came with MVC, had the following constructor:

        public AccountMembershipService()
            : this(null)
        {
        }

        public AccountMembershipService(MembershipProvider provider)
        {
            _provider = provider ?? Membership.Provider;
        }

The problem is that my application loads its service classes using Castle Windsor, and entity framework is loaded from Windsor with a per-web request lifetime. Even though my custom membership provider was retrieving the database context from Windsor, Asp.Net creates the membership provider for the lifetime of the whole application.

So what happened was that I would log in using the database context for the Membership Provider, but when I changed my password the service class would use a different database context. When I changed my password, the password is correctly changed in the database (and the request’s specific database context), but the Membership Provider is still using the original database context. Since Entity Framework caches previously received entities, when I go back to log in entity framework receives the user entity out of the cache, not the database, and thus the old password is what will pass the Membership Provider’s validation check.

The fix was to replace the previous code with:

        public AccountMembershipService(MembershipProvider provider)
        {
            _provider = provider ?? new MyMembershipProvider();
        }

This forces a new membership provider instance to be created for that web request, and thus guarantees that the database context will not be holding a stale user entity.

Hopefully this saves someone from the irritation and wasted time that I went through.


Update: I have written another post adding on to the issues I wrote about here.

Keeping Asp.NET MVC Controller Constructors Clean In a Dependency Injection World – Part 2

Previously I wrote a post an article about a way to keep Asp.net MVC constructors clean with heavy dependency injection. In that post, which can be found here, proposed creating the following class to resolve dependencies on demand:

public class WindsorServiceFactory : IServiceFactory
{
    protected IWindsorContainer _container;

    public WindsorServiceFactory(IWindsorContainer windsorContainer)
    {
        _container = windsorContainer;
    }

    public ServiceType GetService<ServiceType>() where ServiceType : class
    {
        // Use windsor to resolve the service class.  If the dependency can't be resolved throw an exception
        try { return _container.Resolve<ServiceType>(); }
        catch (ComponentNotFoundException) { throw new ServiceNotFoundException(typeof(ServiceType)); }
    }
}

This seemed like a good idea in theory, even though I got told by people on here and on Ayende’s blog that it was a bad idea. After using this in a project, I can now agree that this is a bad idea.

Dependency Ambiguity

One problem is that it doesn’t make it clear what dependencies a class has, and those ambiguities are hidden behind the implementation details. At first this doesn’t seem like a big deal, but it can cause confusion down the road. An example of this is when you have a method that retrieves data about a post in your blog system. When you write a unit test this might fail on a private blog because it makes a call to another class to do a security check (make sure the user has access to the blog or post). However, there is no way to know what service class is going to be used for this call without knowing the implementation details of how the IServiceFactory.GetService() call is made. Instead, when this service class interface is passed in via IoC into the constructor, it is clear to any outside users what service interface is required to use.

IoC Testing

Another (major in my opinion) issue is you cannot easily test that ALL dependencies can be resolved. In order to unit test that all of my dependencies can be resolved by IoC, I have to explicitly create a unit test for each type of class I *could* possibly use in my business layer or presentation layer. I say could because since these dependencies are not being passed in through constructors I don’t know exactly what IServiceFactory.GetService calls will be made. The only way to be 100% sure I added everything to my IoC is by manually running through every path of my application. This is error pron and just bad.

Instead, when you pass dependencies into your constructors, testing that ALL dependencies can be resolved is simple. All you have to do is create one unit test per MVC constructor that looks like:


[TestMethod]
public void Windsor_Can_Resolve_HomeController_Dependencies()
{
	// Setup
	WindsorContainer container = new WindsorContainer();
	container.Kernel.ComponentModelBuilder.AddContributor(new SingletonLifestyleEqualizer());
	container.Install(FromAssembly.Containing<HomeController>());

	// Act
	container.Kernel.Resolve(typeof(HomeController));
}

This will not only verify that all my HomeController dependencies can be resolved, but that all child and grandchild dependencies can be resolved as well. This is MUCH less error prone.

So What Now

So now we come to the question that brought me here in the first place, how do I control constructor bloat when using dependency injection? After thinking about it, it really feels like I have been trying to squash an ant by using a bulldozer. The REAL problem with constructor bloat is that your MVC controllers are doing too much. Once you begin to realize this, the stupidity of my ServiceFactory stuff comes clear. Constructors doing too much have more issues than just constructor bloat, it also makes it hard to navigate through controller actions.

So the real solution to the problem? Utilize areas and spread out your constructor responsibilities among multiple MVC controllers instead of trying to fit all the actions into one controller, even if they seem vaugely related. This will keep your MVC application much more maintainable in the long run, and easier to debug.