message_flags: Short-circuit if no messages changed.

Omit sending an event, and updating the database, if there are no
matching messages.
This commit is contained in:
Alex Vandiver 2022-05-12 18:50:40 -07:00 committed by Tim Abbott
parent 20b7a2d450
commit 803982e872
3 changed files with 21 additions and 18 deletions

View File

@ -98,6 +98,9 @@ def do_mark_stream_messages_as_read(
message_ids = list(msgs.values_list("message_id", flat=True)) message_ids = list(msgs.values_list("message_id", flat=True))
if len(message_ids) == 0:
return 0
count = msgs.update( count = msgs.update(
flags=F("flags").bitor(UserMessage.flags.read), 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)) message_ids = list(messages.values_list("message_id", flat=True))
if len(message_ids) == 0:
return 0
count = messages.update( count = messages.update(
flags=F("flags").bitor(UserMessage.flags.read), flags=F("flags").bitor(UserMessage.flags.read),
) )

View File

@ -1340,10 +1340,9 @@ class NormalActionsTest(BaseAction):
def test_muted_users_events(self) -> None: def test_muted_users_events(self) -> None:
muted_user = self.example_user("othello") muted_user = self.example_user("othello")
events = self.verify_action( 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[0]", events[0])
check_muted_users("events[1]", events[1])
mute_object = get_mute_object(self.user_profile, muted_user) mute_object = get_mute_object(self.user_profile, muted_user)
assert mute_object is not None assert mute_object is not None
@ -2678,7 +2677,7 @@ class SubscribeActionTest(BaseAction):
# Now remove the user himself, to test the 'remove' event flow # Now remove the user himself, to test the 'remove' event flow
action = lambda: bulk_remove_subscriptions(realm, [hamlet], [stream], acting_user=None) action = lambda: bulk_remove_subscriptions(realm, [hamlet], [stream], acting_user=None)
events = self.verify_action( 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]) check_subscription_remove("events[0]", events[0])
self.assert_length(events[0]["subscriptions"], 1) self.assert_length(events[0]["subscriptions"], 1)

View File

@ -2431,7 +2431,7 @@ class StreamAdminTest(ZulipTestCase):
those you aren't on. those you aren't on.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=16, query_count=15,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=True, is_realm_admin=True,
is_subbed=True, is_subbed=True,
@ -2457,7 +2457,7 @@ class StreamAdminTest(ZulipTestCase):
self.example_user(name) for name in ["cordelia", "prospero", "iago", "hamlet", "ZOE"] self.example_user(name) for name in ["cordelia", "prospero", "iago", "hamlet", "ZOE"]
] ]
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=31, query_count=26,
cache_count=9, cache_count=9,
target_users=target_users, target_users=target_users,
is_realm_admin=True, is_realm_admin=True,
@ -2475,7 +2475,7 @@ class StreamAdminTest(ZulipTestCase):
are on. are on.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=17, query_count=16,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=True, is_realm_admin=True,
is_subbed=True, is_subbed=True,
@ -2492,7 +2492,7 @@ class StreamAdminTest(ZulipTestCase):
streams you aren't on. streams you aren't on.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=17, query_count=16,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=True, is_realm_admin=True,
is_subbed=False, is_subbed=False,
@ -2509,7 +2509,7 @@ class StreamAdminTest(ZulipTestCase):
You can remove others from public streams you're a stream administrator of. You can remove others from public streams you're a stream administrator of.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=16, query_count=15,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=False, is_realm_admin=False,
is_stream_admin=True, is_stream_admin=True,
@ -2529,7 +2529,7 @@ class StreamAdminTest(ZulipTestCase):
self.example_user(name) for name in ["cordelia", "prospero", "othello", "hamlet", "ZOE"] self.example_user(name) for name in ["cordelia", "prospero", "othello", "hamlet", "ZOE"]
] ]
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=31, query_count=26,
cache_count=9, cache_count=9,
target_users=target_users, target_users=target_users,
is_realm_admin=False, is_realm_admin=False,
@ -2547,7 +2547,7 @@ class StreamAdminTest(ZulipTestCase):
You can remove others from private streams you're a stream administrator of. You can remove others from private streams you're a stream administrator of.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=17, query_count=16,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=False, is_realm_admin=False,
is_stream_admin=True, is_stream_admin=True,
@ -2574,7 +2574,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_others_from_stream_legacy_emails(self) -> None: def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=16, query_count=15,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=True, is_realm_admin=True,
is_subbed=True, is_subbed=True,
@ -2588,7 +2588,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None: def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=20, query_count=18,
target_users=[self.example_user("cordelia"), self.example_user("prospero")], target_users=[self.example_user("cordelia"), self.example_user("prospero")],
is_realm_admin=True, is_realm_admin=True,
is_subbed=True, is_subbed=True,
@ -4667,11 +4667,9 @@ class SubscriptionAPITest(ZulipTestCase):
self.subscribe(user2, "private_stream") self.subscribe(user2, "private_stream")
self.subscribe(user3, "private_stream") self.subscribe(user3, "private_stream")
# Apart from 3 peer-remove events and 2 unsubscribe event, because `bulk_remove_subscriptions` # Sends 3 peer-remove events and 2 unsubscribe events.
# also marks are read messages in those streams as read, so emits 6 `message_flags` events too
# (for each of the notification bot messages).
events: List[Mapping[str, Any]] = [] 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 queries_captured() as query_count:
with cache_tries_captured() as cache_count: with cache_tries_captured() as cache_count:
bulk_remove_subscriptions( bulk_remove_subscriptions(
@ -4681,7 +4679,7 @@ class SubscriptionAPITest(ZulipTestCase):
acting_user=None, acting_user=None,
) )
self.assert_length(query_count, 23) self.assert_length(query_count, 17)
self.assert_length(cache_count, 3) self.assert_length(cache_count, 3)
peer_events = [e for e in events if e["event"].get("op") == "peer_remove"] peer_events = [e for e in events if e["event"].get("op") == "peer_remove"]