Expose an IDbSet<> object to overcome a limitation for LINQ's Select()

Question

Before posting this, I have spent the past 4 days tirelessly investigating the Internet and the different Stack Exchange websites on how to correctly implement the Unit of Work pattern and Unit Test it using MVC 5 + EF 6.1. So far, the implementation I manage to make it work was this one.

Here's the actual problem:

I am lifting all the lookups of the Database to the Unit of Work (as I've understood). I have a repository called "TournamentRepository" which derives from a GenericRepository with the generic methods. In the example above, it uses a neat trick on implementing statics as extensions so we don't need to create new Interfaces for each new Repository.

  public static async Task<bool> IsUserRegisteredInTournament(this IGenericRepository<PGTournament> tournamentRepository, string tournamentUrl, string userId)
        {
            return await tournamentRepository.Get().Select(x => x.Participants.Where(y => y.UserId == userId))
                .AnyAsync();
        }

What I want to emphasize is the Get() method. I want to know if it is a leaky abstraction.

It is actually from the following:

public class GenericRepository<TEntity>: IGenericRepository<TEntity> where TEntity:class
    {
        private readonly IDbSet<TEntity> _iDbSet;
        private readonly DbSet<TEntity> _dbSet;
        //protected IDbSet<TEntity> Entities { get; set; }


        public GenericRepository(IDbRepositories dbRepositories)
        {
            _iDbSet = dbRepositories.Set<TEntity>();
            _dbSet = dbRepositories.SetDb<TEntity>();
        }
                public IDbSet<TEntity> Get()
    {
        return _iDbSet;
    }

    public async Task<TEntity> FindAsync(int id)
    {
        return await _iDbSet.FindAsync(id);
    }

    public async Task<IEnumerable<TEntity>> GetAllAsync()
    {
        return await _iDbSet.ToListAsync();
    }

    public TEntity SingleOrDefault(Expression<Func<TEntity, bool>> predicate)
    {
        return _iDbSet.SingleOrDefault(predicate);
    }

    public void Add(TEntity entity)
    {
        _iDbSet.Add(entity);
    }

    public void AddRange(IEnumerable<TEntity> entities)
    {
        _dbSet.AddRange(entities);
    }

    public void Remove(TEntity entity)
    {
        _iDbSet.Remove(entity);
    }

    public void RemoveRange(IEnumerable<TEntity> entities)
    {
        _dbSet.RemoveRange(entities);
    }

    /*
     * 
     * Creates a wrapper for Entity Framework's Select Method
     */
    public IQueryable<T> Select<T>(Func<TEntity, T> func ) where T: class 
    {
       return _iDbSet.Select(func).AsQueryable();
    }


    }

Whenever I've tried to do a Select at the repository, I would get no IntelliSense because the object is layered through the interface.

public interface IGenericRepository<TEntity> where TEntity : class
    {
        IDbSet<TEntity> Get();
        Task<IEnumerable<TEntity>> GetAllAsync();
        Task<TEntity> FindAsync(int id);

        // This method was not in the videos, but I thought it would be useful to add.
        TEntity SingleOrDefault(Expression<Func<TEntity, bool>> predicate);

        void Add(TEntity entity);
        void AddRange(IEnumerable<TEntity> entities);

        void Remove(TEntity entity);
        void RemoveRange(IEnumerable<TEntity> entities);

    }

I tried implementing my own Select. The only problem was when I tried to unit test and I ran an Async() method. For unit testing an Async() method, we should follow this.

Unfortunately, I was returning an IQueryable<T> and it did not go through all the conversions that MS does in their TestDbAsyncQueryProvider.

After hours of banging my head, I figured out that I could return the IDbSet<> and do the Select manually.

I rebranded the Get() method for it to return the current IDbSet<>.

Another note

You may have seen that I have a DbSet<> and an IDbSet<>. The reason is (hope you can correct me if I'm wrong), is that RemoveRange and AddRange lack this implementation in IDbSet<> (which I'm using for unit testing, if not, I would receive the un-overridable error for the property not being virtual). So for me, the only way was to actually get the Entity and pass it as both IDbSet and DbSet.

Answer 1

Leaky as an old roof :)

An abstraction is leaky if you need to know details about the concrete implementation in order to work with it properly. Leakiness of an interface may reveal itself in various ways, for example: -

1. if it's very hard (or impossible) to implement the interface with another technology stack than the one for which it was designed. Or

2. if it perfectly exposes major features of one implementation, but would make powerful features of other implementations inaccessible. Or

3. if it enforces a work flow that perfectly fits one implementation but forces other implementations to bend their own preferred work flow. Or

4. If it doesn't tell the whole story, if an implementation has added core features.

IDbSet is so leaky, I wouldn't even call it an abstraction.

Let's look at what you encounter if you want to implement IDbSet for, say, NHibernate.

1. You may succeed in implementing the interface methods in a way that they display the same behavior as DbSet. This is hard enough. Find is far from trivial. Add affects an entire object graph. In fact, all methods affecting an entity's tracking state have nitty-gritty details when it comes to the adhered object graph. If you don't respect these details, saving will behave differently.

2. If forces you to use LINQ-to-NHibernate. This in itself inevitably introduces differences. Both types of LINQ have their own set of supported methods. Both have their own bugs, or run-time issues (like generated queries that perform poorly).
   NHibernate has other powerful query APIs that you can't benefit from, at least not fully.

3. NHibernate's workflow resembles that of EF. But there are important differences. For example NHibernate's auto flush (auto commit) feature.

4. The standard implementation if IDbSet, DbSet, has other important methods that are not part of the interface. For example, AsNoTracking. An NHibernate implementation should also implement these to be even remotely interchangeable (which it never will be).

Extension methods

So far, the challenges only just applied to implementing the interface specification itself. The real bummer are extension methods.

If you've got an IDbSet, it seems reasonable to expect that you can apply extension methods from DbSetMigrationsExtensions or the vast QueryableExtensions class. After all, DbSetMigrationsExtensions extend IDbSet and QueryableExtensions is in the same namespace.

But let's look at the source code of just an arbitrary method, QueryableExtensions.AnyAsync

public static Task<bool> AnyAsync<TSource>(this IQueryable<TSource> source, CancellationToken cancellationToken)
{
    Check.NotNull(source, "source");

    var provider = source.Provider as IDbAsyncQueryProvider;
    if (provider != null)
    {
        return provider.ExecuteAsync<bool>(
            Expression.Call(
                null,
                _any.MakeGenericMethod(typeof(TSource)),
                new[] { source.Expression }
                ),
            cancellationToken);
    }
    else
    {
        throw Error.IQueryable_Provider_Not_Async();
    }
}

As you see, it expects a Provider that implements IDbAsyncQueryProvider;. This is an interface in the System.Data.Entity.Infrastructure namespace. So you run from one implementation challenge into the other. I don't think there will be any provider implementing that interface, other than those created for Entity Framework. And it can be mocked for the purpose of unit tests in an EF environment, as in the link you shared.

Now what?

Does this mean you should revert to the original plan and jump through hoops to return IQueryable correctly? I don't think so. For one, IQueryable is leaky as well. But I may have answered your question in a stricter sense than you expected. Maybe you never intended to craft a completely different implementation. Maybe you only contemplated the implications of exposing IDbSet in a unit test setting. If so, IDbSet, or even DbSet is good enough.

Answer 2

Maybe I misunderstand what you want to achieve, so I put a piece of code here to clarify my position.

I like unit testing. So I like to create an interface for the data layer. Something like this should do;

public interface IMyDbContext
{
    IDbSet<Participant> Participants { get; set; }
    IDbSet<PGTournament> Tournaments { get; set; }
}

I then create a vanilla EF context, like this;

public partial class EFDbContext : System.Data.Entity.DbContext, IMyDbContext
{
    public EFDbContext() : base("name=EFDbContext")
    {
    }

    public virtual IDbSet<Participant> Participants { get; set; }
    public virtual IDbSet<PGTournament> Tournaments { get; set; }
}

Then I create a business logic object (or service object if you prefer);

public class MyBusinessLogic
{
    readonly IMyDbContext _db;

    public MyBusinessLogic(IMyDBContext db)
    {
        _db = db;
    }

    public async Task<bool> IsUserRegisteredInTournament(string tournamentUrl, string userId)
    {
        return await _db.Tournaments
            .Where(t => t.Url == tournamentUrl)
            .SelectMany(t => t.Participants)
            .Where(p => p.UserId == userId)
            .AnyAsync();
    }
}

Calling the business logic from code could look like;

using (var db = new EFDbContext())
using (var bll = new MyBusinessLogic(db))
{
    var isRegistered = await bll.IsUserRegisteredInTournament("http://MyTournament.org", "USER_ID_75");
    Trace.TraceInformation("User is registered: {0}", isRegistered);        
}

There is no leaky abstraction here. The business logic is responsible for business logic. The data layer is responsible for loading and saving data and the UI can request services from the business logic. You can easily test the business logic by injecting a mock IMyDbContext and if you like DI containers you can automate how the classes are created.