Skip to content

[SMALL] Add capability to verify order in ordered filtered include queries #21305

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5006,7 +5006,8 @@ public virtual Task Filtered_include_OrderBy(bool async)
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.OrderBy(x => x.Name))));
includeFilter: x => x.OrderBy(x => x.Name),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5021,7 +5022,8 @@ public virtual Task Filtered_ThenInclude_OrderBy(bool async)
new ExpectedFilteredInclude<Level2, Level3>(
e => e.OneToMany_Optional2,
"OneToMany_Optional1",
includeFilter: x => x.OrderBy(x => x.Name))));
includeFilter: x => x.OrderBy(x => x.Name),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5036,11 +5038,13 @@ public virtual Task Filtered_include_ThenInclude_OrderBy(bool async)
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.OrderBy(x => x.Name)),
includeFilter: x => x.OrderBy(x => x.Name),
assertOrder: true),
new ExpectedFilteredInclude<Level2, Level3>(
e => e.OneToMany_Optional2,
"OneToMany_Optional1",
includeFilter: x => x.OrderByDescending(x => x.Name))));
includeFilter: x => x.OrderByDescending(x => x.Name),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5053,7 +5057,8 @@ public virtual Task Filtered_include_basic_OrderBy_Take(bool async)
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.OrderBy(x => x.Name).Take(3))));
includeFilter: x => x.OrderBy(x => x.Name).Take(3),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5066,7 +5071,8 @@ public virtual Task Filtered_include_basic_OrderBy_Skip(bool async)
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.OrderBy(x => x.Name).Skip(1))));
includeFilter: x => x.OrderBy(x => x.Name).Skip(1),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5079,7 +5085,8 @@ public virtual Task Filtered_include_basic_OrderBy_Skip_Take(bool async)
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))));
includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3),
assertOrder: true)));
}

[ConditionalFact]
Expand Down Expand Up @@ -5112,7 +5119,8 @@ public virtual Task Filtered_include_on_ThenInclude(bool async)
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))));
x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5128,10 +5136,11 @@ public virtual Task Filtered_include_after_reference_navigation(bool async)
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))));
x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3),
assertOrder: true)));
}

[ConditionalTheory]
[ConditionalTheory(Skip = "issue #21338")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_after_different_filtered_include_same_level(bool async)
{
Expand All @@ -5143,10 +5152,12 @@ public virtual Task Filtered_include_after_different_filtered_include_same_level
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3),
assertOrder: true),
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Required1,
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1))));
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5161,11 +5172,13 @@ public virtual Task Filtered_include_after_different_filtered_include_different_
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3),
assertOrder: true),
new ExpectedFilteredInclude<Level2, Level3>(
e => e.OneToMany_Required2,
"OneToMany_Optional1",
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1))));
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5192,7 +5205,7 @@ public virtual async Task Filtered_include_different_filter_set_on_same_navigati
.Include(l1 => l1.OneToMany_Optional1.Where(x => x.Name != "Bar")).ThenInclude(l2 => l2.OneToOne_Required_FK2)))).Message;
}

[ConditionalTheory]
[ConditionalTheory(Skip = "issue #21338")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice(bool async)
{
Expand All @@ -5204,7 +5217,8 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice(bo
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderByDescending(x => x.Id).Take(2))));
includeFilter: x => x.Where(x => x.Name != "Foo").OrderByDescending(x => x.Id).Take(2),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5219,7 +5233,8 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_fo
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2),
assertOrder: true),
new ExpectedInclude<Level2>(e => e.OneToMany_Optional2),
new ExpectedInclude<Level2>(e => e.OneToOne_Required_FK2)));
}
Expand All @@ -5236,7 +5251,8 @@ public virtual Task Filtered_include_multiple_multi_level_includes_with_first_le
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2),
assertOrder: true),
new ExpectedInclude<Level2>(e => e.OneToMany_Optional2, "OneToMany_Optional1"),
new ExpectedInclude<Level2>(e => e.OneToOne_Required_FK2, "OneToMany_Optional1")));
}
Expand All @@ -5253,7 +5269,8 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3))));
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5268,7 +5285,8 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3))));
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3),
assertOrder: true)));
}

[ConditionalTheory]
Expand All @@ -5283,7 +5301,8 @@ public virtual Task Filtered_include_and_non_filtered_include_followed_by_then_i
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
e => e.OneToMany_Optional1,
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1),
assertOrder: true),
new ExpectedInclude<Level2>(e => e.OneToOne_Optional_PK2, "OneToMany_Optional1"),
new ExpectedFilteredInclude<Level3, Level4>(
e => e.OneToMany_Optional3,
Expand All @@ -5305,7 +5324,8 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt
new ExpectedFilteredInclude<Level2, Level3>(
e => e.OneToMany_Optional2,
"OneToMany_Optional1",
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1),
assertOrder: true),
new ExpectedInclude<Level3>(e => e.OneToMany_Optional3, "OneToMany_Optional1.OneToMany_Optional2"),
new ExpectedInclude<Level3>(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2")));
}
Expand All @@ -5324,7 +5344,8 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt
new ExpectedFilteredInclude<Level2, Level3>(
e => e.OneToMany_Optional2,
"OneToMany_Optional1",
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)),
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1),
assertOrder: true),
new ExpectedInclude<Level3>(e => e.OneToMany_Optional3, "OneToMany_Optional1.OneToMany_Optional2"),
new ExpectedInclude<Level3>(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2")));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1800,7 +1800,8 @@ public virtual Task Filtered_include_with_multiple_ordering(bool async)
.Include(c => c.Orders.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate)),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Customer, Order>(c => c.Orders,
includeFilter: os => os.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate))),
includeFilter: os => os.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate),
assertOrder: true)),
entryCount: 64);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ public class ExpectedFilteredInclude<TEntity, TIncluded> : ExpectedInclude<TEnti
{
public Func<IEnumerable<TIncluded>, IEnumerable<TIncluded>> IncludeFilter { get; }

public bool AssertOrder { get; }

public ExpectedFilteredInclude(
Expression<Func<TEntity, IEnumerable<TIncluded>>> include,
string navigationPath = "",
Func<IEnumerable<TIncluded>, IEnumerable<TIncluded>> includeFilter = null)
Func<IEnumerable<TIncluded>, IEnumerable<TIncluded>> includeFilter = null,
bool assertOrder = false)
: base(Convert(include), navigationPath)
{
IncludeFilter = includeFilter;
AssertOrder = assertOrder;
}

private static Expression<Func<TEntity, object>> Convert(Expression<Func<TEntity, IEnumerable<TIncluded>>> include)
Expand Down
20 changes: 14 additions & 6 deletions test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1493,10 +1493,10 @@ public void AssertInclude<TEntity>(TEntity expected, TEntity actual, IExpectedIn
{
_includePath.Clear();

AssertIncludeObject(expected, actual, expectedIncludes);
AssertIncludeObject(expected, actual, expectedIncludes, assertOrder: false);
}

private void AssertIncludeObject(object expected, object actual, IEnumerable<IExpectedInclude> expectedIncludes)
private void AssertIncludeObject(object expected, object actual, IEnumerable<IExpectedInclude> expectedIncludes, bool assertOrder)
{
if (expected == null
&& actual == null)
Expand All @@ -1512,7 +1512,7 @@ private void AssertIncludeObject(object expected, object actual, IEnumerable<IEx
i => i.IsConstructedGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>)))
{
_assertIncludeCollectionMethodInfo.MakeGenericMethod(expectedType.GenericTypeArguments[0])
.Invoke(this, new[] { expected, actual, expectedIncludes });
.Invoke(this, new[] { expected, actual, expectedIncludes, assertOrder });
}
else
{
Expand All @@ -1534,12 +1534,15 @@ private void AssertIncludeEntity<TElement>(TElement expected, TElement actual, I
}

private void AssertIncludeCollection<TElement>(
IEnumerable<TElement> expected, IEnumerable<TElement> actual, IEnumerable<IExpectedInclude> expectedIncludes)
IEnumerable<TElement> expected,
IEnumerable<TElement> actual,
IEnumerable<IExpectedInclude> expectedIncludes,
bool assertOrder)
{
var expectedList = expected.ToList();
var actualList = actual.ToList();

if (_entitySorters.TryGetValue(typeof(TElement), out var sorter))
if (!assertOrder && _entitySorters.TryGetValue(typeof(TElement), out var sorter))
{
var actualSorter = (Func<TElement, object>)sorter;
expectedList = expectedList.OrderBy(actualSorter).ToList();
Expand All @@ -1563,6 +1566,7 @@ private void ProcessIncludes<TEntity>(TEntity expected, TEntity actual, IEnumera
foreach (var expectedInclude in expectedIncludes.OfType<ExpectedInclude<TEntity>>().Where(i => i.NavigationPath == currentPath))
{
var expectedIncludedNavigation = GetIncluded(expected, expectedInclude.IncludeMember);
var assertOrder = false;
if (expectedInclude.GetType().BaseType != typeof(object))
{
var includedType = expectedInclude.GetType().GetGenericArguments()[1];
Expand All @@ -1573,13 +1577,17 @@ private void ProcessIncludes<TEntity>(TEntity expected, TEntity actual, IEnumera
null,
new object[] { expectedIncludedNavigation, expectedInclude },
CultureInfo.CurrentCulture);

assertOrder = (bool)expectedInclude.GetType()
.GetProperty(nameof(ExpectedFilteredInclude<object, object>.AssertOrder))
.GetValue(expectedInclude);
}

var actualIncludedNavigation = GetIncluded(actual, expectedInclude.IncludeMember);

_includePath.Add(expectedInclude.IncludeMember.Name);

AssertIncludeObject(expectedIncludedNavigation, actualIncludedNavigation, expectedIncludes);
AssertIncludeObject(expectedIncludedNavigation, actualIncludedNavigation, expectedIncludes, assertOrder);

_includePath.RemoveAt(_includePath.Count - 1);
}
Expand Down