Change exceptions usage in paginator

CHANGELOG.md

@@ -43,6 +43,13 @@
 - 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)
+- 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)
+- Chg #219: Rename `PaginatorException` to `InvalidPageException` (@vjik)
 
 ## 1.0.1 January 25, 2023
 

src/Paginator/InvalidPageException.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Data\Paginator;
+
+use LogicException;
+
+/**
+ * Thrown when the page is invalid.
+ */
+class InvalidPageException extends LogicException
+{
+}

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.');
         }

src/Paginator/OffsetPaginator.php

@@ -94,14 +94,23 @@ public function __construct(ReadableDataInterface $dataReader)
 
     public function withToken(?PageToken $token): static
     {
-        /** @psalm-suppress ArgumentTypeCoercion */
-        return $this->withCurrentPage($token === null ? 1 : (int)$token->value);
+        if ($token === null) {
+            $page = 1;
+        } else {
+            $page = (int) $token->value;
+            if ($page < 1) {
+                throw new InvalidPageException('Current page should be at least 1.');
+            }
+        }
+
+        return $this->withCurrentPage($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;
@@ -113,17 +122,18 @@ 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 PaginatorException If the current page is incorrect.
+     * @throws InvalidArgumentException If page is not a positive number.
      *
      * @return self New instance.
+     *
+     * @psalm-param positive-int $page
      */
     public function withCurrentPage(int $page): self
     {
         /** @psalm-suppress DocblockTypeContradiction */
         if ($page < 1) {
-            throw new PaginatorException('Current page should be at least 1.');
+            throw new InvalidArgumentException('Current page should be at least 1.');
         }
 
         $new = clone $this;
@@ -276,8 +286,9 @@ 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($currentPage);
         }
 
         $limit = $this->pageSize;
@@ -316,11 +327,7 @@ public function isOnFirstPage(): bool
 
     public function isOnLastPage(): bool
     {
-        if ($this->getCurrentPage() > $this->getInternalTotalPages()) {
-            throw new PageNotFoundException();
-        }
-
-        return $this->getCurrentPage() === $this->getInternalTotalPages();
+        return $this->getCurrentPage() >= $this->getInternalTotalPages();
     }
 
     public function isPaginationRequired(): bool

src/Paginator/PageNotFoundException.php

@@ -4,10 +4,14 @@
 
 namespace Yiisoft\Data\Paginator;
 
-final class PageNotFoundException extends PaginatorException
+use function sprintf;
+
+final class PageNotFoundException extends InvalidPageException
 {
-    public function __construct()
+    public function __construct(int $page)
     {
-        parent::__construct('Page not found.');
+        parent::__construct(
+            sprintf('Page %d not found.', $page)
+        );
     }
 }

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;
@@ -34,7 +35,7 @@ interface PaginatorInterface extends ReadableDataInterface
      *
      * @param PageToken|null $token Page token. `Null` if current page is first.
      *
-     * @throws PaginatorException If page token is incorrect.
+     * @throws InvalidPageException If page token is incorrect.
      *
      * @return static New instance.
      *
@@ -47,9 +48,11 @@ 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.
+     *
+     * @psalm-param positive-int $pageSize
      */
     public function withPageSize(int $pageSize): static;
 
@@ -148,8 +151,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;

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;
@@ -219,7 +218,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);
@@ -235,7 +234,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());
     }
@@ -255,7 +254,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 +399,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 not found.');
-
-        $paginator->isOnLastPage();
-    }
-
     public function testGetCurrentPageSizeFirstFullPage(): void
     {
         $dataReader = new IterableDataReader(self::DEFAULT_DATASET);