fix(backup): Handle SavedSearch collisions (#62697)

Previously, when calculating which collisions we needed to test, we
ignored `UniqueConstraint` clauses in model definition. Now that these
have been added (see `detailed.json` for a list of which models were
included), a new collision has been discovered: two `is_global=True`
instances of the `SavedSearch` model cannot have the same `name`.

We resolve this here by ignoring `is_global=True` instances when
importing in a non-`Global` scope.

fixtures/backup/model_dependencies/detailed.json

@@ -1572,7 +1572,13 @@
       "Region"
     ],
     "table_name": "sentry_discoversavedquery",
-    "uniques": []
+    "uniques": [
+      [
+        "created_by_id",
+        "is_homepage",
+        "organization"
+      ]
+    ]
   },
   "sentry.discoversavedqueryproject": {
     "dangling": false,
@@ -3146,7 +3152,11 @@
       "Region"
     ],
     "table_name": "sentry_metricskeyindexer",
-    "uniques": []
+    "uniques": [
+      [
+        "string"
+      ]
+    ]
   },
   "sentry.monitor": {
     "dangling": false,
@@ -3889,7 +3899,13 @@
       "Region"
     ],
     "table_name": "sentry_perfstringindexer",
-    "uniques": []
+    "uniques": [
+      [
+        "organization_id",
+        "string",
+        "use_case_id"
+      ]
+    ]
   },
   "sentry.platformexternalissue": {
     "dangling": false,
@@ -5314,10 +5330,16 @@
     ],
     "table_name": "sentry_rulesnooze",
     "uniques": [
+      [
+        "alert_rule"
+      ],
       [
         "alert_rule",
         "user_id"
       ],
+      [
+        "rule"
+      ],
       [
         "rule",
         "user_id"
@@ -5345,7 +5367,17 @@
       "Region"
     ],
     "table_name": "sentry_savedsearch",
-    "uniques": []
+    "uniques": [
+      [
+        "is_global",
+        "name"
+      ],
+      [
+        "organization",
+        "owner_id",
+        "type"
+      ]
+    ]
   },
   "sentry.scheduleddeletion": {
     "dangling": false,
@@ -5722,7 +5754,12 @@
       "Region"
     ],
     "table_name": "sentry_stringindexer",
-    "uniques": []
+    "uniques": [
+      [
+        "organization_id",
+        "string"
+      ]
+    ]
   },
   "sentry.team": {
     "dangling": false,

src/sentry/backup/dependencies.py

@@ -7,6 +7,7 @@ from functools import lru_cache
 from typing import NamedTuple, Optional, Tuple, Type
 
 from django.db import models
+from django.db.models import UniqueConstraint
 from django.db.models.fields.related import ForeignKey, OneToOneField
 
 from sentry.backup.helpers import EXCLUDED_APPS
@@ -402,6 +403,9 @@ def dependencies() -> dict[NormalizedModelName, ModelRelations]:
             uniques: set[frozenset[str]] = {
                 frozenset(combo) for combo in model._meta.unique_together
             }
+            for constraint in model._meta.constraints:
+                if isinstance(constraint, UniqueConstraint):
+                    uniques.add(frozenset(constraint.fields))
 
             # Now add a dependency for any FK relation visible to Django.
             for field in model._meta.get_fields():

src/sentry/models/savedsearch.py

@@ -5,7 +5,9 @@ from django.db.models import Q, UniqueConstraint
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
-from sentry.backup.scopes import RelocationScope
+from sentry.backup.dependencies import PrimaryKeyMap
+from sentry.backup.helpers import ImportFlags
+from sentry.backup.scopes import ImportScope, RelocationScope
 from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model, sane_repr
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.db.models.fields.text import CharField
@@ -71,7 +73,7 @@ class SavedSearch(Model):
 
     # Creator of the saved search. When visibility is
     # Visibility.{OWNER,OWNER_PINNED} this field is used to constrain who the
-    # search is visibile to.
+    # search is visible to.
     owner_id = HybridCloudForeignKey("sentry.User", on_delete="CASCADE", null=True)
 
     # Defines who can see the saved search
@@ -103,3 +105,13 @@ class SavedSearch(Model):
         return self.visibility == Visibility.OWNER_PINNED
 
     __repr__ = sane_repr("project_id", "name")
+
+    def normalize_before_relocation_import(
+        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
+    ) -> int | None:
+        # Ignore `is_global` searches from importing users, unless this is the `Global` import
+        # scope.
+        if scope != ImportScope.Global and self.is_global:
+            return None
+
+        return super().normalize_before_relocation_import(pk_map, scope, flags)

tests/sentry/backup/test_imports.py

@@ -54,6 +54,7 @@ from sentry.models.orgauthtoken import OrgAuthToken
 from sentry.models.project import Project
 from sentry.models.projectkey import ProjectKey
 from sentry.models.relay import Relay, RelayUsage
+from sentry.models.savedsearch import SavedSearch, Visibility
 from sentry.models.team import Team
 from sentry.models.user import User
 from sentry.models.useremail import UserEmail
@@ -1621,6 +1622,36 @@ class CollisionTests(ImportTestCase):
                 with open(tmp_path, "rb") as tmp_file:
                     verify_models_in_output(expected_models, json.load(tmp_file))
 
+    @expect_models(COLLISION_TESTED, SavedSearch)
+    def test_colliding_saved_search(self, expected_models: list[Type[Model]]):
+        self.create_organization("some-org", owner=self.user)
+        SavedSearch.objects.create(
+            name="Global Search",
+            query="saved query",
+            is_global=True,
+            visibility=Visibility.ORGANIZATION,
+        )
+        assert SavedSearch.objects.count() == 1
+
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            tmp_path = self.export_to_tmp_file_and_clear_database(tmp_dir)
+            assert SavedSearch.objects.count() == 0
+
+            # Allow `is_global` searches for `ImportScope.Global` imports.
+            with open(tmp_path, "rb") as tmp_file:
+                import_in_global_scope(tmp_file, printer=NOOP_PRINTER)
+
+            assert SavedSearch.objects.count() == 1
+
+            # Disallow `is_global` searches for `ImportScope.Organization` imports.
+            with open(tmp_path, "rb") as tmp_file:
+                import_in_organization_scope(tmp_file, printer=NOOP_PRINTER)
+
+            assert SavedSearch.objects.count() == 1
+
+            with open(tmp_path, "rb") as tmp_file:
+                verify_models_in_output(expected_models, json.load(tmp_file))
+
     @expect_models(COLLISION_TESTED, ControlOption, Option, Relay, RelayUsage, UserRole)
     def test_colliding_configs_overwrite_configs_enabled_in_config_scope(
         self, expected_models: list[Type[Model]]