From 306c60fceb9d9f1ba73f7da2226a64e5ab1f9e42 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Sun, 5 Jul 2026 08:36:16 +0900 Subject: [PATCH 1/5] Add PagesInterface::getNbPages() PagesInterface exposed only the result count via Countable::count(), never the number of pages. Every consumer had to recompute `ceil(count / perPage)` by hand, duplicating logic that already lives in the underlying Pagerfanta and forcing callers to keep perPage around just for that division. Add `getNbPages(): int` to PagesInterface and implement it in Pages (`ceil` of `count / perPage`) and MappedPages (delegation). SqlQuery now forwards the perPage it already receives in getPages() to the Pages constructor, so callers can read the page count directly from the PagesInterface without recomputing it. Note: this adds a method to a public interface and a required constructor argument to the final Pages class. --- src/MappedPages.php | 6 ++++++ src/Pages.php | 8 ++++++++ src/PagesInterface.php | 4 ++++ src/SqlQuery.php | 2 +- tests/Fake/FakeEmptyPages.php | 5 +++++ tests/Fake/FakePages.php | 6 ++++++ tests/Fake/MappedPagesFakePages.php | 5 +++++ tests/PagesTest.php | 1 + tests/SqlQueryTest.php | 7 +++++++ 9 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/MappedPages.php b/src/MappedPages.php index 4c0fc6ff..77de5a7b 100644 --- a/src/MappedPages.php +++ b/src/MappedPages.php @@ -61,4 +61,10 @@ public function count(): int { return $this->pages->count(); } + + #[Override] + public function getNbPages(): int + { + return $this->pages->getNbPages(); + } } diff --git a/src/Pages.php b/src/Pages.php index 4f820e40..50de0d7e 100644 --- a/src/Pages.php +++ b/src/Pages.php @@ -11,6 +11,7 @@ use Ray\AuraSqlModule\Pagerfanta\Page; use Ray\MediaQuery\Exception\LogicException; +use function ceil; use function is_array; /** @template T of class-string|mixed */ @@ -28,6 +29,7 @@ public function __construct( private ExtendedPdoInterface $pdo, private string $sql, private array $params, + private int $perPage, callable|null $rowMapper = null, ) { $this->rowMapper = $rowMapper; @@ -95,4 +97,10 @@ public function count(): int { return (new ExtendedPdoAdapter($this->pdo, $this->sql, $this->params))->getNbResults(); } + + #[Override] + public function getNbPages(): int + { + return (int) ceil($this->count() / $this->perPage); + } } diff --git a/src/PagesInterface.php b/src/PagesInterface.php index 3716bc4b..c020c2ae 100644 --- a/src/PagesInterface.php +++ b/src/PagesInterface.php @@ -10,4 +10,8 @@ /** @extends ArrayAccess */ interface PagesInterface extends ArrayAccess, Countable { + /** + * Returns the total number of pages (ceil of result count / perPage). + */ + public function getNbPages(): int; } diff --git a/src/SqlQuery.php b/src/SqlQuery.php index 0bb8f395..5c42ea38 100644 --- a/src/SqlQuery.php +++ b/src/SqlQuery.php @@ -236,7 +236,7 @@ public function getPages(string $sqlId, array $values, int $perPage, string $que $pager = $this->pagerFactory->newInstance($this->pdo, $this->getSql($sqlId), $values, $perPage, $queryTemplate, $entity); /** @var array $values */ - return new Pages($pager, $this->pdo, $this->getSql($sqlId), $values); + return new Pages($pager, $this->pdo, $this->getSql($sqlId), $values, $perPage); } private function getSql(string $sqlId): string diff --git a/tests/Fake/FakeEmptyPages.php b/tests/Fake/FakeEmptyPages.php index c814381e..d374c3da 100644 --- a/tests/Fake/FakeEmptyPages.php +++ b/tests/Fake/FakeEmptyPages.php @@ -40,4 +40,9 @@ public function count(): int { return $this->iter->count(); } + + public function getNbPages(): int + { + return 0; + } } diff --git a/tests/Fake/FakePages.php b/tests/Fake/FakePages.php index 6c3bdda0..edffb89c 100644 --- a/tests/Fake/FakePages.php +++ b/tests/Fake/FakePages.php @@ -30,4 +30,10 @@ public function count(): int { // TODO: Implement count() method. } + + public function getNbPages(): int + { + // TODO: Implement getNbPages() method. + return 0; + } } diff --git a/tests/Fake/MappedPagesFakePages.php b/tests/Fake/MappedPagesFakePages.php index ea1d5a48..74b2833e 100644 --- a/tests/Fake/MappedPagesFakePages.php +++ b/tests/Fake/MappedPagesFakePages.php @@ -46,4 +46,9 @@ public function count(): int { return 1; } + + public function getNbPages(): int + { + return 1; + } } diff --git a/tests/PagesTest.php b/tests/PagesTest.php index 51fa1c13..883e8f71 100644 --- a/tests/PagesTest.php +++ b/tests/PagesTest.php @@ -20,6 +20,7 @@ public function testOffsetGetReturnsNullWhenDelegateHasNoPage(): void $this->createStub(ExtendedPdoInterface::class), 'SELECT 1', [], + 10, ); $this->assertFalse(isset($pages[3])); diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 9774e76d..9e129224 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -108,6 +108,13 @@ public function testPagerCount(Pages $pages): void $this->assertSame(2, count($pages)); } + /** @param Pages $pages */ + #[Depends('testPager')] + public function testPagerNbPages(Pages $pages): void + { + $this->assertSame(2, $pages->getNbPages()); + } + public function testCount(): void { $this->sqlQuery->exec('todo_add', ['id' => '2', 'title' => 'walk']); From ca6a484d86e4fec3921ebbec25a7d262697374b0 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Sun, 5 Jul 2026 09:12:43 +0900 Subject: [PATCH 2/5] Add getNbPages() coverage for MappedPages and perPage > 1 - MappedPages: test that getNbPages() delegates to the wrapped pages. - Pages: add perPage=2 cases (exact division and ceil round-up) so getNbPages() is covered beyond the perPage=1 smoke test. 119 tests, 221 assertions OK; phpstan and psalm clean. --- tests/MappedPagesTest.php | 11 +++++++++++ tests/SqlQueryTest.php | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/MappedPagesTest.php b/tests/MappedPagesTest.php index 66122a59..a155a9e0 100644 --- a/tests/MappedPagesTest.php +++ b/tests/MappedPagesTest.php @@ -62,6 +62,17 @@ public function testDelegatesMutationAndCount(): void $this->assertSame(1, $pages->count()); } + public function testDelegatesGetNbPages(): void + { + $delegate = $this->pages($this->page([])); + $pages = new MappedPages( + $delegate, + static fn (array $row): array => $row, + ); + + $this->assertSame(1, $pages->getNbPages()); + } + private function page(mixed $data): Page { $reflection = new ReflectionClass(Page::class); diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 9e129224..11ef1a3c 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -115,6 +115,23 @@ public function testPagerNbPages(Pages $pages): void $this->assertSame(2, $pages->getNbPages()); } + public function testPagerNbPagesWithPerPage2(): void + { + $this->sqlQuery->exec('todo_add', ['id' => '2', 'title' => 'walk']); + $pages = $this->sqlQuery->getPages('todo_list', [], 2); + + $this->assertSame(1, $pages->getNbPages()); + } + + public function testPagerNbPagesRoundsUp(): void + { + $this->sqlQuery->exec('todo_add', ['id' => '2', 'title' => 'walk']); + $this->sqlQuery->exec('todo_add', ['id' => '3', 'title' => 'sleep']); + $pages = $this->sqlQuery->getPages('todo_list', [], 2); + + $this->assertSame(2, $pages->getNbPages()); + } + public function testCount(): void { $this->sqlQuery->exec('todo_add', ['id' => '2', 'title' => 'walk']); From ca9dde930431fb82dddbcd9a61f292ef9c293130 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 04:25:44 +0000 Subject: [PATCH 3/5] Guard against perPage < 1 in Pages constructor CodeRabbit flagged that Pages::getNbPages() divides by perPage, which throws DivisionByZeroError if perPage is 0. Pages has a public constructor and can be instantiated directly, so validate perPage there instead of relying on callers like SqlQuery::getPages(). --- src/Exception/InvalidPerPageException.php | 12 ++++++++++ src/Pages.php | 5 +++++ tests/PagesTest.php | 27 +++++++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 src/Exception/InvalidPerPageException.php diff --git a/src/Exception/InvalidPerPageException.php b/src/Exception/InvalidPerPageException.php new file mode 100644 index 00000000..c1049700 --- /dev/null +++ b/src/Exception/InvalidPerPageException.php @@ -0,0 +1,12 @@ +perPage < 1) { + throw new InvalidPerPageException((string) $this->perPage); + } + $this->rowMapper = $rowMapper; } diff --git a/tests/PagesTest.php b/tests/PagesTest.php index 883e8f71..20557548 100644 --- a/tests/PagesTest.php +++ b/tests/PagesTest.php @@ -7,6 +7,7 @@ use Aura\Sql\ExtendedPdoInterface; use PHPUnit\Framework\TestCase; use Ray\AuraSqlModule\Pagerfanta\AuraSqlPagerInterface; +use Ray\MediaQuery\Exception\InvalidPerPageException; final class PagesTest extends TestCase { @@ -26,4 +27,30 @@ public function testOffsetGetReturnsNullWhenDelegateHasNoPage(): void $this->assertFalse(isset($pages[3])); $this->assertNull($pages[3]); } + + public function testConstructorRejectsZeroPerPage(): void + { + $this->expectException(InvalidPerPageException::class); + + new Pages( + $this->createStub(AuraSqlPagerInterface::class), + $this->createStub(ExtendedPdoInterface::class), + 'SELECT 1', + [], + 0, + ); + } + + public function testConstructorRejectsNegativePerPage(): void + { + $this->expectException(InvalidPerPageException::class); + + new Pages( + $this->createStub(AuraSqlPagerInterface::class), + $this->createStub(ExtendedPdoInterface::class), + 'SELECT 1', + [], + -1, + ); + } } From 37d5dbb2c5cbb7698a29c29c3d01016acd2ee0c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 10:28:46 +0000 Subject: [PATCH 4/5] Align getNbPages() with Pagerfanta and memoize the count query - getNbPages() now returns a minimum of 1 for empty result sets, matching Pagerfanta's minimumNbPages() so both layers report the same page count (the stated goal of pairing this with Ray.AuraSqlModule#92). - Memoize nbResults in Pages so count() and getNbPages() share a single COUNT query, as Pagerfanta does. Without this each call re-ran the COUNT - or a full fetch for non-rewritable SQL. - Update FakeEmptyPages, the interface docblock, and add unit and integration coverage for both behaviors. --- src/Pages.php | 8 ++++++-- src/PagesInterface.php | 2 +- tests/Fake/FakeEmptyPages.php | 3 ++- tests/PagesTest.php | 35 +++++++++++++++++++++++++++++++++++ tests/SqlQueryTest.php | 8 ++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Pages.php b/src/Pages.php index d37526f5..8fccf2b3 100644 --- a/src/Pages.php +++ b/src/Pages.php @@ -14,6 +14,7 @@ use function ceil; use function is_array; +use function max; /** @template T of class-string|mixed */ final class Pages implements PagesInterface @@ -21,6 +22,9 @@ final class Pages implements PagesInterface /** @var (callable(array): mixed)|null */ private $rowMapper; + /** Memoized result count, shared by count() and getNbPages() as in Pagerfanta */ + private int|null $nbResults = null; + /** * @param array $params * @param (callable(array): mixed)|null $rowMapper @@ -100,12 +104,12 @@ public function offsetUnset(mixed $offset): void #[Override] public function count(): int { - return (new ExtendedPdoAdapter($this->pdo, $this->sql, $this->params))->getNbResults(); + return $this->nbResults ??= (new ExtendedPdoAdapter($this->pdo, $this->sql, $this->params))->getNbResults(); } #[Override] public function getNbPages(): int { - return (int) ceil($this->count() / $this->perPage); + return max(1, (int) ceil($this->count() / $this->perPage)); } } diff --git a/src/PagesInterface.php b/src/PagesInterface.php index c020c2ae..762f5b2e 100644 --- a/src/PagesInterface.php +++ b/src/PagesInterface.php @@ -11,7 +11,7 @@ interface PagesInterface extends ArrayAccess, Countable { /** - * Returns the total number of pages (ceil of result count / perPage). + * Returns the total number of pages (ceil of result count / perPage, minimum 1 as in Pagerfanta). */ public function getNbPages(): int; } diff --git a/tests/Fake/FakeEmptyPages.php b/tests/Fake/FakeEmptyPages.php index d374c3da..b8eca3ff 100644 --- a/tests/Fake/FakeEmptyPages.php +++ b/tests/Fake/FakeEmptyPages.php @@ -43,6 +43,7 @@ public function count(): int public function getNbPages(): int { - return 0; + // Empty result set is still one (empty) page, as in Pagerfanta + return 1; } } diff --git a/tests/PagesTest.php b/tests/PagesTest.php index 20557548..4e26094f 100644 --- a/tests/PagesTest.php +++ b/tests/PagesTest.php @@ -53,4 +53,39 @@ public function testConstructorRejectsNegativePerPage(): void -1, ); } + + public function testGetNbPagesReturnsOneWhenEmpty(): void + { + $pdo = $this->createStub(ExtendedPdoInterface::class); + $pdo->method('fetchValue')->willReturn('0'); + + $pages = new Pages( + $this->createStub(AuraSqlPagerInterface::class), + $pdo, + 'SELECT * FROM todo', + [], + 10, + ); + + $this->assertSame(0, $pages->count()); + $this->assertSame(1, $pages->getNbPages()); + } + + public function testCountQueryRunsOnlyOnce(): void + { + $pdo = $this->createMock(ExtendedPdoInterface::class); + $pdo->expects($this->once())->method('fetchValue')->willReturn('3'); + + $pages = new Pages( + $this->createStub(AuraSqlPagerInterface::class), + $pdo, + 'SELECT * FROM todo', + [], + 2, + ); + + $this->assertSame(3, $pages->count()); + $this->assertSame(2, $pages->getNbPages()); + $this->assertSame(2, $pages->getNbPages()); + } } diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 11ef1a3c..79d4baa1 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -132,6 +132,14 @@ public function testPagerNbPagesRoundsUp(): void $this->assertSame(2, $pages->getNbPages()); } + public function testPagerNbPagesMinimumOneWhenEmpty(): void + { + $pages = $this->sqlQuery->getPages('todo_item', ['id' => '__none__'], 10); + + $this->assertSame(0, count($pages)); + $this->assertSame(1, $pages->getNbPages()); + } + public function testCount(): void { $this->sqlQuery->exec('todo_add', ['id' => '2', 'title' => 'walk']); From 0ca907cf165d763aa0b52b9ae6817e3df264fc79 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 10:31:01 +0000 Subject: [PATCH 5/5] Restore int<0, max> narrowing on memoized count for PHPStan Countable::count() must return int<0, max>. getNbResults() is declared int<0, max> in Pagerfanta's AdapterInterface, but routing it through the int|null nbResults property widened it to int. Annotate the property with the range type to keep the narrowing. --- src/Pages.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Pages.php b/src/Pages.php index 8fccf2b3..f18cca72 100644 --- a/src/Pages.php +++ b/src/Pages.php @@ -22,7 +22,11 @@ final class Pages implements PagesInterface /** @var (callable(array): mixed)|null */ private $rowMapper; - /** Memoized result count, shared by count() and getNbPages() as in Pagerfanta */ + /** + * Memoized result count, shared by count() and getNbPages() as in Pagerfanta + * + * @var int<0, max>|null + */ private int|null $nbResults = null; /**