Skip to content

Commit 779eb11

Browse files
fix(difs): Use PROTECT for shared debug files
`ProjectDebugFile.file` can now be shared by multiple rows. Update the Django relation from `CASCADE` to `PROTECT` so ORM deletes match the intended ownership model. Keep deleting the backing `File` as a best-effort cleanup step after deleting a `ProjectDebugFile`, but leave it in place when another `ProjectDebugFile` still references it. This prepares us for [#106947](#106947), where aliasing is a concrete use case for shared `File` rows. The database should already reject deleting a referenced `File`; this change makes Django raise `ProtectedError` instead of relying on a DB-level failure path.
1 parent c518842 commit 779eb11

4 files changed

Lines changed: 79 additions & 4 deletions

File tree

migrations_lockfile.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ replays: 0007_organizationmember_replay_access
3333

3434
seer: 0002_add_default_coding_agent
3535

36-
sentry: 1050_projectkeymapping_uniq_constraint
36+
sentry: 1051_protect_projectdebugfile_file
3737

3838
social_auth: 0003_social_auth_json_field
3939

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Generated by Django 5.2.11 on 2026-03-11 11:29
2+
3+
import django.db.models.deletion
4+
import sentry.db.models.fields.foreignkey
5+
from django.db import migrations
6+
7+
from sentry.new_migrations.migrations import CheckedMigration
8+
9+
10+
class Migration(CheckedMigration):
11+
# This flag is used to mark that a migration shouldn't be automatically run in production.
12+
# This should only be used for operations where it's safe to run the migration after your
13+
# code has deployed. So this should not be used for most operations that alter the schema
14+
# of a table.
15+
# Here are some things that make sense to mark as post deployment:
16+
# - Large data migrations. Typically we want these to be run manually so that they can be
17+
# monitored and not block the deploy for a long period of time while they run.
18+
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
19+
# run this outside deployments so that we don't block them. Note that while adding an index
20+
# is a schema change, it's completely safe to run the operation after the code has deployed.
21+
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
22+
23+
is_post_deployment = False
24+
25+
dependencies = [
26+
("sentry", "1050_projectkeymapping_uniq_constraint"),
27+
]
28+
29+
operations = [
30+
migrations.AlterField(
31+
model_name="projectdebugfile",
32+
name="file",
33+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
34+
on_delete=django.db.models.deletion.PROTECT, to="sentry.file"
35+
),
36+
),
37+
]

src/sentry/models/debugfile.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from typing import TYPE_CHECKING, Any, BinaryIO, ClassVar
1616

1717
from django.db import models
18-
from django.db.models import Q
18+
from django.db.models import ProtectedError, Q
1919
from django.db.models.functions import Now
2020
from django.utils import timezone
2121
from symbolic.debuginfo import Archive, BcSymbolMap, Object, UuidMapping, normalize_debug_id
@@ -119,7 +119,7 @@ def find_by_debug_ids(
119119
class ProjectDebugFile(Model):
120120
__relocation_scope__ = RelocationScope.Excluded
121121

122-
file = FlexibleForeignKey("sentry.File")
122+
file = FlexibleForeignKey("sentry.File", on_delete=models.PROTECT)
123123
checksum = models.CharField(max_length=40, null=True, db_index=True)
124124
object_name = models.TextField()
125125
cpu_name = models.CharField(max_length=40)
@@ -195,7 +195,16 @@ def features(self) -> frozenset[str]:
195195

196196
def delete(self, *args: Any, **kwargs: Any) -> tuple[int, dict[str, int]]:
197197
ret = super().delete(*args, **kwargs)
198-
self.file.delete()
198+
199+
# If another debug file row still references this File, keep the File.
200+
# Concurrent last-reference deletes can still leave an unreferenced File
201+
# row behind, but no surviving ProjectDebugFile should point to a deleted
202+
# File.
203+
try:
204+
self.file.delete()
205+
except ProtectedError:
206+
pass
207+
199208
return ret
200209

201210

tests/sentry/models/test_debugfile.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from typing import Any
99

1010
import pytest
11+
from django.core.files.base import ContentFile
1112
from django.core.files.uploadedfile import SimpleUploadedFile
1213
from django.urls import reverse
1314

@@ -43,6 +44,34 @@ def test_delete_dif(self) -> None:
4344
assert not ProjectDebugFile.objects.filter(id=dif_id).exists()
4445
assert not File.objects.filter(id=dif.file.id).exists()
4546

47+
def test_delete_shared_file_keeps_file_until_last_reference_removed(self) -> None:
48+
file = self.create_file(
49+
name="shared.dSYM",
50+
type="project.dif",
51+
headers={"Content-Type": "application/x-mach-binary"},
52+
)
53+
file.putfile(ContentFile(b""))
54+
55+
dif1 = self.create_dif_file(
56+
debug_id="00000000-0000-0000-0000-000000000000",
57+
file=file,
58+
)
59+
dif2 = self.create_dif_file(
60+
debug_id="11111111-1111-1111-1111-111111111111",
61+
file=file,
62+
)
63+
64+
dif1.delete()
65+
66+
assert not ProjectDebugFile.objects.filter(id=dif1.id).exists()
67+
assert ProjectDebugFile.objects.filter(id=dif2.id).exists()
68+
assert File.objects.filter(id=file.id).exists()
69+
70+
dif2.delete()
71+
72+
assert not ProjectDebugFile.objects.filter(id=dif2.id).exists()
73+
assert not File.objects.filter(id=file.id).exists()
74+
4675
def test_find_dif_by_debug_id(self) -> None:
4776
debug_id1 = "dfb8e43a-f242-3d73-a453-aeb6a777ef75"
4877
debug_id2 = "19bd7a09-3e31-4911-a5cd-8e829b845407"

0 commit comments

Comments
 (0)