diff --git a/src/scm/errors.py b/src/scm/errors.py index c4691d3e..0d10b796 100644 --- a/src/scm/errors.py +++ b/src/scm/errors.py @@ -2,6 +2,7 @@ type ErrorCode = Literal[ "malformed_external_id", + "missing_repository_path", "path_is_directory", "path_is_not_directory", "provider_not_found", @@ -38,6 +39,7 @@ ERROR_CODES: dict[ErrorCode, str] = { "malformed_external_id": "The repository's external ID was malformed.", + "missing_repository_path": "No path was given for the repository.", "path_is_directory": "The requested path is a directory, not a file.", "path_is_not_directory": "The requested path is not a directory.", "provider_not_found": "An unsupported integration provider was found.", @@ -127,6 +129,10 @@ class MalformedExternalId(SCMCodedError): code = "malformed_external_id" +class MissingRepositoryPath(SCMCodedError): + code = "missing_repository_path" + + class PathIsDirectory(SCMCodedError): code = "path_is_directory" diff --git a/src/scm/providers/gitlab/provider.py b/src/scm/providers/gitlab/provider.py index dd10f6ce..7f7e62c2 100644 --- a/src/scm/providers/gitlab/provider.py +++ b/src/scm/providers/gitlab/provider.py @@ -9,6 +9,7 @@ from scm.errors import ( InvalidCheckRunStateTransition, MalformedExternalId, + MissingRepositoryPath, PathIsNotDirectory, ReadmeNotFound, ResourceBadRequest, @@ -243,11 +244,14 @@ def __init__(self, client: ApiClient, organization_id: int, repository: Reposito @property def web_repository_path(self) -> str: + """The repository's path on the instance (e.g. "gitlab-org/gitlab-foss"), for web URLs. + + Sentry's name for a GitLab repository is its display name ("GitLab.org / GitLab FOSS"), which differs from + the path in more than spaces, so the path has to come from the repository. """ - Normalize Sentry's GitLab full repo name (e.g., "my-group / my-team / my-subteam / my-project" - --> "my-group/my-team/my-subteam/my-project") for use in web URLs. - """ - return self.repository["name"].replace(" ", "") + if not (path := self.repository.get("path")): + raise MissingRepositoryPath(detail="The GitLab repository has no path.") + return path def request( self, diff --git a/src/scm/rpc/helpers.py b/src/scm/rpc/helpers.py index 46c1d895..33f5b44b 100644 --- a/src/scm/rpc/helpers.py +++ b/src/scm/rpc/helpers.py @@ -49,6 +49,7 @@ def deserialize_repository(content: bytes) -> Repository: "provider_name": repository.attributes.provider_name, "web_base_url": repository.attributes.web_base_url, "installation_id": repository.attributes.installation_id, + "path": repository.attributes.path, } return result @@ -69,6 +70,7 @@ def serialize_repository(repository: Repository) -> bytes: provider_name=repository["provider_name"], web_base_url=repository["web_base_url"], installation_id=repository["installation_id"], + path=repository.get("path"), ), ) ) diff --git a/src/scm/rpc/types.py b/src/scm/rpc/types.py index 77a7ea41..fd182c23 100644 --- a/src/scm/rpc/types.py +++ b/src/scm/rpc/types.py @@ -32,6 +32,7 @@ class RepositoryAttributes(msgspec.Struct): provider_name: ProviderName web_base_url: str | None = None installation_id: str | None = None + path: str | None = None class JsonApiData[Type, Attributes](msgspec.Struct): diff --git a/src/scm/types.py b/src/scm/types.py index 0f53ad60..5a17f0b7 100644 --- a/src/scm/types.py +++ b/src/scm/types.py @@ -342,6 +342,9 @@ class Repository(TypedDict): web_base_url: str | None # The SCM app's external id. installation_id: str | None + # The repository's path on its provider's host, e.g. "gitlab-org/gitlab-foss". Only Sentry's GitLab + # repositories set it today, because their name is a display name ("GitLab.org / GitLab FOSS"). + path: NotRequired[str | None] class GitRepository(TypedDict): diff --git a/tests/unit/provider/test_gitlab.py b/tests/unit/provider/test_gitlab.py index 09d6b67f..35c560cd 100644 --- a/tests/unit/provider/test_gitlab.py +++ b/tests/unit/provider/test_gitlab.py @@ -7,6 +7,7 @@ import pytest from scm.errors import ( + MissingRepositoryPath, RateLimitExceeded, ResourceBadGateway, ResourceBadRequest, @@ -47,7 +48,9 @@ def client() -> ApiClient: return unittest.mock.MagicMock(_name="client") -def _make_gitlab_provider(client: ApiClient, *, name: str = "test-repo") -> GitLabProvider: +def _make_gitlab_provider( + client: ApiClient, *, name: str = "Test Repo", path: str | None = "test-repo" +) -> GitLabProvider: return GitLabProvider( client=client, organization_id=1, @@ -55,6 +58,7 @@ def _make_gitlab_provider(client: ApiClient, *, name: str = "test-repo") -> GitL id=1, integration_id=1, name=name, + path=path, organization_id=1, is_active=True, external_id="gitlab.com:79787061", @@ -13939,24 +13943,23 @@ def test_download_archive_zip_uses_zip_extension(client, provider: GitLabProvide assert call.kwargs["path"] == "/projects/79787061/repository/archive.zip" -@pytest.mark.parametrize( - ("repository_name", "expected_path"), - [ - ("test-org/test-repo", "test-org/test-repo"), - ("my-group / my-project", "my-group/my-project"), - ("my-group / my-subteam / my-project", "my-group/my-subteam/my-project"), - ], -) -def test_web_repository_path_normalizes_namespaced_name( - client: ApiClient, repository_name: str, expected_path: str -) -> None: - provider = _make_gitlab_provider(client, name=repository_name) +def test_web_repository_path_is_the_repository_path_not_its_name(client: ApiClient) -> None: + # Sentry's name for a GitLab repository is its display name, which differs from the path in more than spaces. + provider = _make_gitlab_provider(client, name="GitLab.org / GitLab FOSS", path="gitlab-org/gitlab-foss") + + assert provider.web_repository_path == "gitlab-org/gitlab-foss" + + +@pytest.mark.parametrize("path", [None, ""]) +def test_web_repository_path_needs_the_repository_path(client: ApiClient, path: str | None) -> None: + provider = _make_gitlab_provider(client, name="GitLab.org / GitLab FOSS", path=path) - assert provider.web_repository_path == expected_path + with pytest.raises(MissingRepositoryPath): + provider.get_commit_url("abc123") def test_web_repository_path_used_in_commit_and_mr_urls(client: ApiClient) -> None: - provider = _make_gitlab_provider(client, name="my-group / my-project") + provider = _make_gitlab_provider(client, name="My Group / My Project", path="my-group/my-project") assert provider.get_commit_url("abc123") == "https://gitlab.com/my-group/my-project/-/commit/abc123" assert provider.get_commits_url("abc123") == "https://gitlab.com/my-group/my-project/-/commits/abc123" diff --git a/tests/unit/test_rpc_server.py b/tests/unit/test_rpc_server.py index 62fea8dc..6dd01498 100644 --- a/tests/unit/test_rpc_server.py +++ b/tests/unit/test_rpc_server.py @@ -313,6 +313,17 @@ def test_web_base_url_none_when_not_set(self): decoded = deserialize_repository(serialize_repository(make_repository())) assert decoded["web_base_url"] is None + def test_path_round_trips(self): + repo = make_repository(provider_name="gitlab", path="gitlab-org/gitlab-foss") + decoded = deserialize_repository(serialize_repository(repo)) + assert decoded["path"] == "gitlab-org/gitlab-foss" + + def test_path_none_when_the_server_omits_it(self): + payload = json.loads(serialize_repository(make_repository())) + del payload["data"]["attributes"]["path"] + decoded = deserialize_repository(json.dumps(payload).encode()) + assert decoded["path"] is None + def test_installation_id_round_trips(self): repo = make_repository(provider_name="cursor_origin", installation_id="inst_01example") decoded = deserialize_repository(serialize_repository(repo))