Correctig navigatgion expansion handling of owned navigations

Navigation expansion should not try expand owned navigations. We still add them to navigation tree for tracking purposes but we don't inject LeftJoin calls when processing them. Collection navigations are converted directly to NavigationExpansionExpressions whose root is the navigation itself to avoid rewrite to a subquery.
dotnet · Jun 4, 2019 · 4b4d434 · 4b4d434
1 parent e6acad1
commit 4b4d434
Show file tree

Hide file tree

Showing 7 changed files with 219 additions and 43 deletions.
diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs
@@ -288,13 +288,22 @@ public virtual InternalEntityEntry CreateEntry(IDictionary<string, object> value
             }
 
             var valueBuffer = new ValueBuffer(valuesArray);
-
             var entity = entityType.HasClrType()
                 ? EntityMaterializerSource.GetMaterializer(entityType)(
                     new MaterializationContext(valueBuffer, Context))
                 : null;
 
-            var entry = _internalEntityEntryFactory.Create(this, entityType, entity, valueBuffer);
+            i = 0;
+            var shadowPropertyValuesArray = new object[entityType.ShadowPropertyCount()];
+            foreach (var shadowProperty in entityType.GetProperties().Where(p => p.IsShadowProperty()))
+            {
+                shadowPropertyValuesArray[i++] = values.TryGetValue(shadowProperty.Name, out var shadowValue)
+                    ? shadowValue
+                    : shadowProperty.ClrType.GetDefaultValue();
+            }
+
+            var shadowPropertyValueBuffer = new ValueBuffer(shadowPropertyValuesArray);
+            var entry = _internalEntityEntryFactory.Create(this, entityType, entity, shadowPropertyValueBuffer);
 
             UpdateReferenceMaps(entry, EntityState.Detached, null);
 

diff --git a/src/EFCore/Query/NavigationExpansion/NavigationExpansionHelpers.cs b/src/EFCore/Query/NavigationExpansion/NavigationExpansionHelpers.cs
@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Extensions.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
@@ -182,8 +183,13 @@ var transparentIdentifierCtorInfo
                 foreach (var mapping in state.SourceMappings)
                 {
                     var nodes = include
-                        ? mapping.NavigationTree.Flatten().Where(n => (n.Included == NavigationTreeNodeIncludeMode.ReferenceComplete || n.ExpansionMode == NavigationTreeNodeExpansionMode.ReferenceComplete) && n != navigationTree)
-                        : mapping.NavigationTree.Flatten().Where(n => n.ExpansionMode == NavigationTreeNodeExpansionMode.ReferenceComplete && n != navigationTree);
+                        ? mapping.NavigationTree.Flatten().Where(n => (n.Included == NavigationTreeNodeIncludeMode.ReferenceComplete
+                            || n.ExpansionMode == NavigationTreeNodeExpansionMode.ReferenceComplete
+                            || n.Navigation.ForeignKey.IsOwnership)
+                                && n != navigationTree)
+                        : mapping.NavigationTree.Flatten().Where(n => (n.ExpansionMode == NavigationTreeNodeExpansionMode.ReferenceComplete
+                            || n.Navigation.ForeignKey.IsOwnership)
+                                && n != navigationTree);
 
                     foreach (var navigationTreeNode in nodes)
                     {

diff --git a/src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs b/src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs
@@ -39,6 +39,18 @@ private NavigationTreeNode(
                 Included = NavigationTreeNodeIncludeMode.NotNeeded;
             }
 
+            // for ownership don't mark for include or expansion
+            // just track the navigations in the tree in the original form
+            // they will be expanded/translated later in the pipeline
+            if (navigation.ForeignKey.IsOwnership)
+            {
+                ExpansionMode = NavigationTreeNodeExpansionMode.NotNeeded;
+                Included = NavigationTreeNodeIncludeMode.NotNeeded;
+
+                ToMapping = parent.ToMapping.ToList();
+                ToMapping.Add(navigation.Name);
+            }
+
             foreach (var parentFromMapping in parent.FromMappings)
             {
                 var newMapping = parentFromMapping.ToList();
@@ -92,17 +104,20 @@ public static NavigationTreeNode Create(
             var existingChild = parent.Children.Where(c => c.Navigation == navigation).SingleOrDefault();
             if (existingChild != null)
             {
-                if (include && existingChild.Included == NavigationTreeNodeIncludeMode.NotNeeded)
-                {
-                    existingChild.Included = navigation.IsCollection()
-                        ? NavigationTreeNodeIncludeMode.Collection
-                        : NavigationTreeNodeIncludeMode.ReferencePending;
-                }
-                else if (!include && existingChild.ExpansionMode == NavigationTreeNodeExpansionMode.NotNeeded)
+                if (!navigation.ForeignKey.IsOwnership)
                 {
-                    existingChild.ExpansionMode = navigation.IsCollection()
-                        ? NavigationTreeNodeExpansionMode.Collection
-                        : NavigationTreeNodeExpansionMode.ReferencePending;
+                    if (include && existingChild.Included == NavigationTreeNodeIncludeMode.NotNeeded)
+                    {
+                        existingChild.Included = navigation.IsCollection()
+                            ? NavigationTreeNodeIncludeMode.Collection
+                            : NavigationTreeNodeIncludeMode.ReferencePending;
+                    }
+                    else if (!include && existingChild.ExpansionMode == NavigationTreeNodeExpansionMode.NotNeeded)
+                    {
+                        existingChild.ExpansionMode = navigation.IsCollection()
+                            ? NavigationTreeNodeExpansionMode.Collection
+                            : NavigationTreeNodeExpansionMode.ReferencePending;
+                    }
                 }
 
                 return existingChild;

diff --git a/src/EFCore/Query/NavigationExpansion/Visitors/CollectionNavigationRewritingVisitor.cs b/src/EFCore/Query/NavigationExpansion/Visitors/CollectionNavigationRewritingVisitor.cs
@@ -195,9 +195,9 @@ protected override Expression VisitExtension(Expression extensionExpression)
                     && navigationBindingExpression.NavigationTreeNode.Navigation is INavigation lastNavigation
                     && lastNavigation.IsCollection())
                 {
-                    var result = CreateCollectionNavigationExpression(navigationBindingExpression.NavigationTreeNode, navigationBindingExpression.RootParameter, navigationBindingExpression.SourceMapping);
-
-                    return result;
+                    return lastNavigation.ForeignKey.IsOwnership
+                        ? NavigationExpansionHelpers.CreateNavigationExpansionRoot(navigationBindingExpression, lastNavigation.GetTargetType(), lastNavigation)
+                        : CreateCollectionNavigationExpression(navigationBindingExpression.NavigationTreeNode, navigationBindingExpression.RootParameter, navigationBindingExpression.SourceMapping);
                 }
                 else
                 {

diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs
@@ -29,9 +29,9 @@ FROM root c
 WHERE ((c[""Discriminator""] = ""LeafB"") OR ((c[""Discriminator""] = ""LeafA"") OR ((c[""Discriminator""] = ""Branch"") OR (c[""Discriminator""] = ""OwnedPerson""))))");
         }
 
-        public override void Navigation_rewrite_on_owned_reference()
+        public override void Navigation_rewrite_on_owned_reference_projecting_entity()
         {
-            base.Navigation_rewrite_on_owned_reference();
+            base.Navigation_rewrite_on_owned_reference_projecting_entity();
 
             AssertSql(
                 @"SELECT c