simplesoft-pt / Mediator

Small .NET library that helps with the implementation of mediator pattern for commands, events and queries
MIT License
41 stars 9 forks source link

Adding `Rollback` to Pipeline #26

Closed onurkanbakirci closed 1 month ago

onurkanbakirci commented 1 month ago

Why don't we encapsulate OnCommandAsync() methods as follows to roll back changes in case of errors? If it's suitable, i can open PR

...
         #if NETSTANDARD2_1 
                     await using var tx = await _context.Database.BeginTransactionAsync(ct).ConfigureAwait(false); 
         #else 
                     using var tx = await _context.Database.BeginTransactionAsync(ct).ConfigureAwait(false); 
         #endif 
           try
           {
                await next(cmd, ct).ConfigureAwait(false); 

                _logger.LogDebug("Saving changes into the database"); 

                await _context.SaveChangesAsync(ct).ConfigureAwait(false); 

                #if NETSTANDARD2_1 
                         await tx.CommitAsync(ct).ConfigureAwait(false); 
                #else 
                         tx.Commit(); 
                #endif 
            }
            catch
            {
                tx.Rollback(); //added
            }
...

https://github.com/simplesoft-pt/Mediator/blob/88d8ced13eda00872512e3f18b7b8653d4b18e23/src/SimpleSoft.Mediator.Microsoft.Extensions.EFCoreTransactionPipeline/EFCoreTransactionPipeline.cs#L36-L98

gravity00 commented 1 month ago

By design, any transaction that has begun will rollback when disposed if no explicit commit has been called.

This is exactly the behavior that will happen if an exception is thrown from an inner pipeline or the command handler.

I don't see a reason why rollback should be explicitly called. Do you have a use case in mind?

joaopllopes commented 1 month ago

I'm guessing an explicit rollback could come handy when using this on a single-threaded scenario, such as a console application, where the DbContext might be a singleton, for whatever reason.

If we're going through with something like this, please don't forget throw;.

gravity00 commented 1 month ago

@joaopllopes even in that scenario, the transaction is only open inside this pipeline, being disposed at the end.