From 61b478ab7246348bba876bf694e3f0e23a665e2d Mon Sep 17 00:00:00 2001 From: Thomas Corbett Date: Sun, 24 Mar 2024 14:20:24 -0400 Subject: [PATCH] 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 ------- 8a3f154b49c308ad8cc748caf842b7759132ea34 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 4fff55583e3c345cb835ecd6ba04db6dd70b7762 fix style issue raised by continuous-integration/styleci/pr eaa7993f2cda2754dbd00f8b6190c0a738eefbb8 Added unit test for case of GitHub API returning 204 empty content in ResultPager #1091 08c3d7a918fb4123b444085f9ae4e6d4ef2639bd Updated based on feedback from continuous-integration/styleci/pr a3d2fb821c49a83a65f27f4b8999e3d44532eed7 Another change requested by continuous-integration/styleci/pr (but not for my changed code) --- lib/Github/ResultPager.php | 4 ++ test/Github/Tests/ResultPagerTest.php | 63 +++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/lib/Github/ResultPager.php b/lib/Github/ResultPager.php index cfd1d605e4f..a1d94785ca6 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; diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 264fd42ef75..3df610ee250 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -4,6 +4,7 @@ use Github\Api\Issue; use Github\Api\Organization\Members; +use Github\Api\Repo; use Github\Api\Repository\Statuses; use Github\Api\Search; use Github\Client; @@ -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 */