Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pagination Iterator with params #516

Merged
merged 2 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Zendesk/API/Resources/Core/SharingAgreements.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Zendesk\API\Resources\ResourceAbstract;
use Zendesk\API\Traits\Resource\FindAll;
use Zendesk\API\Traits\Utility\Pagination\ObpStrategy;
use Zendesk\API\Traits\Utility\Pagination\SinglePageStrategy;

/**
Expand Down
13 changes: 9 additions & 4 deletions src/Zendesk/API/Traits/Resource/Pagination.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Zendesk\API\Traits\Utility\Pagination\PaginationIterator;

trait Pagination {

/**
* Usage:
* foreach ($ticketsIterator as $ticket) {
Expand All @@ -19,15 +18,21 @@ trait Pagination {
public function iterator()
{
$strategyClass = $this->paginationStrategyClass();
$strategy = new $strategyClass($this, $this->resourcesKey(), AbstractStrategy::DEFAULT_PAGE_SIZE);
return new PaginationIterator($strategy);
$strategy = new $strategyClass($this->resourcesKey(), AbstractStrategy::DEFAULT_PAGE_SIZE);
return new PaginationIterator($this, $strategy);
Copy link
Collaborator Author

@ecoologic ecoologic Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of passing $this for the strategy to run findAll() duplicated in every strategy inside getPage(), extract it in the Iterator as a function for the strategy to run (see below).

}

/**
* Override this method in your resources
*
* @return string subclass of AbstractStrategy used for fetching pages
*/

protected function paginationStrategyClass() {
return CbpStrategy::class;
}

/*
/**
* @return string eg: "job_statuses"
*/
protected function resourcesKey() {
Expand Down
12 changes: 3 additions & 9 deletions src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,18 @@ abstract class AbstractStrategy
{
public const DEFAULT_PAGE_SIZE = 100;

/*
* @var mixed use trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
*/
protected $clientList;

/*
/**
* @var string The response key where the data is returned
*/
protected $resourcesKey;
protected $pageSize;

public function __construct($clientList, $resourcesKey, $pageSize = self::DEFAULT_PAGE_SIZE)
public function __construct($resourcesKey, $pageSize = self::DEFAULT_PAGE_SIZE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the Strategies are more cohesive.

{
$this->clientList = $clientList;
$this->resourcesKey = $resourcesKey;
$this->pageSize = $pageSize;
}

abstract public function getPage();
abstract public function getPage($pageFn);
abstract public function shouldGetPage($position);
}
8 changes: 6 additions & 2 deletions src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

namespace Zendesk\API\Traits\Utility\Pagination;

/**
* Cursor Based Pagination
* Used in paginationStrategyClass
*/
class CbpStrategy extends AbstractStrategy
{
private $afterCursor = null;
private $started = false;

public function getPage()
public function getPage($pageFn)
{
$this->started = true;
$params = ['page[size]' => $this->pageSize];
if ($this->afterCursor) {
$params['page[after]'] = $this->afterCursor;
}
$response = $this->clientList->findAll($params);
$response = $pageFn($params);

$this->afterCursor = $response->meta->has_more ? $response->meta->after_cursor : null;
return $response->{$this->resourcesKey};
Expand Down
6 changes: 3 additions & 3 deletions src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@

namespace Zendesk\API\Traits\Utility\Pagination;


/**
* Offset Based Pagination
* Used in paginationStrategyClass
*/
class ObpStrategy extends AbstractStrategy
{
private $pageNumber = 0;

public function getPage()
public function getPage($pageFn)
{
++$this->pageNumber;
$params = ['page' => $this->pageNumber, 'page_size' => $this->pageSize];
$response = $this->clientList->findAll($params);
$response = $pageFn($params);

return $response->{$this->resourcesKey};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ class PaginationIterator implements Iterator
private $position = 0;
private $page = [];
private $strategy;
/**
* @var mixed use trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
*/
private $clientList;

public function __construct(AbstractStrategy $strategy)
public function __construct($clientList, AbstractStrategy $strategy)
{
$this->clientList = $clientList;
$this->strategy = $strategy;
}

Expand All @@ -32,7 +37,7 @@ public function rewind()

public function valid()
{
$this->getPageIfNecessary();
$this->getPageIfNeeded();
return !!$this->current();
}

Expand All @@ -45,12 +50,16 @@ public function current()
}
}

private function getPageIfNecessary()
private function getPageIfNeeded()
{
if (!$this->strategy->shouldGetPage($this->position)) {
return;
}

$this->page = array_merge($this->page, $this->strategy->getPage());
$pageFn = function ($params = []) {
return $this->clientList->findAll($params);
};
Copy link
Collaborator Author

@ecoologic ecoologic Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more DRY, so adding params and later maybe making the method dynamic (findAll, find, active, etc) in one place only instead of each Strategy.


$this->page = array_merge($this->page, $this->strategy->getPage($pageFn));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@

/**
* Single Page (no pagination)
* Used in paginationStrategyClass
*/
class SinglePageStrategy extends AbstractStrategy
{
protected $started = false;

public function getPage()
public function getPage($pageFn)
{
$this->started = true;
$response = $this->clientList->findAll();
$response = $pageFn();

return $response->{$this->resourcesKey};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public function testFetchesTickets()
[['id' => 1], ['id' => 2]],
[['id' => 3], ['id' => 4]]
]);
$strategy = new CbpStrategy($mockTickets, 'tickets', 2);
$iterator = new PaginationIterator($strategy);
$strategy = new CbpStrategy('tickets', 2);
$iterator = new PaginationIterator($mockTickets, $strategy);

$tickets = iterator_to_array($iterator);

Expand All @@ -62,8 +62,8 @@ public function testFetchesUsers()
[['id' => 1, 'name' => 'User 1'], ['id' => 2, 'name' => 'User 2']],
[['id' => 3, 'name' => 'User 3'], ['id' => 4, 'name' => 'User 4']]
]);
$strategy = new CbpStrategy($mockUsers, 'users', 2);
$iterator = new PaginationIterator($strategy);
$strategy = new CbpStrategy('users', 2);
$iterator = new PaginationIterator($mockUsers, $strategy);

$users = iterator_to_array($iterator);

Expand Down
Loading