Treat dispose of an active MongoTransaction as rollback

Author: damieng

src/MongoDB.EntityFrameworkCore/Storage/MongoTransaction.cs

@@ -15,7 +15,6 @@
 
 using System;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore;
@@ -91,7 +90,6 @@ internal static MongoTransaction Start(
 
         var transaction = new MongoTransaction(session, context, transactionId, transactionManager, transactionLogger);
         transactionLogger.TransactionStarted(transaction, async, startTime, stopwatch.Elapsed);
-
         return transaction;
     }
 
@@ -234,22 +232,49 @@ public TransactionOptions TransactionOptions
     /// <inheritdoc />
     public void Dispose()
     {
-        if (_transactionState == TransactionState.Disposed) return;
-
-        AssertCorrectState("Dispose", TransactionState.Committed, TransactionState.RolledBack, TransactionState.Failed);
-        _transactionState = TransactionState.Disposed;
-        session.Dispose();
+        switch (_transactionState)
+        {
+            case TransactionState.Disposed:
+                return;
+
+            case TransactionState.Active:
+                Rollback();
+                return;
+
+            case TransactionState.Committed:
+            case TransactionState.RolledBack:
+            case TransactionState.Failed:
+                _transactionState = TransactionState.Disposed;
+                session.Dispose();
+                return;
+
+            default:
+                throw new InvalidOperationException($"Can not Dispose MongoTransaction {TransactionId} because it is {_transactionState}.");
+        }
     }
 
     /// <inheritdoc />
-    public ValueTask DisposeAsync()
+    public async ValueTask DisposeAsync()
     {
-        if (_transactionState == TransactionState.Disposed) return ValueTask.CompletedTask;
-
-        AssertCorrectState("Dispose", TransactionState.Committed, TransactionState.RolledBack, TransactionState.Failed);
-        _transactionState = TransactionState.Disposed;
-        session.Dispose();
-        return ValueTask.CompletedTask;
+        switch (_transactionState)
+        {
+            case TransactionState.Disposed:
+                return;
+
+            case TransactionState.Active:
+                await RollbackAsync();
+                return;
+
+            case TransactionState.Committed:
+            case TransactionState.RolledBack:
+            case TransactionState.Failed:
+                _transactionState = TransactionState.Disposed;
+                session.Dispose();
+                return;
+
+            default:
+                throw new InvalidOperationException($"Can not Dispose MongoTransaction {TransactionId} because it is {_transactionState}.");
+        }
     }
 
     private void AssertCorrectState(string action, TransactionState validState)
@@ -258,11 +283,4 @@ private void AssertCorrectState(string action, TransactionState validState)
             throw new
                 InvalidOperationException($"Can not {action} MongoTransaction {TransactionId} because it is {_transactionState}.");
     }
-
-    private void AssertCorrectState(string action, params TransactionState[] validStates)
-    {
-        if (!validStates.Contains(_transactionState))
-            throw new
-                InvalidOperationException($"Can not {action} MongoTransaction {TransactionId} because it is {_transactionState}.");
-    }
 }

tests/MongoDB.EntityFrameworkCore.FunctionalTests/Design/Generated/EF8/SimpleContextModelBuilder.cs

@@ -20,7 +20,7 @@ partial void Initialize()
             EveryTypeEntityType.CreateAnnotations(everyType);
             OwnedEntityEntityType.CreateAnnotations(ownedEntity);
 
-            AddAnnotation("ProductVersion", "8.0.20");
+            AddAnnotation("ProductVersion", "8.0.22");
         }
     }
 }

tests/MongoDB.EntityFrameworkCore.FunctionalTests/Storage/TransactionTests.cs

@@ -273,27 +273,6 @@ await db.Entities.AddRangeAsync(
         }
     }
 
-    [Fact]
-    public void Explicit_transaction_dispose_without_commit_or_rollback_throws_and_changes_not_visible()
-    {
-        var collection = database.CreateCollection<SimpleEntity>();
-
-        // Use `using` to force Dispose without commit/rollback
-        var ex = Assert.Throws<InvalidOperationException>(() =>
-        {
-            using var db = SingleEntityDbContext.Create(collection);
-            using var transaction = db.Database.BeginTransaction();
-            db.Entities.Add(new SimpleEntity { name = "pending" });
-            db.SaveChanges();
-            // Intentionally not committing or rolling back; disposing transaction should throw
-        });
-
-        Assert.Contains("Dispose", ex.Message);
-
-        using var dbCheck = SingleEntityDbContext.Create(collection);
-        Assert.Empty(dbCheck.Entities); // nothing committed
-    }
-
     [Fact]
     public void Ambient_TransactionScope_blocks_begin_explicit_transaction()
     {
@@ -526,39 +505,29 @@ public async Task Same_context_reads_its_own_writes_inside_explicit_transaction_
     }
 
     [Fact]
-    public void Disposing_explicit_transaction_without_commit_or_rollback_throws_and_does_not_commit()
+    public void Disposing_explicit_transaction_without_commit_or_rollback_does_not_commit()
     {
         var collection = database.CreateCollection<SimpleEntity>();
 
-        var ex = Assert.Throws<InvalidOperationException>(() =>
-        {
-            using var db = SingleEntityDbContext.Create(collection);
-            using var transaction = db.Database.BeginTransaction();
-            db.Entities.Add(new SimpleEntity { name = "pending" });
-            db.SaveChanges();
-            // Transaction goes out of scope without Commit/Rollback -> Dispose should throw
-        });
+        using var db = SingleEntityDbContext.Create(collection);
+        using var transaction = db.Database.BeginTransaction();
+        db.Entities.Add(new SimpleEntity { name = "pending" });
+        db.SaveChanges();
 
-        Assert.Contains("Dispose", ex.Message);
         using var check = SingleEntityDbContext.Create(collection);
         Assert.Empty(check.Entities);
     }
 
     [Fact]
-    public async Task Disposing_explicit_transaction_without_commit_or_rollback_throws_and_does_not_commit_async()
+    public async Task Disposing_explicit_transaction_without_commit_or_rollback_does_not_commit_async()
     {
         var collection = database.CreateCollection<SimpleEntity>();
 
-        var ex = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
-        {
-            await using var db = SingleEntityDbContext.Create(collection);
-            await using var transaction = await db.Database.BeginTransactionAsync();
-            await db.Entities.AddAsync(new SimpleEntity { name = "pending" });
-            await db.SaveChangesAsync();
-            // Transaction goes out of scope without Commit/Rollback -> DisposeAsync should throw
-        });
+        await using var db = SingleEntityDbContext.Create(collection);
+        await using var transaction = await db.Database.BeginTransactionAsync();
+        await db.Entities.AddAsync(new SimpleEntity { name = "pending" });
+        await db.SaveChangesAsync();
 
-        Assert.Contains("Dispose", ex.Message);
         await using var check = SingleEntityDbContext.Create(collection);
         Assert.Empty(await check.Entities.ToListAsync());
     }
@@ -667,4 +636,68 @@ public void Can_start_new_explicit_transaction_on_same_context_after_rollback()
         var names = check.Entities.Select(e => e.name).OrderBy(n => n).ToList();
         Assert.Equal(["transaction2-committed"], names);
     }
+
+    [Theory]
+    [InlineData(false)]
+//    [InlineData(true)]
+    public void Can_execute_transasction_via_execution_strategy(bool succeeds)
+    {
+        var collection = database.CreateCollection<SimpleEntity>(values: [succeeds]);
+
+        var db = SingleEntityDbContext.Create(collection);
+        var strategy = db.Database.CreateExecutionStrategy();
+        var expectedCount = succeeds ? 2 : 0;
+
+        try
+        {
+            strategy.ExecuteInTransaction(
+                db,
+                operation: context =>
+                {
+                    Assert.NotNull(context.Entities);
+                    context.Entities.Add(new SimpleEntity { name = "First" });
+                    context.Entities.Add(new SimpleEntity { name = "Second" });
+                    context.SaveChanges();
+                    if (!succeeds) throw new Exception("Expected failure");
+                },
+                verifySucceeded: _ => true);
+        }
+        catch (Exception ex) when (ex.Message == "Expected failure")
+        {
+        }
+
+        Assert.Equal(expectedCount, db.Entities.Count());
+    }
+
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task Can_execute_transasction_via_execution_strategy_async(bool succeeds)
+    {
+        var collection = database.CreateCollection<SimpleEntity>(values: [succeeds]);
+
+        var db = SingleEntityDbContext.Create(collection);
+        var strategy = db.Database.CreateExecutionStrategy();
+        var expectedCount = succeeds ? 2 : 0;
+
+        try
+        {
+            await strategy.ExecuteInTransactionAsync(
+                db,
+                operation: async (context, cancellationToken) =>
+                {
+                    Assert.NotNull(context.Entities);
+                    context.Entities.Add(new SimpleEntity { name = "First" });
+                    context.Entities.Add(new SimpleEntity { name = "Second" });
+                    await context.SaveChangesAsync(cancellationToken);
+                    if (!succeeds) throw new Exception("Expected failure");
+                },
+                verifySucceeded: (_, _) => Task.FromResult(true));
+        }
+        catch (Exception ex) when (ex.Message == "Expected failure")
+        {
+        }
+
+        Assert.Equal(expectedCount, db.Entities.Count());
+    }
 }