Leaky abstraction and clean architecture template

According to Wikipedia in software development, a leaky abstraction is an abstraction that leaks details that it is supposed to abstract away. The term "leaky abstraction" was popularized in 2002 by Joel Spolsky. An earlier paper by Kiczales describes some of the issues with imperfect abstractions and presents a potential solution to the problem by allowing for the customization of the abstraction itself.

As systems become more complex, software developers must rely upon more abstractions. Each abstraction tries to hide complexity, letting a developer write software that "handles" the many variations of modern computing.
However, this law claims that developers of reliable software must learn the abstraction's underlying details anyway.

During the past year, I've seen several implementations of clean architecture that inspired form Jason Taylor Clean Architecture Solution Template and all of them have a common interface, IApplicationDbContext. This interface aims to hide the underlying data access technology that is being used but when you look at the interface you notice that the interface is coupled to Entity Framework Core. I also can guess Jason Taylor assumed that you are always using EF Core and it never going to change.

Let's see leaked abstraction in action. I want to implement a unit test for UpdateTodoListCommandHandler and assume it is a real-world project and we some logic inside the Handle method but we don't implement integration for each scenario. Here is the actual implementation of :

namespace CleanArchitecture.Application.TodoLists.Commands.UpdateTodoList
{
    public class UpdateTodoListCommand : IRequest
    {
        public int Id { get; set; }

        public string Title { get; set; }
    }

    public class UpdateTodoListCommandHandler : IRequestHandler<UpdateTodoListCommand>
    {
        private readonly IApplicationDbContext _context;

        public UpdateTodoListCommandHandler(IApplicationDbContext context)
        {
            _context = context;
        }

        public async Task<Unit> Handle(UpdateTodoListCommand request, CancellationToken cancellationToken)
        {
            var entity = await _context.TodoLists.FindAsync(request.Id);
            if (entity == null)
            {
                throw new NotFoundException(nameof(TodoList), request.Id);
            }

            entity.Title = request.Title;
            await _context.SaveChangesAsync(cancellationToken);

            return Unit.Value;
        }
    }
}

I want to ensure when the entity does not exist, NotFoundException is thrown.

[Test]
public void UpdateTodoList_WhenEntityNotExist_ThrowsException()
{
    // Arrange
    var list = new List<TodoList>();
    var queryable = list.AsQueryable();

    var dbSet = new Mock<DbSet<TodoList>>();
    dbSet.As<IQueryable<TodoList>>().Setup(m => m.Provider).Returns(queryable.Provider);
    dbSet.As<IQueryable<TodoList>>().Setup(m => m.Expression).Returns(queryable.Expression);
    dbSet.As<IQueryable<TodoList>>().Setup(m => m.ElementType).Returns(queryable.ElementType);
    dbSet.As<IQueryable<TodoList>>().Setup(m => m.GetEnumerator()).Returns(() => queryable.GetEnumerator());
    dbSet.Setup(d => d.FindAsync(It.IsAny<object[]>())).ReturnsAsync((object[] id) => list.SingleOrDefault(t => t.Id == (int)id[0]));

    var dbContext = new Mock<IApplicationDbContext>();
    dbContext.SetupGet(d => d.TodoLists).Returns(dbSet.Object);

    var sut = new UpdateTodoListCommandHandler(dbContext.Object);
    var command = new UpdateTodoListCommand { Id = 1, Title = "Test" };

    // Act
    var exception = Assert.ThrowsAsync<NotFoundException>(() => sut.Handle(command, new CancellationToken()));

    // Assert
    Assert.NotNull(exception);
}

As you can see I tried to mock IApplicationDbContext and the interface should help to hide underlying implementation but to implement such a test you should know how to mock DbSet. If you use the ApplicationDbContext class instead of IApplicationDbContext interface, the result will be the same and same amount of code is need to mock ApplicationDbContext class.

Instead, we can use repository pattern and hide underlying technology and mock it easily.
Lets replace IApplicationDbContext with ITodoListRepository.

public interface ITodoListRepository
{
    Task<TodoList> GetByIdAsync(int id);

    Task UpdateAsync(TodoList todoList);
}

And the handler:

public class UpdateTodoListCommandHandler : IRequestHandler<UpdateTodoListCommand>
{
    private readonly ITodoListRepository _repository;

    public UpdateTodoListCommandHandler(ITodoListRepository repository)
    {
        _repository = repository;
    }

    public async Task<Unit> Handle(UpdateTodoListCommand request, CancellationToken cancellationToken)
    {
        var entity = await _repository.GetByIdAsync(request.Id);
        if (entity == null)
        {
            throw new NotFoundException(nameof(TodoList), request.Id);
        }

        entity.Title = request.Title;
        await _repository.UpdateAsync(entity);

        return Unit.Value;
    }
}

Now we can refactor the test method:

[Test]
public void UpdateTodoList_WhenEntityNotExist_ThrowsException()
{
    // Arrange
    var list = new List<TodoList>();

    var repository = new Mock<ITodoListRepository>();
    repository.Setup(r => r.GetByIdAsync(It.IsAny<int>())).ReturnsAsync((int id) => list.SingleOrDefault(t => t.Id == id));

    var command = new UpdateTodoListCommand { Title = "Test", Id = 1 };
    var sut = new UpdateTodoListCommandHandler(repository.Object);

    // Act
    var exception = Assert.ThrowsAsync<NotFoundException>(async () => await sut.Handle(command, new CancellationToken()));

    // Assert
    Assert.NotNull(exception);
    Assert.AreEqual(typeof(NotFoundException), exception.GetType());
}

Comments

[Jake Carpenter]

There's a lot here that I would avoid entirely.

First, you're going to save yourself a lot of headache if you don't test by working "layer to layer" with mocks. You're testing implementation detail rather than proving your code works and these tests won't stand up to a refactor. You introduced the Repository to allow you to do this; however, it's not something you should strive for. It's funny this is brought up in the context of leaky abstractions, because in an example that isn't simplest case, the Repository will prove to be even worse at leaking the abstraction.

Instead, write an integration test with either a real database or an in-memory one that validates your system reads/writes to the DB correctly, not that some implementation of a Repository does. Testing state rather than behavior is much more resilient to refactoring and mocks aren't warranted here.

Second, the use of NotFoundException is a glorified goto statement here. Sure, your system cannot proceed due to the requested resource not existing, but this is not an exception that should be unexpected to be thrown and caught in an unknown place. This class is perfectly capable of returning a result to indicate to the caller directly that the resource wasn't found. Forcing the use of try/catch only makes the system more complicated and has performance implications. This can also be more effectively tested at the edges of the system than locally with mocks.

[Mohsen Esmailpour]

If you are developing a todo sample app, I completely agree with you but in a real-world application, with a complex business, you need to write unit tests and also unit tests can be replaced with integration but spending extra time running integration tests and makes the ci/cd pipeline much slower. This sample is borrowed from clean architecture template Github repo and it's not a good use case for implementing a unit test to test business logic and I used it to show when you have such an abstraction and if you want to write a unit test, lot's of effort is needed to mock the leaked data access. I would be glad if you provide a sample code that the repository can cause leaky abstraction.

About NotFoundException, as I said I'm not the author of the code and you can refactor the code and send a pull request to make this clean architecture sample more robust.

[Nemanja Đorđević]

I would never mock IRepository/IDbContext (data access) because there is no value in it.

Unit test is for Domain, and this is Application layer. It should be tested with integration tests.
And pipeline for them is nightly build.

[Mohsen Esmailpour]

In this scenario, all domain models are anemic and there is no logic inside domain models. In .NET space most of the time models don't have behavior and logic exists inside service classes and mocking a repo inside service class can't be avoided.