Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 32 additions & 7 deletions src/sentry/preprod/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import logging
from collections import defaultdict
from enum import IntEnum

import sentry_sdk
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)"
Loading