Should repositories be properties on the unit of work when using Entity Framework?

Question

I have scoured the web looking for a good implementation of the Repository/Unit of Work pattern using Entity Framework. Everything I've come across is either tightly coupled at some point in the abstraction or assumes that the DbContext used by the Unit of Work and Repositories is shared and should live for the entire HTTP request (instance per request via dependency injection).

For example, assuming you are consuming repositories from the service layer, a service constructor might look like this:

public DirectoryService(IUnitOfWork unitOfWork, ICountryRepository countryRepo, IProvinceRepository provinceRepo)
{
    /* set member variables... */
}

The unit of work constructor might look like:

public UnitOfWork(IDbContext context)
{
    _context = context;
}

And a repository constructor might look like:

CountryRepository(IDbContext context)
{
    _context = context;
}

This solution makes the blind assumption that the Dependency injection is setting up the Unit of Work and Repositories to share the same IDbContext using instance per request. Is that really a safe assumption to make?

If you are using dependency injection with instance per request, the same IDbContext will be injected into multiple units of work. The unit of work is no longer atomic, is it? I might have pending changes in one service that are then committed in another service because the context is shared across multiple units of work.

To me it seems to make more sense to set up a IDbContextFactory and get a fresh database context with each unit of work.

public interface IDbContextFactory
{
    IDbContext OpenContext();
}

public class UnitOfWork 
{
    private IDbContextFactory _factory;
    private IDbContext _context;

    UnitOfWork(IDbContextFactory factory)
    {
        _factory = factory;
    }

    internal IDbContext Context
    {
        get { return _context ?? (_context = _factory.OpenContext()); }
    }
}

The problem then becomes, how do I make my Unit of Work implementation available to the injected repositories? I don't want to assume instance per request because then I'm right back in the same boat I started in.

The only thing I can think of is to follow Entity Framework's lead and make the repositories (IDbSet<T>) part of the unit of work (DbContext).

So then I might have units of work that look like this:

public class DirectoryUnitOfWork : IDirectoryUnitOfWork
{
    private IDbContextFactory _factory;
    private IDbContext _context;

    public DirectoryUnitOfWork(IDbContextFactory factory)
    {
        _factory = factory;
    }

    protected IDbContext Context 
    {
        get { return _context ?? (_context = _factory.OpenContext()); }
    }

    public ICountryRepository CountryRepository
    {
        get { return _countryRepo ?? (_countryRepo = new CountryRepository(Context)); }
    }

    public IProvinceRepository ProvinceRepository
    {
        get { return _provinceRepo ?? (_provinceRepo = new ProvinceRepository(Context)); }
    }

    void Commit()
    {
        Context.SaveChanges();
    }
}

Then my Directory service starts to look like this

public class DirectoryService : IDirectoryService
{
    private IDirectoryUnitOfWork _unitOfWork;

    public DirectoryService(IDirectoryUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    public GetCountry(int id)
    {
        return _unitOfWork.CountryRepository.GetById(id);
    }

    public GetProvince(int id)
    {
        return _unitOfWork.ProvinceRepository.GetById(id);
    }

    public void AddProvince(Province province)
    {
        _unitOfWork.ProvinceRepository.Create(province);
        Country country = GetCountry(province.CountryId);
        country.NumberOfProvinces++; // Update aggregate count
        _unitOfWork.Commit();
    }

    /* ... and so on ... */
}

It seems like a lot of work, but using this method leaves everything loosely coupled and unit testable. Am I missing an easier way, or is this a good way to do it if I am going to abstract away Entity Framework?

Answer 1

You should never abstract an ORM (itself an abstraction), but you should abstract the Persistence. The only UoW I'm using is a db transaction and that's a persistence detail. You don't need to use UoW and Repository together. You should think if you really need all this stuff.

Personally, I'm using the repository by default because Persistence is the last thing I do in an app. I don't care about patterns per se , I do care about decoupling my BL or the UI from DAL. Your upper layers (everything except DAL, which is the lowest layer from a dependency point of view) should always know about an abstraction, so that you can go as wild as you want in the DAL implementation.

One trick a lot of devs don't know is that design patterns (especially architectural ones) should be treated as a high level principle first and as a technical know-how second. Simply put, the thing that matters the most is the benefit a pattern is trying to achieve (the principle, the bigger picture), not its actual implementation (the "low level" details).

Ask yourself why should the BL know about a UoW in the first place. The BL only knows about an abstraction dealing with business objects. And you never work with the whole BL as once, you're always in a specific BL context. Your DirectoryService seems to be doing way to much for its own good. Updating stats doesn't look like it belongs to the same context as adding a new Province. Also why do you need UoW for queries?

One mistake I see a lot is devs rushing to write whatever code (with a design pattern attached) when they aren't doing the most important part: the design itself. When you have the improper design, problems appear and you start looking for workarounds. One of them is this UoW with Repository properties, which require a high layer like BL to know more than business concerns. Now the BL needs to know that you're using a UoW, a lower level pattern which is great for the DAL, not so great for the BL.

Btw, UoW never makes sense for quering as you're dealing with a 'read' situation, UoW is only for 'writes'. I don't mention EF or any ORM because they don't really matter, your BL service (Directory Service) is already corrupted by an infrastructural detail required by improper design. Note that sometimes you do need to compromise in order to implement a solution, but this is not the case.

Proper design means you know about your bounded context (yeah, DDD concept you can apply it regardless how much DDD you want to do) and don't put everything that might use the same data in one place. You have specific contexts for use cases and counting provinces (a presentation/reporting detail) is really a different use case than adding Province.

Adding the province and then publishing an event which signals to a handler to update stats is a more elegant, maintainable solution. No UoW required either.

Your code would look like this

public class DirectoryService : IDirectoryService
{
     public DirectoryService(IProvinceRepository repo, IDispatchMessage bus) 
     { 
       //assign fields
     }

      /* other stuff */

      //maybe province is an input model which is used by the service to create a business Province?
      public void AddProvince(Province province)
     {

        _repo.Save(province);
        _bus.Publish( new ProvinceCreated(province));
     }
}

  public class StatsUpdater:ISubscribeTo<ProvinceCreated> /* and other stat trigger events */
   {
       public void Handle(ProvinceCreated evnt)
       {
              //update stats here
       }  
   }

In a way it simplifies things, in other way you might thing it complicates stuff. Actually, this is a maintainable approach, because the stats updater can subscribe to many events but the logic stays in one class only. And the DirectoryService does only the things that are assumed it does (what part of the name AddProvince hints you that the method also updates stats?).

In conclusion, you need to design the BL better before rushing to complicate your life with UoW, DbContext, Repositories as properties etc

Answer 2

or assumes that the DbContext used by the Unit of Work and Repositories is shared and should live for the entire HTTP request

This is clearly wrong. Assuming that the context is shared between the UoW and Repositories doesn't mean that the context lifetime should depend on the HTTP request. Rather - you can create new instances of the context and the UoW that use it whenever you want. This is only a convenience to have a default UoW that lives for the HTTP request but creating new, local units of work could be handy.

On the other hand, if repositories are exposed from the UoW:

public class UnitOfWork
{
    ...

    public IUserRepository UserRepo
    {
        get { ... } 
    }

    public IAccountRepository AccountRepo
    {
        get { ... } 
    }

then not sharing the same context between repos could have unexpected results:

 UoW uow = ....

 var u1 = uow.User.FirstOrDefault( u => u.ID == 5 );
 var u2 = uow.Account.FirstOrDefault( a => a.ID_USER == 5 ).User;

You would definitely expect these two to return the same instance of the user of the id 5 and what's more, sharing the same context would mean that the second query could retrieve the user from the 1st level cache. On the other hand, two different contexts for two repos means that you get two different instances.

This also means that this would not be possible

 var u1 = uow.User.FirstOrDefault( u => u.ID == 5 );
 var a1 = uow.Account.FirstOrDefault( a => a.ID == 177 );

 a1.User = u1; // oops!

as mixing entites from different contexts would just raise an exception. But the above scenario is a common one!

The conclusion from these observations is that you should share the context between repos. But, if you need a fresh instance, you just create a local, fresh instance of the context, inject it into the UoW, from where it gets injected into repos, and dispose it at will.