Skip to content

Commit 061c0b6

Browse files
fredericDelaportefredericDelaporte
committed
Avoid initializing proxies when ordering inserts
Fix #1338
1 parent 65750bd commit 061c0b6

File tree

7 files changed

+160
-6
lines changed

7 files changed

+160
-6
lines changed

‎src/NHibernate.Test/Async/Insertordering/AnimalModel/Fixture.cs‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,66 @@ public async System.Threading.Tasks.Task ElaboratedModelAsync()
102102
Assert.DoesNotThrowAsync(() => { return tran.CommitAsync(); });
103103
}
104104
}
105+
106+
// #1338
107+
[Test]
108+
public async System.Threading.Tasks.Task InsertShouldNotInitializeManyToOneProxyAsync()
109+
{
110+
var person = new Person { Name = "AnimalOwner" };
111+
using (var s = OpenSession())
112+
using (var t = s.BeginTransaction())
113+
{
114+
await (s.SaveAsync(person));
115+
await (t.CommitAsync());
116+
}
117+
await (Sfi.EvictAsync(typeof(Person)));
118+
119+
using (var s = OpenSession())
120+
using (var t = s.BeginTransaction())
121+
{
122+
var personProxy = await (s.LoadAsync<Person>(person.Id));
123+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy already initialized after load");
124+
125+
await (s.SaveAsync(new Cat { Name = "Felix", Owner = personProxy }));
126+
await (s.SaveAsync(new Cat { Name = "Loustic", Owner = personProxy }));
127+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy initialized after saves");
128+
await (t.CommitAsync());
129+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy initialized after commit");
130+
}
131+
}
132+
133+
[Test]
134+
public async System.Threading.Tasks.Task InsertShouldNotInitializeOneToManyProxyAsync()
135+
{
136+
var cat = new Cat { Name = "Felix" };
137+
using (var s = OpenSession())
138+
using (var t = s.BeginTransaction())
139+
{
140+
await (s.SaveAsync(cat));
141+
await (t.CommitAsync());
142+
}
143+
await (Sfi.EvictAsync(typeof(Cat)));
144+
145+
using (var s = OpenSession())
146+
using (var t = s.BeginTransaction())
147+
{
148+
var catProxy = await (s.LoadAsync<Cat>(cat.Id));
149+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy already initialized after load");
150+
151+
var owner = new Person { Name = "AnimalOwner" };
152+
owner.AnimalsGeneric.Add(catProxy);
153+
// Following assert would fail if the collection was changed for a set.
154+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after collection add");
155+
await (s.SaveAsync(owner));
156+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after save");
157+
await (t.CommitAsync());
158+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after commit");
159+
// The collection being inverse, the cat owner is not actually set in this test, but that is enough
160+
// to check the trouble. The ordering logic does not short-circuit on inverse collections. (It could
161+
// be an optimization, but it may cause regressions for some edge case mappings, like one having an
162+
// inverse one-to-many with no matching many-to-one but a basic type property for the foreign key
163+
// instead.)
164+
}
165+
}
105166
}
106-
}
167+
}

‎src/NHibernate.Test/Insertordering/AnimalModel/Animal.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,15 @@ public class Animal
77
public virtual Guid Id { get; set; }
88
public virtual string Name { get; set; }
99
public virtual Person Owner { get; set; }
10+
11+
public override bool Equals(object obj)
12+
{
13+
return (obj as Animal)?.Id == Id;
14+
}
15+
16+
public override int GetHashCode()
17+
{
18+
return Id.GetHashCode();
19+
}
1020
}
1121
}

‎src/NHibernate.Test/Insertordering/AnimalModel/Fixture.cs‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,66 @@ public void ElaboratedModel()
9292
Assert.DoesNotThrow(() => { tran.Commit(); });
9393
}
9494
}
95+
96+
// #1338
97+
[Test]
98+
public void InsertShouldNotInitializeManyToOneProxy()
99+
{
100+
var person = new Person { Name = "AnimalOwner" };
101+
using (var s = OpenSession())
102+
using (var t = s.BeginTransaction())
103+
{
104+
s.Save(person);
105+
t.Commit();
106+
}
107+
Sfi.Evict(typeof(Person));
108+
109+
using (var s = OpenSession())
110+
using (var t = s.BeginTransaction())
111+
{
112+
var personProxy = s.Load<Person>(person.Id);
113+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy already initialized after load");
114+
115+
s.Save(new Cat { Name = "Felix", Owner = personProxy });
116+
s.Save(new Cat { Name = "Loustic", Owner = personProxy });
117+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy initialized after saves");
118+
t.Commit();
119+
Assert.That(NHibernateUtil.IsInitialized(personProxy), Is.False, "Person proxy initialized after commit");
120+
}
121+
}
122+
123+
[Test]
124+
public void InsertShouldNotInitializeOneToManyProxy()
125+
{
126+
var cat = new Cat { Name = "Felix" };
127+
using (var s = OpenSession())
128+
using (var t = s.BeginTransaction())
129+
{
130+
s.Save(cat);
131+
t.Commit();
132+
}
133+
Sfi.Evict(typeof(Cat));
134+
135+
using (var s = OpenSession())
136+
using (var t = s.BeginTransaction())
137+
{
138+
var catProxy = s.Load<Cat>(cat.Id);
139+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy already initialized after load");
140+
141+
var owner = new Person { Name = "AnimalOwner" };
142+
owner.AnimalsGeneric.Add(catProxy);
143+
// Following assert would fail if the collection was changed for a set.
144+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after collection add");
145+
s.Save(owner);
146+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after save");
147+
t.Commit();
148+
Assert.That(NHibernateUtil.IsInitialized(catProxy), Is.False, "Cat proxy initialized after commit");
149+
// The collection being inverse, the cat owner is not actually set in this test, but that is enough
150+
// to check the trouble. The ordering logic does not short-circuit on inverse collections. (It could
151+
// be an optimization, but it may cause regressions for some edge case mappings, like one having an
152+
// inverse one-to-many with no matching many-to-one but a basic type property for the foreign key
153+
// instead.)
154+
}
155+
}
95156
}
96-
}
157+
}

‎src/NHibernate.Test/Insertordering/AnimalModel/Person.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,15 @@ public class Person
88
public virtual Guid Id { get; set; }
99
public virtual string Name { get; set; }
1010
public virtual IList<Animal> AnimalsGeneric { get; set; } = new List<Animal>();
11+
12+
public override bool Equals(object obj)
13+
{
14+
return (obj as Person)?.Id == Id;
15+
}
16+
17+
public override int GetHashCode()
18+
{
19+
return Id.GetHashCode();
20+
}
1121
}
1222
}

‎src/NHibernate/Async/Action/IExecutable.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//------------------------------------------------------------------------------
99

1010

11+
using System;
1112
using NHibernate.Engine;
1213

1314
namespace NHibernate.Action

‎src/NHibernate/Async/Engine/ActionQueue.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,6 @@ public async Task AfterTransactionCompletionAsync(bool success, CancellationToke
240240
processes.Clear();
241241
}
242242
}
243-
244243
private partial class BeforeTransactionCompletionDelegatedProcess : IBeforeTransactionCompletionProcess
245244
{
246245

@@ -261,7 +260,6 @@ public Task ExecuteBeforeTransactionCompletionAsync(CancellationToken cancellati
261260
}
262261
}
263262
}
264-
265263
private partial class AfterTransactionCompletionDelegatedProcess : IAfterTransactionCompletionProcess
266264
{
267265

‎src/NHibernate/Engine/ActionQueue.cs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,12 @@ private bool RequireNewBatch(AbstractEntityInsertAction action, int latestBatchN
701701
var value = propertyValues[i];
702702
var type = propertyTypes[i];
703703

704-
if (type.IsEntityType && value != null)
704+
if (type.IsEntityType && value != null &&
705+
// If the value is not initialized, it is a proxy with pending load from database,
706+
// so it can only be an already persisted entity. It can not have its own pending
707+
// insertion batch. So there is no need to seek for it, and it avoids initializing
708+
// it by searching it in a dictionary. Fixes #1338.
709+
NHibernateUtil.IsInitialized(value))
705710
{
706711
// find the batch number associated with the current association, if any.
707712
int associationBatchNumber;
@@ -761,8 +766,16 @@ private void UpdateChildrenDependencies(int batchNumber, AbstractEntityInsertAct
761766

762767
foreach(var child in children)
763768
{
764-
if (child == null)
769+
if (child == null ||
770+
// If the child is not initialized, it is a proxy with pending load from database,
771+
// so it can only be an already persisted entity. It can not have its own pending
772+
// insertion batch. So we do not need to keep track of the highest other batch on
773+
// which it depends. And this avoids initializing the proxy by searching it into
774+
// a dictionary.
775+
!NHibernateUtil.IsInitialized(child))
776+
{
765777
continue;
778+
}
766779

767780
int latestDependency;
768781
if (_entityBatchDependency.TryGetValue(child, out latestDependency) && latestDependency > batchNumber)

0 commit comments

Comments
 (0)