django_api: Rename 'send_event' to 'send_event_rollback_unsafe'.

This commit renames the 'send_event' function to
'send_event_rollback_unsafe' to reflect the fact that it doesn't
wait for the db transaction (within which it gets called, if any)
to commit and sends event irrespective of commit or rollback.

In most of the cases we don't want to send event in the case of
rollbacks, so the caller should be aware that calling the function
directly is rollback unsafe.

zerver/actions/message_flags.py

@@ -20,7 +20,7 @@ from zerver.lib.stream_subscription import get_subscribed_stream_recipient_ids_f
 from zerver.lib.topic import filter_by_topic_name_via_message
 from zerver.lib.user_message import DEFAULT_HISTORICAL_FLAGS, create_historical_user_messages
 from zerver.models import Message, Recipient, UserMessage, UserProfile
-from zerver.tornado.django_api import send_event, send_event_on_commit
+from zerver.tornado.django_api import send_event_on_commit, send_event_rollback_unsafe
 
 
 @dataclass
@@ -96,7 +96,7 @@ def do_mark_all_as_read(user_profile: UserProfile, *, timeout: float | None = No
             all=True,
         )
     )
-    send_event(user_profile.realm, event, [user_profile.id])
+    send_event_rollback_unsafe(user_profile.realm, event, [user_profile.id])
 
     return count
 

zerver/actions/presence.py

@@ -15,7 +15,7 @@ from zerver.lib.users import get_user_ids_who_can_access_user
 from zerver.models import Client, UserPresence, UserProfile
 from zerver.models.clients import get_client
 from zerver.models.users import active_user_ids
-from zerver.tornado.django_api import send_event
+from zerver.tornado.django_api import send_event_rollback_unsafe
 
 logger = logging.getLogger(__name__)
 
@@ -72,7 +72,7 @@ def send_presence_changed(
         server_timestamp=time.time(),
         presence={presence_dict["client"]: presence_dict},
     )
-    send_event(user_profile.realm, event, user_ids)
+    send_event_rollback_unsafe(user_profile.realm, event, user_ids)
 
 
 def consolidate_client(client: Client) -> Client:

zerver/actions/typing.py

@@ -5,7 +5,7 @@ from zerver.lib.exceptions import JsonableError
 from zerver.lib.stream_subscription import get_active_subscriptions_for_stream_id
 from zerver.models import Realm, Stream, UserProfile
 from zerver.models.users import get_user_by_id_in_realm_including_cross_realm
-from zerver.tornado.django_api import send_event
+from zerver.tornado.django_api import send_event_rollback_unsafe
 
 
 def do_send_typing_notification(
@@ -32,7 +32,7 @@ def do_send_typing_notification(
         if user.is_active and user.receives_typing_notifications
     ]
 
-    send_event(realm, event, user_ids_to_notify)
+    send_event_rollback_unsafe(realm, event, user_ids_to_notify)
 
 
 # check_send_typing_notification:
@@ -98,4 +98,4 @@ def do_send_stream_typing_notification(
         .values_list("user_profile_id", flat=True)
     )
 
-    send_event(sender.realm, event, user_ids_to_notify)
+    send_event_rollback_unsafe(sender.realm, event, user_ids_to_notify)

zerver/actions/users.py

@@ -123,8 +123,8 @@ def do_delete_user_preserving_messages(user_profile: UserProfile) -> None:
     conversations that they may have participated in.
 
     Not recommended for general use due to the following quirks:
-    * Does not live-update other clients via `send_event` about the
+    * Does not live-update other clients via `send_event_on_commit`
-      user's new name, email, or other attributes.
+      about the user's new name, email, or other attributes.
     * Not guaranteed to clear caches containing the deleted users. The
       temporary user may be visible briefly in caches due to the
       UserProfile model's post_save hook.

zerver/lib/create_user.py

@@ -24,7 +24,7 @@ def copy_default_settings(
     settings_source: UserProfile | RealmUserDefault, target_profile: UserProfile
 ) -> None:
     # Important note: Code run from here to configure the user's
-    # settings should not call send_event, as that would cause clients
+    # settings should not send events, as that would cause clients
     # to throw an exception (we haven't sent the realm_user/add event
     # yet, so that event will include the updated details of target_profile).
     #

zerver/lib/test_classes.py

@@ -2052,11 +2052,11 @@ class ZulipTestCase(ZulipTestCaseMixin, TestCase):
         # So explicitly change parameter name to 'notice' to work around this problem
         with (
             mock.patch("zerver.tornado.event_queue.process_notification", lst.append),
-            # Some `send_event` calls need to be executed only after the current transaction
+            # Some `send_event_rollback_unsafe` calls need to be executed only after the
-            # commits (using `on_commit` hooks). Because the transaction in Django tests never
+            # current transaction commits (using `on_commit` hooks). Because the transaction
-            # commits (rather, gets rolled back after the test completes), such events would
+            # in Django tests never commits (rather, gets rolled back after the test completes),
-            # never be sent in tests, and we would be unable to verify them. Hence, we use
+            # such events would never be sent in tests, and we would be unable to verify them.
-            # this helper to make sure the `send_event` calls actually run.
+            # Hence, we use this helper to make sure the `send_event_rollback_unsafe` calls actually run.
             self.captureOnCommitCallbacks(execute=True),
         ):
             yield lst

zerver/models/realms.py

@@ -856,7 +856,7 @@ class Realm(models.Model):  # type: ignore[django-manager-missing] # django-stub
         self, include_realm_owners: bool = True
     ) -> QuerySet["UserProfile"]:
         """Use this in contexts where we want administrative users as well as
-        bots with administrator privileges, like send_event calls for
+        bots with administrator privileges, like send_event_on_commit calls for
         notifications to all administrator users.
         """
         if include_realm_owners:

zerver/tests/test_events.py

@@ -252,7 +252,7 @@ from zerver.models.realm_audit_logs import AuditLogEventType
 from zerver.models.streams import get_stream
 from zerver.models.users import get_user_by_delivery_email
 from zerver.openapi.openapi import validate_against_openapi_schema
-from zerver.tornado.django_api import send_event
+from zerver.tornado.django_api import send_event_rollback_unsafe
 from zerver.tornado.event_queue import (
     allocate_client_descriptor,
     clear_client_event_queues_for_testing,
@@ -267,7 +267,7 @@ from zerver.worker.thumbnail import ensure_thumbnails
 
 class BaseAction(ZulipTestCase):
     """Core class for verifying the apply_event race handling logic as
-    well as the event formatting logic of any function using send_event.
+    well as the event formatting logic of any function using send_event_rollback_unsafe.
 
     See https://zulip.readthedocs.io/en/latest/subsystems/events-system.html#testing
     for extensive design details for this testing system.
@@ -355,8 +355,8 @@ class BaseAction(ZulipTestCase):
 
         events: list[dict[str, Any]] = []
 
-        # We want even those `send_event` calls which have been hooked to
+        # We want even those `send_event_rollback_unsafe` calls which have
-        # `transaction.on_commit` to execute in tests.
+        # been hooked to `transaction.on_commit` to execute in tests.
         # See the comment in `ZulipTestCase.capture_send_event_calls`.
         with self.captureOnCommitCallbacks(execute=True):
             yield events
@@ -1174,7 +1174,7 @@ class NormalActionsTest(BaseAction):
 
     def test_heartbeat_event(self) -> None:
         with self.verify_action(state_change_expected=False) as events:
-            send_event(
+            send_event_rollback_unsafe(
                 self.user_profile.realm,
                 create_heartbeat_event(),
                 [self.user_profile.id],

zerver/tests/test_message_delete.py

@@ -488,9 +488,9 @@ class DeleteMessageTest(ZulipTestCase):
 
     def test_delete_event_sent_after_transaction_commits(self) -> None:
         """
-        Tests that `send_event` is hooked to `transaction.on_commit`. This is important, because
+        Tests that `send_event_rollback_unsafe` is hooked to `transaction.on_commit`.
-        we don't want to end up holding locks on message rows for too long if the event queue runs
+        This is important, because we don't want to end up holding locks on message rows
-        into a problem.
+        for too long if the event queue runs into a problem.
         """
         hamlet = self.example_user("hamlet")
         self.send_stream_message(hamlet, "Denmark")

zerver/tests/test_reactions.py

@@ -1124,9 +1124,9 @@ class ReactionAPIEventTest(EmojiReactionBase):
 
     def test_events_sent_after_transaction_commits(self) -> None:
         """
-        Tests that `send_event` is hooked to `transaction.on_commit`. This is important, because
+        Tests that `send_event_rollback_unsafe` is hooked to `transaction.on_commit`.
-        we don't want to end up holding locks on message rows for too long if the event queue runs
+        This is important, because we don't want to end up holding locks on message rows
-        into a problem.
+        for too long if the event queue runs into a problem.
         """
         hamlet = self.example_user("hamlet")
         self.send_stream_message(hamlet, "Denmark")

zerver/tests/test_submessage.py

@@ -187,9 +187,9 @@ class TestBasics(ZulipTestCase):
 
     def test_submessage_event_sent_after_transaction_commits(self) -> None:
         """
-        Tests that `send_event` is hooked to `transaction.on_commit`. This is important, because
+        Tests that `send_event_rollback_unsafe` is hooked to `transaction.on_commit`.
-        we don't want to end up holding locks on message rows for too long if the event queue runs
+        This is important, because we don't want to end up holding locks on message rows
-        into a problem.
+        for too long if the event queue runs into a problem.
         """
         hamlet = self.example_user("hamlet")
         message_id = self.send_stream_message(hamlet, "Denmark")

zerver/tornado/django_api.py

@@ -170,15 +170,21 @@ def send_notification_http(port: int, data: Mapping[str, Any]) -> None:
 
 # The core function for sending an event from Django to Tornado (which
 # will then push it to web and mobile clients for the target users).
-# By convention, send_event should only be called from
+#
-# zerver/actions/*.py, which helps make it easy to find event
+# One should generally use `send_event_on_commit` unless there's a strong
-# generation code.
+# reason to use `send_event_rollback_unsafe` directly, as it doesn't wait
+# for the db transaction (within which it gets called, if any) to commit
+# and sends event irrespective of commit or rollback.
+#
+# By convention, `send_event_rollback_unsafe` / `send_event_on_commit`
+# should only be called from zerver/actions/*.py, which helps make it
+# easy to find event generation code.
 #
 # Every call point should be covered by a test in `test_events.py`,
 # with the schema verified in `zerver/lib/event_schema.py`.
 #
 # See https://zulip.readthedocs.io/en/latest/subsystems/events-system.html
-def send_event(
+def send_event_rollback_unsafe(
     realm: Realm, event: Mapping[str, Any], users: Iterable[int] | Iterable[Mapping[str, Any]]
 ) -> None:
     """`users` is a list of user IDs, or in some special cases like message
@@ -203,4 +209,4 @@ def send_event(
 def send_event_on_commit(
     realm: Realm, event: Mapping[str, Any], users: Iterable[int] | Iterable[Mapping[str, Any]]
 ) -> None:
-    transaction.on_commit(lambda: send_event(realm, event, users))
+    transaction.on_commit(lambda: send_event_rollback_unsafe(realm, event, users))