message: Use .is_channel_message column instead of is_stream_message().

This avoids a potential unnecessary message.recipient fetch required by
is_stream_message(). is_stream_message() methods precedes the addition
of the denormalized is_channel_message column and is now unnecessary.

In practice, we usually fetch Message objects with `.recipient` already,
so I don't expect any notable performance impact here - but it's still a
useful change to make.

(cherry picked from commit 51cef01c29)

zerver/actions/message_delete.py

@@ -111,7 +111,7 @@ def do_delete_messages(
     )
     stream_by_recipient_id = {}
     for message in messages:
-        if message.is_stream_message():
+        if message.is_channel_message:
             recipient_id = message.recipient_id
             # topics are case-insensitive.
             topic_name = message.topic_name().lower()

zerver/actions/message_edit.py

@@ -126,7 +126,7 @@ def validate_message_edit_payload(
     if topic_name is None and content is None and stream_id is None:
         raise JsonableError(_("Nothing to change"))
 
-    if not message.is_stream_message():
+    if not message.is_channel_message:
         if stream_id is not None:
             raise JsonableError(_("Direct messages cannot be moved to channels."))
         if topic_name is not None:
@@ -1454,7 +1454,7 @@ def build_message_edit_request(
             content = "(deleted)"
         new_content = normalize_body(content)
 
-    if not message.is_stream_message():
+    if not message.is_channel_message:
         # We have already validated that at least one of content, topic, or stream
         # must be modified, and for DMs, only the content can be edited.
         return DirectMessageEditRequest(
@@ -1633,12 +1633,12 @@ def check_update_message(
         )
         links_for_embed |= rendering_result.links_for_preview
 
-        if message.is_stream_message() and rendering_result.mentions_stream_wildcard:
+        if message.is_channel_message and rendering_result.mentions_stream_wildcard:
             stream = access_stream_by_id(user_profile, message.recipient.type_id)[0]
             if not stream_wildcard_mention_allowed(message.sender, stream, message.realm):
                 raise StreamWildcardMentionNotAllowedError
 
-        if message.is_stream_message() and rendering_result.mentions_topic_wildcard:
+        if message.is_channel_message and rendering_result.mentions_topic_wildcard:
             topic_participant_count = len(
                 participants_for_topic(message.realm.id, message.recipient.id, message.topic_name())
             )
@@ -1653,7 +1653,7 @@ def check_update_message(
 
     if isinstance(message_edit_request, StreamMessageEditRequest):
         if message_edit_request.is_stream_edited:
-            assert message.is_stream_message()
+            assert message.is_channel_message
             if not can_move_messages_out_of_channel(user_profile, message_edit_request.orig_stream):
                 raise JsonableError(_("You don't have permission to move this message"))
 

zerver/actions/message_flags.py

@@ -212,7 +212,7 @@ def do_update_mobile_push_notification(
     # in a sent notification if a message was edited to mention a
     # group rather than a user (or vice versa), though it is likely
     # not worth the effort to do such a change.
-    if not message.is_stream_message():
+    if not message.is_channel_message:
         return
 
     remove_notify_users = prior_mention_user_ids - mentions_user_ids - stream_push_user_ids

zerver/actions/message_send.py

@@ -608,7 +608,7 @@ def build_message_send_dict(
         message_sender=message.sender,
     )
 
-    if message.is_stream_message():
+    if message.is_channel_message:
         stream_id = message.recipient.type_id
         stream_topic: StreamTopicTarget | None = StreamTopicTarget(
             stream_id=stream_id,
@@ -745,7 +745,7 @@ def create_user_messages(
     # These properties on the Message are set via
     # render_message_markdown by code in the Markdown inline patterns
     ids_with_alert_words = rendering_result.user_ids_with_alert_words
-    is_stream_message = message.is_stream_message()
+    is_stream_message = message.is_channel_message
 
     base_flags = 0
     if rendering_result.mentions_stream_wildcard:
@@ -968,7 +968,7 @@ def do_send_messages(
     # * Adding links to the embed_links queue for open graph processing.
     for send_request in send_message_requests:
         realm_id: int | None = None
-        if send_request.message.is_stream_message():
+        if send_request.message.is_channel_message:
             if send_request.stream is None:
                 stream_id = send_request.message.recipient.type_id
                 send_request.stream = Stream.objects.get(id=stream_id)
@@ -1172,7 +1172,7 @@ def do_send_messages(
             realm_host=send_request.realm.host,
         )
 
-        if send_request.message.is_stream_message():
+        if send_request.message.is_channel_message:
             # Note: This is where authorization for single-stream
             # get_updates happens! We only attach stream data to the
             # notify new_message request if it's a public stream,
@@ -1237,7 +1237,7 @@ def do_send_messages(
 
         # Check if this is a 1:1 DM between a user and the Welcome Bot,
         # in which case we may want to send an automated response.
-        if not send_request.message.is_stream_message() and len(send_request.active_user_ids) == 2:
+        if not send_request.message.is_channel_message and len(send_request.active_user_ids) == 2:
             welcome_bot_id = get_system_bot(settings.WELCOME_BOT, send_request.realm.id).id
             if (
                 welcome_bot_id in send_request.active_user_ids

zerver/actions/uploads.py

@@ -43,7 +43,13 @@ def do_claim_attachments(
         user_profile = message.sender
         is_message_realm_public = False
         is_message_web_public = False
-        if message.is_stream_message():
+        if isinstance(message, Message):
+            is_channel_message = message.is_channel_message
+        else:
+            assert isinstance(message, ScheduledMessage)
+            is_channel_message = message.is_channel_message()
+
+        if is_channel_message:
             stream = Stream.objects.get(id=message.recipient.type_id)
             is_message_realm_public = stream.is_public()
             is_message_web_public = stream.is_web_public

zerver/lib/attachments.py

@@ -140,7 +140,7 @@ def validate_attachment_request(
         user_profile=user_profile, message__in=messages
     ).select_related("message", "message__recipient")
     for um in usermessage_rows:
-        if not um.message.is_stream_message():
+        if not um.message.is_channel_message:
             # If the attachment was sent in a direct message or group direct
             # message then anyone who received that message can access it.
             return True, attachment
@@ -164,7 +164,7 @@ def validate_attachment_request(
 
     message_channel_ids = set()
     for message in messages:
-        if message.is_stream_message():
+        if message.is_channel_message:
             message_channel_ids.add(message.recipient.type_id)
 
     if len(message_channel_ids) == 0:

zerver/lib/email_notifications.py

@@ -685,7 +685,7 @@ def handle_missedmessage_emails(
 
     for msg_list in messages_by_bucket.values():
         msg = min(msg_list, key=lambda msg: msg.date_sent)
-        if msg.is_stream_message() and UserMessage.has_any_mentions(user_profile_id, msg.id):
+        if msg.is_channel_message and UserMessage.has_any_mentions(user_profile_id, msg.id):
             context_messages = get_context_for_message(msg)
             filtered_context_messages = bulk_access_messages(
                 user_profile, context_messages, is_modifying_message=False

zerver/lib/message.py

@@ -465,7 +465,7 @@ def access_web_public_message(
     except Message.DoesNotExist:
         raise MissingAuthenticationError
 
-    if not message.is_stream_message():
+    if not message.is_channel_message:
         raise MissingAuthenticationError
 
     queryset = get_web_public_streams_queryset(realm)
@@ -615,7 +615,7 @@ def event_recipient_ids_for_action_on_messages(
         return set(usermessages.values_list("user_profile_id", flat=True))
 
     sample_message = messages[0]
-    if not sample_message.is_stream_message():
+    if not sample_message.is_channel_message:
         # For DM, event is sent to users who actually received the message.
         return get_user_ids_having_usermessage_row_for_messages(message_ids)
 
@@ -1720,7 +1720,7 @@ def should_change_visibility_policy(
 
 def set_visibility_policy_possible(user_profile: UserProfile, message: Message) -> bool:
     """If the user can set a visibility policy."""
-    if not message.is_stream_message():
+    if not message.is_channel_message:
         return False
 
     if user_profile.is_bot:

zerver/lib/message_report.py

@@ -59,7 +59,7 @@ def send_message_report(
             last_user_mention=last_user_mention,
         )
     else:
-        assert reported_message.is_stream_message() is True
+        assert reported_message.is_channel_message is True
         topic_name = reported_message.topic_name()
         channel_id = reported_message.recipient.type_id
         channel_name = reported_message.recipient.label()

zerver/lib/push_notifications.py

@@ -1058,7 +1058,7 @@ def get_apns_alert_title(message: Message, language: str) -> str:
         assert isinstance(recipients, list)
         if len(recipients) > 2:
             return ", ".join(sorted(r["full_name"] for r in recipients))
-    elif message.is_stream_message():
+    elif message.is_channel_message:
         stream_name = get_message_stream_name_from_database(message)
         topic_display_name = get_topic_display_name(message.topic_name(), language)
         return f"#{stream_name} > {topic_display_name}"
@@ -1718,7 +1718,7 @@ def handle_push_notification(user_profile_id: int, missed_message: dict[str, Any
         user_profile, {trigger}, mentioned_user_group_members_count
     )
 
-    if message.is_stream_message():
+    if message.is_channel_message:
         # This will almost always be True. The corner case where you
         # can be receiving a message from a user you cannot access
         # involves your being a guest user whose access is restricted

zerver/lib/reminders.py

@@ -32,7 +32,7 @@ def get_reminder_formatted_content(
     if note:
         note = normalize_note_text(note)
 
-    if message.is_stream_message():
+    if message.is_channel_message:
         # We don't need to check access here since we already have the message
         # whose access has already been checked by the caller.
         stream = Stream.objects.get(

zerver/lib/test_classes.py

@@ -2775,6 +2775,7 @@ class PushNotificationTestCase(BouncerTestCase):
             rendered_content="This is test content",
             date_sent=timezone_now(),
             sending_client=self.sending_client,
+            is_channel_message=type == Recipient.STREAM,
         )
         message.set_topic_name("Test topic")
         message.save()

zerver/models/messages.py

@@ -283,16 +283,6 @@ class Message(AbstractMessage):
     def set_topic_name(self, topic_name: str) -> None:
         self.subject = topic_name
 
-    def is_stream_message(self) -> bool:
-        """
-        Find out whether a message is a stream message by
-        looking up its recipient.type.  TODO: Make this
-        an easier operation by denormalizing the message
-        type onto Message, either explicitly (message.type)
-        or implicitly (message.stream_id is not None).
-        """
-        return self.recipient.type == Recipient.STREAM
-
     def get_realm(self) -> Realm:
         return self.realm
 

zerver/models/scheduled_jobs.py

@@ -225,7 +225,7 @@ class ScheduledMessage(models.Model):
     def set_topic_name(self, topic_name: str) -> None:
         self.subject = topic_name
 
-    def is_stream_message(self) -> bool:
+    def is_channel_message(self) -> bool:
         return self.recipient.type == Recipient.STREAM
 
     def to_dict(self) -> APIScheduledStreamMessageDict | APIScheduledDirectMessageDict:

zerver/tests/test_message_delete.py

@@ -914,7 +914,7 @@ class DeleteMessageTest(ZulipTestCase):
         self.assertEqual(stream.first_message_id, message_ids[1])
 
         all_messages = Message.objects.filter(id__in=message_ids)
-        with self.assert_database_query_count(27):
+        with self.assert_database_query_count(25):
             do_delete_messages(realm, all_messages, acting_user=None)
         stream = get_stream(stream_name, realm)
         self.assertEqual(stream.first_message_id, None)

zerver/tests/test_message_fetch.py

@@ -2433,7 +2433,7 @@ class GetOldMessagesTest(ZulipTestCase):
         self.assertFalse(aaron.is_active)
 
         personals = [
-            m for m in get_user_messages(self.example_user("hamlet")) if not m.is_stream_message()
+            m for m in get_user_messages(self.example_user("hamlet")) if not m.is_channel_message
         ]
         for personal in personals:
             emails = dr_emails(get_display_recipient(personal.recipient))
@@ -2748,7 +2748,7 @@ class GetOldMessagesTest(ZulipTestCase):
         self.send_personal_message(hamlet, hamlet)
 
         messages = get_user_messages(hamlet)
-        channel_messages = [msg for msg in messages if msg.is_stream_message()]
+        channel_messages = [msg for msg in messages if msg.is_channel_message]
         self.assertGreater(len(messages), len(channel_messages))
         self.assert_length(channel_messages, num_messages_per_channel * len(channel_names))
 
@@ -2804,7 +2804,7 @@ class GetOldMessagesTest(ZulipTestCase):
         )
 
         messages = get_user_messages(self.mit_user("starnine"))
-        channel_messages = [msg for msg in messages if msg.is_stream_message()]
+        channel_messages = [msg for msg in messages if msg.is_channel_message]
 
         self.assert_length(result["messages"], 2)
         for i, message in enumerate(result["messages"]):
@@ -2835,7 +2835,7 @@ class GetOldMessagesTest(ZulipTestCase):
         )
 
         messages = get_user_messages(mit_user_profile)
-        channel_messages = [msg for msg in messages if msg.is_stream_message()]
+        channel_messages = [msg for msg in messages if msg.is_channel_message]
         self.assert_length(result["messages"], 5)
         for i, message in enumerate(result["messages"]):
             self.assertEqual(message["type"], "stream")
@@ -2869,7 +2869,7 @@ class GetOldMessagesTest(ZulipTestCase):
         )
 
         messages = get_user_messages(mit_user_profile)
-        channel_messages = [msg for msg in messages if msg.is_stream_message()]
+        channel_messages = [msg for msg in messages if msg.is_channel_message]
         self.assert_length(result["messages"], 7)
         for i, message in enumerate(result["messages"]):
             self.assertEqual(message["type"], "stream")

zerver/tests/test_retention.py

@@ -1151,7 +1151,7 @@ class TestDoDeleteMessages(ZulipTestCase):
         message_ids = [self.send_stream_message(cordelia, "Verona", str(i)) for i in range(10)]
         messages = Message.objects.filter(id__in=message_ids)
 
-        with self.assert_database_query_count(32):
+        with self.assert_database_query_count(23):
             do_delete_messages(realm, messages, acting_user=None)
         self.assertFalse(Message.objects.filter(id__in=message_ids).exists())
 

zerver/views/message_edit.py

@@ -46,7 +46,7 @@ def fill_edit_history_entries(
     """
     prev_content = message.content
     prev_rendered_content = message.rendered_content
-    is_channel_message = message.is_stream_message()
+    is_channel_message = message.is_channel_message
     if is_channel_message:
         prev_topic_name = maybe_rename_empty_topic_to_general_chat(
             message.topic_name(), is_channel_message, allow_empty_topic_name
@@ -185,7 +185,7 @@ def validate_can_delete_message(user_profile: UserProfile, message: Message) ->
         return
 
     stream: Stream | None = None
-    if message.is_stream_message():
+    if message.is_channel_message:
         stream = get_stream_by_id_in_realm(message.recipient.type_id, user_profile.realm)
         if can_delete_any_message_in_channel(user_profile, stream):
             return
@@ -195,7 +195,7 @@ def validate_can_delete_message(user_profile: UserProfile, message: Message) ->
         raise JsonableError(_("You don't have permission to delete this message"))
 
     if not user_profile.can_delete_own_message():
-        if not message.is_stream_message():
+        if not message.is_channel_message:
             raise JsonableError(_("You don't have permission to delete this message"))
 
         assert stream is not None