Skip to content

refactor(bitbucket): centralize Data Center pagination guard #1105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dpol1 wants to merge 2 commits into
base: main
Choose a base branch
from
+104 −48
Open

refactor(bitbucket): centralize Data Center pagination guard #1105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bce993
refactor(bitbucket): centralize Data Center pagination guard
dpol1 Aug 18, 2026
3e47136
refactor(bitbucket): drop redundant lambda wrappers in pagination test
dpol1 Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 32 additions & 48 deletions tools/bitbucket/src/magpie_bitbucket/datacenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,26 @@ def _api_base(config: BitbucketConfig) -> str:
return f"{base_url}/rest/api/1.0"


def _next_page_start(page: dict[str, Any], start: int) -> int | None:
"""Return the validated next page offset, or None when pagination must stop.

A server that reports a missing, malformed, or non-advancing
``nextPageStart`` would otherwise keep the caller requesting the same
page forever; every Data Center paginator routes through this guard.
"""
if page.get("isLastPage") is True:
return None

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
return None

if next_start <= start:
return None

return next_start


def get_repository(config: BitbucketConfig) -> dict[str, Any]:
"""Fetch repository metadata from Bitbucket Data Center."""
project_key = quote_path(require(config.project_key, "BITBUCKET_PROJECT_KEY"))
Expand Down Expand Up @@ -64,14 +84,8 @@ def get_repository_restrictions(config: BitbucketConfig) -> dict[str, Any]:
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down Expand Up @@ -144,14 +158,8 @@ def list_open_pull_requests(config: BitbucketConfig) -> dict[str, Any]:
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down Expand Up @@ -191,14 +199,8 @@ def get_pull_request_commits(config: BitbucketConfig, pull_request_id: str) -> d
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down Expand Up @@ -298,14 +300,8 @@ def get_pull_request_status(config: BitbucketConfig, pull_request_id: str) -> di
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down Expand Up @@ -341,14 +337,8 @@ def get_pull_request_reviews(config: BitbucketConfig, pull_request_id: str) -> d
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down Expand Up @@ -397,14 +387,8 @@ def get_pull_request_discussion(config: BitbucketConfig, pull_request_id: str) -
if isinstance(values, list):
combined["values"].extend(item for item in values if isinstance(item, dict))

if page.get("isLastPage") is True:
break

next_start = page.get("nextPageStart")
if not isinstance(next_start, int):
break

if next_start <= start:
next_start = _next_page_start(page, start)
if next_start is None:
break

start = next_start
Expand Down
72 changes: 72 additions & 0 deletions tools/bitbucket/tests/test_bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,78 @@ def test_datacenter_get_pull_request_commits_stops_on_non_advancing_next_page_st
assert [item["id"] for item in result["values"]] == ["abc123"]


_DATACENTER_PR_PAGE = {"id": 9, "fromRef": {"latestCommit": "def456"}}


@patch("urllib.request.build_opener")
@pytest.mark.parametrize(
("paginate", "leading_pages"),
[
pytest.param(
datacenter.get_repository_restrictions,
(),
id="repository_restrictions",
),
pytest.param(
datacenter.list_open_pull_requests,
(),
id="open_pull_requests",
),
pytest.param(
lambda config: datacenter.get_pull_request_commits(config, "9"),
(),
id="pull_request_commits",
),
pytest.param(
lambda config: datacenter.get_pull_request_status(config, "9"),
(_DATACENTER_PR_PAGE,),
id="pull_request_status",
),
pytest.param(
lambda config: datacenter.get_pull_request_reviews(config, "9"),
(_DATACENTER_PR_PAGE,),
id="pull_request_reviews",
),
pytest.param(
lambda config: datacenter.get_pull_request_discussion(config, "9"),
(),
id="pull_request_discussion",
),
],
)
def test_datacenter_paginators_stop_on_non_advancing_next_page_start(
mock_build_opener: MagicMock,
paginate: Any,
leading_pages: tuple[dict[str, Any], ...],
datacenter_env: None,
) -> None:
opener = mock_opener(
mock_build_opener,
*leading_pages,
{"values": [{"id": "abc123"}], "isLastPage": False, "nextPageStart": 0},
)

result = paginate(load_config())

assert len(opener.open.call_args_list) == len(leading_pages) + 1
assert [item["id"] for item in result["values"]] == ["abc123"]


@pytest.mark.parametrize(
("page", "start", "expected"),
[
pytest.param({"isLastPage": True, "nextPageStart": 25}, 0, None, id="last_page"),
pytest.param({"isLastPage": False}, 0, None, id="missing_next_page_start"),
pytest.param({"isLastPage": False, "nextPageStart": "25"}, 0, None, id="non_int_next_page_start"),
pytest.param({"isLastPage": False, "nextPageStart": 0}, 0, None, id="repeated_next_page_start"),
pytest.param({"isLastPage": False, "nextPageStart": 5}, 10, None, id="backwards_next_page_start"),
pytest.param({"isLastPage": False, "nextPageStart": 25}, 0, 25, id="advancing_next_page_start"),
],
)
def test_datacenter_next_page_start(page: dict[str, Any], start: int, expected: int | None) -> None:
assert datacenter._next_page_start(page, start) == expected


def test_normalize_cloud_pull_request_commits() -> None:
raw = {
"pull_request_id": "7",
Expand Down
Loading