Skip to content

Commit 199aeeb

Browse files
CopilotAndriySvyryd
Copilot
and
AndriySvyryd
authored
Fix nullable complex property with PropertyValues.ToObject() and SetValues() creating instance instead of null (#37302)
Fixes #37249 Co-authored-by: AndriySvyryd <[email protected]>
1 parent 6d17975 commit 199aeeb

File tree

15 files changed

+396
-170
lines changed

15 files changed

+396
-170
lines changed

‎src/EFCore/ChangeTracking/EntityEntry.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ public virtual PropertyValues OriginalValues
603603
{
604604
var values = Finder.GetDatabaseValues(InternalEntry);
605605

606-
return values == null ? null : new ArrayPropertyValues(InternalEntry, values);
606+
return values == null ? null : new ArrayPropertyValues(InternalEntry, values, null);
607607
}
608608

609609
/// <summary>
@@ -634,7 +634,7 @@ public virtual PropertyValues OriginalValues
634634
{
635635
var values = await Finder.GetDatabaseValuesAsync(InternalEntry, cancellationToken).ConfigureAwait(false);
636636

637-
return values == null ? null : new ArrayPropertyValues(InternalEntry, values);
637+
return values == null ? null : new ArrayPropertyValues(InternalEntry, values, null);
638638
}
639639

640640
/// <summary>

‎src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,20 @@ public class ArrayPropertyValues : PropertyValues
1616
{
1717
private readonly object?[] _values;
1818
private readonly List<ArrayPropertyValues?>?[] _complexCollectionValues;
19+
private readonly bool[]? _nullComplexPropertyFlags;
1920

2021
/// <summary>
2122
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
2223
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
2324
/// any release. You should only use it directly in your code with extreme caution and knowing that
2425
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2526
/// </summary>
26-
public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values)
27+
public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values, bool[]? nullComplexPropertyFlags)
2728
: base(internalEntry)
2829
{
2930
_values = values;
3031
_complexCollectionValues = new List<ArrayPropertyValues?>?[ComplexCollectionProperties.Count];
32+
_nullComplexPropertyFlags = nullComplexPropertyFlags;
3133
}
3234

3335
/// <summary>
@@ -41,6 +43,18 @@ public override object ToObject()
4143
var structuralObject = StructuralType.GetOrCreateMaterializer(MaterializerSource)(
4244
new MaterializationContext(new ValueBuffer(_values), InternalEntry.Context));
4345

46+
if (_nullComplexPropertyFlags != null && NullableComplexProperties != null)
47+
{
48+
for (var i = 0; i < _nullComplexPropertyFlags.Length; i++)
49+
{
50+
if (_nullComplexPropertyFlags[i])
51+
{
52+
var complexProperty = NullableComplexProperties[i];
53+
structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null);
54+
}
55+
}
56+
}
57+
4458
for (var i = 0; i < _complexCollectionValues.Length; i++)
4559
{
4660
var propertyValuesList = _complexCollectionValues[i];
@@ -65,6 +79,15 @@ public override object ToObject()
6579
return structuralObject;
6680
}
6781

82+
/// <summary>
83+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
84+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
85+
/// any release. You should only use it directly in your code with extreme caution and knowing that
86+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
87+
/// </summary>
88+
public override bool IsNullableComplexPropertyNull(int index)
89+
=> _nullComplexPropertyFlags != null && _nullComplexPropertyFlags[index];
90+
6891
/// <summary>
6992
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
7093
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -132,7 +155,15 @@ public override PropertyValues Clone()
132155
var copies = new object[_values.Length];
133156
Array.Copy(_values, copies, _values.Length);
134157

135-
var clone = new ArrayPropertyValues(InternalEntry, copies);
158+
bool[]? flagsCopy = null;
159+
if (_nullComplexPropertyFlags != null)
160+
{
161+
flagsCopy = new bool[_nullComplexPropertyFlags.Length];
162+
Array.Copy(_nullComplexPropertyFlags, flagsCopy, _nullComplexPropertyFlags.Length);
163+
}
164+
165+
var clone = new ArrayPropertyValues(InternalEntry, copies, flagsCopy);
166+
136167
for (var i = 0; i < _complexCollectionValues.Length; i++)
137168
{
138169
var list = _complexCollectionValues[i];
@@ -354,7 +385,7 @@ private void SetValuesFromDictionary<TProperty>(IRuntimeTypeBase structuralType,
354385
var complexEntry = new InternalComplexEntry((IRuntimeComplexType)complexProperty.ComplexType, InternalEntry, i);
355386
var complexType = complexEntry.StructuralType;
356387
var values = new object?[complexType.GetFlattenedProperties().Count()];
357-
var complexPropertyValues = new ArrayPropertyValues(complexEntry, values);
388+
var complexPropertyValues = new ArrayPropertyValues(complexEntry, values, null);
358389
complexPropertyValues.SetValues(itemDict);
359390

360391
propertyValuesList.Add(complexPropertyValues);
@@ -468,7 +499,7 @@ ArrayPropertyValues CreateComplexPropertyValues(object complexObject, InternalCo
468499
values[i] = getter.GetClrValue(complexObject);
469500
}
470501

471-
var complexPropertyValues = new ArrayPropertyValues(entry, values);
502+
var complexPropertyValues = new ArrayPropertyValues(entry, values, null);
472503

473504
foreach (var nestedComplexProperty in complexPropertyValues.ComplexCollectionProperties)
474505
{

‎src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,19 @@ public override PropertyValues Clone()
282282
values[i] = GetValueInternal(InternalEntry, Properties[i]);
283283
}
284284

285-
var cloned = new ArrayPropertyValues(InternalEntry, values);
285+
bool[]? flags = null;
286+
var nullableComplexProperties = NullableComplexProperties;
287+
if (nullableComplexProperties != null && nullableComplexProperties.Count > 0)
288+
{
289+
flags = new bool[nullableComplexProperties.Count];
290+
for (var i = 0; i < nullableComplexProperties.Count; i++)
291+
{
292+
flags[i] = GetValueInternal(InternalEntry, nullableComplexProperties[i]) == null;
293+
}
294+
}
295+
296+
var cloned = new ArrayPropertyValues(InternalEntry, values, flags);
297+
286298
foreach (var complexProperty in ComplexCollectionProperties)
287299
{
288300
var collection = (IList?)GetValueInternal(InternalEntry, complexProperty);
@@ -305,15 +317,58 @@ public override void SetValues(PropertyValues propertyValues)
305317
{
306318
Check.NotNull(propertyValues);
307319

320+
var nullableComplexProperties = NullableComplexProperties;
321+
HashSet<IComplexProperty>? nullComplexProperties = null;
322+
if (nullableComplexProperties != null)
323+
{
324+
for (var i = 0; i < nullableComplexProperties.Count; i++)
325+
{
326+
if (propertyValues.IsNullableComplexPropertyNull(i))
327+
{
328+
nullComplexProperties ??= [];
329+
nullComplexProperties.Add(nullableComplexProperties[i]);
330+
}
331+
}
332+
}
333+
308334
foreach (var property in Properties)
309335
{
336+
if (nullComplexProperties != null && IsPropertyInNullComplexType(property, nullComplexProperties))
337+
{
338+
continue;
339+
}
340+
310341
SetValueInternal(InternalEntry, property, propertyValues[property]);
311342
}
312343

313344
foreach (var complexProperty in ComplexCollectionProperties)
314345
{
315346
SetValueInternal(InternalEntry, complexProperty, propertyValues[complexProperty]);
316347
}
348+
349+
if (nullComplexProperties != null)
350+
{
351+
foreach (var complexProperty in nullComplexProperties)
352+
{
353+
SetValueInternal(InternalEntry, complexProperty, null);
354+
}
355+
}
356+
}
357+
358+
private static bool IsPropertyInNullComplexType(IProperty property, HashSet<IComplexProperty> nullComplexProperties)
359+
{
360+
var declaringType = property.DeclaringType;
361+
while (declaringType is IComplexType complexType)
362+
{
363+
if (nullComplexProperties.Contains(complexType.ComplexProperty))
364+
{
365+
return true;
366+
}
367+
368+
declaringType = complexType.ComplexProperty.DeclaringType;
369+
}
370+
371+
return false;
317372
}
318373

319374
/// <inheritdoc />

‎src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ private PropertyValues GetPropertyValues(InternalEntryBase entry)
9797
values[i] = entry.GetOriginalValue(properties[i]);
9898
}
9999

100-
var cloned = new ArrayPropertyValues(entry, values);
100+
var cloned = new ArrayPropertyValues(entry, values, null);
101101

102102
foreach (var nestedComplexProperty in cloned.ComplexCollectionProperties)
103103
{

‎src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ protected virtual Expression CreateSnapshotExpression(
145145
case var _ when propertyBase.IsShadowProperty():
146146
arguments[i] = CreateSnapshotValueExpression(CreateReadShadowValueExpression(parameter, propertyBase), propertyBase);
147147
continue;
148+
149+
case IComplexProperty { IsCollection: false, IsNullable: true }:
150+
// For nullable non-collection complex properties, convert to object to store the null reference
151+
arguments[i] = Expression.Convert(
152+
CreateSnapshotValueExpression(CreateReadValueExpression(parameter, propertyBase), propertyBase),
153+
typeof(object));
154+
continue;
148155
}
149156

150157
arguments[i] = CreateSnapshotValueExpression(CreateReadValueExpression(parameter, propertyBase), propertyBase);

‎src/EFCore/ChangeTracking/PropertyValues.cs‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking;
2626
/// </remarks>
2727
public abstract class PropertyValues
2828
{
29+
private static readonly bool UseOldBehavior37249 =
30+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37249", out var enabled) && enabled;
31+
2932
private readonly IReadOnlyList<IComplexProperty> _complexCollectionProperties;
33+
private readonly IReadOnlyList<IComplexProperty>? _nullableComplexProperties;
3034

3135
/// <summary>
3236
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -38,10 +42,27 @@ public abstract class PropertyValues
3842
protected PropertyValues(InternalEntryBase internalEntry)
3943
{
4044
InternalEntry = internalEntry;
41-
_complexCollectionProperties = [.. internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => p.IsCollection)];
45+
46+
var complexCollectionProperties = new List<IComplexProperty>();
47+
var nullableComplexProperties = UseOldBehavior37249 ? null : new List<IComplexProperty>();
48+
49+
foreach (var complexProperty in internalEntry.StructuralType.GetFlattenedComplexProperties())
50+
{
51+
if (complexProperty.IsCollection)
52+
{
53+
complexCollectionProperties.Add(complexProperty);
54+
}
55+
else if (!UseOldBehavior37249 && complexProperty.IsNullable && !complexProperty.IsShadowProperty())
56+
{
57+
nullableComplexProperties!.Add(complexProperty);
58+
}
59+
}
60+
61+
_complexCollectionProperties = complexCollectionProperties;
4262
Check.DebugAssert(
4363
_complexCollectionProperties.Select((p, i) => p.GetIndex() == i).All(e => e),
4464
"Complex collection properties indices are not sequential.");
65+
_nullableComplexProperties = nullableComplexProperties?.Count > 0 ? nullableComplexProperties : null;
4566
}
4667

4768
/// <summary>
@@ -154,6 +175,29 @@ public virtual IReadOnlyList<IComplexProperty> ComplexCollectionProperties
154175
get => _complexCollectionProperties;
155176
}
156177

178+
/// <summary>
179+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
180+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
181+
/// any release. You should only use it directly in your code with extreme caution and knowing that
182+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
183+
/// </summary>
184+
[EntityFrameworkInternal]
185+
protected virtual IReadOnlyList<IComplexProperty>? NullableComplexProperties
186+
{
187+
[DebuggerStepThrough]
188+
get => _nullableComplexProperties;
189+
}
190+
191+
/// <summary>
192+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
193+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
194+
/// any release. You should only use it directly in your code with extreme caution and knowing that
195+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
196+
/// </summary>
197+
[EntityFrameworkInternal]
198+
public virtual bool IsNullableComplexPropertyNull(int index)
199+
=> false;
200+
157201
/// <summary>
158202
/// Gets the underlying structural type for which this object is storing values.
159203
/// </summary>

‎src/EFCore/Metadata/Internal/EntityTypeExtensions.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,10 @@ private static void CalculateCounts(
244244
{
245245
var indexes = new PropertyIndexes(
246246
index: complexProperty.IsCollection ? complexCollectionIndex++ : complexPropertyIndex++,
247-
originalValueIndex: complexProperty.IsCollection ? originalValueIndex++ : -1,
247+
originalValueIndex: complexProperty.IsCollection
248+
|| (complexProperty.IsNullable && !complexProperty.IsShadowProperty())
249+
? originalValueIndex++
250+
: -1,
248251
shadowIndex: complexProperty.IsShadowProperty() ? shadowIndex++ : -1,
249252
relationshipIndex: -1,
250253
storeGenerationIndex: -1);

0 commit comments

Comments
 (0)