Fixes cascading orphan delete on versioned entity

src/NHibernate.Test/Async/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs

@@ -496,7 +496,7 @@ public async Task CreateWithNonEmptyManyToManyCollectionMergeWithNewElementAsync
 			s.Close();
 
 			AssertInsertCount(1);
-			AssertUpdateCount(isContractVersioned && isPlanVersioned ? 1 : 0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(isContractVersioned && isPlanVersioned ? 2 : 0);
 			ClearCounts();
 
 			s = OpenSession();

src/NHibernate.Test/Async/NHSpecificTest/GH3325/Fixture.cs

@@ -0,0 +1,121 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+using System;
+using System.Collections.Generic;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH3325
+{
+	using System.Threading.Tasks;
+	using System.Threading;
+	[TestFixture]
+	public class FixtureAsync : BugTestCase
+	{
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public async Task CanRemoveChildAfterSaveAsync()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			parent.Children.Add(child);
+			await (session.SaveAsync(parent));
+			parent.Children.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			await (t.CommitAsync());
+
+			await (AssertParentIsChildlessAsync(parentId, childId));
+		}
+
+		[Test]
+		public async Task CanRemoveChildFromUnwrappedCollectionAfterSaveAsync()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			var parentChildren = parent.Children;
+			parentChildren.Add(child);
+			await (session.SaveAsync(parent));
+			parentChildren.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			await (t.CommitAsync());
+
+			await (AssertParentIsChildlessAsync(parentId, childId));
+		}
+
+		[Test]
+		public async Task CanRemoveChildAfterSaveAndExplicitFlushAsync()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			parent.Children.Add(child);
+			await (session.SaveAsync(parent));
+			await (session.FlushAsync());
+			parent.Children.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			await (t.CommitAsync());
+
+			await (AssertParentIsChildlessAsync(parentId, childId));
+		}
+
+		[Test]
+		public async Task CanRemoveChildFromUnwrappedCollectionAfterSaveAndExplicitFlushAsync()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			var parentChildren = parent.Children;
+			parentChildren.Add(child);
+			await (session.SaveAsync(parent));
+			await (session.FlushAsync());
+			parentChildren.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			await (t.CommitAsync());
+
+			await (AssertParentIsChildlessAsync(parentId, childId));
+		}
+
+		private async Task AssertParentIsChildlessAsync(Guid parentId, Guid childId, CancellationToken cancellationToken = default(CancellationToken))
+		{
+			using var session = OpenSession();
+
+			var parent = await (session.GetAsync<Entity>(parentId, cancellationToken));
+			Assert.That(parent, Is.Not.Null);
+			Assert.That(parent.Children, Has.Count.EqualTo(0));
+
+			var child = await (session.GetAsync<ChildEntity>(childId, cancellationToken));
+			Assert.That(child, Is.Null);
+		}
+	}
+}

src/NHibernate.Test/Async/ReadOnly/ReadOnlyVersionedNodes.cs

@@ -558,7 +558,7 @@ public async Task MergeDetachedChildWithNewParentCommitWithReadOnlyChildAsync()
 				await (t.CommitAsync());
 			}
 
-			AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(1);
 			AssertInsertCount(1);
 			ClearCounts();
 			using (var s = OpenSession())
@@ -571,7 +571,7 @@ public async Task MergeDetachedChildWithNewParentCommitWithReadOnlyChildAsync()
 				Assert.That(child.Version, Is.EqualTo(1));
 				Assert.That(parent, Is.Not.Null);
 				Assert.That(parent.Children.Count, Is.EqualTo(0));
-				Assert.That(parent.Version, Is.EqualTo(1));
+				Assert.That(parent.Version, Is.EqualTo(2));
 				s.SetReadOnly(parent, true);
 				s.SetReadOnly(child, true);
 				await (s.DeleteAsync(parent));
@@ -609,7 +609,7 @@ public async Task GetChildMakeReadOnlyThenMergeDetachedChildWithNewParentAsync()
 				await (t.CommitAsync());
 			}
 
-			AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(1);
 			AssertInsertCount(1);
 			ClearCounts();
 			using (var s = OpenSession())
@@ -622,8 +622,7 @@ public async Task GetChildMakeReadOnlyThenMergeDetachedChildWithNewParentAsync()
 				Assert.That(child.Version, Is.EqualTo(1));
 				Assert.That(parent, Is.Not.Null);
 				Assert.That(parent.Children.Count, Is.EqualTo(0));
-				Assert.That(parent.Version, Is.EqualTo(1));
-					// NH-specific: Hibernate incorrectly increments version number, NH does not
+				Assert.That(parent.Version, Is.EqualTo(2));
 				s.SetReadOnly(parent, true);
 				s.SetReadOnly(child, true);
 				await (s.DeleteAsync(parent));

src/NHibernate.Test/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs

@@ -485,7 +485,7 @@ public void CreateWithNonEmptyManyToManyCollectionMergeWithNewElement()
 			s.Close();
 
 			AssertInsertCount(1);
-			AssertUpdateCount(isContractVersioned && isPlanVersioned ? 1 : 0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(isContractVersioned && isPlanVersioned ? 2 : 0);
 			ClearCounts();
 
 			s = OpenSession();

src/NHibernate.Test/NHSpecificTest/GH3325/Entity.cs

@@ -0,0 +1,26 @@
+using System;
+using System.Collections.Generic;
+
+namespace NHibernate.Test.NHSpecificTest.GH3325
+{
+	public class Entity
+	{
+		public virtual Guid Id { get; set; }
+		public virtual int Version { get; set; } = -1;
+		public virtual string Name { get; set; }
+		public virtual ISet<ChildEntity> Children { get; set; } = new HashSet<ChildEntity>();
+	}
+
+	public class EntityWithoutDeleteOrphan
+	{
+		public virtual Guid Id { get; set; }
+		public virtual string Name { get; set; }
+		public virtual ISet<ChildEntity> Children { get; set; } = new HashSet<ChildEntity>();
+	}
+
+	public class ChildEntity
+	{
+		public virtual Guid Id { get; set; }
+		public virtual string Name { get; set; }
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3325/Fixture.cs

@@ -0,0 +1,109 @@
+using System;
+using System.Collections.Generic;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.GH3325
+{
+	[TestFixture]
+	public class Fixture : BugTestCase
+	{
+		protected override void OnTearDown()
+		{
+			using var session = OpenSession();
+			using var transaction = session.BeginTransaction();
+
+			session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
+			session.CreateQuery("delete from System.Object").ExecuteUpdate();
+
+			transaction.Commit();
+		}
+
+		[Test]
+		public void CanRemoveChildAfterSave()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			parent.Children.Add(child);
+			session.Save(parent);
+			parent.Children.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			t.Commit();
+
+			AssertParentIsChildless(parentId, childId);
+		}
+
+		[Test]
+		public void CanRemoveChildFromUnwrappedCollectionAfterSave()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			var parentChildren = parent.Children;
+			parentChildren.Add(child);
+			session.Save(parent);
+			parentChildren.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			t.Commit();
+
+			AssertParentIsChildless(parentId, childId);
+		}
+
+		[Test]
+		public void CanRemoveChildAfterSaveAndExplicitFlush()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			parent.Children.Add(child);
+			session.Save(parent);
+			session.Flush();
+			parent.Children.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			t.Commit();
+
+			AssertParentIsChildless(parentId, childId);
+		}
+
+		[Test]
+		public void CanRemoveChildFromUnwrappedCollectionAfterSaveAndExplicitFlush()
+		{
+			using var session = OpenSession();
+			using var t = session.BeginTransaction();
+
+			var parent = new Entity { Name = "Parent" };
+			var child = new ChildEntity { Name = "Child" };
+			var parentChildren = parent.Children;
+			parentChildren.Add(child);
+			session.Save(parent);
+			session.Flush();
+			parentChildren.Remove(child);
+			var parentId = parent.Id;
+			var childId = child.Id;
+			t.Commit();
+
+			AssertParentIsChildless(parentId, childId);
+		}
+
+		private void AssertParentIsChildless(Guid parentId, Guid childId)
+		{
+			using var session = OpenSession();
+
+			var parent = session.Get<Entity>(parentId);
+			Assert.That(parent, Is.Not.Null);
+			Assert.That(parent.Children, Has.Count.EqualTo(0));
+
+			var child = session.Get<ChildEntity>(childId);
+			Assert.That(child, Is.Null);
+		}
+	}
+}

src/NHibernate.Test/NHSpecificTest/GH3325/Mappings.hbm.xml

@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
+                   namespace="NHibernate.Test.NHSpecificTest.GH3325">
+
+  <class name="Entity">
+    <id name="Id" generator="guid.comb"/>
+    <version name="Version" column="Version" type="Int32" unsaved-value="-1" />
+    <property name="Name"/>
+    <set name="Children" cascade="persist,delete,save-update,evict,lock,replicate,delete-orphan">
+      <key column="Parent" />
+      <one-to-many class="ChildEntity"/>
+    </set>
+  </class>
+
+  <class name="EntityWithoutDeleteOrphan">
+    <id name="Id" generator="guid.comb"/>
+    <property name="Name"/>
+    <set name="Children" cascade="persist,merge,save-update">
+      <key column="ParentWdo" />
+      <one-to-many class="ChildEntity"/>
+    </set>
+  </class>
+
+  <class name="ChildEntity">
+    <!-- Do not use a persisted on save generator for children. Otherwise flush cascade may
+         trigger parent insertion as a side effect, which tends to hide troubles. -->
+    <id name="Id" generator="guid.comb"/>
+    <property name="Name"/>
+  </class>
+
+</hibernate-mapping>

src/NHibernate.Test/ReadOnly/ReadOnlyVersionedNodes.cs

@@ -624,7 +624,7 @@ public void MergeDetachedChildWithNewParentCommitWithReadOnlyChild()
 				t.Commit();
 			}
 
-			AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(1);
 			AssertInsertCount(1);
 			ClearCounts();
 			using (var s = OpenSession())
@@ -637,7 +637,7 @@ public void MergeDetachedChildWithNewParentCommitWithReadOnlyChild()
 				Assert.That(child.Version, Is.EqualTo(1));
 				Assert.That(parent, Is.Not.Null);
 				Assert.That(parent.Children.Count, Is.EqualTo(0));
-				Assert.That(parent.Version, Is.EqualTo(1));
+				Assert.That(parent.Version, Is.EqualTo(2));
 				s.SetReadOnly(parent, true);
 				s.SetReadOnly(child, true);
 				s.Delete(parent);
@@ -675,7 +675,7 @@ public void GetChildMakeReadOnlyThenMergeDetachedChildWithNewParent()
 				t.Commit();
 			}
 
-			AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
+			AssertUpdateCount(1);
 			AssertInsertCount(1);
 			ClearCounts();
 			using (var s = OpenSession())
@@ -688,8 +688,7 @@ public void GetChildMakeReadOnlyThenMergeDetachedChildWithNewParent()
 				Assert.That(child.Version, Is.EqualTo(1));
 				Assert.That(parent, Is.Not.Null);
 				Assert.That(parent.Children.Count, Is.EqualTo(0));
-				Assert.That(parent.Version, Is.EqualTo(1));
-					// NH-specific: Hibernate incorrectly increments version number, NH does not
+				Assert.That(parent.Version, Is.EqualTo(2));
 				s.SetReadOnly(parent, true);
 				s.SetReadOnly(child, true);
 				s.Delete(parent);

src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs

@@ -210,7 +210,7 @@ protected virtual async Task<object> PerformSaveOrReplicateAsync(object entity,
 
 			if (persister.HasCollections)
 			{
-				substitute = substitute || await (VisitCollectionsBeforeSaveAsync(entity, id, values, types, source, cancellationToken)).ConfigureAwait(false);
+				substitute = await (VisitCollectionsBeforeSaveAsync(entity, id, values, types, source, cancellationToken)).ConfigureAwait(false) || substitute;
 			}
 
 			if (substitute)