From 80f01125d8d85dccaf3a5501784afeae6d4c4a76 Mon Sep 17 00:00:00 2001 From: Hamid Alaei Varnosfaderani Date: Thu, 8 Jul 2021 18:59:50 +0430 Subject: [PATCH] [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 --- .../Database/Concerns/BuildsQueries.php | 56 ++++++++- src/Illuminate/Database/Eloquent/Builder.php | 39 +----- src/Illuminate/Database/Query/Builder.php | 36 +----- .../Pagination/CursorPaginationException.php | 3 + tests/Database/DatabaseQueryBuilderTest.php | 112 ++++++++++++++++-- 5 files changed, 157 insertions(+), 89 deletions(-) diff --git a/src/Illuminate/Database/Concerns/BuildsQueries.php b/src/Illuminate/Database/Concerns/BuildsQueries.php index d95b5de6b70d..6b91a673676a 100644 --- 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 index 70de771f31c5..b13a1b1a0081 100755 --- 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,37 +826,12 @@ 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); } /** @@ -866,18 +839,12 @@ public function cursorPaginate($perPage = null, $columns = ['*'], $cursorName = * * @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 index 2732e33cf8a9..7eaaeb316287 100755 --- 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,35 +2406,10 @@ 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); } /** @@ -2444,18 +2417,11 @@ public function cursorPaginate($perPage = 15, $columns = ['*'], $cursorName = 'c * * @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 index 710401751a56..b12ca607f185 100644 --- 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 index d5013fab3c60..6a511b9232e1 100755 --- 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() {