Correcting navigation 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 5, 2019 · af9e222 · af9e222
1 parent b3782ca
commit af9e222
Show file tree

Hide file tree

Showing 7 changed files with 219 additions and 44 deletions.
diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs
@@ -278,23 +278,31 @@ public virtual InternalEntityEntry CreateEntry(IDictionary<string, object> value
         {
             _trackingQueryMode = TrackingQueryMode.Multiple;
 
-            var i = 0;
+            var (i, j) = (0, 0);
             var valuesArray = new object[entityType.PropertyCount()];
+            var shadowPropertyValuesArray = new object[entityType.ShadowPropertyCount()];
             foreach (var property in entityType.GetProperties())
             {
                 valuesArray[i++] = values.TryGetValue(property.Name, out var value)
                     ? value
                     : property.ClrType.GetDefaultValue();
+
+                if (property.IsShadowProperty())
+                {
+                    shadowPropertyValuesArray[j++] = values.TryGetValue(property.Name, out var shadowValue)
+                        ? shadowValue
+                        : property.ClrType.GetDefaultValue();
+                }
             }
 
             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);
+            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