Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Improved push typing #11409

Merged
merged 11 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11409.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve internal types in push code.
5 changes: 5 additions & 0 deletions docs/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ Below are the templates Synapse will look for when generating the content of an
* `sender_avatar_url`: the avatar URL (as a `mxc://` URL) for the event's
sender
* `sender_hash`: a hash of the user ID of the sender
* `msgtype`: the type of the message
* `body_text_html`: html representation of the message
* `body_text_plain`: plaintext representation of the message
* `image_url`: mxc url of an image, when "msgtype" is "m.image"
* `link`: a `matrix.to` link to the room
* `avator_url`: url to the room's avator
* `reason`: information on the event that triggered the email to be sent. It's an
object with the following attributes:
* `room_id`: the ID of the room the event was sent in
Expand Down
10 changes: 7 additions & 3 deletions synapse/push/emailpusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.push import Pusher, PusherConfig, PusherConfigException, ThrottleParams
from synapse.push.mailer import Mailer
from synapse.push.push_types import EmailReason
from synapse.storage.databases.main.event_push_actions import EmailPushAction
from synapse.util.threepids import validate_email

if TYPE_CHECKING:
Expand Down Expand Up @@ -190,7 +192,7 @@ async def _unsafe_process(self) -> None:
# we then consider all previously outstanding notifications
# to be delivered.

reason = {
reason: EmailReason = {
"room_id": push_action["room_id"],
"now": self.clock.time_msec(),
"received_at": received_at,
Expand Down Expand Up @@ -275,7 +277,7 @@ def room_ready_to_notify_at(self, room_id: str) -> int:
return may_send_at

async def sent_notif_update_throttle(
self, room_id: str, notified_push_action: dict
self, room_id: str, notified_push_action: EmailPushAction
) -> None:
# We have sent a notification, so update the throttle accordingly.
# If the event that triggered the notif happened more than
Expand Down Expand Up @@ -315,7 +317,9 @@ async def sent_notif_update_throttle(
self.pusher_id, room_id, self.throttle_params[room_id]
)

async def send_notification(self, push_actions: List[dict], reason: dict) -> None:
async def send_notification(
self, push_actions: List[EmailPushAction], reason: EmailReason
) -> None:
logger.info("Sending notif email for user %r", self.user_id)

await self.mailer.send_notification_mail(
Expand Down
3 changes: 2 additions & 1 deletion synapse/push/httppusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from synapse.logging import opentracing
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.push import Pusher, PusherConfig, PusherConfigException
from synapse.storage.databases.main.event_push_actions import HttpPushAction

from . import push_rule_evaluator, push_tools

Expand Down Expand Up @@ -273,7 +274,7 @@ async def _unsafe_process(self) -> None:
)
break

async def _process_one(self, push_action: dict) -> bool:
async def _process_one(self, push_action: HttpPushAction) -> bool:
if "notify" not in push_action["actions"]:
return True

Expand Down
72 changes: 42 additions & 30 deletions synapse/push/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import logging
import urllib.parse
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, TypeVar
from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, TypeVar

import bleach
import jinja2
Expand All @@ -28,6 +28,14 @@
descriptor_from_member_events,
name_from_member_event,
)
from synapse.push.push_types import (
EmailReason,
MessageVars,
NotifVars,
RoomVars,
TemplateVars,
)
from synapse.storage.databases.main.event_push_actions import EmailPushAction
from synapse.storage.state import StateFilter
from synapse.types import StateMap, UserID
from synapse.util.async_helpers import concurrently_execute
Expand Down Expand Up @@ -135,7 +143,7 @@ async def send_password_reset_mail(
% urllib.parse.urlencode(params)
)

template_vars = {"link": link}
template_vars: TemplateVars = {"link": link}

await self.send_email(
email_address,
Expand Down Expand Up @@ -165,7 +173,7 @@ async def send_registration_mail(
% urllib.parse.urlencode(params)
)

template_vars = {"link": link}
template_vars: TemplateVars = {"link": link}

await self.send_email(
email_address,
Expand Down Expand Up @@ -196,7 +204,7 @@ async def send_add_threepid_mail(
% urllib.parse.urlencode(params)
)

template_vars = {"link": link}
template_vars: TemplateVars = {"link": link}

await self.send_email(
email_address,
Expand All @@ -210,8 +218,8 @@ async def send_notification_mail(
app_id: str,
user_id: str,
email_address: str,
push_actions: Iterable[Dict[str, Any]],
reason: Dict[str, Any],
push_actions: Iterable[EmailPushAction],
reason: EmailReason,
) -> None:
"""
Send email regarding a user's room notifications
Expand All @@ -230,7 +238,7 @@ async def send_notification_mail(
[pa["event_id"] for pa in push_actions]
)

notifs_by_room: Dict[str, List[Dict[str, Any]]] = {}
notifs_by_room: Dict[str, List[EmailPushAction]] = {}
for pa in push_actions:
notifs_by_room.setdefault(pa["room_id"], []).append(pa)

Expand Down Expand Up @@ -258,7 +266,7 @@ async def _fetch_room_state(room_id: str) -> None:
# actually sort our so-called rooms_in_order list, most recent room first
rooms_in_order.sort(key=lambda r: -(notifs_by_room[r][-1]["received_ts"] or 0))

rooms: List[Dict[str, Any]] = []
rooms: List[RoomVars] = []

for r in rooms_in_order:
roomvars = await self._get_room_vars(
Expand Down Expand Up @@ -289,7 +297,7 @@ async def _fetch_room_state(room_id: str) -> None:
notifs_by_room, state_by_room, notif_events, reason
)

template_vars = {
template_vars: TemplateVars = {
"user_display_name": user_display_name,
"unsubscribe_link": self._make_unsubscribe_link(
user_id, app_id, email_address
Expand All @@ -302,10 +310,10 @@ async def _fetch_room_state(room_id: str) -> None:
await self.send_email(email_address, summary_text, template_vars)

async def send_email(
self, email_address: str, subject: str, extra_template_vars: Dict[str, Any]
self, email_address: str, subject: str, extra_template_vars: TemplateVars
) -> None:
"""Send an email with the given information and template text"""
template_vars = {
template_vars: TemplateVars = {
"app_name": self.app_name,
"server_name": self.hs.config.server.server_name,
}
Expand All @@ -327,10 +335,10 @@ async def _get_room_vars(
self,
room_id: str,
user_id: str,
notifs: Iterable[Dict[str, Any]],
notifs: Iterable[EmailPushAction],
notif_events: Dict[str, EventBase],
room_state_ids: StateMap[str],
) -> Dict[str, Any]:
) -> RoomVars:
"""
Generate the variables for notifications on a per-room basis.

Expand All @@ -356,7 +364,7 @@ async def _get_room_vars(

room_name = await calculate_room_name(self.store, room_state_ids, user_id)

room_vars: Dict[str, Any] = {
room_vars: RoomVars = {
"title": room_name,
"hash": string_ordinal_total(room_id), # See sender avatar hash
"notifs": [],
Expand Down Expand Up @@ -417,11 +425,11 @@ async def _get_room_avatar(

async def _get_notif_vars(
self,
notif: Dict[str, Any],
notif: EmailPushAction,
user_id: str,
notif_event: EventBase,
room_state_ids: StateMap[str],
) -> Dict[str, Any]:
) -> NotifVars:
"""
Generate the variables for a single notification.

Expand All @@ -442,7 +450,7 @@ async def _get_notif_vars(
after_limit=CONTEXT_AFTER,
)

ret = {
ret: NotifVars = {
"link": self._make_notif_link(notif),
"ts": notif["received_ts"],
"messages": [],
Expand All @@ -461,8 +469,8 @@ async def _get_notif_vars(
return ret

async def _get_message_vars(
self, notif: Dict[str, Any], event: EventBase, room_state_ids: StateMap[str]
) -> Optional[Dict[str, Any]]:
self, notif: EmailPushAction, event: EventBase, room_state_ids: StateMap[str]
) -> Optional[MessageVars]:
"""
Generate the variables for a single event, if possible.

Expand Down Expand Up @@ -494,7 +502,9 @@ async def _get_message_vars(

if sender_state_event:
sender_name = name_from_member_event(sender_state_event)
sender_avatar_url = sender_state_event.content.get("avatar_url")
sender_avatar_url: Optional[str] = sender_state_event.content.get(
"avatar_url"
)
else:
# No state could be found, fallback to the MXID.
sender_name = event.sender
Expand All @@ -504,7 +514,7 @@ async def _get_message_vars(
# sender_hash % the number of default images to choose from
sender_hash = string_ordinal_total(event.sender)

ret = {
ret: MessageVars = {
"event_type": event.type,
"is_historical": event.event_id != notif["event_id"],
"id": event.event_id,
Expand All @@ -519,6 +529,8 @@ async def _get_message_vars(
return ret

msgtype = event.content.get("msgtype")
if not isinstance(msgtype, str):
msgtype = None

ret["msgtype"] = msgtype

Expand All @@ -533,7 +545,7 @@ async def _get_message_vars(
return ret

def _add_text_message_vars(
self, messagevars: Dict[str, Any], event: EventBase
self, messagevars: MessageVars, event: EventBase
) -> None:
"""
Potentially add a sanitised message body to the message variables.
Expand All @@ -543,8 +555,8 @@ def _add_text_message_vars(
event: The event under consideration.
"""
msgformat = event.content.get("format")

messagevars["format"] = msgformat
if not isinstance(msgformat, str):
msgformat = None

formatted_body = event.content.get("formatted_body")
body = event.content.get("body")
Expand All @@ -555,7 +567,7 @@ def _add_text_message_vars(
messagevars["body_text_html"] = safe_text(body)

def _add_image_message_vars(
self, messagevars: Dict[str, Any], event: EventBase
self, messagevars: MessageVars, event: EventBase
) -> None:
"""
Potentially add an image URL to the message variables.
Expand All @@ -570,7 +582,7 @@ def _add_image_message_vars(
async def _make_summary_text_single_room(
self,
room_id: str,
notifs: List[Dict[str, Any]],
notifs: List[EmailPushAction],
room_state_ids: StateMap[str],
notif_events: Dict[str, EventBase],
user_id: str,
Expand Down Expand Up @@ -685,10 +697,10 @@ async def _make_summary_text_single_room(

async def _make_summary_text(
self,
notifs_by_room: Dict[str, List[Dict[str, Any]]],
notifs_by_room: Dict[str, List[EmailPushAction]],
room_state_ids: Dict[str, StateMap[str]],
notif_events: Dict[str, EventBase],
reason: Dict[str, Any],
reason: EmailReason,
) -> str:
"""
Make a summary text for the email when multiple rooms have notifications.
Expand Down Expand Up @@ -718,7 +730,7 @@ async def _make_summary_text(
async def _make_summary_text_from_member_events(
self,
room_id: str,
notifs: List[Dict[str, Any]],
notifs: List[EmailPushAction],
room_state_ids: StateMap[str],
notif_events: Dict[str, EventBase],
) -> str:
Expand Down Expand Up @@ -805,7 +817,7 @@ def _make_room_link(self, room_id: str) -> str:
base_url = "https://matrix.to/#"
return "%s/%s" % (base_url, room_id)

def _make_notif_link(self, notif: Dict[str, str]) -> str:
def _make_notif_link(self, notif: EmailPushAction) -> str:
"""
Generate a link to open an event in the web client.

Expand Down
Loading