Fix to #1833 - Support filtered Include

Allows for additional operations to be specified inside Include/ThenInclude expression when the navigation is a collection: - Where, - OrderBy(Descending)/ThenBy(Descending), - Skip, - Take. Those additional operations are treated like any other within the query, so translation restrictions apply. Collections included using new filter operations are considered to be loaded. Only one filter is allowed per navigation. In cases where same navigation is included multiple times (e.g. Include(A).ThenInclude(A_B).Include(A).ThenInclude(A_C)) filter should only be applied once. Alternatively the same exact filter should be applied to all.
dotnet · Mar 24, 2020 · 21b9a35 · 21b9a35 · cgountanis · Mar 30, 2020
1 parent c9f9c33
commit 21b9a35
Show file tree

Hide file tree

Showing 13 changed files with 1,104 additions and 11 deletions.
diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs
diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx
@@ -747,8 +747,8 @@
   <data name="AmbiguousForeignKeyPropertyCandidates" xml:space="preserve">
     <value>Both relationships between '{firstDependentToPrincipalNavigationSpecification}' and '{firstPrincipalToDependentNavigationSpecification}' and between '{secondDependentToPrincipalNavigationSpecification}' and '{secondPrincipalToDependentNavigationSpecification}' could use {foreignKeyProperties} as the foreign key. To resolve this configure the foreign key properties explicitly on at least one of the relationships.</value>
   </data>
-  <data name="InvalidIncludeLambdaExpression" xml:space="preserve">
-    <value>The {methodName} property lambda expression '{includeLambdaExpression}' is invalid. The expression should represent a property access: 't =&gt; t.MyProperty'. To target navigations declared on derived types, specify an explicitly typed lambda parameter of the target type, E.g. '(Derived d) =&gt; d.MyProperty'. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
+  <data name="InvalidIncludeExpression" xml:space="preserve">
+    <value>The expression '{expression}' is invalid inside Include operation. The expression should represent a property access: 't =&gt; t.MyProperty'. To target navigations declared on derived types use cast, e.g. 't =&gt; ((Derived)t).MyProperty' or 'as' operator, e.g. 't =&gt; (t as Derived).MyProperty'. Collection navigation access can be filtered by composing Where, OrderBy(Descending), ThenBy(Descending), Skip or Take operations. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
   </data>
   <data name="AbstractLeafEntityType" xml:space="preserve">
     <value>The corresponding CLR type for entity type '{entityType}' is not instantiable and there is no derived entity type in the model that corresponds to a concrete CLR type.</value>
@@ -1290,6 +1290,9 @@
   <data name="IncludeOnNonEntity" xml:space="preserve">
     <value>Include has been used on non entity queryable.</value>
   </data>
+  <data name="MultipleFilteredIncludesOnSameNavigation" xml:space="preserve">
+    <value>Different filters: '{filter1}' and '{filter2}' have been applied on the same included navigation. Only one unique filter per navigation is allowed. For more information on including related data, see http://go.microsoft.com/fwlink/?LinkID=746393.</value>
+  </data>
   <data name="CannotConvertQueryableToEnumerableMethod" xml:space="preserve">
     <value>Unable to convert queryable method to enumerable method.</value>
   </data>

diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs
@@ -181,9 +181,8 @@ private Expression TryExpandNavigation(Expression root, MemberIdentity memberIde
                 var innerSource = (NavigationExpansionExpression)_navigationExpandingExpressionVisitor.Visit(innerQueryable);
                 if (entityReference.IncludePaths.ContainsKey(navigation))
                 {
-                    var innerIncludeTreeNode = entityReference.IncludePaths[navigation];
                     var innerEntityReference = (EntityReference)((NavigationTreeExpression)innerSource.PendingSelector).Value;
-                    innerEntityReference.SetIncludePaths(innerIncludeTreeNode);
+                    innerEntityReference.SetIncludePaths(entityReference.IncludePaths[navigation]);
                 }
 
                 var innerSourceSequenceType = innerSource.Type.GetSequenceType();
@@ -532,6 +531,20 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR
                         included = ExpandIncludesHelper(included, innerEntityReference);
                     }
 
+                    if (included is MaterializeCollectionNavigationExpression materializeCollectionNavigation)
+                    {
+                        var filterExpression = entityReference.IncludePaths[navigation].FilterExpression;
+                        if (filterExpression != null)
+                        {
+                            var subquery = ReplacingExpressionVisitor.Replace(
+                                filterExpression.Parameters[0],
+                                materializeCollectionNavigation.Subquery,
+                                filterExpression.Body);
+
+                            included = materializeCollectionNavigation.Update(subquery);
+                        }
+                    }
+
                     result = new IncludeExpression(result, included, navigation);
                 }
 

diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.Expressions.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.Expressions.cs
@@ -84,6 +84,8 @@ protected class IncludeTreeNode : Dictionary<INavigation, IncludeTreeNode>
         {
             private EntityReference _entityReference;
 
+            public virtual LambdaExpression FilterExpression { get; set; }
+
             public IncludeTreeNode(IEntityType entityType, EntityReference entityReference)
             {
                 EntityType = entityType;

diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
@@ -32,6 +32,19 @@ public partial class NavigationExpandingExpressionVisitor : ExpressionVisitor
             { QueryableMethods.LastWithPredicate, QueryableMethods.LastWithoutPredicate },
             { QueryableMethods.LastOrDefaultWithPredicate, QueryableMethods.LastOrDefaultWithoutPredicate }
         };
+
+        private static readonly List<MethodInfo> _supportedFilteredIncludeOperations = new List<MethodInfo>
+        {
+            QueryableMethods.Where,
+            QueryableMethods.OrderBy,
+            QueryableMethods.OrderByDescending,
+            QueryableMethods.ThenBy,
+            QueryableMethods.ThenByDescending,
+            QueryableMethods.Skip,
+            QueryableMethods.Take,
+            QueryableMethods.AsQueryable
+        };
+
         private readonly QueryTranslationPreprocessor _queryTranslationPreprocessor;
         private readonly QueryCompilationContext _queryCompilationContext;
         private readonly PendingSelectorExpandingExpressionVisitor _pendingSelectorExpandingExpressionVisitor;
@@ -768,6 +781,7 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
                             foreach (var navigation in FindNavigations(currentNode.EntityType, navigationName))
                             {
                                 var addedNode = currentNode.AddNavigation(navigation);
+
                                 // This is to add eager Loaded navigations when owner type is included.
                                 PopulateEagerLoadedNavigations(addedNode);
                                 includeTreeNodes.Enqueue(addedNode);
@@ -786,19 +800,92 @@ private NavigationExpansionExpression ProcessInclude(NavigationExpansionExpressi
                         ? entityReference.LastIncludeTreeNode
                         : entityReference.IncludePaths;
                     var includeLambda = expression.UnwrapLambdaFromQuote();
-                    var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, includeLambda.Body);
+
+                    var (result, filterExpression) = ExtractIncludeFilter(includeLambda.Body, includeLambda.Body);
+                    var lastIncludeTree = PopulateIncludeTree(currentIncludeTreeNode, result);
                     if (lastIncludeTree == null)
                     {
                         throw new InvalidOperationException(CoreStrings.InvalidLambdaExpressionInsideInclude);
                     }
 
+                    if (filterExpression != null)
+                    {
+                        if (lastIncludeTree.FilterExpression != null
+                            && !ExpressionEqualityComparer.Instance.Equals(filterExpression, lastIncludeTree.FilterExpression))
+                        {
+                            throw new InvalidOperationException(
+                                CoreStrings.MultipleFilteredIncludesOnSameNavigation(
+                                    FormatFilter(filterExpression.Body).Print(),
+                                    FormatFilter(lastIncludeTree.FilterExpression.Body).Print()));
+                        }
+
+                        lastIncludeTree.FilterExpression = filterExpression;
+                    }
+
                     entityReference.SetLastInclude(lastIncludeTree);
                 }
 
                 return source;
             }
 
             throw new InvalidOperationException(CoreStrings.IncludeOnNonEntity);
+
+            static (Expression result, LambdaExpression filterExpression) ExtractIncludeFilter(Expression currentExpression, Expression includeExpression)
+            {
+                if (currentExpression is MemberExpression)
+                {
+                    return (currentExpression, default(LambdaExpression));
+                }
+
+                if (currentExpression is MethodCallExpression methodCallExpression)
+                {
+                    if (!methodCallExpression.Method.IsGenericMethod
+                        || !_supportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
+                    {
+                        throw new InvalidOperationException(CoreStrings.InvalidIncludeExpression(includeExpression));
+                    }
+
+                    var (result, filterExpression) = ExtractIncludeFilter(methodCallExpression.Arguments[0], includeExpression);
+                    if (filterExpression == null)
+                    {
+                        var prm = Expression.Parameter(result.Type);
+                        filterExpression = Expression.Lambda(prm, prm);
+                    }
+
+                    var arguments = new List<Expression>();
+                    arguments.Add(filterExpression.Body);
+                    arguments.AddRange(methodCallExpression.Arguments.Skip(1));
+                    filterExpression = Expression.Lambda(
+                        methodCallExpression.Update(methodCallExpression.Object, arguments),
+                        filterExpression.Parameters);
+
+                    return (result, filterExpression);
+                }
+
+                throw new InvalidOperationException(CoreStrings.InvalidIncludeExpression(includeExpression));
+            }
+
+            static Expression FormatFilter(Expression expression)
+            {
+                if (expression is MethodCallExpression methodCallExpression
+                    && methodCallExpression.Method.IsGenericMethod
+                    && _supportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
+                {
+                    if (methodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.AsQueryable)
+                    {
+                        return Expression.Parameter(expression.Type, "navigation");
+                    }
+
+                    var arguments = new List<Expression>();
+                    var source = FormatFilter(methodCallExpression.Arguments[0]);
+                    arguments.Add(source);
+                    arguments.AddRange(methodCallExpression.Arguments.Skip(1));
+
+                    return methodCallExpression.Update(methodCallExpression.Object, arguments);
+                }
+
+                return expression;
+            }
         }
 
         private NavigationExpansionExpression ProcessJoin(
@@ -1474,8 +1561,10 @@ private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Exp
                     if (navigation != null)
                     {
                         var addedNode = innerIncludeTreeNode.AddNavigation(navigation);
+
                         // This is to add eager Loaded navigations when owner type is included.
                         PopulateEagerLoadedNavigations(addedNode);
+
                         return addedNode;
                     }