diff --git a/lib/Github/Api/AbstractApi.php b/lib/Github/Api/AbstractApi.php index d4d501ed3ec..e6fe4c33385 100644 --- a/lib/Github/Api/AbstractApi.php +++ b/lib/Github/Api/AbstractApi.php @@ -64,6 +64,8 @@ public function setPage($page) } /** + * @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`. + * * @return null|int */ public function getPerPage() @@ -72,6 +74,8 @@ public function getPerPage() } /** + * @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`. + * * @param null|int $perPage */ public function setPerPage($perPage) diff --git a/lib/Github/Api/ApiInterface.php b/lib/Github/Api/ApiInterface.php index 49d5167c29d..61e634cfa78 100644 --- a/lib/Github/Api/ApiInterface.php +++ b/lib/Github/Api/ApiInterface.php @@ -6,6 +6,8 @@ * Api interface. * * @author Joseph Bielawski + * + * @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`. */ interface ApiInterface { diff --git a/lib/Github/ResultPager.php b/lib/Github/ResultPager.php index defcde23e6a..24dff0cd478 100644 --- a/lib/Github/ResultPager.php +++ b/lib/Github/ResultPager.php @@ -2,8 +2,8 @@ namespace Github; +use Github\Api\AbstractApi; use Github\Api\ApiInterface; -use Github\Api\Search; use Github\HttpClient\Message\ResponseMediator; /** @@ -14,6 +14,13 @@ */ class ResultPager implements ResultPagerInterface { + /** + * The default number of entries to request per page. + * + * @var int + */ + private const PER_PAGE = 100; + /** * The GitHub Client to use for pagination. * @@ -28,6 +35,9 @@ class ResultPager implements ResultPagerInterface */ protected $pagination; + /** @var int */ + private $perPage; + /** * The Github client to use for pagination. * @@ -41,9 +51,10 @@ class ResultPager implements ResultPagerInterface * * @param \Github\Client $client */ - public function __construct(Client $client) + public function __construct(Client $client, int $perPage = null) { $this->client = $client; + $this->perPage = $perPage ?? self::PER_PAGE; } /** @@ -59,7 +70,25 @@ public function getPagination() */ public function fetch(ApiInterface $api, $method, array $parameters = []) { + $paginatorPerPage = $this->perPage; + $closure = \Closure::bind(function (ApiInterface $api) use ($paginatorPerPage) { + $clone = clone $api; + + if (null !== $api->getPerPage()) { + @trigger_error(sprintf('Setting the perPage value on an api class is deprecated sinc 2.18 and will be removed in 3.0. Pass the desired items per page value in the constructor of "%s"', __CLASS__), E_USER_DEPRECATED); + + return $clone; + } + + /* @phpstan-ignore-next-line */ + $clone->perPage = $paginatorPerPage; + + return $clone; + }, null, AbstractApi::class); + + $api = $closure($api); $result = $this->callApi($api, $method, $parameters); + $this->postFetch(); return $result; @@ -70,37 +99,24 @@ public function fetch(ApiInterface $api, $method, array $parameters = []) */ public function fetchAll(ApiInterface $api, $method, array $parameters = []) { - $isSearch = $api instanceof Search; - - // get the perPage from the api - $perPage = $api->getPerPage(); - - // set parameters per_page to GitHub max to minimize number of requests - $api->setPerPage(100); + return iterator_to_array($this->fetchAllLazy($api, $method, $parameters)); + } - try { - $result = $this->callApi($api, $method, $parameters); - $this->postFetch(); + public function fetchAllLazy(ApiInterface $api, string $method, array $parameters = []): \Generator + { + $result = $this->fetch($api, $method, $parameters); - if ($isSearch) { - $result = isset($result['items']) ? $result['items'] : $result; - } + foreach ($result['items'] ?? $result as $item) { + yield $item; + } - while ($this->hasNext()) { - $next = $this->fetchNext(); + while ($this->hasNext()) { + $result = $this->fetchNext(); - if ($isSearch) { - $result = array_merge($result, $next['items']); - } else { - $result = array_merge($result, $next); - } + foreach ($result['items'] ?? $result as $item) { + yield $item; } - } finally { - // restore the perPage - $api->setPerPage($perPage); } - - return $result; } /** @@ -187,6 +203,8 @@ protected function get($key) } /** + * @deprecated since 2.18 and will be removed in 3.0. + * * @param ApiInterface $api * @param string $method * @param array $parameters diff --git a/lib/Github/ResultPagerInterface.php b/lib/Github/ResultPagerInterface.php index 80660247900..c8c50d74906 100644 --- a/lib/Github/ResultPagerInterface.php +++ b/lib/Github/ResultPagerInterface.php @@ -9,6 +9,8 @@ * * @author Ramon de la Fuente * @author Mitchel Verschoof + * + * @method fetchAllLazy(ApiInterface $api, string $method, array $parameters = []): iterator */ interface ResultPagerInterface { diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index a105a84f054..7207a5dd3b5 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -7,29 +7,36 @@ use Github\ResultPager; use Github\Tests\Mock\PaginatedResponse; use Http\Client\HttpClient; +use PHPUnit\Framework\TestCase; /** - * ResultPagerTest. - * * @author Ramon de la Fuente * @author Mitchel Verschoof * @author Tobias Nyholm */ -class ResultPagerTest extends \PHPUnit\Framework\TestCase +class ResultPagerTest extends TestCase { + public function provideFetchCases() + { + return [ + ['fetchAll'], + ['fetchAllLazy'], + ]; + } + /** - * @test + * @test provideFetchCases * - * description fetchAll + * @dataProvider provideFetchCases */ - public function shouldGetAllResults() + public function shouldGetAllResults(string $fetchMethod) { $amountLoops = 3; $content = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; $response = new PaginatedResponse($amountLoops, $content); // httpClient mock - $httpClientMock = $this->getMockBuilder(\Http\Client\HttpClient::class) + $httpClientMock = $this->getMockBuilder(HttpClient::class) ->setMethods(['sendRequest']) ->getMock(); $httpClientMock @@ -47,7 +54,13 @@ public function shouldGetAllResults() // Run fetchAll on result paginator $paginator = new ResultPager($client); - $result = $paginator->fetchAll($memberApi, $method, $parameters); + $result = $paginator->$fetchMethod($memberApi, $method, $parameters); + + if (is_array($result)) { + $this->assertSame('fetchAll', $fetchMethod); + } else { + $result = iterator_to_array($result); + } $this->assertCount($amountLoops * count($content), $result); } @@ -76,7 +89,7 @@ public function shouldGetAllSearchResults() $response = new PaginatedResponse($amountLoops, $content); // httpClient mock - $httpClientMock = $this->getMockBuilder(\Http\Client\HttpClient::class) + $httpClientMock = $this->getMockBuilder(HttpClient::class) ->setMethods(['sendRequest']) ->getMock(); $httpClientMock @@ -97,19 +110,21 @@ public function shouldGetAllSearchResults() public function testFetch() { $result = 'foo'; - $method = 'bar'; + $method = 'all'; $parameters = ['baz']; - $api = $this->getMockBuilder(\Github\Api\ApiInterface::class) + $api = $this->getMockBuilder(Members::class) + ->disableOriginalConstructor() + ->setMethods(['all']) ->getMock(); + $api->expects($this->once()) + ->method('all') + ->with(...$parameters) + ->willReturn($result); - $paginator = $this->getMockBuilder(\Github\ResultPager::class) + $paginator = $this->getMockBuilder(ResultPager::class) ->disableOriginalConstructor() - ->setMethods(['callApi', 'postFetch']) + ->setMethods(['postFetch']) ->getMock(); - $paginator->expects($this->once()) - ->method('callApi') - ->with($api, $method, $parameters) - ->willReturn($result); $paginator->expects($this->once()) ->method('postFetch');