streams: Return early if there is no change in subscriptions.

This commit updates code in bulk_remove_subscriptions and
bulk_add_subscriptions to return early if there are no
subscribers to remove or add to the streams.

This change helps us in avoiding unnecessary queries like the
one used to get subscribers list of streams, which is then used
to send events but we would not send any events if no subscribers
are added or removed and some more similar queries.
This commit is contained in:
Sahil Batra 2023-10-27 12:21:43 +05:30 committed by Tim Abbott
parent cdd15b4a69
commit 71b8f49614
2 changed files with 23 additions and 1 deletions

View File

@ -639,6 +639,10 @@ def bulk_add_subscriptions(
sub_info = SubInfo(user_profile, sub, stream)
subs_to_add.append(sub_info)
if len(subs_to_add) == 0 and len(subs_to_activate) == 0:
# We can return early if users are already subscribed to all the streams.
return ([], already_subscribed)
bulk_add_subs_to_db_with_logging(
realm=realm,
acting_user=acting_user,
@ -807,6 +811,11 @@ def bulk_remove_subscriptions(
subs_to_deactivate = [
sub_info for sub_infos in existing_subs_by_user.values() for sub_info in sub_infos
]
if len(subs_to_deactivate) == 0:
# We can return early if users are not subscribed to any of the streams.
return ([], not_subscribed)
sub_ids_to_deactivate = [sub_info.sub.id for sub_info in subs_to_deactivate]
streams_to_unsubscribe = [sub_info.stream for sub_info in subs_to_deactivate]
# We do all the database changes in a transaction to ensure

View File

@ -2712,13 +2712,26 @@ class StreamAdminTest(ZulipTestCase):
self.assert_length(json["removed"], 2)
self.assert_length(json["not_removed"], 0)
def test_remove_unsubbed_user_along_with_subbed(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=17,
target_users=[self.example_user("cordelia"), self.example_user("iago")],
is_realm_admin=True,
is_subbed=True,
invite_only=False,
target_users_subbed=False,
)
json = self.assert_json_success(result)
self.assert_length(json["removed"], 1)
self.assert_length(json["not_removed"], 1)
def test_remove_already_not_subbed(self) -> None:
"""
Trying to unsubscribe someone who already isn't subscribed to a stream
fails gracefully.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=11,
query_count=9,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,