diff --git a/src/NHibernate.Test/Async/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs b/src/NHibernate.Test/Async/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs index a5e9222fd42..2432a8705c1 100644 --- a/src/NHibernate.Test/Async/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs +++ b/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(); diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH3325/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH3325/Fixture.cs new file mode 100644 index 00000000000..da325b51328 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH3325/Fixture.cs @@ -0,0 +1,121 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +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(parentId, cancellationToken)); + Assert.That(parent, Is.Not.Null); + Assert.That(parent.Children, Has.Count.EqualTo(0)); + + var child = await (session.GetAsync(childId, cancellationToken)); + Assert.That(child, Is.Null); + } + } +} diff --git a/src/NHibernate.Test/Async/ReadOnly/ReadOnlyVersionedNodes.cs b/src/NHibernate.Test/Async/ReadOnly/ReadOnlyVersionedNodes.cs index def3e860c51..2adb9224758 100644 --- a/src/NHibernate.Test/Async/ReadOnly/ReadOnlyVersionedNodes.cs +++ b/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)); diff --git a/src/NHibernate.Test/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs b/src/NHibernate.Test/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs index 1e77060ab44..be22e24c08a 100644 --- a/src/NHibernate.Test/Immutable/EntityWithMutableCollection/AbstractEntityWithManyToManyTest.cs +++ b/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(); diff --git a/src/NHibernate.Test/NHSpecificTest/GH3325/Entity.cs b/src/NHibernate.Test/NHSpecificTest/GH3325/Entity.cs new file mode 100644 index 00000000000..e63e6c3819b --- /dev/null +++ b/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 Children { get; set; } = new HashSet(); + } + + public class EntityWithoutDeleteOrphan + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + public virtual ISet Children { get; set; } = new HashSet(); + } + + public class ChildEntity + { + public virtual Guid Id { get; set; } + public virtual string Name { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH3325/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH3325/Fixture.cs new file mode 100644 index 00000000000..a0ba4123dac --- /dev/null +++ b/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(parentId); + Assert.That(parent, Is.Not.Null); + Assert.That(parent.Children, Has.Count.EqualTo(0)); + + var child = session.Get(childId); + Assert.That(child, Is.Null); + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH3325/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH3325/Mappings.hbm.xml new file mode 100644 index 00000000000..712bf46fb1f --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH3325/Mappings.hbm.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/NHibernate.Test/ReadOnly/ReadOnlyVersionedNodes.cs b/src/NHibernate.Test/ReadOnly/ReadOnlyVersionedNodes.cs index 6ef31d2ae23..0773dd3e83e 100644 --- a/src/NHibernate.Test/ReadOnly/ReadOnlyVersionedNodes.cs +++ b/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); diff --git a/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs b/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs index f86e9a6c158..0d7213cc0ad 100644 --- a/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs +++ b/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs @@ -210,7 +210,7 @@ protected virtual async Task 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) diff --git a/src/NHibernate/Event/Default/AbstractSaveEventListener.cs b/src/NHibernate/Event/Default/AbstractSaveEventListener.cs index 8c9f356304f..cbec2218a22 100644 --- a/src/NHibernate/Event/Default/AbstractSaveEventListener.cs +++ b/src/NHibernate/Event/Default/AbstractSaveEventListener.cs @@ -235,7 +235,7 @@ protected virtual object PerformSaveOrReplicate(object entity, EntityKey key, IE if (persister.HasCollections) { - substitute = substitute || VisitCollectionsBeforeSave(entity, id, values, types, source); + substitute = VisitCollectionsBeforeSave(entity, id, values, types, source) || substitute; } if (substitute)