From 8a3f154b49c308ad8cc748caf842b7759132ea34 Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Fri, 22 Mar 2024 16:39:35 +0000 Subject: [PATCH 1/5] 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 --- lib/Github/ResultPager.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Github/ResultPager.php b/lib/Github/ResultPager.php index cfd1d605e4f..61e6c17ae60 100644 --- a/lib/Github/ResultPager.php +++ b/lib/Github/ResultPager.php @@ -86,6 +86,10 @@ public function fetch(AbstractApi $api, string $method, array $parameters = []): $api = $closure($api); $result = $api->$method(...$parameters); + if ($result === "" && $this->client->getLastResponse()->getStatusCode() === 204) { + $result = []; + } + $this->postFetch(true); return $result; From 4fff55583e3c345cb835ecd6ba04db6dd70b7762 Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Fri, 22 Mar 2024 16:45:00 +0000 Subject: [PATCH 2/5] fix style issue raised by continuous-integration/styleci/pr --- lib/Github/ResultPager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Github/ResultPager.php b/lib/Github/ResultPager.php index 61e6c17ae60..a1d94785ca6 100644 --- a/lib/Github/ResultPager.php +++ b/lib/Github/ResultPager.php @@ -86,7 +86,7 @@ public function fetch(AbstractApi $api, string $method, array $parameters = []): $api = $closure($api); $result = $api->$method(...$parameters); - if ($result === "" && $this->client->getLastResponse()->getStatusCode() === 204) { + if ($result === '' && $this->client->getLastResponse()->getStatusCode() === 204) { $result = []; } From eaa7993f2cda2754dbd00f8b6190c0a738eefbb8 Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Sun, 24 Mar 2024 17:14:20 +0000 Subject: [PATCH 3/5] Added unit test for case of GitHub API returning 204 empty content in ResultPager #1091 --- test/Github/Tests/ResultPagerTest.php | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 2839e16f3df..7c6d3afc1a5 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -6,6 +6,7 @@ use Github\Api\Organization\Members; use Github\Api\Repository\Statuses; use Github\Api\Search; +use Github\Api\Repo; use Github\Client; use Github\ResultPager; use Github\Tests\Mock\PaginatedResponse; @@ -116,6 +117,40 @@ public function shouldGetAllSearchResults() $this->assertCount($amountLoops * count($content['items']), $result); } + /** + * @test + */ + public function shouldHandleEmptyContributorListWith204Header() + { + // Set up a 204 response with an empty body + $response = new Response(204, [], ''); + $username = 'testuser'; + $reponame = 'testrepo'; + + // Mock the HttpClient to return the empty response + $httpClientMock = $this->getMockBuilder(HttpClient::class) + ->onlyMethods(['sendRequest']) + ->getMock(); + $httpClientMock + ->method('sendRequest') + ->willReturn($response); + + $client = Client::createWithHttpClient($httpClientMock); + + $repoApi = new Repo($client); + + $paginator = $this->getMockBuilder(ResultPager::class) + ->setConstructorArgs([$client]) // Pass the Client in the constructor + ->onlyMethods(['fetchAll']) + ->getMock(); + $paginator->expects($this->once()) + ->method('fetchAll') + ->with($repoApi, 'contributors', [$username, $reponame]) + ->willReturn([]); + + $this->assertEquals([], $paginator->fetchAll($repoApi, 'contributors', [$username, $reponame])); + } + public function testFetch() { $result = ['foo']; @@ -201,6 +236,34 @@ public function testFetchAllWithoutKeys() $this->assertCount(9, $result); } + public function testFetchAll() + { + $content = [ + ['title' => 'issue 1'], + ['title' => 'issue 2'], + ['title' => 'issue 3'], + ]; + + $response = new PaginatedResponse(3, $content); + + // httpClient mock + $httpClientMock = $this->getMockBuilder(HttpClient::class) + ->onlyMethods(['sendRequest']) + ->getMock(); + $httpClientMock + ->expects($this->exactly(3)) + ->method('sendRequest') + ->willReturn($response); + + $client = Client::createWithHttpClient($httpClientMock); + + $api = new Issue($client); + $paginator = new ResultPager($client); + $result = $paginator->fetchAll($api, 'all', ['knplabs', 'php-github-api']); + + $this->assertCount(9, $result); + } + /** * @group legacy */ From 08c3d7a918fb4123b444085f9ae4e6d4ef2639bd Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Sun, 24 Mar 2024 17:19:14 +0000 Subject: [PATCH 4/5] Updated based on feedback from continuous-integration/styleci/pr --- test/Github/Tests/ResultPagerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 7c6d3afc1a5..08bb6c74f5e 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -4,9 +4,9 @@ use Github\Api\Issue; use Github\Api\Organization\Members; +use Github\Api\Repo; use Github\Api\Repository\Statuses; use Github\Api\Search; -use Github\Api\Repo; use Github\Client; use Github\ResultPager; use Github\Tests\Mock\PaginatedResponse; @@ -118,7 +118,7 @@ public function shouldGetAllSearchResults() } /** - * @test + * @test */ public function shouldHandleEmptyContributorListWith204Header() { From a3d2fb821c49a83a65f27f4b8999e3d44532eed7 Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Sun, 24 Mar 2024 17:47:19 +0000 Subject: [PATCH 5/5] Another change requested by continuous-integration/styleci/pr (but not for my changed code) --- test/Github/Tests/ResultPagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 08bb6c74f5e..3df610ee250 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -187,7 +187,7 @@ public function testFetchAllPreserveKeys() 'sha' => '43068834af7e501778708ed13106de95f782328c', ]; - $response = new Response(200, ['Content-Type'=>'application/json'], Utils::streamFor(json_encode($content))); + $response = new Response(200, ['Content-Type' => 'application/json'], Utils::streamFor(json_encode($content))); // httpClient mock $httpClientMock = $this->getMockBuilder(HttpClient::class)