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

…d 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.

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/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;
@@ -108,6 +107,14 @@ public virtual void Generate(InternalEntityEntry entry, bool includePrimaryKey =
                     property,
                     generatedValue,
                     temporary);
+
+                if (includePrimaryKey
+                    && property.IsPrimaryKey()
+                    && property.IsShadowProperty()
+                    && !property.IsForeignKey())
+                {
+                    entry.MarkUnknown(property);
+                }
             }
         }
 

src/EFCore/Properties/CoreStrings.resx

@@ -1518,6 +1518,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>

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs

@@ -419,6 +419,36 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 });
 
                 modelBuilder.Entity<SharedFkDependant>();
+
+                modelBuilder.Entity<Owner>();
+
+                modelBuilder.Entity<OwnerWithKeyedCollection>(
+                    b =>
+                    {
+                        b.Navigation(e => e.Owned).IsRequired();
+                        b.Navigation(e => e.OwnedWithKey).IsRequired();
+
+                        b.OwnsMany(
+                            e => e.OwnedCollectionPrivateKey,
+                            b => b.HasKey("OwnerWithKeyedCollectionId", "PrivateKey"));
+                    });
+
+                modelBuilder.Entity<OwnerNoKeyGeneration>(
+                    b =>
+                    {
+                        b.Property(e => e.Id).ValueGeneratedNever();
+
+                        b.OwnsOne(
+                            e => e.Owned,
+                            b => b.Property("OwnerNoKeyGenerationId").ValueGeneratedNever());
+                        b.OwnsMany(
+                            e => e.OwnedCollection,
+                            b =>
+                            {
+                                b.Property<int>("OwnedNoKeyGenerationId").ValueGeneratedNever();
+                                b.Property("OwnerNoKeyGenerationId").ValueGeneratedNever();
+                            });
+                    });
             }
 
             protected virtual object CreateFullGraph()
@@ -3092,6 +3122,185 @@ public SharedFkParent Parent
             }
         }
 
+        protected class Owner : NotifyingEntity
+        {
+            private int _id;
+            private Owned _owned;
+            private ICollection<Owned> _ownedCollection = new ObservableHashSet<Owned>();
+
+            public int Id
+            {
+                get => _id;
+                set => SetWithNotify(value, ref _id);
+            }
+
+            public Owned Owned
+            {
+                get => _owned;
+                set => SetWithNotify(value, ref _owned);
+            }
+
+            public ICollection<Owned> OwnedCollection
+            {
+                get => _ownedCollection;
+                set => SetWithNotify(value, ref _ownedCollection);
+            }
+        }
+
+        [Owned]
+        protected class Owned : NotifyingEntity
+        {
+            private int _foo;
+            private string _bar;
+
+            public int Foo
+            {
+                get => _foo;
+                set => SetWithNotify(value, ref _foo);
+            }
+
+            public string Bar
+            {
+                get => _bar;
+                set => SetWithNotify(value, ref _bar);
+            }
+        }
+
+        protected class OwnerWithKeyedCollection : NotifyingEntity
+        {
+            private int _id;
+            private Owned _owned;
+            private OwnedWithKey _ownedWithKey;
+            private ICollection<OwnedWithKey> _ownedCollection = new ObservableHashSet<OwnedWithKey>();
+            private ICollection<OwnedWithPrivateKey> _ownedCollectionPrivateKey = new ObservableHashSet<OwnedWithPrivateKey>();
+
+            public int Id
+            {
+                get => _id;
+                set => SetWithNotify(value, ref _id);
+            }
+
+            public Owned Owned
+            {
+                get => _owned;
+                set => SetWithNotify(value, ref _owned);
+            }
+
+            public OwnedWithKey OwnedWithKey
+            {
+                get => _ownedWithKey;
+                set => SetWithNotify(value, ref _ownedWithKey);
+            }
+
+            public ICollection<OwnedWithKey> OwnedCollection
+            {
+                get => _ownedCollection;
+                set => SetWithNotify(value, ref _ownedCollection);
+            }
+
+            public ICollection<OwnedWithPrivateKey> OwnedCollectionPrivateKey
+            {
+                get => _ownedCollectionPrivateKey;
+                set => SetWithNotify(value, ref _ownedCollectionPrivateKey);
+            }
+        }
+
+        [Owned]
+        protected class OwnedWithKey : NotifyingEntity
+        {
+            private int _foo;
+            private string _bar;
+            private int _ownedWithKeyId;
+
+            public int OwnedWithKeyId
+            {
+                get => _ownedWithKeyId;
+                set => SetWithNotify(value, ref _ownedWithKeyId);
+            }
+
+            public int Foo
+            {
+                get => _foo;
+                set => SetWithNotify(value, ref _foo);
+            }
+
+            public string Bar
+            {
+                get => _bar;
+                set => SetWithNotify(value, ref _bar);
+            }
+        }
+
+        [Owned]
+        protected class OwnedWithPrivateKey : NotifyingEntity
+        {
+            private int _foo;
+            private string _bar;
+            private int _privateKey;
+
+            private int PrivateKey
+            {
+                get => _privateKey;
+                set => SetWithNotify(value, ref _privateKey);
+            }
+
+            public int Foo
+            {
+                get => _foo;
+                set => SetWithNotify(value, ref _foo);
+            }
+
+            public string Bar
+            {
+                get => _bar;
+                set => SetWithNotify(value, ref _bar);
+            }
+        }
+
+        protected class OwnerNoKeyGeneration : NotifyingEntity
+        {
+            private int _id;
+            private OwnedNoKeyGeneration _owned;
+            private ICollection<OwnedNoKeyGeneration> _ownedCollection = new ObservableHashSet<OwnedNoKeyGeneration>();
+
+            public int Id
+            {
+                get => _id;
+                set => SetWithNotify(value, ref _id);
+            }
+
+            public OwnedNoKeyGeneration Owned
+            {
+                get => _owned;
+                set => SetWithNotify(value, ref _owned);
+            }
+
+            public ICollection<OwnedNoKeyGeneration> OwnedCollection
+            {
+                get => _ownedCollection;
+                set => SetWithNotify(value, ref _ownedCollection);
+            }
+        }
+
+        [Owned]
+        protected class OwnedNoKeyGeneration : NotifyingEntity
+        {
+            private int _foo;
+            private string _bar;
+
+            public int Foo
+            {
+                get => _foo;
+                set => SetWithNotify(value, ref _foo);
+            }
+
+            public string Bar
+            {
+                get => _bar;
+                set => SetWithNotify(value, ref _bar);
+            }
+        }
+
         protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged
         {
             protected void SetWithNotify<T>(T value, ref T field, [CallerMemberName] string propertyName = "")