[release/8.0] Ensure that, by convention, dependent properties have the same element type as principal properties (#32582)

* 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

* Qwerk

Author: ajcvickers

All.sln.DotSettings

@@ -268,6 +268,7 @@ The .NET Foundation licenses this file to you under the MIT license.
 	<s:Boolean x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=FileEF4F00E20178B341995BD2EFE53739B5/@KeyIndexDefined">True</s:Boolean>
 	<s:Double x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=FileEF4F00E20178B341995BD2EFE53739B5/RelativePriority/@EntryValue">2</s:Double>
 	<s:String x:Key="/Default/Environment/PerformanceGuide/SwitchBehaviour/=VsBulb/@EntryIndexedValue">DO_NOTHING</s:String>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EFeature_002EServices_002ECodeCleanup_002EFileHeader_002EFileHeaderSettingsMigrate/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EFeature_002EServices_002EDaemon_002ESettings_002EMigration_002ESwaWarningsModeSettingsMigrate/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>

src/EFCore/Metadata/Conventions/ConventionSet.cs

@@ -905,6 +905,12 @@ public virtual void Add(IConvention convention)
         {
             PropertyRemovedConventions.Add(propertyRemovedConvention);
         }
+
+        if (!ElementTypeChangedConvention.UseOldBehavior32411
+            && convention is IPropertyElementTypeChangedConvention elementTypeChangedConvention)
+        {
+            PropertyElementTypeChangedConventions.Add(elementTypeChangedConvention);
+        }
     }
 
     /// <summary>

src/EFCore/Metadata/Conventions/ElementMappingConvention.cs

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

src/EFCore/Metadata/Conventions/ElementTypeChangedConvention.cs

@@ -0,0 +1,61 @@
+// 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
+{
+    internal static readonly bool UseOldBehavior32411 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32411", out var enabled32411) && enabled32411;
+
+    /// <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

@@ -102,6 +102,11 @@ public virtual ConventionSet CreateConventionSet()
         conventionSet.Add(new RuntimeModelConvention(Dependencies));
         conventionSet.Add(new ElementMappingConvention(Dependencies));
 
+        if (!ElementTypeChangedConvention.UseOldBehavior32411)
+        {
+            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>();
+        }
     }
 }