markdown: Refactor out additional properties added to Message.

This adds a new class called MessageRenderingResult to contain the
additional properties we added to the Message object (like alert_words)
as well as the rendered content to ensure typesafe reference. No
behavioral change is made except changes in typing.

This is a preparatory change for adding django-stubs to the backend.

Related: zulip#18777

zerver/lib/actions.py

@@ -96,7 +96,7 @@
 from zerver.lib.external_accounts import DEFAULT_EXTERNAL_ACCOUNTS
 from zerver.lib.hotspots import get_next_hotspots
 from zerver.lib.i18n import get_language_name
-from zerver.lib.markdown import topic_links
+from zerver.lib.markdown import MessageRenderingResult, topic_links
 from zerver.lib.markdown import version as markdown_version
 from zerver.lib.mention import MentionData
 from zerver.lib.message import (
@@ -1414,10 +1414,10 @@ def render_incoming_message(
     realm: Realm,
     mention_data: Optional[MentionData] = None,
     email_gateway: bool = False,
-) -> str:
+) -> MessageRenderingResult:
     realm_alert_words_automaton = get_alert_word_automaton(realm)
     try:
-        rendered_content = render_markdown(
+        result = render_markdown(
             message=message,
             content=content,
             realm=realm,
@@ -1427,7 +1427,7 @@ def render_incoming_message(
         )
     except MarkdownRenderingException:
         raise JsonableError(_("Unable to render message"))
-    return rendered_content
+    return result
 
 
 class RecipientInfoResult(TypedDict):
@@ -1781,28 +1781,28 @@ def build_message_send_dict(
     # Render our message_dicts.
     assert message.rendered_content is None
 
-    rendered_content = render_incoming_message(
+    result = render_incoming_message(
         message,
         message.content,
         info["active_user_ids"],
         realm,
         mention_data=mention_data,
         email_gateway=email_gateway,
     )
-    message.rendered_content = rendered_content
+    message.rendered_content = result["rendered_content"]
     message.rendered_content_version = markdown_version
-    links_for_embed = message.links_for_preview
+    links_for_embed = result["links_for_preview"]
 
     # Add members of the mentioned user groups into `mentions_user_ids`.
-    for group_id in message.mentions_user_group_ids:
+    for group_id in result["mentions_user_group_ids"]:
         members = mention_data.get_group_members(group_id)
-        message.mentions_user_ids.update(members)
+        result["mentions_user_ids"].update(members)
 
     # Only send data to Tornado about wildcard mentions if message
     # rendering determined the message had an actual wildcard
     # mention in it (and not e.g. wildcard mention syntax inside a
     # code block).
-    if message.mentions_wildcard:
+    if result["mentions_wildcard"]:
         wildcard_mention_user_ids = info["wildcard_mention_user_ids"]
     else:
         wildcard_mention_user_ids = set()
@@ -1813,7 +1813,7 @@ def build_message_send_dict(
     who were directly mentioned in this message as eligible to
     get UserMessage rows.
     """
-    mentioned_user_ids = message.mentions_user_ids
+    mentioned_user_ids = result["mentions_user_ids"]
     default_bot_user_ids = info["default_bot_user_ids"]
     mentioned_bot_user_ids = default_bot_user_ids & mentioned_user_ids
     info["um_eligible_user_ids"] |= mentioned_bot_user_ids
@@ -1825,6 +1825,7 @@ def build_message_send_dict(
         realm=realm,
         mention_data=mention_data,
         message=message,
+        rendering_result=result,
         active_user_ids=info["active_user_ids"],
         online_push_user_ids=info["online_push_user_ids"],
         stream_push_user_ids=info["stream_push_user_ids"],
@@ -1867,7 +1868,7 @@ def do_send_messages(
         # Claim attachments in message
         for send_request in send_message_requests:
             if do_claim_attachments(
-                send_request.message, send_request.message.potential_attachment_path_ids
+                send_request.message, send_request.rendering_result["potential_attachment_path_ids"]
             ):
                 send_request.message.has_attachment = True
                 send_request.message.save(update_fields=["has_attachment"])
@@ -1876,14 +1877,15 @@ def do_send_messages(
         for send_request in send_message_requests:
             # Service bots (outgoing webhook bots and embedded bots) don't store UserMessage rows;
             # they will be processed later.
-            mentioned_user_ids = send_request.message.mentions_user_ids
+            mentioned_user_ids = send_request.rendering_result["mentions_user_ids"]
 
             # Extend the set with users who have muted the sender.
             mark_as_read_for_users = send_request.muted_sender_user_ids
             mark_as_read_for_users.update(mark_as_read)
 
             user_messages = create_user_messages(
                 message=send_request.message,
+                rendering_result=send_request.rendering_result,
                 um_eligible_user_ids=send_request.um_eligible_user_ids,
                 long_term_idle_user_ids=send_request.long_term_idle_user_ids,
                 stream_push_user_ids=send_request.stream_push_user_ids,
@@ -2069,6 +2071,7 @@ def flags_list(self) -> List[str]:
 
 def create_user_messages(
     message: Message,
+    rendering_result: MessageRenderingResult,
     um_eligible_user_ids: AbstractSet[int],
     long_term_idle_user_ids: AbstractSet[int],
     stream_push_user_ids: AbstractSet[int],
@@ -2078,12 +2081,12 @@ def create_user_messages(
 ) -> List[UserMessageLite]:
     # These properties on the Message are set via
     # render_markdown by code in the Markdown inline patterns
-    ids_with_alert_words = message.user_ids_with_alert_words
+    ids_with_alert_words = rendering_result["user_ids_with_alert_words"]
     sender_id = message.sender.id
     is_stream_message = message.is_stream_message()
 
     base_flags = 0
-    if message.mentions_wildcard:
+    if rendering_result["mentions_wildcard"]:
         base_flags |= UserMessage.flags.wildcard_mentioned
     if message.recipient.type in [Recipient.HUDDLE, Recipient.PERSONAL]:
         base_flags |= UserMessage.flags.is_private
@@ -2889,7 +2892,7 @@ def check_update_message(
         if (timezone_now() - message.date_sent) > datetime.timedelta(seconds=deadline_seconds):
             raise JsonableError(_("The time limit for editing this message's topic has passed"))
 
-    rendered_content = None
+    rendering_result = None
     links_for_embed: Set[str] = set()
     prior_mention_user_ids: Set[int] = set()
     mention_user_ids: Set[int] = set()
@@ -2910,16 +2913,16 @@ def check_update_message(
         # the cross-realm bots never edit messages, this should be
         # always correct.
         # Note: If rendering fails, the called code will raise a JsonableError.
-        rendered_content = render_incoming_message(
+        rendering_result = render_incoming_message(
             message,
             content,
             user_info["message_user_ids"],
             user_profile.realm,
             mention_data=mention_data,
         )
-        links_for_embed |= message.links_for_preview
+        links_for_embed |= rendering_result["links_for_preview"]
 
-        mention_user_ids = message.mentions_user_ids
+        mention_user_ids = rendering_result["mentions_user_ids"]
 
     new_stream = None
     number_changed = 0
@@ -2949,7 +2952,7 @@ def check_update_message(
         send_notification_to_old_thread,
         send_notification_to_new_thread,
         content,
-        rendered_content,
+        rendering_result,
         prior_mention_user_ids,
         mention_user_ids,
         mention_data,
@@ -3256,7 +3259,7 @@ def check_message(
         email_gateway=email_gateway,
     )
 
-    if stream is not None and message_send_dict.message.mentions_wildcard:
+    if stream is not None and message_send_dict.rendering_result["mentions_wildcard"]:
         if not wildcard_mention_allowed(sender, stream):
             raise JsonableError(
                 _("You do not have permission to use wildcard mentions in this stream.")
@@ -5503,7 +5506,10 @@ def do_mark_muted_user_messages_as_read(
 
 
 def do_update_mobile_push_notification(
-    message: Message, prior_mention_user_ids: Set[int], stream_push_user_ids: Set[int]
+    message: Message,
+    current_mention_user_ids: Set[int],
+    prior_mention_user_ids: Set[int],
+    stream_push_user_ids: Set[int],
 ) -> None:
     # Called during the message edit code path to remove mobile push
     # notifications for users who are no longer mentioned following
@@ -5516,7 +5522,7 @@ def do_update_mobile_push_notification(
     if not message.is_stream_message():
         return
 
-    remove_notify_users = prior_mention_user_ids - message.mentions_user_ids - stream_push_user_ids
+    remove_notify_users = prior_mention_user_ids - current_mention_user_ids - stream_push_user_ids
     do_clear_mobile_push_notifications_for_ids(list(remove_notify_users), [message.id])
 
 
@@ -5734,10 +5740,12 @@ def get_user_info_for_message_updates(message_id: int) -> MessageUpdateUserInfoR
     )
 
 
-def update_user_message_flags(message: Message, ums: Iterable[UserMessage]) -> None:
-    wildcard = message.mentions_wildcard
-    mentioned_ids = message.mentions_user_ids
-    ids_with_alert_words = message.user_ids_with_alert_words
+def update_user_message_flags(
+    rendering_result: MessageRenderingResult, ums: Iterable[UserMessage]
+) -> None:
+    wildcard = rendering_result["mentions_wildcard"]
+    mentioned_ids = rendering_result["mentions_user_ids"]
+    ids_with_alert_words = rendering_result["user_ids_with_alert_words"]
     changed_ums: Set[UserMessage] = set()
 
     def update_flag(um: UserMessage, should_set: bool, flag: int) -> None:
@@ -5786,16 +5794,18 @@ def do_update_embedded_data(
     user_profile: UserProfile,
     message: Message,
     content: Optional[str],
-    rendered_content: Optional[str],
+    rendering_result: Optional[MessageRenderingResult],
 ) -> None:
     event: Dict[str, Any] = {"type": "update_message", "message_id": message.id}
     changed_messages = [message]
+    rendered_content: Optional[str] = None
 
     ums = UserMessage.objects.filter(message=message.id)
 
     if content is not None:
-        update_user_message_flags(message, ums)
-        message.content = content
+        if rendering_result is not None:
+            update_user_message_flags(rendering_result, ums)
+            rendered_content = rendering_result["rendered_content"]
         message.rendered_content = rendered_content
         message.rendered_content_version = markdown_version
         event["content"] = content
@@ -5835,7 +5845,7 @@ def do_update_message(
     send_notification_to_old_thread: bool,
     send_notification_to_new_thread: bool,
     content: Optional[str],
-    rendered_content: Optional[str],
+    rendering_result: Optional[MessageRenderingResult],
     prior_mention_user_ids: Set[int],
     mention_user_ids: Set[int],
     mention_data: Optional[MentionData] = None,
@@ -5879,17 +5889,17 @@ def do_update_message(
     ums = UserMessage.objects.filter(message=target_message.id)
 
     if content is not None:
-        assert rendered_content is not None
+        assert rendering_result is not None
 
         # mention_data is required if there's a content edit.
         assert mention_data is not None
 
         # add data from group mentions to mentions_user_ids.
-        for group_id in target_message.mentions_user_group_ids:
+        for group_id in rendering_result["mentions_user_group_ids"]:
             members = mention_data.get_group_members(group_id)
-            target_message.mentions_user_ids.update(members)
+            rendering_result["mentions_user_ids"].update(members)
 
-        update_user_message_flags(target_message, ums)
+        update_user_message_flags(rendering_result, ums)
 
         # One could imagine checking realm.allow_edit_history here and
         # modifying the events based on that setting, but doing so
@@ -5907,16 +5917,16 @@ def do_update_message(
             "prev_rendered_content_version"
         ] = target_message.rendered_content_version
         target_message.content = content
-        target_message.rendered_content = rendered_content
+        target_message.rendered_content = rendering_result["rendered_content"]
         target_message.rendered_content_version = markdown_version
         event["content"] = content
-        event["rendered_content"] = rendered_content
+        event["rendered_content"] = rendering_result["rendered_content"]
         event["prev_rendered_content_version"] = target_message.rendered_content_version
-        event["is_me_message"] = Message.is_status_message(content, rendered_content)
+        event["is_me_message"] = Message.is_status_message(content, rendering_result["rendered_content"])
 
         # target_message.has_image and target_message.has_link will have been
         # already updated by Markdown rendering in the caller.
-        target_message.has_attachment = check_attachment_reference_change(target_message)
+        target_message.has_attachment = check_attachment_reference_change(target_message, rendering_result)
 
         if target_message.is_stream_message():
             if topic_name is not None:
@@ -5946,13 +5956,16 @@ def do_update_message(
         event["prior_mention_user_ids"] = list(prior_mention_user_ids)
         event["mention_user_ids"] = list(mention_user_ids)
         event["presence_idle_user_ids"] = filter_presence_idle_user_ids(info["active_user_ids"])
-        if target_message.mentions_wildcard:
+        if rendering_result["mentions_wildcard"]:
             event["wildcard_mention_user_ids"] = list(info["wildcard_mention_user_ids"])
         else:
             event["wildcard_mention_user_ids"] = []
 
         do_update_mobile_push_notification(
-            target_message, prior_mention_user_ids, info["stream_push_user_ids"]
+            target_message,
+            rendering_result["mentions_user_ids"],
+            prior_mention_user_ids,
+            info["stream_push_user_ids"],
         )
 
     if topic_name is not None or new_stream is not None:
@@ -7330,12 +7343,12 @@ def do_delete_old_unclaimed_attachments(weeks_ago: int) -> None:
         attachment.delete()
 
 
-def check_attachment_reference_change(message: Message) -> bool:
+def check_attachment_reference_change(message: Message, rendering_result: MessageRenderingResult) -> bool:
     # For a unsaved message edit (message.* has been updated, but not
     # saved to the database), adjusts Attachment data to correspond to
     # the new content.
     prev_attachments = {a.path_id for a in message.attachment_set.all()}
-    new_attachments = set(message.potential_attachment_path_ids)
+    new_attachments = set(rendering_result["potential_attachment_path_ids"])
 
     if new_attachments == prev_attachments:
         return bool(prev_attachments)

zerver/lib/import_realm.py

@@ -347,7 +347,7 @@ def fix_message_rendered_content(
                 message_realm=realm,
                 sent_by_bot=sent_by_bot,
                 translate_emoticons=translate_emoticons,
-            )
+            )["rendered_content"]
 
             message["rendered_content"] = rendered_content
             message["rendered_content_version"] = markdown_version