github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2026-03-22 13:47:19 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: Remove N+1 request from Bitbucket Data Center integration (#13281)

Browse Source
This commit is contained in:
Joe Laverty
2026-03-10 12:08:30 -04:00
committed by GitHub
parent fc24be2627
commit 3432bbbb88
2 changed files with 42 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
openhands/integrations/bitbucket_data_center/service/base.py
View File

@@ -216,13 +216,18 @@ class BitbucketDCMixinBase(BaseGitService, HTTPClient):
)
async def _parse_repository(
self, repo: dict, link_header: str | None = None
self,
repo: dict,
link_header: str | None = None,
fetch_default_branch: bool = False,
) -> Repository:
"""Parse a Bitbucket data center API repository response into a Repository object.
Args:
repo: Repository data from Bitbucket data center API
link_header: Optional link header for pagination
fetch_default_branch: Whether to make an additional API call to fetch the
default branch. Set to False for listing endpoints to avoid N+1 queries.
Returns:
Repository object
@@ -240,14 +245,15 @@ class BitbucketDCMixinBase(BaseGitService, HTTPClient):
is_public = repo.get('public', False)
main_branch: str | None = None
try:
default_branch_url = (
f'{self._repo_api_base(project_key, repo_slug)}/default-branch'
)
default_branch_data, _ = await self._make_request(default_branch_url)
main_branch = default_branch_data.get('displayId') or None
except Exception as e:
logger.debug(f'Could not fetch default branch for {full_name}: {e}')
if fetch_default_branch:
try:
default_branch_url = (
f'{self._repo_api_base(project_key, repo_slug)}/default-branch'
)
default_branch_data, _ = await self._make_request(default_branch_url)
main_branch = default_branch_data.get('displayId') or None
except Exception as e:
logger.debug(f'Could not fetch default branch for {full_name}: {e}')
return Repository(
id=str(repo.get('id', '')),
@@ -275,7 +281,7 @@ class BitbucketDCMixinBase(BaseGitService, HTTPClient):
owner, repo = self._extract_owner_and_repo(repository)
url = self._repo_api_base(owner, repo)
data, _ = await self._make_request(url)
return await self._parse_repository(data)
return await self._parse_repository(data, fetch_default_branch=True)
async def _get_cursorrules_url(self, repository: str) -> str:
"""Get the URL for checking .cursorrules file."""

67
tests/unit/integrations/bitbucket_data_center/test_bitbucket_dc_repos.py
View File

@@ -112,21 +112,15 @@ async def test_search_repositories_slash_query():
query = 'PROJ/myrepo'
mock_repo = _repo_dict('PROJ', slug='myrepo', name='My Repository')
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_fetch_paginated_data',
new=AsyncMock(return_value=[mock_repo]),
) as mock_fetch:
with patch.object(
svc,
'_make_request',
new=AsyncMock(return_value=(mock_default_branch, {})),
):
repos = await svc.search_repositories(
query, 25, 'name', 'asc', False, AppMode.SAAS
)
repos = await svc.search_repositories(
query, 25, 'name', 'asc', False, AppMode.SAAS
)
mock_fetch.assert_called_once_with(
'https://host.example.com/rest/api/1.0/projects/PROJ/repos',
@@ -135,6 +129,7 @@ async def test_search_repositories_slash_query():
)
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/myrepo'
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -143,24 +138,19 @@ async def test_search_repositories_slash_query_filters_by_name():
svc = make_service()
matching = _repo_dict('PROJ', slug='proj-alpha', name='My Repository')
non_matching = _repo_dict('PROJ', slug='proj-beta', name='Other Repo')
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_fetch_paginated_data',
new=AsyncMock(return_value=[matching, non_matching]),
):
with patch.object(
svc,
'_make_request',
new=AsyncMock(return_value=(mock_default_branch, {})),
):
repos = await svc.search_repositories(
'PROJ/my repository', 25, 'name', 'asc', False, AppMode.SAAS
)
repos = await svc.search_repositories(
'PROJ/my repository', 25, 'name', 'asc', False, AppMode.SAAS
)
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/proj-alpha'
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -169,24 +159,19 @@ async def test_search_repositories_slash_query_filters_by_slug():
svc = make_service()
matching = _repo_dict('PROJ', slug='my-repo', name='My Repository')
non_matching = _repo_dict('PROJ', slug='other-repo', name='Other Repository')
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_fetch_paginated_data',
new=AsyncMock(return_value=[matching, non_matching]),
):
with patch.object(
svc,
'_make_request',
new=AsyncMock(return_value=(mock_default_branch, {})),
):
repos = await svc.search_repositories(
'PROJ/my-repo', 25, 'name', 'asc', False, AppMode.SAAS
)
repos = await svc.search_repositories(
'PROJ/my-repo', 25, 'name', 'asc', False, AppMode.SAAS
)
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/my-repo'
assert repos[0].main_branch is None
# ── get_paginated_repos ───────────────────────────────────────────────────────
@@ -199,18 +184,18 @@ async def test_get_paginated_repos_parses_values():
'values': [_repo_dict()],
'isLastPage': True,
}
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_make_request',
side_effect=[(mock_response, {}), (mock_default_branch, {})],
return_value=(mock_response, {}),
):
repos = await svc.get_paginated_repos(1, 25, 'name', 'PROJ')
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/myrepo'
assert repos[0].link_header == ''
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -221,17 +206,17 @@ async def test_get_paginated_repos_has_next_page():
'isLastPage': False,
'nextPageStart': 25,
}
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_make_request',
side_effect=[(mock_response, {}), (mock_default_branch, {})],
return_value=(mock_response, {}),
):
repos = await svc.get_paginated_repos(1, 25, 'name', 'PROJ')
assert len(repos) == 1
assert 'rel="next"' in repos[0].link_header
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -241,17 +226,17 @@ async def test_get_paginated_repos_last_page():
'values': [_repo_dict()],
'isLastPage': True,
}
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_make_request',
side_effect=[(mock_response, {}), (mock_default_branch, {})],
return_value=(mock_response, {}),
):
repos = await svc.get_paginated_repos(1, 25, 'name', 'PROJ')
assert len(repos) == 1
assert repos[0].link_header == ''
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -265,17 +250,17 @@ async def test_get_paginated_repos_filters_by_slug():
],
'isLastPage': True,
}
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_make_request',
side_effect=[(mock_response, {}), (mock_default_branch, {})],
return_value=(mock_response, {}),
):
repos = await svc.get_paginated_repos(1, 25, 'name', 'PROJ', query='my-repo')
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/my-repo'
assert repos[0].main_branch is None
@pytest.mark.asyncio
@@ -289,12 +274,11 @@ async def test_get_paginated_repos_filters_by_name():
],
'isLastPage': True,
}
mock_default_branch = {'displayId': 'main'}
with patch.object(
svc,
'_make_request',
side_effect=[(mock_response, {}), (mock_default_branch, {})],
return_value=(mock_response, {}),
):
repos = await svc.get_paginated_repos(
1, 25, 'name', 'PROJ', query='my repository'
@@ -302,6 +286,7 @@ async def test_get_paginated_repos_filters_by_name():
assert len(repos) == 1
assert repos[0].full_name == 'PROJ/proj-alpha'
assert repos[0].main_branch is None
# ── get_all_repositories ──────────────────────────────────────────────────────
@@ -320,14 +305,14 @@ async def test_get_all_repositories_iterates_projects():
return [_repo_dict('PROJ2', 'repo2')]
return []
mock_default_branch = {'displayId': 'main'}
with patch.object(svc, '_fetch_paginated_data', side_effect=fake_fetch):
with patch.object(svc, '_make_request', return_value=(mock_default_branch, {})):
repos = await svc.get_all_repositories('name', AppMode.SAAS)
repos = await svc.get_all_repositories('name', AppMode.SAAS)
full_names = {r.full_name for r in repos}
assert 'PROJ1/repo1' in full_names
assert 'PROJ2/repo2' in full_names
for repo in repos:
assert repo.main_branch is None
# ── get_installations ─────────────────────────────────────────────────────────
@@ -352,4 +337,4 @@ async def test_get_installations_returns_project_keys():
async def _make_parsed_repo(svc, repo_dict):
"""Helper to create a parsed Repository from a repo dict (with mocked default branch)."""
with patch.object(svc, '_make_request', return_value=({'displayId': 'main'}, {})):
return await svc._parse_repository(repo_dict)
return await svc._parse_repository(repo_dict, fetch_default_branch=True)