ref(backup): Make normalization a standalone method (#56261)

Previously, the `_normalize_before_relocation_import()` method was
intended to be called as part of `write_relocation_import()`. This
change makes it standalone method that the import script calls on its
own. This has a few benefits:

- Simpler function signatures, since we don't need to pipe all of
`normalize...`'s arguments through `write...` anymore.
- A simple two-tuple return from `write...`.
- Less error prone: rather than every override of `write...` needing to
remember to call `normalize...`, we just need to remember to do it once
(in the `_import()` function that orchestrates all of the calls).
- Finally: the next change we make will call `get_relocation_scope`
AFTER we normalize, so that we can look up ForeignKey references on the
model being imported without failing.

Issue: getsentry/team-ospo#199

fixtures/backup/model_dependencies/detailed.json

@@ -1841,7 +1841,10 @@
       }
     },
     "model": "sentry.NotificationAction",
-    "relocation_scope": "Global",
+    "relocation_scope": [
+      "Organization",
+      "Global"
+    ],
     "silos": [
       "Region"
     ]
@@ -1858,7 +1861,10 @@
       }
     },
     "model": "sentry.NotificationActionProject",
-    "relocation_scope": "Global",
+    "relocation_scope": [
+      "Organization",
+      "Global"
+    ],
     "silos": [
       "Region"
     ]

src/sentry/backup/imports.py

@@ -120,17 +120,32 @@ def _import(
             for obj in serializers.deserialize("json", src, stream=True, use_natural_keys=False):
                 o = obj.object
                 if o._meta.app_label not in EXCLUDED_APPS or o:
-                    if o.get_relocation_scope() in allowed_relocation_scopes:
+                    if o.get_possible_relocation_scopes() & allowed_relocation_scopes:
                         o = obj.object
                         model_name = normalize_model_name(o)
                         for f in filters:
                             if f.model == type(o) and getattr(o, f.field, None) not in f.values:
                                 break
                         else:
-                            written = o.write_relocation_import(pk_map, scope, flags)
-                            if written is not None:
-                                old_pk, new_pk, import_kind = written
-                                pk_map.insert(model_name, old_pk, new_pk, import_kind)
+                            # We can only be sure `get_relocation_scope()` will be correct if it is
+                            # fired AFTER normalization, as some `get_relocation_scope()` methods
+                            # rely on being able to correctly resolve foreign keys, which is only
+                            # possible after normalization.
+                            old_pk = o.normalize_before_relocation_import(pk_map, scope, flags)
+                            if old_pk is None:
+                                continue
+
+                            # Now that the model has been normalized, we can ensure that this
+                            # particular instance has a `RelocationScope` that permits importing.
+                            if not o.get_relocation_scope() in allowed_relocation_scopes:
+                                continue
+
+                            written = o.write_relocation_import(scope, flags)
+                            if written is None:
+                                continue
+
+                            new_pk, import_kind = written
+                            pk_map.insert(model_name, old_pk, new_pk, import_kind)
 
     # For all database integrity errors, let's warn users to follow our
     # recommended backup/restore workflow before reraising exception. Most of

src/sentry/db/models/base.py

@@ -125,22 +125,25 @@ class BaseModel(models.Model):
             else {cls.__relocation_scope__}
         )
 
-    def _normalize_before_relocation_import(
+    def normalize_before_relocation_import(
         self, pk_map: PrimaryKeyMap, _s: ImportScope, _f: ImportFlags
     ) -> Optional[int]:
         """
         A helper function that normalizes a deserialized model. Note that this modifies the model in
-        place, so it should generally be done inside of the companion `write_relocation_import`
-        method, to avoid data skew or corrupted local state.
+        place, so it should generally be done immediately prior to a companion
+        `write_relocation_import()` method, to avoid data skew or corrupted local state. The method
+        returns the old `pk` that was replaced, or `None` if normalization failed.
 
-        The only reason this function is left as a standalone, rather than being folded into
+        The primary reason this function is left as a standalone, rather than being folded into
         `write_relocation_import`, is that it is often useful to adjust just the normalization logic
-        by itself. Overrides of this method should take care not to mutate the `pk_map`.
+        by itself without affecting the writing logic.
+
+        Overrides of this method should take care NOT to mutate the `pk_map`. Overrides should also
+        take care NOT to push the updated changes to the database (ie, no calls to `.save()` or
+        `.update()`), as this functionality is delegated to the `write_relocation_import()` method.
 
         The default normalization logic merely replaces foreign keys with their new values from the
         provided `pk_map`.
-
-        The method returns the old `pk` that was replaced.
         """
 
         deps = dependencies()
@@ -161,22 +164,22 @@ class BaseModel(models.Model):
         return old_pk
 
     def write_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> Optional[Tuple[int, int, ImportKind]]:
+        self, _s: ImportScope, _f: ImportFlags
+    ) -> Optional[Tuple[int, ImportKind]]:
         """
         Writes a deserialized model to the database. If this write is successful, this method will
-        return a tuple of the old and new `pk`s.
+        return a tuple of the new `pk` and the `ImportKind` (ie, whether we created a new model or
+        re-used an existing one).
 
         Overrides of this method can throw either `django.core.exceptions.ValidationError` or
         `rest_framework.serializers.ValidationError`.
-        """
 
-        old_pk = self._normalize_before_relocation_import(pk_map, scope, flags)
-        if old_pk is None:
-            return
+        This function should only be executed after `normalize_before_relocation_import()` has fired
+        and returned a not-null `old_pk` input.
+        """
 
         self.save(force_insert=True)
-        return (old_pk, self.pk, ImportKind.Inserted)
+        return (self.pk, ImportKind.Inserted)
 
 
 class Model(BaseModel):

src/sentry/incidents/models.py

@@ -216,10 +216,10 @@ class Incident(Model):
     def duration(self):
         return self.current_end_date - self.date_started
 
-    def _normalize_before_relocation_import(
+    def normalize_before_relocation_import(
         self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
     ) -> Optional[int]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
+        old_pk = super().normalize_before_relocation_import(pk_map, scope, flags)
         if old_pk is None:
             return None
 
@@ -296,10 +296,10 @@ class IncidentActivity(Model):
         app_label = "sentry"
         db_table = "sentry_incidentactivity"
 
-    def _normalize_before_relocation_import(
+    def normalize_before_relocation_import(
         self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
     ) -> Optional[int]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
+        old_pk = super().normalize_before_relocation_import(pk_map, scope, flags)
         if old_pk is None:
             return None
 

src/sentry/models/actor.py

@@ -144,10 +144,10 @@ class Actor(Model):
         return self.get_actor_tuple().get_actor_identifier()
 
     # TODO(hybrid-cloud): actor refactor. Remove this method when done.
-    def _normalize_before_relocation_import(
+    def normalize_before_relocation_import(
         self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
     ) -> Optional[int]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
+        old_pk = super().normalize_before_relocation_import(pk_map, scope, flags)
         if old_pk is None:
             return None
 

src/sentry/models/email.py

@@ -7,7 +7,7 @@ from django.forms import model_to_dict
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
-from sentry.backup.dependencies import ImportKind, PrimaryKeyMap
+from sentry.backup.dependencies import ImportKind
 from sentry.backup.helpers import ImportFlags
 from sentry.backup.scopes import ImportScope, RelocationScope
 from sentry.db.models import CIEmailField, Model, control_silo_only_model, sane_repr
@@ -32,12 +32,8 @@ class Email(Model):
     __repr__ = sane_repr("email")
 
     def write_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> Optional[Tuple[int, int, ImportKind]]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
-        if old_pk is None:
-            return None
-
+        self, _s: ImportScope, _f: ImportFlags
+    ) -> Optional[Tuple[int, ImportKind]]:
         # Ensure that we never attempt to duplicate email entries, as they must always be unique.
         (email, created) = self.__class__.objects.get_or_create(
             email=self.email, defaults=model_to_dict(self)
@@ -46,4 +42,4 @@ class Email(Model):
             self.pk = email.pk
             self.save()
 
-        return (old_pk, self.pk, ImportKind.Inserted if created else ImportKind.Existing)
+        return (self.pk, ImportKind.Inserted if created else ImportKind.Existing)

src/sentry/models/notificationaction.py

@@ -152,7 +152,7 @@ class TriggerGenerator:
 
 @region_silo_only_model
 class NotificationActionProject(Model):
-    __relocation_scope__ = RelocationScope.Global
+    __relocation_scope__ = {RelocationScope.Global, RelocationScope.Organization}
 
     project = FlexibleForeignKey("sentry.Project")
     action = FlexibleForeignKey("sentry.NotificationAction")
@@ -161,6 +161,10 @@ class NotificationActionProject(Model):
         app_label = "sentry"
         db_table = "sentry_notificationactionproject"
 
+    def get_relocation_scope(self) -> RelocationScope:
+        action = NotificationAction.objects.get(id=self.action_id)
+        return action.get_relocation_scope()
+
 
 class ActionRegistration(metaclass=ABCMeta):
     def __init__(self, action: NotificationAction):
@@ -205,7 +209,7 @@ class NotificationAction(AbstractNotificationAction):
     Generic notification action model to programmatically route depending on the trigger (or source) for the notification
     """
 
-    __relocation_scope__ = RelocationScope.Global
+    __relocation_scope__ = {RelocationScope.Global, RelocationScope.Organization}
     __repr__ = sane_repr("id", "trigger_type", "service_type", "target_display")
 
     _trigger_types: tuple[tuple[int, str], ...] = ActionTrigger.as_choices()
@@ -331,3 +335,10 @@ class NotificationAction(AbstractNotificationAction):
                     "target_type": self.target_type,
                 },
             )
+
+    def get_relocation_scope(self) -> RelocationScope:
+        if self.integration_id is not None or self.sentry_app_id is not None:
+            # TODO(getsentry/team-ospo#188): this should be extension scope once that gets added.
+            return RelocationScope.Global
+
+        return RelocationScope.Organization

src/sentry/models/options/project_option.py

@@ -5,7 +5,7 @@ from typing import TYPE_CHECKING, Any, Mapping, Optional, Sequence, Tuple
 from django.db import models
 
 from sentry import projectoptions
-from sentry.backup.dependencies import ImportKind, PrimaryKeyMap
+from sentry.backup.dependencies import ImportKind
 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
@@ -163,12 +163,8 @@ class ProjectOption(Model):
     __repr__ = sane_repr("project_id", "key", "value")
 
     def write_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> Optional[Tuple[int, int, ImportKind]]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
-        if old_pk is None:
-            return None
-
+        self, _s: ImportScope, _f: ImportFlags
+    ) -> Optional[Tuple[int, ImportKind]]:
         (key, created) = self.__class__.objects.get_or_create(
             project=self.project, key=self.key, defaults={"value": self.value}
         )
@@ -176,4 +172,4 @@ class ProjectOption(Model):
             self.pk = key.pk
             self.save()
 
-        return (old_pk, self.pk, ImportKind.Inserted if created else ImportKind.Existing)
+        return (self.pk, ImportKind.Inserted if created else ImportKind.Existing)

src/sentry/models/options/user_option.py

@@ -5,7 +5,7 @@ from typing import TYPE_CHECKING, Any, Mapping, Optional, Tuple
 from django.conf import settings
 from django.db import models
 
-from sentry.backup.dependencies import ImportKind, PrimaryKeyMap
+from sentry.backup.dependencies import ImportKind
 from sentry.backup.helpers import ImportFlags
 from sentry.backup.scopes import ImportScope, RelocationScope
 from sentry.db.models import FlexibleForeignKey, Model, control_silo_only_model, sane_repr
@@ -206,8 +206,8 @@ class UserOption(Model):
     __repr__ = sane_repr("user_id", "project_id", "organization_id", "key", "value")
 
     def write_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> Optional[Tuple[int, int, ImportKind]]:
+        self, scope: ImportScope, flags: ImportFlags
+    ) -> Optional[Tuple[int, ImportKind]]:
         # TODO(getsentry/team-ospo#190): This circular import is a bit gross. See if we can't find a
         # better place for this logic to live.
         from sentry.api.endpoints.user_details import UserOptionsSerializer
@@ -215,4 +215,4 @@ class UserOption(Model):
         serializer_options = UserOptionsSerializer(data={self.key: self.value}, partial=True)
         serializer_options.is_valid(raise_exception=True)
 
-        return super().write_relocation_import(pk_map, scope, flags)
+        return super().write_relocation_import(scope, flags)

src/sentry/models/orgauthtoken.py

@@ -8,7 +8,7 @@ from django.forms import model_to_dict
 from django.utils import timezone
 from django.utils.encoding import force_str
 
-from sentry.backup.dependencies import ImportKind, PrimaryKeyMap
+from sentry.backup.dependencies import ImportKind
 from sentry.backup.helpers import ImportFlags
 from sentry.backup.scopes import ImportScope, RelocationScope
 from sentry.conf.server import SENTRY_SCOPES
@@ -83,17 +83,13 @@ class OrgAuthToken(Model):
         return self.date_deactivated is None
 
     def write_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> Optional[Tuple[int, int, ImportKind]]:
+        self, _s: ImportScope, _f: ImportFlags
+    ) -> Optional[Tuple[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.api.utils import generate_region_url
         from sentry.utils.security.orgauthtoken_token import generate_token, hash_token
 
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope, flags)
-        if old_pk is None:
-            return None
-
         # If there is a token collision, or the token does not exist for some reason, generate a new
         # one.
         matching_token_hashed = self.__class__.objects.filter(
@@ -117,7 +113,7 @@ class OrgAuthToken(Model):
             self.pk = key.pk
             self.save()
 
-        return (old_pk, self.pk, ImportKind.Inserted if created else ImportKind.Existing)
+        return (self.pk, ImportKind.Inserted if created else ImportKind.Existing)
 
 
 def is_org_auth_token_auth(auth: object) -> bool: