From 37c8505435c1b569943a2c2399a4112045270d89 Mon Sep 17 00:00:00 2001
From: sahil839 <sahilbatra839@gmail.com>
Date: Mon, 23 Nov 2020 20:08:48 +0530
Subject: [PATCH] message: Raise exception when trying to mirror an already
 sent message.

Previously we were just returning a dict containing a message id when
trying to mirror a already sent message in 'zephyr_mirror' cases.

This commit changes this behaviour to raise an exception when trying
to mirror an already sent message by adding a new exception class
ZephyrMessageAlreadySentException and then the caller returns the
message_id directly, instead of calling do_send_messages which also
returns a list of size one containing the message_id only.

This is a prep commit for changing the return type of check_message to
be a dataclass instead of a Dict as now we have only single output for
check_message.
---
 zerver/lib/actions.py    | 31 ++++++++++---------------------
 zerver/lib/exceptions.py |  4 ++++
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
index ca44ce0fdc..719d4a2979 100644
--- a/zerver/lib/actions.py
+++ b/zerver/lib/actions.py
@@ -88,6 +88,7 @@ from zerver.lib.exceptions import (
     MarkdownRenderingException,
     StreamDoesNotExistError,
     StreamWithIDDoesNotExistError,
+    ZephyrMessageAlreadySentException,
 )
 from zerver.lib.export import get_realm_exports_serialized
 from zerver.lib.external_accounts import DEFAULT_EXTERNAL_ACCOUNTS
@@ -1521,16 +1522,6 @@ def do_send_messages(messages_maybe_none: Sequence[Optional[MutableMapping[str,
     # Filter out messages which didn't pass internal_prep_message properly
     messages = [message for message in messages_maybe_none if message is not None]
 
-    # Filter out zephyr mirror anomalies where the message was already sent
-    already_sent_ids: List[int] = []
-    new_messages: List[MutableMapping[str, Any]] = []
-    for message in messages:
-        if isinstance(message['message'], int):
-            already_sent_ids.append(message['message'])
-        else:
-            new_messages.append(message)
-    messages = new_messages
-
     # Save the message receipts in the database
     user_message_flags: Dict[int, Dict[int, List[str]]] = defaultdict(dict)
     with transaction.atomic():
@@ -1691,11 +1682,7 @@ def do_send_messages(messages_maybe_none: Sequence[Optional[MutableMapping[str,
                     },
                 )
 
-    # Note that this does not preserve the order of message ids
-    # returned.  In practice, this shouldn't matter, as we only
-    # mirror single zephyr messages at a time and don't otherwise
-    # intermingle sending zephyr messages with other messages.
-    return already_sent_ids + [message['message'].id for message in messages]
+    return [message['message'].id for message in messages]
 
 class UserMessageLite:
     '''
@@ -2172,11 +2159,13 @@ def check_send_message(sender: UserProfile, client: Client, message_type_name: s
         message_type_name,
         message_to,
         topic_name)
-
-    message = check_message(sender, client, addressee,
-                            message_content, realm, forged, forged_timestamp,
-                            forwarder_user_profile, local_id, sender_queue_id,
-                            widget_content)
+    try:
+        message = check_message(sender, client, addressee,
+                                message_content, realm, forged, forged_timestamp,
+                                forwarder_user_profile, local_id, sender_queue_id,
+                                widget_content)
+    except ZephyrMessageAlreadySentException as e:
+        return e.message_id
     return do_send_messages([message])[0]
 
 def check_schedule_message(sender: UserProfile, client: Client,
@@ -2433,7 +2422,7 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee,
     if client.name == "zephyr_mirror":
         id = already_sent_mirrored_message_id(message)
         if id is not None:
-            return {'message': id}
+            raise ZephyrMessageAlreadySentException(id)
 
     if widget_content is not None:
         try:
diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py
index dc76f07d48..6245b0fdb0 100644
--- a/zerver/lib/exceptions.py
+++ b/zerver/lib/exceptions.py
@@ -308,3 +308,7 @@ class InvalidSubdomainError(JsonableError):
     @staticmethod
     def msg_format() -> str:
         return _("Invalid subdomain")
+
+class ZephyrMessageAlreadySentException(Exception):
+    def __init__(self, message_id: int) -> None:
+        self.message_id = message_id