Iterator final touches

CBP_UPGRADE_GUIDE.md

@@ -1,5 +1,7 @@
 # OBP to CBP Upgrade guide
 
+TODO version requirement, how to bump.
+
 ## Useful links
 
 * [This README](./README.md#pagination)
@@ -16,7 +18,7 @@ CBP works based on cursors, so when ordering by `id` with page size 10, if the l
 
 ## API calls
 
-URL example:
+Note the query parameters change in these two URL examples:
 
 * OBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort_order=desc&sort_by=updated_at&per_page=2
 * CBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort=-updated_at&page[size]=2
@@ -28,11 +30,6 @@ When moving from OBP to CBP sorting params change as well:
 * From: `?sort_name=updated_at&sort_order=desc`
 * To: `sort=-updated_at`
 
-Ticket example:
-
-* OBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort_order=desc&sort_by=updated_at&per_page=2
-* CBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort=-updated_at&page[size]=2
-
 However, the list of **attributes you can sort by might also change** with the pagination type:
 
 https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting
@@ -56,7 +53,9 @@ If this is your situation, **you will need to change the sorting order** to a su
 
 ## The new iterator
 
-Please refer to the [README](./README.md#iterator-recommended).
+The best way to upgrade your app to support CBP is to use the iterator we provide. Please refer to the [README](./README.md#iterator-recommended).
+
+**Note**: The iterator will automatically convert these parameters to work with CBP: `page_size, sort_by, sort_order`.
 
 ## Parallel requests
 

README.md

@@ -137,7 +137,7 @@ foreach ($iterator as $ticket) {
 }
 ```
 
-If you want a specific sort order, please refer to the sorting section in the documentation ([Tickets, for example](https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting)). If you are upgrading from OBP, please refer to [the upgrade guide](./UPGRADE_GUIDE.md).
+If you want a specific sort order, please refer to the sorting section in the documentation ([Tickets, for example](https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting)).
 
 ##### Iterator with params example
 
@@ -155,6 +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'];`
 
+##### Iterator API call response
+
+The latest response is exposed in the iterator at `$iterator->latestResponse()`. This could come handy for debugging.
+
 ##### Custom iterators
 
 If you want to use the iterator for custom methods, as opposed to the default `findAll()`, you can create an iterator for your collection:
@@ -176,6 +180,21 @@ This can be useful for filter endpoints like [active automations](https://develo
 $iterator = $client->automations()->iterator($params, 'findActive');
 ```
 
+Which is analogous to:
+
+```php
+use Zendesk\API\Traits\Utility\Pagination\PaginationIterator;
+use Zendesk\API\Traits\Utility\Pagination\CbpStrategy;
+$strategy = new CbpStrategy('automations', $params);
+$iterator = new PaginationIterator(
+    $client->automations(),
+    $strategy,
+    'findActive'
+);
+```
+
+See how the [Pagination Trait](src/Zendesk/API/Traits/Resource/Pagination.php) is used if you need more custom implementations.
+
 ##### Catching API errors
 
 This doesn't change too much:
@@ -187,6 +206,7 @@ try {
     }
 } catch (ApiResponseException $e) {
     $errorMessage = $e->getMessage();
+    $errorDetails = $e=>getErrorDetails();
 }
 ```
 

docker-compose.yml

@@ -8,9 +8,11 @@ services:
       - .:/app
       - vendor:/app/vendor
     command: vendor/bin/phpunit --testsuite "Zendesk API Unit Test Suites"
+    # command: vendor/bin/phpunit --testdox --testsuite "Zendesk API Unit Test Suites"
     # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Traits/Utility/PaginationIteratorTest.php
     # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Core/TicketsTest.php
-    # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Core/OrganizationMembershipsTest.php
+    # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/HttpTest.php
+    # command: vendor/bin/phpunit --testsuite "Zendesk API Live Test Suites"
 
 volumes:
   vendor:

src/Zendesk/API/Traits/Resource/Pagination.php

@@ -15,12 +15,12 @@ trait Pagination {
      *
      * @return PaginationIterator to fetch all pages.
      */
-    public function iterator($params = [])
+    public function iterator($params = [], $method = 'findAll')
     {
         $strategyClass = $this->paginationStrategyClass();
         $strategy = new $strategyClass($this->resourcesKey(), $params);
 
-        return new PaginationIterator($this, $strategy);
+        return new PaginationIterator($this, $strategy, $method);
     }
 
     /**
@@ -33,6 +33,8 @@ protected function paginationStrategyClass() {
     }
 
     /**
+     * The key in the API responses where the resources are returned
+     *
      * @return string eg: "job_statuses"
      */
     protected function resourcesKey() {

src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php

@@ -9,6 +9,7 @@ abstract class AbstractStrategy
     protected $resourcesKey;
     protected $params;
     protected $pageSize;
+    protected $latestResponse;
 
     public function __construct($resourcesKey, $params)
     {
@@ -21,6 +22,21 @@ public function params()
         return $this->params;
     }
 
+    /**
+     * Returns the latest HTTP response, unless an error occurred, which causes an exception
+     *
+     * @return \GuzzleHttp\Psr7\Response
+     */
+    public function latestResponse()
+    {
+        return $this->latestResponse;
+    }
+
+    /**
+     * From the params or the default value
+     *
+     * @return integer
+     */
     protected function pageSize()
     {
         if (isset($this->pageSize)) {

src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php

@@ -4,24 +4,33 @@
 
 /**
  * Cursor Based Pagination
- * Used in paginationStrategyClass
+ * Used in PaginationIterator
  */
 class CbpStrategy extends AbstractStrategy
 {
-    private $afterCursor = null;
+    private $afterCursor;
+    private $hasMore;
     private $started = false;
 
     public function page($getPageFn)
     {
         $this->started = true;
-        $response = $getPageFn();
-        $this->afterCursor = $response->meta->has_more ? $response->meta->after_cursor : null;
+        $this->latestResponse = $getPageFn();
+        if (!isset($this->latestResponse->meta->has_more)) {
+            throw new PaginationError(
+                "Response not conforming to the CBP format, if you think your request is correct, please open an issue at https://github.com/zendesk/zendesk_api_client_php/issues"
+            );
+        }
+        $this->hasMore = $this->latestResponse->meta->has_more;
+        if (isset($this->latestResponse->meta->after_cursor)) {
+            $this->afterCursor = $this->latestResponse->meta->after_cursor;
+        }
 
-        return $response->{$this->resourcesKey};
+        return $this->latestResponse->{$this->resourcesKey};
     }
 
     public function shouldGetPage($position) {
-        return !$this->started || $this->afterCursor;
+        return !$this->started || $this->hasMore;
     }
 
     public function params()
@@ -31,14 +40,15 @@ public function params()
 
         return $result;
     }
+
     /**
      * The params that are needed to ordering in CBP (eg: ["sort" => "-age"])
      * If OBP params are passed, they are converted to CBP
      *
      * OBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort_order=desc&sort_by=updated_at&per_page=2
      * CBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort=-updated_at&page[size]=2
      *
-     * @return array all params with CBP sorting order
+     * @return array Params with proper CBP sorting order
      */
     private function sortParams()
     {
@@ -53,7 +63,7 @@ private function sortParams()
      * The params that are needed to for pagination (eg: ["page[size]" => "100"])
      * If OBP params are passed, they are converted to CBP
      *
-     * @return array all params with CBP sorting order
+     * @return array Params for pagination
      */
     private function paginationParams()
     {

src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php

@@ -21,9 +21,4 @@ public function page($getPageFn)
     public function shouldGetPage($position) {
         return $this->pageNumber == 0 || $position >= $this->pageNumber * $this->pageSize();
     }
-    public function paramsWithPagination()
-    {
-        return ['page' => $this->pageNumber, 'per_page' => $this->pageSize()];
-
-    }
 }

src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php

@@ -2,69 +2,86 @@
 
 namespace Zendesk\API\Traits\Utility\Pagination;
 
+class PaginationError extends \Exception {}
+
 const DEFAULT_PAGE_SIZE = 100;
 
 use Iterator;
 
 class PaginationIterator implements Iterator
 {
-    /**
-     * @var mixed using trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
-     * Eg: `$client->tickets()` which uses FindAll
-     */
     private $clientList;
     private $strategy;
     private $method;
     private $position = 0;
-    private $page = [];
+    private $items = [];
 
-    public function __construct($clientList, AbstractStrategy $strategy, $method = 'findAll')
+    /**
+     * @param mixed using trait FindAll. The resources collection, Eg: `$client->tickets()` which uses FindAll
+     * @param AbstractStrategy $strategy For pagination Logic (OBP, CBP, SinglePage)
+     * @param string $method used to make the API call
+     */
+    public function __construct($clientList, AbstractStrategy $strategy, $method)
     {
         $this->clientList = $clientList;
         $this->strategy = $strategy;
         $this->method = $method;
     }
 
+    #[\ReturnTypeWillChange]
     public function key()
     {
         return $this->position;
     }
 
+    #[\ReturnTypeWillChange]
     public function next()
     {
         ++$this->position;
     }
 
+    #[\ReturnTypeWillChange]
     public function rewind()
     {
         $this->position = 0;
     }
 
+    #[\ReturnTypeWillChange]
     public function valid()
     {
         $this->getPageIfNeeded();
         return !!$this->current();
     }
 
+    #[\ReturnTypeWillChange]
     public function current()
     {
-        if (isset($this->page[$this->position])) {
-            return $this->page[$this->position];
+        if (isset($this->items[$this->position])) {
+            return $this->items[$this->position];
         } else {
             return null;
         }
     }
 
+    /**
+     * Returns the latest HTTP response, unless an error occurred, which causes an exception
+     *
+     * @return \GuzzleHttp\Psr7\Response
+     */
+    public function latestResponse()
+    {
+        return $this->strategy->latestResponse();
+    }
     private function getPageIfNeeded()
     {
-        if (!$this->strategy->shouldGetPage($this->position)) {
+        if (isset($this->items[$this->position]) || !$this->strategy->shouldGetPage($this->position)) {
             return;
         }
 
         $getPageFn = function () {
             return $this->clientList->{$this->method}($this->strategy->params());
         };
 
-        $this->page = array_merge($this->page, $this->strategy->page($getPageFn));
+        $this->items = array_merge($this->items, $this->strategy->page($getPageFn));
     }
 }

tests/Zendesk/API/UnitTests/BasicTest.php

@@ -76,6 +76,7 @@ public function __construct($name = null, array $data = [], $dataName = '')
      */
     protected function setUp()
     {
+        parent::setUp();
         $this->client = new HttpClient($this->subdomain, $this->username, $this->scheme, $this->hostname, $this->port);
         $this->client->setAuth('oauth', ['token' => $this->oAuthToken]);
     }

tests/Zendesk/API/UnitTests/Core/AuthTest.php

@@ -2,11 +2,7 @@
 
 namespace Zendesk\API\UnitTests\Core;
 
-use GuzzleHttp\Client;
 use GuzzleHttp\Psr7\Request;
-use GuzzleHttp\Psr7\Response;
-use Zendesk\API\Http;
-use Zendesk\API\HttpClient;
 use Zendesk\API\UnitTests\BasicTest;
 use Zendesk\API\Utilities\Auth;
 
@@ -15,35 +11,6 @@
  */
 class AuthTest extends BasicTest
 {
-    /**
-     * Test if request is still sent even without authentication
-     */
-    public function testAnonymousAccess()
-    {
-        $this->markTestSkipped('Broken in PHP 7.4 (mocking)');
-
-        // mock client
-        $client = $this
-            ->getMockBuilder(HttpClient::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $client->method('getHeaders')->willReturn([]);
-        $client->expects(self::once())->method('getAuth')->willReturn(null);
-
-        // prepareRequest should not be called
-        $auth = $this
-            ->getMockBuilder(Auth::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $auth->expects(self::never())->method('prepareRequest');
-        $client->expects(self::once())->method('getAuth')->willReturn($auth);
-
-        // send request
-        $client->guzzle = $this->getMockBuilder(Client::class)->getMock();
-        $client->guzzle->method('send')->willReturn(new Response(200, [], ''));
-        Http::send($client, '');
-    }
-
     /**
      * Test the preparing of a request for basic authentication.
      */