Repository pattern - make it testable, DI and IoC friendly and IDisposable

Question

What I have:

public interface IRepository
{
   IDisposable CreateConnection();
   User GetUser();
   //other methods, doesnt matter
}

public class Repository
{
   private SqlConnection _connection;

   IDisposable CreateConnection()
   {
      _connection = new SqlConnection();
      _connection.Open();
      return _connection;
   }

   User GetUser()
   {
      //using _connection gets User from Database
      //assumes _connection is not null and open
   }
   //other methods, doesnt matter 
}

This enables classes that are using IRepository to be easily testable and IoC containers friendly. However someone using this class has to call CreateConnection before calling any methods that are getting something from database, otherwise exception will be thrown. This itself is kind of good - we dont want to have long lasting connections in application. So using this class I do it like this.

using(_repository.CreateConnection())
{
    var user = _repository.GetUser();
    //do something with user
}

Unfortunetelly this is not very good solution because people using this class (including even me!) often forget to call _repository.CreateConnection() before calling methods to get something from database.

To resolve this I was looking at Mark Seemann blog post SUT Double where he implements Repository pattern in correct way. Unfortunately he makes Repository implement IDisposable, which means I cannot simply inject it by IoC and DI to classes and use it after, because after just one usage it will be disposed. He uses it once per request and he uses ASP.NET WebApi capabilities to dispose it after request processing is done. This is something I cannot do because I have my classes instances which use Repository working all the time.

What is the best possible solution here? Should I use some kind of factory that will give me IDisposable IRepository ? Will it be easily testable then?

Answer 1

There are a few problematic spots in your design. First of all, your IRepository interface implements multiple levels of abstractions. Creating a user is a much higher level concept than connection management. By placing these behaviours together you are breaking the Single Responsibility Principle which dictates that a class should only have one responsibility, one reason to change. You are also violating the Interface Segregation Principle that pushes us toward narrow role interfaces.

On top of that, the CreateConnection() and GetUser method are temporal coupled. Temporal Coupling is a code smell and you are already witnessing this to be a problem, because you are able to forget the call to CreateConnection.

Besides this, the creation of the connection is something you will start to see on every repository in the system and every piece of business logic will need to either create a connection, or get an existing connection from the outside. This becomes unmaintainable in the long run. Connection management however is a cross-cutting concern; you don't want the business logic to be concerned in such low level concern.

You should start by splitting the IRepository into two different interfaces:

public interface IRepository
{
    User GetUser();
}

public interface IConnectionFactory
{
    IDisposable CreateConnection();
}

Instead of letting business logic manage the connection itself, you can manage the transaction at a higher level. This could be the request, but this might be too course grained. What you need is to start the transaction somewhere in between the presentation layer code and the business layer code, but without having to having to duplicate yourself. In other words, you want to be able to transparently apply this cross-cutting concern, without having to write it over and over again.

This is one of the many reasons that I started to use application designs as described here a few years ago, where business operations are defined using message objects and their corresponding business logic is hidden behind a generic interface. After applying these patterns, you will have a very clear interception point where you can start transactions with their corresponding connections and let the whole business operation run within that same transaction. For instance, you can use the following generic code that can be applied around every piece of business logic in your application:

public class TransactionCommandHandlerDecorator<TCommand> : ICommandHandler<TCommand>
{
    private readonly ICommandHandler<TCommand> decorated;    
    public TransactionCommandHandlerDecorator(ICommandHandler<TCommand> decorated) {
        this.decorated = decorated;
    }

    public void Handle(TCommand command) {
        using (var scope = new TransactionScope()) {
            this.decorated.Handle(command);
            scope.Complete();
        }
    }   
}

This code wraps everything around a TransactionScope. This allows your repository to simply open and close a connection; this wrapper will ensure that the same connection is used nonetheless. This way you can inject an IConnectionFactory abstraction into your repository and let the repository directly close the connection at the end of its method call, while under the covers .NET will keep the real connection opened.