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.
This commit is contained in:
odunybrad 2021-12-02 18:11:54 -08:00 committed by Tim Abbott
parent 9a39ca217f
commit 90aa45a316
5 changed files with 62 additions and 3 deletions

View File

@ -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)

View File

@ -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",
),
),
]

View File

@ -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

View File

@ -7,6 +7,7 @@ from zerver.lib.actions import (
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 @@ class RealmEmojiTest(ZulipTestCase):
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(
"abc@example.com", 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,
)

View File

@ -107,6 +107,8 @@ class TransferUploadsToS3Test(ZulipTestCase):
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: