Unit of Work & Repository Pattern - Injecting UoW into Repository Base

Question

Description

I am implementing the Unit of Work & Repository Pattern with Dapper as my ORM and PostgreSQL as the relational database. I have seen examples of UoW where:

• The UoW class creates an SQL connection upon construction and injects a transaction into instantiated repositories, and
• The connection & transaction objects are injected into the repository & UoW classes

I decided to inject my UoW class into an abstract RepositoryBase class, allowing all repository classes to share the same transaction and expose a SaveChanges (aka Commit) method.

Question

I would like feedback regarding the injection of the UoW class into a RepositoryBase class. I have not seen it done this way, so I question if there is some grave mistake that I am unaware of.

Code

Unit of Work

The UoW class exposes a SaveChanges and Transaction method. The Transaction method determines if there is currently a transaction, if there is, it is returned, if not, a new connection and transaction are created and the transaction is returned.

public sealed class UnitOfWork : IUnitOfWork
{
 private string _connectionString;
 private DbTransaction? _transaction;
 public UnitOfWork(IDbConfiguration configuration)
 {
 this._connectionString = configuration.ConnectionString;
 this._transaction = null;
 }
 public async Task SaveChangesAsync(CancellationToken cancellationToken = default)
 {
 cancellationToken.ThrowIfCancellationRequested();
 DbTransaction t = await this.Transaction();
 try
 {
 await t.CommitAsync();
 }
 catch
 {
 await t.RollbackAsync();
 throw;
 }
 finally
 {
 await t.DisposeAsync();
 }
 }
 public async Task<DbTransaction> Transaction()
 {
 if (this._transaction is not null && this._transaction.Connection is not null) return this._transaction;
 if (this._transaction is not null) await this._transaction.DisposeAsync();
 DbConnection connection = new NpgsqlConnection(this._connectionString);
 await connection.OpenAsync();
 this._transaction = await connection.BeginTransactionAsync();
 return this._transaction;
 }
}

Repository Base

The UoW class is injected into the RepositoryBase class. The SaveChanges method is public, while the Transaction method is protected, only allowing the repository classes to use it.

public abstract class RepositoryBase<T> : IRepository<T> where T : class
{
 private IUnitOfWork _unitOfWork;
 public RepositoryBase(IUnitOfWork unitOfWork)
 {
 this._unitOfWork = unitOfWork;
 }
 public abstract Task InsertAsync(T item);
 public abstract Task RemoveAsync(T item);
 public async Task SaveChangesAsync(CancellationToken cancellationToken)
 {
 await this._unitOfWork.SaveChangesAsync(cancellationToken);
 }
 protected async Task<DbTransaction> Transaction()
 {
 return await this._unitOfWork.Transaction();
 }
}

Owner Repository

The OwnerRepository class Insert method shows how the transaction is shared.

public sealed class OwnerRepository : RepositoryBase<Owner>, IOwnerRepository
{
 public OwnerRepository(IUnitOfWork unitOfWork) : base(unitOfWork) {}
 private async Task<IEnumerable<Owner>> _GetAsync(string sql, object? param)
 {
 DbTransaction transaction = await this.Transaction();
 DbConnection connection = transaction.Connection!;
 Dictionary<Guid, OwnerTable> ownerDictionary = new Dictionary<Guid, OwnerTable>();
 IEnumerable<OwnerTable> dtos = (
 await connection.QueryAsync<OwnerTable, AccountTable, OwnerTable>(
 sql,
 (ownerDto, accountDto) =>
 {
 OwnerTable ownerEntry;
 if (!ownerDictionary.TryGetValue(ownerDto.owner_id, out ownerEntry!))
 {
 ownerEntry = ownerDto;
 ownerEntry.accounts = new List<AccountTable>();
 ownerDictionary.Add(ownerEntry.owner_id, ownerEntry);
 }
 if (accountDto != null) ownerEntry.accounts?.Add(accountDto);
 return ownerEntry;
 },
 transaction: transaction,
 param: param,
 splitOn: AccountTable.Column.AccountId
 )
 )
 .Distinct();
 return dtos.Select(d => d.Adapt());
 }
 public async Task<IEnumerable<Owner>> GetAllAsync(CancellationToken cancellationToken = default)
 {
 cancellationToken.ThrowIfCancellationRequested();
 string sql = $@"
 SELECT *
 FROM {OwnerTable.Title}
 LEFT JOIN {AccountTable.Title}
 ON ({OwnerTable.Title}.{OwnerTable.Column.OwnerId} = {AccountTable.Title}.{AccountTable.Column.OwnerId})";
 IEnumerable<Owner> owners = await this._GetAsync(sql, null);
 return owners;
 }
 public async Task<Owner> GetByIdAsync(Guid ownerId, CancellationToken cancellationToken = default)
 {
 cancellationToken.ThrowIfCancellationRequested();
 
 string sql = $@"
 SELECT *
 FROM {OwnerTable.Title}
 LEFT JOIN {AccountTable.Title}
 ON ({OwnerTable.Title}.{OwnerTable.Column.OwnerId} = {AccountTable.Title}.{AccountTable.Column.OwnerId})
 WHERE {OwnerTable.Title}.{OwnerTable.Column.OwnerId} = @{nameof(ownerId)}";
 IEnumerable<Owner> owners = await this._GetAsync(sql, new { ownerId });
 Owner owner = owners.DefaultIfEmpty(new Owner()).First();
 return owner;
 }
 public override async Task InsertAsync(Owner owner)
 {
 DbTransaction transaction = await this.Transaction();
 DbConnection connection = transaction.Connection!;
 OwnerTable dto = new OwnerTable(owner);
 string sql = $@"
 INSERT INTO {OwnerTable.Title} 
 (
 {OwnerTable.Column.Name},
 {OwnerTable.Column.DateOfBirth},
 {OwnerTable.Column.Address}
 )
 VALUES 
 (
 @{OwnerTable.Column.Name},
 @{OwnerTable.Column.DateOfBirth},
 @{OwnerTable.Column.Address}
 )";
 await connection.ExecuteAsync(sql, dto, transaction);
 }
 public override async Task RemoveAsync(Owner owner)
 {
 DbTransaction transaction = await this.Transaction();
 DbConnection connection = transaction.Connection!;
 Guid id = owner.Id;
 string sqlOwner = @$"
 DELETE
 FROM {OwnerTable.Title}
 WHERE {OwnerTable.Column.OwnerId} = @{nameof(id)}";
 string sqlAccount = @$"
 DELETE
 FROM {AccountTable.Title}
 WHERE {AccountTable.Column.OwnerId} = @{nameof(id)}";
 await connection.ExecuteAsync(sqlAccount, new {id}, transaction);
 await connection.ExecuteAsync(sqlOwner, new {id}, transaction);
 }
 public async Task UpdateAsync(Owner owner)
 {
 DbTransaction transaction = await this.Transaction();
 DbConnection connection = transaction.Connection!;
 OwnerTable dto = new OwnerTable(owner);
 string sql = @$"
 UPDATE {OwnerTable.Title}
 SET 
 {OwnerTable.Column.Name} = @{nameof(dto.name)},
 {OwnerTable.Column.DateOfBirth} = @{nameof(dto.date_of_birth)},
 {OwnerTable.Column.Address} = @{nameof(dto.address)}
 WHERE {OwnerTable.Column.OwnerId} = @{nameof(dto.owner_id)}";
 await connection.ExecuteAsync(sql, dto, transaction);
 }

Owner Service

The OwnerService class CreateAsync method shows how the UoW is implemented.

public sealed class OwnerService : IOwnerService
{
 private readonly IOwnerRepository _ownerRepository;
 public OwnerService(IOwnerRepository ownerRepository)
 {
 this._ownerRepository = ownerRepository;
 }
 public async Task<OwnerDto> CreateAsync(OwnerForCreationDto ownerForCreationDto, CancellationToken cancellationToken = default)
 {
 Owner owner = ownerForCreationDto.Adapt<Owner>();
 await this._ownerRepository.InsertAsync(owner);
 await this._ownerRepository.SaveChangesAsync(cancellationToken);
 return owner.Adapt<OwnerDto>();
 }
 public async Task DeleteAsync(Guid ownerId, CancellationToken cancellationToken = default)
 {
 Owner owner = await this._ownerRepository.GetByIdAsync(ownerId, cancellationToken);
 if (owner is null)
 {
 throw new OwnerNotFoundException(ownerId);
 }
 await this._ownerRepository.RemoveAsync(owner);
 await this._ownerRepository.SaveChangesAsync(cancellationToken);
 }
 public async Task<IEnumerable<OwnerDto>> GetAllAsync(CancellationToken cancellationToken = default)
 {
 IEnumerable<Owner> owners = await this._ownerRepository.GetAllAsync(cancellationToken);
 IEnumerable<OwnerDto> ownersDto = owners.Adapt<IEnumerable<OwnerDto>>();
 return ownersDto;
 }
 public async Task<OwnerDto> GetByIdAsync(Guid ownerId, CancellationToken cancellationToken = default)
 {
 Owner owner = await this._ownerRepository.GetByIdAsync(ownerId, cancellationToken);
 if (owner is null)
 {
 throw new OwnerNotFoundException(ownerId);
 }
 OwnerDto ownerDto = owner.Adapt<OwnerDto>();
 return ownerDto;
 }
 public async Task UpdateAsync(Guid ownerId, OwnerForUpdateDto ownerForUpdateDto, CancellationToken cancellationToken = default)
 {
 Owner owner = ownerForUpdateDto.Adapt<Owner>();
 owner.Id = ownerId;
 await this._ownerRepository.UpdateAsync(owner);
 await this._ownerRepository.SaveChangesAsync(cancellationToken);
 }
}

Owner API Controller

The OwnerController class CreateOwner endpoint shows how the controller uses the OwnerService to create an Owner.

[ApiController]
[Route("api/[controller]")]
public class OwnersController : ControllerBase
{
 private readonly IOwnerService _ownerService;
 public OwnersController(IOwnerService ownerService) => this._ownerService = ownerService;
 [HttpGet]
 public async Task<IActionResult> GetOwners(CancellationToken cancellationToken)
 {
 IEnumerable<OwnerDto> owners = await this._ownerService.GetAllAsync(cancellationToken);
 return Ok(owners);
 }
 [HttpGet("{ownerId:guid}")]
 public async Task<IActionResult> GetOwnerById(Guid ownerId, CancellationToken cancellationToken)
 {
 OwnerDto owner = await this._ownerService.GetByIdAsync(ownerId, cancellationToken);
 return Ok(owner);
 }
 [HttpPost]
 public async Task<IActionResult> CreateOwner([FromBody] OwnerForCreationDto ownerForCreationDto)
 {
 OwnerDto owner = await this._ownerService.CreateAsync(ownerForCreationDto);
 return CreatedAtAction(nameof(GetOwnerById), new { ownerId = owner.Id }, owner);
 }
 [HttpPut("{ownerId:guid}")]
 public async Task<IActionResult> UpdateOwner(Guid ownerId, [FromBody] OwnerForUpdateDto ownerForUpdateDto, CancellationToken cancellationToken)
 {
 await this._ownerService.UpdateAsync(ownerId, ownerForUpdateDto, cancellationToken);
 return NoContent();
 }
 [HttpDelete("{ownerId:guid}")]
 public async Task<IActionResult> DeleteOwner(Guid ownerId, CancellationToken cancellationToken)
 {
 await this._ownerService.DeleteAsync(ownerId, cancellationToken);
 return NoContent();
 }
}

Composition Root

And finally, the composition root, the ControllerActivator class Create method, composes the OwnersController.

public sealed class ControllerActivator : IControllerActivator, IDisposable
{
 private readonly IDbConfiguration _dbConfiguration;
 public ControllerActivator(IDbConfiguration dBconfiguration) => this._dbConfiguration = dBconfiguration;
 public object Create(ControllerContext context)
 {
 return this.Create(context, context.ActionDescriptor.ControllerTypeInfo.AsType());
 }
 public ControllerBase Create(ControllerContext context, Type controllerType)
 {
 // Scoped services
 IUnitOfWork unitOfWork = new UnitOfWork(this._dbConfiguration);
 switch (controllerType.Name)
 {
 case nameof(OwnersController):
 return new OwnersController(
 new OwnerService(
 new OwnerRepository(unitOfWork)
 )
 );
 case nameof(AccountsController):
 return new AccountsController(
 new AccountService(
 new AccountRepository(unitOfWork)
 )
 );
 default:
 throw new InvalidOperationException($"Unknown controller {controllerType}.");
 }
 }
 public void Dispose()
 {
 Console.WriteLine("Disposing the controller activator!");
 }
 public void Release(ControllerContext context, object controller)
 {
 (controller as IDisposable)?.Dispose();
 }
}

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Do the occasional ellipses in the code (i.e. ...) represent removed code? If you are concerned about the size of your code, bear in mind that the character limit on Code Review was increased to 65k.
• zwoolli: Thank you! The ellipses do indeed represent removed code. Not due to code size, rather to keep the focus on specific class methods. I included links to each class in the repo. Would it be better to show the entire class?
• Sᴀᴍ Onᴇᴌᴀ: Per the on-topic Help page "It is fine to post supplementary code on a third-party site, but the most important parts of the code must be embedded directly in the question." so as long as the essential parts of the code are present that is fine. Be aware that reviewers might attempt to run the code so it might be wise to ensure the ellipses are in comments so as to avoid run-time errors.
• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! I have rolled back Rev 5 → 4. Please see What to do when someone answers - especially the section What should I not do?.

Answer 1

Quick remarks:

• Transaction() is not a proper method name.

• OwnerTable.Title: a table has no "title", it has a name.

• OwnerForCreationDto is an odd name. I get what you're going for, but IMHO code is harder to maintain when you have to parse names to figure out what something is about.

• dBconfiguration is incorrectly capitalized.

---

But IMHO this whole UoW concept is too simplistic, and you can see this in the Create method of ControllerBase. Sure, this works when you've got something simple like "create a new Owner", "create a new Account", but in my daily work that kind of simplistic logic is a very minor part. My time is usually spent on far more complicated business logic which involves inserts or updates or deletes which involve multiple tables which all should happen in a single transaction, combined with lookups in other tables etc. How are you going to deal with that?

Honestly, you're going to spend far too much time on extending the frameworks you've built in order to force your code in that rigid structure, instead of adding value to the business. Why use Dapper and then cram it into a straightjacket and lose so much of the flexibility it offers?

I'm also not a fan of mixing SQL into C#. A single line, that's okay, but your INSERT INTO {OwnerTable.Title} is a fairly simple operation and yet it already involves 10+ lines of SQL code. I find that ugly and unmaintainable. Whenever I have to employ a long-ish SQL query I tend to put it in an .sql file, embed that file into the solution and then use code like this to retrieve the SQL code and parse it. The advantage: the SQL code in the .sql file is treated as SQL code by VS and this get color-coded etc. (I still wish VS would consider using different extensions, e.g. ".tsql" -- for T-SQL -- and ".osql" -- for Oracle queries -- to allow for different types of queries with their proper IntelliSense etc.)

• zwoolli: thank you for the feedback! I agree with all of your "quick remarks". The UoW helps to keep track of everything done during a business transaction that can affect the database. I believe it has nothing to do with how simple or complex my queries are. As for combined look-ups - I included the entire OwnerRepository class. It has a private _GetAsync method that can handle JOINs. I like your QueryRetriever idea, but does this mean your queries are fully hard-typed? Is parsing performant?