Generic UnitOfWork implementation with Dapper. (Updated)

Question

Here's my attempt to implement the suggested corrections I received in my previous post. Since System.Data.IDbTransaction doesn't support CommitAsync or RollbackAsync, I kept the base UnitOfWork class generic and synchronous, and implemented async capabilities in a subclass.

UnitOfWork

public class UnitOfWork : IUnitOfWork
{
 private readonly IDbConnection _connection;
 private readonly IServiceProvider _serviceProvider;
 protected IDbTransaction _transaction;
 private bool _disposed;
 private readonly List<IRepository> _repositories = new();
 private readonly object _repositoriesLock = new();
 public IDbTransaction Transaction => _transaction;
 public UnitOfWork(IDbConnection dbConnection, IServiceProvider serviceProvider)
 {
 _connection = dbConnection ?? throw new ArgumentNullException(nameof(dbConnection), "dbConnection cannot be null");
 _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider), "serviceProvider cannot be null");
 if (_connection.State != ConnectionState.Open) _connection.Open();
 _transaction = _connection.BeginTransaction();
 var connection = new NpgsqlConnection(_connection.ConnectionString);
 }
 public TRepository? GetRepository<TRepository>() where TRepository : IRepository
 {
 var repository = _serviceProvider.GetService<TRepository>() ?? throw new InvalidOperationException($"Couldn't inject instanstance of the {typeof(TRepository)} type");
 repository.SetTransaction(_transaction);
 AddRepository(repository);
 return repository;
 }
 public void Commit()
 {
 try
 {
 _transaction.Commit();
 }
 finally
 {
 UpdateTransaction();
 }
 }
 public void Rollback()
 {
 _transaction.Rollback();
 UpdateTransaction();
 }
 protected virtual void UpdateTransaction()
 {
 lock (_repositoriesLock)
 {
 _transaction = _connection.BeginTransaction();
 foreach (var repository in _repositories)
 {
 repository.SetTransaction(_transaction);
 }
 }
 }
 public void Dispose()
 {
 Dispose(true);
 GC.SuppressFinalize(this);
 }
 protected virtual void Dispose(bool disposing)
 {
 if (!_disposed)
 {
 _disposed = true;
 if (disposing)
 {
 _transaction?.Dispose();
 _connection?.Dispose();
 lock (_repositoriesLock)
 {
 foreach (var repository in _repositories)
 {
 if (repository is IDisposable disposable)
 {
 disposable.Dispose();
 }
 }
 _repositories.Clear();
 }
 }
 }
 }
 public void AddRepository(IRepository repository)
 {
 lock (_repositoriesLock)
 {
 if (_repositories.Contains(repository)) return;
 _repositories.Add(repository);
 }
 }
}

AsyncUnitOfWork

public class UnitOfWorkNpgsql(NpgsqlConnection npgsqlConnection, IServiceProvider serviceProvider) : UnitOfWork(npgsqlConnection, serviceProvider), IUnitOfWorkAsync
{
 public async Task CommitAsync()
 {
 try
 {
 await ((NpgsqlTransaction)_transaction).CommitAsync();
 }
 finally
 {
 UpdateTransaction();
 }
 }
 public async Task RollbackAsync()
 {
 await ((NpgsqlTransaction)_transaction).RollbackAsync();
 UpdateTransaction();
 }
}

• Peter Csala: I still believe that the constructor has too much responsibilities. It should not open database connection. Also depending on the .NET version you could use the ThrowIfNull static method during parameter checks like this ArgumentNullException.ThrowIfNull(dbConnection, nameof(dbConnection));
• Peter Csala: The UnitOfWorkNpqsql should implement IAsyncDisposable. Also consider to rollback uncommited changes inside the Dispose.