realm emoji: Use a single cache for all lookups.

The active realm emoji are just a subset of all your
realm emoji, so just use a single cache entry per
realm.

Cache misses should be very infrequent per realm.

If a realm has lots of deactivated realm emoji, then
there's a minor expense to deserialize them, but that
is gonna be dwarfed by all the other more expensive
operations in message-send.

I also renamed the two related functions.  I erred on
the side of using somewhat verbose names, as we don't
want folks to confuse the two use cases. Fortunately
there are somewhat natural affordances to use one or
the other, and mypy helps too.

Finally, I use realm_id instead of realm in places
where we don't need the full Realm object.

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,

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."))

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:

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 = {}
 

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

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

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

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