mirror of
https://github.com/zulip/zulip.git
synced 2025-10-30 03:23:50 +00:00
signup: Avoid bloated Stream objects for default streams.
Basically, I eliminate the use of select_all() in a query that still makes a single round trip. We have good test enforcement that Django never needs to lazily fetch objects off the Stream object. (It used to be common to fetch stream.realm a while back, but we upgraded bulk_add_subscription, in particular, a while back.)
This commit is contained in:
@@ -18,7 +18,7 @@ from zerver.actions.user_groups import do_send_user_group_members_update_event
|
||||
from zerver.actions.users import change_user_is_active, get_service_dicts_for_bot
|
||||
from zerver.lib.avatar import avatar_url
|
||||
from zerver.lib.create_user import create_user
|
||||
from zerver.lib.default_streams import get_default_streams_for_realm
|
||||
from zerver.lib.default_streams import get_slim_realm_default_streams
|
||||
from zerver.lib.email_notifications import enqueue_welcome_emails
|
||||
from zerver.lib.mention import silent_mention_syntax_for_user
|
||||
from zerver.lib.send_email import clear_scheduled_invitation_emails
|
||||
@@ -137,10 +137,13 @@ def set_up_streams_for_new_human_user(
|
||||
prereg_user.referred_by is not None or prereg_user.multiuse_invite is not None
|
||||
)
|
||||
|
||||
# If the Preregistration object didn't explicitly list some streams (it happens when user
|
||||
# directly signs up without any invitation), we add the default streams
|
||||
# If the Preregistration object didn't explicitly list some streams (it
|
||||
# happens when user directly signs up without any invitation), we add the
|
||||
# default streams for the realm. Note that we are fine with "slim" Stream
|
||||
# objects for calling bulk_add_subscriptions and add_new_user_history,
|
||||
# which we verify in StreamSetupTest tests that check query counts.
|
||||
if len(streams) == 0 and not user_was_invited:
|
||||
streams = get_default_subs(user_profile)
|
||||
streams = get_slim_realm_default_streams(realm.id)
|
||||
|
||||
for default_stream_group in default_stream_groups:
|
||||
default_stream_group_streams = default_stream_group.streams.all()
|
||||
@@ -610,9 +613,3 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserP
|
||||
stream_dict=stream_dict,
|
||||
private_peer_dict=subscriber_peer_info.private_peer_dict,
|
||||
)
|
||||
|
||||
|
||||
def get_default_subs(user_profile: UserProfile) -> List[Stream]:
|
||||
# Right now default streams are realm-wide. This wrapper gives us flexibility
|
||||
# to some day further customize how we set up default streams for new users.
|
||||
return get_default_streams_for_realm(user_profile.realm_id)
|
||||
|
||||
@@ -4,14 +4,18 @@ from zerver.lib.types import APIStreamDict
|
||||
from zerver.models import DefaultStream, Stream
|
||||
|
||||
|
||||
def get_default_streams_for_realm(realm_id: int) -> List[Stream]:
|
||||
# TODO: Deprecate this extremely expensive query. We can't immediately
|
||||
# just improve it by removing select_related(), because then you
|
||||
# may get the opposite problem of multiple round trips.
|
||||
return [
|
||||
default.stream
|
||||
for default in DefaultStream.objects.select_related().filter(realm_id=realm_id)
|
||||
]
|
||||
def get_slim_realm_default_streams(realm_id: int) -> List[Stream]:
|
||||
# We really want this query to be simple and just get "thin" Stream objects
|
||||
# in one round trip.
|
||||
#
|
||||
# The above is enforced by at least three tests that verify query counts,
|
||||
# and test_query_count in test_subs.py makes sure that the query itself is
|
||||
# not like 11000 bytes, which is what we had in a prior version that used
|
||||
# select_related() with not arguments (and thus joined to too many tables).
|
||||
#
|
||||
# Please be careful about modifying this code, as it has had a history
|
||||
# of performance problems.
|
||||
return list(Stream.objects.filter(defaultstream__realm_id=realm_id))
|
||||
|
||||
|
||||
def get_default_stream_ids_for_realm(realm_id: int) -> Set[int]:
|
||||
@@ -23,11 +27,6 @@ def get_default_streams_for_realm_as_dicts(realm_id: int) -> List[APIStreamDict]
|
||||
Return all the default streams for a realm using a list of dictionaries sorted
|
||||
by stream name.
|
||||
"""
|
||||
|
||||
# This slightly convoluted construction makes it so that the Django ORM gets
|
||||
# all the data it needs to serialize default streams as a list of dictionaries
|
||||
# using a single query without using select_related().
|
||||
# This is enforced by test_query_count in test_subs.py.
|
||||
stream_ids = DefaultStream.objects.filter(realm_id=realm_id).values_list("stream_id")
|
||||
streams = Stream.objects.filter(id__in=stream_ids)
|
||||
return sorted((stream.to_dict() for stream in streams), key=lambda elt: elt["name"])
|
||||
streams = get_slim_realm_default_streams(realm_id)
|
||||
stream_dicts = [stream.to_dict() for stream in streams]
|
||||
return sorted(stream_dicts, key=lambda stream: stream["name"])
|
||||
|
||||
Reference in New Issue
Block a user