Mark owned collections composite key properties use generated values …

…even after the first two usages

Fixes #20932

The first time a CLR type is used as an owned collection it is not weak. Value generation is set correctly.
The second time, it becomes weak, and value generation is ste correction for both usages.
The first time, it is already weak, but the FK is marked as IsOwnersShip after value generation has been set.

The fix to run the value generation convention again when FK ownership changes.

src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs

@@ -8,7 +8,6 @@
 using System.Linq;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
-using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.InMemory.Internal;
 using Microsoft.EntityFrameworkCore.InMemory.ValueGeneration.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
@@ -26,7 +25,6 @@ namespace Microsoft.EntityFrameworkCore.InMemory.Storage.Internal
     /// </summary>
     public class InMemoryTable<TKey> : IInMemoryTable
     {
-        // WARNING: The in-memory provider is using EF internal code here. This should not be copied by other providers. See #15096
         private readonly IPrincipalKeyValueFactory<TKey> _keyValueFactory;
         private readonly bool _sensitiveLoggingEnabled;
         private readonly Dictionary<TKey, object[]> _rows;
@@ -45,7 +43,6 @@ public InMemoryTable([NotNull] IEntityType entityType, [CanBeNull] IInMemoryTabl
         {
             EntityType = entityType;
             BaseTable = baseTable;
-            // WARNING: The in-memory provider is using EF internal code here. This should not be copied by other providers. See #15096
             _keyValueFactory = entityType.FindPrimaryKey().GetPrincipalKeyValueFactory<TKey>();
             _sensitiveLoggingEnabled = sensitiveLoggingEnabled;
             _rows = new Dictionary<TKey, object[]>(_keyValueFactory.EqualityComparer);
@@ -314,9 +311,8 @@ public virtual void BumpValueGenerators(object[] row)
             }
         }
 
-        // WARNING: The in-memory provider is using EF internal code here. This should not be copied by other providers. See #15096
         private TKey CreateKey(IUpdateEntry entry)
-            => _keyValueFactory.CreateFromCurrentValues((InternalEntityEntry)entry);
+            => _keyValueFactory.CreateFromCurrentValues(entry);
 
         private static object SnapshotValue(IProperty property, ValueComparer comparer, IUpdateEntry entry)
         {

src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs

@@ -6,7 +6,6 @@
 using System.Linq;
 using System.Text;
 using JetBrains.Annotations;
-using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Metadata;
@@ -491,7 +490,7 @@ private Dictionary<IKeyValueIndex, List<ModificationCommand>> CreateKeyValuePred
 
                             var principalKeyValue = _keyValueIndexFactorySource
                                 .GetKeyValueIndexFactory(foreignKey.PrincipalKey)
-                                .CreatePrincipalKeyValue((InternalEntityEntry)entry, foreignKey);
+                                .CreatePrincipalKeyValue(entry, foreignKey);
 
                             if (principalKeyValue != null)
                             {
@@ -527,7 +526,7 @@ private Dictionary<IKeyValueIndex, List<ModificationCommand>> CreateKeyValuePred
 
                             var dependentKeyValue = _keyValueIndexFactorySource
                                 .GetKeyValueIndexFactory(foreignKey.PrincipalKey)
-                                .CreateDependentKeyValueFromOriginalValues((InternalEntityEntry)entry, foreignKey);
+                                .CreateDependentKeyValueFromOriginalValues(entry, foreignKey);
 
                             if (dependentKeyValue != null)
                             {
@@ -577,7 +576,7 @@ private void AddForeignKeyEdges(
 
                                 var dependentKeyValue = _keyValueIndexFactorySource
                                     .GetKeyValueIndexFactory(foreignKey.PrincipalKey)
-                                    .CreateDependentKeyValue((InternalEntityEntry)entry, foreignKey);
+                                    .CreateDependentKeyValue(entry, foreignKey);
                                 if (dependentKeyValue == null)
                                 {
                                     continue;
@@ -605,7 +604,7 @@ private void AddForeignKeyEdges(
 
                                 var principalKeyValue = _keyValueIndexFactorySource
                                     .GetKeyValueIndexFactory(foreignKey.PrincipalKey)
-                                    .CreatePrincipalKeyValueFromOriginalValues((InternalEntityEntry)entry, foreignKey);
+                                    .CreatePrincipalKeyValueFromOriginalValues(entry, foreignKey);
                                 if (principalKeyValue != null)
                                 {
                                     AddMatchingPredecessorEdge(

src/EFCore.Relational/Update/Internal/IKeyValueIndexFactory.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using JetBrains.Annotations;
-using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 
 namespace Microsoft.EntityFrameworkCore.Update.Internal
@@ -21,30 +20,30 @@ public interface IKeyValueIndexFactory
         ///     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.
         /// </summary>
-        IKeyValueIndex CreatePrincipalKeyValue([NotNull] InternalEntityEntry entry, [NotNull] IForeignKey foreignKey);
+        IKeyValueIndex CreatePrincipalKeyValue([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
 
         /// <summary>
         ///     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.
         /// </summary>
-        IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues([NotNull] InternalEntityEntry entry, [NotNull] IForeignKey foreignKey);
+        IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
 
         /// <summary>
         ///     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.
         /// </summary>
-        IKeyValueIndex CreateDependentKeyValue([NotNull] InternalEntityEntry entry, [NotNull] IForeignKey foreignKey);
+        IKeyValueIndex CreateDependentKeyValue([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
 
         /// <summary>
         ///     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.
         /// </summary>
-        IKeyValueIndex CreateDependentKeyValueFromOriginalValues([NotNull] InternalEntityEntry entry, [NotNull] IForeignKey foreignKey);
+        IKeyValueIndex CreateDependentKeyValueFromOriginalValues([NotNull] IUpdateEntry entry, [NotNull] IForeignKey foreignKey);
     }
 }

src/EFCore.Relational/Update/Internal/KeyValueIndexFactory.cs

@@ -3,7 +3,6 @@
 
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
-using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 
 namespace Microsoft.EntityFrameworkCore.Update.Internal
@@ -33,7 +32,7 @@ public KeyValueIndexFactory([NotNull] IPrincipalKeyValueFactory<TKey> principalK
         ///     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.
         /// </summary>
-        public virtual IKeyValueIndex CreatePrincipalKeyValue(InternalEntityEntry entry, IForeignKey foreignKey)
+        public virtual IKeyValueIndex CreatePrincipalKeyValue(IUpdateEntry entry, IForeignKey foreignKey)
             => new KeyValueIndex<TKey>(
                 foreignKey,
                 _principalKeyValueFactory.CreateFromCurrentValues(entry),
@@ -46,7 +45,7 @@ public virtual IKeyValueIndex CreatePrincipalKeyValue(InternalEntityEntry entry,
         ///     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.
         /// </summary>
-        public virtual IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues(InternalEntityEntry entry, IForeignKey foreignKey)
+        public virtual IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues(IUpdateEntry entry, IForeignKey foreignKey)
             => new KeyValueIndex<TKey>(
                 foreignKey,
                 _principalKeyValueFactory.CreateFromOriginalValues(entry),
@@ -59,7 +58,7 @@ public virtual IKeyValueIndex CreatePrincipalKeyValueFromOriginalValues(Internal
         ///     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.
         /// </summary>
-        public virtual IKeyValueIndex CreateDependentKeyValue(InternalEntityEntry entry, IForeignKey foreignKey)
+        public virtual IKeyValueIndex CreateDependentKeyValue(IUpdateEntry entry, IForeignKey foreignKey)
             => foreignKey.GetDependentKeyValueFactory<TKey>().TryCreateFromCurrentValues(entry, out var keyValue)
                 ? new KeyValueIndex<TKey>(foreignKey, keyValue, _principalKeyValueFactory.EqualityComparer, fromOriginalValues: false)
                 : null;
@@ -70,7 +69,7 @@ public virtual IKeyValueIndex CreateDependentKeyValue(InternalEntityEntry entry,
         ///     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.
         /// </summary>
-        public virtual IKeyValueIndex CreateDependentKeyValueFromOriginalValues(InternalEntityEntry entry, IForeignKey foreignKey)
+        public virtual IKeyValueIndex CreateDependentKeyValueFromOriginalValues(IUpdateEntry entry, IForeignKey foreignKey)
             => foreignKey.GetDependentKeyValueFactory<TKey>().TryCreateFromOriginalValues(entry, out var keyValue)
                 ? new KeyValueIndex<TKey>(foreignKey, keyValue, _principalKeyValueFactory.EqualityComparer, fromOriginalValues: true)
                 : null;

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -279,7 +279,7 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
                         else if ((updating && property.GetAfterSaveBehavior() == PropertySaveBehavior.Save)
                             || (!isKey && nonMainEntry))
                         {
-                            writeValue = columnPropagator?.TryPropagate(property, (InternalEntityEntry)entry)
+                            writeValue = columnPropagator?.TryPropagate(property, entry)
                                 ?? entry.IsModified(property);
                         }
                     }
@@ -421,6 +421,7 @@ public bool TryPropagate(IProperty property, IUpdateEntry entry)
                         || (entry.EntityState == EntityState.Modified && !entry.IsModified(property))
                         || (entry.EntityState == EntityState.Added && Equals(_originalValue, entry.GetCurrentValue(property)))))
                 {
+                    // Should be `entry.SetStoreGeneratedValue(property, _currentValue);` but see issue #21041
                     ((InternalEntityEntry)entry)[property] = _currentValue;
 
                     return false;

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

@@ -164,6 +164,7 @@ public virtual ConventionSet CreateConventionSet()
             conventionSet.ForeignKeyOwnershipChangedConventions.Add(new NavigationEagerLoadingConvention(Dependencies));
             conventionSet.ForeignKeyOwnershipChangedConventions.Add(keyDiscoveryConvention);
             conventionSet.ForeignKeyOwnershipChangedConventions.Add(relationshipDiscoveryConvention);
+            conventionSet.ForeignKeyOwnershipChangedConventions.Add(valueGeneratorConvention);
 
             conventionSet.ModelInitializedConventions.Add(new DbSetFindingConvention(Dependencies));
 

src/EFCore/Metadata/Conventions/ValueGenerationConvention.cs

@@ -19,7 +19,8 @@ public class ValueGenerationConvention :
         IForeignKeyAddedConvention,
         IForeignKeyRemovedConvention,
         IForeignKeyPropertiesChangedConvention,
-        IEntityTypeBaseTypeChangedConvention
+        IEntityTypeBaseTypeChangedConvention,
+        IForeignKeyOwnershipChangedConvention
     {
         /// <summary>
         ///     Creates a new instance of <see cref="ValueGenerationConvention" />.
@@ -205,5 +206,19 @@ private static bool CanBeGenerated(IProperty property)
                     && propertyType != typeof(byte))
                 || propertyType == typeof(Guid);
         }
+
+        /// <summary>
+        ///     Called after the ownership value for a foreign key is changed.
+        /// </summary>
+        /// <param name="relationshipBuilder"> The builder for the foreign key. </param>
+        /// <param name="context"> Additional information associated with convention execution. </param>
+        public virtual void ProcessForeignKeyOwnershipChanged(
+            IConventionForeignKeyBuilder relationshipBuilder, IConventionContext<bool?> context)
+        {
+            foreach (var property in relationshipBuilder.Metadata.DeclaringEntityType.GetProperties())
+            {
+                property.Builder.ValueGenerated(GetValueGenerated(property));
+            }
+        }
     }
 }

test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs

@@ -2266,6 +2266,104 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder)
             }
         }
 
+        [ConditionalFact]
+        public void Indexes_for_owned_collection_types_are_calculated_correctly()
+        {
+            using var context = new SideBySide();
+            var model = context.Model;
+
+            var parent = model.FindEntityType(typeof(Parent1Entity));
+            var indexes = GetIndexes(parent.GetPropertiesAndNavigations());
+            Assert.Equal(2, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, -1, 0, 0, 0), indexes[nameof(Parent1Entity.Id)]);
+            Assert.Equal((0, -1, -1, -1, 1), indexes[nameof(Parent1Entity.Children)]);
+
+            indexes = GetIndexes(model.FindEntityType(typeof(ChildEntity), nameof(Parent1Entity.Children), parent).GetProperties());
+            Assert.Equal(3, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, 0, 0, 0, 0), indexes[nameof(Parent1Entity) + "Id"]);
+            Assert.Equal((1, 1, 1, 1, 1), indexes["Id"]);
+            Assert.Equal((2, -1, 2, -1, -1), indexes[nameof(ChildEntity.Name)]);
+
+            parent = model.FindEntityType(typeof(Parent2Entity));
+            indexes = GetIndexes(parent.GetPropertiesAndNavigations());
+            Assert.Equal(2, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, -1, 0, 0, 0), indexes[nameof(Parent2Entity.Id)]);
+            Assert.Equal((0, -1, -1, -1, 1), indexes[nameof(Parent2Entity.Children)]);
+
+            indexes = GetIndexes(model.FindEntityType(typeof(ChildEntity), nameof(Parent2Entity.Children), parent).GetProperties());
+            Assert.Equal(3, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, 0, 0, 0, 0), indexes[nameof(Parent2Entity) + "Id"]);
+            Assert.Equal((1, 1, 1, 1, 1), indexes["Id"]);
+            Assert.Equal((2, -1, 2, -1, -1), indexes[nameof(ChildEntity.Name)]);
+
+            parent = model.FindEntityType(typeof(Parent3Entity));
+            indexes = GetIndexes(parent.GetPropertiesAndNavigations());
+            Assert.Equal(2, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, -1, 0, 0, 0), indexes[nameof(Parent3Entity.Id)]);
+            Assert.Equal((0, -1, -1, -1, 1), indexes[nameof(Parent3Entity.Children)]);
+
+            indexes = GetIndexes(model.FindEntityType(typeof(ChildEntity), nameof(Parent3Entity.Children), parent).GetProperties());
+            Assert.Equal(3, indexes.Count);
+            // Order: Index, Shadow, Original, StoreGenerated, Relationship
+            Assert.Equal((0, 0, 0, 0, 0), indexes[nameof(Parent3Entity) + "Id"]);
+            Assert.Equal((1, 1, 1, 1, 1), indexes["Id"]);
+            Assert.Equal((2, -1, 2, -1, -1), indexes[nameof(ChildEntity.Name)]);
+
+            Dictionary<string, (int, int, int, int, int)> GetIndexes(IEnumerable<IPropertyBase> properties)
+                => properties.ToDictionary(
+                    p => p.Name,
+                    p =>
+                        (p.GetIndex(),
+                            p.GetShadowIndex(),
+                            p.GetOriginalValueIndex(),
+                            p.GetStoreGeneratedIndex(),
+                            p.GetRelationshipIndex()
+                        ));
+        }
+
+        private class SideBySide : DbContext
+        {
+            protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
+                => optionsBuilder
+                    .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider)
+                    .UseInMemoryDatabase(Guid.NewGuid().ToString());
+
+            protected internal override void OnModelCreating(ModelBuilder modelBuilder)
+            {
+                modelBuilder.Entity<Parent1Entity>().OwnsMany(e => e.Children);
+                modelBuilder.Entity<Parent2Entity>().OwnsMany(e => e.Children);
+                modelBuilder.Entity<Parent3Entity>().OwnsMany(e => e.Children);
+            }
+        }
+
+        private class Parent1Entity
+        {
+            public Guid Id { get; set; }
+            public ICollection<ChildEntity> Children { get; set; }
+        }
+
+        private class Parent2Entity
+        {
+            public Guid Id { get; set; }
+            public ICollection<ChildEntity> Children { get; set; }
+        }
+
+        private class Parent3Entity
+        {
+            public Guid Id { get; set; }
+            public ICollection<ChildEntity> Children { get; set; }
+        }
+
+        private class ChildEntity
+        {
+            public string Name { get; set; }
+        }
+
         [ConditionalFact]
         public void Indexes_are_ordered_by_property_count_then_property_names()
         {