Query: Disallow tracking queries for owned entites without their owners

Resolves #13036
dotnet · Aug 7, 2019 · c8e2522 · c8e2522
1 parent f0dd703
commit c8e2522
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 2 deletions.
diff --git a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
@@ -179,6 +179,7 @@ private static readonly MethodInfo _startTrackingMethodInfo
 
             private readonly IEntityMaterializerSource _entityMaterializerSource;
             private readonly bool _trackQueryResults;
+            private readonly ISet<IEntityType> _visitedEntityTypes = new HashSet<IEntityType>();
             private int _currentEntityIndex;
 
             public EntityMaterializerInjectingExpressionVisitor(
@@ -188,7 +189,30 @@ public EntityMaterializerInjectingExpressionVisitor(
                 _trackQueryResults = trackQueryResults;
             }
 
-            public Expression Inject(Expression expression) => Visit(expression);
+            public Expression Inject(Expression expression)
+            {
+                var result = Visit(expression);
+                if (_trackQueryResults)
+                {
+                    bool containsOwner(IEntityType owner)
+                        => owner != null && (_visitedEntityTypes.Contains(owner) || containsOwner(owner.BaseType));
+
+                    foreach (var entityType in _visitedEntityTypes)
+                    {
+                        if ((entityType.HasDefiningNavigation()
+                            && !containsOwner(entityType.DefiningEntityType))
+                            || (entityType.FindOwnership() is IForeignKey ownership
+                                && !containsOwner(ownership.PrincipalEntityType)))
+                        {
+                            throw new InvalidOperationException("A tracking query projects owned entity without corresponding owner in result. " +
+                                "Owned entities cannot be tracked without their owner. " +
+                                "Either include the owner entity in the result or make query non-tracking using AsNoTracking().");
+                        }
+                    }
+                }
+
+                return result;
+            }
 
             protected override Expression VisitExtension(Expression extensionExpression)
                 => extensionExpression is EntityShaperExpression entityShaperExpression
@@ -379,6 +403,8 @@ var discriminatorValue
 
                 if (_trackQueryResults)
                 {
+                    _visitedEntityTypes.Add(entityType);
+
                     expressions.Add(
                         Expression.Assign(
                             entryVariable, Expression.Call(

diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs
@@ -180,5 +180,10 @@ public override void Member_pushdown_chain_3_levels_deep()
         public override void Member_pushdown_with_collection_navigation_in_the_middle()
         {
         }
+
+        // Invalid test due to projecting owned entity without owner in tracking query
+        public override void Entries_for_detached_entities_are_removed()
+        {
+        }
     }
 }
diff --git a/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
@@ -447,6 +447,23 @@ public virtual void Query_loads_reference_nav_automatically_in_projection()
             }
         }
 
+        [ConditionalFact]
+        public virtual void Throw_for_owned_entities_without_owner_in_tracking_query()
+        {
+            using (var context = CreateContext())
+            {
+                var query = context.Set<OwnedPerson>().Select(e => e.PersonAddress);
+
+                var result = query.AsNoTracking().ToList();
+                Assert.Equal(4, result.Count);
+                Assert.Equal(0, context.ChangeTracker.Entries().Count());
+
+                Assert.Throws<InvalidOperationException>(() => query.AsTracking().ToList());
+
+                Assert.Equal(0, context.ChangeTracker.Entries().Count());
+            }
+        }
+
         protected virtual DbContext CreateContext() => Fixture.CreateContext();
 
         public abstract class OwnedQueryFixtureBase : SharedStoreFixtureBase<PoolableDbContext>
@@ -687,7 +704,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         b.HasData(
                             new Barton
                             {
-                                Id = 1, Simple = "Simple"
+                                Id = 1,
+                                Simple = "Simple"
                             });
 
                     });