Skip to content

Commit f6af4ee

Browse files
committed
fix(merge): Handle unique constraint conflicts in merge_users_for_model_in_org
Before updating user references during a user merge, delete from_user rows that would violate unique constraints by conflicting with existing to_user rows. Uses Exists/OuterRef subqueries to generically detect conflicts for any model, including conditional UniqueConstraints. Fixes SENTRY-5HVP
1 parent 352c3c9 commit f6af4ee

File tree

2 files changed

+92
-1
lines changed

2 files changed

+92
-1
lines changed

src/sentry/backup/dependencies.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from typing import NamedTuple
88

99
from django.db import models
10-
from django.db.models import Q, UniqueConstraint
10+
from django.db.models import Exists, OuterRef, Q, UniqueConstraint
1111
from django.db.models.fields.related import ForeignKey, OneToOneField
1212

1313
from sentry.backup.helpers import EXCLUDED_APPS
@@ -704,6 +704,26 @@ def dedupe_and_reassign_groupsubscription_in_org(
704704
GroupSubscription.objects.filter(scoped, user_id=from_user_id).update(user_id=to_user_id)
705705

706706

707+
def _get_unique_constraints_for_field(
708+
model: type[models.base.Model], field_name: str
709+
) -> list[tuple[frozenset[str], Q | None]]:
710+
"""
711+
Return all unique constraints on ``model`` that include ``field_name``.
712+
Each entry is ``(frozenset_of_field_names, optional_condition_Q)``.
713+
"""
714+
results: list[tuple[frozenset[str], Q | None]] = []
715+
for combo in model._meta.unique_together:
716+
fields = frozenset(combo)
717+
if field_name in fields:
718+
results.append((fields, None))
719+
for constraint in model._meta.constraints:
720+
if isinstance(constraint, UniqueConstraint):
721+
fields = frozenset(constraint.fields)
722+
if field_name in fields:
723+
results.append((fields, getattr(constraint, "condition", None)))
724+
return results
725+
726+
707727
def merge_users_for_model_in_org(
708728
model: type[models.base.Model], *, organization_id: int, from_user_id: int, to_user_id: int
709729
) -> None:
@@ -739,5 +759,26 @@ def merge_users_for_model_in_org(
739759
for_this_org = Q(**{field_name: organization_id for field_name in org_refs})
740760

741761
for user_ref in user_refs:
762+
# Delete from_user rows that would violate unique constraints when updated.
763+
for unique_fields, condition in _get_unique_constraints_for_field(model, user_ref):
764+
other_fields = unique_fields - {user_ref}
765+
if not other_fields:
766+
continue
767+
768+
# Build Exists subquery: find to_user rows matching on all non-user fields.
769+
subquery_kwargs: dict[str, object] = {user_ref: to_user_id}
770+
for field in other_fields:
771+
subquery_kwargs[field] = OuterRef(field)
772+
773+
subquery = model.objects.filter(**subquery_kwargs)
774+
if condition is not None:
775+
subquery = subquery.filter(condition)
776+
777+
qs = model.objects.filter(for_this_org, **{user_ref: from_user_id})
778+
if condition is not None:
779+
qs = qs.filter(condition)
780+
qs.filter(Exists(subquery)).delete()
781+
782+
# Now safe to update remaining rows.
742783
q = for_this_org & Q(**{user_ref: from_user_id})
743784
model.objects.filter(q).update(**{user_ref: to_user_id})

tests/sentry/users/models/test_user.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,56 @@ def test_merge_handles_groupsubscription_conflicts(self) -> None:
307307
assert not GroupSubscription.objects.filter(group=group, user_id=from_user.id).exists()
308308
assert GroupSubscription.objects.filter(group=group, user_id=to_user.id).count() == 1
309309

310+
def test_merge_handles_recentsearch_conflicts(self) -> None:
311+
from sentry.utils.hashlib import md5_text
312+
313+
from_user = self.create_user("from-user@example.com")
314+
to_user = self.create_user("to-user@example.com")
315+
org = self.create_organization(name="recentsearch-conflict-org")
316+
317+
with outbox_runner():
318+
with assume_test_silo_mode(SiloMode.CELL):
319+
self.create_member(user=from_user, organization=org)
320+
321+
with assume_test_silo_mode(SiloMode.CELL):
322+
query = "is:unresolved"
323+
query_hash = md5_text(query).hexdigest()
324+
RecentSearch.objects.create(
325+
organization=org, user_id=from_user.id, type=0, query=query, query_hash=query_hash
326+
)
327+
RecentSearch.objects.create(
328+
organization=org, user_id=to_user.id, type=0, query=query, query_hash=query_hash
329+
)
330+
331+
with outbox_runner():
332+
from_user.merge_to(to_user)
333+
334+
with assume_test_silo_mode(SiloMode.CELL):
335+
assert not RecentSearch.objects.filter(organization=org, user_id=from_user.id).exists()
336+
assert RecentSearch.objects.filter(organization=org, user_id=to_user.id).count() == 1
337+
338+
def test_merge_handles_groupbookmark_conflicts(self) -> None:
339+
from_user = self.create_user("from-user@example.com")
340+
to_user = self.create_user("to-user@example.com")
341+
org = self.create_organization(name="bookmark-conflict-org")
342+
343+
with outbox_runner():
344+
with assume_test_silo_mode(SiloMode.CELL):
345+
self.create_member(user=from_user, organization=org)
346+
347+
with assume_test_silo_mode(SiloMode.CELL):
348+
project = self.create_project(organization=org)
349+
group = self.create_group(project=project)
350+
GroupBookmark.objects.create(project=project, group=group, user_id=from_user.id)
351+
GroupBookmark.objects.create(project=project, group=group, user_id=to_user.id)
352+
353+
with outbox_runner():
354+
from_user.merge_to(to_user)
355+
356+
with assume_test_silo_mode(SiloMode.CELL):
357+
assert not GroupBookmark.objects.filter(group=group, user_id=from_user.id).exists()
358+
assert GroupBookmark.objects.filter(group=group, user_id=to_user.id).count() == 1
359+
310360
@expect_models(
311361
ORG_MEMBER_MERGE_TESTED,
312362
OrgAuthToken,

0 commit comments

Comments
 (0)