Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow use of EF.Property for Include #28411

Merged
merged 1 commit into from
Jul 12, 2022
Merged

Allow use of EF.Property for Include #28411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 50 additions & 30 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()))
{
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/NorthwindEFPropertyIncludeQueryInMemoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}
}
105 changes: 105 additions & 0 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsCollectionsQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions .../EFCore.Specification.Tests/Query/ComplexNavigationsCollectionsSharedTypeQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
47 changes: 47 additions & 0 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Loading