Correct nested owned JSON processing (#263)

Fixes #261
efcore · Jan 16, 2024 · 3df96e8 · 3df96e8
1 parent effd12a
commit 3df96e8
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 10 deletions.
diff --git a/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs b/EFCore.NamingConventions.Test/NameRewritingConventionTest.cs
@@ -648,6 +648,30 @@ public void Owned_json_entity_with_OwnsMany()
             .GetColumnName(StoreObjectIdentifier.Create(ownedEntityType, StoreObjectType.Table)!.Value));
     }
 
+    [Fact]
+    public void Owned_json_entity_with_OnesOne_and_nested_OwnsMany()
+    {
+        var model = BuildModel(mb => mb.Entity<Owner>().OwnsOne(o => o.Owned, p =>
+        {
+            p.OwnsMany(o => o.NestedOwnedCollection);
+            p.ToJson();
+        }));
+
+        var ownerEntityType = model.FindEntityType(typeof(Owner))!;
+        var ownedEntityType = model.FindEntityType(typeof(Owned))!;
+        var nestedOwnedCollectionEntityType = model.FindEntityType(typeof(NestedOwned))!;
+
+        Assert.Equal("owner", ownerEntityType.GetTableName());
+
+        Assert.Equal("owner", ownedEntityType.GetTableName());
+        Assert.Null(ownedEntityType.FindPrimaryKey()!.GetName());
+        Assert.Equal("owned", ownedEntityType.GetContainerColumnName());
+
+        Assert.Equal("owner", nestedOwnedCollectionEntityType.GetTableName());
+        Assert.Null(ownedEntityType.FindPrimaryKey()!.GetName());
+        Assert.Equal("owned", ownedEntityType.GetContainerColumnName());
+    }
+
     [Fact]
     public void Complex_property()
     {
@@ -817,6 +841,13 @@ public class Owner
     public class Owned
     {
         public int OwnedProperty { get; set; }
+        [NotMapped]
+        public required List<NestedOwned> NestedOwnedCollection { get; set; }
+    }
+
+    public class NestedOwned
+    {
+        public int Foo { get; set; }
     }
 
     public class Waypoint

diff --git a/EFCore.NamingConventions/Internal/NameRewritingConvention.cs b/EFCore.NamingConventions/Internal/NameRewritingConvention.cs
@@ -169,19 +169,32 @@ private void ProcessOwnershipChange(IConventionForeignKey foreignKey, IConventio
 
             if (ownedEntityType.IsMappedToJson())
             {
-                ownedEntityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
-                ownedEntityType.Builder.HasNoAnnotation(RelationalAnnotationNames.Schema);
+                ProcessJsonOwnedEntity(ownedEntityType, ownedEntityType.GetContainerColumnName());
 
-                if (ownedEntityType.GetContainerColumnName() is string containerColumnName)
+                void ProcessJsonOwnedEntity(IConventionEntityType entityType, string? containerColumnName)
                 {
-                    ownedEntityType.SetContainerColumnName(_namingNameRewriter.RewriteName(containerColumnName));
-                }
+                    entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
+                    entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.Schema);
 
-                // TODO: Note that we do not rewrite names of JSON properties (which aren't relational columns).
-                // TODO: We could introduce an option for doing so, though that's probably not usually what people want when doing JSON
-                foreach (var property in ownedEntityType.GetProperties())
-                {
-                    property.Builder.HasNoAnnotation(RelationalAnnotationNames.ColumnName);
+                    if (containerColumnName is not null)
+                    {
+                        entityType.SetContainerColumnName(_namingNameRewriter.RewriteName(containerColumnName));
+                    }
+
+                    // TODO: Note that we do not rewrite names of JSON properties (which aren't relational columns).
+                    // TODO: We could introduce an option for doing so, though that's probably not usually what people want when doing JSON
+                    foreach (var property in entityType.GetProperties())
+                    {
+                        property.Builder.HasNoAnnotation(RelationalAnnotationNames.ColumnName);
+                    }
+
+                    // ToJson() may be called only on the top-most owned entity; but we need to recurse and propagate the changes to the
+                    // entire owned tree.
+                    foreach (var navigation in entityType.GetNavigations()
+                                 .Where(n => n is { IsOnDependent: false, ForeignKey.IsOwnership: true }))
+                    {
+                        ProcessJsonOwnedEntity(navigation.TargetEntityType, containerColumnName);
+                    }
                 }
             }
             else