From 352dbf9387acae3123c85ab04c9e793c95d1cddf Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Mon, 4 Mar 2024 20:08:53 +0000 Subject: [PATCH] caches: Only take the realm_id, not the Realm, as a cache key function. This saves a hit to the database to fetch the Realm of a UserProfile that we are trying to flush. --- zerver/actions/message_send.py | 2 +- zerver/actions/user_settings.py | 2 +- zerver/lib/cache.py | 16 ++++++++-------- zerver/lib/cache_helpers.py | 4 ++-- zerver/tests/test_soft_deactivation.py | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/zerver/actions/message_send.py b/zerver/actions/message_send.py index 97e13a131b..d58a8279a5 100644 --- a/zerver/actions/message_send.py +++ b/zerver/actions/message_send.py @@ -122,7 +122,7 @@ def compute_jabber_user_fullname(email: str) -> str: def get_user_profile_delivery_email_cache_key( realm: Realm, email: str, email_to_fullname: Callable[[str], str] ) -> str: - return user_profile_delivery_email_cache_key(email, realm) + return user_profile_delivery_email_cache_key(email, realm.id) @cache_with_key( diff --git a/zerver/actions/user_settings.py b/zerver/actions/user_settings.py index 1c0405dff5..aa23990884 100644 --- a/zerver/actions/user_settings.py +++ b/zerver/actions/user_settings.py @@ -109,7 +109,7 @@ def send_delivery_email_update_events( @transaction.atomic(savepoint=False) def do_change_user_delivery_email(user_profile: UserProfile, new_email: str) -> None: - delete_user_profile_caches([user_profile], user_profile.realm) + delete_user_profile_caches([user_profile], user_profile.realm_id) user_profile.delivery_email = new_email if user_profile.email_address_is_realm_public(): diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index 90dfdca49a..0200718ffd 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -431,8 +431,8 @@ def user_profile_cache_key(email: str, realm: "Realm") -> str: return user_profile_cache_key_id(email, realm.id) -def user_profile_delivery_email_cache_key(delivery_email: str, realm: "Realm") -> str: - return f"user_profile_by_delivery_email:{hashlib.sha1(delivery_email.strip().encode()).hexdigest()}:{realm.id}" +def user_profile_delivery_email_cache_key(delivery_email: str, realm_id: int) -> str: + return f"user_profile_by_delivery_email:{hashlib.sha1(delivery_email.strip().encode()).hexdigest()}:{realm_id}" def bot_profile_cache_key(email: str, realm_id: int) -> str: @@ -515,7 +515,7 @@ def bot_dicts_in_realm_cache_key(realm_id: int) -> str: return f"bot_dicts_in_realm:{realm_id}" -def delete_user_profile_caches(user_profiles: Iterable["UserProfile"], realm: "Realm") -> None: +def delete_user_profile_caches(user_profiles: Iterable["UserProfile"], realm_id: int) -> None: # Imported here to avoid cyclic dependency. from zerver.lib.users import get_all_api_keys from zerver.models.users import is_cross_realm_bot_email @@ -524,11 +524,11 @@ def delete_user_profile_caches(user_profiles: Iterable["UserProfile"], realm: "R for user_profile in user_profiles: keys.append(user_profile_by_id_cache_key(user_profile.id)) keys += map(user_profile_by_api_key_cache_key, get_all_api_keys(user_profile)) - keys.append(user_profile_cache_key(user_profile.email, realm)) - keys.append(user_profile_delivery_email_cache_key(user_profile.delivery_email, realm)) + keys.append(user_profile_cache_key_id(user_profile.email, realm_id)) + keys.append(user_profile_delivery_email_cache_key(user_profile.delivery_email, realm_id)) if user_profile.is_bot and is_cross_realm_bot_email(user_profile.email): # Handle clearing system bots from their special cache. - keys.append(bot_profile_cache_key(user_profile.email, realm.id)) + keys.append(bot_profile_cache_key(user_profile.email, realm_id)) keys.append(get_cross_realm_dicts_key()) cache_delete_many(keys) @@ -563,7 +563,7 @@ def flush_user_profile( **kwargs: object, ) -> None: user_profile = instance - delete_user_profile_caches([user_profile], user_profile.realm) + delete_user_profile_caches([user_profile], user_profile.realm_id) # Invalidate our active_users_in_realm info dict if any user has changed # the fields in the dict or become (in)active @@ -603,7 +603,7 @@ def flush_realm( ) -> None: realm = instance users = realm.get_active_users() - delete_user_profile_caches(users, realm) + delete_user_profile_caches(users, realm.id) if ( from_deletion diff --git a/zerver/lib/cache_helpers.py b/zerver/lib/cache_helpers.py index e54b27a4b9..5dbd3efb2e 100644 --- a/zerver/lib/cache_helpers.py +++ b/zerver/lib/cache_helpers.py @@ -19,7 +19,7 @@ from zerver.lib.cache import ( get_remote_cache_requests, get_remote_cache_time, user_profile_by_api_key_cache_key, - user_profile_cache_key, + user_profile_cache_key_id, ) from zerver.lib.safe_session_cached_db import SessionStore from zerver.lib.sessions import session_engine @@ -33,7 +33,7 @@ def user_cache_items( ) -> None: for api_key in get_all_api_keys(user_profile): items_for_remote_cache[user_profile_by_api_key_cache_key(api_key)] = (user_profile,) - items_for_remote_cache[user_profile_cache_key(user_profile.email, user_profile.realm)] = ( + items_for_remote_cache[user_profile_cache_key_id(user_profile.email, user_profile.realm_id)] = ( user_profile, ) # We have other user_profile caches, but none of them are on the diff --git a/zerver/tests/test_soft_deactivation.py b/zerver/tests/test_soft_deactivation.py index d7dea720c7..0b0a44118e 100644 --- a/zerver/tests/test_soft_deactivation.py +++ b/zerver/tests/test_soft_deactivation.py @@ -398,7 +398,7 @@ class SoftDeactivationMessageTest(ZulipTestCase): idle_user_msg_list = get_user_messages(long_term_idle_user) idle_user_msg_count = len(idle_user_msg_list) self.assertNotEqual(idle_user_msg_list[-1], sent_message) - with self.assert_database_query_count(6): + with self.assert_database_query_count(5): add_missing_messages(long_term_idle_user) idle_user_msg_list = get_user_messages(long_term_idle_user) self.assert_length(idle_user_msg_list, idle_user_msg_count + 1)