Fix bug in flush_user_profile().

Every time we updated a UserProfile object, we were calling
delete_display_recipient_cache(), which churns the cache and
does an extra database hop to find subscriptions.  This was
due to saying `updated_fields` instead of `update_fields`.

This made us prone to cache churn for fields like UserProfile.pointer
that are fairly volatile.

Now we use the helper function changed().  To prevent the
opposite problem, we use all the fields that could invalidate
the cache.
This commit is contained in:
Steve Howell
2017-10-21 18:14:44 -07:00
committed by Tim Abbott
parent c8875693c8
commit 14d2d4e506
5 changed files with 20 additions and 5 deletions

View File

@@ -401,8 +401,7 @@ def flush_user_profile(sender, **kwargs):
if changed(['is_active']):
cache_delete(active_user_ids_cache_key(user_profile.realm_id))
if kwargs.get('updated_fields') is None or \
'email' in kwargs['update_fields']:
if changed(['email', 'full_name', 'short_name', 'id', 'is_mirror_dummy']):
delete_display_recipient_cache(user_profile)
# Invalidate our bots_in_realm info dict if any bot has

View File

@@ -178,7 +178,7 @@ class HomeTest(ZulipTestCase):
with patch('zerver.lib.cache.cache_set') as cache_mock:
result = self._get_home_page(stream='Denmark')
self.assert_length(queries, 41)
self.assert_length(queries, 40)
self.assert_length(cache_mock.call_args_list, 10)
html = result.content.decode('utf-8')

View File

@@ -2412,7 +2412,7 @@ class SoftDeactivationMessageTest(ZulipTestCase):
self.assertNotEqual(idle_user_msg_list[-1].content, message)
with queries_captured() as queries:
maybe_catch_up_soft_deactivated_user(long_term_idle_user)
self.assert_length(queries, 8)
self.assert_length(queries, 7)
self.assertFalse(long_term_idle_user.long_term_idle)
self.assertEqual(last_realm_audit_log_entry(
'user_soft_activated').modified_user, long_term_idle_user)

View File

@@ -324,7 +324,7 @@ class LoginTest(ZulipTestCase):
with queries_captured() as queries:
self.register(self.nonreg_email('test'), "test")
# Ensure the number of queries we make is not O(streams)
self.assert_length(queries, 68)
self.assert_length(queries, 67)
user_profile = self.nonreg_user('test')
self.assertEqual(get_session_dict_user(self.client.session), user_profile.id)
self.assertFalse(user_profile.enable_stream_desktop_notifications)

View File

@@ -38,6 +38,7 @@ from zerver.lib.stream_topic import StreamTopicTarget
from django.conf import settings
import datetime
import mock
import os
import sys
import time
@@ -260,6 +261,21 @@ class UserProfileTest(ZulipTestCase):
self.assertEqual(dct[hamlet.id], self.example_email("hamlet"))
self.assertEqual(dct[othello.id], self.example_email("othello"))
def test_cache_invalidation(self):
# type: () -> None
hamlet = self.example_user('hamlet')
with mock.patch('zerver.lib.cache.delete_display_recipient_cache') as m:
hamlet.full_name = 'Hamlet Junior'
hamlet.save(update_fields=["full_name"])
self.assertTrue(m.called)
with mock.patch('zerver.lib.cache.delete_display_recipient_cache') as m:
hamlet.long_term_idle = True
hamlet.save(update_fields=["long_term_idle"])
self.assertFalse(m.called)
class ActivateTest(ZulipTestCase):
def test_basics(self):
# type: () -> None