Fixed issues with ToPageAsync() and relative cursors. (#8110)

Author: michaelstaib

src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs

@@ -40,7 +40,7 @@ internal static class ExpressionHelpers
     /// <exception cref="ArgumentException">
     /// If the number of keys does not match the number of values.
     /// </exception>
-    public static (Expression<Func<T, bool>> WhereExpression, int Offset, bool ReverseOrder) BuildWhereExpression<T>(
+    public static (Expression<Func<T, bool>> WhereExpression, int Offset) BuildWhereExpression<T>(
         ReadOnlySpan<CursorKey> keys,
         Cursor cursor,
         bool forward)
@@ -77,53 +77,34 @@ public static (Expression<Func<T, bool>> WhereExpression, int Offset, bool Rever
             {
                 var handledKey = handled[j];
 
-                keyExpr =
-                    Expression.Equal(
-                        Expression.Call(
-                            ReplaceParameter(handledKey.Expression, parameter),
-                            handledKey.CompareMethod,
-                            cursorExpr[j]),
-                        zero);
-
-                current = current is null
-                    ? keyExpr
-                    : Expression.AndAlso(current, keyExpr);
+                keyExpr = Expression.Equal(
+                    Expression.Call(ReplaceParameter(handledKey.Expression, parameter), handledKey.CompareMethod,
+                        cursorExpr[j]), zero);
+
+                current = current is null ? keyExpr : Expression.AndAlso(current, keyExpr);
             }
 
             var greaterThan = forward
-                ? key.Direction is CursorKeyDirection.Ascending
-                : key.Direction is CursorKeyDirection.Descending;
-
-            keyExpr =
-                greaterThan
-                    ? Expression.GreaterThan(
-                        Expression.Call(
-                            ReplaceParameter(key.Expression, parameter),
-                            key.CompareMethod,
-                            cursorExpr[i]),
-                        zero)
-                    : Expression.LessThan(
-                        Expression.Call(
-                            ReplaceParameter(key.Expression, parameter),
-                            key.CompareMethod,
-                            cursorExpr[i]),
-                        zero);
-
-            current = current is null
-                ? keyExpr
-                : Expression.AndAlso(current, keyExpr);
-            expression = expression is null
-                ? current
-                : Expression.OrElse(expression, current);
+                ? key.Direction == CursorKeyDirection.Ascending
+                : key.Direction == CursorKeyDirection.Descending;
+
+            keyExpr = greaterThan
+                ? Expression.GreaterThan(
+                    Expression.Call(ReplaceParameter(key.Expression, parameter), key.CompareMethod, cursorExpr[i]),
+                    zero)
+                : Expression.LessThan(
+                    Expression.Call(ReplaceParameter(key.Expression, parameter), key.CompareMethod, cursorExpr[i]),
+                    zero);
+
+            current = current is null ? keyExpr : Expression.AndAlso(current, keyExpr);
+            expression = expression is null ? current : Expression.OrElse(expression, current);
             handled.Add(key);
         }
 
-        var offset = cursor.Offset ?? 0;
-        var reverseOrder = offset < 0; // Reverse order if offset is negative
-
-        return (Expression.Lambda<Func<T, bool>>(expression!, parameter), Math.Abs(offset), reverseOrder);
+        return (Expression.Lambda<Func<T, bool>>(expression!, parameter), cursor.Offset ?? 0);
     }
 
+
     /// <summary>
     /// Build the select expression for a batch paging expression that uses grouping.
     /// </summary>
@@ -151,12 +132,11 @@ public static (Expression<Func<T, bool>> WhereExpression, int Offset, bool Rever
     /// <typeparam name="TV">
     /// The value type.
     /// </typeparam>
-    /// <returns></returns>
     /// <exception cref="ArgumentException">
     /// If the number of keys is less than one or
     /// the number of order expressions does not match the number of order methods.
     /// </exception>
-    public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpression, bool ReverseOrder) BuildBatchSelectExpression<TK, TV>(
+    public static BatchExpression<TK, TV> BuildBatchExpression<TK, TV>(
         PagingArguments arguments,
         ReadOnlySpan<CursorKey> keys,
         ReadOnlySpan<LambdaExpression> orderExpressions,
@@ -187,7 +167,8 @@ public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpressi
             var methodName = forward ? orderMethods[i] : ReverseOrder(orderMethods[i]);
             var orderExpression = orderExpressions[i];
             var delegateType = typeof(Func<,>).MakeGenericType(typeof(TV), orderExpression.Body.Type);
-            var typedOrderExpression = Expression.Lambda(delegateType, orderExpression.Body, orderExpression.Parameters);
+            var typedOrderExpression =
+                Expression.Lambda(delegateType, orderExpression.Body, orderExpression.Parameters);
 
             var method = GetEnumerableMethod(methodName, typeof(TV), typedOrderExpression);
 
@@ -197,44 +178,68 @@ public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpressi
                 typedOrderExpression);
         }
 
-        var reverseOrder = false;
         var offset = 0;
+        var usesRelativeCursors = false;
+        Cursor? cursor = null;
 
         if (arguments.After is not null)
         {
-            var cursor = CursorParser.Parse(arguments.After, keys);
-            var (whereExpr, cursorOffset, reverse) = BuildWhereExpression<TV>(keys, cursor, forward: true);
+            cursor = CursorParser.Parse(arguments.After, keys);
+            var (whereExpr, cursorOffset) = BuildWhereExpression<TV>(keys, cursor, forward: true);
             source = Expression.Call(typeof(Enumerable), "Where", [typeof(TV)], source, whereExpr);
             offset = cursorOffset;
-            reverseOrder = reverse;
+
+            if (cursor.IsRelative)
+            {
+                usesRelativeCursors = true;
+            }
         }
 
         if (arguments.Before is not null)
         {
-            var cursor = CursorParser.Parse(arguments.Before, keys);
-            var (whereExpr, cursorOffset, reverse) = BuildWhereExpression<TV>(keys, cursor, forward: false);
+            if (usesRelativeCursors)
+            {
+                throw new ArgumentException(
+                    "You cannot use `before` and `after` with relative cursors at the same time.",
+                    nameof(arguments));
+            }
+
+            cursor = CursorParser.Parse(arguments.Before, keys);
+            var (whereExpr, cursorOffset) = BuildWhereExpression<TV>(keys, cursor, forward: false);
             source = Expression.Call(typeof(Enumerable), "Where", [typeof(TV)], source, whereExpr);
             offset = cursorOffset;
-            reverseOrder = reverse;
         }
 
-        if (reverseOrder)
+        if (arguments.First is not null)
         {
-            source = Expression.Call(
-                typeof(Enumerable),
-                "Reverse",
-                [typeof(TV)],
-                source);
+            requestedCount = arguments.First.Value;
+        }
+
+        if (arguments.Last is not null)
+        {
+            requestedCount = arguments.Last.Value;
+        }
+
+        if (arguments.EnableRelativeCursors && cursor?.IsRelative == true)
+        {
+            if ((arguments.Last is not null && cursor.Offset > 0) || (arguments.First is not null && cursor.Offset < 0))
+            {
+                throw new ArgumentException(
+                    "Positive offsets are not allowed with `last`, and negative offsets are not allowed with `first`.",
+                    nameof(arguments));
+            }
         }
 
+        offset = Math.Abs(offset);
+
         if (offset > 0)
         {
             source = Expression.Call(
                 typeof(Enumerable),
                 "Skip",
                 [typeof(TV)],
                 source,
-                Expression.Constant(offset));
+                Expression.Constant(offset * requestedCount));
         }
 
         if (arguments.First is not null)
@@ -245,7 +250,6 @@ public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpressi
                 [typeof(TV)],
                 source,
                 Expression.Constant(arguments.First.Value + 1));
-            requestedCount = arguments.First.Value;
         }
 
         if (arguments.Last is not null)
@@ -256,7 +260,6 @@ public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpressi
                 [typeof(TV)],
                 source,
                 Expression.Constant(arguments.Last.Value + 1));
-            requestedCount = arguments.Last.Value;
         }
 
         source = Expression.Call(
@@ -273,7 +276,10 @@ public static (Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpressi
         };
 
         var createGroup = Expression.MemberInit(Expression.New(groupType), bindings);
-        return (Expression.Lambda<Func<IGrouping<TK, TV>, Group<TK, TV>>>(createGroup, group), reverseOrder);
+        return new BatchExpression<TK, TV>(
+            Expression.Lambda<Func<IGrouping<TK, TV>, Group<TK, TV>>>(createGroup, group),
+            arguments.Last is not null,
+            cursor);
 
         static string ReverseOrder(string method)
             => method switch
@@ -292,84 +298,6 @@ static MethodInfo GetEnumerableMethod(string methodName, Type elementType, Lambd
                 .MakeGenericMethod(elementType, keySelector.Body.Type);
     }
 
-    private static MethodCallExpression BuildBatchWhereExpression<T>(
-        Expression enumerable,
-        ReadOnlySpan<CursorKey> keys,
-        object?[] cursor,
-        bool forward)
-    {
-        var cursorExpr = new Expression[cursor.Length];
-
-        for (var i = 0; i < cursor.Length; i++)
-        {
-            cursorExpr[i] = CreateParameter(cursor[i], keys[i].Expression.ReturnType);
-        }
-
-        var handled = new List<CursorKey>();
-        Expression? expression = null;
-
-        var parameter = Expression.Parameter(typeof(T), "t");
-        var zero = Expression.Constant(0);
-
-        for (var i = 0; i < keys.Length; i++)
-        {
-            var key = keys[i];
-            Expression? current = null;
-            Expression keyExpr;
-
-            for (var j = 0; j < handled.Count; j++)
-            {
-                var handledKey = handled[j];
-
-                keyExpr =
-                    Expression.Equal(
-                        Expression.Call(
-                            ReplaceParameter(handledKey.Expression, parameter),
-                            handledKey.CompareMethod,
-                            cursorExpr[j]),
-                        zero);
-
-                current = current is null
-                    ? keyExpr
-                    : Expression.AndAlso(current, keyExpr);
-            }
-
-            var greaterThan = forward
-                ? key.Direction is CursorKeyDirection.Ascending
-                : key.Direction is CursorKeyDirection.Descending;
-
-            keyExpr =
-                greaterThan
-                    ? Expression.GreaterThan(
-                        Expression.Call(
-                            ReplaceParameter(key.Expression, parameter),
-                            key.CompareMethod,
-                            cursorExpr[i]),
-                        zero)
-                    : Expression.LessThan(
-                        Expression.Call(
-                            ReplaceParameter(key.Expression, parameter),
-                            key.CompareMethod,
-                            cursorExpr[i]),
-                        zero);
-
-            current = current is null
-                ? keyExpr
-                : Expression.AndAlso(current, keyExpr);
-            expression = expression is null
-                ? current
-                : Expression.OrElse(expression, current);
-            handled.Add(key);
-        }
-
-        return Expression.Call(
-            typeof(Enumerable),
-            "Where",
-            [typeof(T)],
-            enumerable,
-            Expression.Lambda<Func<T, bool>>(expression!, parameter));
-    }
-
     /// <summary>
     /// Extracts and removes the orderBy and thenBy expressions from the given expression tree.
     /// </summary>
@@ -504,4 +432,14 @@ private static Expression StripQuotes(Expression e)
             return e;
         }
     }
+
+    internal readonly struct BatchExpression<TK, TV>(
+        Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> selectExpression,
+        bool isBackward,
+        Cursor? cursor)
+    {
+        public Expression<Func<IGrouping<TK, TV>, Group<TK, TV>>> SelectExpression { get; } = selectExpression;
+        public bool IsBackward { get; } = isBackward;
+        public Cursor? Cursor { get; } = cursor;
+    }
 }

src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ReverseOrderExpressionRewriter.cs

@@ -0,0 +1,60 @@
+using System.Linq.Expressions;
+using System.Reflection;
+
+namespace GreenDonut.Data.Expressions;
+
+public class ReverseOrderExpressionRewriter : ExpressionVisitor
+{
+    private static readonly MethodInfo _orderByMethod = typeof(Queryable).GetMethods()
+        .First(m => m.Name == nameof(Queryable.OrderBy) && m.GetParameters().Length == 2);
+
+    private static readonly MethodInfo _orderByDescendingMethod = typeof(Queryable).GetMethods()
+        .First(m => m.Name == nameof(Queryable.OrderByDescending) && m.GetParameters().Length == 2);
+
+    private static readonly MethodInfo _thenByMethod = typeof(Queryable).GetMethods()
+        .First(m => m.Name == nameof(Queryable.ThenBy) && m.GetParameters().Length == 2);
+
+    private static readonly MethodInfo _thenByDescendingMethod = typeof(Queryable).GetMethods()
+        .First(m => m.Name == nameof(Queryable.ThenByDescending) && m.GetParameters().Length == 2);
+
+    protected override Expression VisitMethodCall(MethodCallExpression node)
+    {
+        var visitedArguments = node.Arguments.Select(Visit).Cast<Expression>().ToArray();
+
+        if (node.Method.Name == nameof(Queryable.OrderBy))
+        {
+            return Expression.Call(
+                _orderByDescendingMethod.MakeGenericMethod(node.Method.GetGenericArguments()),
+                visitedArguments);
+        }
+
+        if (node.Method.Name == nameof(Queryable.OrderByDescending))
+        {
+            return Expression.Call(
+                _orderByMethod.MakeGenericMethod(node.Method.GetGenericArguments()),
+                visitedArguments);
+        }
+
+        if (node.Method.Name == nameof(Queryable.ThenBy))
+        {
+            return Expression.Call(
+                _thenByDescendingMethod.MakeGenericMethod(node.Method.GetGenericArguments()),
+                visitedArguments);
+        }
+
+        if (node.Method.Name == nameof(Queryable.ThenByDescending))
+        {
+            return Expression.Call(
+                _thenByMethod.MakeGenericMethod(node.Method.GetGenericArguments()),
+                visitedArguments);
+        }
+
+        return base.VisitMethodCall(node);
+    }
+
+    public static IQueryable<T> Rewrite<T>(IQueryable<T> query)
+    {
+        var reversedExpression = new ReverseOrderExpressionRewriter().Visit(query.Expression);
+        return query.Provider.CreateQuery<T>(reversedExpression);
+    }
+}