From fb5834365bda1793bd7c1517607c3d2479e98a57 Mon Sep 17 00:00:00 2001 From: Jonathan Martins Date: Fri, 13 Jun 2025 17:16:19 -0300 Subject: [PATCH 1/4] sometimes the API error returns a description instead of a message --- src/RequestTransport.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RequestTransport.php b/src/RequestTransport.php index ed4b440..2b2bdde 100644 --- a/src/RequestTransport.php +++ b/src/RequestTransport.php @@ -139,7 +139,7 @@ private function hydrateResponse(ResponseInterface $response): array $error = $contents["error"]; if (is_array($error)) { - $error = $error["message"]; + $error = $error["message"] ?? $error["description"]; } throw new ApiErrorException($error); From 9222c936a10a4b1028da1cff46eee098e6b3b4a6 Mon Sep 17 00:00:00 2001 From: Jonathan Martins Date: Thu, 18 Sep 2025 17:12:50 -0300 Subject: [PATCH 2/4] Last Page Bug Fix + Tests --- src/Paginator.php | 2 +- tests/PaginatorTest.php | 276 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 275 insertions(+), 3 deletions(-) diff --git a/src/Paginator.php b/src/Paginator.php index be93712..0b876b8 100644 --- a/src/Paginator.php +++ b/src/Paginator.php @@ -180,7 +180,7 @@ public function valid(): bool return true; } - return $this->getPagination()["hasNextPage"]; + return $this->skip < $this->getPagination()["totalCount"]; } /** diff --git a/tests/PaginatorTest.php b/tests/PaginatorTest.php index 3596c11..3c777c8 100644 --- a/tests/PaginatorTest.php +++ b/tests/PaginatorTest.php @@ -87,9 +87,24 @@ public function testCurrent(): void public function testValid(): void { - $paginator = $this->makePaginator(["pageInfo" => ["hasNextPage" => false]]); + $paginator = $this->makePaginator(); + $this->assertTrue($paginator->valid(), 'Should be valid when lastResult is null'); + + $singlePageResult = [ + "pageInfo" => [ + "skip" => 0, + "limit" => 30, + "totalCount" => 7, + "hasNextPage" => false, + "hasPreviousPage" => false + ] + ]; + $paginator = $this->makePaginator($singlePageResult); + $this->assertTrue($paginator->valid(), 'Should be valid for single page with data'); - $this->assertFalse($paginator->valid()); + $paginator = $this->makePaginator($singlePageResult); + $paginator->skip(10); + $this->assertFalse($paginator->valid(), 'Should be invalid when skip > totalCount'); } public function testGetTotalResourcesCount(): void @@ -99,6 +114,263 @@ public function testGetTotalResourcesCount(): void $this->assertSame(10, $paginator->getTotalResourcesCount()); } + public function testSinglePageIteration(): void + { + $singlePageResponse = [ + 'transactions' => [ + ['id' => 1, 'amount' => 100], + ['id' => 2, 'amount' => 200], + ['id' => 3, 'amount' => 300], + ['id' => 4, 'amount' => 400], + ['id' => 5, 'amount' => 500], + ['id' => 6, 'amount' => 600], + ['id' => 7, 'amount' => 700], + ], + 'pageInfo' => [ + 'skip' => 0, + 'limit' => 30, + 'totalCount' => 7, + 'hasPreviousPage' => false, + 'hasNextPage' => false, + ] + ]; + + $listRequestMock = $this->createMock(Request::class); + $listRequestMock->expects($this->once()) + ->method("pagination") + ->willReturnSelf(); + + $requestTransportMock = $this->createMock(RequestTransport::class); + $requestTransportMock->expects($this->once()) + ->method('transport') + ->with($listRequestMock) + ->willReturn($singlePageResponse); + + $paginator = new Paginator($requestTransportMock, $listRequestMock); + + $iterations = 0; + $processedTransactions = []; + + foreach ($paginator as $result) + { + $iterations++; + $this->assertArrayHasKey('transactions', $result); + $this->assertArrayHasKey('pageInfo', $result); + $this->assertCount(7, $result['transactions']); + + foreach ($result['transactions'] as $transaction) + { + $processedTransactions[] = $transaction; + } + } + + $this->assertSame(1, $iterations, 'Should iterate exactly once for single page'); + $this->assertCount(7, $processedTransactions, 'Should process all 7 transactions'); + $this->assertSame(100, $processedTransactions[0]['amount'], 'First transaction should be accessible'); + $this->assertSame(700, $processedTransactions[6]['amount'], 'Last transaction should be accessible'); + } + + public function testMultiPageIncludesLastPage(): void + { + $responses = [ + [ + 'charges' => [ + ['id' => 1, 'amount' => 100], + ['id' => 2, 'amount' => 200], + ], + 'pageInfo' => [ + 'skip' => 0, + 'limit' => 2, + 'totalCount' => 5, + 'hasPreviousPage' => false, + 'hasNextPage' => true + ] + ], + [ + 'charges' => [ + ['id' => 3, 'amount' => 300], + ['id' => 4, 'amount' => 400], + ], + 'pageInfo' => [ + 'skip' => 2, + 'limit' => 2, + 'totalCount' => 5, + 'hasPreviousPage' => true, + 'hasNextPage' => true + ] + ], + [ + 'charges' => [ + ['id' => 5, 'amount' => 500], + ], + 'pageInfo' => [ + 'skip' => 4, + 'limit' => 2, + 'totalCount' => 5, + 'hasPreviousPage' => true, + 'hasNextPage' => false // Last page + ] + ] + ]; + + $listRequestMock = $this->createMock(Request::class); + $listRequestMock->expects($this->exactly(3)) + ->method("pagination") + ->willReturnSelf(); + + $requestTransportMock = $this->createMock(RequestTransport::class); + $requestTransportMock->expects($this->exactly(3)) + ->method('transport') + ->with($listRequestMock) + ->willReturnOnConsecutiveCalls(...$responses); + + $paginator = new Paginator($requestTransportMock, $listRequestMock); + $paginator->perPage(2); // Set page size to 2 to create 3 pages + + $iterations = 0; + $allCharges = []; + + foreach ($paginator as $result) + { + $iterations++; + $this->assertArrayHasKey('charges', $result); + + foreach ($result['charges'] as $charge) + { + $allCharges[] = $charge; + } + } + + $this->assertSame(3, $iterations, 'Should iterate through all 3 pages'); + $this->assertCount(5, $allCharges, 'Should process all 5 charges including last page'); + + $lastCharge = end($allCharges); + $this->assertSame(5, $lastCharge['id'], 'Should include charge from last page'); + $this->assertSame(500, $lastCharge['amount'], 'Last page data should be accessible'); + } + + public function testEmptyResultsIteration(): void + { + $emptyResponse = [ + 'items' => [], + 'pageInfo' => [ + 'skip' => 0, + 'limit' => 30, + 'totalCount' => 0, + 'hasPreviousPage' => false, + 'hasNextPage' => false + ] + ]; + + $listRequestMock = $this->createMock(Request::class); + $listRequestMock->expects($this->once()) + ->method("pagination") + ->willReturnSelf(); + + $requestTransportMock = $this->createMock(RequestTransport::class); + $requestTransportMock->expects($this->once()) + ->method('transport') + ->with($listRequestMock) + ->willReturn($emptyResponse); + + $paginator = new Paginator($requestTransportMock, $listRequestMock); + + $iterations = 0; + foreach ($paginator as $result) + { + $iterations++; + $this->assertArrayHasKey('items', $result); + $this->assertEmpty($result['items']); + } + + $this->assertSame(1, $iterations, 'Should iterate once even for empty results'); + } + + public function testOriginalBugReportScenario(): void + { + $getTransactionsResponse = [ + 'transactions' => [ + ['id' => 't1', 'amount' => 1000, 'date' => '2024-01-01'], + ['id' => 't2', 'amount' => 2000, 'date' => '2024-01-02'], + ['id' => 't3', 'amount' => 3000, 'date' => '2024-01-03'], + ['id' => 't4', 'amount' => 4000, 'date' => '2024-01-04'], + ['id' => 't5', 'amount' => 5000, 'date' => '2024-01-05'], + ['id' => 't6', 'amount' => 6000, 'date' => '2024-01-06'], + ['id' => 't7', 'amount' => 7000, 'date' => '2024-01-07'], + ], + 'pageInfo' => [ + 'skip' => 0, + 'limit' => 30, + 'totalCount' => 7, + 'hasPreviousPage' => false, + 'hasNextPage' => false + ] + ]; + + $listRequestMock = $this->createMock(Request::class); + $listRequestMock->expects($this->once()) + ->method("pagination") + ->willReturnSelf(); + + $requestTransportMock = $this->createMock(RequestTransport::class); + $requestTransportMock->expects($this->once()) + ->method('transport') + ->with($listRequestMock) + ->willReturn($getTransactionsResponse); + + $paginator = new Paginator($requestTransportMock, $listRequestMock); + + $outerLoopExecuted = false; + $transactionIds = []; + + foreach ($paginator as $result) + { + $outerLoopExecuted = true; + + foreach ($result['transactions'] as $transaction) + { + $transactionIds[] = $transaction['id']; + } + } + + $this->assertTrue($outerLoopExecuted, 'Outer foreach loop must execute'); + $this->assertCount(7, $transactionIds, 'Should access all 7 transactions'); + $this->assertSame(['t1', 't2', 't3', 't4', 't5', 't6', 't7'], $transactionIds); + } + + public function testValidWithDifferentSkipPositions(): void + { + $result = [ + "pageInfo" => [ + "skip" => 0, + "limit" => 10, + "totalCount" => 25, + "hasNextPage" => true, + "hasPreviousPage" => false + ] + ]; + + $paginator = $this->makePaginator($result); + $paginator->skip(0); + $this->assertTrue($paginator->valid(), 'Should be valid at position 0'); + + $paginator = $this->makePaginator($result); + $paginator->skip(15); + $this->assertTrue($paginator->valid(), 'Should be valid when skip < totalCount'); + + $paginator = $this->makePaginator($result); + $paginator->skip(24); + $this->assertTrue($paginator->valid(), 'Should be valid when skip = totalCount - 1'); + + $paginator = $this->makePaginator($result); + $paginator->skip(25); + $this->assertFalse($paginator->valid(), 'Should be invalid when skip >= totalCount'); + + $paginator = $this->makePaginator($result); + $paginator->skip(30); + $this->assertFalse($paginator->valid(), 'Should be invalid when skip > totalCount'); + } + private function testPaginatorNavigation(callable $navigate, int $expectedSkip = 0, int $skip = 0, int $perPage = 30): void { $paginator = $this->makePaginator(); From f9251c6ce7b7befd0c677727e9f4f3224a32d693 Mon Sep 17 00:00:00 2001 From: Jonathan Martins Date: Thu, 18 Sep 2025 17:35:09 -0300 Subject: [PATCH 3/4] make all phpstan stuff pass --- src/Paginator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Paginator.php b/src/Paginator.php index 0b876b8..fca1f5e 100644 --- a/src/Paginator.php +++ b/src/Paginator.php @@ -27,7 +27,7 @@ * $result = $paginator->current(); * ``` * - * @implements Iterator> + * @implements Iterator> * @phpstan-type Pagination array{skip: int, limit: int, totalCount: int, hasPreviousPage: bool, hasNextPage: bool} */ class Paginator implements Iterator @@ -79,6 +79,7 @@ class Paginator implements Iterator public function __construct( RequestTransport $requestTransport, Request $listRequest, + /** @phpstan-ignore-next-line */ array $lastResult = null ) { $this->requestTransport = $requestTransport; From c3930b70b69a8ef4fb758e1f97f3dcdc4a2e74f1 Mon Sep 17 00:00:00 2001 From: Jonathan Martins Date: Thu, 18 Sep 2025 17:38:02 -0300 Subject: [PATCH 4/4] removing @phpstan-ignore-next-line --- src/Paginator.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Paginator.php b/src/Paginator.php index fca1f5e..6456093 100644 --- a/src/Paginator.php +++ b/src/Paginator.php @@ -79,7 +79,6 @@ class Paginator implements Iterator public function __construct( RequestTransport $requestTransport, Request $listRequest, - /** @phpstan-ignore-next-line */ array $lastResult = null ) { $this->requestTransport = $requestTransport;