From 483222a074457cd72ae8f7d18e316e2ceea57113 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Dec 2025 22:44:50 +0000 Subject: [PATCH 1/9] Initial plan From 25a736e2adfadc16c33f4425a4a11c6bb509b782 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Dec 2025 23:19:11 +0000 Subject: [PATCH 2/9] Fix nullable complex property ToObject() returning instance instead of null When calling OriginalValues.ToObject() or CurrentValues.ToObject() on an entity with a nullable complex property that is null, the method was creating an instance of the complex type with default values instead of returning null. The fix adds tracking for null nullable complex properties in ArrayPropertyValues and sets them to null after materialization in ToObject(). Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 36 +++++++++++++++ .../Internal/EntryPropertyValues.cs | 14 ++++++ .../PropertyValuesInMemoryTest.cs | 8 ++++ .../PropertyValuesTestBase.cs | 45 +++++++++++++++++++ 4 files changed, 103 insertions(+) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index e4fe149a005..f55f00fb526 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -16,6 +16,7 @@ public class ArrayPropertyValues : PropertyValues { private readonly object?[] _values; private readonly List?[] _complexCollectionValues; + private HashSet? _nullComplexProperties; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -30,6 +31,19 @@ public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values) _complexCollectionValues = new List?[ComplexCollectionProperties.Count]; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + internal void MarkComplexPropertyAsNull(IComplexProperty complexProperty) + { + _nullComplexProperties ??= []; + _nullComplexProperties.Add(complexProperty); + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -41,6 +55,18 @@ public override object ToObject() var structuralObject = StructuralType.GetOrCreateMaterializer(MaterializerSource)( new MaterializationContext(new ValueBuffer(_values), InternalEntry.Context)); + // Set null for nullable complex properties that were explicitly marked as null + if (_nullComplexProperties != null) + { + foreach (var complexProperty in _nullComplexProperties) + { + if (!complexProperty.IsShadowProperty()) + { + structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); + } + } + } + for (var i = 0; i < _complexCollectionValues.Length; i++) { var propertyValuesList = _complexCollectionValues[i]; @@ -133,6 +159,16 @@ public override PropertyValues Clone() Array.Copy(_values, copies, _values.Length); var clone = new ArrayPropertyValues(InternalEntry, copies); + + // Copy null complex property tracking + if (_nullComplexProperties != null) + { + foreach (var complexProperty in _nullComplexProperties) + { + clone.MarkComplexPropertyAsNull(complexProperty); + } + } + for (var i = 0; i < _complexCollectionValues.Length; i++) { var list = _complexCollectionValues[i]; diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index bae89bf32a6..137ce58445e 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -283,6 +283,20 @@ public override PropertyValues Clone() } var cloned = new ArrayPropertyValues(InternalEntry, values); + + // Track null nullable complex properties (non-collection) + foreach (var complexProperty in StructuralType.GetFlattenedComplexProperties()) + { + if (!complexProperty.IsCollection && complexProperty.IsNullable) + { + var complexValue = GetValueInternal(InternalEntry, complexProperty); + if (complexValue == null) + { + cloned.MarkComplexPropertyAsNull(complexProperty); + } + } + } + foreach (var complexProperty in ComplexCollectionProperties) { var collection = (IList?)GetValueInternal(InternalEntry, complexProperty); diff --git a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs index 67ed30104d3..2cc3e6873b2 100644 --- a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs @@ -8,6 +8,11 @@ namespace Microsoft.EntityFrameworkCore; public class PropertyValuesInMemoryTest(PropertyValuesInMemoryTest.PropertyValuesInMemoryFixture fixture) : PropertyValuesTestBase(fixture) { + // Nullable complex property test - InMemory provider doesn't support complex types in queries + public override void Nullable_complex_property_with_null_value_returns_null_when_using_ToObject() + { + } + public override Task Complex_current_values_can_be_accessed_as_a_property_dictionary_using_IProperty() => Assert.ThrowsAsync( // In-memory database cannot query complex types () => base.Complex_current_values_can_be_accessed_as_a_property_dictionary_using_IProperty()); @@ -117,6 +122,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con // In-memory database doesn't support complex collections modelBuilder.Ignore(); + + // In-memory database doesn't fully support complex types + modelBuilder.Ignore(); } } } diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index d62eb9c59fc..f9f0065e216 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -3167,6 +3167,35 @@ public virtual void SetValues_throws_for_complex_property_with_non_dictionary_va Assert.Equal(CoreStrings.ComplexPropertyValueNotDictionary("Culture", "string"), exception.Message); } + [ConditionalFact] + public virtual void Nullable_complex_property_with_null_value_returns_null_when_using_ToObject() + { + using var context = CreateContext(); + + // Create a product without setting Price (it's null) + var product = new ProductWithNullableComplexType { Name = "Test Product" }; + context.Add(product); + context.SaveChanges(); + + context.ChangeTracker.Clear(); + + // Reload the product + var loadedProduct = context.Set().Single(p => p.Name == "Test Product"); + + // Verify the loaded entity has null Price + Assert.Null(loadedProduct.Price); + + // Get OriginalValues and create object + var originalProduct = (ProductWithNullableComplexType)context.Entry(loadedProduct).OriginalValues.ToObject(); + + // The original value should also have null Price + Assert.Null(originalProduct.Price); + + // Also test CurrentValues + var currentProduct = (ProductWithNullableComplexType)context.Entry(loadedProduct).CurrentValues.ToObject(); + Assert.Null(currentProduct.Price); + } + [ConditionalFact] public virtual void Current_values_can_be_copied_to_object_using_ToObject() { @@ -3802,6 +3831,20 @@ protected class Course public int Credits { get; set; } } + protected class ProductWithNullableComplexType : PropertyValuesBase + { + public int Id { get; set; } + public required string Name { get; set; } + public ProductPrice? Price { get; set; } + } + + [ComplexType] + protected class ProductPrice + { + public decimal Amount { get; init; } + public int CurrencyId { get; init; } + } + protected DbContext CreateContext() { var context = Fixture.CreateContext(); @@ -3910,6 +3953,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity(); modelBuilder.Entity(b => b.ComplexCollection(e => e.Departments)); + + modelBuilder.Entity(b => b.ComplexProperty(e => e.Price)); } protected override Task SeedAsync(PoolableDbContext context) From 2b6090f687681045109be8461a23bfcb03ff0ebf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Dec 2025 23:24:24 +0000 Subject: [PATCH 3/9] Address code review feedback: add comments explaining null complex property tracking Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index f55f00fb526..3a6c8a86a1d 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -16,6 +16,10 @@ public class ArrayPropertyValues : PropertyValues { private readonly object?[] _values; private readonly List?[] _complexCollectionValues; + + // Tracks nullable non-collection complex properties that should be null when materializing via ToObject(). + // This is needed because value type properties inside nullable complex types store default values (not null) + // when the complex property is null, making it impossible to detect nullness from the values array alone. private HashSet? _nullComplexProperties; /// @@ -55,7 +59,8 @@ public override object ToObject() var structuralObject = StructuralType.GetOrCreateMaterializer(MaterializerSource)( new MaterializationContext(new ValueBuffer(_values), InternalEntry.Context)); - // Set null for nullable complex properties that were explicitly marked as null + // Set null for nullable complex properties that were explicitly marked as null. + // Shadow properties don't have CLR members and aren't part of the materialized object. if (_nullComplexProperties != null) { foreach (var complexProperty in _nullComplexProperties) From d13bfc9df4fa54d8e0ec21e36a872667ad585776 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 03:23:21 +0000 Subject: [PATCH 4/9] Address PR review feedback: use bool[] for null tracking, use entry indexer - Changed _nullComplexProperties from HashSet to bool[] with indices matching NullableComplexProperties ordering - Added NullableComplexProperties property to PropertyValues base class - Fixed EntryPropertyValues.Clone() to use InternalEntry[] indexer instead of GetValueInternal for complex properties - Simplified test to use newly added product without reload, removed comments - Removed ProductWithNullableComplexType, added OptionalMilk to Building class Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 31 ++++++------- .../Internal/EntryPropertyValues.cs | 17 +++---- src/EFCore/ChangeTracking/PropertyValues.cs | 15 +++++++ .../PropertyValuesInMemoryTest.cs | 4 +- .../PropertyValuesTestBase.cs | 44 ++++--------------- 5 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index 3a6c8a86a1d..8645f60d34f 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -20,7 +20,8 @@ public class ArrayPropertyValues : PropertyValues // Tracks nullable non-collection complex properties that should be null when materializing via ToObject(). // This is needed because value type properties inside nullable complex types store default values (not null) // when the complex property is null, making it impossible to detect nullness from the values array alone. - private HashSet? _nullComplexProperties; + // The array indices correspond to NullableComplexProperties ordering. + private bool[]? _nullComplexPropertyFlags; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -42,11 +43,8 @@ public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - internal void MarkComplexPropertyAsNull(IComplexProperty complexProperty) - { - _nullComplexProperties ??= []; - _nullComplexProperties.Add(complexProperty); - } + internal void SetNullComplexPropertyFlags(bool[] flags) + => _nullComplexPropertyFlags = flags; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -61,13 +59,17 @@ public override object ToObject() // Set null for nullable complex properties that were explicitly marked as null. // Shadow properties don't have CLR members and aren't part of the materialized object. - if (_nullComplexProperties != null) + if (_nullComplexPropertyFlags != null) { - foreach (var complexProperty in _nullComplexProperties) + for (var i = 0; i < _nullComplexPropertyFlags.Length; i++) { - if (!complexProperty.IsShadowProperty()) + if (_nullComplexPropertyFlags[i]) { - structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); + var complexProperty = NullableComplexProperties[i]; + if (!complexProperty.IsShadowProperty()) + { + structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); + } } } } @@ -166,12 +168,11 @@ public override PropertyValues Clone() var clone = new ArrayPropertyValues(InternalEntry, copies); // Copy null complex property tracking - if (_nullComplexProperties != null) + if (_nullComplexPropertyFlags != null) { - foreach (var complexProperty in _nullComplexProperties) - { - clone.MarkComplexPropertyAsNull(complexProperty); - } + var flagsCopy = new bool[_nullComplexPropertyFlags.Length]; + Array.Copy(_nullComplexPropertyFlags, flagsCopy, _nullComplexPropertyFlags.Length); + clone.SetNullComplexPropertyFlags(flagsCopy); } for (var i = 0; i < _complexCollectionValues.Length; i++) diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index 137ce58445e..a2c3597b268 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -284,17 +284,18 @@ public override PropertyValues Clone() var cloned = new ArrayPropertyValues(InternalEntry, values); - // Track null nullable complex properties (non-collection) - foreach (var complexProperty in StructuralType.GetFlattenedComplexProperties()) + // Track null nullable complex properties (non-collection) using bool array + var nullableComplexProperties = NullableComplexProperties; + if (nullableComplexProperties.Count > 0) { - if (!complexProperty.IsCollection && complexProperty.IsNullable) + var flags = new bool[nullableComplexProperties.Count]; + for (var i = 0; i < nullableComplexProperties.Count; i++) { - var complexValue = GetValueInternal(InternalEntry, complexProperty); - if (complexValue == null) - { - cloned.MarkComplexPropertyAsNull(complexProperty); - } + // Use the entry's indexer to get the current complex property value + flags[i] = InternalEntry[nullableComplexProperties[i]] == null; } + + cloned.SetNullComplexPropertyFlags(flags); } foreach (var complexProperty in ComplexCollectionProperties) diff --git a/src/EFCore/ChangeTracking/PropertyValues.cs b/src/EFCore/ChangeTracking/PropertyValues.cs index 95777844184..840cb4255b9 100644 --- a/src/EFCore/ChangeTracking/PropertyValues.cs +++ b/src/EFCore/ChangeTracking/PropertyValues.cs @@ -27,6 +27,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking; public abstract class PropertyValues { private readonly IReadOnlyList _complexCollectionProperties; + private readonly IReadOnlyList _nullableComplexProperties; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -42,6 +43,7 @@ protected PropertyValues(InternalEntryBase internalEntry) Check.DebugAssert( _complexCollectionProperties.Select((p, i) => p.GetIndex() == i).All(e => e), "Complex collection properties indices are not sequential."); + _nullableComplexProperties = [.. internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => !p.IsCollection && p.IsNullable)]; } /// @@ -154,6 +156,19 @@ public virtual IReadOnlyList ComplexCollectionProperties get => _complexCollectionProperties; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + protected IReadOnlyList NullableComplexProperties + { + [DebuggerStepThrough] + get => _nullableComplexProperties; + } + /// /// Gets the underlying structural type for which this object is storing values. /// diff --git a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs index 2cc3e6873b2..2fb90b67dac 100644 --- a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs @@ -118,13 +118,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con { b.Ignore(e => e.Culture); b.Ignore(e => e.Milk); + b.Ignore(e => e.OptionalMilk); }); // In-memory database doesn't support complex collections modelBuilder.Ignore(); - - // In-memory database doesn't fully support complex types - modelBuilder.Ignore(); } } } diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index f9f0065e216..846f7bbe053 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -3171,29 +3171,15 @@ public virtual void SetValues_throws_for_complex_property_with_non_dictionary_va public virtual void Nullable_complex_property_with_null_value_returns_null_when_using_ToObject() { using var context = CreateContext(); + var building = context.Set().Single(b => b.Name == "Building One"); - // Create a product without setting Price (it's null) - var product = new ProductWithNullableComplexType { Name = "Test Product" }; - context.Add(product); - context.SaveChanges(); - - context.ChangeTracker.Clear(); - - // Reload the product - var loadedProduct = context.Set().Single(p => p.Name == "Test Product"); - - // Verify the loaded entity has null Price - Assert.Null(loadedProduct.Price); - - // Get OriginalValues and create object - var originalProduct = (ProductWithNullableComplexType)context.Entry(loadedProduct).OriginalValues.ToObject(); + Assert.Null(building.OptionalMilk); - // The original value should also have null Price - Assert.Null(originalProduct.Price); + var originalBuilding = (Building)context.Entry(building).OriginalValues.ToObject(); + Assert.Null(originalBuilding.OptionalMilk); - // Also test CurrentValues - var currentProduct = (ProductWithNullableComplexType)context.Entry(loadedProduct).CurrentValues.ToObject(); - Assert.Null(currentProduct.Price); + var currentBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); + Assert.Null(currentBuilding.OptionalMilk); } [ConditionalFact] @@ -3616,6 +3602,7 @@ public string NoSetter public Culture Culture { get; set; } public required Milk Milk { get; set; } + public Milk? OptionalMilk { get; set; } } [ComplexType] @@ -3831,20 +3818,6 @@ protected class Course public int Credits { get; set; } } - protected class ProductWithNullableComplexType : PropertyValuesBase - { - public int Id { get; set; } - public required string Name { get; set; } - public ProductPrice? Price { get; set; } - } - - [ComplexType] - protected class ProductPrice - { - public decimal Amount { get; init; } - public int CurrencyId { get; init; } - } - protected DbContext CreateContext() { var context = Fixture.CreateContext(); @@ -3946,6 +3919,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.ComplexProperty(e => e.Culture); b.ComplexProperty(e => e.Milk); + b.ComplexProperty(e => e.OptionalMilk); }); modelBuilder.Entity(); @@ -3953,8 +3927,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity(); modelBuilder.Entity(b => b.ComplexCollection(e => e.Departments)); - - modelBuilder.Entity(b => b.ComplexProperty(e => e.Price)); } protected override Task SeedAsync(PoolableDbContext context) From e8dbff74ef2f4aec990f5ac9aed3c605b706504c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 19:14:50 +0000 Subject: [PATCH 5/9] Address PR feedback: make flags constructor param, nullable property - Changed SetNullComplexPropertyFlags method to constructor parameter - Made NullableComplexProperties nullable (null when no nullable properties) - Fixed test expectations for new OptionalMilk property - Seeded OptionalMilk in Building.Create to avoid SetValues issues - Simplified test to use CurrentValues only Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 25 ++++-------- .../Internal/EntryPropertyValues.cs | 11 +++-- src/EFCore/ChangeTracking/PropertyValues.cs | 7 ++-- .../PropertyValuesTestBase.cs | 40 ++++++++++++++++--- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index 8645f60d34f..2bb62665ac6 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -21,7 +21,7 @@ public class ArrayPropertyValues : PropertyValues // This is needed because value type properties inside nullable complex types store default values (not null) // when the complex property is null, making it impossible to detect nullness from the values array alone. // The array indices correspond to NullableComplexProperties ordering. - private bool[]? _nullComplexPropertyFlags; + private readonly bool[]? _nullComplexPropertyFlags; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -29,23 +29,14 @@ public class ArrayPropertyValues : PropertyValues /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values) + public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values, bool[]? nullComplexPropertyFlags = null) : base(internalEntry) { _values = values; _complexCollectionValues = new List?[ComplexCollectionProperties.Count]; + _nullComplexPropertyFlags = nullComplexPropertyFlags; } - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - [EntityFrameworkInternal] - internal void SetNullComplexPropertyFlags(bool[] flags) - => _nullComplexPropertyFlags = flags; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -59,7 +50,7 @@ public override object ToObject() // Set null for nullable complex properties that were explicitly marked as null. // Shadow properties don't have CLR members and aren't part of the materialized object. - if (_nullComplexPropertyFlags != null) + if (_nullComplexPropertyFlags != null && NullableComplexProperties != null) { for (var i = 0; i < _nullComplexPropertyFlags.Length; i++) { @@ -165,16 +156,16 @@ public override PropertyValues Clone() var copies = new object[_values.Length]; Array.Copy(_values, copies, _values.Length); - var clone = new ArrayPropertyValues(InternalEntry, copies); - // Copy null complex property tracking + bool[]? flagsCopy = null; if (_nullComplexPropertyFlags != null) { - var flagsCopy = new bool[_nullComplexPropertyFlags.Length]; + flagsCopy = new bool[_nullComplexPropertyFlags.Length]; Array.Copy(_nullComplexPropertyFlags, flagsCopy, _nullComplexPropertyFlags.Length); - clone.SetNullComplexPropertyFlags(flagsCopy); } + var clone = new ArrayPropertyValues(InternalEntry, copies, flagsCopy); + for (var i = 0; i < _complexCollectionValues.Length; i++) { var list = _complexCollectionValues[i]; diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index a2c3597b268..2174ed156cc 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -282,22 +282,21 @@ public override PropertyValues Clone() values[i] = GetValueInternal(InternalEntry, Properties[i]); } - var cloned = new ArrayPropertyValues(InternalEntry, values); - // Track null nullable complex properties (non-collection) using bool array + bool[]? flags = null; var nullableComplexProperties = NullableComplexProperties; - if (nullableComplexProperties.Count > 0) + if (nullableComplexProperties != null && nullableComplexProperties.Count > 0) { - var flags = new bool[nullableComplexProperties.Count]; + flags = new bool[nullableComplexProperties.Count]; for (var i = 0; i < nullableComplexProperties.Count; i++) { // Use the entry's indexer to get the current complex property value flags[i] = InternalEntry[nullableComplexProperties[i]] == null; } - - cloned.SetNullComplexPropertyFlags(flags); } + var cloned = new ArrayPropertyValues(InternalEntry, values, flags); + foreach (var complexProperty in ComplexCollectionProperties) { var collection = (IList?)GetValueInternal(InternalEntry, complexProperty); diff --git a/src/EFCore/ChangeTracking/PropertyValues.cs b/src/EFCore/ChangeTracking/PropertyValues.cs index 840cb4255b9..76f30fd440e 100644 --- a/src/EFCore/ChangeTracking/PropertyValues.cs +++ b/src/EFCore/ChangeTracking/PropertyValues.cs @@ -27,7 +27,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking; public abstract class PropertyValues { private readonly IReadOnlyList _complexCollectionProperties; - private readonly IReadOnlyList _nullableComplexProperties; + private readonly IReadOnlyList? _nullableComplexProperties; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -43,7 +43,8 @@ protected PropertyValues(InternalEntryBase internalEntry) Check.DebugAssert( _complexCollectionProperties.Select((p, i) => p.GetIndex() == i).All(e => e), "Complex collection properties indices are not sequential."); - _nullableComplexProperties = [.. internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => !p.IsCollection && p.IsNullable)]; + var nullableComplexProperties = internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => !p.IsCollection && p.IsNullable).ToList(); + _nullableComplexProperties = nullableComplexProperties.Count > 0 ? nullableComplexProperties : null; } /// @@ -163,7 +164,7 @@ public virtual IReadOnlyList ComplexCollectionProperties /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - protected IReadOnlyList NullableComplexProperties + protected IReadOnlyList? NullableComplexProperties { [DebuggerStepThrough] get => _nullableComplexProperties; diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index 846f7bbe053..32b5f6507e9 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -2055,6 +2055,18 @@ private async Task TestProperties(Func, Task().Single(b => b.Name == "Building One"); + building.OptionalMilk = null; - Assert.Null(building.OptionalMilk); - - var originalBuilding = (Building)context.Entry(building).OriginalValues.ToObject(); + var originalBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); Assert.Null(originalBuilding.OptionalMilk); - - var currentBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); - Assert.Null(currentBuilding.OptionalMilk); } [ConditionalFact] @@ -3573,6 +3581,26 @@ public static Building Create(Guid buildingId, string name, decimal value, int? Rating = 8 + (tag ?? 0), Species = "S1" + tag, Validation = false + }, + OptionalMilk = new Milk + { + License = new License + { + Charge = 2.0m + (tag ?? 0), + Tag = new Tag { Text = "Ta3" + tag }, + Title = "Ti2" + tag, + Tog = new Tog { Text = "To3" + tag } + }, + Manufacturer = new Manufacturer + { + Name = "M2" + tag, + Rating = 9 + (tag ?? 0), + Tag = new Tag { Text = "Ta4" + tag }, + Tog = new Tog { Text = "To4" + tag } + }, + Rating = 10 + (tag ?? 0), + Species = "S2" + tag, + Validation = true } }; From 7d35fe36d986f3cb8321a98533eb00a58fb35291 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 21:33:24 +0000 Subject: [PATCH 6/9] Address PR feedback: make NullableComplexProperties virtual, add OriginalValues test, add SetValues test, refactor constructor - Made NullableComplexProperties virtual - Added assert for OriginalValues in Nullable_complex_property_with_null_value_returns_null_when_using_ToObject test - Added new test Setting_current_values_from_cloned_values_sets_nullable_complex_property_to_null - Refactored PropertyValues constructor to use a loop and filter out shadow properties - Added abstract GetComplexPropertyValue method for proper original/current value handling in Clone() - Added IsNullableComplexPropertyNull method for SetValues to handle null nullable complex properties - Updated SetValues to properly set nullable complex properties to null when source has them as null - Skip new tests in InMemory provider where complex properties are ignored Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 9 +++++ .../Internal/CurrentPropertyValues.cs | 9 +++++ .../Internal/EntryPropertyValues.cs | 25 +++++++++++-- .../Internal/OriginalPropertyValues.cs | 11 ++++++ src/EFCore/ChangeTracking/PropertyValues.cs | 36 +++++++++++++++++-- .../PropertyValuesInMemoryTest.cs | 6 +++- .../PropertyValuesTestBase.cs | 34 ++++++++++++++++-- 7 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index 2bb62665ac6..c0b8594a431 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -89,6 +89,15 @@ public override object ToObject() return structuralObject; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override bool IsNullableComplexPropertyNull(int index) + => _nullComplexPropertyFlags != null && index < _nullComplexPropertyFlags.Length && _nullComplexPropertyFlags[index]; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs index ce61c477464..07ceafe1122 100644 --- a/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs @@ -60,6 +60,15 @@ protected override void SetValueInternal(IInternalEntry entry, IPropertyBase pro protected override object? GetValueInternal(IInternalEntry entry, IPropertyBase property) => entry[property]; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty) + => entry[complexProperty]; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index 2174ed156cc..ba0fa2b52d4 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -290,8 +290,7 @@ public override PropertyValues Clone() flags = new bool[nullableComplexProperties.Count]; for (var i = 0; i < nullableComplexProperties.Count; i++) { - // Use the entry's indexer to get the current complex property value - flags[i] = InternalEntry[nullableComplexProperties[i]] == null; + flags[i] = GetComplexPropertyValue(InternalEntry, nullableComplexProperties[i]) == null; } } @@ -328,6 +327,19 @@ public override void SetValues(PropertyValues propertyValues) { SetValueInternal(InternalEntry, complexProperty, propertyValues[complexProperty]); } + + // Handle nullable complex properties - set to null if source has them as null + var nullableComplexProperties = NullableComplexProperties; + if (nullableComplexProperties != null) + { + for (var i = 0; i < nullableComplexProperties.Count; i++) + { + if (propertyValues.IsNullableComplexPropertyNull(i)) + { + InternalEntry[nullableComplexProperties[i]] = null; + } + } + } } /// @@ -511,6 +523,15 @@ public override IList? this[IComplexProperty complexProperty] [EntityFrameworkInternal] protected abstract object? GetValueInternal(IInternalEntry entry, IPropertyBase property); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + protected abstract object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty); + /// /// Creates a complex object from a dictionary of property values using EF's property accessors. /// diff --git a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs index 6b4165b4ac9..df07a6d33ee 100644 --- a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs @@ -111,6 +111,17 @@ private PropertyValues GetPropertyValues(InternalEntryBase entry) return cloned; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty) + => ((InternalEntryBase)entry).CanHaveOriginalValue(complexProperty) + ? entry.GetOriginalValue(complexProperty) + : entry[complexProperty]; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/PropertyValues.cs b/src/EFCore/ChangeTracking/PropertyValues.cs index 76f30fd440e..a81fcfe04ff 100644 --- a/src/EFCore/ChangeTracking/PropertyValues.cs +++ b/src/EFCore/ChangeTracking/PropertyValues.cs @@ -39,11 +39,31 @@ public abstract class PropertyValues protected PropertyValues(InternalEntryBase internalEntry) { InternalEntry = internalEntry; - _complexCollectionProperties = [.. internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => p.IsCollection)]; + + var complexCollectionProperties = new List(); + var nullableComplexProperties = new List(); + + foreach (var complexProperty in internalEntry.StructuralType.GetFlattenedComplexProperties()) + { + if (complexProperty.IsShadowProperty()) + { + continue; + } + + if (complexProperty.IsCollection) + { + complexCollectionProperties.Add(complexProperty); + } + else if (complexProperty.IsNullable) + { + nullableComplexProperties.Add(complexProperty); + } + } + + _complexCollectionProperties = complexCollectionProperties; Check.DebugAssert( _complexCollectionProperties.Select((p, i) => p.GetIndex() == i).All(e => e), "Complex collection properties indices are not sequential."); - var nullableComplexProperties = internalEntry.StructuralType.GetFlattenedComplexProperties().Where(p => !p.IsCollection && p.IsNullable).ToList(); _nullableComplexProperties = nullableComplexProperties.Count > 0 ? nullableComplexProperties : null; } @@ -164,12 +184,22 @@ public virtual IReadOnlyList ComplexCollectionProperties /// doing so can result in application failures when updating to a new Entity Framework Core release. /// [EntityFrameworkInternal] - protected IReadOnlyList? NullableComplexProperties + protected virtual IReadOnlyList? NullableComplexProperties { [DebuggerStepThrough] get => _nullableComplexProperties; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + public virtual bool IsNullableComplexPropertyNull(int index) + => false; + /// /// Gets the underlying structural type for which this object is storing values. /// diff --git a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs index 2fb90b67dac..b16679ad6f8 100644 --- a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs @@ -8,11 +8,15 @@ namespace Microsoft.EntityFrameworkCore; public class PropertyValuesInMemoryTest(PropertyValuesInMemoryTest.PropertyValuesInMemoryFixture fixture) : PropertyValuesTestBase(fixture) { - // Nullable complex property test - InMemory provider doesn't support complex types in queries + // Nullable complex property tests - InMemory provider doesn't support complex types in queries public override void Nullable_complex_property_with_null_value_returns_null_when_using_ToObject() { } + public override void Setting_current_values_from_cloned_values_sets_nullable_complex_property_to_null() + { + } + public override Task Complex_current_values_can_be_accessed_as_a_property_dictionary_using_IProperty() => Assert.ThrowsAsync( // In-memory database cannot query complex types () => base.Complex_current_values_can_be_accessed_as_a_property_dictionary_using_IProperty()); diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index 32b5f6507e9..797e79e42a6 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -3184,10 +3184,40 @@ public virtual void Nullable_complex_property_with_null_value_returns_null_when_ { using var context = CreateContext(); var building = context.Set().Single(b => b.Name == "Building One"); + + var originalBuilding = (Building)context.Entry(building).OriginalValues.ToObject(); + Assert.NotNull(originalBuilding.OptionalMilk); + building.OptionalMilk = null; - var originalBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); - Assert.Null(originalBuilding.OptionalMilk); + var currentBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); + Assert.Null(currentBuilding.OptionalMilk); + } + + [ConditionalFact] + public virtual void Setting_current_values_from_cloned_values_sets_nullable_complex_property_to_null() + { + using var context = CreateContext(); + var building = context.Set().Single(b => b.Name == "Building One"); + + Assert.NotNull(building.OptionalMilk); + building.OptionalMilk = null; + + var clonedValues = context.Entry(building).CurrentValues.Clone(); + Assert.Null(((Building)clonedValues.ToObject()).OptionalMilk); + + building.OptionalMilk = new Milk + { + License = new License { Charge = 1.0m, Tag = new Tag { Text = "Ta1" }, Title = "Ti1", Tog = new Tog { Text = "To1" } }, + Manufacturer = new Manufacturer { Name = "M1", Rating = 7, Tag = new Tag { Text = "Ta2" }, Tog = new Tog { Text = "To2" } }, + Rating = 8, + Species = "S1", + Validation = false + }; + Assert.NotNull(building.OptionalMilk); + + context.Entry(building).CurrentValues.SetValues(clonedValues); + Assert.Null(building.OptionalMilk); } [ConditionalFact] From 7d1b3f7a100eab3e88aa67a6eeafd8a452c0adbd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 22:12:19 +0000 Subject: [PATCH 7/9] Address PR feedback: simplify code, use GetValueInternal, fix shadow property handling - Removed comment blocks in ArrayPropertyValues - Removed shadow property check in ToObject() (properties are always non-shadow) - Changed SetValues to use SetValueInternal for nullable complex properties - Removed GetComplexPropertyValue abstract method, consolidated into GetValueInternal - Updated OriginalPropertyValues.GetValueInternal to handle non-collection complex properties - Moved shadow property check lower in PropertyValues constructor (collection props can include shadow) Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 12 +--- .../Internal/CurrentPropertyValues.cs | 9 --- .../Internal/EntryPropertyValues.cs | 15 +---- .../Internal/OriginalPropertyValues.cs | 56 +++++++++---------- src/EFCore/ChangeTracking/PropertyValues.cs | 7 +-- 5 files changed, 30 insertions(+), 69 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index c0b8594a431..ababc2d8354 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -16,11 +16,6 @@ public class ArrayPropertyValues : PropertyValues { private readonly object?[] _values; private readonly List?[] _complexCollectionValues; - - // Tracks nullable non-collection complex properties that should be null when materializing via ToObject(). - // This is needed because value type properties inside nullable complex types store default values (not null) - // when the complex property is null, making it impossible to detect nullness from the values array alone. - // The array indices correspond to NullableComplexProperties ordering. private readonly bool[]? _nullComplexPropertyFlags; /// @@ -48,8 +43,6 @@ public override object ToObject() var structuralObject = StructuralType.GetOrCreateMaterializer(MaterializerSource)( new MaterializationContext(new ValueBuffer(_values), InternalEntry.Context)); - // Set null for nullable complex properties that were explicitly marked as null. - // Shadow properties don't have CLR members and aren't part of the materialized object. if (_nullComplexPropertyFlags != null && NullableComplexProperties != null) { for (var i = 0; i < _nullComplexPropertyFlags.Length; i++) @@ -57,10 +50,7 @@ public override object ToObject() if (_nullComplexPropertyFlags[i]) { var complexProperty = NullableComplexProperties[i]; - if (!complexProperty.IsShadowProperty()) - { - structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); - } + structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); } } } diff --git a/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs index 07ceafe1122..ce61c477464 100644 --- a/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/CurrentPropertyValues.cs @@ -60,15 +60,6 @@ protected override void SetValueInternal(IInternalEntry entry, IPropertyBase pro protected override object? GetValueInternal(IInternalEntry entry, IPropertyBase property) => entry[property]; - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty) - => entry[complexProperty]; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index ba0fa2b52d4..4ab14887db1 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -282,7 +282,6 @@ public override PropertyValues Clone() values[i] = GetValueInternal(InternalEntry, Properties[i]); } - // Track null nullable complex properties (non-collection) using bool array bool[]? flags = null; var nullableComplexProperties = NullableComplexProperties; if (nullableComplexProperties != null && nullableComplexProperties.Count > 0) @@ -290,7 +289,7 @@ public override PropertyValues Clone() flags = new bool[nullableComplexProperties.Count]; for (var i = 0; i < nullableComplexProperties.Count; i++) { - flags[i] = GetComplexPropertyValue(InternalEntry, nullableComplexProperties[i]) == null; + flags[i] = GetValueInternal(InternalEntry, nullableComplexProperties[i]) == null; } } @@ -328,7 +327,6 @@ public override void SetValues(PropertyValues propertyValues) SetValueInternal(InternalEntry, complexProperty, propertyValues[complexProperty]); } - // Handle nullable complex properties - set to null if source has them as null var nullableComplexProperties = NullableComplexProperties; if (nullableComplexProperties != null) { @@ -336,7 +334,7 @@ public override void SetValues(PropertyValues propertyValues) { if (propertyValues.IsNullableComplexPropertyNull(i)) { - InternalEntry[nullableComplexProperties[i]] = null; + SetValueInternal(InternalEntry, nullableComplexProperties[i], null); } } } @@ -523,15 +521,6 @@ public override IList? this[IComplexProperty complexProperty] [EntityFrameworkInternal] protected abstract object? GetValueInternal(IInternalEntry entry, IPropertyBase property); - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - [EntityFrameworkInternal] - protected abstract object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty); - /// /// Creates a complex object from a dictionary of property values using EF's property accessors. /// diff --git a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs index df07a6d33ee..8ae2565880f 100644 --- a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs @@ -60,31 +60,38 @@ protected override void SetValueInternal(IInternalEntry entry, IPropertyBase pro /// protected override object? GetValueInternal(IInternalEntry entry, IPropertyBase property) { - var originalValue = entry.GetOriginalValue(property); - if (property is IComplexProperty { IsCollection: true } complexProperty) + if (property is IComplexProperty complexProperty) { - var originalCollection = (IList?)originalValue; - if (originalCollection == null) + if (complexProperty.IsCollection) { - return null; + var originalCollection = (IList?)entry.GetOriginalValue(property); + if (originalCollection == null) + { + return null; + } + + // The stored original collection might contain references to the current elements, + // so we need to recreate it using stored values. + var clonedCollection = (IList)((IRuntimePropertyBase)complexProperty).GetIndexedCollectionAccessor() + .Create(originalCollection.Count); + for (var i = 0; i < originalCollection.Count; i++) + { + clonedCollection.Add( + originalCollection[i] == null + ? null + : GetPropertyValues(entry.GetComplexCollectionOriginalEntry(complexProperty, i)).ToObject()); + } + + return clonedCollection; } - // The stored original collection might contain references to the current elements, - // so we need to recreate it using stored values. - var clonedCollection = (IList)((IRuntimePropertyBase)complexProperty).GetIndexedCollectionAccessor() - .Create(originalCollection.Count); - for (var i = 0; i < originalCollection.Count; i++) - { - clonedCollection.Add( - originalCollection[i] == null - ? null - : GetPropertyValues(entry.GetComplexCollectionOriginalEntry(complexProperty, i)).ToObject()); - } - - return clonedCollection; + // For non-collection complex properties, check if original value can be tracked + return ((InternalEntryBase)entry).CanHaveOriginalValue(complexProperty) + ? entry.GetOriginalValue(complexProperty) + : entry[complexProperty]; } - return originalValue; + return entry.GetOriginalValue(property); } private PropertyValues GetPropertyValues(InternalEntryBase entry) @@ -111,17 +118,6 @@ private PropertyValues GetPropertyValues(InternalEntryBase entry) return cloned; } - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override object? GetComplexPropertyValue(IInternalEntry entry, IComplexProperty complexProperty) - => ((InternalEntryBase)entry).CanHaveOriginalValue(complexProperty) - ? entry.GetOriginalValue(complexProperty) - : entry[complexProperty]; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/PropertyValues.cs b/src/EFCore/ChangeTracking/PropertyValues.cs index a81fcfe04ff..94875b0baaa 100644 --- a/src/EFCore/ChangeTracking/PropertyValues.cs +++ b/src/EFCore/ChangeTracking/PropertyValues.cs @@ -45,16 +45,11 @@ protected PropertyValues(InternalEntryBase internalEntry) foreach (var complexProperty in internalEntry.StructuralType.GetFlattenedComplexProperties()) { - if (complexProperty.IsShadowProperty()) - { - continue; - } - if (complexProperty.IsCollection) { complexCollectionProperties.Add(complexProperty); } - else if (complexProperty.IsNullable) + else if (complexProperty.IsNullable && !complexProperty.IsShadowProperty()) { nullableComplexProperties.Add(complexProperty); } From bbdc285e8d59e7352f21e917125afd106bc698fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:13:39 +0000 Subject: [PATCH 8/9] Fix original value tracking for nullable complex properties - Updated CountComplexProperties to set originalValueIndex for nullable non-shadow non-collection complex properties - Simplified OriginalPropertyValues.GetValueInternal to let it throw if original value cannot be tracked - Removed comment in ArrayPropertyValues.Clone() Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/ArrayPropertyValues.cs | 1 - .../Internal/OriginalPropertyValues.cs | 40 ++++++++----------- .../Metadata/Internal/EntityTypeExtensions.cs | 5 ++- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index ababc2d8354..2b8f36f7872 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -155,7 +155,6 @@ public override PropertyValues Clone() var copies = new object[_values.Length]; Array.Copy(_values, copies, _values.Length); - // Copy null complex property tracking bool[]? flagsCopy = null; if (_nullComplexPropertyFlags != null) { diff --git a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs index 8ae2565880f..decf2412e10 100644 --- a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs @@ -60,35 +60,27 @@ protected override void SetValueInternal(IInternalEntry entry, IPropertyBase pro /// protected override object? GetValueInternal(IInternalEntry entry, IPropertyBase property) { - if (property is IComplexProperty complexProperty) + if (property is IComplexProperty { IsCollection: true } complexProperty) { - if (complexProperty.IsCollection) + var originalCollection = (IList?)entry.GetOriginalValue(property); + if (originalCollection == null) { - var originalCollection = (IList?)entry.GetOriginalValue(property); - if (originalCollection == null) - { - return null; - } - - // The stored original collection might contain references to the current elements, - // so we need to recreate it using stored values. - var clonedCollection = (IList)((IRuntimePropertyBase)complexProperty).GetIndexedCollectionAccessor() - .Create(originalCollection.Count); - for (var i = 0; i < originalCollection.Count; i++) - { - clonedCollection.Add( - originalCollection[i] == null - ? null - : GetPropertyValues(entry.GetComplexCollectionOriginalEntry(complexProperty, i)).ToObject()); - } + return null; + } - return clonedCollection; + // The stored original collection might contain references to the current elements, + // so we need to recreate it using stored values. + var clonedCollection = (IList)((IRuntimePropertyBase)complexProperty).GetIndexedCollectionAccessor() + .Create(originalCollection.Count); + for (var i = 0; i < originalCollection.Count; i++) + { + clonedCollection.Add( + originalCollection[i] == null + ? null + : GetPropertyValues(entry.GetComplexCollectionOriginalEntry(complexProperty, i)).ToObject()); } - // For non-collection complex properties, check if original value can be tracked - return ((InternalEntryBase)entry).CanHaveOriginalValue(complexProperty) - ? entry.GetOriginalValue(complexProperty) - : entry[complexProperty]; + return clonedCollection; } return entry.GetOriginalValue(property); diff --git a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs index a3479000238..96f3b31dd5b 100644 --- a/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs +++ b/src/EFCore/Metadata/Internal/EntityTypeExtensions.cs @@ -244,7 +244,10 @@ private static void CalculateCounts( { var indexes = new PropertyIndexes( index: complexProperty.IsCollection ? complexCollectionIndex++ : complexPropertyIndex++, - originalValueIndex: complexProperty.IsCollection ? originalValueIndex++ : -1, + originalValueIndex: complexProperty.IsCollection + || (complexProperty.IsNullable && !complexProperty.IsShadowProperty()) + ? originalValueIndex++ + : -1, shadowIndex: complexProperty.IsShadowProperty() ? shadowIndex++ : -1, relationshipIndex: -1, storeGenerationIndex: -1); From e3a0c0eb3efc37d47626231dc32f4f3490722a0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:56:35 +0000 Subject: [PATCH 9/9] Address PR feedback: remove default value, simplify IsNullableComplexPropertyNull, fix SetValues for null nullable complex properties - Removed default value from ArrayPropertyValues constructor parameter - Simplified IsNullableComplexPropertyNull to not check array length - Added null check in SetValues to skip properties inside null nullable complex types - Added IsPropertyInNullComplexType helper method - Kept OptionalMilk seeding to avoid breaking Reload tests - Updated tests to assert correct behavior for ToObject with null nullable complex properties Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/ChangeTracking/EntityEntry.cs | 4 +- .../Internal/ArrayPropertyValues.cs | 8 ++-- .../Internal/EntryPropertyValues.cs | 45 ++++++++++++++++--- .../Internal/OriginalPropertyValues.cs | 2 +- .../PropertyValuesTestBase.cs | 7 +-- 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/EFCore/ChangeTracking/EntityEntry.cs b/src/EFCore/ChangeTracking/EntityEntry.cs index a2d1e5812c7..f628c229c13 100644 --- a/src/EFCore/ChangeTracking/EntityEntry.cs +++ b/src/EFCore/ChangeTracking/EntityEntry.cs @@ -603,7 +603,7 @@ public virtual PropertyValues OriginalValues { var values = Finder.GetDatabaseValues(InternalEntry); - return values == null ? null : new ArrayPropertyValues(InternalEntry, values); + return values == null ? null : new ArrayPropertyValues(InternalEntry, values, null); } /// @@ -634,7 +634,7 @@ public virtual PropertyValues OriginalValues { var values = await Finder.GetDatabaseValuesAsync(InternalEntry, cancellationToken).ConfigureAwait(false); - return values == null ? null : new ArrayPropertyValues(InternalEntry, values); + return values == null ? null : new ArrayPropertyValues(InternalEntry, values, null); } /// diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index 2b8f36f7872..5b7a3e651f8 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -24,7 +24,7 @@ public class ArrayPropertyValues : PropertyValues /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values, bool[]? nullComplexPropertyFlags = null) + public ArrayPropertyValues(InternalEntryBase internalEntry, object?[] values, bool[]? nullComplexPropertyFlags) : base(internalEntry) { _values = values; @@ -86,7 +86,7 @@ public override object ToObject() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public override bool IsNullableComplexPropertyNull(int index) - => _nullComplexPropertyFlags != null && index < _nullComplexPropertyFlags.Length && _nullComplexPropertyFlags[index]; + => _nullComplexPropertyFlags != null && _nullComplexPropertyFlags[index]; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -385,7 +385,7 @@ private void SetValuesFromDictionary(IRuntimeTypeBase structuralType, var complexEntry = new InternalComplexEntry((IRuntimeComplexType)complexProperty.ComplexType, InternalEntry, i); var complexType = complexEntry.StructuralType; var values = new object?[complexType.GetFlattenedProperties().Count()]; - var complexPropertyValues = new ArrayPropertyValues(complexEntry, values); + var complexPropertyValues = new ArrayPropertyValues(complexEntry, values, null); complexPropertyValues.SetValues(itemDict); propertyValuesList.Add(complexPropertyValues); @@ -499,7 +499,7 @@ ArrayPropertyValues CreateComplexPropertyValues(object complexObject, InternalCo values[i] = getter.GetClrValue(complexObject); } - var complexPropertyValues = new ArrayPropertyValues(entry, values); + var complexPropertyValues = new ArrayPropertyValues(entry, values, null); foreach (var nestedComplexProperty in complexPropertyValues.ComplexCollectionProperties) { diff --git a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs index 4ab14887db1..880b50fd607 100644 --- a/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/EntryPropertyValues.cs @@ -317,8 +317,27 @@ public override void SetValues(PropertyValues propertyValues) { Check.NotNull(propertyValues); + var nullableComplexProperties = NullableComplexProperties; + HashSet? nullComplexProperties = null; + if (nullableComplexProperties != null) + { + for (var i = 0; i < nullableComplexProperties.Count; i++) + { + if (propertyValues.IsNullableComplexPropertyNull(i)) + { + nullComplexProperties ??= []; + nullComplexProperties.Add(nullableComplexProperties[i]); + } + } + } + foreach (var property in Properties) { + if (nullComplexProperties != null && IsPropertyInNullComplexType(property, nullComplexProperties)) + { + continue; + } + SetValueInternal(InternalEntry, property, propertyValues[property]); } @@ -327,19 +346,31 @@ public override void SetValues(PropertyValues propertyValues) SetValueInternal(InternalEntry, complexProperty, propertyValues[complexProperty]); } - var nullableComplexProperties = NullableComplexProperties; - if (nullableComplexProperties != null) + if (nullComplexProperties != null) { - for (var i = 0; i < nullableComplexProperties.Count; i++) + foreach (var complexProperty in nullComplexProperties) { - if (propertyValues.IsNullableComplexPropertyNull(i)) - { - SetValueInternal(InternalEntry, nullableComplexProperties[i], null); - } + SetValueInternal(InternalEntry, complexProperty, null); } } } + private static bool IsPropertyInNullComplexType(IProperty property, HashSet nullComplexProperties) + { + var declaringType = property.DeclaringType; + while (declaringType is IComplexType complexType) + { + if (nullComplexProperties.Contains(complexType.ComplexProperty)) + { + return true; + } + + declaringType = complexType.ComplexProperty.DeclaringType; + } + + return false; + } + /// public override void SetValues(IDictionary values) => SetValuesFromDictionary(InternalEntry, (IRuntimeTypeBase)StructuralType, Check.NotNull(values)); diff --git a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs index decf2412e10..90d08419970 100644 --- a/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/OriginalPropertyValues.cs @@ -96,7 +96,7 @@ private PropertyValues GetPropertyValues(InternalEntryBase entry) values[i] = entry.GetOriginalValue(properties[i]); } - var cloned = new ArrayPropertyValues(entry, values); + var cloned = new ArrayPropertyValues(entry, values, null); foreach (var nestedComplexProperty in cloned.ComplexCollectionProperties) { diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index 797e79e42a6..db0be471c8a 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -3185,13 +3185,14 @@ public virtual void Nullable_complex_property_with_null_value_returns_null_when_ using var context = CreateContext(); var building = context.Set().Single(b => b.Name == "Building One"); - var originalBuilding = (Building)context.Entry(building).OriginalValues.ToObject(); - Assert.NotNull(originalBuilding.OptionalMilk); - + Assert.NotNull(building.OptionalMilk); building.OptionalMilk = null; var currentBuilding = (Building)context.Entry(building).CurrentValues.ToObject(); Assert.Null(currentBuilding.OptionalMilk); + + var originalBuilding = (Building)context.Entry(building).OriginalValues.ToObject(); + Assert.NotNull(originalBuilding.OptionalMilk); } [ConditionalFact]