[8.x] Mixed orders in cursor paginate (#37762)

* Mixed orders in cursor paginate * Test cursor paginate with mixed orders * formatting Co-authored-by: Taylor Otwell <[email protected]>
laravel · Jul 8, 2021 · 80f0112 · 80f0112
1 parent c35e8b1
commit 80f0112
Show file tree

Hide file tree

Showing 5 changed files with 157 additions and 89 deletions.
diff --git a/src/Illuminate/Database/Concerns/BuildsQueries.php b/src/Illuminate/Database/Concerns/BuildsQueries.php
@@ -282,14 +282,49 @@ public function sole($columns = ['*'])
     }
 
     /**
-     * Pass the query to a given callback.
+     * Paginate the given query using a cursor paginator.
      *
-     * @param  callable  $callback
-     * @return $this
+     * @param  int  $perPage
+     * @param  array  $columns
+     * @param  string  $cursorName
+     * @param  string|null  $cursor
+     * @return \Illuminate\Contracts\Pagination\CursorPaginator
      */
-    public function tap($callback)
+    protected function paginateUsingCursor($perPage, $columns = ['*'], $cursorName = 'cursor', $cursor = null)
     {
-        return $this->when(true, $callback);
+        $cursor = $cursor ?: CursorPaginator::resolveCurrentCursor($cursorName);
+
+        $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems());
+
+        if (! is_null($cursor)) {
+            $addCursorConditions = function (self $builder, $previousColumn, $i) use (&$addCursorConditions, $cursor, $orders) {
+                if (! is_null($previousColumn)) {
+                    $builder->where($previousColumn, '=', $cursor->parameter($previousColumn));
+                }
+
+                $builder->where(function (self $builder) use ($addCursorConditions, $cursor, $orders, $i) {
+                    ['column' => $column, 'direction' => $direction] = $orders[$i];
+
+                    $builder->where($column, $direction === 'asc' ? '>' : '<', $cursor->parameter($column));
+
+                    if ($i < $orders->count() - 1) {
+                        $builder->orWhere(function (self $builder) use ($addCursorConditions, $column, $i) {
+                            $addCursorConditions($builder, $column, $i + 1);
+                        });
+                    }
+                });
+            };
+
+            $addCursorConditions($this, null, 0);
+        }
+
+        $this->limit($perPage + 1);
+
+        return $this->cursorPaginator($this->get($columns), $perPage, $cursor, [
+            'path' => Paginator::resolveCurrentPath(),
+            'cursorName' => $cursorName,
+            'parameters' => $orders->pluck('column')->toArray(),
+        ]);
     }
 
     /**
@@ -340,4 +375,15 @@ protected function cursorPaginator($items, $perPage, $cursor, $options)
             'items', 'perPage', 'cursor', 'options'
         ));
     }
+
+    /**
+     * Pass the query to a given callback.
+     *
+     * @param  callable  $callback
+     * @return $this
+     */
+    public function tap($callback)
+    {
+        return $this->when(true, $callback);
+    }
 }
diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php
@@ -12,8 +12,6 @@
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Database\Query\Builder as QueryBuilder;
 use Illuminate\Database\RecordsNotFoundException;
-use Illuminate\Pagination\CursorPaginationException;
-use Illuminate\Pagination\CursorPaginator;
 use Illuminate\Pagination\Paginator;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
@@ -828,56 +826,25 @@ public function simplePaginate($perPage = null, $columns = ['*'], $pageName = 'p
      * @param  string  $cursorName
      * @param  string|null  $cursor
      * @return \Illuminate\Contracts\Pagination\CursorPaginator
-     * @throws \Illuminate\Pagination\CursorPaginationException
      */
     public function cursorPaginate($perPage = null, $columns = ['*'], $cursorName = 'cursor', $cursor = null)
     {
-        $cursor = $cursor ?: CursorPaginator::resolveCurrentCursor($cursorName);
-
         $perPage = $perPage ?: $this->model->getPerPage();
 
-        $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems());
-
-        $orderDirection = $orders->first()['direction'] ?? 'asc';
-
-        $comparisonOperator = $orderDirection === 'asc' ? '>' : '<';
-
-        $parameters = $orders->pluck('column')->toArray();
-
-        if (! is_null($cursor)) {
-            if (count($parameters) === 1) {
-                $this->where($column = $parameters[0], $comparisonOperator, $cursor->parameter($column));
-            } elseif (count($parameters) > 1) {
-                $this->whereRowValues($parameters, $comparisonOperator, $cursor->parameters($parameters));
-            }
-        }
-
-        $this->take($perPage + 1);
-
-        return $this->cursorPaginator($this->get($columns), $perPage, $cursor, [
-            'path' => Paginator::resolveCurrentPath(),
-            'cursorName' => $cursorName,
-            'parameters' => $parameters,
-        ]);
+        return $this->paginateUsingCursor($perPage, $columns, $cursorName, $cursor);
     }
 
     /**
      * Ensure the proper order by required for cursor pagination.
      *
      * @param  bool  $shouldReverse
      * @return \Illuminate\Support\Collection
-     *
-     * @throws \Illuminate\Pagination\CursorPaginationException
      */
     protected function ensureOrderForCursorPagination($shouldReverse = false)
     {
-        $orderDirections = collect($this->query->orders)->pluck('direction')->unique();
-
-        if ($orderDirections->count() > 1) {
-            throw new CursorPaginationException('Only a single order by direction is supported when using cursor pagination.');
-        }
+        $orders = collect($this->query->orders);
 
-        if ($orderDirections->count() === 0) {
+        if ($orders->count() === 0) {
             $this->enforceOrderBy();
         }
 

diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php
@@ -12,8 +12,6 @@
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Database\Query\Grammars\Grammar;
 use Illuminate\Database\Query\Processors\Processor;
-use Illuminate\Pagination\CursorPaginationException;
-use Illuminate\Pagination\CursorPaginator;
 use Illuminate\Pagination\Paginator;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
@@ -2408,54 +2406,22 @@ public function simplePaginate($perPage = 15, $columns = ['*'], $pageName = 'pag
      * @param  string  $cursorName
      * @param  string|null  $cursor
      * @return \Illuminate\Contracts\Pagination\CursorPaginator
-     * @throws \Illuminate\Pagination\CursorPaginationException
      */
     public function cursorPaginate($perPage = 15, $columns = ['*'], $cursorName = 'cursor', $cursor = null)
     {
-        $cursor = $cursor ?: CursorPaginator::resolveCurrentCursor($cursorName);
-
-        $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems());
-
-        $orderDirection = $orders->first()['direction'] ?? 'asc';
-
-        $comparisonOperator = $orderDirection === 'asc' ? '>' : '<';
-
-        $parameters = $orders->pluck('column')->toArray();
-
-        if (! is_null($cursor)) {
-            if (count($parameters) === 1) {
-                $this->where($column = $parameters[0], $comparisonOperator, $cursor->parameter($column));
-            } elseif (count($parameters) > 1) {
-                $this->whereRowValues($parameters, $comparisonOperator, $cursor->parameters($parameters));
-            }
-        }
-
-        $this->limit($perPage + 1);
-
-        return $this->cursorPaginator($this->get($columns), $perPage, $cursor, [
-            'path' => Paginator::resolveCurrentPath(),
-            'cursorName' => $cursorName,
-            'parameters' => $parameters,
-        ]);
+        return $this->paginateUsingCursor($perPage, $columns, $cursorName, $cursor);
     }
 
     /**
      * Ensure the proper order by required for cursor pagination.
      *
      * @param  bool  $shouldReverse
      * @return \Illuminate\Support\Collection
-     * @throws \Illuminate\Pagination\CursorPaginationException
      */
     protected function ensureOrderForCursorPagination($shouldReverse = false)
     {
         $this->enforceOrderBy();
 
-        $orderDirections = collect($this->orders)->pluck('direction')->unique();
-
-        if ($orderDirections->count() > 1) {
-            throw new CursorPaginationException('Only a single order by direction is supported when using cursor pagination.');
-        }
-
         if ($shouldReverse) {
             $this->orders = collect($this->orders)->map(function ($order) {
                 $order['direction'] = $order['direction'] === 'asc' ? 'desc' : 'asc';

diff --git a/src/Illuminate/Pagination/CursorPaginationException.php b/src/Illuminate/Pagination/CursorPaginationException.php
@@ -4,6 +4,9 @@
 
 use RuntimeException;
 
+/**
+ * @deprecated Will be removed in a future Laravel version.
+ */
 class CursorPaginationException extends RuntimeException
 {
     //

diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php
@@ -3662,13 +3662,24 @@ public function testCursorPaginate()
         $columns = ['test'];
         $cursorName = 'cursor-name';
         $cursor = new Cursor(['test' => 'bar']);
-        $builder = $this->getMockQueryBuilder()->orderBy('test');
+        $builder = $this->getMockQueryBuilder();
+        $builder->from('foobar')->orderBy('test');
+        $builder->shouldReceive('newQuery')->andReturnUsing(function () use ($builder) {
+            return new Builder($builder->connection, $builder->grammar, $builder->processor);
+        });
+
         $path = 'http://foo.bar?cursor='.$cursor->encode();
 
         $results = collect([['test' => 'foo'], ['test' => 'bar']]);
 
-        $builder->shouldReceive('where')->with('test', '>', 'bar')->once()->andReturnSelf();
-        $builder->shouldReceive('get')->once()->andReturn($results);
+        $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results) {
+            $this->assertEquals(
+                'select * from "foobar" where ("test" > ?) order by "test" asc limit 17',
+                $builder->toSql());
+            $this->assertEquals(['bar'], $builder->bindings['where']);
+
+            return $results;
+        });
 
         Paginator::currentPathResolver(function () use ($path) {
             return $path;
@@ -3686,16 +3697,28 @@ public function testCursorPaginate()
     public function testCursorPaginateMultipleOrderColumns()
     {
         $perPage = 16;
-        $columns = ['test'];
+        $columns = ['test', 'another'];
         $cursorName = 'cursor-name';
         $cursor = new Cursor(['test' => 'bar', 'another' => 'foo']);
-        $builder = $this->getMockQueryBuilder()->orderBy('test')->orderBy('another');
+        $builder = $this->getMockQueryBuilder();
+        $builder->from('foobar')->orderBy('test')->orderBy('another');
+        $builder->shouldReceive('newQuery')->andReturnUsing(function () use ($builder) {
+            return new Builder($builder->connection, $builder->grammar, $builder->processor);
+        });
+
         $path = 'http://foo.bar?cursor='.$cursor->encode();
 
-        $results = collect([['test' => 'foo'], ['test' => 'bar']]);
+        $results = collect([['test' => 'foo', 'another' => 1], ['test' => 'bar', 'another' => 2]]);
 
-        $builder->shouldReceive('whereRowValues')->with(['test', 'another'], '>', ['bar', 'foo'])->once()->andReturnSelf();
-        $builder->shouldReceive('get')->once()->andReturn($results);
+        $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results) {
+            $this->assertEquals(
+                'select * from "foobar" where ("test" > ? or ("test" = ? and ("another" > ?))) order by "test" asc, "another" asc limit 17',
+                $builder->toSql()
+            );
+            $this->assertEquals(['bar', 'bar', 'foo'], $builder->bindings['where']);
+
+            return $results;
+        });
 
         Paginator::currentPathResolver(function () use ($path) {
             return $path;
@@ -3715,12 +3738,24 @@ public function testCursorPaginateWithDefaultArguments()
         $perPage = 15;
         $cursorName = 'cursor';
         $cursor = new Cursor(['test' => 'bar']);
-        $builder = $this->getMockQueryBuilder()->orderBy('test');
+        $builder = $this->getMockQueryBuilder();
+        $builder->from('foobar')->orderBy('test');
+        $builder->shouldReceive('newQuery')->andReturnUsing(function () use ($builder) {
+            return new Builder($builder->connection, $builder->grammar, $builder->processor);
+        });
+
         $path = 'http://foo.bar?cursor='.$cursor->encode();
 
         $results = collect([['test' => 'foo'], ['test' => 'bar']]);
 
-        $builder->shouldReceive('get')->once()->andReturn($results);
+        $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results) {
+            $this->assertEquals(
+                'select * from "foobar" where ("test" > ?) order by "test" asc limit 16',
+                $builder->toSql());
+            $this->assertEquals(['bar'], $builder->bindings['where']);
+
+            return $results;
+        });
 
         CursorPaginator::currentCursorResolver(function () use ($cursor) {
             return $cursor;
@@ -3773,12 +3808,24 @@ public function testCursorPaginateWithSpecificColumns()
         $columns = ['id', 'name'];
         $cursorName = 'cursor-name';
         $cursor = new Cursor(['id' => 2]);
-        $builder = $this->getMockQueryBuilder()->orderBy('id');
+        $builder = $this->getMockQueryBuilder();
+        $builder->from('foobar')->orderBy('id');
+        $builder->shouldReceive('newQuery')->andReturnUsing(function () use ($builder) {
+            return new Builder($builder->connection, $builder->grammar, $builder->processor);
+        });
+
         $path = 'http://foo.bar?cursor=3';
 
         $results = collect([['id' => 3, 'name' => 'Taylor'], ['id' => 5, 'name' => 'Mohamed']]);
 
-        $builder->shouldReceive('get')->once()->andReturn($results);
+        $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results) {
+            $this->assertEquals(
+                'select * from "foobar" where ("id" > ?) order by "id" asc limit 17',
+                $builder->toSql());
+            $this->assertEquals([2], $builder->bindings['where']);
+
+            return $results;
+        });
 
         Paginator::currentPathResolver(function () use ($path) {
             return $path;
@@ -3793,6 +3840,45 @@ public function testCursorPaginateWithSpecificColumns()
         ]), $result);
     }
 
+    public function testCursorPaginateWithMixedOrders()
+    {
+        $perPage = 16;
+        $columns = ['foo', 'bar', 'baz'];
+        $cursorName = 'cursor-name';
+        $cursor = new Cursor(['foo' => 1, 'bar' => 2, 'baz' => 3]);
+        $builder = $this->getMockQueryBuilder();
+        $builder->from('foobar')->orderBy('foo')->orderByDesc('bar')->orderBy('baz');
+        $builder->shouldReceive('newQuery')->andReturnUsing(function () use ($builder) {
+            return new Builder($builder->connection, $builder->grammar, $builder->processor);
+        });
+
+        $path = 'http://foo.bar?cursor='.$cursor->encode();
+
+        $results = collect([['foo' => 1, 'bar' => 2, 'baz' => 4], ['foo' => 1, 'bar' => 1, 'baz' => 1]]);
+
+        $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results) {
+            $this->assertEquals(
+                'select * from "foobar" where ("foo" > ? or ("foo" = ? and ("bar" < ? or ("bar" = ? and ("baz" > ?))))) order by "foo" asc, "bar" desc, "baz" asc limit 17',
+                $builder->toSql()
+            );
+            $this->assertEquals([1, 1, 2, 2, 3], $builder->bindings['where']);
+
+            return $results;
+        });
+
+        Paginator::currentPathResolver(function () use ($path) {
+            return $path;
+        });
+
+        $result = $builder->cursorPaginate($perPage, $columns, $cursorName, $cursor);
+
+        $this->assertEquals(new CursorPaginator($results, $perPage, $cursor, [
+            'path' => $path,
+            'cursorName' => $cursorName,
+            'parameters' => ['foo', 'bar', 'baz'],
+        ]), $result);
+    }
+
     public function testWhereRowValues()
     {
         $builder = $this->getBuilder();
@@ -4185,7 +4271,7 @@ protected function getMySqlBuilderWithProcessor()
     }
 
     /**
-     * @return m\MockInterface
+     * @return m\MockInterface|Builder
      */
     protected function getMockQueryBuilder()
     {
-Original file line number
+Diff line change
@@ Expand Up / @@ -4,6 +4,9 @@ @@
     use RuntimeException;
+    /**
+     * @deprecated Will be removed in a future Laravel version.
+     */
     class CursorPaginationException extends RuntimeException
     {
         //
@@ Expand Down @@