From d5a391a56b2305e934fb7fbc696f9b52f518b2cc Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 18 Nov 2024 12:15:04 +0530 Subject: [PATCH] streams: Optmize code to send events on creating stream. This commit updates code to effectively compute the setting values when creating stream object to be sent in stream creation events. --- zerver/lib/streams.py | 24 ++++++++++++++++++++---- zerver/tests/test_subs.py | 8 ++++---- zerver/views/streams.py | 11 ++++++++++- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 624833d165..9912e617b0 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -151,6 +151,7 @@ def create_stream_if_needed( message_retention_days: int | None = None, can_remove_subscribers_group: UserGroup | None = None, acting_user: UserProfile | None = None, + setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] | None = None, ) -> tuple[Stream, bool]: history_public_to_subscribers = get_default_value_for_history_public_to_subscribers( realm, invite_only, history_public_to_subscribers @@ -197,21 +198,31 @@ def create_stream_if_needed( event_time=event_time, ) + if setting_groups_dict is None: + setting_groups_dict = get_group_setting_value_dict_for_streams([stream]) + if stream.is_public(): if stream.is_web_public: notify_user_ids = active_user_ids(stream.realm_id) else: notify_user_ids = active_non_guest_user_ids(stream.realm_id) - send_stream_creation_event(realm, stream, notify_user_ids) + send_stream_creation_event( + realm, stream, notify_user_ids, setting_groups_dict=setting_groups_dict + ) else: realm_admin_ids = [user.id for user in stream.realm.get_admin_users_and_bots()] - send_stream_creation_event(realm, stream, realm_admin_ids) + send_stream_creation_event( + realm, stream, realm_admin_ids, setting_groups_dict=setting_groups_dict + ) return stream, created def create_streams_if_needed( - realm: Realm, stream_dicts: list[StreamDict], acting_user: UserProfile | None = None + realm: Realm, + stream_dicts: list[StreamDict], + acting_user: UserProfile | None = None, + setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] | None = None, ) -> tuple[list[Stream], list[Stream]]: """Note that stream_dict["name"] is assumed to already be stripped of whitespace""" @@ -232,6 +243,7 @@ def create_streams_if_needed( message_retention_days=stream_dict.get("message_retention_days", None), can_remove_subscribers_group=stream_dict.get("can_remove_subscribers_group", None), acting_user=acting_user, + setting_groups_dict=setting_groups_dict, ) if created: @@ -717,6 +729,7 @@ def list_to_streams( autocreate: bool = False, unsubscribing_others: bool = False, is_default_stream: bool = False, + setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] | None = None, ) -> tuple[list[Stream], list[Stream]]: """Converts list of dicts to a list of Streams, validating input in the process @@ -817,7 +830,10 @@ def list_to_streams( # paranoid approach, since often on Zulip two people will discuss # creating a new stream, and both people eagerly do it.) created_streams, dup_streams = create_streams_if_needed( - realm=user_profile.realm, stream_dicts=missing_stream_dicts, acting_user=user_profile + realm=user_profile.realm, + stream_dicts=missing_stream_dicts, + acting_user=user_profile, + setting_groups_dict=setting_groups_dict, ) existing_streams += dup_streams diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index a61de9f017..dea65654bd 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -4801,7 +4801,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub = ["multi_user_stream"] with ( self.capture_send_event_calls(expected_num_events=5) as events, - self.assert_database_query_count(39), + self.assert_database_query_count(38), ): self.common_subscribe_to_streams( self.test_user, @@ -5641,7 +5641,7 @@ class SubscriptionAPITest(ZulipTestCase): ] # Test creating a public stream when realm does not have a notification stream. - with self.assert_database_query_count(39): + with self.assert_database_query_count(38): self.common_subscribe_to_streams( self.test_user, [new_streams[0]], @@ -5649,7 +5649,7 @@ class SubscriptionAPITest(ZulipTestCase): ) # Test creating private stream. - with self.assert_database_query_count(42): + with self.assert_database_query_count(41): self.common_subscribe_to_streams( self.test_user, [new_streams[1]], @@ -5661,7 +5661,7 @@ class SubscriptionAPITest(ZulipTestCase): new_stream_announcements_stream = get_stream(self.streams[0], self.test_realm) self.test_realm.new_stream_announcements_stream_id = new_stream_announcements_stream.id self.test_realm.save() - with self.assert_database_query_count(50): + with self.assert_database_query_count(49): self.common_subscribe_to_streams( self.test_user, [new_streams[2]], diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 7cf45cb89c..b73131ab05 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -590,6 +590,7 @@ def add_subscriptions_backend( if principals is None: principals = [] + setting_groups_dict = {} if can_remove_subscribers_group is not None: setting_value = parse_group_setting_value( can_remove_subscribers_group, "can_remove_subscribers_group" @@ -603,6 +604,7 @@ def add_subscriptions_backend( setting_name="can_remove_subscribers_group", permission_configuration=permission_configuration, ) + setting_groups_dict[can_remove_subscribers_group_value.id] = setting_value else: can_remove_subscribers_group_default_name = Stream.stream_permission_group_settings[ "can_remove_subscribers_group" @@ -612,6 +614,9 @@ def add_subscriptions_backend( realm=user_profile.realm, is_system_group=True, ) + setting_groups_dict[can_remove_subscribers_group_value.id] = ( + can_remove_subscribers_group_value.id + ) for stream_obj in streams_raw: # 'color' field is optional @@ -655,7 +660,11 @@ def add_subscriptions_backend( # can_create_streams policy and check_stream_name policy is inside # list_to_streams. existing_streams, created_streams = list_to_streams( - stream_dicts, user_profile, autocreate=True, is_default_stream=is_default_stream + stream_dicts, + user_profile, + autocreate=True, + is_default_stream=is_default_stream, + setting_groups_dict=setting_groups_dict, ) authorized_streams, unauthorized_streams = filter_stream_authorization( user_profile, existing_streams