making IDocumentSession.Delete() operations purge pending operations …

…involving that same document. Closes GH-2660
JasperFx · Nov 27, 2023 · b6efdc2 · b6efdc2
1 parent 3c0520a
commit b6efdc2
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 0 deletions.
diff --git a/src/DocumentDbTests/Bugs/Bug_2660_deletes_in_complex_order.cs b/src/DocumentDbTests/Bugs/Bug_2660_deletes_in_complex_order.cs
@@ -0,0 +1,79 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Marten;
+using Marten.Testing.Harness;
+using Xunit;
+using Shouldly;
+
+namespace DocumentDbTests.Bugs;
+
+public class Bug_2660_deletes_in_complex_order : BugIntegrationContext
+{
+        // works
+        [Fact]
+        public async Task Deleting_Single_DocType_In_One_Session_Works()
+        {
+            // store & delete within same session
+            await using var session = theStore.IdentitySession();
+            var id = Guid.NewGuid();
+            session.Store(new FooModel() { Id = id });
+            session.Delete<FooModel>(id);
+            await session.SaveChangesAsync();
+
+            await using var session2 = theStore.IdentitySession();
+            var model = await session.LoadAsync<FooModel>(id);
+            model.ShouldBeNull();
+        }
+
+        // fails
+        [Fact]
+        public async Task Deleting_Multiple_DocTypes_In_One_Session_Works()
+        {
+            // store & delete within same session
+            await using var session = theStore.IdentitySession();
+            var id = Guid.NewGuid();
+            session.Store(new FooModel() { Id = id });
+            session.Store(new BarModel() { Id = id });
+            session.Delete<FooModel>(id);
+            session.Delete<BarModel>(id);
+
+            session.PendingChanges.Operations().Count().ShouldBe(2);
+            session.PendingChanges.Deletions().Count().ShouldBe(2);
+
+            await session.SaveChangesAsync();
+
+            await using var session2 = theStore.IdentitySession();
+            var model = await session.LoadAsync<FooModel>(id);
+            model.ShouldBeNull();
+        }
+
+        // also fails
+        [Fact]
+        public async Task Delete_Where_Within_Same_Session_Doesnt_Affect_Actual_Delete()
+        {
+            // store & delete in the same session
+            using var session = theStore.IdentitySession();
+            var id = Guid.NewGuid();
+            session.Store(new FooModel() { Id = id });
+            session.Store(new BarModel() { Id = Guid.NewGuid(), RelGuid = id });
+            session.Delete<FooModel>(id);
+            session.DeleteWhere<BarModel>(x => x.RelGuid == id);
+            await session.SaveChangesAsync();
+
+            await using var session2 = theStore.IdentitySession();
+            var model = await session.LoadAsync<FooModel>(id);
+            model.ShouldBeNull();
+        }
+}
+
+public class FooModel
+{
+    public Guid Id { get; set; }
+}
+
+public class BarModel
+{
+    public Guid Id { get; set; }
+    public Guid RelGuid { get; set; }
+}
diff --git a/src/Marten/Events/Daemon/ProjectionUpdateBatch.cs b/src/Marten/Events/Daemon/ProjectionUpdateBatch.cs
@@ -189,6 +189,11 @@ public void EjectAll()
         throw new NotSupportedException();
     }
 
+    public void PurgeOperations<T, TId>(TId id) where T : notnull
+    {
+        // Do nothing here
+    }
+
     void IUpdateBatch.ApplyChanges(IMartenSession session)
     {
         if (_token.IsCancellationRequested)

diff --git a/src/Marten/Internal/Operations/StorageOperation.cs b/src/Marten/Internal/Operations/StorageOperation.cs
@@ -33,6 +33,8 @@ public StorageOperation(T document, TId id, Dictionary<TId, Guid> versions, Docu
         _tableName = mapping.TableName.Name;
     }
 
+    public TId Id => _id;
+
     public object Document => _document;
 
     public IChangeTracker ToTracker(IMartenSession session)

diff --git a/src/Marten/Internal/Sessions/DocumentSessionBase.Deletes.cs b/src/Marten/Internal/Sessions/DocumentSessionBase.Deletes.cs
@@ -17,6 +17,11 @@ public void Delete<T>(T entity) where T : notnull
         assertNotDisposed();
         var documentStorage = StorageFor<T>();
 
+        if (documentStorage is IDocumentStorage<T, Guid> g) _workTracker.PurgeOperations<T, Guid>(g.Identity(entity));
+        if (documentStorage is IDocumentStorage<T, string> s) _workTracker.PurgeOperations<T, string>(s.Identity(entity));
+        if (documentStorage is IDocumentStorage<T, int> i) _workTracker.PurgeOperations<T, int>(i.Identity(entity));
+        if (documentStorage is IDocumentStorage<T, long> l) _workTracker.PurgeOperations<T, long>(l.Identity(entity));
+
         var deletion = documentStorage.DeleteForDocument(entity, TenantId);
         _workTracker.Add(deletion);
 
@@ -33,12 +38,14 @@ public void Delete<T>(int id) where T : notnull
         {
             _workTracker.Add(i.DeleteForId(id, TenantId));
 
+            _workTracker.PurgeOperations<T, int>(id);
             ejectById<T>(id);
         }
         else if (storage is IDocumentStorage<T, long> l)
         {
             _workTracker.Add(l.DeleteForId(id, TenantId));
 
+            _workTracker.PurgeOperations<T, long>(id);
             ejectById<T>((long)id);
         }
         else
@@ -53,13 +60,15 @@ public void Delete<T>(long id) where T : notnull
         var deletion = StorageFor<T, long>().DeleteForId(id, TenantId);
         _workTracker.Add(deletion);
 
+        _workTracker.PurgeOperations<T, long>(id);
         ejectById<T>(id);
     }
 
     public void Delete<T>(Guid id) where T : notnull
     {
         assertNotDisposed();
         var deletion = StorageFor<T, Guid>().DeleteForId(id, TenantId);
+        _workTracker.PurgeOperations<T, Guid>(id);
         _workTracker.Add(deletion);
 
         ejectById<T>(id);
@@ -72,6 +81,7 @@ public void Delete<T>(string id) where T : notnull
         var deletion = StorageFor<T, string>().DeleteForId(id, TenantId);
         _workTracker.Add(deletion);
 
+        _workTracker.PurgeOperations<T, string>(id);
         ejectById<T>(id);
     }
 

diff --git a/src/Marten/Internal/UnitOfWork.cs b/src/Marten/Internal/UnitOfWork.cs
@@ -210,6 +210,11 @@ public void EjectAll()
         Streams.Clear();
     }
 
+    public void PurgeOperations<T, TId>(TId id) where T : notnull
+    {
+        _operations.RemoveAll(op => op is StorageOperation<T, TId> storage && storage.Id.Equals(id));
+    }
+
     public bool TryFindStream(string streamKey, out StreamAction stream)
     {
         stream = Streams

diff --git a/src/Marten/Services/ISessionWorkTracker.cs b/src/Marten/Services/ISessionWorkTracker.cs
@@ -18,4 +18,12 @@ internal interface ISessionWorkTracker: IUnitOfWork, IChangeSet
     bool TryFindStream(Guid streamId, out StreamAction stream);
     bool HasOutstandingWork();
     void EjectAll();
+
+    /// <summary>
+    /// Remove all outstanding operations for the designated document
+    /// </summary>
+    /// <param name="id"></param>
+    /// <typeparam name="T"></typeparam>
+    /// <typeparam name="TId"></typeparam>
+    void PurgeOperations<T, TId>(TId id) where T : notnull;
 }