diff --git a/src/sentry/preprod/models.py b/src/sentry/preprod/models.py index 192e2b905b941d..a9ed7faae406d0 100644 --- a/src/sentry/preprod/models.py +++ b/src/sentry/preprod/models.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from collections import defaultdict from enum import IntEnum import sentry_sdk @@ -142,23 +143,47 @@ def as_choices(cls) -> tuple[tuple[int, str], ...]: # The objectstore id of the app icon app_icon_id = models.CharField(max_length=255, null=True) - def get_sibling_artifacts_for_commit(self) -> models.QuerySet[PreprodArtifact]: + def get_sibling_artifacts_for_commit(self) -> list[PreprodArtifact]: """ - Get all artifacts for the same commit comparison (monorepo scenario). + Get sibling artifacts for the same commit, deduplicated by (app_id, artifact_type). + + When multiple artifacts exist for the same (app_id, artifact_type) combination + (e.g., due to reprocessing or CI retries), this method returns only one artifact + per combination to prevent duplicate rows in status checks: + - For the calling artifact's (app_id, artifact_type): Returns the calling artifact itself + - For other combinations: Returns the earliest (oldest) artifact for that combination + + Note: Deduplication by both app_id and artifact_type is necessary because + iOS and Android apps can share the same app_id (e.g., "com.example.app"). - Note: Always includes the calling artifact itself along with any siblings. Results are filtered by the current artifact's organization for security. Returns: - QuerySet of PreprodArtifact objects, ordered by app_id for stable results + List of PreprodArtifact objects, deduplicated by (app_id, artifact_type), + ordered by app_id """ if not self.commit_comparison: - return PreprodArtifact.objects.none() + return [] - return PreprodArtifact.objects.filter( + all_artifacts = PreprodArtifact.objects.filter( commit_comparison=self.commit_comparison, project__organization_id=self.project.organization_id, - ).order_by("app_id") + ).order_by("app_id", "artifact_type", "date_added") + + artifacts_by_key = defaultdict(list) + for artifact in all_artifacts: + key = (artifact.app_id, artifact.artifact_type) + artifacts_by_key[key].append(artifact) + + selected_artifacts = [] + for (app_id, artifact_type), artifacts in artifacts_by_key.items(): + if self.app_id == app_id and self.artifact_type == artifact_type: + selected_artifacts.append(self) + else: + selected_artifacts.append(artifacts[0]) + + selected_artifacts.sort(key=lambda a: a.app_id or "") + return selected_artifacts def get_base_artifact_for_commit( self, artifact_type: ArtifactType | None = None diff --git a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py index 439ce11c2870e3..d21673a18f1af4 100644 --- a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py +++ b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py @@ -1,9 +1,11 @@ from __future__ import annotations import uuid +from datetime import timedelta from unittest.mock import Mock, patch import responses +from django.utils import timezone from sentry.integrations.source_code_management.status_check import StatusCheckStatus from sentry.models.commitcomparison import CommitComparison @@ -727,3 +729,262 @@ def test_create_preprod_status_check_task_truncates_long_summary(self): assert summary_bytes <= 65535, f"Summary has {summary_bytes} bytes, exceeds GitHub limit" assert summary.endswith("..."), "Truncated summary should end with '...'" + + def test_sibling_deduplication_after_reprocessing(self): + """Test that get_sibling_artifacts_for_commit() deduplicates by (app_id, artifact_type). + + When artifacts are reprocessed (e.g., CI retry), new artifacts are created + with the same (app_id, artifact_type). This test verifies that the sibling lookup + returns only one artifact per (app_id, artifact_type) to prevent duplicate rows + in status checks. + """ + commit_comparison = CommitComparison.objects.create( + organization_id=self.organization.id, + head_sha="a" * 40, + base_sha="b" * 40, + provider="github", + head_repo_name="owner/repo", + base_repo_name="owner/repo", + head_ref="feature/reprocess-test", + base_ref="main", + ) + + ios_old = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.XCARCHIVE, + app_id="com.example.ios", + app_name="iOS App Old", + build_version="1.0.0", + build_number=1, + commit_comparison=commit_comparison, + ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=ios_old, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + android_old = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.AAB, + app_id="com.example.android", + app_name="Android App Old", + build_version="1.0.0", + build_number=1, + commit_comparison=commit_comparison, + ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=android_old, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + later_time = timezone.now() + timedelta(hours=1) + + ios_new = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.XCARCHIVE, + app_id="com.example.ios", + app_name="iOS App New", + build_version="1.0.0", + build_number=2, + commit_comparison=commit_comparison, + ) + ios_new.date_added = later_time + ios_new.save(update_fields=["date_added"]) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=ios_new, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + android_new = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.AAB, + app_id="com.example.android", + app_name="Android App New", + build_version="1.0.0", + build_number=2, + commit_comparison=commit_comparison, + ) + android_new.date_added = later_time + android_new.save(update_fields=["date_added"]) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=android_new, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + siblings_from_ios_new = list(ios_new.get_sibling_artifacts_for_commit()) + assert len(siblings_from_ios_new) == 2, ( + f"Expected 2 siblings (one per app_id), got {len(siblings_from_ios_new)}. " + f"Deduplication by app_id should prevent showing all 4 artifacts." + ) + + sibling_ids_from_ios_new = {s.id for s in siblings_from_ios_new} + assert ( + ios_new.id in sibling_ids_from_ios_new + ), "Triggering artifact (ios_new) should be included in its own app_id group" + assert ( + android_old.id in sibling_ids_from_ios_new + ), "For other app_ids, should use earliest artifact (android_old, not android_new)" + + siblings_from_android_new = list(android_new.get_sibling_artifacts_for_commit()) + assert len(siblings_from_android_new) == 2 + + sibling_ids_from_android_new = {s.id for s in siblings_from_android_new} + assert ( + android_new.id in sibling_ids_from_android_new + ), "Triggering artifact (android_new) should be included in its own app_id group" + assert ( + ios_old.id in sibling_ids_from_android_new + ), "For other app_ids, should use earliest artifact (ios_old, not ios_new)" + + siblings_from_ios_old = list(ios_old.get_sibling_artifacts_for_commit()) + assert len(siblings_from_ios_old) == 2 + sibling_ids_from_ios_old = {s.id for s in siblings_from_ios_old} + assert ios_old.id in sibling_ids_from_ios_old + assert android_old.id in sibling_ids_from_ios_old + + def test_sibling_deduplication_with_same_app_id_different_platforms(self): + """Test that iOS and Android builds with the same app_id are not deduplicated. + + Users can upload both Android and iOS builds with the same app_id (e.g., "com.example.app"). + This test verifies that the sibling lookup returns both platform artifacts even if they + share the same app_id, because deduplication is by (app_id, artifact_type). + """ + commit_comparison = CommitComparison.objects.create( + organization_id=self.organization.id, + head_sha="c" * 40, + base_sha="d" * 40, + provider="github", + head_repo_name="owner/repo", + base_repo_name="owner/repo", + head_ref="feature/cross-platform", + base_ref="main", + ) + + same_app_id = "com.example.multiplatform" + + ios_artifact = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.XCARCHIVE, + app_id=same_app_id, + app_name="Multiplatform App (iOS)", + build_version="1.0.0", + build_number=1, + commit_comparison=commit_comparison, + ) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=ios_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + android_artifact = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.AAB, + app_id=same_app_id, + app_name="Multiplatform App (Android)", + build_version="1.0.0", + build_number=1, + commit_comparison=commit_comparison, + ) + android_artifact.date_added = timezone.now() + timedelta(seconds=1) + android_artifact.save(update_fields=["date_added"]) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=android_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + siblings_from_ios = list(ios_artifact.get_sibling_artifacts_for_commit()) + assert len(siblings_from_ios) == 2, ( + f"Expected 2 siblings (iOS + Android with same app_id), got {len(siblings_from_ios)}. " + f"Both platforms should be included even with the same app_id." + ) + + sibling_ids_from_ios = {s.id for s in siblings_from_ios} + assert ( + ios_artifact.id in sibling_ids_from_ios + ), "iOS artifact should be included in siblings" + assert ( + android_artifact.id in sibling_ids_from_ios + ), "Android artifact should be included even with same app_id (different platform)" + + siblings_from_android = list(android_artifact.get_sibling_artifacts_for_commit()) + assert len(siblings_from_android) == 2 + + sibling_ids_from_android = {s.id for s in siblings_from_android} + assert android_artifact.id in sibling_ids_from_android + assert ios_artifact.id in sibling_ids_from_android + + later_time = timezone.now() + timedelta(hours=1) + ios_artifact_new = PreprodArtifact.objects.create( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + artifact_type=PreprodArtifact.ArtifactType.XCARCHIVE, + app_id=same_app_id, + app_name="Multiplatform App (iOS v2)", + build_version="1.0.0", + build_number=2, + commit_comparison=commit_comparison, + ) + ios_artifact_new.date_added = later_time + ios_artifact_new.save(update_fields=["date_added"]) + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=ios_artifact_new, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + siblings_from_ios_new = list(ios_artifact_new.get_sibling_artifacts_for_commit()) + assert len(siblings_from_ios_new) == 2, ( + f"Expected 2 siblings after iOS reprocessing, got {len(siblings_from_ios_new)}. " + f"Should show 1 iOS (newest) + 1 Android (only one)." + ) + + sibling_ids_from_ios_new = {s.id for s in siblings_from_ios_new} + assert ( + ios_artifact_new.id in sibling_ids_from_ios_new + ), "New iOS artifact should be included (triggering artifact)" + assert ( + android_artifact.id in sibling_ids_from_ios_new + ), "Original Android should still be included" + assert ( + ios_artifact.id not in sibling_ids_from_ios_new + ), "Old iOS artifact should be deduplicated (not the triggering artifact)"