From b978d745eead20cf7b523e4c90c4f938e3ec1502 Mon Sep 17 00:00:00 2001 From: "Erik Trapin (ecoologic)" Date: Fri, 17 Nov 2023 13:25:28 +1000 Subject: [PATCH 1/2] PaginationIterator storing only one page at a time --- README.md | 5 +++- .../Utility/Pagination/PaginationIterator.php | 4 ++-- tests/Zendesk/API/UnitTests/BasicTest.php | 16 +++++++++++++ .../API/UnitTests/Core/AutomationsTest.php | 4 ++-- .../Zendesk/API/UnitTests/Core/MacrosTest.php | 2 +- .../Core/OrganizationMembershipsTest.php | 2 +- .../API/UnitTests/Core/OrganizationsTest.php | 2 +- .../API/UnitTests/Core/RequestsTest.php | 2 +- .../Core/SatisfactionRatingsTest.php | 2 +- tests/Zendesk/API/UnitTests/Core/TagsTest.php | 2 +- .../API/UnitTests/Core/TicketsTest.php | 2 +- .../Traits/Utility/PaginationIteratorTest.php | 24 +++++++++++++++---- 12 files changed, 51 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index b09caf4f..9ec885de 100755 --- 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/PaginationIterator.php b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php index 29366eb8..a0a61200 100644 --- a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php +++ b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php @@ -81,7 +81,7 @@ private function getPageIfNeeded() $getPageFn = function () { return $this->clientList->{$this->method}($this->strategy->params()); }; - - $this->items = array_merge($this->items, $this->strategy->page($getPageFn)); + $this->items = $this->strategy->page($getPageFn); + $this->position = 0; } } diff --git a/tests/Zendesk/API/UnitTests/BasicTest.php b/tests/Zendesk/API/UnitTests/BasicTest.php index 2854feed..a9e472a3 100644 --- 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 index baa7fd21..aad8bd5a 100755 --- 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 index da581824..510e1880 100755 --- 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 index 8b116302..4e70cd53 100644 --- 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 index c0c1051b..74ff41e7 100644 --- 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 index dd321f25..e3f0e288 100755 --- 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 index 339c24e5..b5d37e88 100755 --- 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 index d8b56a7d..1b471f8f 100644 --- 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 index a4882466..26e947c1 100755 --- 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 index 1e9fa7b4..c6c5a52c 100644 --- 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'], From ef32bb421d08eae54b67f0289805e03c37b95848 Mon Sep 17 00:00:00 2001 From: "Erik Trapin (ecoologic)" Date: Fri, 17 Nov 2023 13:31:58 +1000 Subject: [PATCH 2/2] Pagination/AbstractStrategy shouldGetPage($current_page) --- .../API/Traits/Utility/Pagination/AbstractStrategy.php | 2 +- .../API/Traits/Utility/Pagination/CbpStrategy.php | 2 +- .../API/Traits/Utility/Pagination/ObpStrategy.php | 4 ++-- .../Traits/Utility/Pagination/PaginationIterator.php | 10 +++++----- .../Traits/Utility/Pagination/SinglePageStrategy.php | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php index f0a954f3..5838505c 100644 --- 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 index f392fbe3..6a7874fe 100644 --- 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 index c32b4cd5..966355b2 100644 --- 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 index a0a61200..15e11e02 100644 --- 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 = $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 index 819fade4..6e1e9286 100644 --- 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; } }