emoji: Add database-level uniqueness constraint for RealmEmoji.

While races here are unlikely, it is most correct to enforce this
invariant at the database layer, and having a database-level
constraint makes the models file a bit more readable.

Author: odunybrad

zerver/lib/actions.py

@@ -7385,9 +7385,13 @@ def notify_realm_emoji(realm: Realm) -> None:
 def check_add_realm_emoji(
     realm: Realm, name: str, author: UserProfile, image_file: IO[bytes]
 ) -> Optional[RealmEmoji]:
-    realm_emoji = RealmEmoji(realm=realm, name=name, author=author)
-    realm_emoji.full_clean()
-    realm_emoji.save()
+    try:
+        realm_emoji = RealmEmoji(realm=realm, name=name, author=author)
+        realm_emoji.full_clean()
+        realm_emoji.save()
+    except django.db.utils.IntegrityError:
+        # Match the string in upload_emoji.
+        raise JsonableError(_("A custom emoji with this name already exists."))
 
     emoji_file_name = get_emoji_file_name(image_file.name, realm_emoji.id)
 

zerver/migrations/0372_realmemoji_unique_realm_emoji_when_false_deactivated.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.9 on 2021-12-09 19:46
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zerver", "0371_invalid_characters_in_topics"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="realmemoji",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("deactivated", False)),
+                fields=("realm", "name"),
+                name="unique_realm_emoji_when_false_deactivated",
+            ),
+        ),
+    ]

zerver/models.py

@@ -1048,6 +1048,15 @@ class RealmEmoji(models.Model):
     def __str__(self) -> str:
         return f"<RealmEmoji({self.realm.string_id}): {self.id} {self.name} {self.deactivated} {self.file_name}>"
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                fields=["realm", "name"],
+                condition=Q(deactivated=False),
+                name="unique_realm_emoji_when_false_deactivated",
+            ),
+        ]
+
 
 def get_realm_emoji_dicts(
     realm: Realm, only_active_emojis: bool = False

zerver/tests/test_realm_emoji.py

@@ -7,6 +7,7 @@
     do_create_user,
     do_set_realm_property,
 )
+from zerver.lib.exceptions import JsonableError
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import get_test_image_file
 from zerver.models import Realm, RealmEmoji, UserProfile, get_realm
@@ -343,3 +344,25 @@ def test_check_admin_different_realm_emoji(self) -> None:
         self.login_user(emoji_author_2)
         result = self.client_delete("/json/realm/emoji/test_emoji")
         self.assert_json_success(result)
+
+    def test_upload_already_existed_emoji_in_check_add_realm_emoji(self) -> None:
+        realm_1 = do_create_realm("test_realm", "test_realm")
+        emoji_author = do_create_user(
+            "[email protected]", password="abc", realm=realm_1, full_name="abc", acting_user=None
+        )
+        emoji_name = "emoji_test"
+        with get_test_image_file("img.png") as img_file:
+            # Because we want to verify the IntegrityError handling
+            # logic in check_add_realm_emoji rather than the primary
+            # check in upload_emoji, we need to make this request via
+            # that helper rather than via the API.
+            check_add_realm_emoji(
+                realm=emoji_author.realm, name=emoji_name, author=emoji_author, image_file=img_file
+            )
+            with self.assertRaises(JsonableError):
+                check_add_realm_emoji(
+                    realm=emoji_author.realm,
+                    name=emoji_name,
+                    author=emoji_author,
+                    image_file=img_file,
+                )

zerver/tests/test_transfer.py

@@ -107,6 +107,8 @@ def test_transfer_emoji_to_s3(self) -> None:
         self.assertEqual(image_data, original_key.get()["Body"].read())
         self.assertEqual(resized_image_data, resized_key.get()["Body"].read())
 
+        emoji_name = "emoji2.png"
+
         with get_test_image_file("animated_img.gif") as image_file:
             emoji = check_add_realm_emoji(othello.realm, emoji_name, othello, image_file)
         if not emoji: