cache: Avoid flushing invalid cache for realm emoji.

In certain cases, we call `RealmEmoji.save()` before the filename
becomes available. This result in getting invalid urls generated and
flushed. Normally we call it again shortly after, making it harder to
trigger this bug.

Fixes #22552.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This commit is contained in:
Zixuan James Li 2022-08-05 17:10:04 -04:00 committed by Tim Abbott
parent 5a29f4133b
commit 172a166159
2 changed files with 12 additions and 1 deletions

View File

@ -1121,6 +1121,7 @@ def get_realm_emoji_dicts(realm: Realm, only_active_emojis: bool = False) -> Dic
author_id = None author_id = None
if realm_emoji.author: if realm_emoji.author:
author_id = realm_emoji.author_id author_id = realm_emoji.author_id
assert realm_emoji.file_name is not None
emoji_url = get_emoji_url(realm_emoji.file_name, realm_emoji.realm_id) emoji_url = get_emoji_url(realm_emoji.file_name, realm_emoji.realm_id)
emoji_dict: EmojiInfo = dict( emoji_dict: EmojiInfo = dict(
@ -1159,6 +1160,16 @@ def get_active_realm_emoji_uncached(realm: Realm) -> Dict[str, EmojiInfo]:
def flush_realm_emoji(*, instance: RealmEmoji, **kwargs: object) -> None: def flush_realm_emoji(*, instance: RealmEmoji, **kwargs: object) -> None:
if instance.file_name is None:
# Because we construct RealmEmoji.file_name using the ID for
# the RealmEmoji object, it will always have file_name=None,
# and then it'll be updated with the actual filename as soon
# as the upload completes successfully.
#
# Doing nothing when file_name=None is the best option, since
# such an object shouldn't have been cached yet, and this
# function will be called again when file_name is set.
return
realm = instance.realm realm = instance.realm
cache_set( cache_set(
get_realm_emoji_cache_key(realm), get_realm_emoji_uncached(realm), timeout=3600 * 24 * 7 get_realm_emoji_cache_key(realm), get_realm_emoji_uncached(realm), timeout=3600 * 24 * 7

View File

@ -23,7 +23,7 @@ class RealmEmojiTest(ZulipTestCase):
return realm_emoji return realm_emoji
def create_test_emoji_with_no_author(self, name: str, realm: Realm) -> RealmEmoji: def create_test_emoji_with_no_author(self, name: str, realm: Realm) -> RealmEmoji:
realm_emoji = RealmEmoji.objects.create(realm=realm, name=name) realm_emoji = RealmEmoji.objects.create(realm=realm, name=name, file_name=name)
return realm_emoji return realm_emoji
def test_list(self) -> None: def test_list(self) -> None: