Skip to content

Commit

Permalink
fix(api): fix potential issues with concurrent updates
Browse files Browse the repository at this point in the history
  • Loading branch information
VMelnalksnis committed Sep 29, 2024
1 parent a1ab803 commit 34506dd
Show file tree
Hide file tree
Showing 28 changed files with 357 additions and 342 deletions.
4 changes: 4 additions & 0 deletions docs/changelog.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ <h3>Fixed</h3>
Display of account/counterparty name in transaction overview in
<a href="https://github.com/VMelnalksnis/Gnomeshade/pull/1345">#1345</a>
</li>
<li>
Potential issues with concurrent updates in
<a href="https://github.com/VMelnalksnis/Gnomeshade/pull/1393">#1393</a>
</li>
</ul>
</section>

Expand Down
2 changes: 1 addition & 1 deletion docs/sitemap.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
</url>
<url>
<loc>https://www.gnomeshade.org/changelog</loc>
<lastmod>2024-09-22</lastmod>
<lastmod>2024-09-29</lastmod>
</url>
</urlset>
49 changes: 9 additions & 40 deletions source/Gnomeshade.Data/AccountUnitOfWork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,24 @@ namespace Gnomeshade.Data;
/// <summary>Performs bulk actions on account entities.</summary>
public sealed class AccountUnitOfWork
{
private readonly DbConnection _dbConnection;
private readonly AccountRepository _repository;
private readonly AccountInCurrencyRepository _inCurrencyRepository;
private readonly CounterpartyRepository _counterpartyRepository;

/// <summary>Initializes a new instance of the <see cref="AccountUnitOfWork"/> class.</summary>
/// <param name="dbConnection">The database connection for executing queries.</param>
/// <param name="repository">The repository for managing accounts.</param>
/// <param name="inCurrencyRepository">The repository for managing accounts in currencies.</param>
/// <param name="counterpartyRepository">The repository for managing counterparties.</param>
public AccountUnitOfWork(
DbConnection dbConnection,
AccountRepository repository,
AccountInCurrencyRepository inCurrencyRepository,
CounterpartyRepository counterpartyRepository)
{
_dbConnection = dbConnection;
_repository = repository;
_inCurrencyRepository = inCurrencyRepository;
_counterpartyRepository = counterpartyRepository;
}

/// <summary>Creates a new account with the currencies in <see cref="AccountEntity.Currencies"/>.</summary>
/// <param name="account">The account to create.</param>
/// <returns>The id of the created account.</returns>
public async Task<Guid> AddAsync(AccountEntity account)
{
await using var dbTransaction = await _dbConnection.OpenAndBeginTransaction();
var id = await AddAsync(account, dbTransaction).ConfigureAwait(false);
await dbTransaction.CommitAsync();
return id;
}

/// <summary>Creates a new account with the currencies in <see cref="AccountEntity.Currencies"/>.</summary>
/// <param name="account">The account to create.</param>
/// <param name="dbTransaction">The database transaction to use for queries.</param>
Expand Down Expand Up @@ -105,30 +90,9 @@ public async Task<Guid> AddWithCounterpartyAsync(AccountEntity account, DbTransa
/// <summary>Deletes the specified account and all its currencies.</summary>
/// <param name="account">The account to delete.</param>
/// <param name="userId">The id of the owner of the entity.</param>
/// <param name="dbTransaction">The database transaction to use for queries.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task DeleteAsync(AccountEntity account, Guid userId)
{
await using var dbTransaction = await _dbConnection.OpenAndBeginTransaction();
await DeleteAsync(account, userId, dbTransaction).ConfigureAwait(false);
await dbTransaction.CommitAsync();
}

/// <summary>Updates the specified account.</summary>
/// <param name="account">The account to update.</param>
/// <param name="modifiedBy">The user which modified the <paramref name="account"/>.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
public async Task UpdateAsync(AccountEntity account, UserEntity modifiedBy)
{
await using var dbTransaction = await _dbConnection.OpenAndBeginTransaction();
if (await UpdateAsync(account, modifiedBy, dbTransaction) is not 1)
{
throw new InvalidOperationException("Failed to update account");
}

await dbTransaction.CommitAsync();
}

private async Task DeleteAsync(AccountEntity account, Guid userId, DbTransaction dbTransaction)
public async Task DeleteAsync(AccountEntity account, Guid userId, DbTransaction dbTransaction)
{
foreach (var currency in account.Currencies)
{
Expand All @@ -144,10 +108,15 @@ private async Task DeleteAsync(AccountEntity account, Guid userId, DbTransaction
}
}

/// <summary>Updates the specified account.</summary>
/// <param name="account">The account to update.</param>
/// <param name="modifiedBy">The user which modified the <paramref name="account"/>.</param>
/// <param name="dbTransaction">The database transaction to use for queries.</param>
/// <returns>The number of affected rows.</returns>
[MustUseReturnValue]
private Task<int> UpdateAsync(AccountEntity account, UserEntity modifiedBy, DbTransaction dbTransaction)
public async Task<int> UpdateAsync(AccountEntity account, UserEntity modifiedBy, DbTransaction dbTransaction)
{
account.ModifiedByUserId = modifiedBy.Id;
return _repository.UpdateAsync(account, dbTransaction);
return await _repository.UpdateAsync(account, dbTransaction);
}
}
18 changes: 3 additions & 15 deletions source/Gnomeshade.Data/Repositories/AccountInCurrencyRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,9 @@
namespace Gnomeshade.Data.Repositories;

/// <summary>Database backed <see cref="AccountInCurrencyRepository"/> repository.</summary>
public sealed class AccountInCurrencyRepository : Repository<AccountInCurrencyEntity>
public sealed class AccountInCurrencyRepository(ILogger<AccountInCurrencyRepository> logger, DbConnection dbConnection)
: Repository<AccountInCurrencyEntity>(logger, dbConnection)
{
/// <summary>Initializes a new instance of the <see cref="AccountInCurrencyRepository"/> class with a database connection.</summary>
/// <param name="logger">Logger for logging in the specified category.</param>
/// <param name="dbConnection">The database connection for executing queries.</param>
public AccountInCurrencyRepository(ILogger<AccountInCurrencyRepository> logger, DbConnection dbConnection)
: base(logger, dbConnection)
{
}

/// <inheritdoc />
protected override string DeleteSql => Queries.AccountInCurrency.Delete;

Expand All @@ -48,16 +41,11 @@ public AccountInCurrencyRepository(ILogger<AccountInCurrencyRepository> logger,
/// <inheritdoc />
protected override string NotDeleted => "a.deleted_at IS NULL";

public Task RestoreDeletedAsync(Guid id, Guid userId)
{
return DbConnection.ExecuteAsync(Queries.AccountInCurrency.RestoreDeleted, new { id, userId });
}

public Task RestoreDeletedAsync(Guid id, Guid userId, DbTransaction dbTransaction)
{
return DbConnection.ExecuteAsync(
Queries.AccountInCurrency.RestoreDeleted,
new { id, userId },
transaction: dbTransaction);
dbTransaction);
}
}
13 changes: 3 additions & 10 deletions source/Gnomeshade.Data/Repositories/OwnershipRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,9 @@
namespace Gnomeshade.Data.Repositories;

/// <summary>Persistence store of <see cref="OwnershipEntity"/>.</summary>
public sealed class OwnershipRepository : Repository<OwnershipEntity>
public sealed class OwnershipRepository(ILogger<OwnershipRepository> logger, DbConnection dbConnection)
: Repository<OwnershipEntity>(logger, dbConnection)
{
/// <summary>Initializes a new instance of the <see cref="OwnershipRepository"/> class.</summary>
/// <param name="logger">Logger for logging in the specified category.</param>
/// <param name="dbConnection">The database connection for executing queries.</param>
public OwnershipRepository(ILogger<OwnershipRepository> logger, DbConnection dbConnection)
: base(logger, dbConnection)
{
}

/// <inheritdoc />
protected override string DeleteSql => Queries.Ownership.Delete;

Expand Down Expand Up @@ -56,6 +49,6 @@ public async Task AddDefaultAsync(Guid id, DbTransaction dbTransaction)
var accessCommand = new CommandDefinition(text, null, dbTransaction);
var accessId = await DbConnection.QuerySingleAsync<Guid>(accessCommand);

await AddAsync(new() { Id = id, OwnerId = id, UserId = id, AccessId = accessId });
await AddAsync(new() { Id = id, OwnerId = id, UserId = id, AccessId = accessId }, dbTransaction);
}
}
8 changes: 4 additions & 4 deletions source/Gnomeshade.Data/Repositories/ProjectRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public sealed class ProjectRepository(ILogger<ProjectRepository> logger, DbConne
/// <inheritdoc />
protected override string SelectSql => Queries.Project.Select;

public Task<int> AddPurchaseAsync(Guid id, Guid purchaseId, Guid userId)
public Task<int> AddPurchaseAsync(Guid id, Guid purchaseId, Guid userId, DbTransaction dbTransaction)
{
const string sql =
"""
Expand All @@ -54,11 +54,11 @@ INSERT INTO project_purchases
(CURRENT_TIMESTAMP, @userId, @id, @purchaseId);
""";

var command = new CommandDefinition(sql, new { id, purchaseId, userId });
var command = new CommandDefinition(sql, new { id, purchaseId, userId }, dbTransaction);
return DbConnection.ExecuteAsync(command);
}

public Task<int> RemovePurchaseAsync(Guid id, Guid purchaseId, Guid userId)
public Task<int> RemovePurchaseAsync(Guid id, Guid purchaseId, Guid userId, DbTransaction dbTransaction)
{
const string sql =
"""
Expand All @@ -67,7 +67,7 @@ DELETE FROM project_purchases
AND project_purchases.purchase_id = @purchaseId;
""";

var command = new CommandDefinition(sql, new { id, purchaseId, userId });
var command = new CommandDefinition(sql, new { id, purchaseId, userId }, dbTransaction);
return DbConnection.ExecuteAsync(command);
}
}
51 changes: 13 additions & 38 deletions source/Gnomeshade.Data/Repositories/Repository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,6 @@ protected Repository(ILogger<Repository<TEntity>> logger, DbConnection dbConnect
/// <summary>Gets the SQL for filtering out deleted rows.</summary>
protected abstract string NotDeleted { get; }

/// <summary>Adds a new entity.</summary>
/// <param name="entity">The entity to add.</param>
/// <returns>The id of the created entity.</returns>
public Task<Guid> AddAsync(TEntity entity)
{
Logger.AddingEntity();
return DbConnection.QuerySingleAsync<Guid>(InsertSql, entity);
}

/// <summary>Adds a new entity using the specified database transaction.</summary>
/// <param name="entity">The entity to add.</param>
/// <param name="dbTransaction">The database transaction to use for the query.</param>
Expand All @@ -85,19 +76,6 @@ public Task<Guid> AddAsync(TEntity entity, DbTransaction dbTransaction)
return DbConnection.QuerySingleAsync<Guid>(command);
}

/// <summary>Deletes the entity with the specified id.</summary>
/// <param name="id">The id of the entity to delete.</param>
/// <param name="userId">The id of the user requesting access to the entity.</param>
/// <returns>The number of affected rows.</returns>
[MustUseReturnValue]
public async Task<int> DeleteAsync(Guid id, Guid userId)
{
Logger.DeletingEntity(id);
var count = await DbConnection.ExecuteAsync(DeleteSql, new { id, userId });
Logger.DeletedRows(count);
return count;
}

/// <summary>Deletes the entity with the specified id using the specified database transaction.</summary>
/// <param name="id">The id of the entity to delete.</param>
/// <param name="userId">The id of the user requesting access to the entity.</param>
Expand Down Expand Up @@ -136,6 +114,15 @@ public Task<IEnumerable<TEntity>> GetAsync(Guid userId, CancellationToken cancel
cancellationToken: cancellationToken));
}

public Task<IEnumerable<TEntity>> GetAsync(Guid userId, DbTransaction dbTransaction)
{
Logger.GetAll();
return GetEntitiesAsync(new(
$"{SelectActiveSql} {GroupBy};",
new { userId, access = Read.ToParam() },
dbTransaction));
}

/// <summary>Gets an entity with the specified id.</summary>
/// <param name="id">The id of the entity to get.</param>
/// <param name="userId">The id of the user requesting access to the entity.</param>
Expand Down Expand Up @@ -165,7 +152,7 @@ public Task<TEntity> GetByIdAsync(Guid id, Guid userId, DbTransaction dbTransact
}

/// <summary>Searches for an entity with the specified id.</summary>
/// <param name="id">The id to to search by.</param>
/// <param name="id">The id to search by.</param>
/// <param name="userId">The id of the user requesting access to the entity.</param>
/// <param name="accessLevel">The access level to check.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> to observe while waiting for the task to complete.</param>
Expand All @@ -184,7 +171,7 @@ public Task<TEntity> GetByIdAsync(Guid id, Guid userId, DbTransaction dbTransact
}

/// <summary>Searches for an entity with the specified id using the specified database transaction.</summary>
/// <param name="id">The id to to search by.</param>
/// <param name="id">The id to search by.</param>
/// <param name="userId">The id of the user requesting access to the entity.</param>
/// <param name="dbTransaction">The database transaction to use for the query.</param>
/// <param name="accessLevel">The access level to check.</param>
Expand All @@ -203,7 +190,7 @@ public Task<TEntity> GetByIdAsync(Guid id, Guid userId, DbTransaction dbTransact
}

/// <summary>Searches for an entity with the specified id.</summary>
/// <param name="id">The id to to search by.</param>
/// <param name="id">The id to search by.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> to observe while waiting for the task to complete.</param>
/// <returns>The entity if one exists, otherwise <see langword="null"/>.</returns>
public Task<TEntity?> FindByIdAsync(Guid id, CancellationToken cancellationToken = default)
Expand All @@ -216,7 +203,7 @@ public Task<TEntity> GetByIdAsync(Guid id, Guid userId, DbTransaction dbTransact
}

/// <summary>Searches for an entity with the specified id.</summary>
/// <param name="id">The id to to search by.</param>
/// <param name="id">The id to search by.</param>
/// <param name="dbTransaction">The database transaction to use for the query.</param>
/// <returns>The entity if one exists, otherwise <see langword="null"/>.</returns>
public Task<TEntity?> FindByIdAsync(Guid id, DbTransaction dbTransaction)
Expand All @@ -228,18 +215,6 @@ public Task<TEntity> GetByIdAsync(Guid id, Guid userId, DbTransaction dbTransact
dbTransaction));
}

/// <summary>Updates an existing entity with the specified id.</summary>
/// <param name="entity">The entity to update.</param>
/// <returns>The number of affected rows.</returns>
[MustUseReturnValue]
public async Task<int> UpdateAsync(TEntity entity)
{
Logger.UpdatingEntity();
var count = await DbConnection.ExecuteAsync(UpdateSql, entity);
Logger.UpdatedRows(count);
return count;
}

/// <summary>Updates an existing entity with the specified id using the specified database transaction.</summary>
/// <param name="entity">The entity to update.</param>
/// <param name="dbTransaction">The database transaction to use for the query.</param>
Expand Down
15 changes: 4 additions & 11 deletions source/Gnomeshade.Data/Repositories/TransactionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,9 @@
namespace Gnomeshade.Data.Repositories;

/// <summary>Database backed <see cref="TransactionEntity"/> repository.</summary>
public sealed class TransactionRepository : Repository<TransactionEntity>
public sealed class TransactionRepository(ILogger<TransactionRepository> logger, DbConnection dbConnection)
: Repository<TransactionEntity>(logger, dbConnection)
{
/// <summary>Initializes a new instance of the <see cref="TransactionRepository"/> class with a database connection.</summary>
/// <param name="logger">Logger for logging in the specified category.</param>
/// <param name="dbConnection">The database connection for executing queries.</param>
public TransactionRepository(ILogger<TransactionRepository> logger, DbConnection dbConnection)
: base(logger, dbConnection)
{
}

/// <inheritdoc />
protected override string DeleteSql => Queries.Transaction.Delete;

Expand Down Expand Up @@ -136,7 +129,7 @@ public async Task<IEnumerable<LinkEntity>> GetAllLinksAsync(
return entities.DistinctBy(entity => entity.Id);
}

public Task<int> AddLinkAsync(Guid id, Guid linkId, Guid userId)
public Task<int> AddLinkAsync(Guid id, Guid linkId, Guid userId, DbTransaction dbTransaction)
{
const string sql = @"
INSERT INTO transaction_links
Expand All @@ -148,7 +141,7 @@ INSERT INTO transaction_links
return DbConnection.ExecuteAsync(command);
}

public Task<int> RemoveLinkAsync(Guid id, Guid linkId, Guid userId)
public Task<int> RemoveLinkAsync(Guid id, Guid linkId, Guid userId, DbTransaction dbTransaction)
{
const string sql = @"
DELETE FROM transaction_links
Expand Down
Loading

0 comments on commit 34506dd

Please sign in to comment.