From 8c31e6f96ed092432c0b5c9f633e5fa52758a909 Mon Sep 17 00:00:00 2001 From: Riken Shah Date: Thu, 12 Aug 2021 08:19:53 +0000 Subject: [PATCH] emoji: Add backend changes to support still image for animated emojis. Now, when we add a custom animated emoji to the realm we also save a still image of it (1st frame of the gif). So we can avoid showing an animated emoji every time. --- templates/zerver/api/changelog.md | 8 ++ version.py | 2 +- zerver/lib/actions.py | 6 +- zerver/lib/emoji.py | 4 +- zerver/lib/event_schema.py | 5 +- zerver/lib/upload.py | 110 ++++++++++++++---- .../migrations/0347_realm_emoji_animated.py | 18 +++ zerver/models.py | 19 ++- zerver/openapi/zulip.yaml | 23 ++++ zerver/tests/test_transfer.py | 39 ++++++- zerver/tests/test_upload.py | 80 ++++++++++++- 11 files changed, 281 insertions(+), 33 deletions(-) create mode 100644 zerver/migrations/0347_realm_emoji_animated.py diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index d4a4b64452..a9d6b82123 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -11,6 +11,14 @@ below features are supported. ## Changes in Zulip 5.0 +**Feature level 97** + +* `GET /realm/emoji`, `POST /realm/emoji/{emoji_name}`, [`GET + /events`](/api/get-events), [`POST /register`](/api/register-queue): + Custom emoji objects may now contain a `still_url` field, with the + URL of a PNG still image version of the emoji. This field will only be + present for animated emoji. + **Feature level 96** * [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults): diff --git a/version.py b/version.py index e26f90ab55..94b3773ef2 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 96 +API_FEATURE_LEVEL = 97 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index d2119fb36c..b9c1cc3dcf 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -7214,8 +7214,9 @@ def check_add_realm_emoji( emoji_file_name = mark_sanitized(emoji_file_name) emoji_uploaded_successfully = False + is_animated = False try: - upload_emoji_image(image_file, emoji_file_name, author) + is_animated = upload_emoji_image(image_file, emoji_file_name, author) emoji_uploaded_successfully = True finally: if not emoji_uploaded_successfully: @@ -7223,7 +7224,8 @@ def check_add_realm_emoji( return None else: realm_emoji.file_name = emoji_file_name - realm_emoji.save(update_fields=["file_name"]) + realm_emoji.is_animated = is_animated + realm_emoji.save(update_fields=["file_name", "is_animated"]) notify_realm_emoji(realm_emoji.realm) return realm_emoji diff --git a/zerver/lib/emoji.py b/zerver/lib/emoji.py index e75d154b4b..df5a050c96 100644 --- a/zerver/lib/emoji.py +++ b/zerver/lib/emoji.py @@ -107,8 +107,8 @@ def check_valid_emoji_name(emoji_name: str) -> None: raise JsonableError(_("Emoji name is missing")) -def get_emoji_url(emoji_file_name: str, realm_id: int) -> str: - return upload_backend.get_emoji_url(emoji_file_name, realm_id) +def get_emoji_url(emoji_file_name: str, realm_id: int, still: bool = False) -> str: + return upload_backend.get_emoji_url(emoji_file_name, realm_id, still) def get_emoji_file_name(emoji_file_name: str, emoji_id: int) -> str: diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 53b680afe7..41ec65c856 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -712,7 +712,10 @@ realm_emoji_type = DictType( ("source_url", str), ("deactivated", bool), ("author_id", int), - ] + ], + optional_keys=[ + ("still_url", str), + ], ) realm_emoji_update_event = event_dict_type( diff --git a/zerver/lib/upload.py b/zerver/lib/upload.py index f2a46fde5a..640dfc0f43 100644 --- a/zerver/lib/upload.py +++ b/zerver/lib/upload.py @@ -166,7 +166,14 @@ def resize_gif(im: GifImageFile, size: int = DEFAULT_EMOJI_SIZE) -> bytes: return out.getvalue() -def resize_emoji(image_data: bytes, size: int = DEFAULT_EMOJI_SIZE) -> bytes: +def resize_emoji( + image_data: bytes, size: int = DEFAULT_EMOJI_SIZE +) -> Tuple[bytes, bool, Optional[bytes]]: + # This function returns three values: + # 1) Emoji image data. + # 2) If emoji is gif i.e animated. + # 3) If is animated then return still image data i.e first frame of gif. + try: im = Image.open(io.BytesIO(image_data)) image_format = im.format @@ -181,13 +188,29 @@ def resize_emoji(image_data: bytes, size: int = DEFAULT_EMOJI_SIZE) -> bytes: or im.size[0] > MAX_EMOJI_GIF_SIZE # dimensions too large or len(image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES # filesize too large ) - return resize_gif(im, size) if should_resize else image_data + + # Generate a still image from the first frame. Since + # we're converting the format to PNG anyway, we resize unconditionally. + still_image = im.copy() + still_image.seek(0) + still_image = ImageOps.exif_transpose(still_image) + still_image = ImageOps.fit(still_image, (size, size), Image.ANTIALIAS) + out = io.BytesIO() + still_image.save(out, format="PNG") + still_image_data = out.getvalue() + + if should_resize: + image_data = resize_gif(im, size) + + return image_data, True, still_image_data else: + # Note that this is essentially duplicated in the + # still_image code path, above. im = ImageOps.exif_transpose(im) im = ImageOps.fit(im, (size, size), Image.ANTIALIAS) out = io.BytesIO() im.save(out, format=image_format) - return out.getvalue() + return out.getvalue(), False, None except OSError: raise BadImageError(_("Could not decode image; did you upload an image file?")) except DecompressionBombError: @@ -255,10 +278,10 @@ class ZulipUploadBackend: def upload_emoji_image( self, emoji_file: IO[bytes], emoji_file_name: str, user_profile: UserProfile - ) -> None: + ) -> bool: raise NotImplementedError() - def get_emoji_url(self, emoji_file_name: str, realm_id: int) -> str: + def get_emoji_url(self, emoji_file_name: str, realm_id: int, still: bool = False) -> str: raise NotImplementedError() def upload_export_tarball( @@ -654,7 +677,7 @@ class S3UploadBackend(ZulipUploadBackend): def upload_emoji_image( self, emoji_file: IO[bytes], emoji_file_name: str, user_profile: UserProfile - ) -> None: + ) -> bool: content_type = guess_type(emoji_file.name)[0] emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( realm_id=user_profile.realm_id, @@ -662,7 +685,7 @@ class S3UploadBackend(ZulipUploadBackend): ) image_data = emoji_file.read() - resized_image_data = resize_emoji(image_data) + resized_image_data, is_animated, still_image_data = resize_emoji(image_data) upload_image_to_s3( self.avatar_bucket, ".".join((emoji_path, "original")), @@ -677,12 +700,36 @@ class S3UploadBackend(ZulipUploadBackend): user_profile, resized_image_data, ) + if is_animated: + still_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=user_profile.realm_id, + emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], + ) + assert still_image_data is not None + upload_image_to_s3( + self.avatar_bucket, + still_path, + "image/png", + user_profile, + still_image_data, + ) - def get_emoji_url(self, emoji_file_name: str, realm_id: int) -> str: - emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( - realm_id=realm_id, emoji_file_name=emoji_file_name - ) - return self.get_public_upload_url(emoji_path) + return is_animated + + def get_emoji_url(self, emoji_file_name: str, realm_id: int, still: bool = False) -> str: + if still: + # We currently only support animated GIFs. + assert emoji_file_name.endswith(".gif") + emoji_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=realm_id, + emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], + ) + return self.get_public_upload_url(emoji_path) + else: + emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( + realm_id=realm_id, emoji_file_name=emoji_file_name + ) + return self.get_public_upload_url(emoji_path) def upload_export_tarball( self, @@ -906,22 +953,43 @@ class LocalUploadBackend(ZulipUploadBackend): def upload_emoji_image( self, emoji_file: IO[bytes], emoji_file_name: str, user_profile: UserProfile - ) -> None: + ) -> bool: emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( realm_id=user_profile.realm_id, emoji_file_name=emoji_file_name, ) image_data = emoji_file.read() - resized_image_data = resize_emoji(image_data) + resized_image_data, is_animated, still_image_data = resize_emoji(image_data) write_local_file("avatars", ".".join((emoji_path, "original")), image_data) write_local_file("avatars", emoji_path, resized_image_data) + if is_animated: + assert still_image_data is not None + still_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=user_profile.realm_id, + emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], + ) + write_local_file("avatars", still_path, still_image_data) + return is_animated - def get_emoji_url(self, emoji_file_name: str, realm_id: int) -> str: - return os.path.join( - "/user_avatars", - RealmEmoji.PATH_ID_TEMPLATE.format(realm_id=realm_id, emoji_file_name=emoji_file_name), - ) + def get_emoji_url(self, emoji_file_name: str, realm_id: int, still: bool = False) -> str: + if still: + # We currently only support animated GIFs. + assert emoji_file_name.endswith(".gif") + return os.path.join( + "/user_avatars", + RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=realm_id, + emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], + ), + ) + else: + return os.path.join( + "/user_avatars", + RealmEmoji.PATH_ID_TEMPLATE.format( + realm_id=realm_id, emoji_file_name=emoji_file_name + ), + ) def upload_export_tarball( self, @@ -998,8 +1066,8 @@ def upload_logo_image(user_file: IO[bytes], user_profile: UserProfile, night: bo def upload_emoji_image( emoji_file: IO[bytes], emoji_file_name: str, user_profile: UserProfile -) -> None: - upload_backend.upload_emoji_image(emoji_file, emoji_file_name, user_profile) +) -> bool: + return upload_backend.upload_emoji_image(emoji_file, emoji_file_name, user_profile) def upload_message_file( diff --git a/zerver/migrations/0347_realm_emoji_animated.py b/zerver/migrations/0347_realm_emoji_animated.py new file mode 100644 index 0000000000..8e3c4530e3 --- /dev/null +++ b/zerver/migrations/0347_realm_emoji_animated.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.6 on 2021-09-11 16:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zerver", "0346_create_realm_user_default_table"), + ] + + operations = [ + migrations.AddField( + model_name="realmemoji", + name="is_animated", + field=models.BooleanField(default=False), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 3cc064fa5a..81fe54587e 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -971,9 +971,13 @@ class RealmEmoji(models.Model): # The basename of the custom emoji's filename; see PATH_ID_TEMPLATE for the full path. file_name: Optional[str] = models.TextField(db_index=True, null=True, blank=True) + # Whether this custom emoji is an animated image. + is_animated: bool = models.BooleanField(default=False) + deactivated: bool = models.BooleanField(default=False) PATH_ID_TEMPLATE = "{realm_id}/emoji/images/{emoji_file_name}" + STILL_PATH_ID_TEMPLATE = "{realm_id}/emoji/images/still/{emoji_filename_without_extension}.png" def __str__(self) -> str: return f"" @@ -993,13 +997,26 @@ def get_realm_emoji_dicts( if realm_emoji.author: author_id = realm_emoji.author_id emoji_url = get_emoji_url(realm_emoji.file_name, realm_emoji.realm_id) - d[str(realm_emoji.id)] = dict( + + emoji_dict = dict( id=str(realm_emoji.id), name=realm_emoji.name, source_url=emoji_url, deactivated=realm_emoji.deactivated, author_id=author_id, ) + + if realm_emoji.is_animated: + # For animated emoji, we include still_url with a static + # version of the image, so that clients can display the + # emoji in a less distracting (not animated) fashion when + # desired. + emoji_dict["still_url"] = get_emoji_url( + realm_emoji.file_name, realm_emoji.realm_id, still=True + ) + + d[str(realm_emoji.id)] = emoji_dict + return d diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 6d1294e2a5..320b12f967 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -11471,6 +11471,15 @@ paths: "name": "green_tick", "source_url": "/user_avatars/1/emoji/images/1.png", }, + "2": + { + "author_id": 3, + "deactivated": false, + "id": "2", + "name": "animated_img", + "source_url": "/user_avatars/1/emoji/images/animated_img.gif", + "still_url": "/user_avatars/1/emoji/images/still/animated_img.png", + }, }, "result": "success", "zulip_feature_level": 2, @@ -13709,6 +13718,20 @@ components: description: | The path relative to the organization's URL where the emoji's image can be found. + still_url: + type: string + description: | + Only present when the emoji's image is animated. + + The path relative to the organization's URL where a still + (not animated) version of the emoji can be found. (This is + currently always the first frame of the animation). + + This is useful for clients to display the emoji in contexts + where continuously animating it would be a bad user experience + (E.g. because it would be distracting). + + **Changes**: New in Zulip 5.0 (feature level 97). deactivated: type: boolean description: | diff --git a/zerver/tests/test_transfer.py b/zerver/tests/test_transfer.py index cbd2bd9c31..85f8b5f1c7 100644 --- a/zerver/tests/test_transfer.py +++ b/zerver/tests/test_transfer.py @@ -1,3 +1,4 @@ +import os from unittest.mock import Mock, patch from django.conf import settings @@ -99,7 +100,43 @@ class TransferUploadsToS3Test(ZulipTestCase): with get_test_image_file("img.png") as image_file: image_data = image_file.read() - resized_image_data = resize_emoji(image_data) + resized_image_data, is_animated, still_image_data = resize_emoji(image_data) + self.assertEqual(is_animated, False) + self.assertEqual(still_image_data, None) self.assertEqual(image_data, original_key.get()["Body"].read()) self.assertEqual(resized_image_data, resized_key.get()["Body"].read()) + + 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: + raise AssertionError("Unable to add emoji.") + + emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( + realm_id=othello.realm_id, + emoji_file_name=emoji.file_name, + ) + + with self.assertLogs(level="INFO"): + transfer_emoji_to_s3(1) + + self.assert_length(list(bucket.objects.all()), 5) + original_key = bucket.Object(emoji_path + ".original") + resized_key = bucket.Object(emoji_path) + assert emoji.file_name + still_key = bucket.Object( + RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=othello.realm_id, + emoji_filename_without_extension=os.path.splitext(emoji.file_name)[0], + ) + ) + + with get_test_image_file("animated_img.gif") as image_file: + image_data = image_file.read() + resized_image_data, is_animated, still_image_data = resize_emoji(image_data) + + self.assertEqual(is_animated, True) + self.assertEqual(type(still_image_data), bytes) + self.assertEqual(image_data, original_key.get()["Body"].read()) + self.assertEqual(resized_image_data, resized_key.get()["Body"].read()) + self.assertEqual(still_image_data, still_key.get()["Body"].read()) diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 76ccdc9647..87268b7c49 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -1271,9 +1271,15 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): # Test unequal width and height of animated GIF image with get_test_image_file("animated_unequal_img.gif") as f: animated_unequal_img_data = f.read() - resized_img_data = resize_emoji(animated_unequal_img_data, size=50) + resized_img_data, is_animated, still_img_data = resize_emoji( + animated_unequal_img_data, size=50 + ) im = Image.open(io.BytesIO(resized_img_data)) self.assertEqual((50, 50), im.size) + self.assertTrue(is_animated) + assert still_img_data is not None + still_image = Image.open(io.BytesIO(still_img_data)) + self.assertEqual((50, 50), still_image.size) # Test corrupt image exception with get_test_image_file("corrupt.gif") as f: @@ -1285,25 +1291,45 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): with get_test_image_file("animated_large_img.gif") as f: animated_large_img_data = f.read() with patch("zerver.lib.upload.MAX_EMOJI_GIF_SIZE", 128): - resized_img_data = resize_emoji(animated_large_img_data, size=50) + resized_img_data, is_animated, still_img_data = resize_emoji( + animated_large_img_data, size=50 + ) im = Image.open(io.BytesIO(resized_img_data)) self.assertEqual((50, 50), im.size) + self.assertTrue(is_animated) + assert still_img_data + still_image = Image.open(io.BytesIO(still_img_data)) + self.assertEqual((50, 50), still_image.size) # Test an image file larger than max is resized with get_test_image_file("animated_large_img.gif") as f: animated_large_img_data = f.read() with patch("zerver.lib.upload.MAX_EMOJI_GIF_FILE_SIZE_BYTES", 3 * 1024 * 1024): - resized_img_data = resize_emoji(animated_large_img_data, size=50) + resized_img_data, is_animated, still_img_data = resize_emoji( + animated_large_img_data, size=50 + ) im = Image.open(io.BytesIO(resized_img_data)) self.assertEqual((50, 50), im.size) + self.assertTrue(is_animated) + assert still_img_data is not None + still_image = Image.open(io.BytesIO(still_img_data)) + self.assertEqual((50, 50), still_image.size) # Test an image smaller than max and smaller than file size max is not resized with get_test_image_file("animated_large_img.gif") as f: animated_large_img_data = f.read() with patch("zerver.lib.upload.MAX_EMOJI_GIF_SIZE", 512): - resized_img_data = resize_emoji(animated_large_img_data, size=50) + resized_img_data, is_animated, still_img_data = resize_emoji( + animated_large_img_data, size=50 + ) im = Image.open(io.BytesIO(resized_img_data)) self.assertEqual((256, 256), im.size) + self.assertTrue(is_animated) + + # We unconditionally resize the still_image + assert still_img_data is not None + still_image = Image.open(io.BytesIO(still_img_data)) + self.assertEqual((50, 50), still_image.size) def tearDown(self) -> None: destroy_uploads() @@ -1714,6 +1740,13 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): upload_emoji_image(image_file, file_name, user_profile) url = zerver.lib.upload.upload_backend.get_emoji_url(file_name, user_profile.realm_id) + # Verify the assert statement for trying to fetch the still + # version of a non-GIF image, since we only support animated GIFs. + with self.assertRaises(AssertionError): + still_url = zerver.lib.upload.upload_backend.get_emoji_url( + file_name, user_profile.realm_id, still=True + ) + emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( realm_id=user_profile.realm_id, emoji_file_name=file_name, @@ -1721,6 +1754,28 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): expected_url = f"/user_avatars/{emoji_path}" self.assertEqual(expected_url, url) + file_name = "emoji.gif" + with get_test_image_file("animated_img.gif") as image_file: + upload_emoji_image(image_file, file_name, user_profile) + url = zerver.lib.upload.upload_backend.get_emoji_url(file_name, user_profile.realm_id) + still_url = zerver.lib.upload.upload_backend.get_emoji_url( + file_name, user_profile.realm_id, still=True + ) + + emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format( + realm_id=user_profile.realm_id, + emoji_file_name=file_name, + ) + + still_emoji_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=user_profile.realm_id, + emoji_filename_without_extension=os.path.splitext(file_name)[0], + ) + expected_url = f"/user_avatars/{emoji_path}" + self.assertEqual(expected_url, url) + expected_still_url = f"/user_avatars/{still_emoji_path}" + self.assertEqual(expected_still_url, still_url) + def test_tarball_upload_and_deletion_local(self) -> None: user_profile = self.example_user("iago") self.assertTrue(user_profile.is_realm_admin) @@ -2075,6 +2130,23 @@ class S3Test(ZulipTestCase): expected_url = f"https://{bucket}.s3.amazonaws.com/{path}" self.assertEqual(expected_url, url) + emoji_name = "animated_image.gif" + + path = RealmEmoji.PATH_ID_TEMPLATE.format(realm_id=realm_id, emoji_file_name=emoji_name) + still_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=realm_id, emoji_filename_without_extension=os.path.splitext(emoji_name)[0] + ) + + url = zerver.lib.upload.upload_backend.get_emoji_url("animated_image.gif", realm_id) + still_url = zerver.lib.upload.upload_backend.get_emoji_url( + "animated_image.gif", realm_id, still=True + ) + + expected_url = f"https://{bucket}.s3.amazonaws.com/{path}" + self.assertEqual(expected_url, url) + expected_still_url = f"https://{bucket}.s3.amazonaws.com/{still_path}" + self.assertEqual(expected_still_url, still_url) + @use_s3_backend def test_tarball_upload_and_deletion(self) -> None: bucket = create_s3_buckets(settings.S3_AVATAR_BUCKET)[0]