From 976e554799a03ff9d82d7b75d77d45985cd25df4 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 27 Apr 2020 22:29:08 -0700 Subject: [PATCH] Revert "home: Don't use pointer to compute furthest_read_time." This reverts commit 8f32db81a1a1230b425aef8f7c6d2b7fc4543ee7. This change unfortunately requires an index that we don't have, and thus is incredibly expensive. We'll need to do a thoughtful reworking before we can integrate it again. --- zerver/models.py | 4 ---- zerver/tests/test_home.py | 18 ++++-------------- zerver/views/home.py | 7 +++++-- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/zerver/models.py b/zerver/models.py index 555bb0ea2f..24376f00c8 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1962,10 +1962,6 @@ def get_usermessage_by_message_id(user_profile: UserProfile, message_id: int) -> except UserMessage.DoesNotExist: return None -def get_latest_read_usermessage(user_profile: UserProfile) -> Optional[UserMessage]: - return UserMessage.objects.select_related().filter(user_profile=user_profile, - flags=UserMessage.flags.read).last() - class ArchivedUserMessage(AbstractUserMessage): """Used as a temporary holding place for deleted UserMessages objects before they are permanently deleted. This is an important part of diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 93fa95072b..4223239cca 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -22,7 +22,7 @@ from zerver.lib.users import compute_show_invites_and_add_streams from zerver.models import ( get_realm, get_stream, get_user, UserProfile, flush_per_request_caches, DefaultStream, Realm, - get_system_bot, UserMessage + get_system_bot, ) from zerver.views.home import sent_time_in_epoch_seconds, compute_navbar_logo_url from corporate.models import Customer, CustomerPlan @@ -458,19 +458,9 @@ class HomeTest(ZulipTestCase): user_profile.save() self.login_user(user_profile) - result = self._get_home_page() - self._sanity_check(result) - - def test_no_read_user_messages(self) -> None: - user_profile = self.example_user('hamlet') - user_profile.pointer = 1 - user_profile.save() - - # Mark all messages as unread - UserMessage.objects.filter(user_profile=user_profile).update(flags=~UserMessage.flags.read) - - self.login_user(user_profile) - result = self._get_home_page() + with patch('logging.warning') as mock: + result = self._get_home_page() + mock.assert_called_once_with('User %s has invalid pointer 999999' % (user_profile.id,)) self._sanity_check(result) def test_topic_narrow(self) -> None: diff --git a/zerver/views/home.py b/zerver/views/home.py index f35e00f05b..c3f37b3f4e 100644 --- a/zerver/views/home.py +++ b/zerver/views/home.py @@ -13,7 +13,7 @@ from zerver.forms import ToSForm from zerver.models import Message, Stream, UserProfile, \ Realm, UserMessage, \ PreregistrationUser, \ - get_latest_read_usermessage + get_usermessage_by_message_id from zerver.lib.events import do_events_register from zerver.lib.actions import do_change_tos_version, \ realm_user_count @@ -223,7 +223,10 @@ def home_real(request: HttpRequest) -> HttpResponse: register_ret['pointer'] = register_ret['max_message_id'] furthest_read_time = None else: - latest_read = get_latest_read_usermessage(user_profile) + latest_read = get_usermessage_by_message_id(user_profile, user_profile.pointer) + if latest_read is None: + # Don't completely fail if your saved pointer ID is invalid + logging.warning("User %s has invalid pointer %s" % (user_profile.id, user_profile.pointer)) furthest_read_time = sent_time_in_epoch_seconds(latest_read) # We pick a language for the user as follows: