From f8895e1bdd5fa9a52333e22b70bf1f0a7e6f1f75 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Mon, 2 Oct 2023 12:43:52 -0700 Subject: [PATCH] Cosmos: persist non-shadow int keys on owned entity types Fixes #31664 --- .../Extensions/CosmosPropertyExtensions.cs | 1 + .../CosmosValueGenerationConvention.cs | 1 + .../Internal/CosmosPropertyExtensions.cs | 8 +++---- .../EmbeddedDocumentsTest.cs | 24 ++++++++++++------- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs index cb706b255ea..08c7ec86466 100644 --- 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 index 3a7836693c1..32d02a1e88a 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/CosmosValueGenerationConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/CosmosValueGenerationConvention.cs @@ -80,6 +80,7 @@ public virtual void ProcessEntityTypeAnnotationChanged( 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 index 98ead684f5e..24231c8a73c 100644 --- 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 index f64175b751a..bb02fe750af 100644 --- 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 { - 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 @@ await context.AddAsync( 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().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