Skip to content

Commit 743d93d

Browse files
csharper2010hazzikbahusoid
authored
Fixes cascading orphan delete on versioned entity (#3326)
VisitCollectionsBeforeSave must be called no matter if substitute is already true. On versioned entities this is always the case. Fixes #3325 Co-authored-by: Alex Zaytsev <hazzik@gmail.com> Co-authored-by: Roman Artiukhin <bahusdrive@gmail.com>
1 parent 1b13f03 commit 743d93d

File tree

10 files changed

+299
-14
lines changed

10 files changed

+299
-14
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ public async Task CreateWithNonEmptyManyToManyCollectionMergeWithNewElementAsync
496496
s.Close();
497497

498498
AssertInsertCount(1);
499-
AssertUpdateCount(isContractVersioned && isPlanVersioned ? 1 : 0); // NH-specific: Hibernate issues a separate UPDATE for the version number
499+
AssertUpdateCount(isContractVersioned && isPlanVersioned ? 2 : 0);
500500
ClearCounts();
501501

502502
s = OpenSession();

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System;
12+
using System.Collections.Generic;
13+
using NUnit.Framework;
14+
15+
namespace NHibernate.Test.NHSpecificTest.GH3325
16+
{
17+
using System.Threading.Tasks;
18+
using System.Threading;
19+
[TestFixture]
20+
public class FixtureAsync : BugTestCase
21+
{
22+
protected override void OnTearDown()
23+
{
24+
using var session = OpenSession();
25+
using var transaction = session.BeginTransaction();
26+
27+
session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
28+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
29+
30+
transaction.Commit();
31+
}
32+
33+
[Test]
34+
public async Task CanRemoveChildAfterSaveAsync()
35+
{
36+
using var session = OpenSession();
37+
using var t = session.BeginTransaction();
38+
39+
var parent = new Entity { Name = "Parent" };
40+
var child = new ChildEntity { Name = "Child" };
41+
parent.Children.Add(child);
42+
await (session.SaveAsync(parent));
43+
parent.Children.Remove(child);
44+
var parentId = parent.Id;
45+
var childId = child.Id;
46+
await (t.CommitAsync());
47+
48+
await (AssertParentIsChildlessAsync(parentId, childId));
49+
}
50+
51+
[Test]
52+
public async Task CanRemoveChildFromUnwrappedCollectionAfterSaveAsync()
53+
{
54+
using var session = OpenSession();
55+
using var t = session.BeginTransaction();
56+
57+
var parent = new Entity { Name = "Parent" };
58+
var child = new ChildEntity { Name = "Child" };
59+
var parentChildren = parent.Children;
60+
parentChildren.Add(child);
61+
await (session.SaveAsync(parent));
62+
parentChildren.Remove(child);
63+
var parentId = parent.Id;
64+
var childId = child.Id;
65+
await (t.CommitAsync());
66+
67+
await (AssertParentIsChildlessAsync(parentId, childId));
68+
}
69+
70+
[Test]
71+
public async Task CanRemoveChildAfterSaveAndExplicitFlushAsync()
72+
{
73+
using var session = OpenSession();
74+
using var t = session.BeginTransaction();
75+
76+
var parent = new Entity { Name = "Parent" };
77+
var child = new ChildEntity { Name = "Child" };
78+
parent.Children.Add(child);
79+
await (session.SaveAsync(parent));
80+
await (session.FlushAsync());
81+
parent.Children.Remove(child);
82+
var parentId = parent.Id;
83+
var childId = child.Id;
84+
await (t.CommitAsync());
85+
86+
await (AssertParentIsChildlessAsync(parentId, childId));
87+
}
88+
89+
[Test]
90+
public async Task CanRemoveChildFromUnwrappedCollectionAfterSaveAndExplicitFlushAsync()
91+
{
92+
using var session = OpenSession();
93+
using var t = session.BeginTransaction();
94+
95+
var parent = new Entity { Name = "Parent" };
96+
var child = new ChildEntity { Name = "Child" };
97+
var parentChildren = parent.Children;
98+
parentChildren.Add(child);
99+
await (session.SaveAsync(parent));
100+
await (session.FlushAsync());
101+
parentChildren.Remove(child);
102+
var parentId = parent.Id;
103+
var childId = child.Id;
104+
await (t.CommitAsync());
105+
106+
await (AssertParentIsChildlessAsync(parentId, childId));
107+
}
108+
109+
private async Task AssertParentIsChildlessAsync(Guid parentId, Guid childId, CancellationToken cancellationToken = default(CancellationToken))
110+
{
111+
using var session = OpenSession();
112+
113+
var parent = await (session.GetAsync<Entity>(parentId, cancellationToken));
114+
Assert.That(parent, Is.Not.Null);
115+
Assert.That(parent.Children, Has.Count.EqualTo(0));
116+
117+
var child = await (session.GetAsync<ChildEntity>(childId, cancellationToken));
118+
Assert.That(child, Is.Null);
119+
}
120+
}
121+
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ public async Task MergeDetachedChildWithNewParentCommitWithReadOnlyChildAsync()
558558
await (t.CommitAsync());
559559
}
560560

561-
AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
561+
AssertUpdateCount(1);
562562
AssertInsertCount(1);
563563
ClearCounts();
564564
using (var s = OpenSession())
@@ -571,7 +571,7 @@ public async Task MergeDetachedChildWithNewParentCommitWithReadOnlyChildAsync()
571571
Assert.That(child.Version, Is.EqualTo(1));
572572
Assert.That(parent, Is.Not.Null);
573573
Assert.That(parent.Children.Count, Is.EqualTo(0));
574-
Assert.That(parent.Version, Is.EqualTo(1));
574+
Assert.That(parent.Version, Is.EqualTo(2));
575575
s.SetReadOnly(parent, true);
576576
s.SetReadOnly(child, true);
577577
await (s.DeleteAsync(parent));
@@ -609,7 +609,7 @@ public async Task GetChildMakeReadOnlyThenMergeDetachedChildWithNewParentAsync()
609609
await (t.CommitAsync());
610610
}
611611

612-
AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
612+
AssertUpdateCount(1);
613613
AssertInsertCount(1);
614614
ClearCounts();
615615
using (var s = OpenSession())
@@ -622,8 +622,7 @@ public async Task GetChildMakeReadOnlyThenMergeDetachedChildWithNewParentAsync()
622622
Assert.That(child.Version, Is.EqualTo(1));
623623
Assert.That(parent, Is.Not.Null);
624624
Assert.That(parent.Children.Count, Is.EqualTo(0));
625-
Assert.That(parent.Version, Is.EqualTo(1));
626-
// NH-specific: Hibernate incorrectly increments version number, NH does not
625+
Assert.That(parent.Version, Is.EqualTo(2));
627626
s.SetReadOnly(parent, true);
628627
s.SetReadOnly(child, true);
629628
await (s.DeleteAsync(parent));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ public void CreateWithNonEmptyManyToManyCollectionMergeWithNewElement()
485485
s.Close();
486486

487487
AssertInsertCount(1);
488-
AssertUpdateCount(isContractVersioned && isPlanVersioned ? 1 : 0); // NH-specific: Hibernate issues a separate UPDATE for the version number
488+
AssertUpdateCount(isContractVersioned && isPlanVersioned ? 2 : 0);
489489
ClearCounts();
490490

491491
s = OpenSession();

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
namespace NHibernate.Test.NHSpecificTest.GH3325
5+
{
6+
public class Entity
7+
{
8+
public virtual Guid Id { get; set; }
9+
public virtual int Version { get; set; } = -1;
10+
public virtual string Name { get; set; }
11+
public virtual ISet<ChildEntity> Children { get; set; } = new HashSet<ChildEntity>();
12+
}
13+
14+
public class EntityWithoutDeleteOrphan
15+
{
16+
public virtual Guid Id { get; set; }
17+
public virtual string Name { get; set; }
18+
public virtual ISet<ChildEntity> Children { get; set; } = new HashSet<ChildEntity>();
19+
}
20+
21+
public class ChildEntity
22+
{
23+
public virtual Guid Id { get; set; }
24+
public virtual string Name { get; set; }
25+
}
26+
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using NUnit.Framework;
4+
5+
namespace NHibernate.Test.NHSpecificTest.GH3325
6+
{
7+
[TestFixture]
8+
public class Fixture : BugTestCase
9+
{
10+
protected override void OnTearDown()
11+
{
12+
using var session = OpenSession();
13+
using var transaction = session.BeginTransaction();
14+
15+
session.CreateQuery("delete from ChildEntity").ExecuteUpdate();
16+
session.CreateQuery("delete from System.Object").ExecuteUpdate();
17+
18+
transaction.Commit();
19+
}
20+
21+
[Test]
22+
public void CanRemoveChildAfterSave()
23+
{
24+
using var session = OpenSession();
25+
using var t = session.BeginTransaction();
26+
27+
var parent = new Entity { Name = "Parent" };
28+
var child = new ChildEntity { Name = "Child" };
29+
parent.Children.Add(child);
30+
session.Save(parent);
31+
parent.Children.Remove(child);
32+
var parentId = parent.Id;
33+
var childId = child.Id;
34+
t.Commit();
35+
36+
AssertParentIsChildless(parentId, childId);
37+
}
38+
39+
[Test]
40+
public void CanRemoveChildFromUnwrappedCollectionAfterSave()
41+
{
42+
using var session = OpenSession();
43+
using var t = session.BeginTransaction();
44+
45+
var parent = new Entity { Name = "Parent" };
46+
var child = new ChildEntity { Name = "Child" };
47+
var parentChildren = parent.Children;
48+
parentChildren.Add(child);
49+
session.Save(parent);
50+
parentChildren.Remove(child);
51+
var parentId = parent.Id;
52+
var childId = child.Id;
53+
t.Commit();
54+
55+
AssertParentIsChildless(parentId, childId);
56+
}
57+
58+
[Test]
59+
public void CanRemoveChildAfterSaveAndExplicitFlush()
60+
{
61+
using var session = OpenSession();
62+
using var t = session.BeginTransaction();
63+
64+
var parent = new Entity { Name = "Parent" };
65+
var child = new ChildEntity { Name = "Child" };
66+
parent.Children.Add(child);
67+
session.Save(parent);
68+
session.Flush();
69+
parent.Children.Remove(child);
70+
var parentId = parent.Id;
71+
var childId = child.Id;
72+
t.Commit();
73+
74+
AssertParentIsChildless(parentId, childId);
75+
}
76+
77+
[Test]
78+
public void CanRemoveChildFromUnwrappedCollectionAfterSaveAndExplicitFlush()
79+
{
80+
using var session = OpenSession();
81+
using var t = session.BeginTransaction();
82+
83+
var parent = new Entity { Name = "Parent" };
84+
var child = new ChildEntity { Name = "Child" };
85+
var parentChildren = parent.Children;
86+
parentChildren.Add(child);
87+
session.Save(parent);
88+
session.Flush();
89+
parentChildren.Remove(child);
90+
var parentId = parent.Id;
91+
var childId = child.Id;
92+
t.Commit();
93+
94+
AssertParentIsChildless(parentId, childId);
95+
}
96+
97+
private void AssertParentIsChildless(Guid parentId, Guid childId)
98+
{
99+
using var session = OpenSession();
100+
101+
var parent = session.Get<Entity>(parentId);
102+
Assert.That(parent, Is.Not.Null);
103+
Assert.That(parent.Children, Has.Count.EqualTo(0));
104+
105+
var child = session.Get<ChildEntity>(childId);
106+
Assert.That(child, Is.Null);
107+
}
108+
}
109+
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
3+
namespace="NHibernate.Test.NHSpecificTest.GH3325">
4+
5+
<class name="Entity">
6+
<id name="Id" generator="guid.comb"/>
7+
<version name="Version" column="Version" type="Int32" unsaved-value="-1" />
8+
<property name="Name"/>
9+
<set name="Children" cascade="persist,delete,save-update,evict,lock,replicate,delete-orphan">
10+
<key column="Parent" />
11+
<one-to-many class="ChildEntity"/>
12+
</set>
13+
</class>
14+
15+
<class name="EntityWithoutDeleteOrphan">
16+
<id name="Id" generator="guid.comb"/>
17+
<property name="Name"/>
18+
<set name="Children" cascade="persist,merge,save-update">
19+
<key column="ParentWdo" />
20+
<one-to-many class="ChildEntity"/>
21+
</set>
22+
</class>
23+
24+
<class name="ChildEntity">
25+
<!-- Do not use a persisted on save generator for children. Otherwise flush cascade may
26+
trigger parent insertion as a side effect, which tends to hide troubles. -->
27+
<id name="Id" generator="guid.comb"/>
28+
<property name="Name"/>
29+
</class>
30+
31+
</hibernate-mapping>

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ public void MergeDetachedChildWithNewParentCommitWithReadOnlyChild()
624624
t.Commit();
625625
}
626626

627-
AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
627+
AssertUpdateCount(1);
628628
AssertInsertCount(1);
629629
ClearCounts();
630630
using (var s = OpenSession())
@@ -637,7 +637,7 @@ public void MergeDetachedChildWithNewParentCommitWithReadOnlyChild()
637637
Assert.That(child.Version, Is.EqualTo(1));
638638
Assert.That(parent, Is.Not.Null);
639639
Assert.That(parent.Children.Count, Is.EqualTo(0));
640-
Assert.That(parent.Version, Is.EqualTo(1));
640+
Assert.That(parent.Version, Is.EqualTo(2));
641641
s.SetReadOnly(parent, true);
642642
s.SetReadOnly(child, true);
643643
s.Delete(parent);
@@ -675,7 +675,7 @@ public void GetChildMakeReadOnlyThenMergeDetachedChildWithNewParent()
675675
t.Commit();
676676
}
677677

678-
AssertUpdateCount(0); // NH-specific: Hibernate issues a separate UPDATE for the version number
678+
AssertUpdateCount(1);
679679
AssertInsertCount(1);
680680
ClearCounts();
681681
using (var s = OpenSession())
@@ -688,8 +688,7 @@ public void GetChildMakeReadOnlyThenMergeDetachedChildWithNewParent()
688688
Assert.That(child.Version, Is.EqualTo(1));
689689
Assert.That(parent, Is.Not.Null);
690690
Assert.That(parent.Children.Count, Is.EqualTo(0));
691-
Assert.That(parent.Version, Is.EqualTo(1));
692-
// NH-specific: Hibernate incorrectly increments version number, NH does not
691+
Assert.That(parent.Version, Is.EqualTo(2));
693692
s.SetReadOnly(parent, true);
694693
s.SetReadOnly(child, true);
695694
s.Delete(parent);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ protected virtual async Task<object> PerformSaveOrReplicateAsync(object entity,
210210

211211
if (persister.HasCollections)
212212
{
213-
substitute = substitute || await (VisitCollectionsBeforeSaveAsync(entity, id, values, types, source, cancellationToken)).ConfigureAwait(false);
213+
substitute = await (VisitCollectionsBeforeSaveAsync(entity, id, values, types, source, cancellationToken)).ConfigureAwait(false) || substitute;
214214
}
215215

216216
if (substitute)

0 commit comments

Comments
 (0)