diff --git a/pyproject.toml b/pyproject.toml index 8822ab3a908e56..cf615998200532 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -445,6 +445,7 @@ module = [ "sentry.integrations.msteams.handlers.*", "sentry.integrations.opsgenie.handlers.*", "sentry.integrations.pagerduty.*", + "sentry.integrations.perforce.*", "sentry.integrations.project_management.*", "sentry.integrations.repository.*", "sentry.integrations.services.*", diff --git a/src/sentry/integrations/perforce/client.py b/src/sentry/integrations/perforce/client.py index ffb199398c1b63..f4961ed2aaa935 100644 --- a/src/sentry/integrations/perforce/client.py +++ b/src/sentry/integrations/perforce/client.py @@ -1,9 +1,9 @@ from __future__ import annotations import logging -from collections.abc import Sequence +from collections.abc import Generator, Sequence from contextlib import contextmanager -from typing import Any +from typing import Any, TypedDict from P4 import P4, P4Exception @@ -22,6 +22,69 @@ logger = logging.getLogger(__name__) +# Default buffer size when fetching changelist ranges to ensure complete coverage +DEFAULT_REVISION_RANGE = 10 + + +class P4ChangeInfo(TypedDict): + """Type definition for Perforce changelist information.""" + + change: str + user: str + client: str + time: str + desc: str + + +class P4DepotInfo(TypedDict): + """Type definition for Perforce depot information.""" + + name: str + type: str + description: str + + +class P4UserInfo(TypedDict, total=False): + """Type definition for Perforce user information.""" + + email: str + full_name: str + username: str + + +class P4CommitInfo(TypedDict): + """Type definition for Sentry commit format.""" + + id: str + repository: str + author_email: str + author_name: str + message: str + timestamp: str + patch_set: list[Any] + + +class P4DepotPath: + """Encapsulates Perforce depot path logic.""" + + def __init__(self, path: str): + """ + Initialize depot path. + + Args: + path: Depot path (e.g., //depot or //depot/project) + """ + self.path = path + + def depot_name(self) -> str: + """ + Extract depot name from path. + + Returns: + Depot name (e.g., "depot" from "//depot/project") + """ + return self.path.strip("/").split("/")[0] + class PerforceClient(RepositoryClient, CommitContextClient): """ @@ -62,7 +125,7 @@ def __init__( self.ssl_fingerprint = metadata.get("ssl_fingerprint") @contextmanager - def _connect(self): + def _connect(self) -> Generator[P4]: """ Context manager for P4 connections with automatic cleanup. @@ -241,7 +304,7 @@ def build_depot_path(self, repo: Repository, path: str, stream: str | None = Non return full_path - def get_depots(self) -> list[dict[str, Any]]: + def get_depots(self) -> Sequence[P4DepotInfo]: """ List all depots accessible to the user. @@ -249,20 +312,20 @@ def get_depots(self) -> list[dict[str, Any]]: API docs: https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_depots.html Returns: - List of depot info dictionaries + Sequence of depot info dictionaries """ with self._connect() as p4: depots = p4.run("depots") return [ - { - "name": depot.get("name"), - "type": depot.get("type"), - "description": depot.get("desc", ""), - } + P4DepotInfo( + name=str(depot.get("name", "")), + type=str(depot.get("type", "")), + description=str(depot.get("desc", "")), + ) for depot in depots ] - def get_user(self, username: str) -> dict[str, Any] | None: + def get_user(self, username: str) -> P4UserInfo | None: """ Get user information from Perforce. @@ -286,29 +349,87 @@ def get_user(self, username: str) -> dict[str, Any] | None: # Check if user actually exists by verifying Update field is set if not user_info.get("Update"): return None - return { - "email": user_info.get("Email", ""), - "full_name": user_info.get("FullName", ""), - "username": user_info.get("User", username), - } + return P4UserInfo( + email=str(user_info.get("Email", "")), + full_name=str(user_info.get("FullName", "")), + username=str(user_info.get("User", username)), + ) # User not found - return None (not an error condition) return None def get_changes( - self, depot_path: str, max_changes: int = 20, start_cl: str | None = None - ) -> list[dict[str, Any]]: + self, + depot_path: str, + max_changes: int = 20, + start_cl: int | None = None, + end_cl: int | None = None, + ) -> Sequence[P4ChangeInfo]: """ Get changelists for a depot path. + Uses p4 changes command to list changelists. + API docs: https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_changes.html + Args: depot_path: Depot path (e.g., //depot/main/...) - max_changes: Maximum number of changes to return - start_cl: Starting changelist number + max_changes: Maximum number of changes to return when start_cl/end_cl not specified + start_cl: Starting changelist number (exclusive) - returns changes > start_cl. Must be int. + end_cl: Ending changelist number (inclusive) - returns changes <= end_cl. Must be int. Returns: - List of changelist dictionaries + Sequence of changelist dictionaries in range (start_cl, end_cl] + + Raises: + TypeError: If start_cl or end_cl are not integers """ - return [] + with self._connect() as p4: + # Validate types - changelists must be integers + if start_cl is not None and not isinstance(start_cl, int): + raise TypeError( + f"start_cl must be an integer or None, got {type(start_cl).__name__}" + ) + if end_cl is not None and not isinstance(end_cl, int): + raise TypeError(f"end_cl must be an integer or None, got {type(end_cl).__name__}") + + start_cl_num = start_cl + end_cl_num = end_cl + + # Calculate how many changes to fetch based on range + if start_cl_num is not None and end_cl_num is not None: + # Fetch enough to cover the range, adding buffer for safety + range_size = abs(end_cl_num - start_cl_num) + DEFAULT_REVISION_RANGE + fetch_limit = max(range_size, max_changes) + else: + fetch_limit = max_changes + + args = ["-m", str(fetch_limit), "-l"] + + # P4 -e flag: return changes at or before specified changelist (upper bound) + # Use it for end_cl (inclusive upper bound) + if end_cl_num is not None: + args.extend(["-e", str(end_cl_num)]) + + args.append(depot_path) + + changes = p4.run("changes", *args) + + # Client-side filter for start_cl (exclusive lower bound) + # Filter out changes <= start_cl to get changes > start_cl + if start_cl_num is not None: + changes = [ + c for c in changes if c.get("change") and int(c["change"]) > start_cl_num + ] + + return [ + P4ChangeInfo( + change=str(change.get("change", "")), + user=str(change.get("user", "")), + client=str(change.get("client", "")), + time=str(change.get("time", "")), + desc=str(change.get("desc", "")), + ) + for change in changes + ] def get_blame_for_files( self, files: Sequence[SourceLineInfo], extra: dict[str, Any] @@ -331,7 +452,7 @@ def get_file( ) -> str: """ Get file contents from Perforce depot. - Required by abstract base class but not used (CODEOWNERS feature removed). + Required by abstract base class but not used (CODEOWNERS). """ raise NotImplementedError("get_file is not supported for Perforce") diff --git a/src/sentry/integrations/perforce/integration.py b/src/sentry/integrations/perforce/integration.py index 93e85eeac0e1cb..bf034e292574bf 100644 --- a/src/sentry/integrations/perforce/integration.py +++ b/src/sentry/integrations/perforce/integration.py @@ -152,11 +152,11 @@ class PerforceInstallationForm(forms.Form): assume_scheme="https", ) - def clean_p4port(self): + def clean_p4port(self) -> str: """Strip off trailing / and whitespace from p4port""" return self.cleaned_data["p4port"].strip().rstrip("/") - def clean_web_url(self): + def clean_web_url(self) -> str: """Strip off trailing / from web_url""" web_url = self.cleaned_data.get("web_url", "") if web_url: @@ -541,7 +541,7 @@ class PerforceIntegrationProvider(IntegrationProvider): ) requires_feature_flag = True - def get_pipeline_views(self) -> Sequence[PipelineView]: + def get_pipeline_views(self) -> Sequence[PipelineView[IntegrationPipeline]]: """Get pipeline views for installation flow.""" return [PerforceInstallationView()] diff --git a/src/sentry/integrations/perforce/repository.py b/src/sentry/integrations/perforce/repository.py index b17bb24d42a5a7..f2235b4649f8b4 100644 --- a/src/sentry/integrations/perforce/repository.py +++ b/src/sentry/integrations/perforce/repository.py @@ -2,14 +2,24 @@ import logging from collections.abc import Mapping, Sequence +from datetime import datetime, timezone from typing import Any +from sentry.integrations.perforce.client import ( + P4ChangeInfo, + P4CommitInfo, + P4DepotPath, + P4UserInfo, + PerforceClient, +) +from sentry.integrations.services.integration import integration_service from sentry.models.organization import Organization from sentry.models.pullrequest import PullRequest from sentry.models.repository import Repository from sentry.organizations.services.organization.model import RpcOrganization from sentry.plugins.providers import IntegrationRepositoryProvider from sentry.plugins.providers.integration_repository import RepositoryConfig +from sentry.shared_integrations.exceptions import IntegrationError logger = logging.getLogger(__name__) @@ -20,6 +30,30 @@ class PerforceRepositoryProvider(IntegrationRepositoryProvider): name = "Perforce" repo_provider = "perforce" + def _get_client_from_repo(self, repo: Repository) -> PerforceClient: + """ + Get Perforce client from repository. + + Args: + repo: Repository instance + + Returns: + PerforceClient instance + + Raises: + NotImplementedError: If integration not found + """ + integration_id = repo.integration_id + if integration_id is None: + raise NotImplementedError("Perforce integration requires an integration_id") + + integration = integration_service.get_integration(integration_id=integration_id) + if integration is None: + raise NotImplementedError("Integration not found") + + installation = integration.get_installation(organization_id=repo.organization_id) + return installation.get_client() + def get_repository_data( self, organization: Organization, config: dict[str, Any] ) -> Mapping[str, Any]: @@ -33,7 +67,39 @@ def get_repository_data( Returns: Repository configuration dictionary """ - return {} + installation = self.get_installation(config.get("installation"), organization.id) + client = installation.get_client() + + depot_path = P4DepotPath(config["identifier"]) # e.g., //depot or //depot/project + + # Validate depot exists and is accessible + try: + depots = client.get_depots() + + if not any(d["name"] == depot_path.depot_name() for d in depots): + raise IntegrationError( + f"Depot not found or no access: {depot_path.path}. Available depots: {[d['name'] for d in depots]}" + ) + + except IntegrationError: + # Re-raise validation errors so user sees them + raise + except Exception as e: + # Log and re-raise connection/P4 errors + # We cannot create a repository if we can't validate the depot exists + logger.exception( + "perforce.get_repository_data.depot_validation_failed", + extra={"depot_path": depot_path.path}, + ) + raise IntegrationError( + f"Failed to validate depot: {depot_path.path}. " + f"Please check your Perforce server connection and credentials." + ) from e + + config["external_id"] = depot_path.path + config["integration_id"] = installation.model.id + + return config def build_repository_config( self, organization: RpcOrganization, data: dict[str, Any] @@ -48,51 +114,202 @@ def build_repository_config( Returns: Repository configuration """ + depot_path = data["identifier"] + return { - "name": "", - "external_id": "", - "url": "", - "config": {}, - "integration_id": 0, + "name": depot_path, + "external_id": data["external_id"], + "url": f"p4:{depot_path}", + "config": { + "depot_path": depot_path, + "name": depot_path, + }, + "integration_id": data["integration_id"], } - def compare_commits( - self, repo: Repository, start_sha: str | None, end_sha: str - ) -> Sequence[Mapping[str, Any]]: + def _extract_commit_info( + self, + change: P4ChangeInfo, + depot_path: str, + client: PerforceClient, + user_cache: dict[str, P4UserInfo | None], + ) -> P4CommitInfo: """ - Compare commits (changelists) between two versions. + Extract commit info from a Perforce changelist. Args: - repo: Repository instance - start_sha: Starting changelist number (or None for initial) - end_sha: Ending changelist number + change: Perforce changelist info + depot_path: Depot path for repository field + client: Perforce client for user lookups + user_cache: Cache of user info to avoid redundant lookups Returns: - List of changelist dictionaries + Commit info in Sentry format """ - return [] + # Handle potentially null/invalid time field + time_value = change.get("time") or 0 + try: + time_int = int(time_value) + except (TypeError, ValueError) as e: + logger.warning( + "perforce.format_commits.invalid_time_value", + extra={ + "changelist": change.get("change"), + "time_value": time_value, + "error": str(e), + }, + ) + time_int = 0 + + # Convert Unix timestamp to ISO 8601 format + timestamp = datetime.fromtimestamp(time_int, tz=timezone.utc).isoformat() + + # Get user information from Perforce + username = change.get("user", "unknown") + author_email = f"{username}@perforce" + author_name = username + + # Fetch user info if not in cache (skip "unknown" placeholder) + if username != "unknown" and username not in user_cache: + try: + user_cache[username] = client.get_user(username) + except Exception as e: + # Log user lookup failures but don't fail the entire commit processing + logger.warning( + "perforce.format_commits.user_lookup_failed", + extra={ + "changelist": change.get("change"), + "username": username, + "error": str(e), + "error_type": type(e).__name__, + }, + ) + # Cache None to avoid repeated failed lookups for the same user + user_cache[username] = None + + user_info = user_cache.get(username) + if user_info: + # Use actual email from Perforce if available + if user_info.get("email"): + author_email = user_info["email"] + # Use full name from Perforce if available + if user_info.get("full_name"): + author_name = user_info["full_name"] + + return P4CommitInfo( + id=str(change["change"]), + repository=depot_path, + author_email=author_email, + author_name=author_name, + message=change.get("desc", ""), + timestamp=timestamp, + patch_set=[], + ) def _format_commits( - self, changelists: list[dict[str, Any]], depot_path: str - ) -> Sequence[Mapping[str, Any]]: + self, changelists: Sequence[P4ChangeInfo], depot_path: str, client: PerforceClient + ) -> list[P4CommitInfo]: """ Format Perforce changelists into Sentry commit format. Args: changelists: List of changelist dictionaries from P4 depot_path: Depot path + client: Perforce client instance Returns: List of commits in Sentry format """ - return [] + commits: list[P4CommitInfo] = [] + user_cache: dict[str, P4UserInfo | None] = {} + + for change in changelists: + try: + commit = self._extract_commit_info(change, depot_path, client, user_cache) + commits.append(commit) + except (KeyError, TypeError) as e: + logger.warning( + "perforce.format_commits.invalid_changelist_data", + extra={ + "changelist": change.get("change"), + "depot_path": depot_path, + "error": str(e), + "error_type": type(e).__name__, + }, + ) + continue + + return commits + + def compare_commits( + self, repo: Repository, start_sha: str | None, end_sha: str + ) -> Sequence[Mapping[str, Any]]: + """ + Compare commits (changelists) between two versions. + + Used for release tracking when users manually specify changelist numbers. + + Args: + repo: Repository instance + start_sha: Starting changelist number (or None for initial) + end_sha: Ending changelist number + + Returns: + List of changelist dictionaries in Sentry commit format + """ + client = self._get_client_from_repo(repo) + depot_path = repo.config.get("depot_path", repo.name) + + try: + # Convert changelist numbers from strings to integers + # In Perforce, SHAs are changelist numbers + start_cl = int(start_sha) if start_sha else None + end_cl = int(end_sha) + + # Get changelists in range (start_sha, end_sha] + changes = client.get_changes( + f"{depot_path}/...", + max_changes=20, + start_cl=start_cl, + end_cl=end_cl, + ) + + return self._format_commits(changes, depot_path, client) + + except (ValueError, TypeError) as e: + # Log conversion errors for debugging + logger.exception( + "perforce.compare_commits.invalid_changelist", + extra={ + "start_sha": start_sha, + "end_sha": end_sha, + "depot_path": depot_path, + "repo_id": repo.id, + "error": str(e), + }, + ) + return [] + except Exception as e: + logger.exception( + "perforce.compare_commits.failed", + extra={ + "start_sha": start_sha, + "end_sha": end_sha, + "depot_path": depot_path, + "repo_id": repo.id, + "integration_id": repo.integration_id, + "error": str(e), + "error_type": type(e).__name__, + }, + ) + return [] - def pull_request_url(self, repo: Repository, pull_request: PullRequest) -> str: + def pull_request_url(self, repo: Repository, pull_request: PullRequest) -> str | None: """ Get URL for pull request. - Perforce doesn't have native PRs, but might integrate with Swarm. + Perforce doesn't have native pull requests. """ - return "" + return None def repository_external_slug(self, repo: Repository) -> str: """Get external slug for repository.""" diff --git a/tests/sentry/integrations/perforce/test_code_mapping.py b/tests/sentry/integrations/perforce/test_code_mapping.py new file mode 100644 index 00000000000000..621d2138209494 --- /dev/null +++ b/tests/sentry/integrations/perforce/test_code_mapping.py @@ -0,0 +1,455 @@ +from unittest.mock import patch + +from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig +from sentry.integrations.perforce.integration import ( + PerforceIntegration, + PerforceIntegrationProvider, +) +from sentry.issues.auto_source_code_config.code_mapping import ( + convert_stacktrace_frame_path_to_source_path, +) +from sentry.models.repository import Repository +from sentry.testutils.cases import IntegrationTestCase +from sentry.utils.event_frames import EventFrame + + +class PerforceCodeMappingTest(IntegrationTestCase): + """Tests for code mapping integration with Perforce""" + + provider = PerforceIntegrationProvider + installation: PerforceIntegration + + def setUp(self): + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test", + metadata={}, + ) + self.installation = self.integration.get_installation(self.organization.id) + self.project = self.create_project(organization=self.organization) + self.org_integration = self.integration.organizationintegration_set.first() + assert self.org_integration is not None + + def tearDown(self): + super().tearDown() + + def test_code_mapping_depot_root_to_slash(self): + """ + Test code mapping: depot/ -> / + This is the correct mapping for Perforce where depot name is part of path. + """ + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Test Python SDK path: depot/app/services/processor.py + frame = EventFrame( + filename="depot/app/services/processor.py", + abs_path="depot/app/services/processor.py", + ) + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="python", sdk_name="sentry.python" + ) + + # Should strip "depot/" leaving "app/services/processor.py" + assert result == "app/services/processor.py" + + def test_code_mapping_with_symbolic_revision_syntax(self): + """ + Test code mapping with Symbolic's @revision syntax. + The @revision should be preserved in the output. + """ + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Test C++ path from Symbolic: depot/game/src/main.cpp@42 + frame = EventFrame( + filename="depot/game/src/main.cpp@42", abs_path="depot/game/src/main.cpp@42" + ) + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="native", sdk_name="sentry.native" + ) + + # Should strip "depot/" and preserve "@42" + assert result == "game/src/main.cpp@42" + + def test_code_mapping_multiple_depots(self): + """Test code mappings for multiple depots (depot and myproject)""" + depot_repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + myproject_repo = Repository.objects.create( + name="//myproject", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//myproject"}, + ) + + depot_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=depot_repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + myproject_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=myproject_repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="myproject/", + source_root="/", + default_branch=None, + ) + + # Test depot path + frame1 = EventFrame( + filename="depot/app/services/processor.py", + abs_path="depot/app/services/processor.py", + ) + + result1 = convert_stacktrace_frame_path_to_source_path( + frame=frame1, code_mapping=depot_mapping, platform="python", sdk_name="sentry.python" + ) + assert result1 == "app/services/processor.py" + + # Test myproject path + frame2 = EventFrame( + filename="myproject/app/services/handler.py", + abs_path="myproject/app/services/handler.py", + ) + + result2 = convert_stacktrace_frame_path_to_source_path( + frame=frame2, + code_mapping=myproject_mapping, + platform="python", + sdk_name="sentry.python", + ) + assert result2 == "app/services/handler.py" + + def test_code_mapping_no_match_different_depot(self): + """Test that code mapping doesn't match paths from different depots""" + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Try to match a path from different depot + frame = EventFrame( + filename="myproject/app/services/processor.py", + abs_path="myproject/app/services/processor.py", + ) + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="python", sdk_name="sentry.python" + ) + + # Should not match + assert result is None + + def test_code_mapping_abs_path_fallback(self): + """Test that code mapping works with abs_path when filename is just basename""" + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Some platforms only provide basename in filename + frame = EventFrame(filename="processor.py", abs_path="depot/app/services/processor.py") + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="python", sdk_name="sentry.python" + ) + + # Should use abs_path and strip "depot/" + assert result == "app/services/processor.py" + + def test_code_mapping_nested_depot_paths(self): + """Test code mapping with nested depot paths""" + repo = Repository.objects.create( + name="//depot/game/project", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot/game/project"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/game/project/", + source_root="/", + default_branch=None, + ) + + frame = EventFrame( + filename="depot/game/project/src/main.cpp", + abs_path="depot/game/project/src/main.cpp", + ) + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="native", sdk_name="sentry.native" + ) + + assert result == "src/main.cpp" + + def test_code_mapping_preserves_windows_backslash_conversion(self): + """ + Test that code mapping handles Windows-style paths. + + Note: The generic code_mapping system does not automatically convert + backslashes to forward slashes. SDKs should normalize paths before + sending to Sentry. This test documents current behavior. + """ + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Windows-style path with backslashes + frame = EventFrame( + filename="depot\\app\\services\\processor.cpp", + abs_path="depot\\app\\services\\processor.cpp", + ) + + result = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=code_mapping, platform="native", sdk_name="sentry.native" + ) + + # Generic code mapping doesn't normalize backslashes - returns None + # SDKs should normalize paths to forward slashes before sending + assert result is None + + +class PerforceEndToEndCodeMappingTest(IntegrationTestCase): + """End-to-end tests for code mapping + format_source_url flow""" + + provider = PerforceIntegrationProvider + + def setUp(self): + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test", + metadata={}, + ) + self.installation = self.integration.get_installation(self.organization.id) + self.project = self.create_project(organization=self.organization) + self.org_integration = self.integration.organizationintegration_set.first() + assert self.org_integration is not None + + self.repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + self.code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=self.repo, + organization_integration_id=self.org_integration.id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Mock the Perforce client's check_file to avoid actual P4 connection + self.check_file_patcher = patch( + "sentry.integrations.perforce.client.PerforceClient.check_file", + return_value={"depotFile": "//depot/mock"}, + ) + self.check_file_patcher.start() + + def tearDown(self): + self.check_file_patcher.stop() + super().tearDown() + + def test_python_sdk_path_full_flow(self): + """Test full flow: Python SDK -> code mapping -> format_source_url""" + # 1. Python SDK sends this path + frame = EventFrame( + filename="depot/app/services/processor.py", + abs_path="depot/app/services/processor.py", + ) + + # 2. Code mapping transforms it + mapped_path = convert_stacktrace_frame_path_to_source_path( + frame=frame, + code_mapping=self.code_mapping, + platform="python", + sdk_name="sentry.python", + ) + assert mapped_path == "app/services/processor.py" + + # 3. format_source_url creates final URL + url = self.installation.format_source_url(repo=self.repo, filepath=mapped_path, branch=None) + assert url == "p4://depot/app/services/processor.py" + + def test_symbolic_cpp_path_full_flow(self): + """Test full flow: Symbolic C++ -> code mapping -> format_source_url""" + # 1. Symbolic transformer sends this path + frame = EventFrame( + filename="depot/game/src/main.cpp@42", abs_path="depot/game/src/main.cpp@42" + ) + + # 2. Code mapping transforms it (use existing code_mapping from setUp) + mapped_path = convert_stacktrace_frame_path_to_source_path( + frame=frame, code_mapping=self.code_mapping, platform="native", sdk_name="sentry.native" + ) + assert mapped_path == "game/src/main.cpp@42" + + # 3. format_source_url creates final URL (preserves @42) + url = self.installation.format_source_url(repo=self.repo, filepath=mapped_path, branch=None) + assert url == "p4://depot/game/src/main.cpp@42" + + def test_full_flow_with_web_viewer(self): + """Test full flow with P4Web viewer configuration""" + integration_with_web = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test-web-flow", + metadata={ + "p4port": "ssl:perforce.example.com:1666", + "user": "testuser", + "password": "testpass", + "auth_type": "password", + "web_url": "https://p4web.example.com", + }, + ) + installation: PerforceIntegration = integration_with_web.get_installation(self.organization.id) # type: ignore[assignment] + + # Create repo with web viewer integration + repo_web = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=integration_with_web.id, + config={"depot_path": "//depot"}, + ) + + # Use a new project to avoid unique constraint on (project_id, stack_root) + project_web = self.create_project(organization=self.organization) + + org_integration_web = integration_with_web.organizationintegration_set.first() + assert org_integration_web is not None + + code_mapping_web = RepositoryProjectPathConfig.objects.create( + project=project_web, + organization_id=self.organization.id, + repository=repo_web, + organization_integration_id=org_integration_web.id, + integration_id=integration_with_web.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Python SDK path with #revision from Symbolic + frame = EventFrame( + filename="depot/app/services/processor.py#42", + abs_path="depot/app/services/processor.py#42", + ) + + # Code mapping + mapped_path = convert_stacktrace_frame_path_to_source_path( + frame=frame, + code_mapping=code_mapping_web, + platform="python", + sdk_name="sentry.python", + ) + + # format_source_url with web viewer (revision extracted from filename) + assert mapped_path is not None + url = installation.format_source_url(repo=repo_web, filepath=mapped_path, branch=None) + + # Swarm format: /files/?v= + assert url == "https://p4web.example.com/files//depot/app/services/processor.py?v=42" diff --git a/tests/sentry/integrations/perforce/test_integration.py b/tests/sentry/integrations/perforce/test_integration.py index 1a957e2c6a0c95..69f1f08dd4ee3c 100644 --- a/tests/sentry/integrations/perforce/test_integration.py +++ b/tests/sentry/integrations/perforce/test_integration.py @@ -501,7 +501,7 @@ def test_integration_lifecycle(self): # Test get_organization_config returns form schema org_config = installation.get_organization_config() - assert len(org_config) == 7 # 7 configuration fields (added auth_type) + assert len(org_config) == 7 # 7 configuration fields field_names = {field["name"] for field in org_config} assert field_names == { "p4port", @@ -515,9 +515,13 @@ def test_integration_lifecycle(self): # Verify field types field_types = {field["name"]: field["type"] for field in org_config} - assert field_types["password"] == "secret" assert field_types["p4port"] == "string" assert field_types["user"] == "string" + assert field_types["auth_type"] == "choice" + assert field_types["password"] == "secret" + assert field_types["ssl_fingerprint"] == "string" + assert field_types["client"] == "string" + assert field_types["web_url"] == "string" # Test get_config_data returns current values config_data = installation.get_config_data() diff --git a/tests/sentry/integrations/perforce/test_stacktrace_link.py b/tests/sentry/integrations/perforce/test_stacktrace_link.py new file mode 100644 index 00000000000000..715c19ae50b47e --- /dev/null +++ b/tests/sentry/integrations/perforce/test_stacktrace_link.py @@ -0,0 +1,477 @@ +from unittest.mock import patch + +from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig +from sentry.integrations.perforce.integration import PerforceIntegrationProvider +from sentry.integrations.utils.stacktrace_link import get_stacktrace_config +from sentry.issues.endpoints.project_stacktrace_link import StacktraceLinkContext +from sentry.models.repository import Repository +from sentry.testutils.cases import IntegrationTestCase + + +class PerforceStacktraceLinkTest(IntegrationTestCase): + """Tests for Perforce stacktrace link generation""" + + provider = PerforceIntegrationProvider + + def setUp(self): + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test", + metadata={}, + ) + self.installation = self.integration.get_installation(self.organization.id) + self.project = self.create_project(organization=self.organization) + + self.repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + self.code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=self.repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Mock the Perforce client's check_file to avoid actual P4 connection + self.check_file_patcher = patch( + "sentry.integrations.perforce.client.PerforceClient.check_file", + return_value={"depotFile": "//depot/mock"}, + ) + self.check_file_patcher.start() + + def tearDown(self): + self.check_file_patcher.stop() + super().tearDown() + + def test_get_stacktrace_config_python_path(self): + """Test stacktrace link generation for Python SDK path""" + ctx: StacktraceLinkContext = { + "file": "depot/app/services/processor.py", + "filename": "depot/app/services/processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + assert "//depot/app/services/processor.py" in result["source_url"] + assert result["error"] is None + assert result["src_path"] == "app/services/processor.py" + + def test_get_stacktrace_config_cpp_path_with_revision(self): + """Test stacktrace link generation for C++ path with @revision""" + ctx: StacktraceLinkContext = { + "file": "depot/game/src/main.cpp@42", + "filename": "depot/game/src/main.cpp@42", + "abs_path": "depot/game/src/main.cpp@42", + "platform": "native", + "sdk_name": "sentry.native", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + # URL will be encoded: p4://depot/game/src/main.cpp%4042 + assert "depot/game/src/main.cpp" in result["source_url"] + assert result["error"] is None + assert result["src_path"] == "game/src/main.cpp@42" + + def test_get_stacktrace_config_no_matching_code_mapping(self): + """Test stacktrace link when no code mapping matches""" + ctx: StacktraceLinkContext = { + "file": "other/app/services/processor.py", + "filename": "other/app/services/processor.py", + "abs_path": "other/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping], ctx) + + assert result["source_url"] is None + assert result["error"] == "stack_root_mismatch" + assert result["src_path"] is None + + def test_get_stacktrace_config_multiple_code_mappings(self): + """Test stacktrace link with multiple code mappings""" + # Add another depot mapping + myproject_repo = Repository.objects.create( + name="//myproject", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//myproject"}, + ) + + myproject_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=myproject_repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="myproject/", + source_root="/", + default_branch=None, + ) + + # Test with myproject path + ctx: StacktraceLinkContext = { + "file": "myproject/app/services/handler.py", + "filename": "myproject/app/services/handler.py", + "abs_path": "myproject/app/services/handler.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping, myproject_mapping], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + assert "//myproject/app/services/handler.py" in result["source_url"] + assert result["src_path"] == "app/services/handler.py" + + def test_get_stacktrace_config_with_web_viewer(self): + """Test stacktrace link with P4Web viewer""" + integration_with_web = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test-web-link", + metadata={ + "web_url": "https://p4web.example.com", + }, + ) + + # Create new code mapping with new integration + # Use different project to avoid unique constraint + project_web = self.create_project(organization=self.organization) + + repo_web = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=integration_with_web.id, + config={"depot_path": "//depot"}, + ) + + org_integration = integration_with_web.organizationintegration_set.first() + assert org_integration is not None + code_mapping_web = RepositoryProjectPathConfig.objects.create( + project=project_web, + organization_id=self.organization.id, + repository=repo_web, + organization_integration_id=org_integration.id, + integration_id=integration_with_web.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + ctx: StacktraceLinkContext = { + "file": "depot/app/services/processor.py", + "filename": "depot/app/services/processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([code_mapping_web], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + # Swarm format uses /files/ prefix + assert ( + "https://p4web.example.com/files//depot/app/services/processor.py" + in result["source_url"] + ) + + def test_get_stacktrace_config_abs_path_fallback(self): + """Test stacktrace link uses abs_path when filename is just basename""" + ctx: StacktraceLinkContext = { + "file": "processor.py", + "filename": "processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + assert "//depot/app/services/processor.py" in result["source_url"] + assert result["src_path"] == "app/services/processor.py" + + def test_get_stacktrace_config_iteration_count(self): + """Test that iteration_count is incremented only for matching mappings""" + # Add a non-matching mapping + other_repo = Repository.objects.create( + name="//other", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//other"}, + ) + + other_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=other_repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="other/", + source_root="/", + default_branch=None, + ) + + ctx: StacktraceLinkContext = { + "file": "depot/app/services/processor.py", + "filename": "depot/app/services/processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([other_mapping, self.code_mapping], ctx) + + # iteration_count should be 1 (only depot mapping matched) + assert result["iteration_count"] == 1 + assert result["source_url"] is not None + + def test_get_stacktrace_config_stops_on_first_match(self): + """Test that iteration stops after first successful match""" + # Add another depot mapping (shouldn't be checked if first matches) + # Use different project to avoid unique constraint + project2 = self.create_project(organization=self.organization) + + myproject_repo = Repository.objects.create( + name="//myproject", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//myproject"}, + ) + + myproject_mapping = RepositoryProjectPathConfig.objects.create( + project=project2, + organization_id=self.organization.id, + repository=myproject_repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="depot/", # Same stack_root as depot mapping (but different project) + source_root="/", + default_branch=None, + ) + + ctx: StacktraceLinkContext = { + "file": "depot/app/services/processor.py", + "filename": "depot/app/services/processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([self.code_mapping, myproject_mapping], ctx) + + # Should stop after first match (depot mapping) + assert result["iteration_count"] == 1 + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + assert "//depot/app/services/processor.py" in result["source_url"] + + +class PerforceStacktraceLinkEdgeCasesTest(IntegrationTestCase): + """Edge case tests for Perforce stacktrace links""" + + provider = PerforceIntegrationProvider + + def setUp(self): + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="perforce", + name="Perforce", + external_id="perforce-test", + metadata={}, + ) + self.project = self.create_project(organization=self.organization) + + # Mock the Perforce client's check_file to avoid actual P4 connection + self.check_file_patcher = patch( + "sentry.integrations.perforce.client.PerforceClient.check_file", + return_value={"depotFile": "//depot/mock"}, + ) + self.check_file_patcher.start() + + def tearDown(self): + self.check_file_patcher.stop() + super().tearDown() + + def test_stacktrace_link_empty_stack_root(self): + """Test stacktrace link with empty stack_root (shouldn't match anything)""" + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="", + source_root="/", + default_branch=None, + ) + + ctx: StacktraceLinkContext = { + "file": "depot/app/services/processor.py", + "filename": "depot/app/services/processor.py", + "abs_path": "depot/app/services/processor.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([code_mapping], ctx) + + # Empty stack_root should match any path + assert result["source_url"] is not None + + def test_stacktrace_link_with_special_characters_in_path(self): + """Test stacktrace link with special characters in file path""" + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + # Path with spaces and special chars + ctx: StacktraceLinkContext = { + "file": "depot/app/my services/processor-v2.py", + "filename": "depot/app/my services/processor-v2.py", + "abs_path": "depot/app/my services/processor-v2.py", + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([code_mapping], ctx) + + assert result["source_url"] is not None + assert result["src_path"] == "app/my services/processor-v2.py" + + def test_stacktrace_link_deeply_nested_path(self): + """Test stacktrace link with very deeply nested path""" + repo = Repository.objects.create( + name="//depot", + organization_id=self.organization.id, + integration_id=self.integration.id, + config={"depot_path": "//depot"}, + ) + + code_mapping = RepositoryProjectPathConfig.objects.create( + project=self.project, + organization_id=self.organization.id, + repository=repo, + organization_integration_id=self.integration.organizationintegration_set.first().id, + integration_id=self.integration.id, + stack_root="depot/", + source_root="/", + default_branch=None, + ) + + deep_path = "depot/" + "/".join([f"level{i}" for i in range(20)]) + "/file.py" + + ctx: StacktraceLinkContext = { + "file": deep_path, + "filename": deep_path, + "abs_path": deep_path, + "platform": "python", + "sdk_name": "sentry.python", + "commit_id": None, + "group_id": None, + "line_no": None, + "module": None, + "package": None, + } + + result = get_stacktrace_config([code_mapping], ctx) + + assert result["source_url"] is not None + assert isinstance(result["source_url"], str) + assert "//depot/" in result["source_url"]