Allow use of EF.Property for Include (#28411)

src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.EntityFrameworkCore.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Query.Internal;
@@ -1070,6 +1071,11 @@ private NavigationExpansionExpression ProcessInclude(
 
             if (currentExpression is MethodCallExpression methodCallExpression)
             {
+                if (methodCallExpression.Method.IsEFPropertyMethod())
+                {
+                    return (currentExpression, default);
+                }
+
                 if (!methodCallExpression.Method.IsGenericMethod
                     || !SupportedFilteredIncludeOperations.Contains(methodCallExpression.Method.GetGenericMethodDefinition()))
                 {
@@ -2037,48 +2043,62 @@ private IncludeTreeNode PopulateIncludeTree(IncludeTreeNode includeTreeNode, Exp
             case ParameterExpression:
                 return includeTreeNode;
 
-            case MemberExpression memberExpression
-                when memberExpression.Expression != null:
-                var innerExpression = memberExpression.Expression.UnwrapTypeConversion(out var convertedType);
-                var innerIncludeTreeNode = PopulateIncludeTree(includeTreeNode, innerExpression, setLoaded);
-                var entityType = innerIncludeTreeNode.EntityType;
-                if (convertedType != null)
+            case MethodCallExpression methodCallExpression
+                when methodCallExpression.TryGetEFPropertyArguments(out var entityExpression, out var propertyName):
+                if (TryExtractIncludeTreeNode(entityExpression, propertyName, out var addedEfPropertyNode))
                 {
-                    entityType = entityType.GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive())
-                        .FirstOrDefault(et => et.ClrType == convertedType);
-                    if (entityType == null)
-                    {
-                        throw new InvalidOperationException(
-                            CoreStrings.InvalidTypeConversationWithInclude(expression, convertedType.ShortDisplayName()));
-                    }
+                    return addedEfPropertyNode;
                 }
 
-                var navigation = entityType.FindNavigation(memberExpression.Member);
-                if (navigation != null)
-                {
-                    var addedNode = innerIncludeTreeNode.AddNavigation(navigation, setLoaded);
-
-                    // This is to add eager Loaded navigations when owner type is included.
-                    PopulateEagerLoadedNavigations(addedNode);
-
-                    return addedNode;
-                }
+                break;
 
-                var skipNavigation = entityType.FindSkipNavigation(memberExpression.Member);
-                if (skipNavigation != null)
+            case MemberExpression { Expression: { } } memberExpression:
+                if (TryExtractIncludeTreeNode(memberExpression.Expression, memberExpression.Member.Name, out var addedNode))
                 {
-                    var addedNode = innerIncludeTreeNode.AddNavigation(skipNavigation, setLoaded);
-
-                    // This is to add eager Loaded navigations when owner type is included.
-                    PopulateEagerLoadedNavigations(addedNode);
-
                     return addedNode;
                 }
 
                 break;
         }
 
         throw new InvalidOperationException(CoreStrings.InvalidIncludeExpression(expression));
+
+        bool TryExtractIncludeTreeNode(
+            Expression innerExpression, 
+            string propertyName,
+            [NotNullWhen(true)] out IncludeTreeNode? addedNode)
+        {
+            innerExpression = innerExpression.UnwrapTypeConversion(out var convertedType);
+            var innerIncludeTreeNode = PopulateIncludeTree(includeTreeNode, innerExpression, setLoaded);
+            var entityType = innerIncludeTreeNode.EntityType;
+
+            if (convertedType != null)
+            {
+                entityType = entityType.GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive())
+                    .FirstOrDefault(et => et.ClrType == convertedType);
+                if (entityType == null)
+                {
+                    throw new InvalidOperationException(
+                        CoreStrings.InvalidTypeConversationWithInclude(expression, convertedType.ShortDisplayName()));
+                }
+            }
+
+            var navigation = (INavigationBase?)entityType.FindNavigation(propertyName)
+                ?? entityType.FindSkipNavigation(propertyName);
+
+            if (navigation != null)
+            {
+                addedNode = innerIncludeTreeNode.AddNavigation(navigation, setLoaded);
+
+                // This is to add eager Loaded navigations when owner type is included.
+                PopulateEagerLoadedNavigations(addedNode);
+
+                return true;
+            }
+
+            addedNode = null;
+            return false;
+        }
     }
 
     private Expression Reduce(Expression source)

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs

@@ -4146,6 +4146,13 @@ public override async Task String_include_on_incorrect_property_throws(bool asyn
         AssertSql();
     }
 
+    public override async Task EF_Property_include_on_incorrect_property_throws(bool async)
+    {
+        await base.EF_Property_include_on_incorrect_property_throws(async);
+
+        AssertSql();
+    }
+
     public override async Task SkipWhile_throws_meaningful_exception(bool async)
     {
         await base.SkipWhile_throws_meaningful_exception(async);

test/EFCore.InMemory.FunctionalTests/Query/NorthwindEFPropertyIncludeQueryInMemoryTest.cs

@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Query;
+
+public class NorthwindEFPropertyIncludeQueryInMemoryTest : NorthwindEFPropertyIncludeQueryTestBase<
+    NorthwindQueryInMemoryFixture<NoopModelCustomizer>>
+{
+    public NorthwindEFPropertyIncludeQueryInMemoryTest(
+        NorthwindQueryInMemoryFixture<NoopModelCustomizer> fixture,
+        ITestOutputHelper testOutputHelper)
+        : base(fixture)
+    {
+        //TestLoggerFactory.TestOutputHelper = testOutputHelper;
+    }
+}

test/EFCore.Specification.Tests/Query/ComplexNavigationsCollectionsQueryTestBase.cs

@@ -612,6 +612,23 @@ public virtual Task Multiple_complex_includes(bool async)
                 new ExpectedInclude<Level1>(l1 => l1.OneToMany_Optional1),
                 new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2, "OneToMany_Optional1")));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Multiple_complex_includes_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .Include(e => EF.Property<Level2>(e, "OneToOne_Optional_FK1"))
+                .ThenInclude(e => EF.Property<ICollection<Level3>>(e, "OneToMany_Optional2"))
+                .Include(e => EF.Property<ICollection<Level2>>(e, "OneToMany_Optional1"))
+                .ThenInclude(e => EF.Property<Level3>(e, "OneToOne_Optional_FK2")),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a,
+                new ExpectedInclude<Level1>(l1 => l1.OneToOne_Optional_FK1),
+                new ExpectedInclude<Level2>(l2 => l2.OneToMany_Optional2, "OneToOne_Optional_FK1"),
+                new ExpectedInclude<Level1>(l1 => l1.OneToMany_Optional1),
+                new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2, "OneToMany_Optional1")));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Multiple_complex_includes_self_ref(bool async)
@@ -628,6 +645,22 @@ public virtual Task Multiple_complex_includes_self_ref(bool async)
                 new ExpectedInclude<Level1>(l1 => l1.OneToMany_Optional_Self1),
                 new ExpectedInclude<Level1>(l2 => l2.OneToOne_Optional_Self1, "OneToMany_Optional_Self1")));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Multiple_complex_includes_self_ref_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .Include(e => EF.Property<Level1>(e, "OneToOne_Optional_Self1"))
+                .ThenInclude(e => EF.Property<ICollection<Level1>>(e, "OneToMany_Optional_Self1"))
+                .Include(e => EF.Property<ICollection<Level1>>(e, "OneToMany_Optional_Self1"))
+                .ThenInclude(e => EF.Property<Level1>(e, "OneToOne_Optional_Self1")),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a, new ExpectedInclude<Level1>(l1 => l1.OneToOne_Optional_Self1),
+                new ExpectedInclude<Level1>(l2 => l2.OneToMany_Optional_Self1, "OneToOne_Optional_Self1"),
+                new ExpectedInclude<Level1>(l1 => l1.OneToMany_Optional_Self1),
+                new ExpectedInclude<Level1>(l2 => l2.OneToOne_Optional_Self1, "OneToMany_Optional_Self1")));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Include_reference_and_collection_order_by(bool async)
@@ -1310,6 +1343,21 @@ public virtual Task Include_collection_multiple_with_filter(bool async)
                 new ExpectedInclude<Level2>(e2 => e2.OneToOne_Optional_PK2, "OneToMany_Optional1"),
                 new ExpectedInclude<Level3>(e3 => e3.OneToOne_Optional_FK3, "OneToMany_Optional1.OneToOne_Optional_PK2")));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Include_collection_multiple_with_filter_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .Include(l1 => EF.Property<ICollection<Level2>>(l1, "OneToMany_Optional1"))
+                .ThenInclude(l2 => EF.Property<Level3>(l2, "OneToOne_Optional_PK2"))
+                .ThenInclude(l3 => EF.Property<Level4>(l3, "OneToOne_Optional_FK3"))
+                .Where(l1 => EF.Property<ICollection<Level2>>(l1, "OneToMany_Optional1").Where(l2 => l2.OneToOne_Optional_PK2.Name != "Foo").Count() > 0),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a, new ExpectedInclude<Level1>(e1 => e1.OneToMany_Optional1),
+                new ExpectedInclude<Level2>(e2 => e2.OneToOne_Optional_PK2, "OneToMany_Optional1"),
+                new ExpectedInclude<Level3>(e3 => e3.OneToOne_Optional_FK3, "OneToMany_Optional1.OneToOne_Optional_PK2")));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Including_reference_navigation_and_projecting_collection_navigation(bool async)
@@ -1353,6 +1401,18 @@ public virtual Task Filtered_include_basic_Where(bool async)
                     e => e.OneToMany_Optional1,
                     includeFilter: x => x.Where(l2 => l2.Id > 5))));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Filtered_include_basic_Where_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>().Include(l1 => EF.Property<ICollection<Level2>>(l1, "OneToMany_Optional1").Where(l2 => l2.Id > 5)),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a,
+                new ExpectedFilteredInclude<Level1, Level2>(
+                    e => e.OneToMany_Optional1,
+                    includeFilter: x => x.Where(l2 => l2.Id > 5))));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Filtered_include_OrderBy(bool async)
@@ -1366,6 +1426,19 @@ public virtual Task Filtered_include_OrderBy(bool async)
                     includeFilter: x => x.OrderBy(x => x.Name),
                     assertOrder: true)));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Filtered_include_OrderBy_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>().Include(l1 => EF.Property<ICollection<Level2>>(l1, "OneToMany_Optional1").OrderBy(x => x.Name)),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a,
+                new ExpectedFilteredInclude<Level1, Level2>(
+                    e => e.OneToMany_Optional1,
+                    includeFilter: x => x.OrderBy(x => x.Name),
+                    assertOrder: true)));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Filtered_ThenInclude_OrderBy(bool async)
@@ -1440,6 +1513,20 @@ public virtual Task Filtered_include_basic_OrderBy_Skip_Take(bool async)
                     includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3),
                     assertOrder: true)));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Filtered_include_basic_OrderBy_Skip_Take_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>().Include(l1 => EF.Property<ICollection<Level2>>(l1, "OneToMany_Optional1")
+                .OrderBy(x => x.Name).Skip(1).Take(3)),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a,
+                new ExpectedFilteredInclude<Level1, Level2>(
+                    e => e.OneToMany_Optional1,
+                    includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3),
+                    assertOrder: true)));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Filtered_include_Skip_without_OrderBy(bool async)
@@ -1483,6 +1570,24 @@ public virtual Task Filtered_include_on_ThenInclude(bool async)
                     x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3),
                     assertOrder: true)));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Filtered_include_on_ThenInclude_EF_Property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .Include(l1 => EF.Property<Level2>(l1, "OneToOne_Optional_FK1"))
+                .ThenInclude(l2 => EF.Property<ICollection<Level3>>(l2, "OneToMany_Optional2")
+                    .Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3)),
+            elementAsserter: (e, a) => AssertInclude(
+                e, a,
+                new ExpectedInclude<Level1>(e => e.OneToOne_Optional_FK1),
+                new ExpectedFilteredInclude<Level2, Level3>(
+                    e => e.OneToMany_Optional2,
+                    "OneToOne_Optional_FK1",
+                    x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3),
+                    assertOrder: true)));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Filtered_include_after_reference_navigation(bool async)

test/EFCore.Specification.Tests/Query/ComplexNavigationsCollectionsSharedTypeQueryTestBase.cs

@@ -18,6 +18,12 @@ public override async Task Multiple_complex_includes_self_ref(bool async)
             (await Assert.ThrowsAsync<InvalidOperationException>(
                 () => base.Multiple_complex_includes_self_ref(async))).Message);
 
+    public override async Task Multiple_complex_includes_self_ref_EF_Property(bool async)
+        => Assert.Equal(
+            CoreStrings.InvalidIncludeExpression("Property(e, \"OneToOne_Optional_Self1\")"),
+            (await Assert.ThrowsAsync<InvalidOperationException>(
+                () => base.Multiple_complex_includes_self_ref_EF_Property(async))).Message);
+
     public override Task
         Complex_SelectMany_with_nested_navigations_and_explicit_DefaultIfEmpty_with_other_query_operators_composed_on_top(bool async)
         => AssertTranslationFailed(

test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs

@@ -1270,6 +1270,16 @@ public virtual Task SelectMany_with_string_based_Include1(bool async)
                 .Include("OneToOne_Required_FK2"),
             elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Required_FK2)));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task SelectMany_with_EF_Property_Include1(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .SelectMany(l1 => l1.OneToMany_Optional1)
+                .Include(l2 => EF.Property<Level2>(l2, "OneToOne_Required_FK2")),
+            elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Required_FK2)));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task SelectMany_with_string_based_Include2(bool async)
@@ -1293,6 +1303,17 @@ public virtual Task Multiple_SelectMany_with_string_based_Include(bool async)
                 .Include("OneToOne_Required_FK3"),
             elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level3>(l3 => l3.OneToOne_Required_FK3)));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Multiple_SelectMany_with_EF_Property_Include(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Level1>()
+                .SelectMany(l1 => l1.OneToMany_Optional1)
+                .SelectMany(l1 => l1.OneToMany_Optional2)
+                .Include(l3 => EF.Property<Level3>(l3, "OneToOne_Required_FK3")),
+            elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level3>(l3 => l3.OneToOne_Required_FK3)));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Multiple_required_navigations_with_Include(bool async)
@@ -1330,6 +1351,19 @@ public virtual Task Multiple_required_navigation_with_string_based_Include(bool
                     .Include("OneToOne_Optional_FK2"),
                 elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2))));
 
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Multiple_required_navigation_with_EF_Property_Include(bool async)
+        // Include after select. Issue #16752.
+        => AssertIncludeOnNonEntity(
+            () => AssertQuery(
+                async,
+                ss => ss.Set<Level4>()
+                    .Select(l4 => l4.OneToOne_Required_FK_Inverse4.OneToOne_Required_FK_Inverse3)
+                    .Include(l2 => EF.Property<Level2>(l2, "OneToOne_Optional_FK2")),
+                elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2))));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Multiple_required_navigation_using_multiple_selects_with_string_based_Include(bool async)
@@ -1343,6 +1377,19 @@ public virtual Task Multiple_required_navigation_using_multiple_selects_with_str
                     .Include("OneToOne_Optional_FK2"),
                 elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2))));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Multiple_required_navigation_using_multiple_selects_with_EF_Property_Include(bool async)
+        // Include after select. Issue #16752.
+        => AssertIncludeOnNonEntity(
+            () => AssertQuery(
+                async,
+                ss => ss.Set<Level4>()
+                    .Select(l4 => l4.OneToOne_Required_FK_Inverse4)
+                    .Select(l3 => l3.OneToOne_Required_FK_Inverse3)
+                    .Include(l2 => EF.Property<Level2>(l2, "OneToOne_Optional_FK2")),
+                elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedInclude<Level2>(l2 => l2.OneToOne_Optional_FK2))));
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Optional_navigation_with_Include(bool async)