fix(backup): Handle UserRole name collisions

Issue: getsentry/team-ospo#184

Author: azaslavsky

src/sentry/models/userrole.py

@@ -1,14 +1,19 @@
-from typing import FrozenSet
+from typing import FrozenSet, Optional
 
 from django.conf import settings
 from django.db import models
 
-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 ArrayField, DefaultFieldsModel, control_silo_only_model, sane_repr
 from sentry.db.models.fields.foreignkey import FlexibleForeignKey
+from sentry.db.models.utils import unique_db_instance
 from sentry.signals import post_upgrade
 from sentry.silo import SiloMode
 
+MAX_USER_ROLE_NAME_LENGTH = 32
+
 
 @control_silo_only_model
 class UserRole(DefaultFieldsModel):
@@ -18,7 +23,7 @@ class UserRole(DefaultFieldsModel):
 
     __relocation_scope__ = RelocationScope.Config
 
-    name = models.CharField(max_length=32, unique=True)
+    name = models.CharField(max_length=MAX_USER_ROLE_NAME_LENGTH, unique=True)
     permissions = ArrayField()
     users = models.ManyToManyField("sentry.User", through="sentry.UserRoleUser")
 
@@ -39,6 +44,25 @@ def permissions_for_user(cls, user_id: int) -> FrozenSet[str]:
             for i in sl
         )
 
+    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)
+        if old_pk is None:
+            return None
+
+        # Circumvent unique name collisions.
+        if self.objects.filter(name__exact=self.name):
+            unique_db_instance(
+                self,
+                self.name,
+                max_length=MAX_USER_ROLE_NAME_LENGTH,
+                field_name="name",
+            )
+
+        return old_pk
+
 
 @control_silo_only_model
 class UserRoleUser(DefaultFieldsModel):

tests/sentry/backup/test_imports.py

@@ -741,6 +741,30 @@ def test_colliding_query_subscription(self):
                 == 1
             )
 
+    def test_colliding_user_role(self):
+        self.create_exhaustive_user("owner", is_admin=True)
+
+        # Take note of the `UserRole` - this is the one we'll be importing.
+        colliding = UserRole.objects.all().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 `UserRole` as the
+            # one found in the import.
+            colliding.save()
+
+            assert UserRole.objects.count() == 1
+            assert UserRole.objects.filter(name="test-admin-role").count() == 1
+
+            with open(tmp_path) as tmp_file:
+                import_in_global_scope(tmp_file, printer=NOOP_PRINTER)
+
+        assert UserRole.objects.count() == 2
+        assert UserRole.objects.filter(name__contains="test-admin-role").count() == 2
+        assert UserRole.objects.filter(name__exact="test-admin-role").count() == 1
+        assert UserRole.objects.filter(name__contains="test-admin-role-").count() == 1
+
     def test_colliding_user_with_merging_enabled_in_user_scope(self):
         self.create_exhaustive_user(username="owner", email="[email protected]")