refactor: Only return sub from access_stream_common.

Let the callers access stream.recipient as needed.
It costs the same, and some of the callers can
actually stop caring about the actual Recipient
object.
This commit is contained in:
Steve Howell 2020-10-16 15:11:46 +00:00 committed by Tim Abbott
parent bfd6e2b1fd
commit 31622feb87
1 changed files with 28 additions and 19 deletions

View File

@ -269,10 +269,13 @@ def access_stream_for_delete_or_update(user_profile: UserProfile,
# Only set allow_realm_admin flag to True when you want to allow realm admin to # Only set allow_realm_admin flag to True when you want to allow realm admin to
# access unsubscribed private stream content. # access unsubscribed private stream content.
def access_stream_common(user_profile: UserProfile, stream: Stream, def access_stream_common(
user_profile: UserProfile,
stream: Stream,
error: str, error: str,
require_active: bool=True, require_active: bool=True,
allow_realm_admin: bool=False) -> Tuple[Recipient, Optional[Subscription]]: allow_realm_admin: bool=False
) -> Optional[Subscription]:
"""Common function for backend code where the target use attempts to """Common function for backend code where the target use attempts to
access the target stream, returning all the data fetched along the access the target stream, returning all the data fetched along the
way. If that user does not have permission to access that stream, way. If that user does not have permission to access that stream,
@ -283,33 +286,31 @@ def access_stream_common(user_profile: UserProfile, stream: Stream,
if stream.realm_id != user_profile.realm_id: if stream.realm_id != user_profile.realm_id:
raise JsonableError(error) raise JsonableError(error)
recipient = stream.recipient
try: try:
sub = Subscription.objects.get(user_profile=user_profile, sub = Subscription.objects.get(user_profile=user_profile,
recipient=recipient, recipient_id=stream.recipient_id,
active=require_active) active=require_active)
except Subscription.DoesNotExist: except Subscription.DoesNotExist:
sub = None sub = None
# Any realm user, even guests, can access web_public streams. # Any realm user, even guests, can access web_public streams.
if stream.is_web_public: if stream.is_web_public:
return (recipient, sub) return sub
# If the stream is in your realm and public, you can access it. # If the stream is in your realm and public, you can access it.
if stream.is_public() and not user_profile.is_guest: if stream.is_public() and not user_profile.is_guest:
return (recipient, sub) return sub
# Or if you are subscribed to the stream, you can access it. # Or if you are subscribed to the stream, you can access it.
if sub is not None: if sub is not None:
return (recipient, sub) return sub
# For some specific callers (e.g. getting list of subscribers, # For some specific callers (e.g. getting list of subscribers,
# removing other users from a stream, and updating stream name and # removing other users from a stream, and updating stream name and
# description), we allow realm admins to access stream even if # description), we allow realm admins to access stream even if
# they are not subscribed to a private stream. # they are not subscribed to a private stream.
if user_profile.is_realm_admin and allow_realm_admin: if user_profile.is_realm_admin and allow_realm_admin:
return (recipient, sub) return sub
# Otherwise it is a private stream and you're not on it, so throw # Otherwise it is a private stream and you're not on it, so throw
# an error. # an error.
@ -322,10 +323,14 @@ def access_stream_by_id(user_profile: UserProfile,
stream = get_stream_by_id(stream_id) stream = get_stream_by_id(stream_id)
error = _("Invalid stream id") error = _("Invalid stream id")
(recipient, sub) = access_stream_common(user_profile, stream, error, sub = access_stream_common(
user_profile,
stream,
error,
require_active=require_active, require_active=require_active,
allow_realm_admin=allow_realm_admin) allow_realm_admin=allow_realm_admin,
return (stream, recipient, sub) )
return (stream, stream.recipient, sub)
def get_public_streams_queryset(realm: Realm) -> 'QuerySet[Stream]': def get_public_streams_queryset(realm: Realm) -> 'QuerySet[Stream]':
return Stream.objects.filter(realm=realm, invite_only=False, return Stream.objects.filter(realm=realm, invite_only=False,
@ -363,9 +368,13 @@ def access_stream_by_name(user_profile: UserProfile,
except Stream.DoesNotExist: except Stream.DoesNotExist:
raise JsonableError(error) raise JsonableError(error)
(recipient, sub) = access_stream_common(user_profile, stream, error, sub = access_stream_common(
allow_realm_admin=allow_realm_admin) user_profile,
return (stream, recipient, sub) stream,
error,
allow_realm_admin=allow_realm_admin,
)
return (stream, stream.recipient, sub)
def access_web_public_stream(stream_id: int, realm: Realm) -> Stream: def access_web_public_stream(stream_id: int, realm: Realm) -> Stream:
error = _("Invalid stream id") error = _("Invalid stream id")
@ -435,7 +444,7 @@ def can_access_stream_history(user_profile: UserProfile, stream: Stream) -> bool
# In this case, we check if the user is subscribed. # In this case, we check if the user is subscribed.
error = _("Invalid stream name '{}'").format(stream.name) error = _("Invalid stream name '{}'").format(stream.name)
try: try:
(recipient, sub) = access_stream_common(user_profile, stream, error) access_stream_common(user_profile, stream, error)
except JsonableError: except JsonableError:
return False return False
return True return True