Skip to content

Commit

Permalink
django_api: Extract send_event_on_commit helper.
Browse files Browse the repository at this point in the history
django-stubs 4.2.1 gives transaction.on_commit a more accurate type
annotation, but this exposed that mypy can’t handle the lambda default
parameters that we use to recapture loop variables such as

    for stream_id in public_stream_ids:
        peer_user_ids = …
        event = …

        transaction.on_commit(
            lambda event=event, peer_user_ids=peer_user_ids: send_event(
                realm, event, peer_user_ids
            )
        )

python/mypy#15459

A workaround that mypy accepts is

        transaction.on_commit(
            (
                lambda event, peer_user_ids: lambda: send_event(
                    realm, event, peer_user_ids
                )
            )(event, peer_user_ids)
        )

But that’s kind of ugly and potentially error-prone, so let’s make a
helper function for this very common pattern.

        send_event_on_commit(realm, event, peer_user_ids)

Signed-off-by: Anders Kaseorg <[email protected]>
  • Loading branch information
andersk committed Jun 18, 2023
1 parent 92c83c1 commit bd00c8d
Show file tree
Hide file tree
Showing 22 changed files with 114 additions and 180 deletions.
58 changes: 23 additions & 35 deletions zerver/actions/bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from zerver.actions.create_user import created_bot_event
from zerver.models import RealmAuditLog, Stream, UserProfile, active_user_ids, bot_owner_user_ids
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


@transaction.atomic(durable=True)
Expand Down Expand Up @@ -41,20 +41,18 @@ def do_change_bot_owner(
),
)
previous_owner_id = previous_owner.id
transaction.on_commit(
lambda: send_event(
user_profile.realm,
delete_event,
{previous_owner_id},
)
send_event_on_commit(
user_profile.realm,
delete_event,
{previous_owner_id},
)
# Do not send update event for previous bot owner.
update_users = update_users - {previous_owner.id}

# Notify the new owner that the bot has been added.
if not bot_owner.is_realm_admin:
add_event = created_bot_event(user_profile)
transaction.on_commit(lambda: send_event(user_profile.realm, add_event, {bot_owner.id}))
send_event_on_commit(user_profile.realm, add_event, {bot_owner.id})
# Do not send update event for bot_owner.
update_users = update_users - {bot_owner.id}

Expand All @@ -66,12 +64,10 @@ def do_change_bot_owner(
owner_id=user_profile.bot_owner.id,
),
)
transaction.on_commit(
lambda: send_event(
user_profile.realm,
bot_event,
update_users,
)
send_event_on_commit(
user_profile.realm,
bot_event,
update_users,
)

# Since `bot_owner_id` is included in the user profile dict we need
Expand All @@ -84,9 +80,7 @@ def do_change_bot_owner(
bot_owner_id=user_profile.bot_owner.id,
),
)
transaction.on_commit(
lambda: send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id))
)
send_event_on_commit(user_profile.realm, event, active_user_ids(user_profile.realm_id))


@transaction.atomic(durable=True)
Expand Down Expand Up @@ -125,12 +119,10 @@ def do_change_default_sending_stream(
default_sending_stream=stream_name,
),
)
transaction.on_commit(
lambda: send_event(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)
send_event_on_commit(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)


Expand Down Expand Up @@ -171,12 +163,10 @@ def do_change_default_events_register_stream(
default_events_register_stream=stream_name,
),
)
transaction.on_commit(
lambda: send_event(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)
send_event_on_commit(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)


Expand Down Expand Up @@ -212,10 +202,8 @@ def do_change_default_all_public_streams(
default_all_public_streams=user_profile.default_all_public_streams,
),
)
transaction.on_commit(
lambda: send_event(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)
send_event_on_commit(
user_profile.realm,
event,
bot_owner_user_ids(user_profile),
)
18 changes: 4 additions & 14 deletions zerver/actions/create_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
bot_owner_user_ids,
get_system_bot,
)
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit

if settings.BILLING_ENABLED:
from corporate.lib.stripe import update_license_ledger_if_needed
Expand Down Expand Up @@ -311,20 +311,12 @@ def notify_created_user(user_profile: UserProfile) -> None:
event: Dict[str, Any] = dict(
type="realm_user", op="add", person=person_for_real_email_access_users
)
transaction.on_commit(
lambda event=event: send_event(
user_profile.realm, event, user_ids_with_real_email_access
)
)
send_event_on_commit(user_profile.realm, event, user_ids_with_real_email_access)

if user_ids_without_real_email_access:
assert person_for_without_real_email_access_users is not None
event = dict(type="realm_user", op="add", person=person_for_without_real_email_access_users)
transaction.on_commit(
lambda event=event: send_event(
user_profile.realm, event, user_ids_without_real_email_access
)
)
send_event_on_commit(user_profile.realm, event, user_ids_without_real_email_access)


def created_bot_event(user_profile: UserProfile) -> Dict[str, Any]:
Expand Down Expand Up @@ -361,9 +353,7 @@ def stream_name(stream: Optional[Stream]) -> Optional[str]:

def notify_created_bot(user_profile: UserProfile) -> None:
event = created_bot_event(user_profile)
transaction.on_commit(
lambda: send_event(user_profile.realm, event, bot_owner_user_ids(user_profile))
)
send_event_on_commit(user_profile.realm, event, bot_owner_user_ids(user_profile))


def do_create_user(
Expand Down
6 changes: 3 additions & 3 deletions zerver/actions/default_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
active_non_guest_user_ids,
get_default_stream_groups,
)
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def check_default_stream_group_name(group_name: str) -> None:
Expand Down Expand Up @@ -52,7 +52,7 @@ def notify_default_streams(realm: Realm) -> None:
type="default_streams",
default_streams=streams_to_dicts_sorted(get_default_streams_for_realm(realm.id)),
)
transaction.on_commit(lambda: send_event(realm, event, active_non_guest_user_ids(realm.id)))
send_event_on_commit(realm, event, active_non_guest_user_ids(realm.id))


def notify_default_stream_groups(realm: Realm) -> None:
Expand All @@ -62,7 +62,7 @@ def notify_default_stream_groups(realm: Realm) -> None:
get_default_stream_groups(realm)
),
)
transaction.on_commit(lambda: send_event(realm, event, active_non_guest_user_ids(realm.id)))
send_event_on_commit(realm, event, active_non_guest_user_ids(realm.id))


def do_add_default_stream(stream: Stream) -> None:
Expand Down
6 changes: 2 additions & 4 deletions zerver/actions/message_delete.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
from typing import Iterable, List, TypedDict

from django.db import transaction

from zerver.lib import retention
from zerver.lib.retention import move_messages_to_archive
from zerver.lib.stream_subscription import get_active_subscriptions_for_stream_id
from zerver.models import Message, Realm, UserMessage, UserProfile
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


class DeleteMessagesEvent(TypedDict, total=False):
Expand Down Expand Up @@ -56,7 +54,7 @@ def do_delete_messages(realm: Realm, messages: Iterable[Message]) -> None:
move_messages_to_archive(message_ids, realm=realm, chunk_size=archiving_chunk_size)

event["message_type"] = message_type
transaction.on_commit(lambda: send_event(realm, event, users_to_notify))
send_event_on_commit(realm, event, users_to_notify)


def do_delete_messages_by_sender(user: UserProfile) -> None:
Expand Down
5 changes: 2 additions & 3 deletions zerver/actions/reactions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Any, Dict, Optional

from django.db import transaction
from django.utils.translation import gettext as _

from zerver.actions.create_user import create_historical_user_messages
Expand All @@ -9,7 +8,7 @@
from zerver.lib.message import access_message, update_to_dict_cache
from zerver.lib.stream_subscription import subscriber_ids_with_stream_history_access
from zerver.models import Message, Reaction, Recipient, Stream, UserMessage, UserProfile
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def notify_reaction_update(
Expand Down Expand Up @@ -60,7 +59,7 @@ def notify_reaction_update(
stream = Stream.objects.get(id=stream_id)
user_ids |= subscriber_ids_with_stream_history_access(stream)

transaction.on_commit(lambda: send_event(user_profile.realm, event, list(user_ids)))
send_event_on_commit(user_profile.realm, event, list(user_ids))


def do_add_reaction(
Expand Down
10 changes: 4 additions & 6 deletions zerver/actions/realm_domains.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
active_user_ids,
get_realm_domains,
)
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


@transaction.atomic(durable=True)
Expand Down Expand Up @@ -46,7 +46,7 @@ def do_add_realm_domain(
domain=realm_domain.domain, allow_subdomains=realm_domain.allow_subdomains
),
)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))

return realm_domain

Expand Down Expand Up @@ -82,9 +82,7 @@ def do_change_realm_domain(
domain=realm_domain.domain, allow_subdomains=realm_domain.allow_subdomains
),
)
transaction.on_commit(
lambda: send_event(realm_domain.realm, event, active_user_ids(realm_domain.realm_id))
)
send_event_on_commit(realm_domain.realm, event, active_user_ids(realm_domain.realm_id))


@transaction.atomic(durable=True)
Expand Down Expand Up @@ -119,4 +117,4 @@ def do_remove_realm_domain(
# confusing than the alternative.
do_set_realm_property(realm, "emails_restricted_to_domains", False, acting_user=acting_user)
event = dict(type="realm_domains", op="remove", domain=domain)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))
4 changes: 2 additions & 2 deletions zerver/actions/realm_emoji.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
from zerver.lib.pysa import mark_sanitized
from zerver.lib.upload import upload_emoji_image
from zerver.models import EmojiInfo, Realm, RealmAuditLog, RealmEmoji, UserProfile, active_user_ids
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def notify_realm_emoji(realm: Realm, realm_emoji: Dict[str, EmojiInfo]) -> None:
event = dict(type="realm_emoji", op="update", realm_emoji=realm_emoji)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))


def check_add_realm_emoji(
Expand Down
5 changes: 2 additions & 3 deletions zerver/actions/realm_export.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import orjson
from django.db import transaction
from django.utils.timezone import now as timezone_now

from zerver.lib.export import get_realm_exports_serialized
from zerver.lib.upload import delete_export_tarball
from zerver.models import RealmAuditLog, UserProfile
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def notify_realm_export(user_profile: UserProfile) -> None:
# In the future, we may want to send this event to all realm admins.
event = dict(type="realm_export", exports=get_realm_exports_serialized(user_profile))
transaction.on_commit(lambda: send_event(user_profile.realm, event, [user_profile.id]))
send_event_on_commit(user_profile.realm, event, [user_profile.id])


def do_delete_realm_export(user_profile: UserProfile, export: RealmAuditLog) -> None:
Expand Down
12 changes: 5 additions & 7 deletions zerver/actions/realm_icon.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from zerver.lib.realm_icon import realm_icon_url
from zerver.models import Realm, RealmAuditLog, UserProfile, active_user_ids
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


@transaction.atomic(durable=True)
Expand All @@ -31,10 +31,8 @@ def do_change_icon_source(
property="icon",
data=dict(icon_source=realm.icon_source, icon_url=realm_icon_url(realm)),
)
transaction.on_commit(
lambda: send_event(
realm,
event,
active_user_ids(realm.id),
)
send_event_on_commit(
realm,
event,
active_user_ids(realm.id),
)
4 changes: 2 additions & 2 deletions zerver/actions/realm_linkifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
active_user_ids,
linkifiers_for_realm,
)
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def notify_linkifiers(realm: Realm, realm_linkifiers: List[LinkifierDict]) -> None:
event: Dict[str, object] = dict(type="realm_linkifiers", realm_linkifiers=realm_linkifiers)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))


# NOTE: Regexes must be simple enough that they can be easily translated to JavaScript
Expand Down
4 changes: 2 additions & 2 deletions zerver/actions/realm_logo.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from zerver.lib.realm_logo import get_realm_logo_data
from zerver.models import Realm, RealmAuditLog, UserProfile, active_user_ids
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


@transaction.atomic(durable=True)
Expand Down Expand Up @@ -35,4 +35,4 @@ def do_change_logo_source(
property="night_logo" if night else "logo",
data=get_realm_logo_data(realm, night),
)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))
4 changes: 2 additions & 2 deletions zerver/actions/realm_playgrounds.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
active_user_ids,
get_realm_playgrounds,
)
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit


def notify_realm_playgrounds(realm: Realm, realm_playgrounds: List[RealmPlaygroundDict]) -> None:
event = dict(type="realm_playgrounds", realm_playgrounds=realm_playgrounds)
transaction.on_commit(lambda: send_event(realm, event, active_user_ids(realm.id)))
send_event_on_commit(realm, event, active_user_ids(realm.id))


@transaction.atomic(durable=True)
Expand Down
Loading

0 comments on commit bd00c8d

Please sign in to comment.