Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions src/Application/Common/Behaviors/TransactionBehavior.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,14 @@ public async Task<TResponse> Handle(TRequest request, RequestHandlerDelegate<TRe

logger.LogDebug("Starting transaction for {RequestName}", typeof(TRequest).Name);

await unitOfWork.BeginTransactionAsync(cancellationToken);

try
return await unitOfWork.ExecuteInTransactionAsync(async () =>
{
var response = await next(cancellationToken);
logger.LogDebug("Starting transaction for {RequestName}", typeof(TRequest).Name);

// Save changes within transaction
await unitOfWork.SaveChangesAsync(cancellationToken);
logger.LogDebug("Changes saved for {RequestName}", typeof(TRequest).Name);

await unitOfWork.CommitTransactionAsync(cancellationToken);
var response = await next(cancellationToken);

logger.LogDebug("Transaction committed for {RequestName}", typeof(TRequest).Name);
return response;
}
catch (Exception ex)
{
await unitOfWork.RollbackTransactionAsync(cancellationToken);

logger.LogError(ex, "Transaction rolled back for {RequestName}", typeof(TRequest).Name);
throw new InvalidOperationException($"Transaction for {typeof(TRequest).Name} failed", ex);
}
}, cancellationToken);
}
}
2 changes: 2 additions & 0 deletions src/Application/Common/IUnitOfWork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ public interface IUnitOfWork
/// Rollback the current transaction
/// </summary>
Task RollbackTransactionAsync(CancellationToken cancellationToken = default);

Task<T> ExecuteInTransactionAsync<T>(Func<Task<T>> action, CancellationToken cancellationToken = default);
}
27 changes: 27 additions & 0 deletions src/Infrastructure/Data/Contexts/DataContext.UnitOfWork.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Domain.Common;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage;

namespace Infrastructure.Data.Contexts;
Expand Down Expand Up @@ -32,6 +33,32 @@ public async Task BeginTransactionAsync(CancellationToken cancellationToken = de
_currentTransaction = await Database.BeginTransactionAsync(cancellationToken);
}

public async Task<T> ExecuteInTransactionAsync<T>(Func<Task<T>> action, CancellationToken cancellationToken = default)
{
var strategy = Database.CreateExecutionStrategy();

return await strategy.ExecuteAsync(async () =>
{
await using var tx = await Database.BeginTransactionAsync(cancellationToken);

try
{
var result = await action();

await SaveChangesAsync(cancellationToken);
await tx.CommitAsync(cancellationToken);

return result;
}
catch
{
await tx.RollbackAsync(cancellationToken);
throw;
}
});
}
Comment on lines +36 to +59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Note: Duplicate logging in TransactionBehavior.cs.

The TransactionBehavior.cs file (shown in relevant snippets) has duplicate "Starting transaction" log statements at lines 29 and 31. One should be removed to avoid redundant log entries.

🤖 Prompt for AI Agents
In TransactionBehavior.cs around lines 29 to 31 there are duplicate "Starting
transaction" log statements; remove the redundant one (keep a single,
appropriately leveled log) so the message is only emitted once when a
transaction begins, and ensure any contextual data passed to the logger remains
on the retained call.

⚠️ Potential issue | 🔴 Critical

Transaction state inconsistency: ExecuteInTransactionAsync doesn't coordinate with _currentTransaction.

The new ExecuteInTransactionAsync method creates its own transaction (line 42) without checking or updating the _currentTransaction field that's used by the existing transaction methods. This creates several problems:

  1. HasActiveTransaction (line 13) will return false even when a transaction is active via ExecuteInTransactionAsync
  2. If code calls BeginTransactionAsync while inside ExecuteInTransactionAsync, it will silently succeed (line 30 returns early) but won't actually start a nested transaction
  3. Mixing the new wrapper pattern with manual BeginTransactionAsync/CommitTransactionAsync calls could lead to unexpected behavior

Consider one of these approaches:

Option 1: Make ExecuteInTransactionAsync coordinate with _currentTransaction

 public async Task<T> ExecuteInTransactionAsync<T>(Func<Task<T>> action, CancellationToken cancellationToken = default)
 {
+    if (_currentTransaction != null)
+    {
+        throw new InvalidOperationException("Cannot execute in transaction when another transaction is already active");
+    }
+
     var strategy = Database.CreateExecutionStrategy();

     return await strategy.ExecuteAsync(async () =>
     {
-        await using var tx = await Database.BeginTransactionAsync(cancellationToken);
+        _currentTransaction = await Database.BeginTransactionAsync(cancellationToken);

         try
         {
             var result = await action();

             await SaveChangesAsync(cancellationToken);
-            await tx.CommitAsync(cancellationToken);
+            await _currentTransaction.CommitAsync(cancellationToken);

             return result;
         }
         catch
         {
-            await tx.RollbackAsync(cancellationToken);
+            await _currentTransaction.RollbackAsync(cancellationToken);
             throw;
         }
+        finally
+        {
+            if (_currentTransaction != null)
+            {
+                await _currentTransaction.DisposeAsync();
+                _currentTransaction = null;
+            }
+        }
     });
 }

Option 2: Deprecate manual transaction methods

If the new pattern is preferred, consider marking BeginTransactionAsync, CommitTransactionAsync, and RollbackTransactionAsync as [Obsolete] and migrating all usage to ExecuteInTransactionAsync.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public async Task<T> ExecuteInTransactionAsync<T>(Func<Task<T>> action, CancellationToken cancellationToken = default)
{
var strategy = Database.CreateExecutionStrategy();
return await strategy.ExecuteAsync(async () =>
{
await using var tx = await Database.BeginTransactionAsync(cancellationToken);
try
{
var result = await action();
await SaveChangesAsync(cancellationToken);
await tx.CommitAsync(cancellationToken);
return result;
}
catch
{
await tx.RollbackAsync(cancellationToken);
throw;
}
});
}
public async Task<T> ExecuteInTransactionAsync<T>(Func<Task<T>> action, CancellationToken cancellationToken = default)
{
if (_currentTransaction != null)
{
throw new InvalidOperationException("Cannot execute in transaction when another transaction is already active");
}
var strategy = Database.CreateExecutionStrategy();
return await strategy.ExecuteAsync(async () =>
{
_currentTransaction = await Database.BeginTransactionAsync(cancellationToken);
try
{
var result = await action();
await SaveChangesAsync(cancellationToken);
await _currentTransaction.CommitAsync(cancellationToken);
return result;
}
catch
{
await _currentTransaction.RollbackAsync(cancellationToken);
throw;
}
finally
{
if (_currentTransaction != null)
{
await _currentTransaction.DisposeAsync();
_currentTransaction = null;
}
}
});
}



public async Task CommitTransactionAsync(CancellationToken cancellationToken = default)
{
if (_currentTransaction == null)
Expand Down
6 changes: 6 additions & 0 deletions src/Infrastructure/Data/Contexts/DataContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Identity.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Storage;

namespace Infrastructure.Data.Contexts;
Expand Down Expand Up @@ -53,4 +54,9 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
optionsBuilder.EnableDetailedErrors();
#endif
}

public IExecutionStrategy CreateExecutionStrategy()
{
return Database.CreateExecutionStrategy();
}
}
Loading