Skip to content

Commit 1a41692

Browse files
Backport ResultPager changes to 2.x
Co-authored-by: Graham Campbell <[email protected]>
1 parent e561339 commit 1a41692

File tree

5 files changed

+80
-42
lines changed

5 files changed

+80
-42
lines changed

lib/Github/Api/AbstractApi.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ public function setPage($page)
6464
}
6565

6666
/**
67+
* @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`.
68+
*
6769
* @return null|int
6870
*/
6971
public function getPerPage()
@@ -72,6 +74,8 @@ public function getPerPage()
7274
}
7375

7476
/**
77+
* @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`.
78+
*
7579
* @param null|int $perPage
7680
*/
7781
public function setPerPage($perPage)

lib/Github/Api/ApiInterface.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Api interface.
77
*
88
* @author Joseph Bielawski <[email protected]>
9+
* @deprecated since 2.18 and will removed in 3.0. Changing items per page will be done internally by the `ResultPager`.
910
*/
1011
interface ApiInterface
1112
{

lib/Github/ResultPager.php

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
namespace Github;
44

5+
use Github\Api\AbstractApi;
56
use Github\Api\ApiInterface;
6-
use Github\Api\Search;
77
use Github\HttpClient\Message\ResponseMediator;
88

99
/**
@@ -14,6 +14,12 @@
1414
*/
1515
class ResultPager implements ResultPagerInterface
1616
{
17+
/**
18+
* The default number of entries to request per page.
19+
* @var int
20+
*/
21+
private const PER_PAGE = 100;
22+
1723
/**
1824
* The GitHub Client to use for pagination.
1925
*
@@ -28,6 +34,8 @@ class ResultPager implements ResultPagerInterface
2834
*/
2935
protected $pagination;
3036

37+
private $perPage;
38+
3139
/**
3240
* The Github client to use for pagination.
3341
*
@@ -41,9 +49,10 @@ class ResultPager implements ResultPagerInterface
4149
*
4250
* @param \Github\Client $client
4351
*/
44-
public function __construct(Client $client)
52+
public function __construct(Client $client, int $perPage = null)
4553
{
4654
$this->client = $client;
55+
$this->perPage = $perPage ?? self::PER_PAGE;
4756
}
4857

4958
/**
@@ -59,7 +68,23 @@ public function getPagination()
5968
*/
6069
public function fetch(ApiInterface $api, $method, array $parameters = [])
6170
{
71+
$paginatorPerPage = $this->perPage;
72+
$closure = \Closure::bind(function (AbstractApi $api) use ($paginatorPerPage) {
73+
$clone = clone $api;
74+
75+
if(null !== $api->getPerPage()) {
76+
@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);
77+
return $clone;
78+
}
79+
80+
$clone->perPage = $paginatorPerPage;
81+
82+
return $clone;
83+
}, null, AbstractApi::class);
84+
85+
$api = $closure($api);
6286
$result = $this->callApi($api, $method, $parameters);
87+
6388
$this->postFetch();
6489

6590
return $result;
@@ -70,34 +95,23 @@ public function fetch(ApiInterface $api, $method, array $parameters = [])
7095
*/
7196
public function fetchAll(ApiInterface $api, $method, array $parameters = [])
7297
{
73-
$isSearch = $api instanceof Search;
74-
75-
// get the perPage from the api
76-
$perPage = $api->getPerPage();
77-
78-
// set parameters per_page to GitHub max to minimize number of requests
79-
$api->setPerPage(100);
98+
return iterator_to_array($this->fetchAllLazy($api, $method, $parameters));
99+
}
80100

81-
try {
82-
$result = $this->callApi($api, $method, $parameters);
83-
$this->postFetch();
101+
public function fetchAllLazy(ApiInterface $api, string $method, array $parameters = [])
102+
{
103+
$result = $this->fetch($api, $method, $parameters);
84104

85-
if ($isSearch) {
86-
$result = isset($result['items']) ? $result['items'] : $result;
87-
}
105+
foreach ($result['items'] ?? $result as $item) {
106+
yield $item;
107+
}
88108

89-
while ($this->hasNext()) {
90-
$next = $this->fetchNext();
109+
while ($this->hasNext()) {
110+
$result = $this->fetchNext();
91111

92-
if ($isSearch) {
93-
$result = array_merge($result, $next['items']);
94-
} else {
95-
$result = array_merge($result, $next);
96-
}
112+
foreach ($result['items'] ?? $result as $item) {
113+
yield $item;
97114
}
98-
} finally {
99-
// restore the perPage
100-
$api->setPerPage($perPage);
101115
}
102116

103117
return $result;
@@ -187,6 +201,8 @@ protected function get($key)
187201
}
188202

189203
/**
204+
* @deprecated since 2.18 and will be removed in 3.0.
205+
*
190206
* @param ApiInterface $api
191207
* @param string $method
192208
* @param array $parameters

lib/Github/ResultPagerInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
*
1010
* @author Ramon de la Fuente <[email protected]>
1111
* @author Mitchel Verschoof <[email protected]>
12+
*
13+
* @method fetchAllLazy(ApiInterface $api, string $method, array $parameters = [])
1214
*/
1315
interface ResultPagerInterface
1416
{

test/Github/Tests/ResultPagerTest.php

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,36 @@
77
use Github\ResultPager;
88
use Github\Tests\Mock\PaginatedResponse;
99
use Http\Client\HttpClient;
10+
use PHPUnit\Framework\TestCase;
1011

1112
/**
12-
* ResultPagerTest.
13-
*
1413
* @author Ramon de la Fuente <[email protected]>
1514
* @author Mitchel Verschoof <[email protected]>
1615
* @author Tobias Nyholm <[email protected]>
1716
*/
18-
class ResultPagerTest extends \PHPUnit\Framework\TestCase
17+
class ResultPagerTest extends TestCase
1918
{
19+
public function provideFetchCases()
20+
{
21+
return [
22+
['fetchAll'],
23+
['fetchAllLazy'],
24+
];
25+
}
26+
2027
/**
21-
* @test
28+
* @test provideFetchCases
2229
*
23-
* description fetchAll
30+
* @dataProvider provideFetchCases
2431
*/
25-
public function shouldGetAllResults()
32+
public function shouldGetAllResults(string $fetchMethod)
2633
{
2734
$amountLoops = 3;
2835
$content = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
2936
$response = new PaginatedResponse($amountLoops, $content);
3037

3138
// httpClient mock
32-
$httpClientMock = $this->getMockBuilder(\Http\Client\HttpClient::class)
39+
$httpClientMock = $this->getMockBuilder(HttpClient::class)
3340
->setMethods(['sendRequest'])
3441
->getMock();
3542
$httpClientMock
@@ -47,7 +54,13 @@ public function shouldGetAllResults()
4754

4855
// Run fetchAll on result paginator
4956
$paginator = new ResultPager($client);
50-
$result = $paginator->fetchAll($memberApi, $method, $parameters);
57+
$result = $paginator->$fetchMethod($memberApi, $method, $parameters);
58+
59+
if (is_array($result)) {
60+
$this->assertSame('fetchAll', $fetchMethod);
61+
} else {
62+
$result = iterator_to_array($result);
63+
}
5164

5265
$this->assertCount($amountLoops * count($content), $result);
5366
}
@@ -76,7 +89,7 @@ public function shouldGetAllSearchResults()
7689
$response = new PaginatedResponse($amountLoops, $content);
7790

7891
// httpClient mock
79-
$httpClientMock = $this->getMockBuilder(\Http\Client\HttpClient::class)
92+
$httpClientMock = $this->getMockBuilder(HttpClient::class)
8093
->setMethods(['sendRequest'])
8194
->getMock();
8295
$httpClientMock
@@ -97,19 +110,21 @@ public function shouldGetAllSearchResults()
97110
public function testFetch()
98111
{
99112
$result = 'foo';
100-
$method = 'bar';
113+
$method = 'all';
101114
$parameters = ['baz'];
102-
$api = $this->getMockBuilder(\Github\Api\ApiInterface::class)
115+
$api = $this->getMockBuilder(Members::class)
116+
->disableOriginalConstructor()
117+
->setMethods(['all'])
103118
->getMock();
119+
$api->expects($this->once())
120+
->method('all')
121+
->with(...$parameters)
122+
->willReturn($result);
104123

105-
$paginator = $this->getMockBuilder(\Github\ResultPager::class)
124+
$paginator = $this->getMockBuilder(ResultPager::class)
106125
->disableOriginalConstructor()
107-
->setMethods(['callApi', 'postFetch'])
126+
->setMethods(['postFetch'])
108127
->getMock();
109-
$paginator->expects($this->once())
110-
->method('callApi')
111-
->with($api, $method, $parameters)
112-
->willReturn($result);
113128

114129
$paginator->expects($this->once())
115130
->method('postFetch');

0 commit comments

Comments
 (0)