Merge pull request #524 from zendesk/RED-1955-iterator-page

PaginationIterator storing only one page at a time
zendesk · Nov 17, 2023 · d1f7c81 · d1f7c81
2 parents 2ce6e7f + ef32bb4
commit d1f7c81
Show file tree

Hide file tree

Showing 16 changed files with 60 additions and 25 deletions.
diff --git a/README.md b/README.md
@@ -155,7 +155,10 @@ foreach ($iterator as $ticket) {
   * Refer to the docs for details, including allowed sort fields
 * Combine everything: `$params = ['page[size]' => 2, 'sort' => 'updated_at', 'extra' => 'param'];`
 
-**Note**: Refer to the documentation for the correct params for sorting with the pagination type you're using.
+**Note**:
+
+* Refer to the documentation for the correct params for sorting with the pagination type you're using
+* The helper method `iterator_to_array` doesn't work with this implementation
 
 ##### Iterator API call response
 

diff --git a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
@@ -53,5 +53,5 @@ protected function pageSize()
     }
 
     abstract public function page($getPageFn);
-    abstract public function shouldGetPage($position);
+    abstract public function shouldGetPage($current_page);
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
@@ -29,7 +29,7 @@ public function page($getPageFn)
         return $this->latestResponse->{$this->resourcesKey};
     }
 
-    public function shouldGetPage($position) {
+    public function shouldGetPage($current_page) {
         return !$this->started || $this->hasMore;
     }
 

diff --git a/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php
@@ -18,7 +18,7 @@ public function page($getPageFn)
         return $response->{$this->resourcesKey};
     }
 
-    public function shouldGetPage($position) {
-        return $this->pageNumber == 0 || $position >= $this->pageNumber * $this->pageSize();
+    public function shouldGetPage($current_page) {
+        return $this->pageNumber == 0 || count($current_page) == 0;
     }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
@@ -14,7 +14,7 @@ class PaginationIterator implements Iterator
     private $strategy;
     private $method;
     private $position = 0;
-    private $items = [];
+    private $page = [];
 
     /**
      * @param mixed using trait FindAll. The resources collection, Eg: `$client->tickets()` which uses FindAll
@@ -56,8 +56,8 @@ public function valid()
     #[\ReturnTypeWillChange]
     public function current()
     {
-        if (isset($this->items[$this->position])) {
-            return $this->items[$this->position];
+        if (isset($this->page[$this->position])) {
+            return $this->page[$this->position];
         } else {
             return null;
         }
@@ -74,14 +74,14 @@ public function latestResponse()
     }
     private function getPageIfNeeded()
     {
-        if (isset($this->items[$this->position]) || !$this->strategy->shouldGetPage($this->position)) {
+        if (isset($this->page[$this->position]) || !$this->strategy->shouldGetPage($this->page)) {
             return;
         }
 
         $getPageFn = function () {
             return $this->clientList->{$this->method}($this->strategy->params());
         };
-
-        $this->items = array_merge($this->items, $this->strategy->page($getPageFn));
+        $this->page = $this->strategy->page($getPageFn);
+        $this->position = 0;
     }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php
@@ -19,7 +19,7 @@ public function page($getPageFn)
         return $response->{$this->resourcesKey};
     }
 
-    public function shouldGetPage($position) {
+    public function shouldGetPage($current_page) {
         return !$this->started;
     }
 }
diff --git a/tests/Zendesk/API/UnitTests/BasicTest.php b/tests/Zendesk/API/UnitTests/BasicTest.php
@@ -236,4 +236,20 @@ protected function assertEndpointCalled($userFunction, $endpoint, $method = 'GET
             ])
         );
     }
+
+    /**
+     * replacement for iterator_to_array
+     * which doesn't work when storing with single page
+     *
+     * @param \Iterator $iterator
+     * @return array all the items
+     */
+    protected function iterator_to_array($iterator)
+    {
+        $results = [];
+        foreach ($iterator as $item) {
+            $results[] = $item;
+        }
+        return $results;
+    }
 }
diff --git a/tests/Zendesk/API/UnitTests/Core/AutomationsTest.php b/tests/Zendesk/API/UnitTests/Core/AutomationsTest.php
@@ -40,7 +40,7 @@ public function testIterator()
 
         $iterator = $this->client->automations()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);
@@ -65,7 +65,7 @@ public function testIteratorFindActive()
 
         $iterator = $this->client->automations()->iterator([], 'findActive');
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
 
         $this->assertLastRequestIs(
             [

diff --git a/tests/Zendesk/API/UnitTests/Core/MacrosTest.php b/tests/Zendesk/API/UnitTests/Core/MacrosTest.php
@@ -41,7 +41,7 @@ public function testIterator()
 
         $iterator = $this->client->macros()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/OrganizationMembershipsTest.php b/tests/Zendesk/API/UnitTests/Core/OrganizationMembershipsTest.php
@@ -40,7 +40,7 @@ public function testIterator()
 
         $iterator = $this->client->organizationMemberships()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/OrganizationsTest.php b/tests/Zendesk/API/UnitTests/Core/OrganizationsTest.php
@@ -40,7 +40,7 @@ public function testIterator()
 
         $iterator = $this->client->organizations()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/RequestsTest.php b/tests/Zendesk/API/UnitTests/Core/RequestsTest.php
@@ -40,7 +40,7 @@ public function testIterator()
 
         $iterator = $this->client->requests()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/SatisfactionRatingsTest.php b/tests/Zendesk/API/UnitTests/Core/SatisfactionRatingsTest.php
@@ -40,7 +40,7 @@ public function testIterator()
 
         $iterator = $this->client->satisfactionRatings()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
         $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/TagsTest.php b/tests/Zendesk/API/UnitTests/Core/TagsTest.php
@@ -39,7 +39,7 @@ public function testIterator()
         ]);
         $iterator = $this->client->tags()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
 
         $this->assertCount(3, $actual);
         $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);

diff --git a/tests/Zendesk/API/UnitTests/Core/TicketsTest.php b/tests/Zendesk/API/UnitTests/Core/TicketsTest.php
@@ -53,7 +53,7 @@ public function testIteratorToArray()
 
         $iterator = $this->client->tickets()->iterator();
 
-        $actual = iterator_to_array($iterator);
+        $actual = $this->iterator_to_array($iterator);
         $this->assertCount(2, $actual);
         $this->assertEquals($this->testTicket['subject'], $actual[0]->subject);
         $this->assertEquals($this->testTicket2['subject'], $actual[1]->subject);

diff --git a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationIteratorTest.php b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationIteratorTest.php
@@ -82,6 +82,22 @@ public function testFetchesTickets()
         $strategy = new CbpStrategy('tickets', ['page[size]' => 2]);
         $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
+        $tickets = $this->iterator_to_array($iterator);
+
+        $this->assertEquals([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]], $tickets);
+        $this->assertEquals($mockTickets->response, $iterator->latestResponse());
+    }
+
+    public function testFetchesTicketsIteratorToArray()
+    {
+        $this->markTestSkipped("Doesn't work unless you store all pages in the iterator");
+        $mockTickets = new MockResource('tickets', [
+            [['id' => 1], ['id' => 2]],
+            [['id' => 3], ['id' => 4]]
+        ]);
+        $strategy = new CbpStrategy('tickets', ['page[size]' => 2]);
+        $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
+
         $tickets = iterator_to_array($iterator);
 
         $this->assertEquals([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]], $tickets);
@@ -97,7 +113,7 @@ public function testFetchesUsers()
         $strategy = new CbpStrategy('users', ['page[size]' => 2]);
         $iterator = new PaginationIterator($mockUsers, $strategy, 'findAll');
 
-        $users = iterator_to_array($iterator);
+        $users = $this->iterator_to_array($iterator);
 
         $this->assertEquals([
             ['id' => 1, 'name' => 'User 1'],
@@ -116,7 +132,7 @@ public function testFetchesCbpWithParams()
         $strategy = new CbpStrategy('tickets', ['page[size]' => 2, 'any' => 'param']);
         $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
-        $tickets = iterator_to_array($iterator);
+        $tickets = $this->iterator_to_array($iterator);
 
         $this->assertEquals([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]], $tickets);
         $this->assertEquals([
@@ -135,7 +151,7 @@ public function testFetchesSinglePageWithParams()
         $strategy = new SinglePageStrategy($resultsKey, $userParams);
         $iterator = new PaginationIterator($mockResults, $strategy, 'findAll');
 
-        $resources = iterator_to_array($iterator);
+        $resources = $this->iterator_to_array($iterator);
 
         $this->assertEquals([
             ['id' => 1, 'name' => 'Resource 1'],
@@ -153,7 +169,7 @@ public function testCustomMethod()
         $strategy = new SinglePageStrategy($resultsKey, $userParams);
         $iterator = new PaginationIterator($mockResults, $strategy, 'findDifferent');
 
-        $resources = iterator_to_array($iterator);
+        $resources = $this->iterator_to_array($iterator);
 
         $this->assertEquals([
             ['id' => 1, 'name' => 'Resource 1'],