Skip to content

Commit 6e88252

Browse files
committed
Remove loop from listBranches, use page/perPage for single-page fetch
1 parent a413e42 commit 6e88252

2 files changed

Lines changed: 23 additions & 24 deletions

File tree

src/VCS/Adapter/Git/GitHub.php

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -754,28 +754,20 @@ public function listBranches(string $owner, string $repositoryName, int $perPage
754754
{
755755
$url = "/repos/$owner/$repositoryName/branches";
756756
$perPage = min(max($perPage, 1), 100);
757-
$names = [];
758757

759-
do {
760-
$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [
761-
'page' => $page,
762-
'per_page' => $perPage,
763-
]);
764-
765-
$responseBody = $response['body'] ?? [];
766-
767-
if (!is_array($responseBody)) {
768-
break;
769-
}
758+
$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [
759+
'page' => $page,
760+
'per_page' => $perPage,
761+
]);
770762

771-
foreach ($responseBody as $subarray) {
772-
$names[] = $subarray['name'] ?? '';
773-
}
763+
$statusCode = $response['headers']['status-code'] ?? 0;
764+
$responseBody = $response['body'] ?? [];
774765

775-
$page++;
776-
} while (count($responseBody) === $perPage);
766+
if ($statusCode < 200 || $statusCode >= 300 || !is_array($responseBody)) {
767+
return [];
768+
}
777769

778-
return $names;
770+
return array_values(array_map(fn ($branch) => $branch['name'] ?? '', $responseBody));
779771
}
780772

781773
/**

tests/VCS/Adapter/GitHubTest.php

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ public function testGetCommitWithInvalidHash(): void
525525
}
526526
}
527527

528-
public function testListBranchesFetchesMultiplePages(): void
528+
public function testListBranchesPagination(): void
529529
{
530530
$repositoryName = 'test-list-branches-pages-' . \uniqid();
531531
$this->vcsAdapter->createRepository(static::$owner, $repositoryName, false);
@@ -537,12 +537,19 @@ public function testListBranchesFetchesMultiplePages(): void
537537

538538
/** @var GitHub $adapter */
539539
$adapter = $this->vcsAdapter;
540-
$branches = $adapter->listBranches(static::$owner, $repositoryName, 1);
541540

542-
$this->assertIsArray($branches);
543-
$this->assertContains(static::$defaultBranch, $branches);
544-
$this->assertContains('branch-a', $branches);
545-
$this->assertContains('branch-b', $branches);
541+
$page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1);
542+
$this->assertCount(1, $page1);
543+
544+
$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2);
545+
$this->assertCount(1, $page2);
546+
547+
$this->assertNotSame($page1[0], $page2[0]);
548+
549+
$all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1);
550+
$this->assertContains(static::$defaultBranch, $all);
551+
$this->assertContains('branch-a', $all);
552+
$this->assertContains('branch-b', $all);
546553
} finally {
547554
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
548555
}

0 commit comments

Comments
 (0)