diff --git a/readme.md b/readme.md index 131fca4..b6542a4 100644 --- a/readme.md +++ b/readme.md @@ -16,6 +16,7 @@ https://nuget.org/packages/EfOrderBy/ - **Automatic ordering**: Queries without explicit `OrderBy` automatically use configured default ordering - **Include() support**: Nested collections in `.Include()` expressions are automatically ordered +- **Projection support**: Collection navigations loaded by a `.Select()` projection are automatically ordered - **Deterministic paging**: Ordering is applied before `Skip`/`Take`/`First`, so pages and single results are stable - **Inheritance support**: Ordering configured on a base entity type is automatically inherited by derived types (TPH) - **Fluent configuration**: Configure default ordering using the familiar EF Core fluent API @@ -132,6 +133,34 @@ var departments = await context.Departments +## Projection Support + +A collection navigation read inside a `Select` projection is loaded by a subquery of its own, and is ordered the same way as one in an `Include`: + + + +```cs +// Departments ordered by DisplayOrder +// Employees ordered by HireDate, then Salary descending +var departments = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Salary > 0) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); +``` +snippet source | anchor + + +The ordering goes after any `Where` filters and before the operators that project the entity away or choose which rows come back: `Select`, `Skip`, `Take`, `First`, `FirstOrDefault`, `Last`, `LastOrDefault`, `ElementAt`, `ElementAtOrDefault`, `ToList`, `ToArray` and `AsEnumerable`. A navigation bound into the result as it is, such as `new { _.Name, _.Employees }`, is ordered too. + +A collection whose order cannot reach the result is left alone: one that is only aggregated, with `Count`, `Any`, `Sum` and the like, or one composed with an operator EF Core drops ordering beneath, such as `Distinct`. Explicit ordering inside the projection is kept, and is checked by [redundant ordering detection](#detect-redundant-ordering) the same way as ordering inside an `Include`. + + ## Inheritance Support When using TPH (Table Per Hierarchy) inheritance, ordering configured on a base entity type is automatically inherited by derived types. This eliminates the need to duplicate `.OrderBy()` on every derived type. @@ -182,7 +211,7 @@ public class InheritanceDbContext : DbContext public DbSet DerivedEntitiesB => Set(); } ``` -snippet source | anchor +snippet source | anchor Behavior: @@ -205,7 +234,7 @@ builder.Entity() .ThenBy(_ => _.Name) .ThenByDescending(_ => _.Price); ``` -snippet source | anchor +snippet source | anchor @@ -548,7 +577,7 @@ public class AppDbContext : DbContext public DbSet Employees => Set(); } ``` -snippet source | anchor +snippet source | anchor diff --git a/src/Directory.Build.props b/src/Directory.Build.props index d76e846..7463b7d 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -5,7 +5,7 @@ enable latest true - 1.3.0 + 1.4.0 1.0.0 true true diff --git a/src/EfOrderBy/GlobalUsings.cs b/src/EfOrderBy/GlobalUsings.cs index e79f46f..6f403cc 100644 --- a/src/EfOrderBy/GlobalUsings.cs +++ b/src/EfOrderBy/GlobalUsings.cs @@ -1,4 +1,5 @@ global using System.Collections.Concurrent; +global using System.Diagnostics.CodeAnalysis; global using System.Linq.Expressions; global using EfOrderBy; global using Microsoft.EntityFrameworkCore; diff --git a/src/EfOrderBy/Interceptor.cs b/src/EfOrderBy/Interceptor.cs index fee34d9..fc73eda 100644 --- a/src/EfOrderBy/Interceptor.cs +++ b/src/EfOrderBy/Interceptor.cs @@ -24,18 +24,24 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven var detectRedundantOrdering = extension?.ThrowOnRedundantOrderBy ?? OrderBySettings.ThrowOnRedundantOrderBy; - // Analyze the query for ordering and includes in a single pass - var (hasOrdering, hasInclude) = QueryAnalyzer.Analyze(query); + // Analyze the query for ordering, includes and projections in a single pass + var (hasOrdering, hasInclude, hasSelect) = QueryAnalyzer.Analyze(query); var queryWithOrderedIncludes = query; // Ordering nested collections rewrites the whole tree, so only do it when there is an - // Include to rewrite. Neither flag above can change as a result, since the rewrite only - // touches the inside of Include lambdas. + // Include or a projection to rewrite. None of the flags above can change as a result, since + // the rewrites only touch the inside of Include and Select lambdas. if (hasInclude) { var visitor = new IncludeOrderingApplicator(model, detectRedundantOrdering); - queryWithOrderedIncludes = visitor.Visit(query); + queryWithOrderedIncludes = visitor.Visit(queryWithOrderedIncludes); + } + + if (hasSelect) + { + var visitor = new ProjectionOrderingApplicator(model, detectRedundantOrdering); + queryWithOrderedIncludes = visitor.Visit(queryWithOrderedIncludes); } if (hasOrdering) diff --git a/src/EfOrderBy/ProjectionOrderingApplicator.cs b/src/EfOrderBy/ProjectionOrderingApplicator.cs new file mode 100644 index 0000000..c4f8695 --- /dev/null +++ b/src/EfOrderBy/ProjectionOrderingApplicator.cs @@ -0,0 +1,353 @@ +/// +/// Visitor that applies default ordering to the collection navigations a Select projection loads. +/// +/// +/// A collection navigation read inside a projection, for example +/// Select(_ => new { Employees = _.Employees.Where(...).Select(...).ToList() }), is loaded by a +/// subquery of its own. Without ordering, EF Core returns that subquery's rows in key order, so +/// the default ordering configured for the collection's entity was ignored, the same as it was for +/// an Include before IncludeOrderingApplicator. +/// The ordering is placed after any Where filters and before the operators that project the entity +/// away or choose which rows come back, mirroring how the outer query is ordered. A collection +/// whose order cannot affect the result, such as one only counted or tested with Any, is left as +/// it is, as is one composed with an operator the ordering could not be placed beneath. +/// +sealed class ProjectionOrderingApplicator(IModel model, bool detectRedundantOrdering) : + ExpressionVisitor +{ + static readonly ConcurrentDictionary toListMethodCache = new(); + + static readonly MethodInfo toListMethod = typeof(Enumerable) + .GetMethod(nameof(Enumerable.ToList))!; + + // Lambdas of the Select projections currently being visited. Ordering is only applied inside one, + // since that is where a collection is loaded into the result. A collection read in a Where or + // OrderBy lambda of the outer query only filters or sorts it. + int projectionDepth; + + protected override Expression VisitMethodCall(MethodCallExpression node) + { + if (IsQueryableSelect(node)) + { + return VisitSelect(node); + } + + if (projectionDepth > 0 && + TryFindNavigationChain(node, out var navigation)) + { + return ProcessChain(node, navigation); + } + + return base.VisitMethodCall(node); + } + + static bool IsQueryableSelect(MethodCallExpression node) => + node.Method.DeclaringType == typeof(Queryable) && + node.Method.Name == nameof(Queryable.Select) && + node.Arguments.Count == 2 && + node.Arguments[1] is UnaryExpression { Operand: LambdaExpression }; + + Expression VisitSelect(MethodCallExpression node) + { + var source = Visit(node.Arguments[0]); + + var quote = (UnaryExpression) node.Arguments[1]; + projectionDepth++; + Expression selector; + try + { + selector = Visit(quote); + } + finally + { + projectionDepth--; + } + + return node.Update(node.Object, [source, selector]); + } + + // A navigation bound into the result as it is, for example new { _.Employees }, is loaded the + // same way as one read through a chain of operators, so it is ordered as well. It is replaced + // by the ordered navigation copied into a list, which only fits where a list can stand in for + // the navigation's declared type. + protected override MemberAssignment VisitMemberAssignment(MemberAssignment node) + { + var visited = base.VisitMemberAssignment(node); + if (projectionDepth == 0) + { + return visited; + } + + return visited.Update(OrderBoundNavigation(visited.Expression)); + } + + protected override Expression VisitNew(NewExpression node) + { + var visited = (NewExpression) base.VisitNew(node); + if (projectionDepth == 0) + { + return visited; + } + + Expression[]? arguments = null; + for (var index = 0; index < visited.Arguments.Count; index++) + { + var argument = visited.Arguments[index]; + var ordered = OrderBoundNavigation(argument); + if (ReferenceEquals(ordered, argument)) + { + continue; + } + + arguments ??= visited.Arguments.ToArray(); + arguments[index] = ordered; + } + + if (arguments == null) + { + return visited; + } + + return visited.Update(arguments); + } + + Expression OrderBoundNavigation(Expression expression) + { + if (expression is not MemberExpression member || + FindCollectionNavigation(member) is not { } navigation || + GetConfiguration(navigation) is not { } configuration) + { + return expression; + } + + var elementType = navigation.TargetEntityType.ClrType; + var listType = typeof(List<>).MakeGenericType(elementType); + if (!expression.Type.IsAssignableFrom(listType)) + { + return expression; + } + + var toList = toListMethodCache.GetOrAdd( + elementType, + static _ => toListMethod.MakeGenericMethod(_)); + return Expression.Call(toList, AppendOrdering(expression, configuration)); + } + + /// + /// Whether is the outermost operator of a chain over a collection + /// navigation, such as _.Employees.Where(...).Select(...).ToList(). + /// + bool TryFindNavigationChain(MethodCallExpression node, [NotNullWhen(true)] out INavigationBase? navigation) + { + navigation = null; + Expression current = node; + while (current is MethodCallExpression call) + { + if (!IsChainOperator(call)) + { + return false; + } + + current = call.Arguments[0]; + } + + if (current is not MemberExpression member) + { + return false; + } + + navigation = FindCollectionNavigation(member); + return navigation != null; + } + + static bool IsChainOperator(MethodCallExpression call) => + call.Method.IsStatic && + call.Method.DeclaringType == typeof(Enumerable) && + call.Arguments.Count > 0; + + INavigationBase? FindCollectionNavigation(MemberExpression member) + { + if (member.Expression == null) + { + return null; + } + + var entity = model.FindEntityType(member.Expression.Type); + if (entity == null) + { + return null; + } + + var name = member.Member.Name; + INavigationBase? navigation = entity.FindNavigation(name); + navigation ??= entity.FindSkipNavigation(name); + if (navigation is not { IsCollection: true }) + { + return null; + } + + return navigation; + } + + Expression ProcessChain(MethodCallExpression node, INavigationBase navigation) + { + // Lambdas passed along the chain can load collections of their own, for example + // _.Employees.Select(employee => new { Tasks = employee.Tasks.ToList() }) + var chain = VisitChainArguments(node); + + if (QueryAnalyzer.HasOrdering(chain)) + { + if (detectRedundantOrdering) + { + RedundantOrder.Validate(chain); + } + + return chain; + } + + if (GetConfiguration(navigation) is not { } configuration || + !CanOrder(chain)) + { + return chain; + } + + return ApplyOrdering(chain, configuration); + } + + // Visits every argument of each operator in the chain except the source it composes onto, which + // is the next operator down or the navigation itself. Visiting the navigation as a member would + // order it a second time as if it were bound into the result. + Expression VisitChainArguments(Expression expression) + { + if (expression is not MethodCallExpression call) + { + return expression; + } + + var arguments = new Expression[call.Arguments.Count]; + arguments[0] = VisitChainArguments(call.Arguments[0]); + for (var index = 1; index < arguments.Length; index++) + { + arguments[index] = VisitChainArgument(call.Arguments[index]); + } + + return call.Update(call.Object, arguments); + } + + // A lambda passed to the chain, such as a Select selector, can load collections into the result. + // Any other argument is not part of the result as it is: the second sequence of a Concat or a + // Union is one operand of a set operation, and ordering an operand orders nothing. + Expression VisitChainArgument(Expression argument) + { + if (argument is LambdaExpression) + { + return Visit(argument); + } + + var depth = projectionDepth; + projectionDepth = 0; + try + { + return Visit(argument); + } + finally + { + projectionDepth = depth; + } + } + + /// + /// Operators the default ordering has to be applied beneath rather than after. + /// + /// + /// Select projects the entity away, Skip, Take and the single result operators choose which + /// rows come back, and the rest copy the sequence as it is. + /// + static bool ShouldOrderBefore(MethodCallExpression call) => + call.Method.Name is + nameof(Enumerable.Select) or + nameof(Enumerable.Skip) or + nameof(Enumerable.Take) or + nameof(Enumerable.First) or + nameof(Enumerable.FirstOrDefault) or + nameof(Enumerable.Last) or + nameof(Enumerable.LastOrDefault) or + nameof(Enumerable.ElementAt) or + nameof(Enumerable.ElementAtOrDefault) or + nameof(Enumerable.ToList) or + nameof(Enumerable.ToArray) or + nameof(Enumerable.AsEnumerable); + + /// + /// Whether the ordering can go in beneath the operators that come after it, with only Where + /// filters between it and the navigation. Anything else, an aggregate such as Count or Any, or + /// an operator such as Distinct that EF Core drops ordering beneath, means the order of the + /// collection does not reach the result. + /// + static bool CanOrder(Expression chain) + { + var current = chain; + while (current is MethodCallExpression call && + ShouldOrderBefore(call)) + { + current = call.Arguments[0]; + } + + while (current is MethodCallExpression call) + { + if (call.Method.Name != nameof(Enumerable.Where)) + { + return false; + } + + current = call.Arguments[0]; + } + + return true; + } + + static Expression ApplyOrdering(Expression chain, Configuration configuration) + { + // Push past everything the ordering has to precede, then rebuild the chain around it + if (chain is MethodCallExpression call && + ShouldOrderBefore(call)) + { + var arguments = call.Arguments.ToArray(); + arguments[0] = ApplyOrdering(arguments[0], configuration); + return call.Update(call.Object, arguments); + } + + return AppendOrdering(chain, configuration); + } + + static Expression AppendOrdering(Expression source, Configuration configuration) + { + var result = source; + + foreach (var clause in configuration.Clauses) + { + result = clause.AppendEnumerableOrder(result); + } + + return result; + } + + static Configuration? GetConfiguration(INavigationBase navigation) + { + var target = navigation.TargetEntityType; + + // A JSON mapped collection is read out of its parent's JSON document rather than joined, + // so its document order is left as it is, the same as for an Include + if (target.IsMappedToJson()) + { + return null; + } + + if (Configuration.TryGet(target.ClrType) is not { Clauses.Count: > 0 } configuration) + { + return null; + } + + return configuration; + } +} diff --git a/src/EfOrderBy/QueryAnalyzer.cs b/src/EfOrderBy/QueryAnalyzer.cs index c1c61dc..9d4867d 100644 --- a/src/EfOrderBy/QueryAnalyzer.cs +++ b/src/EfOrderBy/QueryAnalyzer.cs @@ -1,5 +1,6 @@ /// -/// Scans the outermost chain of a query for existing ordering and for Include calls. +/// Scans the outermost chain of a query for existing ordering, for Include calls, and for Select +/// projections. /// /// /// Only the chain is walked, never the lambdas passed to it. Ordering nested inside a lambda, @@ -8,10 +9,11 @@ /// static class QueryAnalyzer { - public static (bool HasOrdering, bool HasInclude) Analyze(Expression expression) + public static (bool HasOrdering, bool HasInclude, bool HasSelect) Analyze(Expression expression) { var hasOrdering = false; var hasInclude = false; + var hasSelect = false; while (expression is MethodCallExpression call) { @@ -24,6 +26,11 @@ public static (bool HasOrdering, bool HasInclude) Analyze(Expression expression) { hasInclude = true; } + else if (declaringType == typeof(Queryable) && + name == "Select") + { + hasSelect = true; + } else if ((declaringType == typeof(Queryable) || declaringType == typeof(Enumerable)) && name is @@ -45,7 +52,7 @@ name is expression = call.Arguments[0]; } - return (hasOrdering, hasInclude); + return (hasOrdering, hasInclude, hasSelect); } public static bool HasOrdering(Expression expression) => diff --git a/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt b/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt index 0822389..c2f4324 100644 --- a/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt +++ b/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt @@ -1,4 +1,4 @@ SELECT [d].[Id], [d].[Name], [d].[DisplayOrder], [e].[Id], [e].[DepartmentId], [e].[HireDate], [e].[Name], [e].[Salary] FROM [Departments] AS [d] LEFT JOIN [Employees] AS [e] ON [d].[Id] = [e].[DepartmentId] -ORDER BY [d].[DisplayOrder], [d].[Id] \ No newline at end of file +ORDER BY [d].[DisplayOrder], [d].[Id], [e].[HireDate] DESC \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt b/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt new file mode 100644 index 0000000..177393b --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt @@ -0,0 +1,15 @@ +SELECT [d].[Name], ( + SELECT COUNT(*) + FROM [Employees] AS [e] + WHERE [d].[Id] = [e].[DepartmentId]) AS [Count], CASE + WHEN EXISTS ( + SELECT 1 + FROM [Employees] AS [e0] + WHERE [d].[Id] = [e0].[DepartmentId] AND [e0].[HireDate] > '2024-01-01T00:00:00.0000000') THEN CAST(1 AS bit) + ELSE CAST(0 AS bit) +END AS [AnyRecent], ( + SELECT COALESCE(SUM([e1].[Salary]), 0) + FROM [Employees] AS [e1] + WHERE [d].[Id] = [e1].[DepartmentId]) AS [Payroll] +FROM [Departments] AS [d] +ORDER BY [d].[DisplayOrder] \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..90898fc --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt @@ -0,0 +1,79 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + { + Id: 2, + DepartmentId: 1, + Name: Bob, + HireDate: 2024-03-20, + Salary: 85000 + }, + { + Id: 1, + DepartmentId: 1, + Name: Alice, + HireDate: 2024-01-15, + Salary: 90000 + }, + { + Id: 3, + DepartmentId: 1, + Name: Charlie, + HireDate: 2023-06-10, + Salary: 95000 + } + ] + }, + { + Name: Sales, + Employees: [ + { + Id: 4, + DepartmentId: 2, + Name: Diana, + HireDate: 2024-02-05, + Salary: 70000 + }, + { + Id: 5, + DepartmentId: 2, + Name: Eve, + HireDate: 2023-11-01, + Salary: 72000 + } + ] + }, + { + Name: HR, + Employees: [ + { + Id: 6, + DepartmentId: 3, + Name: Frank, + HireDate: 2024-04-10, + Salary: 65000 + } + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e.Id, + e.DepartmentId, + e.HireDate, + e.Name, + e.Salary +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt b/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt new file mode 100644 index 0000000..8af7cf8 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt @@ -0,0 +1,11 @@ +DECLARE @names1 nvarchar(4000) = N'Bob'; +DECLARE @names2 nvarchar(4000) = N'Alice'; + +SELECT [d].[Name], [d].[Id], [e0].[Name], [e0].[Id] +FROM [Departments] AS [d] +LEFT JOIN ( + SELECT [e].[Name], [e].[Id], [e].[HireDate], [e].[DepartmentId] + FROM [Employees] AS [e] + WHERE [e].[Name] IN (@names1, @names2) +) AS [e0] ON [d].[Id] = [e0].[DepartmentId] +ORDER BY [d].[DisplayOrder], [d].[Id], [e0].[HireDate] DESC \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt b/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt new file mode 100644 index 0000000..3bcd31a --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt @@ -0,0 +1,8 @@ +SELECT [d].[Name], [d].[Id], [e0].[Name] +FROM [Departments] AS [d] +OUTER APPLY ( + SELECT DISTINCT [e].[Name] + FROM [Employees] AS [e] + WHERE [d].[Id] = [e].[DepartmentId] +) AS [e0] +ORDER BY [d].[DisplayOrder], [d].[Id] \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt b/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt new file mode 100644 index 0000000..0ec4164 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt @@ -0,0 +1,40 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Alice, + Bob, + Charlie + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e.Name, + e.Id +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.Name, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt b/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt new file mode 100644 index 0000000..2c08815 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt @@ -0,0 +1,44 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Bob, + Charlie + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e0.Name, + e0.Id +from Departments as d + left outer join + (select e.Name, + e.Id, + e.HireDate, + e.DepartmentId + from Employees as e + where e.Name <> N'Alice') as e0 + on d.Id = e0.DepartmentId +order by d.DisplayOrder, + d.Id, + e0.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..25af99e --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt @@ -0,0 +1,27 @@ +{ + target: [ + { + Name: Engineering, + NewestHire: Bob + }, + { + Name: Sales, + NewestHire: Diana + }, + { + Name: HR, + NewestHire: Frank + } + ], + sql: { + Text: +select d.Name, + (select top (1) e.Name + from Employees as e + where d.Id = e.DepartmentId + order by e.HireDate desc) as NewestHire +from Departments as d +order by d.DisplayOrder, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt new file mode 100644 index 0000000..e7cf7e9 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt @@ -0,0 +1,75 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + { + Name: Bob, + Tasks: [ + Monitor, + Deploy + ] + }, + { + Name: Alice, + Tasks: [ + Code review, + Testing, + Design + ] + }, + { + Name: Charlie + } + ] + }, + { + Name: Sales, + Employees: [ + { + Name: Diana + }, + { + Name: Eve + } + ] + }, + { + Name: HR, + Employees: [ + { + Name: Frank + } + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + s.Name, + s.Id, + s.Title, + s.Id0 +from Departments as d + left outer join + (select e.Name, + e.Id, + e0.Title, + e0.Id as Id0, + e.HireDate, + e0.Priority, + e.DepartmentId + from Employees as e + left outer join + EmployeeTasks as e0 + on e.Id = e0.EmployeeId) as s + on d.Id = s.DepartmentId +order by d.DisplayOrder, + d.Id, + s.HireDate desc, + s.Id, + s.Priority, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..74f28b7 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt @@ -0,0 +1,68 @@ +{ + target: [ + { + Id: 1, + Name: Engineering, + Employees: [ + { + Id: 2, + Name: Bob, + HireDate: 2024-03-20 + }, + { + Id: 1, + Name: Alice, + HireDate: 2024-01-15 + }, + { + Id: 3, + Name: Charlie, + HireDate: 2023-06-10 + } + ] + }, + { + Id: 2, + Name: Sales, + Employees: [ + { + Id: 4, + Name: Diana, + HireDate: 2024-02-05 + }, + { + Id: 5, + Name: Eve, + HireDate: 2023-11-01 + } + ] + }, + { + Id: 3, + Name: HR, + Employees: [ + { + Id: 6, + Name: Frank, + HireDate: 2024-04-10 + } + ] + } + ], + sql: { + Text: +select d.Id, + d.Name, + e.Id, + e.Name, + e.HireDate +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt b/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt new file mode 100644 index 0000000..77d73d1 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt @@ -0,0 +1,50 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Bob, + Alice + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e1.Name, + e1.Id +from Departments as d + left outer join + (select e0.Name, + e0.Id, + e0.DepartmentId, + e0.HireDate + from (select e.Name, + e.Id, + e.DepartmentId, + ROW_NUMBER() over (partition by e.DepartmentId order by e.HireDate desc) as row, + e.HireDate + from Employees as e) as e0 + where e0.row <= 2) as e1 + on d.Id = e1.DepartmentId +order by d.DisplayOrder, + d.Id, + e1.DepartmentId, + e1.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.cs b/src/Tests/ProjectionOrderingTests.cs new file mode 100644 index 0000000..a388a59 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.cs @@ -0,0 +1,248 @@ +// A collection navigation read inside a Select projection is loaded by a subquery of its own, and +// gets the default ordering of its entity the same way a collection in an Include does. +// Department: DisplayOrder. Employee: HireDate descending. EmployeeTask: Priority. +public class ProjectionOrderingTests +{ + [Test] + public async Task ProjectedCollection_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + + // The shape GraphQL.EntityFramework builds for a navigation list field + var results = await context.Departments + .Select(_ => new Department + { + Id = _.Id, + Name = _.Name, + Employees = _.Employees + .Select(employee => new Employee + { + Id = employee.Id, + Name = employee.Name, + HireDate = employee.HireDate + }) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees.Select(_ => _.Name)).IsEquivalentTo([ + "Bob", // 2024-03-20 + "Alice", // 2024-01-15 + "Charlie" // 2023-06-10 + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task FilteredCollection_OrdersAfterFilter() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Name != "Alice") + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Bob", "Charlie"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task BoundNavigation_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + _.Employees + }) + .ToListAsync(); + + await Assert.That(results[0].Employees.Select(_ => _.Name)).IsEquivalentTo([ + "Bob", + "Alice", + "Charlie" + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task NestedCollections_EachGetTheirDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .AsSingleQuery() + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Select(employee => new + { + employee.Name, + Tasks = employee.Tasks + .Select(task => task.Title) + .ToList() + }) + .ToList() + }) + .ToListAsync(); + + var engineering = results[0].Employees; + await Assert.That(engineering.Select(_ => _.Name)).IsEquivalentTo(["Bob", "Alice", "Charlie"], CollectionOrdering.Matching); + var alice = engineering[1]; + await Assert.That(alice.Tasks).IsEquivalentTo([ + "Code review", // Priority 1 + "Testing", // Priority 2 + "Design" // Priority 3 + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task FirstOrDefaultInProjection_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + NewestHire = _.Employees + .Select(employee => employee.Name) + .FirstOrDefault() + }) + .ToListAsync(); + + await Assert.That(results.Select(_ => _.NewestHire!)).IsEquivalentTo(["Bob", "Diana", "Frank"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task TakeInProjection_OrdersBeforeTake() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Take(2) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Bob", "Alice"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task ExplicitOrderingInProjection_IsKept() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .OrderBy(employee => employee.Name) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Alice", "Bob", "Charlie"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task AggregateInProjection_IsNotOrdered() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + var sql = context.Departments + .Select(_ => new + { + _.Name, + Count = _.Employees.Count(), + AnyRecent = _.Employees.Any(employee => employee.HireDate > new DateTime(2024, 1, 1)), + Payroll = _.Employees.Select(employee => employee.Salary).Sum() + }) + .ToQueryString(); + + await Assert.That(sql).DoesNotContain("ORDER BY [e"); + await Verify(sql); + } + + [Test] + public async Task DistinctInProjection_IsNotOrdered() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + // EF Core drops ordering beneath a Distinct, so there is nothing to place it beneath + var sql = context.Departments + .Select(_ => new + { + _.Name, + Names = _.Employees + .Select(employee => employee.Name) + .Distinct() + .ToList() + }) + .ToQueryString(); + + await Assert.That(sql).DoesNotContain("HireDate"); + await Verify(sql); + } + + [Test] + public async Task CollectionThatIsNotANavigation_IsLeftAlone() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + // Only the navigation is ordered. The list of names is a parameter, not a navigation. + var names = new List { "Bob", "Alice" }; + var sql = context.Departments + .Select(_ => new + { + _.Name, + Matching = _.Employees + .Where(employee => names.Contains(employee.Name)) + .Select(employee => employee.Name) + .ToList() + }) + .ToQueryString(); + + await Assert.That(sql).Contains("[HireDate] DESC"); + await Verify(sql); + } +} diff --git a/src/Tests/RedundantOrderByTests.cs b/src/Tests/RedundantOrderByTests.cs index 37fdad7..13bd95a 100644 --- a/src/Tests/RedundantOrderByTests.cs +++ b/src/Tests/RedundantOrderByTests.cs @@ -206,6 +206,90 @@ await Assert.That( .ToQueryString()).ThrowsNothing(); } + [Test] + public async Task Projection_ExactMatch_Throws() + { + using var context = new RedundantEnabledContext(enabled); + + var exception = Assert.ThrowsExactly( + () => context.Entities + .Select(_ => new + { + _.Name, + Children = _.Children + .OrderBy(child => child.SortOrder) + .Select(child => child.Title) + .ToList() + }) + .ToQueryString()); + + await Assert.That(exception!.Message).Contains("RedundantChild"); + await Assert.That(exception.Message).Contains("OrderBy(SortOrder)"); + } + + [Test] + public void Projection_ExactMatchWithFilter_Throws() + { + using var context = new RedundantEnabledContext(enabled); + + Assert.ThrowsExactly( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .Where(child => child.SortOrder > 0) + .OrderBy(child => child.SortOrder) + .ToList() + }) + .ToQueryString()); + } + + [Test] + public async Task Projection_DifferentOrdering_DoesNotThrow() + { + using var context = new RedundantEnabledContext(enabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .OrderBy(child => child.Title) + .ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + + [Test] + public async Task Projection_WithoutOrdering_DoesNotThrow() + { + using var context = new RedundantEnabledContext(enabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children.ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + + [Test] + public async Task Projection_ExactMatch_DetectionDisabled_DoesNotThrow() + { + using var context = new RedundantDisabledContext(disabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .OrderBy(child => child.SortOrder) + .ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + [Test] public async Task ExactMatchInsideConcat_DoesNotThrow() { diff --git a/src/Tests/Snippets.cs b/src/Tests/Snippets.cs index 18aee2d..6f60d42 100644 --- a/src/Tests/Snippets.cs +++ b/src/Tests/Snippets.cs @@ -148,6 +148,28 @@ static async Task IncludeSupport() #endregion } + static async Task ProjectionSupport() + { + AppDbContext context = null!; + + #region ProjectionSupport + + // Departments ordered by DisplayOrder + // Employees ordered by HireDate, then Salary descending + var departments = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Salary > 0) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + #endregion + } + static void MultiColumnOrdering(ModelBuilder builder) { #region MultiColumnOrdering