push notifications: Go to the DB for streams.

We want to phase out the use of get_display_recipient
for streams, and this is the last place that I
eliminate it. The next commit will eliminate the
dead code and make mypy types tighter.

This change will make push notifications slightly
slower in some situations, but we avoid all the
complexity of a cache, and this code tends to run
offline.

We could always make this code a bit more efficient
by being a little smarter about what data we fetch
up front. For example, get_apns_alert_title gets
called by a function that already has the stream
name. It's just a bit of a pain to refactor when
you have all the DM codepath mucked up with the
stream codepath.
This commit is contained in:
Steve Howell
2023-07-16 11:59:49 +00:00
committed by Tim Abbott
parent 63c0ed303d
commit 1b7880fc21
2 changed files with 16 additions and 4 deletions

View File

@@ -40,7 +40,7 @@ def get_display_recipient_remote_cache(
stream this will be the stream name as a string. For a huddle or
personal, it will be an array of dicts about each recipient.
"""
if recipient_type == Recipient.STREAM:
if recipient_type == Recipient.STREAM: # nocoverage
assert recipient_type_id is not None
stream = Stream.objects.values("name").get(id=recipient_type_id)
return stream["name"]

View File

@@ -33,6 +33,7 @@ from zerver.models import (
NotificationTriggers,
PushDeviceToken,
Recipient,
Stream,
UserGroup,
UserMessage,
UserProfile,
@@ -60,6 +61,16 @@ def hex_to_b64(data: str) -> str:
return base64.b64encode(bytes.fromhex(data)).decode()
def get_message_stream_name_from_database(message: Message) -> str:
"""
Never use this function outside of the push-notifications
codepath. Most of our code knows how to get streams
up front in a more efficient manner.
"""
stream_id = message.recipient.type_id
return Stream.objects.get(id=stream_id).name
class UserPushIdentityCompat:
"""Compatibility class for supporting the transition from remote servers
sending their UserProfile ids to the bouncer to sending UserProfile uuids instead.
@@ -669,7 +680,7 @@ def get_gcm_alert(
return f"New direct message from {sender_str}"
assert message.is_stream_message()
stream_name = get_display_recipient(message.recipient)
stream_name = get_message_stream_name_from_database(message)
if trigger == NotificationTriggers.MENTION:
if mentioned_user_group_name is None:
@@ -805,7 +816,7 @@ def get_message_payload(
if message.recipient.type == Recipient.STREAM:
data["recipient_type"] = "stream"
data["stream"] = get_display_recipient(message.recipient)
data["stream"] = get_message_stream_name_from_database(message)
data["stream_id"] = message.recipient.type_id
data["topic"] = message.topic_name()
elif message.recipient.type == Recipient.HUDDLE:
@@ -826,7 +837,8 @@ def get_apns_alert_title(message: Message) -> str:
assert isinstance(recipients, list)
return ", ".join(sorted(r["full_name"] for r in recipients))
elif message.is_stream_message():
return f"#{get_display_recipient(message.recipient)} > {message.topic_name()}"
stream_name = get_message_stream_name_from_database(message)
return f"#{stream_name} > {message.topic_name()}"
# For 1:1 direct messages, we just show the sender name.
return message.sender.full_name