diff --git a/zerver/actions/realm_emoji.py b/zerver/actions/realm_emoji.py index a2faf6e8a1..9c67dd316a 100644 --- a/zerver/actions/realm_emoji.py +++ b/zerver/actions/realm_emoji.py @@ -11,7 +11,15 @@ from zerver.lib.emoji import get_emoji_file_name from zerver.lib.exceptions import JsonableError from zerver.lib.pysa import mark_sanitized from zerver.lib.upload import upload_emoji_image -from zerver.models import EmojiInfo, Realm, RealmAuditLog, RealmEmoji, UserProfile, active_user_ids +from zerver.models import ( + EmojiInfo, + Realm, + RealmAuditLog, + RealmEmoji, + UserProfile, + active_user_ids, + get_all_custom_emoji_for_realm, +) from zerver.tornado.django_api import send_event_on_commit @@ -49,7 +57,7 @@ def check_add_realm_emoji( realm_emoji.is_animated = is_animated realm_emoji.save(update_fields=["file_name", "is_animated"]) - realm_emoji_dict = realm.get_emoji() + realm_emoji_dict = get_all_custom_emoji_for_realm(realm.id) RealmAuditLog.objects.create( realm=realm, acting_user=author, @@ -72,7 +80,7 @@ def do_remove_realm_emoji(realm: Realm, name: str, *, acting_user: Optional[User emoji.deactivated = True emoji.save(update_fields=["deactivated"]) - realm_emoji_dict = realm.get_emoji() + realm_emoji_dict = get_all_custom_emoji_for_realm(realm.id) RealmAuditLog.objects.create( realm=realm, acting_user=acting_user, diff --git a/zerver/lib/emoji.py b/zerver/lib/emoji.py index c406250337..56dd59c48f 100644 --- a/zerver/lib/emoji.py +++ b/zerver/lib/emoji.py @@ -9,7 +9,14 @@ from django.utils.translation import gettext as _ from zerver.lib.exceptions import JsonableError from zerver.lib.storage import static_path from zerver.lib.upload import upload_backend -from zerver.models import Reaction, Realm, RealmEmoji, UserProfile +from zerver.models import ( + Reaction, + Realm, + RealmEmoji, + UserProfile, + get_all_custom_emoji_for_realm, + get_name_keyed_dict_for_active_realm_emoji, +) emoji_codes_path = static_path("generated/emoji/emoji_codes.json") if not os.path.exists(emoji_codes_path): # nocoverage @@ -56,10 +63,12 @@ def translate_emoticons(text: str) -> str: def emoji_name_to_emoji_code(realm: Realm, emoji_name: str) -> Tuple[str, str]: - realm_emojis = realm.get_active_emoji() - realm_emoji = realm_emojis.get(emoji_name) + # TODO: Just ask callers for realm_id. Also consider returning a + # tiny dataclass instead of a tuple. + realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm.id) + realm_emoji = realm_emoji_dict.get(emoji_name) if realm_emoji is not None: - return str(realm_emojis[emoji_name]["id"]), Reaction.REALM_EMOJI + return str(realm_emoji["id"]), Reaction.REALM_EMOJI if emoji_name == "zulip": return emoji_name, Reaction.ZULIP_EXTRA_EMOJI if emoji_name in name_to_codepoint: @@ -71,7 +80,10 @@ def check_emoji_request(realm: Realm, emoji_name: str, emoji_code: str, emoji_ty # For a given realm and emoji type, checks whether an emoji # code is valid for new reactions, or not. if emoji_type == "realm_emoji": - realm_emojis = realm.get_emoji() + # We cache emoji, so this generally avoids a round trip, + # but it does require deserializing a bigger data structure + # than we need. + realm_emojis = get_all_custom_emoji_for_realm(realm.id) realm_emoji = realm_emojis.get(emoji_code) if realm_emoji is None: raise JsonableError(_("Invalid custom emoji.")) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 448af85a90..3215eaa532 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -67,6 +67,7 @@ from zerver.models import ( UserStatus, UserTopic, custom_profile_fields_for_realm, + get_all_custom_emoji_for_realm, get_default_stream_groups, get_realm_domains, get_realm_playgrounds, @@ -384,7 +385,7 @@ def fetch_initial_state_data( state["realm_domains"] = get_realm_domains(realm) if want("realm_emoji"): - state["realm_emoji"] = realm.get_emoji() + state["realm_emoji"] = get_all_custom_emoji_for_realm(realm.id) if want("realm_linkifiers"): if linkifier_url_template: diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py index 5e8b750ada..9e93f8b9bd 100644 --- a/zerver/lib/markdown/__init__.py +++ b/zerver/lib/markdown/__init__.py @@ -70,7 +70,13 @@ from zerver.lib.timezone import common_timezones from zerver.lib.types import LinkifierDict from zerver.lib.url_encoding import encode_stream, hash_util_encode from zerver.lib.url_preview.types import UrlEmbedData, UrlOEmbedData -from zerver.models import EmojiInfo, Message, Realm, linkifiers_for_realm +from zerver.models import ( + EmojiInfo, + Message, + Realm, + get_name_keyed_dict_for_active_realm_emoji, + linkifiers_for_realm, +) ReturnT = TypeVar("ReturnT") @@ -2537,7 +2543,7 @@ def do_convert( stream_name_info = mention_data.get_stream_name_map(stream_names) if content_has_emoji_syntax(content): - active_realm_emoji = message_realm.get_active_emoji() + active_realm_emoji = get_name_keyed_dict_for_active_realm_emoji(message_realm.id) else: active_realm_emoji = {} diff --git a/zerver/models.py b/zerver/models.py index 159be1eb52..2af0a1960b 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -239,14 +239,10 @@ def get_recipient_ids( return to, recipient_type_str -def get_realm_emoji_cache_key(realm_id: int) -> str: +def get_all_custom_emoji_for_realm_cache_key(realm_id: int) -> str: return f"realm_emoji:{realm_id}" -def get_active_realm_emoji_cache_key(realm_id: int) -> str: - return f"active_realm_emoji:{realm_id}" - - # This simple call-once caching saves ~500us in auth_enabled_helper, # which is a significant optimization for common_context. Note that # these values cannot change in a running production system, but do @@ -837,16 +833,6 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub ret[realm_authentication_method.name] = True return ret - # `realm` instead of `self` here to make sure the parameters of the cache key - # function matches the original method. - @cache_with_key(lambda realm: get_realm_emoji_cache_key(realm.id), timeout=3600 * 24 * 7) - def get_emoji(realm) -> Dict[str, EmojiInfo]: # noqa: N805 - return get_realm_emoji_uncached(realm.id) - - @cache_with_key(lambda realm: get_active_realm_emoji_cache_key(realm.id), timeout=3600 * 24 * 7) - def get_active_emoji(realm) -> Dict[str, EmojiInfo]: # noqa: N805 - return get_active_realm_emoji_uncached(realm.id) - def get_admin_users_and_bots( self, include_realm_owners: bool = True ) -> QuerySet["UserProfile"]: @@ -1161,7 +1147,7 @@ class RealmEmoji(models.Model): return f"{self.realm.string_id}: {self.id} {self.name} {self.deactivated} {self.file_name}" -def get_realm_emoji_dicts(realm_id: int, only_active_emojis: bool = False) -> Dict[str, EmojiInfo]: +def get_all_custom_emoji_for_realm_uncached(realm_id: int) -> Dict[str, EmojiInfo]: # RealmEmoji objects with file_name=None are still in the process # of being uploaded, and we expect to be cleaned up by a # try/finally block if the upload fails, so it's correct to @@ -1169,8 +1155,6 @@ def get_realm_emoji_dicts(realm_id: int, only_active_emojis: bool = False) -> Di query = RealmEmoji.objects.filter(realm_id=realm_id).exclude( file_name=None, ) - if only_active_emojis: - query = query.filter(deactivated=False) d = {} from zerver.lib.emoji import get_emoji_url @@ -1202,16 +1186,15 @@ def get_realm_emoji_dicts(realm_id: int, only_active_emojis: bool = False) -> Di return d -def get_realm_emoji_uncached(realm_id: int) -> Dict[str, EmojiInfo]: - return get_realm_emoji_dicts(realm_id) +@cache_with_key(get_all_custom_emoji_for_realm_cache_key, timeout=3600 * 24 * 7) +def get_all_custom_emoji_for_realm(realm_id: int) -> Dict[str, EmojiInfo]: + return get_all_custom_emoji_for_realm_uncached(realm_id) -def get_active_realm_emoji_uncached(realm_id: int) -> Dict[str, EmojiInfo]: - realm_emojis = get_realm_emoji_dicts(realm_id, only_active_emojis=True) - d = {} - for emoji_id, emoji_dict in realm_emojis.items(): - d[emoji_dict["name"]] = emoji_dict - return d +def get_name_keyed_dict_for_active_realm_emoji(realm_id: int) -> Dict[str, EmojiInfo]: + # It's important to use the cached version here. + realm_emojis = get_all_custom_emoji_for_realm(realm_id) + return {row["name"]: row for row in realm_emojis.values() if not row["deactivated"]} def flush_realm_emoji(*, instance: RealmEmoji, **kwargs: object) -> None: @@ -1227,13 +1210,8 @@ def flush_realm_emoji(*, instance: RealmEmoji, **kwargs: object) -> None: return realm_id = instance.realm_id cache_set( - get_realm_emoji_cache_key(realm_id), - get_realm_emoji_uncached(realm_id), - timeout=3600 * 24 * 7, - ) - cache_set( - get_active_realm_emoji_cache_key(realm_id), - get_active_realm_emoji_uncached(realm_id), + get_all_custom_emoji_for_realm_cache_key(realm_id), + get_all_custom_emoji_for_realm_uncached(realm_id), timeout=3600 * 24 * 7, ) diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py index 9446573f49..70cf29d197 100644 --- a/zerver/tests/test_audit_log.py +++ b/zerver/tests/test_audit_log.py @@ -84,6 +84,7 @@ from zerver.models import ( Subscription, UserGroup, UserProfile, + get_all_custom_emoji_for_realm, get_realm, get_realm_domains, get_realm_playgrounds, @@ -1001,7 +1002,7 @@ class TestRealmAuditLog(ZulipTestCase): def test_realm_emoji_entries(self) -> None: user = self.example_user("iago") - realm_emoji_dict = user.realm.get_emoji() + realm_emoji_dict = get_all_custom_emoji_for_realm(user.realm_id) now = timezone_now() with get_test_image_file("img.png") as img_file: # Because we want to verify the IntegrityError handling diff --git a/zerver/tests/test_email_notifications.py b/zerver/tests/test_email_notifications.py index a4d0862e0d..78f1c9240f 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -42,6 +42,7 @@ from zerver.models import ( UserMessage, UserProfile, UserTopic, + get_name_keyed_dict_for_active_realm_emoji, get_realm, get_stream, ) @@ -1492,7 +1493,8 @@ class TestMissedMessages(ZulipTestCase): self.example_user("hamlet"), "Extremely personal message with a realm emoji :green_tick:!", ) - realm_emoji_id = realm.get_active_emoji()["green_tick"]["id"] + realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm.id) + realm_emoji_id = realm_emoji_dict["green_tick"]["id"] realm_emoji_url = ( f"http://zulip.testserver/user_avatars/{realm.id}/emoji/images/{realm_emoji_id}.png" ) diff --git a/zerver/views/realm_emoji.py b/zerver/views/realm_emoji.py index 4d72a0d9f6..e7389a0a2f 100644 --- a/zerver/views/realm_emoji.py +++ b/zerver/views/realm_emoji.py @@ -9,13 +9,15 @@ from zerver.lib.emoji import check_remove_custom_emoji, check_valid_emoji_name, from zerver.lib.exceptions import JsonableError, ResourceNotFoundError from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success -from zerver.models import RealmEmoji, UserProfile +from zerver.models import RealmEmoji, UserProfile, get_all_custom_emoji_for_realm def list_emoji(request: HttpRequest, user_profile: UserProfile) -> HttpResponse: # We don't do any checks here because the list of realm # emoji is public. - return json_success(request, data={"emoji": user_profile.realm.get_emoji()}) + return json_success( + request, data=dict(emoji=get_all_custom_emoji_for_realm(user_profile.realm_id)) + ) @require_member_or_admin