From 99429c0ef596498a769d220c995ee31d23b3b386 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 17 Oct 2024 23:55:03 +0300 Subject: [PATCH 1/3] Check that correct sort is passed to `withSort()` of keyset paginator --- CHANGELOG.md | 3 ++- src/Paginator/KeysetPaginator.php | 12 ++++++++++-- tests/Paginator/KeysetPaginatorTest.php | 26 +++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b81381..0d2088c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,8 @@ - Chg #163: Rename `FilterableDataInterface::withFilterHandlers()` to `FilterableDataInterface::withAddedFilterHandlers()` (@samdark) - Enh #190: Use `str_contains` for case-sensitive match in `LikeHandler` (@samdark) - Enh #194: Improve psalm annotations in `LimitableDataInterface` (@vjik) -- Bug #195: Fix invalid count in `IterableDataReader` when limit or/and offset used (@vjik) +- Bug #195: Fix invalid count in `IterableDataReader` when limit or/and offset used (@vjik) +- Enh #202: Check that correct sort is passed to `withSort()` of keyset paginator (@samdark) ## 1.0.1 January 25, 2023 diff --git a/src/Paginator/KeysetPaginator.php b/src/Paginator/KeysetPaginator.php index 6945ee5..674dde5 100644 --- a/src/Paginator/KeysetPaginator.php +++ b/src/Paginator/KeysetPaginator.php @@ -125,11 +125,11 @@ public function __construct(ReadableDataInterface $dataReader) $sort = $dataReader->getSort(); if ($sort === null) { - throw new RuntimeException('Data sorting should be configured to work with keyset pagination.'); + throw new InvalidArgumentException('Data sorting should be configured to work with keyset pagination.'); } if (empty($sort->getOrder())) { - throw new RuntimeException('Data should be always sorted to work with keyset pagination.'); + throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.'); } $this->dataReader = $dataReader; @@ -262,6 +262,14 @@ public function isSortable(): bool public function withSort(?Sort $sort): static { + if ($sort === null) { + throw new InvalidArgumentException('Data sorting should be configured to work with keyset pagination.'); + } + + if (empty($sort->getOrder())) { + throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.'); + } + $new = clone $this; $new->dataReader = $this->dataReader->withSort($sort); return $new; diff --git a/tests/Paginator/KeysetPaginatorTest.php b/tests/Paginator/KeysetPaginatorTest.php index 1a3317e..088bed8 100644 --- a/tests/Paginator/KeysetPaginatorTest.php +++ b/tests/Paginator/KeysetPaginatorTest.php @@ -149,23 +149,45 @@ public function testThrowsExceptionWhenReaderHasNoSort(): void { $dataReader = new IterableDataReader(self::getDataSet()); - $this->expectException(RuntimeException::class); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Data sorting should be configured to work with keyset pagination.'); new KeysetPaginator($dataReader); } + public function testThrowsExceptionForWithSortNull(): void + { + $sort = Sort::only(['id', 'name'])->withOrderString('id'); + $dataReader = (new IterableDataReader(self::getDataSet()))->withSort($sort); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Data sorting should be configured to work with keyset pagination.'); + + (new KeysetPaginator($dataReader))->withSort(null); + } + public function testThrowsExceptionWhenNotSorted(): void { $sort = Sort::only(['id', 'name']); $dataReader = (new IterableDataReader(self::getDataSet()))->withSort($sort); - $this->expectException(RuntimeException::class); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Data should be always sorted to work with keyset pagination.'); new KeysetPaginator($dataReader); } + public function testThrowsExceptionForWithSortNotSorted(): void + { + $sort = Sort::only(['id', 'name'])->withOrderString('id'); + $dataReader = (new IterableDataReader(self::getDataSet()))->withSort($sort); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Data should be always sorted to work with keyset pagination.'); + + (new KeysetPaginator($dataReader))->withSort(Sort::only(['id', 'name'])); + } + public function testPageSizeCannotBeLessThanOne(): void { $sort = Sort::only(['id', 'name'])->withOrderString('id'); From 8bd435c68d268653c66c879ef49757142951c3c4 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Thu, 17 Oct 2024 20:55:54 +0000 Subject: [PATCH 2/3] Apply fixes from StyleCI --- src/Paginator/KeysetPaginator.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Paginator/KeysetPaginator.php b/src/Paginator/KeysetPaginator.php index 674dde5..cb03456 100644 --- a/src/Paginator/KeysetPaginator.php +++ b/src/Paginator/KeysetPaginator.php @@ -6,7 +6,6 @@ use Closure; use InvalidArgumentException; -use RuntimeException; use Yiisoft\Arrays\ArrayHelper; use Yiisoft\Data\Reader\Filter\GreaterThan; use Yiisoft\Data\Reader\Filter\GreaterThanOrEqual; From 1a0894bfa71bb1149e11b172938d4e0525943d5d Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sat, 19 Oct 2024 00:10:03 +0300 Subject: [PATCH 3/3] Refactor --- src/Paginator/KeysetPaginator.php | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Paginator/KeysetPaginator.php b/src/Paginator/KeysetPaginator.php index cb03456..c42a945 100644 --- a/src/Paginator/KeysetPaginator.php +++ b/src/Paginator/KeysetPaginator.php @@ -122,14 +122,7 @@ public function __construct(ReadableDataInterface $dataReader) } $sort = $dataReader->getSort(); - - if ($sort === null) { - throw new InvalidArgumentException('Data sorting should be configured to work with keyset pagination.'); - } - - if (empty($sort->getOrder())) { - throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.'); - } + $this->assertSort($sort); $this->dataReader = $dataReader; } @@ -261,13 +254,7 @@ public function isSortable(): bool public function withSort(?Sort $sort): static { - if ($sort === null) { - throw new InvalidArgumentException('Data sorting should be configured to work with keyset pagination.'); - } - - if (empty($sort->getOrder())) { - throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.'); - } + $this->assertSort($sort); $new = clone $this; $new->dataReader = $this->dataReader->withSort($sort); @@ -447,4 +434,15 @@ private function getFieldAndSortingFromSort(Sort $sort): array reset($order) === 'asc' ? SORT_ASC : SORT_DESC, ]; } + + private function assertSort(?Sort $sort): void + { + if ($sort === null) { + throw new InvalidArgumentException('Data sorting should be configured to work with keyset pagination.'); + } + + if (empty($sort->getOrder())) { + throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.'); + } + } }