feat(backup): Generate cloned QuerySubscription on import

To avoid creating duplicate `subscription_id`s, rather than naively
copying in imported model data, we instead take this data to construct a
new call to `create_snuba_subscription`. This will generate an identical
subscription (modulo creation/edit dates, which we overwrite), but with
a new `subscription_id`.

Closes getsentry/team-ospo#193

src/sentry/backup/comparators.py

@@ -519,6 +519,39 @@ def __init__(self, bytes: int, *fields: str):
         super().__init__(re.compile(f"""^[0-9a-f]{{{bytes * 2}}}$"""), *fields)
 
 
+class SubscriptionIDComparator(RegexComparator):
+    """Compare the basic format of `QuerySubscription` IDs, which is basically a UUID1 with a numeric prefix. Ensure that the two values are NOT equivalent."""
+
+    def __init__(self, *fields: str):
+        super().__init__(re.compile("^\\d+/[0-9a-f]{32}$"), *fields)
+
+    def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[ComparatorFinding]:
+        # First, ensure that the two sides are not equivalent.
+        findings = []
+        fields = sorted(self.fields)
+        for f in fields:
+            if left["fields"].get(f) is None and right["fields"].get(f) is None:
+                continue
+
+            lv = left["fields"][f]
+            rv = right["fields"][f]
+            if lv == rv:
+                findings.append(
+                    ComparatorFinding(
+                        kind=self.get_kind(),
+                        on=on,
+                        left_pk=left["pk"],
+                        right_pk=right["pk"],
+                        reason=f"""the left value ({lv}) of the subscription ID field `{f}` was
+                                equal to the right value ({rv})""",
+                    )
+                )
+
+        # Now, make sure both IDs' regex are valid.
+        findings.extend(super().compare(on, left, right))
+        return findings
+
+
 # Note: we could also use the `uuid` Python uuid module for this, but it is finicky and accepts some
 # weird syntactic variations that are not very common and may cause weird failures when they are
 # rejected elsewhere.
@@ -653,7 +686,12 @@ def get_default_comparators():
                 HashObfuscatingComparator("public_key", "secret_key"),
                 SecretHexComparator(16, "public_key", "secret_key"),
             ],
-            "sentry.querysubscription": [DateUpdatedComparator("date_updated")],
+            "sentry.querysubscription": [
+                DateUpdatedComparator("date_updated"),
+                # We regenerate subscriptions when importing them, so even though all of the
+                # particulars stay the same, the `subscription_id`s will be different.
+                SubscriptionIDComparator("subscription_id"),
+            ],
             "sentry.relay": [HashObfuscatingComparator("relay_id", "public_key")],
             "sentry.relayusage": [HashObfuscatingComparator("relay_id", "public_key")],
             "sentry.sentryapp": [

src/sentry/backup/findings.py

@@ -90,6 +90,13 @@ class ComparatorFindingKind(IntEnum):
     # present or `None`.
     SecretHexComparatorExistenceCheck = auto()
 
+    # Subscription ID fields did not match their regex specification.
+    SubscriptionIDComparator = auto()
+
+    # Failed to compare a subscription id field because one of the fields being compared was not
+    # present or `None`.
+    SubscriptionIDComparatorExistenceCheck = auto()
+
     # UUID4 fields did not match their regex specification.
     UUID4Comparator = auto()
 

src/sentry/snuba/models.py

@@ -1,13 +1,15 @@
 from datetime import timedelta
 from enum import Enum
+from typing import Optional, Tuple
 
 from django.db import models
 from django.utils import timezone
 
-from sentry.backup.scopes import RelocationScope
-from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model
+from sentry.backup.dependencies import ImportKind, PrimaryKeyMap
+from sentry.backup.helpers import ImportFlags
+from sentry.backup.scopes import ImportScope, RelocationScope
+from sentry.db.models import BaseManager, FlexibleForeignKey, Model, region_silo_only_model
 from sentry.db.models.base import DefaultFieldsModel
-from sentry.db.models.manager import BaseManager
 
 
 class QueryAggregations(Enum):
@@ -97,3 +99,26 @@ class Status(Enum):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_querysubscription"
+
+    # We want the `QuerySubscription` to get properly created in Snuba, so we'll run it through the
+    # purpose-built logic for that operation rather than copying the data verbatim. This will result
+    # in an identical duplicate of the `QuerySubscription` model with a unique `subscription_id`.
+    def write_relocation_import(
+        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
+    ) -> Optional[Tuple[int, int, ImportKind]]:
+        # TODO(getsentry/team-ospo#190): Prevents a circular import; could probably split up the
+        # source module in such a way that this is no longer an issue.
+        from sentry.snuba.subscriptions import create_snuba_subscription
+
+        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
+        if old_pk is None:
+            return None
+
+        subscription = create_snuba_subscription(self.project, self.type, self.snuba_query)
+
+        # Keep the original dates.
+        subscription.date_added = self.date_added
+        subscription.date_updated = self.date_updated
+        subscription.save()
+
+        return (old_pk, subscription.pk, ImportKind.Inserted)

tests/sentry/backup/test_comparators.py

@@ -12,6 +12,7 @@
     IgnoredComparator,
     ScrubbedData,
     SecretHexComparator,
+    SubscriptionIDComparator,
     UserPasswordObfuscatingComparator,
     UUID4Comparator,
 )
@@ -181,7 +182,7 @@ def test_good_auto_suffix_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "auto_suffix_field" in res[0].reason
+    assert "`auto_suffix_field`" in res[0].reason
 
 
 def test_good_auto_suffix_comparator_scrubbed():
@@ -417,7 +418,7 @@ def test_good_email_obfuscating_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "email_obfuscating_field" in res[0].reason
+    assert "`email_obfuscating_field`" in res[0].reason
 
 
 def test_good_email_obfuscating_comparator_scrubbed():
@@ -555,7 +556,7 @@ def test_good_hash_obfuscating_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "hash_obfuscating_field" in res[0].reason
+    assert "`hash_obfuscating_field`" in res[0].reason
 
 
 def test_good_hash_obfuscating_comparator_scrubbed():
@@ -678,7 +679,7 @@ def test_good_foreign_key_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "user" in res[0].reason
+    assert "`user`" in res[0].reason
 
 
 def test_good_foreign_key_comparator_scrubbed():
@@ -894,7 +895,7 @@ def test_good_ignored_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "ignored_field" in res[0].reason
+    assert "`ignored_field`" in res[0].reason
 
 
 def test_good_ignored_comparator_scrubbed():
@@ -1017,6 +1018,141 @@ def test_good_secret_hex_comparator_scrubbed():
     assert right["scrubbed"]["SecretHexComparator::secret_hex_field"] is ScrubbedData()
 
 
+def test_good_subscription_id_comparator():
+    cmp = SubscriptionIDComparator("subscription_id_field")
+    id = InstanceID("test", 0)
+    left: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "subscription_id_field": "0/12363aae153911eeac590242ac130004",
+        },
+    }
+    right: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "subscription_id_field": "0/45663aae153911eeac590242acabc123",
+        },
+    }
+    assert not cmp.compare(id, left, right)
+
+
+def test_bad_subscription_id_comparator():
+    cmp = SubscriptionIDComparator("same", "invalid_left", "invalid_right")
+    id = InstanceID("test", 0)
+    left: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "same": "0/12363aae153911eeac590242ac130004",
+            "invalid_left": "12363aae153911eeac590242ac130004",
+            "invalid_right": "0/12363aae153911eeac590242ac130004",
+        },
+    }
+    right: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "same": "0/12363aae153911eeac590242ac130004",
+            "invalid_left": "0/12363aae153911eeac590242ac130004",
+            "invalid_right": "0/foobar",
+        },
+    }
+    res = cmp.compare(id, left, right)
+    assert res
+    assert len(res) == 3
+
+    assert res[0]
+    assert res[0].kind == ComparatorFindingKind.SubscriptionIDComparator
+    assert res[0].on == id
+    assert res[0].left_pk == 1
+    assert res[0].right_pk == 1
+    assert "`same`" in res[0].reason
+    assert "equal" in res[0].reason
+    assert "0/12363aae153911eeac590242ac130004" in res[0].reason
+
+    assert res[1]
+    assert res[1].kind == ComparatorFindingKind.SubscriptionIDComparator
+    assert res[1].on == id
+    assert res[1].left_pk == 1
+    assert res[1].right_pk == 1
+    assert "`invalid_left`" in res[1].reason
+    assert "left" in res[1].reason
+    assert "regex" in res[1].reason
+    assert "12363aae153911eeac590242ac130004" in res[1].reason
+
+    assert res[2]
+    assert res[2].kind == ComparatorFindingKind.SubscriptionIDComparator
+    assert res[2].on == id
+    assert res[2].left_pk == 1
+    assert res[2].right_pk == 1
+    assert "`invalid_right`" in res[2].reason
+    assert "right" in res[2].reason
+    assert "regex" in res[2].reason
+    assert "0/foobar" in res[2].reason
+
+
+def test_good_subscription_id_comparator_existence():
+    cmp = SubscriptionIDComparator("subscription_id_field")
+    id = InstanceID("test", 0)
+    present: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "subscription_id_field": "0/45663aae153911eeac590242acabc123",
+        },
+    }
+    missing: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {},
+    }
+    res = cmp.existence(id, missing, present)
+    assert res
+    assert len(res) == 1
+
+    assert res[0]
+    assert res[0].on == id
+    assert res[0].kind == ComparatorFindingKind.SubscriptionIDComparatorExistenceCheck
+    assert res[0].left_pk == 1
+    assert res[0].right_pk == 1
+    assert "left" in res[0].reason
+    assert "`subscription_id_field`" in res[0].reason
+
+
+def test_good_subscription_id_comparator_scrubbed():
+    cmp = SubscriptionIDComparator("subscription_id_field")
+    left: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "subscription_id_field": "0/12363aae153911eeac590242ac130004",
+        },
+    }
+    right: JSONData = {
+        "model": "test",
+        "ordinal": 1,
+        "pk": 1,
+        "fields": {
+            "subscription_id_field": "0/45663aae153911eeac590242acabc123",
+        },
+    }
+    cmp.scrub(left, right)
+    assert left["scrubbed"]
+    assert left["scrubbed"]["SubscriptionIDComparator::subscription_id_field"] is ScrubbedData()
+
+    assert right["scrubbed"]
+    assert right["scrubbed"]["SubscriptionIDComparator::subscription_id_field"] is ScrubbedData()
+
+
 def test_good_uuid4_comparator():
     cmp = UUID4Comparator("guid_field")
     id = InstanceID("test", 0)
@@ -1349,7 +1485,7 @@ def test_good_user_password_obfuscating_comparator_existence():
     assert res[0].left_pk == 1
     assert res[0].right_pk == 1
     assert "left" in res[0].reason
-    assert "password" in res[0].reason
+    assert "`password`" in res[0].reason
 
 
 def test_good_user_password_obfuscating_comparator_scrubbed_long():

tests/sentry/backup/test_imports.py

@@ -32,6 +32,7 @@
 from sentry.models.userip import UserIP
 from sentry.models.userpermission import UserPermission
 from sentry.models.userrole import UserRole, UserRoleUser
+from sentry.snuba.models import QuerySubscription, SnubaQuery
 from sentry.testutils.factories import get_fixture_path
 from sentry.testutils.helpers.backups import (
     NOOP_PRINTER,
@@ -636,7 +637,7 @@ def test_colliding_org_auth_token(self):
         with tempfile.TemporaryDirectory() as tmp_dir:
             tmp_path = self.export_to_tmp_file_and_clear_database(tmp_dir)
 
-            # After exporting and clearing the database, insert a copy of the same `ProjectKey` as
+            # After exporting and clearing the database, insert a copy of the same `OrgAuthToken` as
             # the one found in the import.
             org = self.create_organization()
             colliding.organization_id = org.id
@@ -693,6 +694,53 @@ def test_colliding_project_key(self):
         assert ProjectKey.objects.filter(public_key=colliding.public_key).count() == 1
         assert ProjectKey.objects.filter(secret_key=colliding.secret_key).count() == 1
 
+    def test_colliding_query_subscription(self):
+        # We need a celery task running to properly test the `subscription_id` assignment, otherwise
+        # its value just defaults to `None`.
+        with self.tasks():
+            owner = self.create_exhaustive_user("owner")
+            invited = self.create_exhaustive_user("invited")
+            member = self.create_exhaustive_user("member")
+            self.create_exhaustive_organization("some-org", owner, invited, [member])
+
+            # Take note of the `QuerySubscription` that was created by the exhaustive organization -
+            # this is the one we'll be importing.
+            colliding_snuba_query = SnubaQuery.objects.all().first()
+            colliding_query_subscription = QuerySubscription.objects.filter(
+                snuba_query=colliding_snuba_query
+            ).first()
+
+            with tempfile.TemporaryDirectory() as tmp_dir:
+                tmp_path = self.export_to_tmp_file_and_clear_database(tmp_dir)
+
+                # After exporting and clearing the database, insert a copy of the same
+                # `QuerySubscription.subscription_id` as the one found in the import.
+                colliding_snuba_query.save()
+                colliding_query_subscription.project = self.create_project()
+                colliding_query_subscription.snuba_query = colliding_snuba_query
+                colliding_query_subscription.save()
+
+                assert SnubaQuery.objects.count() == 1
+                assert QuerySubscription.objects.count() == 1
+                assert (
+                    QuerySubscription.objects.filter(
+                        subscription_id=colliding_query_subscription.subscription_id
+                    ).count()
+                    == 1
+                )
+
+                with open(tmp_path) as tmp_file:
+                    import_in_organization_scope(tmp_file, printer=NOOP_PRINTER)
+
+            assert SnubaQuery.objects.count() > 1
+            assert QuerySubscription.objects.count() > 1
+            assert (
+                QuerySubscription.objects.filter(
+                    subscription_id=colliding_query_subscription.subscription_id
+                ).count()
+                == 1
+            )
+
     def test_colliding_user_with_merging_enabled_in_user_scope(self):
         self.create_exhaustive_user(username="owner", email="owner@example.com")