Cosmos: persist non-shadow int keys on owned entity types (#31933)

Fixes #31664
dotnet · Oct 3, 2023 · 2a7a49b · 2a7a49b
1 parent c7cd3aa
commit 2a7a49b
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 12 deletions.
diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs
@@ -35,6 +35,7 @@ private static string GetDefaultJsonPropertyName(IReadOnlyProperty property)
             var pk = property.FindContainingPrimaryKey();
             if (pk != null
                 && (property.ClrType == typeof(int) || ownership.Properties.Contains(property))
+                && property.IsShadowProperty()
                 && pk.Properties.Count == ownership.Properties.Count + (ownership.IsUnique ? 0 : 1)
                 && ownership.Properties.All(fkProperty => pk.Properties.Contains(fkProperty)))
             {

diff --git a/src/EFCore.Cosmos/Metadata/Conventions/CosmosValueGenerationConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/CosmosValueGenerationConvention.cs
@@ -80,6 +80,7 @@ public class CosmosValueGenerationConvention :
                 if (pk != null
                     && !property.IsForeignKey()
                     && pk.Properties.Count == ownership.Properties.Count + 1
+                    && property.IsShadowProperty()
                     && ownership.Properties.All(fkProperty => pk.Properties.Contains(fkProperty)))
                 {
                     return ValueGenerated.OnAddOrUpdate;

diff --git a/src/EFCore.Cosmos/Metadata/Internal/CosmosPropertyExtensions.cs b/src/EFCore.Cosmos/Metadata/Internal/CosmosPropertyExtensions.cs
@@ -21,11 +21,11 @@ public static bool IsOrdinalKeyProperty(this IReadOnlyProperty property)
     {
         Check.DebugAssert(
             (property.DeclaringType as IEntityType)?.IsOwned() == true, $"Expected {property.DeclaringType.DisplayName()} to be owned.");
-        Check.DebugAssert(property.GetJsonPropertyName().Length == 0, $"Expected {property.Name} to be non-persisted.");
 
-        return property.FindContainingPrimaryKey() is { Properties.Count: > 1 }
+        return property.ClrType == typeof(int)
             && !property.IsForeignKey()
-            && property.ClrType == typeof(int)
-            && (property.ValueGenerated & ValueGenerated.OnAdd) != 0;
+            && (property.ValueGenerated & ValueGenerated.OnAdd) != 0
+            && property.FindContainingPrimaryKey() is { Properties.Count: > 1 }
+            && property.GetJsonPropertyName().Length == 0;
     }
 }
diff --git a/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs b/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs
@@ -118,7 +118,7 @@ public virtual async Task Can_manipulate_embedded_collections(bool useIds)
             {
                 existingAddress1Person2.IdNotes = new List<NoteWithId>
                 {
-                    new() { Content = "First note" }, new() { Content = "Second note" }
+                    new() { Id = 4, Content = "First note" }, new() { Id = 3, Content = "Second note" }
                 };
             }
             else
@@ -260,15 +260,15 @@ public virtual async Task Can_manipulate_embedded_collections(bool useIds)
 
             await context.SaveChangesAsync();
 
-            await AssertState(context);
+            await AssertState(context, useIds);
         }
 
         using (var context = new EmbeddedTransportationContext(options))
         {
-            await AssertState(context);
+            await AssertState(context, useIds);
         }
 
-        async Task AssertState(EmbeddedTransportationContext context)
+        async Task AssertState(EmbeddedTransportationContext context, bool useIds)
         {
             var people = await context.Set<Person>().OrderBy(o => o.Id).ToListAsync();
             var firstAddress = people[0].Addresses.Single();
@@ -294,9 +294,17 @@ async Task AssertState(EmbeddedTransportationContext context)
             {
                 var notes = addresses[1].IdNotes;
                 Assert.Equal(2, notes.Count);
-                Assert.Equal(1, notes.First().Id);
+                if (useIds)
+                {
+                    Assert.Equal(4, notes.First().Id);
+                    Assert.Equal(3, notes.Last().Id);
+                }
+                else
+                {
+                    Assert.Equal(1, notes.First().Id);
+                    Assert.Equal(2, notes.Last().Id);
+                }
                 Assert.Equal("First note", notes.First().Content);
-                Assert.Equal(2, notes.Last().Id);
                 Assert.Equal("Second note", notes.Last().Content);
             }
             else
@@ -339,7 +347,7 @@ async Task AssertState(EmbeddedTransportationContext context)
             if (useIds)
             {
                 Assert.Equal(1, addresses[1].IdNotes.Count);
-                Assert.Equal(1, addresses[1].IdNotes.First().Id);
+                Assert.Equal(-1, addresses[1].IdNotes.First().Id);
                 Assert.Equal("Another note", addresses[1].IdNotes.First().Content);
             }
             else
@@ -354,7 +362,7 @@ async Task AssertState(EmbeddedTransportationContext context)
             if (useIds)
             {
                 Assert.Equal(1, addresses[2].IdNotes.Count);
-                Assert.Equal(1, addresses[2].IdNotes.First().Id);
+                Assert.Equal(4, addresses[2].IdNotes.First().Id);
                 Assert.Equal("City note", addresses[2].IdNotes.First().Content);
             }
             else