diff --git a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs index 5b7a3e651f8..e1a3c619019 100644 --- a/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs +++ b/src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs @@ -18,6 +18,9 @@ public class ArrayPropertyValues : PropertyValues private readonly List?[] _complexCollectionValues; private readonly bool[]? _nullComplexPropertyFlags; + private static readonly bool UseOldBehavior37516 = + AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37516", out var enabled) && enabled; + /// /// 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 @@ -50,7 +53,7 @@ public override object ToObject() if (_nullComplexPropertyFlags[i]) { var complexProperty = NullableComplexProperties[i]; - structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, null); + structuralObject = SetNestedComplexPropertyValue(structuralObject, complexProperty, null); } } } @@ -68,7 +71,7 @@ public override object ToObject() !complexProperty.IsShadowProperty(), $"Shadow complex property {complexProperty.Name} is not supported. Issue #31243"); var list = (IList)((IRuntimeComplexProperty)complexProperty).GetIndexedCollectionAccessor().Create(propertyValuesList.Count); - structuralObject = ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, list); + structuralObject = SetNestedComplexPropertyValue(structuralObject, complexProperty, list); foreach (var propertyValues in propertyValuesList) { @@ -79,6 +82,33 @@ public override object ToObject() return structuralObject; } + private object SetNestedComplexPropertyValue(object structuralObject, IComplexProperty complexProperty, object? value) + { + return UseOldBehavior37516 + ? ((IRuntimeComplexProperty)complexProperty).GetSetter().SetClrValue(structuralObject, value) + : SetValueRecursively(structuralObject, complexProperty.GetChainToComplexProperty(fromEntity: false), 0, value); + + static object SetValueRecursively(object instance, IReadOnlyList chain, int index, object? value) + { + var currentProperty = (IRuntimeComplexProperty)chain[index]; + if (index == chain.Count - 1) + { + return currentProperty.GetSetter().SetClrValue(instance, value); + } + + var child = currentProperty.GetGetter().GetClrValue(instance); + if (child == null) + { + return instance; + } + + var updated = SetValueRecursively(child, chain, index + 1, value); + // Need to update the child value as well because it could be a value type + // TODO: Improve this, see #36041 + return currentProperty.GetSetter().SetClrValue(instance, updated); + } + } + /// /// 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 @@ -495,20 +525,44 @@ ArrayPropertyValues CreateComplexPropertyValues(object complexObject, InternalCo for (var i = 0; i < properties.Count; i++) { var property = properties[i]; - var getter = property.GetGetter(); - values[i] = getter.GetClrValue(complexObject); + var targetObject = NavigateToDeclaringType(complexObject, property.DeclaringType, complexType); + values[i] = targetObject == null ? null : property.GetGetter().GetClrValue(targetObject); } var complexPropertyValues = new ArrayPropertyValues(entry, values, null); foreach (var nestedComplexProperty in complexPropertyValues.ComplexCollectionProperties) { - var nestedCollection = (IList?)nestedComplexProperty.GetGetter().GetClrValue(complexObject); - var propertyValuesList = GetComplexCollectionPropertyValues(nestedComplexProperty, nestedCollection); - complexPropertyValues.SetComplexCollectionValue(nestedComplexProperty, propertyValuesList); + var targetObject = NavigateToDeclaringType(complexObject, nestedComplexProperty.DeclaringType, complexType); + var nestedCollection = targetObject == null ? null : (IList?)nestedComplexProperty.GetGetter().GetClrValue(targetObject); + var nestedPropertyValuesList = GetComplexCollectionPropertyValues(nestedComplexProperty, nestedCollection); + complexPropertyValues.SetComplexCollectionValue(nestedComplexProperty, nestedPropertyValuesList); } return complexPropertyValues; } + + static object? NavigateToDeclaringType(object root, ITypeBase declaringType, IRuntimeTypeBase rootType) + { + if (declaringType == rootType + || UseOldBehavior37516) + { + return root; + } + + if (declaringType is not IComplexType ct) + { + return root; + } + + var chain = ct.ComplexProperty.GetChainToComplexProperty(fromEntity: false); + object? target = root; + for (var i = 0; i < chain.Count && target != null; i++) + { + target = chain[i].GetGetter().GetClrValue(target); + } + + return target; + } } } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs index fe3def8e7ae..f701c8bedef 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/PropertyValuesTest.cs @@ -1,28 +1,33 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#nullable enable + +using System.Collections; + namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal; public class PropertyValuesTest { [ConditionalFact] - public void Can_safely_get_originalvalue_and_currentvalue_with_tryget() + public void Can_safely_get_originalvalue_and_currentvalue_with_TryGetValue() { - // Arrange + var stateManager = CreateStateManager(mb => mb.Entity()); + const string NameValue = "Simple Name"; const string NewNameValue = "A New Name"; - using var ctx = new CurrentValuesDb(); - var entity = ctx.SimpleEntities.Add(new SimpleEntity { Name = NameValue }); - ctx.SaveChanges(); + var entity = new SimpleEntity { Name = NameValue }; - entity.Entity.Name = NewNameValue; + var entityEntry = stateManager.GetOrCreateEntry(entity); + entityEntry.SetEntityState(EntityState.Unchanged); + var entry = new EntityEntry(entityEntry); - // Act - var current = entity.CurrentValues.TryGetValue("Name", out var currentName); - var original = entity.OriginalValues.TryGetValue("Name", out var originalName); + entity.Name = NewNameValue; + + var current = entry.CurrentValues.TryGetValue("Name", out var currentName); + var original = entry.OriginalValues.TryGetValue("Name", out var originalName); - // Assert Assert.True(current); Assert.True(original); @@ -31,23 +36,24 @@ public void Can_safely_get_originalvalue_and_currentvalue_with_tryget() } [ConditionalFact] - public void Should_not_throw_error_when_property_do_not_exist() + public void TryGetValue_should_not_throw_error_when_property_does_not_exist() { - // Arrange + var stateManager = CreateStateManager(mb => mb.Entity()); + const string NameValue = "Simple Name"; const string NewNameValue = "A New Name"; - using var ctx = new CurrentValuesDb(); - var entity = ctx.SimpleEntities.Add(new SimpleEntity { Name = NameValue }); - ctx.SaveChanges(); + var entity = new SimpleEntity { Name = NameValue }; - entity.Entity.Name = NewNameValue; + var entityEntry = stateManager.GetOrCreateEntry(entity); + entityEntry.SetEntityState(EntityState.Unchanged); + var entry = new EntityEntry(entityEntry); - // Act - var current = entity.CurrentValues.TryGetValue("Non_Existent_Property", out var non_existent_current); - var original = entity.OriginalValues.TryGetValue("Non_Existent_Property", out var non_existent_original); + entity.Name = NewNameValue; + + var current = entry.CurrentValues.TryGetValue("Non_Existent_Property", out var non_existent_current); + var original = entry.OriginalValues.TryGetValue("Non_Existent_Property", out var non_existent_original); - // Assert Assert.False(current); Assert.False(original); @@ -55,22 +61,361 @@ public void Should_not_throw_error_when_property_do_not_exist() Assert.Null(non_existent_original); } - private class CurrentValuesDb : DbContext + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_null_complex_property(bool? useOriginalValues) + { + var job = new Job { Id = 1, Name = "Job with No Error" }; + + var result = GetToObjectResult(job, useOriginalValues); + + Assert.Equal("Job with No Error", result.Name); + Assert.Null(result.Error); + } + + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_all_nested_complex_properties_populated(bool? useOriginalValues) + { + var job = new Job + { + Id = 1, + Name = "Job with Error + Inner Errors", + Error = new RootJobError + { + Code = "500", + Message = "Internal Server Error", + InnerError = new JobError + { + Code = "501", + Message = "Not Implemented", + InnerError = new LeafJobError + { + Code = "502", + Message = "Bad Gateway" + } + } + } + }; + + var result = GetToObjectResult(job, useOriginalValues); + + Assert.Equal("Job with Error + Inner Errors", result.Name); + Assert.NotNull(result.Error); + Assert.Equal("500", result.Error!.Code); + Assert.NotNull(result.Error.InnerError); + Assert.Equal("501", result.Error.InnerError!.Code); + Assert.NotNull(result.Error.InnerError.InnerError); + Assert.Equal("502", result.Error.InnerError.InnerError!.Code); + } + + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_null_nested_complex_property(bool? useOriginalValues) { - // ReSharper disable once UnusedAutoPropertyAccessor.Local - public DbSet SimpleEntities { get; set; } + var job = new Job + { + Id = 1, + Name = "Job with Error only", + Error = new RootJobError + { + Code = "400", + Message = "Bad Request" + } + }; + + var result = GetToObjectResult(job, useOriginalValues); - protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) - => optionsBuilder.UseInMemoryDatabase("DB1"); + Assert.Equal("Job with Error only", result.Name); + Assert.NotNull(result.Error); + Assert.Equal("400", result.Error!.Code); + Assert.Null(result.Error.InnerError); + } + + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_complex_collection_containing_nested_nullable_complex_properties( + bool? useOriginalValues) + { + var job = new Job + { + Id = 1, + Name = "Test Job", + Errors = + [ + new RootJobError { Code = "400", Message = "Bad Request", InnerError = null }, + new RootJobError + { + Code = "500", + Message = "Server Error", + InnerError = new JobError { Code = "501", Message = "Not Implemented", InnerError = null } + }, + new RootJobError + { + Code = "503", + Message = "Service Unavailable", + InnerError = new JobError + { + Code = "504", + Message = "Gateway Timeout", + InnerError = new LeafJobError { Code = "505", Message = "Internal Error" } + } + } + ] + }; + + var result = GetToObjectResult(job, useOriginalValues); + + Assert.Equal("Test Job", result.Name); + Assert.Null(result.Error); + Assert.NotNull(result.Errors); + Assert.Equal(3, result.Errors.Count); + + Assert.Equal("400", result.Errors[0].Code); + Assert.Equal("Bad Request", result.Errors[0].Message); + Assert.Null(result.Errors[0].InnerError); + + Assert.Equal("500", result.Errors[1].Code); + Assert.Equal("Server Error", result.Errors[1].Message); + Assert.NotNull(result.Errors[1].InnerError); + Assert.Equal("501", result.Errors[1].InnerError!.Code); + Assert.Null(result.Errors[1].InnerError!.InnerError); + + Assert.Equal("503", result.Errors[2].Code); + Assert.NotNull(result.Errors[2].InnerError); + Assert.Equal("504", result.Errors[2].InnerError!.Code); + Assert.NotNull(result.Errors[2].InnerError!.InnerError); + Assert.Equal("505", result.Errors[2].InnerError!.InnerError!.Code); + } + + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_null_complex_property_in_complex_collection_in_complex_collection( + bool? useOriginalValues) + { + var job = new Job + { + Id = 1, + Name = "Double Collection Test", + Errors = + [ + new RootJobError + { + Code = "400", + Message = "Bad Request", + InnerErrors = + [ + new JobError { Code = "401", Message = "Unauthorized", InnerError = null }, + new JobError + { + Code = "402", + Message = "Payment Required", + InnerError = new LeafJobError { Code = "403", Message = "Forbidden" } + } + ] + } + ] + }; + + var result = GetToObjectResult(job, useOriginalValues); + + Assert.Equal("Double Collection Test", result.Name); + Assert.Null(result.Error); + Assert.NotNull(result.Errors); + Assert.Single(result.Errors); + + var error = result.Errors[0]; + Assert.Equal("400", error.Code); + Assert.Null(error.InnerError); + Assert.NotNull(error.InnerErrors); + Assert.Equal(2, error.InnerErrors.Count); + + Assert.Equal("401", error.InnerErrors[0].Code); + Assert.Null(error.InnerErrors[0].InnerError); + + Assert.Equal("402", error.InnerErrors[1].Code); + Assert.NotNull(error.InnerErrors[1].InnerError); + Assert.Equal("403", error.InnerErrors[1].InnerError!.Code); + } + + [ConditionalTheory] + [ClassData(typeof(DataGenerator))] + public void ToObject_with_null_complex_property_in_complex_collection_element_in_complex_property( + bool? useOriginalValues) + { + var job = new Job + { + Id = 1, + Name = "Nested Collection Test", + Error = new RootJobError + { + Code = "500", + Message = "Server Error", + InnerErrors = + [ + new JobError { Code = "501", Message = "Not Implemented", InnerError = null }, + new JobError + { + Code = "502", + Message = "Bad Gateway", + InnerError = new LeafJobError { Code = "503", Message = "Service Unavailable" } + } + ] + } + }; + + var result = GetToObjectResult(job, useOriginalValues); + + Assert.Equal("Nested Collection Test", result.Name); + Assert.NotNull(result.Error); + Assert.Equal("500", result.Error!.Code); + Assert.Null(result.Error.InnerError); + Assert.NotNull(result.Error.InnerErrors); + Assert.Equal(2, result.Error.InnerErrors.Count); + + Assert.Equal("501", result.Error.InnerErrors[0].Code); + Assert.Null(result.Error.InnerErrors[0].InnerError); + + Assert.Equal("502", result.Error.InnerErrors[1].Code); + Assert.NotNull(result.Error.InnerErrors[1].InnerError); + Assert.Equal("503", result.Error.InnerErrors[1].InnerError!.Code); + } + + #region Helpers + + private static IStateManager CreateStateManager(Action buildModel) + { + var modelBuilder = InMemoryTestHelpers.Instance.CreateConventionBuilder(); + buildModel(modelBuilder); + var model = modelBuilder.FinalizeModel(); + return CreateStateManager(model); + } + + private static IStateManager CreateStateManager(IModel model) + { + var serviceProvider = InMemoryTestHelpers.Instance.CreateContextServices(model); + return serviceProvider.GetRequiredService(); + } + + private static void BuildJobModel(ModelBuilder mb) + => mb.Entity(b => + { + b.ComplexProperty(e => e.Error, eb => + { + eb.ComplexProperty(ne => ne.InnerError, neb => neb.ComplexProperty(dne => dne.InnerError)); + eb.ComplexCollection(ne => ne.InnerErrors, neb => neb.ComplexProperty(dne => dne.InnerError)); + }); + b.ComplexCollection(e => e.Errors, eb => + { + eb.ComplexProperty(ne => ne.InnerError, neb => neb.ComplexProperty(dne => dne.InnerError)); + eb.ComplexCollection(ne => ne.InnerErrors, neb => neb.ComplexProperty(dne => dne.InnerError)); + }); + }); + + private static Job GetToObjectResult(Job job, bool? useOriginalValues) + { + var stateManager = CreateStateManager(BuildJobModel); + + var internalEntry = stateManager.GetOrCreateEntry(job); + internalEntry.SetEntityState(EntityState.Unchanged); + var entry = new EntityEntry(internalEntry); + var targetValues = useOriginalValues == null + ? entry.CurrentValues.Clone() + : useOriginalValues.Value + ? entry.OriginalValues + : entry.CurrentValues; + + return (Job)targetValues.ToObject(); + } + + private static Dictionary ToDictionary(object obj, ITypeBase structuralType) + { + var dict = new Dictionary(); + var complexPropertyNames = new HashSet(); + + foreach (var complexProperty in structuralType.GetComplexProperties()) + { + complexPropertyNames.Add(complexProperty.Name); + var pi = obj.GetType().GetProperty(complexProperty.Name); + if (pi == null) + { + continue; + } + + var value = pi.GetValue(obj); + if (value == null) + { + dict[complexProperty.Name] = null; + } + else if (complexProperty.IsCollection && value is IList list) + { + var items = new List?>(); + for (var i = 0; i < list.Count; i++) + { + items.Add(list[i] == null ? null : ToDictionary(list[i]!, complexProperty.ComplexType)); + } + + dict[complexProperty.Name] = items; + } + else if (!complexProperty.IsCollection) + { + dict[complexProperty.Name] = ToDictionary(value, complexProperty.ComplexType); + } + } + + foreach (var pi in obj.GetType().GetProperties()) + { + if (!complexPropertyNames.Contains(pi.Name) && !dict.ContainsKey(pi.Name)) + { + dict[pi.Name] = pi.GetValue(obj); + } + } + + return dict; + } + + #endregion + + #region Model + + private class Job + { + public int Id { get; set; } + public string? Name { get; set; } + public RootJobError? Error { get; set; } + public List Errors { get; set; } = null!; + } + + // Change these to structs once #31411 is fixed + private class RootJobError + { + public string? Code { get; set; } + public string? Message { get; set; } + public JobError? InnerError { get; set; } + public List InnerErrors { get; set; } = null!; + } + + private class JobError + { + public string? Code { get; set; } + public string? Message { get; set; } + public LeafJobError? InnerError { get; set; } + public List InnerErrors { get; set; } = null!; + } + + private class LeafJobError + { + public string? Code { get; set; } + public string? Message { get; set; } } private class SimpleEntity { public int Id { get; set; } - public string Name { get; set; } + public string? Name { get; set; } - public IEnumerable RelatedEntities { get; set; } + public IEnumerable RelatedEntities { get; set; } = null!; } private class RelatedEntity @@ -79,8 +424,10 @@ private class RelatedEntity public int? SimpleEntityId { get; set; } - public SimpleEntity SimpleEntity { get; set; } + public SimpleEntity? SimpleEntity { get; set; } - public string Name { get; set; } + public string? Name { get; set; } } + + #endregion }