Skip to content

Commit a9f1df7

Browse files
committed
fix: remove N-sequential-call loop; GitHub pagination is cursor-only
1 parent 1bfc5c4 commit a9f1df7

3 files changed

Lines changed: 9 additions & 33 deletions

File tree

src/VCS/Adapter.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ abstract public function getRepositoryName(string $repositoryId): string;
233233
* @param string $owner Owner name of the repository
234234
* @param string $repositoryName Name of the repository
235235
* @param int $perPage Number of results per page, clamped to [1, 100]
236-
* @param int|string|null $page 1-based integer page number, or an opaque cursor string from a previous nextCursor (cursor form only supported by GitHub)
236+
* @param int|string|null $page For GitHub: pass 1 for the first page; for subsequent pages always pass the opaque cursor string from nextCursor — GitHub has no concept of integer offset pages and any integer other than 1 is treated as page 1. For other providers: 1-based integer page number; string cursors are ignored and treated as page 1.
237237
* @param string $search Prefix filter for branch names; empty string returns all branches
238238
* @return array{items: array<string>, hasNext: bool, nextCursor: string|null}
239239
*/

src/VCS/Adapter/Git/GitHub.php

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -752,40 +752,18 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName,
752752
* @param string $owner
753753
* @param string $repositoryName
754754
* @param int $perPage Clamped to [1, 100]
755-
* @param int|string|null $page 1-based page number or opaque GraphQL cursor
755+
* @param int|string|null $page Pass 1 (or null) for the first page. For subsequent pages
756+
* always pass the opaque cursor string from the previous nextCursor — GitHub uses
757+
* cursor-based GraphQL pagination and has no concept of integer page offsets.
758+
* Any integer value other than 1 is treated as page 1.
756759
* @param string $search Prefix filter; empty returns all branches
757760
* @return array{items: array<string>, hasNext: bool, nextCursor: string|null}
758761
*/
759762
public function listBranches(string $owner, string $repositoryName, int $perPage = 100, int|string|null $page = 1, string $search = ''): array
760763
{
761764
$perPage = min(max($perPage, 1), 100);
762765
$cursor = is_string($page) ? $page : null;
763-
$page = is_int($page) ? max($page, 1) : 1;
764-
$result = [
765-
'items' => [],
766-
'hasNext' => false,
767-
'nextCursor' => null,
768-
];
769-
770-
for ($currentPage = 1; $currentPage <= $page; $currentPage++) {
771-
$result = $this->listBranchesPage($owner, $repositoryName, $perPage, $cursor, $search);
772-
773-
if ($currentPage === $page) {
774-
return $result;
775-
}
776-
777-
if ($result['hasNext'] === false) {
778-
return [
779-
'items' => [],
780-
'hasNext' => false,
781-
'nextCursor' => null,
782-
];
783-
}
784-
785-
$cursor = $result['nextCursor'];
786-
}
787-
788-
return $result;
766+
return $this->listBranchesPage($owner, $repositoryName, $perPage, $cursor, $search);
789767
}
790768

791769
/**

tests/VCS/Adapter/GitHubTest.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -546,20 +546,18 @@ public function testListBranchesPagination(): void
546546
/** @var GitHub $adapter */
547547
$adapter = $this->vcsAdapter;
548548

549+
// Cursor-based navigation: always use nextCursor from the previous response
549550
$page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1);
550551
$this->assertSame(['branch-a'], $page1['items']);
551552
$this->assertTrue($page1['hasNext']);
552553
$this->assertNotEmpty($page1['nextCursor']);
553554

554-
$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2);
555+
$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, $page1['nextCursor']);
555556
$this->assertSame(['branch-b'], $page2['items']);
556557
$this->assertTrue($page2['hasNext']);
557558
$this->assertNotEmpty($page2['nextCursor']);
558559

559-
$cursorPage2 = $adapter->listBranches(static::$owner, $repositoryName, 1, $page1['nextCursor']);
560-
$this->assertSame($page2, $cursorPage2);
561-
562-
$page3 = $adapter->listBranches(static::$owner, $repositoryName, 1, 3);
560+
$page3 = $adapter->listBranches(static::$owner, $repositoryName, 1, $page2['nextCursor']);
563561
$this->assertSame([static::$defaultBranch], $page3['items']);
564562
$this->assertFalse($page3['hasNext']);
565563
$this->assertNull($page3['nextCursor']);

0 commit comments

Comments
 (0)