Merge pull request #153 from microsoftgraph/fix/page-iterator

Initialise `hasNext()` to false if the initial collection is empty/null

Author: Ndiritu

src/Tasks/PageIterator.php

@@ -35,7 +35,7 @@ class PageIterator
     private ?array $requestOptions = [];
 
     /**
-     * @param Parsable|array<mixed>|object|null $response paged collection response
+     * @param Parsable|array<mixed>|object $response paged collection response
      * @param RequestAdapter $requestAdapter
      * @param array{class-string<T>,string}|null $constructorCallable
      * The method to construct a paged response object.
@@ -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());
         }
     }
 
@@ -124,7 +124,7 @@ public function next(): ?PageResult
     }
 
     /**
-     * @param object|array<mixed>|null $response
+     * @param object|array<mixed>|Parsable|null $response
      * @return PageResult|null
      * @throws JsonException
      */
@@ -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();

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());
+    }
 }