Skip to content

Commit 61b478a

Browse files
authored
bug #1135 Handle case of GitHub returning 204 No Content in some scenarios (tomcorbett)
This PR was squashed before being merged into the 3.14-dev branch. Discussion ---------- ResultPager breaks because no array is returned (it's an empty string) - issue ResultPager::get() can return string #1091 Commits ------- 8a3f154 Handle case of GitHub returning 204 No Content in some scenarios which breaks ResultPager because no array is returned (it's an empty string) - issue ResultPager::get() can return string #1091 4fff555 fix style issue raised by continuous-integration/styleci/pr eaa7993 Added unit test for case of GitHub API returning 204 empty content in ResultPager #1091 08c3d7a Updated based on feedback from continuous-integration/styleci/pr a3d2fb8 Another change requested by continuous-integration/styleci/pr (but not for my changed code)
1 parent a162dd3 commit 61b478a

File tree

2 files changed

+67
-0
lines changed

2 files changed

+67
-0
lines changed

lib/Github/ResultPager.php

+4
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ public function fetch(AbstractApi $api, string $method, array $parameters = []):
8686
$api = $closure($api);
8787
$result = $api->$method(...$parameters);
8888

89+
if ($result === '' && $this->client->getLastResponse()->getStatusCode() === 204) {
90+
$result = [];
91+
}
92+
8993
$this->postFetch(true);
9094

9195
return $result;

test/Github/Tests/ResultPagerTest.php

+63
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Github\Api\Issue;
66
use Github\Api\Organization\Members;
7+
use Github\Api\Repo;
78
use Github\Api\Repository\Statuses;
89
use Github\Api\Search;
910
use Github\Client;
@@ -116,6 +117,40 @@ public function shouldGetAllSearchResults()
116117
$this->assertCount($amountLoops * count($content['items']), $result);
117118
}
118119

120+
/**
121+
* @test
122+
*/
123+
public function shouldHandleEmptyContributorListWith204Header()
124+
{
125+
// Set up a 204 response with an empty body
126+
$response = new Response(204, [], '');
127+
$username = 'testuser';
128+
$reponame = 'testrepo';
129+
130+
// Mock the HttpClient to return the empty response
131+
$httpClientMock = $this->getMockBuilder(HttpClient::class)
132+
->onlyMethods(['sendRequest'])
133+
->getMock();
134+
$httpClientMock
135+
->method('sendRequest')
136+
->willReturn($response);
137+
138+
$client = Client::createWithHttpClient($httpClientMock);
139+
140+
$repoApi = new Repo($client);
141+
142+
$paginator = $this->getMockBuilder(ResultPager::class)
143+
->setConstructorArgs([$client]) // Pass the Client in the constructor
144+
->onlyMethods(['fetchAll'])
145+
->getMock();
146+
$paginator->expects($this->once())
147+
->method('fetchAll')
148+
->with($repoApi, 'contributors', [$username, $reponame])
149+
->willReturn([]);
150+
151+
$this->assertEquals([], $paginator->fetchAll($repoApi, 'contributors', [$username, $reponame]));
152+
}
153+
119154
public function testFetch()
120155
{
121156
$result = ['foo'];
@@ -201,6 +236,34 @@ public function testFetchAllWithoutKeys()
201236
$this->assertCount(9, $result);
202237
}
203238

239+
public function testFetchAll()
240+
{
241+
$content = [
242+
['title' => 'issue 1'],
243+
['title' => 'issue 2'],
244+
['title' => 'issue 3'],
245+
];
246+
247+
$response = new PaginatedResponse(3, $content);
248+
249+
// httpClient mock
250+
$httpClientMock = $this->getMockBuilder(HttpClient::class)
251+
->onlyMethods(['sendRequest'])
252+
->getMock();
253+
$httpClientMock
254+
->expects($this->exactly(3))
255+
->method('sendRequest')
256+
->willReturn($response);
257+
258+
$client = Client::createWithHttpClient($httpClientMock);
259+
260+
$api = new Issue($client);
261+
$paginator = new ResultPager($client);
262+
$result = $paginator->fetchAll($api, 'all', ['knplabs', 'php-github-api']);
263+
264+
$this->assertCount(9, $result);
265+
}
266+
204267
/**
205268
* @group legacy
206269
*/

0 commit comments

Comments
 (0)