bug #1135 Handle case of GitHub returning 204 No Content in some scen…

…arios (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)

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;

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
      */