Making legacy Unity code a bit more testable

A current project I'm working on uses Unity to do dependency injection which has created a little bit of a challenge. Much of this code was architected and built several years ago, and generally follows a repository/DAO pattern where:

A static InjectionContainer class holds a reference to our UnityContainer. This container maps all the interfaces to the implementation instances, and is configured at Application_Start as defined in Global.asax.cs (as this is a .NET web application).

void Application_Start(object sender, EventArgs e)
{
    // Code that runs on application startup
    var container = new UnityContainer();
    container.RegisterInstance<IMessagingDao>(new MessagingDao());
    InjectionContainer.Container = container;
}

To actually "do" something, a page/control will create an instance of a repository class. The repository can then resolve and invoke methods on any relevant DAO object(s) as required, as well as internally encapsulating all business logic. Each instance of the repository lives only as long as the calling method requires, and it is disposed of when it falls out of scope. The intent here is to keep all the business logic at the repository layer, separate from the data access layer. A repository class might look similar to:

public class MessagingRepository
{
    // Many methods, similar to:
    public void SendMessage(Message message)
    {
        // (Additional repo-level logic omitted from here)
        var dao = InjectionContainer.Container.Resolve<IMessagingDao>();
        dao.SendMessage(message);
    }
}

Yes, there are lots of newer/better patterns, but this is what we've got and it works. Without a good business case for why we should rip it out, refactor/rewrite and regression test everything - it's going to stick around for a little while.

The challenge here was to make this more testable. The complications were:

1. To minimise changes to the existing code. The purpose is to make the code testable, and possibly a little bit cleaner along the way.
2. To work around the anti-pattern of having a static IoC class. The static InjectionContainer class also causes complications running tests concurrently in the same app domain when you want to have different mappings configured. I'm a big fan of NCrunch so I wanted whatever solution I used to play nicely with it's test runner.

Unfortunately we can't just jump to constructor injection as the change was too big and would necessitate us re-testing everything at a point in time where we weren't able to do this. But we can take a few steps in that direction...

Each repository contains several methods, many of which will ultimately be resolving the DAO. So why not replace all the calls to resolve the interface with a method to give us a single interception point?

public class MessagingRepository
{
    private IMessagingDao _messagingDao;
    private IMessagingDao ResolveMessagingDao()
    {
        if (_messagingDao == null)
            _messagingDao = InjectionContainer.Container.Resolve<IMessagingDao>();
        return _messagingDao;
    }

    public void SendMessage(Message message)
    {
        var dao = ResolveMessagingDao();
        dao.SendMessage(message);
    }
}

I used the more verbose name "ResolveMessagingDao" as some of the repositories rely upon multiple DAOs, so it was easier to be clearly consistent everywhere rather than having some methods named "ResolveDao" if only a single DAO was used in that repository.

This new ResolveMessagingDao() method will now resolve and cache the reference to the DAO object for the lifetime of the repository, which is fine given that our usage pattern is to create an instance of the repository when required, use the repository, then discard it when it falls out of scope. This gives us the point to add constructor injection, by replacing the implicit default constructor with two explicit constructors:

public MessagingRepository()
{
    // Explicit parameterless constructor to replace the implicit
    // default constructor which we used to rely on.
}

public MessagingRepository(IMessagingDao messagingDao)
{
    if (messagingDao == null)
        throw new ArgumentNullException("messagingDao");

    _messagingDao = messagingDao;
}

These constructors give us two options:

1. Implicit support for the old behaviour, where creating an instance of the repository using the parameterless constructor will cause the DAOs to be resolved via Unity. Additionally, the reference to this DAO instance is then cached within the repository for each subsequent use (although admittedly the "cost" of resolving it is insignificant).
2. Explicit support via the new constructor, which allows us to supply the instances of the DAO we want the repository to use. Additionally by preventing null values from being provided to that constructor, I'm making sure that I can't pass in null unexpectedly and then have it (silently) fall back to using Unity for resolution.

Another option would have been to resolve the DAOs in the parameterless constructor, but this didn't feel like the correct solution as there are often repository methods that don't even need to access anything at the DAO layer at all.

Ultimately this solution worked and made all the repository-layer code testable by allowing it to be run against explicitly-created mock DAO objects with minimal side effects on the main project itself.