From 803982e87254e3b1ebcb16ed795e224afceea3a3 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 12 May 2022 18:50:40 -0700 Subject: [PATCH] message_flags: Short-circuit if no messages changed. Omit sending an event, and updating the database, if there are no matching messages. --- zerver/actions/message_flags.py | 6 ++++++ zerver/tests/test_events.py | 7 +++---- zerver/tests/test_subs.py | 26 ++++++++++++-------------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/zerver/actions/message_flags.py b/zerver/actions/message_flags.py index ad3666d65d..ced7723f82 100644 --- a/zerver/actions/message_flags.py +++ b/zerver/actions/message_flags.py @@ -98,6 +98,9 @@ def do_mark_stream_messages_as_read( message_ids = list(msgs.values_list("message_id", flat=True)) + if len(message_ids) == 0: + return 0 + count = msgs.update( flags=F("flags").bitor(UserMessage.flags.read), ) @@ -136,6 +139,9 @@ def do_mark_muted_user_messages_as_read( message_ids = list(messages.values_list("message_id", flat=True)) + if len(message_ids) == 0: + return 0 + count = messages.update( flags=F("flags").bitor(UserMessage.flags.read), ) diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 35b46991dc..7596107aef 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1340,10 +1340,9 @@ class NormalActionsTest(BaseAction): def test_muted_users_events(self) -> None: muted_user = self.example_user("othello") events = self.verify_action( - lambda: do_mute_user(self.user_profile, muted_user), num_events=2 + lambda: do_mute_user(self.user_profile, muted_user), num_events=1 ) - check_update_message_flags_add("events[0]", events[0]) - check_muted_users("events[1]", events[1]) + check_muted_users("events[0]", events[0]) mute_object = get_mute_object(self.user_profile, muted_user) assert mute_object is not None @@ -2678,7 +2677,7 @@ class SubscribeActionTest(BaseAction): # Now remove the user himself, to test the 'remove' event flow action = lambda: bulk_remove_subscriptions(realm, [hamlet], [stream], acting_user=None) events = self.verify_action( - action, include_subscribers=include_subscribers, include_streams=False, num_events=2 + action, include_subscribers=include_subscribers, include_streams=False, num_events=1 ) check_subscription_remove("events[0]", events[0]) self.assert_length(events[0]["subscriptions"], 1) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index f556ecd59b..42b56f310a 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -2431,7 +2431,7 @@ class StreamAdminTest(ZulipTestCase): those you aren't on. """ result = self.attempt_unsubscribe_of_principal( - query_count=16, + query_count=15, target_users=[self.example_user("cordelia")], is_realm_admin=True, is_subbed=True, @@ -2457,7 +2457,7 @@ class StreamAdminTest(ZulipTestCase): self.example_user(name) for name in ["cordelia", "prospero", "iago", "hamlet", "ZOE"] ] result = self.attempt_unsubscribe_of_principal( - query_count=31, + query_count=26, cache_count=9, target_users=target_users, is_realm_admin=True, @@ -2475,7 +2475,7 @@ class StreamAdminTest(ZulipTestCase): are on. """ result = self.attempt_unsubscribe_of_principal( - query_count=17, + query_count=16, target_users=[self.example_user("cordelia")], is_realm_admin=True, is_subbed=True, @@ -2492,7 +2492,7 @@ class StreamAdminTest(ZulipTestCase): streams you aren't on. """ result = self.attempt_unsubscribe_of_principal( - query_count=17, + query_count=16, target_users=[self.example_user("cordelia")], is_realm_admin=True, is_subbed=False, @@ -2509,7 +2509,7 @@ class StreamAdminTest(ZulipTestCase): You can remove others from public streams you're a stream administrator of. """ result = self.attempt_unsubscribe_of_principal( - query_count=16, + query_count=15, target_users=[self.example_user("cordelia")], is_realm_admin=False, is_stream_admin=True, @@ -2529,7 +2529,7 @@ class StreamAdminTest(ZulipTestCase): self.example_user(name) for name in ["cordelia", "prospero", "othello", "hamlet", "ZOE"] ] result = self.attempt_unsubscribe_of_principal( - query_count=31, + query_count=26, cache_count=9, target_users=target_users, is_realm_admin=False, @@ -2547,7 +2547,7 @@ class StreamAdminTest(ZulipTestCase): You can remove others from private streams you're a stream administrator of. """ result = self.attempt_unsubscribe_of_principal( - query_count=17, + query_count=16, target_users=[self.example_user("cordelia")], is_realm_admin=False, is_stream_admin=True, @@ -2574,7 +2574,7 @@ class StreamAdminTest(ZulipTestCase): def test_admin_remove_others_from_stream_legacy_emails(self) -> None: result = self.attempt_unsubscribe_of_principal( - query_count=16, + query_count=15, target_users=[self.example_user("cordelia")], is_realm_admin=True, is_subbed=True, @@ -2588,7 +2588,7 @@ class StreamAdminTest(ZulipTestCase): def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None: result = self.attempt_unsubscribe_of_principal( - query_count=20, + query_count=18, target_users=[self.example_user("cordelia"), self.example_user("prospero")], is_realm_admin=True, is_subbed=True, @@ -4667,11 +4667,9 @@ class SubscriptionAPITest(ZulipTestCase): self.subscribe(user2, "private_stream") self.subscribe(user3, "private_stream") - # Apart from 3 peer-remove events and 2 unsubscribe event, because `bulk_remove_subscriptions` - # also marks are read messages in those streams as read, so emits 6 `message_flags` events too - # (for each of the notification bot messages). + # Sends 3 peer-remove events and 2 unsubscribe events. events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events, expected_num_events=11): + with self.tornado_redirected_to_list(events, expected_num_events=5): with queries_captured() as query_count: with cache_tries_captured() as cache_count: bulk_remove_subscriptions( @@ -4681,7 +4679,7 @@ class SubscriptionAPITest(ZulipTestCase): acting_user=None, ) - self.assert_length(query_count, 23) + self.assert_length(query_count, 17) self.assert_length(cache_count, 3) peer_events = [e for e in events if e["event"].get("op") == "peer_remove"]