mikependon / RepoDB.NET

A hybrid ORM library for .NET.
MIT License
5 stars 8 forks source link

Bug: UnitOfWork example contains errors #43

Open zetomatoz opened 2 years ago

zetomatoz commented 2 years ago

The UnitOfWork implementation example contains several issues.

  1. The IUnitOfWork interface declares a Begin method, which is not implemented. Instead, UnitOfWork implements a Start method, which should be renamed Begin.
public interface IUnitOfWork<TDbConnection>
{
    TDbConnection Connection { get; }
    DbTransaction Transaction { get ; }
    void Begin();
    void Rollback();
    void Commit();
}
public void Start()
{
    if (transaction != null)
    {
        throw new InvalidOperationException("Cannot start a new transaction while the existing other one is still open.");
    }
    var connection = EnsureConnection();
    transaction = connection.BeginTransaction();
}
  1. The EnsureConnection method contains a bug that can produce a runtime exception when the Start method gets called because it tries to get a Transaction instance even if the Connection instance is not opened yet.
private SqlConnection EnsureConnection() =>
    connection ?? connection = new SqlConnection(settings.ConnectionString);

Instead EnsureConnection should be implemented like that:

private SqlConnection EnsureConnection() =>
    connection ??= (SqlConnection) new SqlConnection(settings.ConnectionString).EnsureOpen();
mikependon commented 2 years ago

Thanks for leading us on this documentation problem. We will do fix this.

Vinko90 commented 2 years ago

I am trying to implement the UoW from documentation and I have another issue on this topic:

This is my IUnitOfWork:

public interface IUnitOfWork<TDbConnection>
{
    TDbConnection Connection { get; }
    DbTransaction Transaction { get ; }
    void Begin();
    void Rollback();
    void Commit();
}

And this is my IRepository.cs

public interface IRepository<TEntity> where TEntity : class
{
    void Attach(IUnitOfWork unitOfWork);
    TResult Add<TResult>(TEntity entity);
    int AddAll<TResult>(IEnumerable<TEntity> entities);
    int Delete(object id);
    int Delete(TEntity entity);
    TResult Merge<TResult>(TEntity entity);
    TEntity Query(object id);
    int Update(TEntity entity);
}

However on the Attach method I have a red line under IUnitOfWork: "Incorrect number of type parameters in reference to interface 'Demo.Data.IUnitOfWork'"

I can't figure out what to do? Can maybe someone point me out to a working implementation of UoW with RepoDB that use transactions? Thank you!

mikependon commented 2 years ago

@Vinko90 @zetomatoz here is a sample project that implements the UnitOfWork. I created it from scratch, which it also shows how we in the organization uses ours. I strongly suggest you follow the implementation here.

Note: We will update the documentation now and will push the changes right away.

This sample project is complete only as a reference for the UoW implementation, but is not a working solution in overall. If you need a fully working solution, please do let us know.

APIProjectWithUnitOfWork.zip

[EDIT] The document to the Unit of Work has also been updated.

Vinko90 commented 2 years ago

First of all, thank you very much, the code was really helpful to double check what I did wrong!

Also, I wanted to let you know that running exactly the code you provided (And now in the documentation), I was getting the exception that zetomatoz reported: "Can't start a transaction because the connection is not open".

By updating the UnitOfWork in the following way, everything works as expected :) :

public NpgsqlConnection Connection => _connection;
public DbTransaction Transaction => _transaction;

private NpgsqlConnection EnsureConnection() =>
        _connection ??= (NpgsqlConnection) new NpgsqlConnection(_dbSettings.ConnectionString).EnsureOpen();

public void Begin()
{
    if (_transaction != null)
    {
        throw new InvalidOperationException("Cannot start a new transaction while the existing one is still open.");
    }
    _connection = EnsureConnection();
    _transaction = _connection.BeginTransaction();
}
mikependon commented 2 years ago

Glad you made it, that is the area where you should I put it as well. 👍