internal_prep_stream_message: Support accepting a Stream object.

If the caller has access to a Stream object, it is wasteful to
query a database for a stream by ID or name. In addition, not
having to go through stream names eliminates various classes of
possible bugs involved with getting a Stream object back.
This commit is contained in:
Eeshan Garg 2019-02-06 21:35:34 -03:30 committed by Tim Abbott
parent 849c296f90
commit c240008edb
6 changed files with 49 additions and 17 deletions

View File

@ -1688,9 +1688,10 @@ def prep_stream_welcome_message(stream: Stream) -> Optional[Dict[str, Any]]:
message = internal_prep_stream_message( message = internal_prep_stream_message(
realm=realm, realm=realm,
sender=sender, sender=sender,
stream_name=stream.name,
topic=topic, topic=topic,
content=content) content=content,
stream=stream
)
return message return message
@ -2224,8 +2225,10 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee,
if stream_name is not None: if stream_name is not None:
stream = validate_stream_name_with_pm_notification(stream_name, realm, sender) stream = validate_stream_name_with_pm_notification(stream_name, realm, sender)
else: elif stream_id is not None:
stream = validate_stream_id_with_pm_notification(stream_id, realm, sender) stream = validate_stream_id_with_pm_notification(stream_id, realm, sender)
else:
stream = addressee.stream()
recipient = get_stream_recipient(stream.id) recipient = get_stream_recipient(stream.id)
@ -2320,13 +2323,20 @@ def _internal_prep_message(realm: Realm,
return None return None
def internal_prep_stream_message(realm: Realm, sender: UserProfile, def internal_prep_stream_message(
stream_name: str, topic: str, realm: Realm, sender: UserProfile,
content: str) -> Optional[Dict[str, Any]]: topic: str, content: str,
stream_name: Optional[str]=None,
stream: Optional[Stream]=None
) -> Optional[Dict[str, Any]]:
""" """
See _internal_prep_message for details of how this works. See _internal_prep_message for details of how this works.
""" """
addressee = Addressee.for_stream_name(stream_name, topic) assert stream_name is not None or stream is not None
if stream is not None:
addressee = Addressee.for_stream(stream, topic)
else:
addressee = Addressee.for_stream_name(stream_name, topic)
return _internal_prep_message( return _internal_prep_message(
realm=realm, realm=realm,
@ -2392,7 +2402,8 @@ def internal_send_private_message(realm: Realm,
def internal_send_stream_message(realm: Realm, sender: UserProfile, stream_name: str, def internal_send_stream_message(realm: Realm, sender: UserProfile, stream_name: str,
topic: str, content: str) -> None: topic: str, content: str) -> None:
message = internal_prep_stream_message(realm, sender, stream_name, topic, content) message = internal_prep_stream_message(realm, sender, topic,
content, stream_name=stream_name)
if message is None: if message is None:
return return
do_send_messages([message]) do_send_messages([message])

View File

@ -8,6 +8,7 @@ from zerver.models import (
UserProfile, UserProfile,
get_user_including_cross_realm, get_user_including_cross_realm,
get_user_by_id_in_realm_including_cross_realm, get_user_by_id_in_realm_including_cross_realm,
Stream,
) )
def raw_pm_with_emails(email_str: str, my_email: str) -> List[str]: def raw_pm_with_emails(email_str: str, my_email: str) -> List[str]:
@ -63,12 +64,14 @@ class Addressee:
# This should be treated as an immutable class. # This should be treated as an immutable class.
def __init__(self, msg_type: str, def __init__(self, msg_type: str,
user_profiles: Optional[Sequence[UserProfile]]=None, user_profiles: Optional[Sequence[UserProfile]]=None,
stream: Optional[Stream]=None,
stream_name: Optional[str]=None, stream_name: Optional[str]=None,
stream_id: Optional[int]=None, stream_id: Optional[int]=None,
topic: Optional[str]=None) -> None: topic: Optional[str]=None) -> None:
assert(msg_type in ['stream', 'private']) assert(msg_type in ['stream', 'private'])
self._msg_type = msg_type self._msg_type = msg_type
self._user_profiles = user_profiles self._user_profiles = user_profiles
self._stream = stream
self._stream_name = stream_name self._stream_name = stream_name
self._stream_id = stream_id self._stream_id = stream_id
self._topic = topic self._topic = topic
@ -83,6 +86,10 @@ class Addressee:
assert(self.is_private()) assert(self.is_private())
return self._user_profiles # type: ignore # assertion protects us return self._user_profiles # type: ignore # assertion protects us
def stream(self) -> Optional[Stream]:
assert(self.is_stream())
return self._stream
def stream_name(self) -> Optional[str]: def stream_name(self) -> Optional[str]:
assert(self.is_stream()) assert(self.is_stream())
return self._stream_name return self._stream_name
@ -144,6 +151,15 @@ class Addressee:
else: else:
raise JsonableError(_("Invalid message type")) raise JsonableError(_("Invalid message type"))
@staticmethod
def for_stream(stream: Stream, topic: str) -> 'Addressee':
topic = validate_topic(topic)
return Addressee(
msg_type='stream',
stream=stream,
topic=topic,
)
@staticmethod @staticmethod
def for_stream_name(stream_name: str, topic: str) -> 'Addressee': def for_stream_name(stream_name: str, topic: str) -> 'Addressee':
topic = validate_topic(topic) topic = validate_topic(topic)

View File

@ -109,7 +109,9 @@ def send_initial_realm_messages(realm: Realm) -> None:
] # type: List[Dict[str, str]] ] # type: List[Dict[str, str]]
messages = [internal_prep_stream_message( messages = [internal_prep_stream_message(
realm, welcome_bot, realm, welcome_bot,
message['stream'], message['topic'], message['content']) for message in welcome_messages] message['topic'], message['content'],
stream_name=message['stream']
) for message in welcome_messages]
message_ids = do_send_messages(messages) message_ids = do_send_messages(messages)
# We find the one of our just-sent messages with turtle.png in it, # We find the one of our just-sent messages with turtle.png in it,

View File

@ -2183,7 +2183,7 @@ class SubscriptionAPITest(ZulipTestCase):
streams_to_sub, streams_to_sub,
dict(principals=ujson.dumps([user1.email, user2.email])), dict(principals=ujson.dumps([user1.email, user2.email])),
) )
self.assert_length(queries, 45) self.assert_length(queries, 43)
self.assert_length(events, 7) self.assert_length(events, 7)
for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]:
@ -2944,7 +2944,7 @@ class SubscriptionAPITest(ZulipTestCase):
[new_streams[0]], [new_streams[0]],
dict(principals=ujson.dumps([user1.email, user2.email])), dict(principals=ujson.dumps([user1.email, user2.email])),
) )
self.assert_length(queries, 45) self.assert_length(queries, 43)
# Test creating private stream. # Test creating private stream.
with queries_captured() as queries: with queries_captured() as queries:
@ -2954,7 +2954,7 @@ class SubscriptionAPITest(ZulipTestCase):
dict(principals=ujson.dumps([user1.email, user2.email])), dict(principals=ujson.dumps([user1.email, user2.email])),
invite_only=True, invite_only=True,
) )
self.assert_length(queries, 40) self.assert_length(queries, 38)
# Test creating a public stream with announce when realm has a notification stream. # Test creating a public stream with announce when realm has a notification stream.
notifications_stream = get_stream(self.streams[0], self.test_realm) notifications_stream = get_stream(self.streams[0], self.test_realm)
@ -2969,7 +2969,7 @@ class SubscriptionAPITest(ZulipTestCase):
principals=ujson.dumps([user1.email, user2.email]) principals=ujson.dumps([user1.email, user2.email])
) )
) )
self.assert_length(queries, 55) self.assert_length(queries, 51)
class GetPublicStreamsTest(ZulipTestCase): class GetPublicStreamsTest(ZulipTestCase):

View File

@ -392,16 +392,17 @@ def add_subscriptions_backend(
msg = ("_@**%s|%d** just created %s" % (user_profile.full_name, user_profile.id, stream_msg)) msg = ("_@**%s|%d** just created %s" % (user_profile.full_name, user_profile.id, stream_msg))
sender = get_system_bot(settings.NOTIFICATION_BOT) sender = get_system_bot(settings.NOTIFICATION_BOT)
stream_name = notifications_stream.name
topic = 'Streams' topic = 'Streams'
notifications.append( notifications.append(
internal_prep_stream_message( internal_prep_stream_message(
realm=user_profile.realm, realm=user_profile.realm,
sender=sender, sender=sender,
stream_name=stream_name,
topic=topic, topic=topic,
content=msg)) content=msg,
stream=notifications_stream
)
)
if not user_profile.realm.is_zephyr_mirror_realm: if not user_profile.realm.is_zephyr_mirror_realm:
for stream in created_streams: for stream in created_streams:

View File

@ -78,7 +78,9 @@ From image editing program:
] # type: List[Dict[str, Any]] ] # type: List[Dict[str, Any]]
messages = [internal_prep_stream_message( messages = [internal_prep_stream_message(
realm, message['sender'], stream.name, 'message formatting', message['content'] realm, message['sender'],
'message formatting', message['content'],
stream=stream
) for message in staged_messages] ) for message in staged_messages]
message_ids = do_send_messages(messages) message_ids = do_send_messages(messages)