Skip to content

Commit 3c8abeb

Browse files
fredericDelaportehazzik
fredericDelaporte
and
hazzik
committed
Fix cascade + one-to-one failing on null no-proxy association
This is a lightweight fix which does not change the no-proxy principles at the root of the bug. The no-proxy is currently implemented as a special case in the lazy property interceptor, not flagged as a lazy property. Instead, on parent entity load, it hides, in the parent loaded state for the no-proxy, a proxy (!) flagged for unwrapping the associated entity it proxifies. This hiden unwrapping proxy supports proxifying non-existent entities. The property interceptor does the unwrapping when accessing it. But when the proxy unwraps to null, this causes the loaded state to have a reference where it should be indeed null, causing dirty checking issues. A better fix would be to flag the property as lazy for no-proxy one-to-one associations, allowing to get in the loadedState the "uninitializedProperty" instance, allowing the dirty checking mechanism to know that this part of state is unknown (till loading it, at which point it should be adjusted to its actual value). But this causes the property interceptor to trigger initialization of any other lazy properties when accessing a one-to-one. Avoiding this would require to implement some lazy property fetch group semantic. Fixes #1719 Co-authored-by: Alexander Zaytsev <[email protected]>
1 parent 19480fa commit 3c8abeb

File tree

2 files changed

+64
-61
lines changed

2 files changed

+64
-61
lines changed

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

Lines changed: 56 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using NHibernate.Event;
1515
using NHibernate.Persister.Collection;
1616
using NHibernate.Persister.Entity;
17+
using NHibernate.Proxy;
1718
using NHibernate.Type;
1819
using NHibernate.Util;
1920

@@ -85,81 +86,76 @@ public async Task CascadeOnAsync(IEntityPersister persister, object parent, obje
8586
}
8687

8788
/// <summary> Cascade an action to the child or children</summary>
88-
private Task CascadePropertyAsync(object parent, object child, IType type, CascadeStyle style, string propertyName, object anything, bool isCascadeDeleteEnabled, CancellationToken cancellationToken)
89+
private async Task CascadePropertyAsync(object parent, object child, IType type, CascadeStyle style, string propertyName, object anything, bool isCascadeDeleteEnabled, CancellationToken cancellationToken)
8990
{
90-
if (cancellationToken.IsCancellationRequested)
91-
{
92-
return Task.FromCanceled<object>(cancellationToken);
93-
}
94-
try
91+
cancellationToken.ThrowIfCancellationRequested();
92+
if (child != null)
9593
{
96-
if (child != null)
94+
if (type.IsAssociationType)
9795
{
98-
if (type.IsAssociationType)
96+
IAssociationType associationType = (IAssociationType)type;
97+
if (CascadeAssociationNow(associationType))
9998
{
100-
IAssociationType associationType = (IAssociationType)type;
101-
if (CascadeAssociationNow(associationType))
102-
{
103-
return CascadeAssociationAsync(parent, child, type, style, anything, isCascadeDeleteEnabled, cancellationToken);
104-
}
105-
}
106-
else if (type.IsComponentType)
107-
{
108-
return CascadeComponentAsync(parent, child, (IAbstractComponentType)type, propertyName, anything, cancellationToken);
99+
await (CascadeAssociationAsync(parent, child, type, style, anything, isCascadeDeleteEnabled, cancellationToken)).ConfigureAwait(false);
109100
}
110101
}
111-
else
102+
else if (type.IsComponentType)
112103
{
113-
// potentially we need to handle orphan deletes for one-to-ones here...
114-
if (type.IsEntityType && ((EntityType)type).IsLogicalOneToOne())
104+
await (CascadeComponentAsync(parent, child, (IAbstractComponentType)type, propertyName, anything, cancellationToken)).ConfigureAwait(false);
105+
}
106+
}
107+
else
108+
{
109+
// potentially we need to handle orphan deletes for one-to-ones here...
110+
if (type.IsEntityType && ((EntityType)type).IsLogicalOneToOne())
111+
{
112+
// We have a physical or logical one-to-one and from previous checks we know we
113+
// have a null value. See if the attribute cascade settings and action-type require
114+
// orphan checking
115+
if (style.HasOrphanDelete && action.DeleteOrphans)
115116
{
116-
// We have a physical or logical one-to-one and from previous checks we know we
117-
// have a null value. See if the attribute cascade settings and action-type require
118-
// orphan checking
119-
if (style.HasOrphanDelete && action.DeleteOrphans)
117+
// value is orphaned if loaded state for this property shows not null
118+
// because it is currently null.
119+
EntityEntry entry = eventSource.PersistenceContext.GetEntry(parent);
120+
if (entry != null && entry.Status != Status.Saving)
120121
{
121-
// value is orphaned if loaded state for this property shows not null
122-
// because it is currently null.
123-
EntityEntry entry = eventSource.PersistenceContext.GetEntry(parent);
124-
if (entry != null && entry.Status != Status.Saving)
122+
object loadedValue;
123+
if (componentPathStack.Count == 0)
124+
{
125+
// association defined on entity
126+
loadedValue = entry.GetLoadedValue(propertyName);
127+
128+
// Check this is not a null carrying proxy. The no-proxy load is currently handled by
129+
// putting a proxy (!) flagged for unwrapping (even for non-constrained one-to-one,
130+
// which association may be null) in the loadedState of the parent. The unwrap flag
131+
// causes it to support having a null implementation, instead of throwing an entity
132+
// not found error.
133+
loadedValue = await (eventSource.PersistenceContext.UnproxyAndReassociateAsync(loadedValue, cancellationToken)).ConfigureAwait(false);
134+
}
135+
else
125136
{
126-
EntityType entityType = (EntityType)type;
127-
object loadedValue;
128-
if (componentPathStack.Count == 0)
129-
{
130-
// association defined on entity
131-
loadedValue = entry.GetLoadedValue(propertyName);
132-
}
133-
else
134-
{
135-
// association defined on component
136-
// todo : this is currently unsupported because of the fact that
137-
// we do not know the loaded state of this value properly
138-
// and doing so would be very difficult given how components and
139-
// entities are loaded (and how 'loaded state' is put into the
140-
// EntityEntry). Solutions here are to either:
141-
// 1) properly account for components as a 2-phase load construct
142-
// 2) just assume the association was just now orphaned and
143-
// issue the orphan delete. This would require a special
144-
// set of SQL statements though since we do not know the
145-
// orphaned value, something a delete with a subquery to
146-
// match the owner.
147-
loadedValue = null;
148-
}
137+
// association defined on component
138+
// todo : this is currently unsupported because of the fact that
139+
// we do not know the loaded state of this value properly
140+
// and doing so would be very difficult given how components and
141+
// entities are loaded (and how 'loaded state' is put into the
142+
// EntityEntry). Solutions here are to either:
143+
// 1) properly account for components as a 2-phase load construct
144+
// 2) just assume the association was just now orphaned and
145+
// issue the orphan delete. This would require a special
146+
// set of SQL statements though since we do not know the
147+
// orphaned value, something a delete with a subquery to
148+
// match the owner.
149+
loadedValue = null;
150+
}
149151

150-
if (loadedValue != null)
151-
{
152-
return eventSource.DeleteAsync(entry.Persister.EntityName, loadedValue, false, null, cancellationToken);
153-
}
152+
if (loadedValue != null)
153+
{
154+
await (eventSource.DeleteAsync(entry.Persister.EntityName, loadedValue, false, null, cancellationToken)).ConfigureAwait(false);
154155
}
155156
}
156157
}
157158
}
158-
return Task.CompletedTask;
159-
}
160-
catch (System.Exception ex)
161-
{
162-
return Task.FromException<object>(ex);
163159
}
164160
}
165161

‎src/NHibernate/Engine/Cascade.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using NHibernate.Event;
55
using NHibernate.Persister.Collection;
66
using NHibernate.Persister.Entity;
7+
using NHibernate.Proxy;
78
using NHibernate.Type;
89
using NHibernate.Util;
910

@@ -170,12 +171,18 @@ private void CascadeProperty(object parent, object child, IType type, CascadeSty
170171
EntityEntry entry = eventSource.PersistenceContext.GetEntry(parent);
171172
if (entry != null && entry.Status != Status.Saving)
172173
{
173-
EntityType entityType = (EntityType)type;
174174
object loadedValue;
175175
if (componentPathStack.Count == 0)
176176
{
177177
// association defined on entity
178178
loadedValue = entry.GetLoadedValue(propertyName);
179+
180+
// Check this is not a null carrying proxy. The no-proxy load is currently handled by
181+
// putting a proxy (!) flagged for unwrapping (even for non-constrained one-to-one,
182+
// which association may be null) in the loadedState of the parent. The unwrap flag
183+
// causes it to support having a null implementation, instead of throwing an entity
184+
// not found error.
185+
loadedValue = eventSource.PersistenceContext.UnproxyAndReassociate(loadedValue);
179186
}
180187
else
181188
{

0 commit comments

Comments
 (0)