From 4d73490d3cbc4d2672914575bfb1bce033ba7e89 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 23 Nov 2023 14:06:58 +0300 Subject: [PATCH 1/3] Remove null as possible type in PHPDoc since it causes an InvalidArgumentException --- src/Tasks/PageIterator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tasks/PageIterator.php b/src/Tasks/PageIterator.php index 409c525a..b3a8ff15 100644 --- a/src/Tasks/PageIterator.php +++ b/src/Tasks/PageIterator.php @@ -35,7 +35,7 @@ class PageIterator private ?array $requestOptions = []; /** - * @param Parsable|array|object|null $response paged collection response + * @param Parsable|array|object $response paged collection response * @param RequestAdapter $requestAdapter * @param array{class-string,string}|null $constructorCallable * The method to construct a paged response object. @@ -124,7 +124,7 @@ public function next(): ?PageResult } /** - * @param object|array|null $response + * @param object|array $response * @return PageResult|null * @throws JsonException */ From 0013cd5911a1a7aa4579f9fb834813170a399352 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 23 Nov 2023 14:32:49 +0300 Subject: [PATCH 2/3] Initialise hasNext to false if the initial collection is empty/null --- src/Tasks/PageIterator.php | 4 +-- tests/Tasks/PageIteratorTest.php | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/Tasks/PageIterator.php b/src/Tasks/PageIterator.php index b3a8ff15..8e225372 100644 --- a/src/Tasks/PageIterator.php +++ b/src/Tasks/PageIterator.php @@ -57,7 +57,7 @@ public function __construct($response, RequestAdapter $requestAdapter, ?array $c if ($page !== null) { $this->currentPage = $page; - $this->hasNext = true; + $this->hasNext = !(empty($page->getValue()) || is_null($page->getValue())); } } @@ -137,7 +137,7 @@ public static function convertToPage($response): ?PageResult $value = null; if (is_array($response)) { - $value = $response['value'] ?? ['value' => []]; + $value = $response['value'] ?? []; } elseif ($response instanceof Parsable && method_exists($response, 'getValue')) { $value = $response->getValue(); diff --git a/tests/Tasks/PageIteratorTest.php b/tests/Tasks/PageIteratorTest.php index ce1fefcd..495e6570 100644 --- a/tests/Tasks/PageIteratorTest.php +++ b/tests/Tasks/PageIteratorTest.php @@ -224,4 +224,47 @@ public function testCanGetFromParsablePageObject(): void $this->assertEquals(2, $count); $this->assertInstanceOf(User::class, $data[0]); } + + public function testHasNextIsFalseWithEmptyOrNullInitialCollection(): void + { + $pageIterator = new PageIterator([], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator([1, 2, 3], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator(['value' => []], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator(['value' => null], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator((object) [], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator((object) [1, 2], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $pageIterator = new PageIterator((object) [], $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + + $usersResponse = new UsersResponse(); + $usersResponse->setValue([]); + $pageIterator = new PageIterator($usersResponse, $this->mockRequestAdapter); + $this->assertFalse($pageIterator->hasNext()); + } + + public function testHasNextInitialisedToTrueWhenValueHasItems(): void + { + $pageIterator = new PageIterator(['value' => [1, 2, 3]], $this->mockRequestAdapter); + $this->assertTrue($pageIterator->hasNext()); + + $pageIterator = new PageIterator((object) ['value' => [1]], $this->mockRequestAdapter); + $this->assertTrue($pageIterator->hasNext()); + + $usersResponse = new UsersResponse(); + $usersResponse->setValue([1, 2]); + $pageIterator = new PageIterator($usersResponse, $this->mockRequestAdapter); + $this->assertTrue($pageIterator->hasNext()); + } } From abed5baa257063b2ec534b67b0acff245839d4ad Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 23 Nov 2023 16:49:05 +0300 Subject: [PATCH 3/3] Fix PHPStan failures --- src/Tasks/PageIterator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tasks/PageIterator.php b/src/Tasks/PageIterator.php index 8e225372..73070003 100644 --- a/src/Tasks/PageIterator.php +++ b/src/Tasks/PageIterator.php @@ -57,7 +57,7 @@ public function __construct($response, RequestAdapter $requestAdapter, ?array $c if ($page !== null) { $this->currentPage = $page; - $this->hasNext = !(empty($page->getValue()) || is_null($page->getValue())); + $this->hasNext = !empty($page->getValue()); } } @@ -124,7 +124,7 @@ public function next(): ?PageResult } /** - * @param object|array $response + * @param object|array|Parsable|null $response * @return PageResult|null * @throws JsonException */