Set ordinal key values when saving embedded owned collection entities.

Consistently allow readonly properties to be set the current store values during SaveChanges to allow propagation of the new values.

Fixes #18948

src/EFCore.Cosmos/Metadata/Internal/CosmosPropertyExtensions.cs

@@ -0,0 +1,35 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Diagnostics;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+namespace Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal
+{
+    /// <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 static class CosmosPropertyExtensions
+    {
+        /// <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 static bool IsOrdinalKeyProperty(this IProperty property)
+        {
+            Debug.Assert(property.DeclaringEntityType.IsOwned());
+            Debug.Assert(property.GetPropertyName().Length == 0);
+
+            return property.IsPrimaryKey()
+                && !property.IsForeignKey()
+                && property.ClrType == typeof(int)
+                && property.ValueGenerated == ValueGenerated.OnAdd
+                && property.DeclaringEntityType.FindPrimaryKey().Properties.Count > 1;
+        }
+    }
+}

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs

@@ -604,9 +604,7 @@ private Expression CreateGetValueExpression(
                     {
                         var ownership = entityType.FindOwnership();
                         if (!ownership.IsUnique
-                            && property.IsPrimaryKey()
-                            && !property.IsForeignKey()
-                            && property.ClrType == typeof(int))
+                            && property.IsOrdinalKeyProperty())
                         {
                             Expression readExpression = _ordinalParameterBindings[jObjectExpression];
                             if (readExpression.Type != clrType)

src/EFCore.Cosmos/Update/Internal/DocumentSource.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Collections;
+using System.Linq;
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
@@ -64,6 +65,15 @@ public virtual string GetId(IUpdateEntry entry)
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual JObject CreateDocument(IUpdateEntry entry)
+            => CreateDocument(entry, null);
+
+        /// <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 virtual JObject CreateDocument(IUpdateEntry entry, int? ordinal)
         {
             var document = new JObject();
             foreach (var property in entry.EntityType.GetProperties())
@@ -75,7 +85,11 @@ public virtual JObject CreateDocument(IUpdateEntry entry)
                 }
                 else if (entry.HasTemporaryValue(property))
                 {
-                    ((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property);
+                    if (ordinal != null
+                        && property.IsOrdinalKeyProperty())
+                    {
+                        entry.SetStoreGeneratedValue(property, ordinal.Value);
+                    }
                 }
             }
 
@@ -102,11 +116,13 @@ public virtual JObject CreateDocument(IUpdateEntry entry)
                 }
                 else
                 {
+                    var embeddedOrdinal = 0;
                     var array = new JArray();
                     foreach (var dependent in (IEnumerable)embeddedValue)
                     {
                         var dependentEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(dependent, fk.DeclaringEntityType);
-                        array.Add(_database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry));
+                        array.Add(_database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry, embeddedOrdinal));
+                        embeddedOrdinal++;
                     }
 
                     document[embeddedPropertyName] = array;
@@ -123,8 +139,18 @@ public virtual JObject CreateDocument(IUpdateEntry entry)
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
+            => UpdateDocument(document, entry, null);
+
+        /// <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 virtual JObject UpdateDocument(JObject document, IUpdateEntry entry, int? ordinal)
         {
             var anyPropertyUpdated = false;
+            var stateManager = ((InternalEntityEntry)entry).StateManager;
             foreach (var property in entry.EntityType.GetProperties())
             {
                 if (entry.EntityState == EntityState.Added
@@ -136,10 +162,13 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
                         document[storeName] = ConvertPropertyValue(property, entry.GetCurrentValue(property));
                         anyPropertyUpdated = true;
                     }
-                    else if (entry.HasTemporaryValue(property))
-                    {
-                        ((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property);
-                    }
+                }
+
+                if (ordinal != null
+                    && entry.HasTemporaryValue(property)
+                    && property.IsOrdinalKeyProperty())
+                {
+                    entry.SetStoreGeneratedValue(property, ordinal.Value);
                 }
             }
 
@@ -185,6 +214,47 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
                 }
                 else
                 {
+                    var embeddedOrdinal = 0;
+                    var ordinalKeyProperty = GetOrdinalKeyProperty(fk.DeclaringEntityType);
+                    if (ordinalKeyProperty != null)
+                    {
+                        var shouldSetTemporaryKeys = false;
+                        foreach (var dependent in (IEnumerable)embeddedValue)
+                        {
+                            var embeddedEntry = stateManager.TryGetEntry(dependent, fk.DeclaringEntityType);
+                            if (embeddedEntry == null)
+                            {
+                                continue;
+                            }
+
+                            if ((int)embeddedEntry.GetCurrentValue(ordinalKeyProperty) != embeddedOrdinal)
+                            {
+                                shouldSetTemporaryKeys = true;
+                                break;
+                            }
+
+                            embeddedOrdinal++;
+                        }
+
+                        if (shouldSetTemporaryKeys)
+                        {
+                            var temporaryOrdinal = -1;
+                            foreach (var dependent in (IEnumerable)embeddedValue)
+                            {
+                                var embeddedEntry = stateManager.TryGetEntry(dependent, fk.DeclaringEntityType);
+                                if (embeddedEntry == null)
+                                {
+                                    continue;
+                                }
+
+                                embeddedEntry.SetTemporaryValue(ordinalKeyProperty, temporaryOrdinal, setModified: false);
+
+                                temporaryOrdinal--;
+                            }
+                        }
+                    }
+
+                    embeddedOrdinal = 0;
                     var array = new JArray();
                     foreach (var dependent in (IEnumerable)embeddedValue)
                     {
@@ -196,10 +266,11 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
 
                         var embeddedDocument = embeddedDocumentSource.GetCurrentDocument(embeddedEntry);
                         embeddedDocument = embeddedDocument != null
-                            ? embeddedDocumentSource.UpdateDocument(embeddedDocument, embeddedEntry) ?? embeddedDocument
-                            : embeddedDocumentSource.CreateDocument(embeddedEntry);
+                            ? embeddedDocumentSource.UpdateDocument(embeddedDocument, embeddedEntry, embeddedOrdinal) ?? embeddedDocument
+                            : embeddedDocumentSource.CreateDocument(embeddedEntry, embeddedOrdinal);
 
                         array.Add(embeddedDocument);
+                        embeddedOrdinal++;
                     }
 
                     document[embeddedPropertyName] = array;
@@ -210,6 +281,10 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
             return anyPropertyUpdated ? document : null;
         }
 
+        private IProperty GetOrdinalKeyProperty(IEntityType entityType)
+            => entityType.FindPrimaryKey().Properties.FirstOrDefault(p =>
+                p.GetPropertyName().Length == 0 && p.IsOrdinalKeyProperty());
+
         public virtual JObject GetCurrentDocument(IUpdateEntry entry)
             => _jObjectProperty != null
                 ? (JObject)(entry.SharedIdentityEntry ?? entry).GetCurrentValue(_jObjectProperty)

src/EFCore/ChangeTracking/Internal/IStateManager.cs

@@ -57,6 +57,14 @@ public interface IStateManager : IResettableService
         /// </summary>
         CascadeTiming CascadeDeleteTiming { get; set; }
 
+        /// <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>
+        bool SavingChanges { get; }
+
         /// <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

@@ -449,7 +449,9 @@ public virtual void SetPropertyModified(
             if (changeState
                 && !isConceptualNull
                 && isModified
-                && property.IsKey())
+                && !StateManager.SavingChanges
+                && property.IsKey()
+                && property.GetAfterSaveBehavior() == PropertySaveBehavior.Throw)
             {
                 throw new InvalidOperationException(CoreStrings.KeyReadOnly(property.Name, EntityType.DisplayName()));
             }
@@ -608,7 +610,12 @@ public virtual void SetStoreGeneratedValue(IProperty property, object value)
             }
 
             SetProperty(
-                property, value, isMaterialization: false, setModified: true, isCascadeDelete: false, CurrentValueType.StoreGenerated);
+                property,
+                value,
+                isMaterialization: false,
+                setModified: true,
+                isCascadeDelete: false,
+                CurrentValueType.StoreGenerated);
         }
 
         /// <summary>

src/EFCore/ChangeTracking/Internal/StateManager.cs

@@ -123,6 +123,14 @@ public StateManager([NotNull] StateManagerDependencies dependencies)
         /// </summary>
         public virtual CascadeTiming CascadeDeleteTiming { get; set; } = CascadeTiming.Immediate;
 
+        /// <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 virtual bool SavingChanges { get; set; }
+
         /// <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
@@ -651,6 +659,8 @@ public virtual void ResetState()
 
             Tracked = null;
             StateChanged = null;
+
+            SavingChanges = false;
         }
 
         /// <summary>
@@ -1062,7 +1072,6 @@ protected virtual async Task<int> SaveChangesAsync(
             }
         }
 
-
         /// <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
@@ -1089,6 +1098,7 @@ private int SaveChanges(DbContext _, bool acceptAllChangesOnSuccess)
 
             try
             {
+                SavingChanges = true;
                 var result = SaveChanges(entriesToSave);
 
                 if (acceptAllChangesOnSuccess)
@@ -1107,6 +1117,10 @@ private int SaveChanges(DbContext _, bool acceptAllChangesOnSuccess)
 
                 throw;
             }
+            finally
+            {
+                SavingChanges = false;
+            }
         }
 
         /// <summary>
@@ -1137,6 +1151,7 @@ private async Task<int> SaveChangesAsync(
 
             try
             {
+                SavingChanges = true;
                 var result = await SaveChangesAsync(entriesToSave, cancellationToken);
 
                 if (acceptAllChangesOnSuccess)
@@ -1155,6 +1170,10 @@ private async Task<int> SaveChangesAsync(
 
                 throw;
             }
+            finally
+            {
+                SavingChanges = false;
+            }
         }
 
         /// <summary>