Detect unknown shadow primary key values when attempting to save owne…

…d collection (#25652)

* Detect unknown shadow primary key values when attempting to save owned collection

Fixes #19856

The fundamental issue here is that the key for owned collections is, by default, formed from shadow properties. This means the values of these properties are lost when the entities in the collection are no longer tracked by a DbContext. When the entities are then later attached there is no way to get these values back. (Contrast this with owned non-collection entities, where the shadow key value can be synthesized from the owner key.) This in turn means that there is no way to identify these entities in the database, and there is therefore no way to update or delete them.

This PR detects this situation and throws an exception with a (hopefully) helpful error message.

The best way to deal with this is to add a CLR property for the key (which can be private) to the owned entity type. The key values are then preserved while the entities are not being tracked, and re-attaching the entities works correctly. We should document this.

* Adding async code and tests.

Also, fixes #21206.

Let value generators indicate that they return stable values, and don't mark these values as Unknown. This stops values generated for discriminator properties from being marked as Unknown.

src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs

@@ -26,6 +26,15 @@ public class IdValueGenerator : ValueGenerator
         public override bool GeneratesTemporaryValues
             => false;
 
+        /// <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>
+        public override bool GeneratesStableValues
+            => true;
+
         /// <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

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -1443,12 +1443,30 @@ public InternalEntityEntry PrepareToSave()
                                 property.Name,
                                 EntityType.DisplayName()));
                     }
+
+                    CheckForUnknownKey(property);
+                }
+            }
+            else if (EntityState == EntityState.Deleted)
+            {
+                foreach (var property in entityType.GetProperties())
+                {
+                    CheckForUnknownKey(property);
                 }
             }
 
             DiscardStoreGeneratedValues();
 
             return this;
+
+            void CheckForUnknownKey(IProperty property)
+            {
+                if (property.IsKey()
+                    && _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown))
+                {
+                    throw new InvalidOperationException(CoreStrings.UnknownShadowKeyValue(entityType.DisplayName(), property.Name));
+                }
+            }
         }
 
         /// <summary>

src/EFCore/ChangeTracking/Internal/KeyPropagator.cs

@@ -59,7 +59,12 @@ public KeyPropagator(
                 && property.IsKey()
                 && !property.IsForeignKeyToSelf())
             {
-                var valueGenerator = TryGetValueGenerator(generationProperty);
+                var valueGenerator = TryGetValueGenerator(
+                    generationProperty,
+                    generationProperty == property
+                        ? entry.EntityType
+                        : generationProperty?.DeclaringEntityType);
+
                 if (valueGenerator != null)
                 {
                     var value = valueGenerator.Next(new EntityEntry(entry));
@@ -73,7 +78,10 @@ public KeyPropagator(
                         entry[property] = value;
                     }
 
-                    entry.MarkUnknown(property);
+                    if (!valueGenerator.GeneratesStableValues)
+                    {
+                        entry.MarkUnknown(property);
+                    }
                 }
             }
 
@@ -99,7 +107,12 @@ public KeyPropagator(
             if (principalEntry == null
                 && property.IsKey())
             {
-                var valueGenerator = TryGetValueGenerator(generationProperty);
+                var valueGenerator = TryGetValueGenerator(
+                    generationProperty,
+                    generationProperty == property
+                        ? entry.EntityType
+                        : generationProperty?.DeclaringEntityType);
+
                 if (valueGenerator != null)
                 {
                     var value = await valueGenerator.NextAsync(new EntityEntry(entry), cancellationToken)
@@ -114,7 +127,10 @@ public KeyPropagator(
                         entry[property] = value;
                     }
 
-                    entry.MarkUnknown(property);
+                    if (!valueGenerator.GeneratesStableValues)
+                    {
+                        entry.MarkUnknown(property);
+                    }
                 }
             }
 
@@ -177,9 +193,9 @@ public KeyPropagator(
             return null;
         }
 
-        private ValueGenerator? TryGetValueGenerator(IProperty? generationProperty)
+        private ValueGenerator? TryGetValueGenerator(IProperty? generationProperty, IEntityType? entityType)
             => generationProperty != null
-                ? _valueGeneratorSelector.Select(generationProperty, generationProperty.DeclaringEntityType)
+                ? _valueGeneratorSelector.Select(generationProperty, entityType!)
                 : null;
     }
 }

src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore.Diagnostics;
@@ -103,11 +102,9 @@ public virtual void Generate(InternalEntityEntry entry, bool includePrimaryKey =
 
                 Log(entry, property, generatedValue, temporary);
 
-                SetGeneratedValue(
-                    entry,
-                    property,
-                    generatedValue,
-                    temporary);
+                SetGeneratedValue(entry, property, generatedValue, temporary);
+
+                MarkKeyUnknown(entry, includePrimaryKey, property, valueGenerator);
             }
         }
 
@@ -157,6 +154,8 @@ public virtual async Task GenerateAsync(
                     property,
                     generatedValue,
                     temporary);
+
+                MarkKeyUnknown(entry, includePrimaryKey, property, valueGenerator);
             }
         }
 
@@ -190,5 +189,21 @@ private static void SetGeneratedValue(InternalEntityEntry entry, IProperty prope
                 }
             }
         }
+
+        private static void MarkKeyUnknown(
+            InternalEntityEntry entry, 
+            bool includePrimaryKey, 
+            IProperty property, 
+            ValueGenerator valueGenerator)
+        {
+            if (includePrimaryKey
+                && property.IsKey()
+                && property.IsShadowProperty()
+                && !property.IsForeignKey()
+                && !valueGenerator.GeneratesStableValues)
+            {
+                entry.MarkUnknown(property);
+            }
+        }
     }
 }

src/EFCore/Properties/CoreStrings.resx

@@ -1524,6 +1524,9 @@
   <data name="UnknownKeyValue" xml:space="preserve">
     <value>The value of '{entityType}.{property}' is unknown when attempting to save changes. This is because the property is also part of a foreign key for which the principal entity in the relationship is not known.</value>
   </data>
+  <data name="UnknownShadowKeyValue" xml:space="preserve">
+    <value>The value of shadow key property '{entityType}.{property}' is unknown when attempting to save changes. This is because shadow property values cannot be preserved when the entity is not being tracked. Consider adding the property to the entity's .NET type. See https://aka.ms/efcore-docs-owned-collections for more information.</value>
+  </data>
   <data name="UnnamedIndexDefinedOnIgnoredProperty" xml:space="preserve">
     <value>The unnamed index specified via [Index] attribute on the entity type '{entityType}' with properties {indexProperties} is invalid. The property '{propertyName}' was marked as unmapped by [NotMapped] attribute or 'Ignore()' in 'OnModelCreating'. An index cannot use unmapped properties.</value>
   </data>

src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs

@@ -43,5 +43,14 @@ protected override object NextValue(EntityEntry entry)
         /// </summary>
         public override bool GeneratesTemporaryValues
             => false;
+
+        /// <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>
+        public override bool GeneratesStableValues
+            => true;
     }
 }

src/EFCore/ValueGeneration/ValueGenerator.cs

@@ -66,5 +66,14 @@ public abstract class ValueGenerator
         ///     </para>
         /// </summary>
         public abstract bool GeneratesTemporaryValues { get; }
+
+        /// <summary>
+        ///     Gets a value indicating whether the values generated are stable. That is, the value will always be the
+        ///     same for a given property in a given entity, and does not depend on what other values may have been generated
+        ///     previously. For example, discriminator values generated for a TPH hierarchy are stable. Stable values will never
+        ///     be marked as unknown.
+        /// </summary>
+        public virtual bool GeneratesStableValues
+            => false;
     }
 }

test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesInMemoryTest.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
@@ -161,6 +162,18 @@ protected override void ExecuteWithStrategyInTransaction(
             Fixture.Reseed();
         }
 
+        protected override async Task ExecuteWithStrategyInTransactionAsync(
+            Func<DbContext, Task> testOperation,
+            Func<DbContext, Task> nestedTestOperation1 = null,
+            Func<DbContext, Task> nestedTestOperation2 = null,
+            Func<DbContext, Task> nestedTestOperation3 = null)
+        {
+            await base.ExecuteWithStrategyInTransactionAsync(
+                testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
+
+            Fixture.Reseed();
+        }
+
         public class InMemoryFixture : GraphUpdatesFixtureBase
         {
             protected override string StoreName { get; } = "GraphUpdatesTest";