From bea3c2db8ee4802f68954c3d61d300bcc1e99292 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:03:04 +0300 Subject: [PATCH 01/13] Throw `PageNotFoundException` instead of `PaginatorException` in `OfferPaginator::withCurrentPage()` --- CHANGELOG.md | 2 ++ src/Paginator/OffsetPaginator.php | 20 +++++++++++++------- src/Paginator/PageNotFoundException.php | 4 ++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bffd0e1b..9757e26d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ - Enh #214: Improved interface hierarchy (@samdark) - Enh #207: More specific Psalm type for `OffsetPaginator::withCurrentPage()` parameter (@samdark) - Enh #210: More specific Psalm type for `PaginatorInterface::getPageSize()` result (@vjik) +- Enh #219: Add ability to set custom message in `PageNotFoundException` (@vjik) +- Enh #219: Throw `PageNotFoundException` instead of `PaginatorException` in `OfferPaginator::withCurrentPage()` (@vjik) ## 1.0.1 January 25, 2023 diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index addebdcf..13a88e78 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -115,7 +115,7 @@ public function withPageSize(int $pageSize): static * @param int $page Page number. * @psalm-param positive-int $page * - * @throws PaginatorException If the current page is incorrect. + * @throws PageNotFoundException If the current page is zero or less. * * @return self New instance. */ @@ -123,7 +123,7 @@ public function withCurrentPage(int $page): self { /** @psalm-suppress DocblockTypeContradiction */ if ($page < 1) { - throw new PaginatorException('Current page should be at least 1.'); + throw new PageNotFoundException('Current page should be at least 1.'); } $new = clone $this; @@ -276,8 +276,11 @@ public function withFilter(FilterInterface $filter): static */ public function read(): iterable { - if ($this->getCurrentPage() > $this->getInternalTotalPages()) { - throw new PageNotFoundException(); + $currentPage = $this->getCurrentPage(); + if ($currentPage > $this->getInternalTotalPages()) { + throw new PageNotFoundException( + sprintf('Page %d not found.', $currentPage) + ); } $limit = $this->pageSize; @@ -316,11 +319,14 @@ public function isOnFirstPage(): bool public function isOnLastPage(): bool { - if ($this->getCurrentPage() > $this->getInternalTotalPages()) { - throw new PageNotFoundException(); + $currentPage = $this->getCurrentPage(); + if ($currentPage > $this->getInternalTotalPages()) { + throw new PageNotFoundException( + sprintf('Page %d not found.', $currentPage) + ); } - return $this->getCurrentPage() === $this->getInternalTotalPages(); + return $currentPage === $this->getInternalTotalPages(); } public function isPaginationRequired(): bool diff --git a/src/Paginator/PageNotFoundException.php b/src/Paginator/PageNotFoundException.php index 279967fb..6164a36a 100644 --- a/src/Paginator/PageNotFoundException.php +++ b/src/Paginator/PageNotFoundException.php @@ -6,8 +6,8 @@ final class PageNotFoundException extends PaginatorException { - public function __construct() + public function __construct(string $message = 'Page not found.') { - parent::__construct('Page not found.'); + parent::__construct($message); } } From cd535c15e8dd32f09a613d653d7dc8368b932126 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:07:56 +0300 Subject: [PATCH 02/13] fix tests --- tests/Paginator/OffsetPaginatorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Paginator/OffsetPaginatorTest.php b/tests/Paginator/OffsetPaginatorTest.php index 03419c57..26b38a87 100644 --- a/tests/Paginator/OffsetPaginatorTest.php +++ b/tests/Paginator/OffsetPaginatorTest.php @@ -235,7 +235,7 @@ public function testReadCurrentPageCannotBeLargerThanMaxPages(): void $this->assertSame(3, $paginator->getTotalPages()); $this->expectException(PageNotFoundException::class); - $this->expectExceptionMessage('Page not found.'); + $this->expectExceptionMessage('Page 4 not found.'); $this->iterableToArray($paginator->read()); } @@ -410,7 +410,7 @@ public function testIsLastPageBeyondMaxPages(): void $this->assertSame(3, $paginator->getTotalPages()); $this->expectException(PageNotFoundException::class); - $this->expectExceptionMessage('Page not found.'); + $this->expectExceptionMessage('Page 4 not found.'); $paginator->isOnLastPage(); } From 5a62c0f46f4af6cda69590c6ce287cbe5fbd7efd Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:20:41 +0300 Subject: [PATCH 03/13] improve --- src/Paginator/KeysetPaginator.php | 1 + src/Paginator/OffsetPaginator.php | 28 ++++++++++--------------- src/Paginator/PageNotFoundException.php | 8 +++++-- src/Paginator/PaginatorInterface.php | 2 ++ tests/Paginator/OffsetPaginatorTest.php | 19 ++--------------- 5 files changed, 22 insertions(+), 36 deletions(-) diff --git a/src/Paginator/KeysetPaginator.php b/src/Paginator/KeysetPaginator.php index c42a9455..2d4e90f1 100644 --- a/src/Paginator/KeysetPaginator.php +++ b/src/Paginator/KeysetPaginator.php @@ -150,6 +150,7 @@ public function getToken(): ?PageToken public function withPageSize(int $pageSize): static { + /** @psalm-suppress DocblockTypeContradiction We don't believe in psalm types */ if ($pageSize < 1) { throw new InvalidArgumentException('Page size should be at least 1.'); } diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index 13a88e78..694117d1 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -94,14 +94,19 @@ public function __construct(ReadableDataInterface $dataReader) public function withToken(?PageToken $token): static { - /** @psalm-suppress ArgumentTypeCoercion */ - return $this->withCurrentPage($token === null ? 1 : (int)$token->value); + $page = $token === null ? 1 : (int) $token->value; + if ($page < 1) { + throw new PaginatorException('Current page should be at least 1.'); + } + + return $this->withCurrentPage($token === null ? 1 : $page); } public function withPageSize(int $pageSize): static { + /** @psalm-suppress DocblockTypeContradiction We don't believe in psalm types */ if ($pageSize < 1) { - throw new PaginatorException('Page size should be at least 1.'); + throw new InvalidArgumentException('Page size should be at least 1.'); } $new = clone $this; @@ -115,15 +120,13 @@ public function withPageSize(int $pageSize): static * @param int $page Page number. * @psalm-param positive-int $page * - * @throws PageNotFoundException If the current page is zero or less. - * * @return self New instance. */ public function withCurrentPage(int $page): self { /** @psalm-suppress DocblockTypeContradiction */ if ($page < 1) { - throw new PageNotFoundException('Current page should be at least 1.'); + throw new InvalidArgumentException('Current page should be at least 1.'); } $new = clone $this; @@ -278,9 +281,7 @@ public function read(): iterable { $currentPage = $this->getCurrentPage(); if ($currentPage > $this->getInternalTotalPages()) { - throw new PageNotFoundException( - sprintf('Page %d not found.', $currentPage) - ); + throw new PageNotFoundException($currentPage); } $limit = $this->pageSize; @@ -319,14 +320,7 @@ public function isOnFirstPage(): bool public function isOnLastPage(): bool { - $currentPage = $this->getCurrentPage(); - if ($currentPage > $this->getInternalTotalPages()) { - throw new PageNotFoundException( - sprintf('Page %d not found.', $currentPage) - ); - } - - return $currentPage === $this->getInternalTotalPages(); + return $this->getCurrentPage() === $this->getInternalTotalPages(); } public function isPaginationRequired(): bool diff --git a/src/Paginator/PageNotFoundException.php b/src/Paginator/PageNotFoundException.php index 6164a36a..d6d4065e 100644 --- a/src/Paginator/PageNotFoundException.php +++ b/src/Paginator/PageNotFoundException.php @@ -4,10 +4,14 @@ namespace Yiisoft\Data\Paginator; +use function sprintf; + final class PageNotFoundException extends PaginatorException { - public function __construct(string $message = 'Page not found.') + public function __construct(int $page) { - parent::__construct($message); + parent::__construct( + sprintf('Page %d not found.', $page) + ); } } diff --git a/src/Paginator/PaginatorInterface.php b/src/Paginator/PaginatorInterface.php index ac0e8200..539c9929 100644 --- a/src/Paginator/PaginatorInterface.php +++ b/src/Paginator/PaginatorInterface.php @@ -50,6 +50,8 @@ public function withToken(?PageToken $token): static; * @throws PaginatorException If page size is incorrect. * * @return static New instance. + * + * @psalm-param positive-int $pageSize */ public function withPageSize(int $pageSize): static; diff --git a/tests/Paginator/OffsetPaginatorTest.php b/tests/Paginator/OffsetPaginatorTest.php index 26b38a87..985b5d75 100644 --- a/tests/Paginator/OffsetPaginatorTest.php +++ b/tests/Paginator/OffsetPaginatorTest.php @@ -219,7 +219,7 @@ public function testCurrentPageCannotBeLessThanOne(): void $dataReader = new IterableDataReader(self::DEFAULT_DATASET); $paginator = new OffsetPaginator($dataReader); - $this->expectException(PaginatorException::class); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Current page should be at least 1.'); $paginator->withCurrentPage(0); @@ -255,7 +255,7 @@ public function testPageSizeCannotBeLessThanOne(): void $dataReader = new IterableDataReader(self::DEFAULT_DATASET); $paginator = new OffsetPaginator($dataReader); - $this->expectException(PaginatorException::class); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Page size should be at least 1.'); $paginator->withPageSize(0); @@ -400,21 +400,6 @@ public function testIsLastPageOnLastPage(): void $this->assertTrue($paginator->isOnLastPage()); } - public function testIsLastPageBeyondMaxPages(): void - { - $dataReader = new IterableDataReader(self::DEFAULT_DATASET); - $paginator = (new OffsetPaginator($dataReader)) - ->withPageSize(2) - ->withCurrentPage(4) - ; - - $this->assertSame(3, $paginator->getTotalPages()); - $this->expectException(PageNotFoundException::class); - $this->expectExceptionMessage('Page 4 not found.'); - - $paginator->isOnLastPage(); - } - public function testGetCurrentPageSizeFirstFullPage(): void { $dataReader = new IterableDataReader(self::DEFAULT_DATASET); From 6c0b89bdc44838190fd581683c3cd12fbf3c8f94 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Tue, 24 Dec 2024 14:21:04 +0000 Subject: [PATCH 04/13] Apply fixes from StyleCI --- tests/Paginator/OffsetPaginatorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Paginator/OffsetPaginatorTest.php b/tests/Paginator/OffsetPaginatorTest.php index 985b5d75..5989d018 100644 --- a/tests/Paginator/OffsetPaginatorTest.php +++ b/tests/Paginator/OffsetPaginatorTest.php @@ -10,7 +10,6 @@ use Yiisoft\Data\Paginator\OffsetPaginator; use Yiisoft\Data\Paginator\PageNotFoundException; use Yiisoft\Data\Paginator\PageToken; -use Yiisoft\Data\Paginator\PaginatorException; use Yiisoft\Data\Paginator\PaginatorInterface; use Yiisoft\Data\Reader\CountableDataInterface; use Yiisoft\Data\Reader\Filter\Equals; From f335ae9b2ab93208c6bdcef5158cc8b02deb8de0 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:25:18 +0300 Subject: [PATCH 05/13] improve --- src/Paginator/OffsetPaginator.php | 5 ++++- src/Paginator/PaginatorInterface.php | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index 694117d1..28c19efd 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -118,9 +118,12 @@ public function withPageSize(int $pageSize): static * Get a new instance with the given current page number set. * * @param int $page Page number. - * @psalm-param positive-int $page + * + * @throws InvalidArgumentException If page is not positive number. * * @return self New instance. + * + * @psalm-param positive-int $page */ public function withCurrentPage(int $page): self { diff --git a/src/Paginator/PaginatorInterface.php b/src/Paginator/PaginatorInterface.php index 539c9929..2fe44539 100644 --- a/src/Paginator/PaginatorInterface.php +++ b/src/Paginator/PaginatorInterface.php @@ -4,6 +4,7 @@ namespace Yiisoft\Data\Paginator; +use InvalidArgumentException; use LogicException; use Yiisoft\Data\Reader\FilterInterface; use Yiisoft\Data\Reader\ReadableDataInterface; @@ -48,6 +49,7 @@ public function withToken(?PageToken $token): static; * @param int $pageSize Maximum number of items per page. * * @throws PaginatorException If page size is incorrect. + * @throws InvalidArgumentException If page size is not positive number. * * @return static New instance. * From 8e7642eb750a3f3fae9d5ff2d5e55643db05923e Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:33:17 +0300 Subject: [PATCH 06/13] improve --- src/Paginator/OffsetPaginator.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index 28c19efd..5310fe62 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -94,12 +94,16 @@ public function __construct(ReadableDataInterface $dataReader) public function withToken(?PageToken $token): static { - $page = $token === null ? 1 : (int) $token->value; - if ($page < 1) { - throw new PaginatorException('Current page should be at least 1.'); + if ($token === null) { + $page = 1; + } else { + $page = (int) $token->value; + if ($page < 1) { + throw new PaginatorException('Current page should be at least 1.'); + } } - return $this->withCurrentPage($token === null ? 1 : $page); + return $this->withCurrentPage($page); } public function withPageSize(int $pageSize): static From b0a4f93824af3fbdc44a0f04a20cff895b0dbe42 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Tue, 24 Dec 2024 17:37:31 +0300 Subject: [PATCH 07/13] improve --- CHANGELOG.md | 8 ++++++-- src/Paginator/PaginatorInterface.php | 2 -- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9757e26d..11c28efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,8 +43,12 @@ - Enh #214: Improved interface hierarchy (@samdark) - Enh #207: More specific Psalm type for `OffsetPaginator::withCurrentPage()` parameter (@samdark) - Enh #210: More specific Psalm type for `PaginatorInterface::getPageSize()` result (@vjik) -- Enh #219: Add ability to set custom message in `PageNotFoundException` (@vjik) -- Enh #219: Throw `PageNotFoundException` instead of `PaginatorException` in `OfferPaginator::withCurrentPage()` (@vjik) +- Chg #219: Narrow type of page size in `PaginatorInterface::withPageSize()` method to positive int by psalm + annotation and throw `InvalidArgumentException` if non-positive value is passed (@vjik) +- Enh #219: Add page to message of `PageNotFoundException` exception (@vjik) +- Chg #219: Throw `InvalidArgumentException` instead of `PaginatorException` in `OffsetPaginator::withCurrentPage()` + method when non-positive value is passed (@vjik) +- Chg #219: Don't check correctness of current page in `PaginatorInterface::isOnLastPage()` method (@vjik) ## 1.0.1 January 25, 2023 diff --git a/src/Paginator/PaginatorInterface.php b/src/Paginator/PaginatorInterface.php index 2fe44539..d90ba1ce 100644 --- a/src/Paginator/PaginatorInterface.php +++ b/src/Paginator/PaginatorInterface.php @@ -152,8 +152,6 @@ public function read(): iterable; /** * Get whether the current page is the last one. * - * @throws PageNotFoundException If the page specified isn't found. - * * @return bool Whether the current page is the last one. */ public function isOnLastPage(): bool; From e9ba79094a8e5a3a2fb05915a896451bf3060633 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Wed, 25 Dec 2024 08:53:53 +0300 Subject: [PATCH 08/13] Update src/Paginator/OffsetPaginator.php Co-authored-by: Alexander Makarov --- src/Paginator/OffsetPaginator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index 5310fe62..8bbb1571 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -123,7 +123,7 @@ public function withPageSize(int $pageSize): static * * @param int $page Page number. * - * @throws InvalidArgumentException If page is not positive number. + * @throws InvalidArgumentException If page is not a positive number. * * @return self New instance. * From 2a01f9861e9fad27424b5e66cb1b8d87142a4d90 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Wed, 25 Dec 2024 08:53:59 +0300 Subject: [PATCH 09/13] Update src/Paginator/PaginatorInterface.php Co-authored-by: Alexander Makarov --- src/Paginator/PaginatorInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Paginator/PaginatorInterface.php b/src/Paginator/PaginatorInterface.php index d90ba1ce..8e4f85a0 100644 --- a/src/Paginator/PaginatorInterface.php +++ b/src/Paginator/PaginatorInterface.php @@ -49,7 +49,7 @@ public function withToken(?PageToken $token): static; * @param int $pageSize Maximum number of items per page. * * @throws PaginatorException If page size is incorrect. - * @throws InvalidArgumentException If page size is not positive number. + * @throws InvalidArgumentException If page size is not a positive number. * * @return static New instance. * From 576b1a879507216e94ab7dfa716b09092f5c0538 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Wed, 25 Dec 2024 08:56:52 +0300 Subject: [PATCH 10/13] fix phpdoc --- src/Paginator/PaginatorInterface.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Paginator/PaginatorInterface.php b/src/Paginator/PaginatorInterface.php index 8e4f85a0..27eff992 100644 --- a/src/Paginator/PaginatorInterface.php +++ b/src/Paginator/PaginatorInterface.php @@ -48,7 +48,6 @@ public function withToken(?PageToken $token): static; * * @param int $pageSize Maximum number of items per page. * - * @throws PaginatorException If page size is incorrect. * @throws InvalidArgumentException If page size is not a positive number. * * @return static New instance. From f953f91df51cc8675b5fbfb184eb1a7188bed9e7 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Sun, 5 Jan 2025 18:33:07 +0300 Subject: [PATCH 11/13] rename `PaginatorException` to `InvalidPageException` --- src/Paginator/InvalidPageException.php | 14 ++++++++++++++ src/Paginator/OffsetPaginator.php | 2 +- src/Paginator/PageNotFoundException.php | 2 +- src/Paginator/PaginatorException.php | 16 ---------------- src/Paginator/PaginatorInterface.php | 2 +- 5 files changed, 17 insertions(+), 19 deletions(-) create mode 100644 src/Paginator/InvalidPageException.php delete mode 100644 src/Paginator/PaginatorException.php diff --git a/src/Paginator/InvalidPageException.php b/src/Paginator/InvalidPageException.php new file mode 100644 index 00000000..b91eb7b4 --- /dev/null +++ b/src/Paginator/InvalidPageException.php @@ -0,0 +1,14 @@ +value; if ($page < 1) { - throw new PaginatorException('Current page should be at least 1.'); + throw new InvalidPageException('Current page should be at least 1.'); } } diff --git a/src/Paginator/PageNotFoundException.php b/src/Paginator/PageNotFoundException.php index d6d4065e..1341829e 100644 --- a/src/Paginator/PageNotFoundException.php +++ b/src/Paginator/PageNotFoundException.php @@ -6,7 +6,7 @@ use function sprintf; -final class PageNotFoundException extends PaginatorException +final class PageNotFoundException extends InvalidPageException { public function __construct(int $page) { diff --git a/src/Paginator/PaginatorException.php b/src/Paginator/PaginatorException.php deleted file mode 100644 index c821196e..00000000 --- a/src/Paginator/PaginatorException.php +++ /dev/null @@ -1,16 +0,0 @@ - Date: Sun, 5 Jan 2025 18:35:21 +0300 Subject: [PATCH 12/13] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11c28efd..b9bd47fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ - Chg #219: Throw `InvalidArgumentException` instead of `PaginatorException` in `OffsetPaginator::withCurrentPage()` method when non-positive value is passed (@vjik) - Chg #219: Don't check correctness of current page in `PaginatorInterface::isOnLastPage()` method (@vjik) +- Chg #219: Rename `PaginatorException` to `InvalidPageException` (@vjik) ## 1.0.1 January 25, 2023 From c763b78637fd47cc0d5fa5689ad12851e43f9db8 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Sun, 5 Jan 2025 18:38:47 +0300 Subject: [PATCH 13/13] fix --- src/Paginator/OffsetPaginator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Paginator/OffsetPaginator.php b/src/Paginator/OffsetPaginator.php index 74ec96de..add94ed3 100644 --- a/src/Paginator/OffsetPaginator.php +++ b/src/Paginator/OffsetPaginator.php @@ -327,7 +327,7 @@ public function isOnFirstPage(): bool public function isOnLastPage(): bool { - return $this->getCurrentPage() === $this->getInternalTotalPages(); + return $this->getCurrentPage() >= $this->getInternalTotalPages(); } public function isPaginationRequired(): bool