Skip to content

Commit

Permalink
Fix to #7131 - Projections With Full Types Not Fixing Up Navigations …
Browse files Browse the repository at this point in the history
…on Sqlite and SQL Server

Problem was that entities created by MaterializeCollectionNavigation were not being tracked - TrackEntities only works on "naked" entities.

Fix is to add delegate argument to MaterializeCollectionNavigation method that would track the elements of the collection if the query is supposed to be tracked. We already have a similar logic for the Include.
  • Loading branch information
maumar committed May 23, 2017
1 parent be2cc81 commit 049eb62
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 22 deletions.
45 changes: 44 additions & 1 deletion src/EFCore.Specification.Tests/QueryNavigationsTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,29 @@ join dynamic efItem in efItems on l2oItem.CustomerID equals efItem.CustomerID
{
Assert.Equal(pair.l2oItem.Orders, pair.efItem.Orders);
}
});
},
entryCount: 30);
}

[ConditionalFact]
public virtual void Select_entity_and_collection_navigation()
{
AssertQuery<Customer>(
cs => from c in cs
where c.CustomerID.StartsWith("A")
orderby c.CustomerID
select new { c, c.Orders },
asserter: (l2oItems, efItems) =>
{
foreach (var pair in
from dynamic l2oItem in l2oItems
join dynamic efItem in efItems on l2oItem.c.CustomerID equals efItem.c.CustomerID
select new { l2oItem, efItem })
{
Assert.Equal(pair.l2oItem.Orders, pair.efItem.Orders);
}
},
entryCount: 34);
}

[ConditionalFact]
Expand All @@ -448,6 +470,27 @@ join dynamic efItem in efItems on l2oItem.OrderID equals efItem.OrderID
{
Assert.Equal(pair.l2oItem.Orders, pair.efItem.Orders);
}
},
entryCount: 6);
}

[ConditionalFact]
public virtual void Elements_of_materialized_collection_navigation_not_tracked_for_queries_with_AsNoTracking()
{
AssertQuery<Customer>(
cs => from c in cs.AsNoTracking()
where c.CustomerID.StartsWith("A")
orderby c.CustomerID
select new { c.CustomerID, c.Orders },
asserter: (l2oItems, efItems) =>
{
foreach (var pair in
from dynamic l2oItem in l2oItems
join dynamic efItem in efItems on l2oItem.CustomerID equals efItem.CustomerID
select new { l2oItem, efItem })
{
Assert.Equal(pair.l2oItem.Orders, pair.efItem.Orders);
}
});
}

Expand Down
13 changes: 11 additions & 2 deletions src/EFCore/Metadata/Internal/ClrICollectionAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,24 @@ public virtual void AddRange(object instance, IEnumerable<object> values)
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual object Create(IEnumerable<object> values)
public virtual object Create()
{
if (_createCollection == null)
{
throw new InvalidOperationException(CoreStrings.NavigationCannotCreateType(
_propertyName, typeof(TEntity).ShortDisplayName(), typeof(TCollection).ShortDisplayName()));
}

var collection = (ICollection<TElement>)_createCollection();
return _createCollection();
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual object Create(IEnumerable<object> values)
{
var collection = (ICollection<TElement>)Create();
foreach (TElement value in values)
{
collection.Add(value);
Expand Down
6 changes: 6 additions & 0 deletions src/EFCore/Metadata/Internal/IClrCollectionAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public interface IClrCollectionAccessor
/// </summary>
void Remove([NotNull] object instance, [NotNull] object value);

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
object Create();

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
Expand Down
9 changes: 5 additions & 4 deletions src/EFCore/Query/EntityQueryModelVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,16 @@ protected virtual void OptimizeQueryModel(

var includeCompiler = new IncludeCompiler(QueryCompilationContext, _querySourceTracingExpressionVisitorFactory);

includeCompiler.CompileIncludes(queryModel, TrackResults(queryModel), asyncQuery);
var trackingQuery = TrackResults(queryModel);
includeCompiler.CompileIncludes(queryModel, trackingQuery, asyncQuery);

queryModel.TransformExpressions(new CollectionNavigationSubqueryInjector(this).Visit);
queryModel.TransformExpressions(new CollectionNavigationSubqueryInjector(this, /*shouldInject*/ false, trackingQuery).Visit);

var navigationRewritingExpressionVisitor = _navigationRewritingExpressionVisitorFactory.Create(this);
var navigationRewritingExpressionVisitor = _navigationRewritingExpressionVisitorFactory.Create(this, trackingQuery);

navigationRewritingExpressionVisitor.Rewrite(queryModel, parentQueryModel: null);

includeCompiler.CompileIncludes(queryModel, TrackResults(queryModel), asyncQuery);
includeCompiler.CompileIncludes(queryModel, trackingQuery, asyncQuery);

navigationRewritingExpressionVisitor.Rewrite(queryModel, parentQueryModel: null);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
Expand All @@ -9,6 +10,7 @@
using Microsoft.EntityFrameworkCore.Extensions.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Microsoft.EntityFrameworkCore.Utilities;
using Remotion.Linq;
using Remotion.Linq.Clauses;
Expand All @@ -25,15 +27,20 @@ public class CollectionNavigationSubqueryInjector : RelinqExpressionVisitor
{
private readonly EntityQueryModelVisitor _queryModelVisitor;
private bool _shouldInject;
private readonly bool _trackingQuery;

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public CollectionNavigationSubqueryInjector([NotNull] EntityQueryModelVisitor queryModelVisitor, bool shouldInject = false)
public CollectionNavigationSubqueryInjector(
[NotNull] EntityQueryModelVisitor queryModelVisitor,
bool shouldInject,
bool trackingQuery)
{
_queryModelVisitor = queryModelVisitor;
_shouldInject = shouldInject;
_trackingQuery = trackingQuery;
}

/// <summary>
Expand Down Expand Up @@ -101,7 +108,12 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
return newMethodCallExpression ?? base.VisitMethodCall(methodCallExpression);
}

private static Expression InjectSubquery(Expression expression, INavigation collectionNavigation)
private static readonly MethodInfo _queryBufferStartTrackingMethodInfo
= typeof(IQueryBuffer).GetTypeInfo()
.GetDeclaredMethods(nameof(IQueryBuffer.StartTracking))
.Single(mi => mi.GetParameters()[1].ParameterType == typeof(IEntityType));

private Expression InjectSubquery(Expression expression, INavigation collectionNavigation)
{
var targetType = collectionNavigation.GetTargetType().ClrType;
var mainFromClause = new MainFromClause(targetType.Name.Substring(0, 1).ToLowerInvariant(), targetType, expression);
Expand All @@ -111,22 +123,51 @@ private static Expression InjectSubquery(Expression expression, INavigation coll
var subqueryExpression = new SubQueryExpression(subqueryModel);

var resultCollectionType = collectionNavigation.GetCollectionAccessor().CollectionType;
var entityParameter = Expression.Parameter(targetType, name: "entity");

var result = Expression.Call(
MaterializeCollectionNavigationMethodInfo.MakeGenericMethod(targetType),
Expression.Constant(collectionNavigation), subqueryExpression);
Expression.Constant(collectionNavigation),
subqueryExpression,
EntityQueryModelVisitor.QueryContextParameter,
Expression.Constant(_trackingQuery),
Expression.Lambda(
Expression.Call(
Expression.Property(
EntityQueryModelVisitor.QueryContextParameter,
nameof(QueryContext.QueryBuffer)),
_queryBufferStartTrackingMethodInfo,
entityParameter,
Expression.Constant(
_queryModelVisitor.QueryCompilationContext.FindEntityType(selector.ReferencedQuerySource)
?? _queryModelVisitor.QueryCompilationContext.Model.FindEntityType(entityParameter.Type))),
entityParameter));

return resultCollectionType.GetTypeInfo().IsGenericType && resultCollectionType.GetGenericTypeDefinition() == typeof(ICollection<>)
? (Expression)result
: Expression.Convert(result, resultCollectionType);
}

[UsedImplicitly]
private static ICollection<TEntity> MaterializeCollectionNavigation<TEntity>(INavigation navigation, IEnumerable<object> elements)
private static ICollection<TEntity> MaterializeCollectionNavigation<TEntity>(
INavigation navigation,
IEnumerable<object> elements,
QueryContext queryContext,
bool trackingQuery,
Action<TEntity> trackAction)
{
var collection = navigation.GetCollectionAccessor().Create(elements);
var collection = (ICollection<TEntity>)navigation.GetCollectionAccessor().Create();
foreach (TEntity entity in elements)
{
if (entity != null && trackingQuery)
{
trackAction(entity);
}

collection.Add(entity);
}

return (ICollection<TEntity>)collection;
return collection;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ public interface INavigationRewritingExpressionVisitorFactory
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
NavigationRewritingExpressionVisitor Create([NotNull] EntityQueryModelVisitor queryModelVisitor);
NavigationRewritingExpressionVisitor Create([NotNull] EntityQueryModelVisitor queryModelVisitor, bool trackingQuery);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -206,21 +206,24 @@ private void Insert(QueryModel queryModel, ref int insertionIndex)
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public NavigationRewritingExpressionVisitor([NotNull] EntityQueryModelVisitor queryModelVisitor)
: this(queryModelVisitor, navigationExpansionSubquery: false)
public NavigationRewritingExpressionVisitor([NotNull] EntityQueryModelVisitor queryModelVisitor, bool trackingQuery)
: this(queryModelVisitor, /*navigationExpansionSubquery:*/ false, trackingQuery)
{
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public NavigationRewritingExpressionVisitor([NotNull] EntityQueryModelVisitor queryModelVisitor, bool navigationExpansionSubquery)
public NavigationRewritingExpressionVisitor(
[NotNull] EntityQueryModelVisitor queryModelVisitor,
bool navigationExpansionSubquery,
bool trackingQuery)
{
Check.NotNull(queryModelVisitor, nameof(queryModelVisitor));

_queryModelVisitor = queryModelVisitor;
_navigationRewritingQueryModelVisitor = new NavigationRewritingQueryModelVisitor(this, _queryModelVisitor, navigationExpansionSubquery);
_navigationRewritingQueryModelVisitor = new NavigationRewritingQueryModelVisitor(this, _queryModelVisitor, navigationExpansionSubquery, trackingQuery);
}

/// <summary>
Expand Down Expand Up @@ -1364,10 +1367,11 @@ private class NavigationRewritingQueryModelVisitor : ExpressionTransformingQuery
public NavigationRewritingQueryModelVisitor(
NavigationRewritingExpressionVisitor transformingVisitor,
EntityQueryModelVisitor queryModelVisitor,
bool navigationExpansionSubquery)
bool navigationExpansionSubquery,
bool trackingQuery)
: base(transformingVisitor)
{
_subqueryInjector = new CollectionNavigationSubqueryInjector(queryModelVisitor, shouldInject: true);
_subqueryInjector = new CollectionNavigationSubqueryInjector(queryModelVisitor, /*shouldInject*/ true, trackingQuery);
_navigationExpansionSubquery = navigationExpansionSubquery;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class NavigationRewritingExpressionVisitorFactory : INavigationRewritingE
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual NavigationRewritingExpressionVisitor Create(EntityQueryModelVisitor queryModelVisitor)
=> new NavigationRewritingExpressionVisitor(queryModelVisitor);
public virtual NavigationRewritingExpressionVisitor Create(EntityQueryModelVisitor queryModelVisitor, bool trackingQuery)
=> new NavigationRewritingExpressionVisitor(queryModelVisitor, trackingQuery);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,41 @@ FROM [Orders] AS [o]
//
@"@_outer_CustomerID: AROUT (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]");
}

public override void Select_entity_and_collection_navigation()
{
base.Select_entity_and_collection_navigation();

AssertSql(
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] LIKE N'A' + N'%' AND (LEFT([c].[CustomerID], LEN(N'A')) = N'A')
ORDER BY [c].[CustomerID]",
//
@"@_outer_CustomerID: ALFKI (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: ANATR (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: ANTON (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: AROUT (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]");
Expand Down Expand Up @@ -555,6 +590,41 @@ FROM [Orders] AS [o0]
WHERE @_outer_CustomerID = [o0].[CustomerID]");
}

public override void Elements_of_materialized_collection_navigation_not_tracked_for_queries_with_AsNoTracking()
{
base.Elements_of_materialized_collection_navigation_not_tracked_for_queries_with_AsNoTracking();

AssertSql(
@"SELECT [c].[CustomerID]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] LIKE N'A' + N'%' AND (LEFT([c].[CustomerID], LEN(N'A')) = N'A')
ORDER BY [c].[CustomerID]",
//
@"@_outer_CustomerID: ALFKI (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: ANATR (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: ANTON (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]",
//
@"@_outer_CustomerID: AROUT (Size = 450)
SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE @_outer_CustomerID = [o].[CustomerID]");
}

public override void Collection_select_nav_prop_any()
{
base.Collection_select_nav_prop_any();
Expand Down

0 comments on commit 049eb62

Please sign in to comment.