Skip to content

Commit 9b0a7c3

Browse files
GrahamCampbellGrahamCampbell
authored
feature KnpLabs#907 [3.x] Re-worked pagination to not mutate the api classes (GrahamCampbell)
This PR was squashed before being merged into the 3.0.x-dev branch. Discussion ---------- This re-works pagination to avoid mutating the API classes. It took a few attempts to get the implementation details right. Let me explain why I chose to use clone: 1. If we were to use `new static(...)`, then I would have had to have made the abstract api constructor final. This could cause problems for people mocking the api classes, and also limits the possibility to thunk up arguments to the api classes, say if we want to introduce an api class which takes an extra parameter which is always used (this is common place in the gitlab fork of this library). 2. Is there another way to do things without making the constructor final or using clone? Well, we could have every api object implement perPage for themselves. This is kinda cumbersome though, and I don't think we want to do this. --- This PR partially addresses KnpLabs#904. So, I think cloning then setting the private property, within the abstract api class is the way to go, and nicely separates responsibilities. Classes that extend the abstract api class cannot mutate the property, and outsiders also cannot either. They have to call perPage which gives a fresh instance (one we've cloned). Commits ------- 54c652c Re-worked pagination to not mutate the api classes 4ba3e48 Avoid needing getPerPage and perPage decf105 Restored test coverage
1 parent 58ec8f5 commit 9b0a7c3

27 files changed

+217
-233
lines changed

‎composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
"psr/cache": "^1.0",
2727
"psr/http-client-implementation": "^1.0",
2828
"psr/http-factory-implementation": "^1.0",
29-
"psr/http-message": "^1.0"
29+
"psr/http-message": "^1.0",
30+
"symfony/polyfill-php80": "^1.17"
3031
},
3132
"require-dev": {
3233
"symfony/cache": "^5.1.8",

‎lib/Github/Api/AbstractApi.php

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,79 +6,59 @@
66
use Github\HttpClient\Message\ResponseMediator;
77

88
/**
9-
* Abstract class for Api classes.
10-
*
119
* @author Joseph Bielawski <stloyd@gmail.com>
10+
* @author Graham Campbell <graham@alt-three.com>
1211
*/
13-
abstract class AbstractApi implements ApiInterface
12+
abstract class AbstractApi
1413
{
1514
/**
16-
* The client.
15+
* The client instance.
1716
*
1817
* @var Client
1918
*/
20-
protected $client;
19+
private $client;
2120

2221
/**
23-
* The requested page (GitHub pagination).
22+
* The per page parameter.
2423
*
25-
* @var null|int
24+
* @var int|null
2625
*/
27-
private $page;
26+
private $perPage;
2827

2928
/**
30-
* Number of items per page (GitHub pagination).
29+
* Create a new API instance.
3130
*
32-
* @var null|int
33-
*/
34-
protected $perPage;
35-
36-
/**
3731
* @param Client $client
32+
*
33+
* @return void
3834
*/
3935
public function __construct(Client $client)
4036
{
4137
$this->client = $client;
4238
}
4339

44-
public function configure()
45-
{
46-
}
47-
48-
/**
49-
* @return null|int
50-
*/
51-
public function getPage()
52-
{
53-
return $this->page;
54-
}
55-
5640
/**
57-
* @param null|int $page
41+
* Get the client instance.
42+
*
43+
* @return Client
5844
*/
59-
public function setPage($page)
45+
protected function getClient()
6046
{
61-
$this->page = (null === $page ? $page : (int) $page);
62-
63-
return $this;
47+
return $this->client;
6448
}
6549

6650
/**
67-
* @return null|int
51+
* Get the API version.
52+
*
53+
* @return string
6854
*/
69-
public function getPerPage()
55+
protected function getApiVersion()
7056
{
71-
return $this->perPage;
57+
return $this->client->getApiVersion();
7258
}
7359

74-
/**
75-
* @param null|int $perPage
76-
*/
77-
public function setPerPage($perPage)
60+
public function configure()
7861
{
79-
$this->perPage = (null === $perPage ? $perPage : (int) $perPage);
80-
81-
return $this;
8262
}
8363

8464
/**
@@ -92,12 +72,10 @@ public function setPerPage($perPage)
9272
*/
9373
protected function get($path, array $parameters = [], array $requestHeaders = [])
9474
{
95-
if (null !== $this->page && !isset($parameters['page'])) {
96-
$parameters['page'] = $this->page;
97-
}
9875
if (null !== $this->perPage && !isset($parameters['per_page'])) {
9976
$parameters['per_page'] = $this->perPage;
10077
}
78+
10179
if (array_key_exists('ref', $parameters) && null === $parameters['ref']) {
10280
unset($parameters['ref']);
10381
}

‎lib/Github/Api/ApiInterface.php

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎lib/Github/Api/CurrentUser.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ public function update(array $params)
3535
*/
3636
public function emails()
3737
{
38-
return new Emails($this->client);
38+
return new Emails($this->getClient());
3939
}
4040

4141
/**
4242
* @return Followers
4343
*/
4444
public function follow()
4545
{
46-
return new Followers($this->client);
46+
return new Followers($this->getClient());
4747
}
4848

4949
public function followers($page = 1)
@@ -71,23 +71,23 @@ public function issues(array $params = [], $includeOrgIssues = true)
7171
*/
7272
public function keys()
7373
{
74-
return new PublicKeys($this->client);
74+
return new PublicKeys($this->getClient());
7575
}
7676

7777
/**
7878
* @return Notifications
7979
*/
8080
public function notifications()
8181
{
82-
return new Notifications($this->client);
82+
return new Notifications($this->getClient());
8383
}
8484

8585
/**
8686
* @return Memberships
8787
*/
8888
public function memberships()
8989
{
90-
return new Memberships($this->client);
90+
return new Memberships($this->getClient());
9191
}
9292

9393
/**
@@ -147,15 +147,15 @@ public function repositories($type = 'owner', $sort = 'full_name', $direction =
147147
*/
148148
public function watchers()
149149
{
150-
return new Watchers($this->client);
150+
return new Watchers($this->getClient());
151151
}
152152

153153
/**
154154
* @return Starring
155155
*/
156156
public function starring()
157157
{
158-
return new Starring($this->client);
158+
return new Starring($this->getClient());
159159
}
160160

161161
/**

‎lib/Github/Api/CurrentUser/Starring.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Starring extends AbstractApi
2626
public function configure($bodyType = null)
2727
{
2828
if ('star' === $bodyType) {
29-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.star+json', $this->client->getApiVersion());
29+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.star+json', $this->getApiVersion());
3030
}
3131

3232
return $this;

‎lib/Github/Api/Enterprise.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,30 @@ class Enterprise extends AbstractApi
2222
*/
2323
public function stats()
2424
{
25-
return new Stats($this->client);
25+
return new Stats($this->getClient());
2626
}
2727

2828
/**
2929
* @return License
3030
*/
3131
public function license()
3232
{
33-
return new License($this->client);
33+
return new License($this->getClient());
3434
}
3535

3636
/**
3737
* @return ManagementConsole
3838
*/
3939
public function console()
4040
{
41-
return new ManagementConsole($this->client);
41+
return new ManagementConsole($this->getClient());
4242
}
4343

4444
/**
4545
* @return UserAdmin
4646
*/
4747
public function userAdmin()
4848
{
49-
return new UserAdmin($this->client);
49+
return new UserAdmin($this->getClient());
5050
}
5151
}

‎lib/Github/Api/Gist/Comments.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function configure($bodyType = null)
2929
$bodyType = 'raw';
3030
}
3131

32-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->client->getApiVersion(), $bodyType);
32+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->getApiVersion(), $bodyType);
3333

3434
return $this;
3535
}

‎lib/Github/Api/Gists.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function configure($bodyType = null)
3232
$bodyType = 'raw';
3333
}
3434

35-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s', $this->client->getApiVersion(), $bodyType);
35+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s', $this->getApiVersion(), $bodyType);
3636

3737
return $this;
3838
}
@@ -177,6 +177,6 @@ public function unstar($id)
177177
*/
178178
public function comments()
179179
{
180-
return new Comments($this->client);
180+
return new Comments($this->getClient());
181181
}
182182
}

‎lib/Github/Api/GitData.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,38 +22,38 @@ class GitData extends AbstractApi
2222
*/
2323
public function blobs()
2424
{
25-
return new Blobs($this->client);
25+
return new Blobs($this->getClient());
2626
}
2727

2828
/**
2929
* @return Commits
3030
*/
3131
public function commits()
3232
{
33-
return new Commits($this->client);
33+
return new Commits($this->getClient());
3434
}
3535

3636
/**
3737
* @return References
3838
*/
3939
public function references()
4040
{
41-
return new References($this->client);
41+
return new References($this->getClient());
4242
}
4343

4444
/**
4545
* @return Tags
4646
*/
4747
public function tags()
4848
{
49-
return new Tags($this->client);
49+
return new Tags($this->getClient());
5050
}
5151

5252
/**
5353
* @return Trees
5454
*/
5555
public function trees()
5656
{
57-
return new Trees($this->client);
57+
return new Trees($this->getClient());
5858
}
5959
}

‎lib/Github/Api/GitData/Blobs.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Blobs extends AbstractApi
2626
public function configure($bodyType = null)
2727
{
2828
if ('raw' === $bodyType) {
29-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.raw', $this->client->getApiVersion());
29+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.raw', $this->getApiVersion());
3030
}
3131

3232
return $this;

‎lib/Github/Api/Issue.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function configure($bodyType = null)
3737
$bodyType = 'raw';
3838
}
3939

40-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->client->getApiVersion(), $bodyType);
40+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->getApiVersion(), $bodyType);
4141

4242
return $this;
4343
}
@@ -176,7 +176,7 @@ public function unlock($username, $repository, $id)
176176
*/
177177
public function comments()
178178
{
179-
return new Comments($this->client);
179+
return new Comments($this->getClient());
180180
}
181181

182182
/**
@@ -188,7 +188,7 @@ public function comments()
188188
*/
189189
public function events()
190190
{
191-
return new Events($this->client);
191+
return new Events($this->getClient());
192192
}
193193

194194
/**
@@ -200,7 +200,7 @@ public function events()
200200
*/
201201
public function labels()
202202
{
203-
return new Labels($this->client);
203+
return new Labels($this->getClient());
204204
}
205205

206206
/**
@@ -212,7 +212,7 @@ public function labels()
212212
*/
213213
public function milestones()
214214
{
215-
return new Milestones($this->client);
215+
return new Milestones($this->getClient());
216216
}
217217

218218
/**
@@ -224,7 +224,7 @@ public function milestones()
224224
*/
225225
public function assignees()
226226
{
227-
return new Assignees($this->client);
227+
return new Assignees($this->getClient());
228228
}
229229

230230
/**
@@ -236,6 +236,6 @@ public function assignees()
236236
*/
237237
public function timeline()
238238
{
239-
return new Timeline($this->client);
239+
return new Timeline($this->getClient());
240240
}
241241
}

‎lib/Github/Api/Issue/Comments.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function configure($bodyType = null)
3131
$bodyType = 'full';
3232
}
3333

34-
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->client->getApiVersion(), $bodyType);
34+
$this->acceptHeaderValue = sprintf('application/vnd.github.%s.%s+json', $this->getApiVersion(), $bodyType);
3535

3636
return $this;
3737
}

0 commit comments

Comments
 (0)