From 514f62fb08b0a1b382a88cc1789ae77cd715e415 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 25 Mar 2024 14:11:17 +0000 Subject: [PATCH] Return flattened properties of complex types in correct order Fixes #33307 Property indexes are created such that properties of derived types are at the end--it has to be this way so that the indexes for properties on the base type remain the same for all derived types. However, when getting the flattened properties, we were returning them in a different order, such that properties with indexes for inherited type were returned in the middle of the list. This change instead returns the properties in the same order as their indexes. --- src/EFCore/Metadata/Internal/TypeBase.cs | 14 +-- src/EFCore/Metadata/RuntimeTypeBase.cs | 14 +-- .../PropertyValuesInMemoryTest.cs | 4 + .../PropertyValuesTestBase.cs | 112 +++++++++++++++++- 4 files changed, 124 insertions(+), 20 deletions(-) diff --git a/src/EFCore/Metadata/Internal/TypeBase.cs b/src/EFCore/Metadata/Internal/TypeBase.cs index 8aed3c7bc18..7b0209c193a 100644 --- a/src/EFCore/Metadata/Internal/TypeBase.cs +++ b/src/EFCore/Metadata/Internal/TypeBase.cs @@ -1315,18 +1315,18 @@ protected static IEnumerable ToEnumerable(T? element) /// The properties. public virtual IEnumerable GetFlattenedProperties() { - foreach (var property in GetProperties()) - { - yield return property; - } - - foreach (var complexProperty in GetComplexProperties()) + if (_baseType != null) { - foreach (var property in complexProperty.ComplexType.GetFlattenedProperties()) + foreach (var property in _baseType.GetFlattenedProperties()) { yield return property; } } + + foreach (var property in GetFlattenedDeclaredProperties()) + { + yield return property; + } } /// diff --git a/src/EFCore/Metadata/RuntimeTypeBase.cs b/src/EFCore/Metadata/RuntimeTypeBase.cs index aea301e943a..e6f6dc88923 100644 --- a/src/EFCore/Metadata/RuntimeTypeBase.cs +++ b/src/EFCore/Metadata/RuntimeTypeBase.cs @@ -515,18 +515,18 @@ public virtual IEnumerable GetFlattenedProperties() static IEnumerable Create(RuntimeTypeBase type) { - foreach (var property in type.GetProperties()) + if (type._baseType != null) { - yield return property; - } - - foreach (var complexProperty in type.GetComplexProperties()) - { - foreach (var property in complexProperty.ComplexType.GetFlattenedProperties()) + foreach (var property in type._baseType.GetFlattenedProperties()) { yield return property; } } + + foreach (var property in type.GetFlattenedDeclaredProperties()) + { + yield return property; + } } } diff --git a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs index 9fe0942086c..e53710149e9 100644 --- a/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs @@ -21,6 +21,10 @@ public override Task Complex_store_values_can_be_accessed_asynchronously_as_a_pr => Assert.ThrowsAsync( // In-memory database cannot query complex types () => base.Complex_store_values_can_be_accessed_asynchronously_as_a_property_dictionary_using_IProperty()); + public override Task Values_can_be_reloaded_from_database_for_entity_in_any_state_with_inheritance(EntityState state, bool async) + => Assert.ThrowsAnyAsync( // In-memory database cannot query complex types + () => base.Values_can_be_reloaded_from_database_for_entity_in_any_state_with_inheritance(state, async)); + public class PropertyValuesInMemoryFixture : PropertyValuesFixtureBase { public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs index a02f56cde4e..ec42fcf5f7c 100644 --- a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs +++ b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs @@ -1282,6 +1282,53 @@ public async Task Reload_when_entity_deleted_in_store_can_happen_for_any_state(E Assert.Contains(office, building.Offices); } + [ConditionalTheory] + [InlineData(EntityState.Unchanged, true)] + [InlineData(EntityState.Unchanged, false)] + [InlineData(EntityState.Modified, true)] + [InlineData(EntityState.Modified, false)] + [InlineData(EntityState.Added, true)] + [InlineData(EntityState.Added, false)] + [InlineData(EntityState.Deleted, true)] + [InlineData(EntityState.Deleted, false)] + [InlineData(EntityState.Detached, true)] + [InlineData(EntityState.Detached, false)] + public virtual async Task Values_can_be_reloaded_from_database_for_entity_in_any_state_with_inheritance(EntityState state, bool async) + { + using var context = CreateContext(); + var supplier = context.Set().Single(); + var customer = context.Set().Single(); + + supplier.Name = "X"; + supplier.Foo = "Z"; + customer.Name = "Y"; + customer.Bar = 77; + customer.Address.Street = "New Road"; + supplier.Address.Street = "New Lane"; + + context.Entry(supplier).State = state; + context.Entry(customer).State = state; + + if (async) + { + await context.Entry(supplier).ReloadAsync(); + await context.Entry(customer).ReloadAsync(); + } + else + { + context.Entry(supplier).Reload(); + context.Entry(customer).Reload(); + } + + Assert.Equal("Bar", customer.Name); + Assert.Equal(11, customer.Bar); + Assert.Equal("Two", customer.Address.Street); + + Assert.Equal("Foo", supplier.Name); + Assert.Equal("F", supplier.Foo); + Assert.Equal("One", supplier.Address.Street); + } + [ConditionalFact] public virtual void Current_values_can_be_set_from_an_object_using_generic_dictionary() => TestGenericObjectSetValues(e => e.CurrentValues, (e, n) => e.Property(n).CurrentValue!); @@ -2301,6 +2348,31 @@ public string NoSetter public required Milk Milk { get; set; } } + [ComplexType] + public class Address33307 + { + public required string Street { get; set; } + public double? Altitude { get; set; } + public int? Number { get; set; } + } + + public abstract class Contact33307 + { + public int Id { get; set; } + public required string Name { get; set; } + public required Address33307 Address { get; set; } + } + + public class Supplier33307 : Contact33307 + { + public string? Foo { get; set; } + } + + public class Customer33307 : Contact33307 + { + public int Bar { get; set; } + } + protected struct Culture { public string Species { get; set; } @@ -2555,6 +2627,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con }); }); }); + + modelBuilder.Entity(); + modelBuilder.Entity(); + modelBuilder.Entity(); } protected override Task SeedAsync(PoolableDbContext context) @@ -2678,6 +2754,30 @@ protected override Task SeedAsync(PoolableDbContext context) Assert.True((bool)joinEntry.Entity["InitializedCalled"]); } + context.Add(new Supplier33307 + { + Name = "Foo", + Address = new() + { + Street = "One", + Altitude = Math.PI, + Number = 42, + }, + Foo = "F" + }); + + context.Add(new Customer33307 + { + Name = "Bar", + Address = new() + { + Street = "Two", + Altitude = Math.E, + Number = 42, + }, + Bar = 11 + }); + return context.SaveChangesAsync(); } } @@ -2695,9 +2795,9 @@ public object CreatedInstance(MaterializationInterceptionData materializationDat { joinEntity["CreatedCalled"] = true; } - else + else if (entity is PropertyValuesBase propertyValuesBase) { - ((PropertyValuesBase)entity).CreatedCalled = true; + propertyValuesBase.CreatedCalled = true; } return entity; @@ -2712,9 +2812,9 @@ public InterceptionResult InitializingInstance( { joinEntity["InitializingCalled"] = true; } - else + else if (entity is PropertyValuesBase propertyValuesBase) { - ((PropertyValuesBase)entity).InitializingCalled = true; + propertyValuesBase.InitializingCalled = true; } return result; @@ -2726,9 +2826,9 @@ public object InitializedInstance(MaterializationInterceptionData materializatio { joinEntity["InitializedCalled"] = true; } - else + else if (entity is PropertyValuesBase propertyValuesBase) { - ((PropertyValuesBase)entity).InitializedCalled = true; + propertyValuesBase.InitializedCalled = true; } return entity;