Ensure that, by convention, dependent properties have the same element type as principal properties (#32560)

* Ensure that, by convention, dependent properties have the same element type as principal properties

Fixes #32411

The issue here is that when a value converter is applied to a principal property, then that converter is used by the dependent properties unless something else is explicitly configured. However, this meant that when the PK has a converter but the FK does not, then the FK can get configured as a primitive collection, since it doesn't have a converter preventing this.

The fix is to add a convention that sets the element type for dependent properties to match that for principal properties.

* Update based on review

Author: ajcvickers

src/EFCore/Metadata/Conventions/ElementMappingConvention.cs

@@ -42,7 +42,7 @@ void Validate(IConventionTypeBase typeBase)
                 var typeMapping = Dependencies.TypeMappingSource.FindMapping((IProperty)property);
                 if (typeMapping is { ElementTypeMapping: not null })
                 {
-                    property.SetElementType(property.ClrType.TryGetElementType(typeof(IEnumerable<>)));
+                    property.Builder.SetElementType(property.ClrType.TryGetElementType(typeof(IEnumerable<>)));
                 }
             }
 

src/EFCore/Metadata/Conventions/ElementTypeChangedConvention.cs

@@ -0,0 +1,58 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
+
+/// <summary>
+///     A convention that reacts to changes made to element types of primitive collections.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
+/// </remarks>
+public class ElementTypeChangedConvention : IPropertyElementTypeChangedConvention, IForeignKeyAddedConvention
+{
+    /// <summary>
+    ///     Creates a new instance of <see cref="ElementTypeChangedConvention" />.
+    /// </summary>
+    /// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
+    public ElementTypeChangedConvention(ProviderConventionSetBuilderDependencies dependencies)
+    {
+        Dependencies = dependencies;
+    }
+
+    /// <summary>
+    ///     Dependencies for this service.
+    /// </summary>
+    protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }
+
+    /// <inheritdoc />
+    public void ProcessPropertyElementTypeChanged(
+        IConventionPropertyBuilder propertyBuilder,
+        IElementType? newElementType,
+        IElementType? oldElementType,
+        IConventionContext<IElementType> context)
+    {
+        var keyProperty = propertyBuilder.Metadata;
+        foreach (var key in keyProperty.GetContainingKeys())
+        {
+            var index = key.Properties.IndexOf(keyProperty);
+            foreach (var foreignKey in key.GetReferencingForeignKeys())
+            {
+                var foreignKeyProperty = foreignKey.Properties[index];
+                foreignKeyProperty.Builder.SetElementType(newElementType?.ClrType);
+            }
+        }
+    }
+
+    /// <inheritdoc />
+    public void ProcessForeignKeyAdded(
+        IConventionForeignKeyBuilder foreignKeyBuilder, IConventionContext<IConventionForeignKeyBuilder> context)
+    {
+        var foreignKeyProperties = foreignKeyBuilder.Metadata.Properties;
+        var principalKeyProperties = foreignKeyBuilder.Metadata.PrincipalKey.Properties;
+        for (var i = 0; i < foreignKeyProperties.Count; i++)
+        {
+            foreignKeyProperties[i].Builder.SetElementType(principalKeyProperties[i].GetElementType()?.ClrType);
+        }
+    }
+}

src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs

@@ -101,6 +101,7 @@ public virtual ConventionSet CreateConventionSet()
         conventionSet.Add(new QueryFilterRewritingConvention(Dependencies));
         conventionSet.Add(new RuntimeModelConvention(Dependencies));
         conventionSet.Add(new ElementMappingConvention(Dependencies));
+        conventionSet.Add(new ElementTypeChangedConvention(Dependencies));
 
         return conventionSet;
     }

test/EFCore.Cosmos.FunctionalTests/KeysWithConvertersCosmosTest.cs

@@ -146,6 +146,10 @@ public override void Can_insert_and_read_back_with_bare_class_key_and_optional_d
     public override void Can_insert_and_read_back_with_comparable_class_key_and_optional_dependents_with_shadow_FK()
         => base.Can_insert_and_read_back_with_comparable_class_key_and_optional_dependents_with_shadow_FK();
 
+    [ConditionalFact(Skip = "Issue=#26239")]
+    public override void Can_insert_and_read_back_with_enumerable_class_key_and_optional_dependents()
+        => base.Can_insert_and_read_back_with_enumerable_class_key_and_optional_dependents();
+
     public class KeysWithConvertersCosmosFixture : KeysWithConvertersFixtureBase
     {
         protected override ITestStoreFactory TestStoreFactory
@@ -537,6 +541,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                     b.Property(e => e.Id).HasConversion(GenericComparableIntClassKey.Converter);
                     b.OwnsOne(e => e.Owned);
                 });
+
+            modelBuilder.Ignore<EnumerableClassKeyPrincipal>();
+            modelBuilder.Ignore<EnumerableClassKeyOptionalDependent>();
+            modelBuilder.Ignore<EnumerableClassKeyRequiredDependent>();
         }
+
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(w => w.Ignore(CoreEventId.MappedEntityTypeIgnoredWarning)));
     }
 }

test/EFCore.InMemory.FunctionalTests/KeysWithConvertersInMemoryTest.cs

@@ -35,9 +35,26 @@ public override void Can_query_and_update_owned_entity_with_value_converter()
     public override void Can_query_and_update_owned_entity_with_int_bare_class_key()
         => base.Can_query_and_update_owned_entity_with_int_bare_class_key();
 
+    [ConditionalFact(Skip = "Issue #26238")]
+    public override void Can_insert_and_read_back_with_enumerable_class_key_and_optional_dependents()
+        => base.Can_insert_and_read_back_with_enumerable_class_key_and_optional_dependents();
+
     public class KeysWithConvertersInMemoryFixture : KeysWithConvertersFixtureBase
     {
         protected override ITestStoreFactory TestStoreFactory
             => InMemoryTestStoreFactory.Instance;
+
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(w => w.Ignore(CoreEventId.MappedEntityTypeIgnoredWarning)));
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+        {
+            base.OnModelCreating(modelBuilder, context);
+
+            // Issue #26238
+            modelBuilder.Ignore<EnumerableClassKeyPrincipal>();
+            modelBuilder.Ignore<EnumerableClassKeyOptionalDependent>();
+            modelBuilder.Ignore<EnumerableClassKeyRequiredDependent>();
+        }
     }
 }