Improved push typing (#11409)

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

7 files changed, 210 insertions, 36 deletions

diff --git a/changelog.d/11409.misc b/changelog.d/11409.misc
new file mode 100644
index 0000000000..f9e8ae9e3a
--- /dev/null
+++ b/changelog.d/11409.misc
@@ -0,0 +1 @@
+Improve internal types in push code.
diff --git a/docs/templates.md b/docs/templates.md
index a240f58b54..2b66e9d862 100644
--- a/docs/templates.md
+++ b/docs/templates.md
@@ -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
diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py
index cf5abdfbda..4f13c0418a 100644
--- a/synapse/push/emailpusher.py
+++ b/synapse/push/emailpusher.py
@@ -21,6 +21,8 @@ from twisted.internet.interfaces import IDelayedCall
 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:
@@ -190,7 +192,7 @@ class EmailPusher(Pusher):
                 # 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,
@@ -275,7 +277,7 @@ class EmailPusher(Pusher):
         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
@@ -315,7 +317,9 @@ class EmailPusher(Pusher):
             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(
diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py
index dbf4ad7f97..3fa603ccb7 100644
--- a/synapse/push/httppusher.py
+++ b/synapse/push/httppusher.py
@@ -26,6 +26,7 @@ from synapse.events import EventBase
 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
 
@@ -273,7 +274,7 @@ class HttpPusher(Pusher):
                     )
                     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
 
diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index ce299ba3da..ba4f866487 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -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
@@ -28,6 +28,14 @@ from synapse.push.presentable_names import (
     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
@@ -135,7 +143,7 @@ class Mailer:
             % urllib.parse.urlencode(params)
         )
 
-        template_vars = {"link": link}
+        template_vars: TemplateVars = {"link": link}
 
         await self.send_email(
             email_address,
@@ -165,7 +173,7 @@ class Mailer:
             % urllib.parse.urlencode(params)
         )
 
-        template_vars = {"link": link}
+        template_vars: TemplateVars = {"link": link}
 
         await self.send_email(
             email_address,
@@ -196,7 +204,7 @@ class Mailer:
             % urllib.parse.urlencode(params)
         )
 
-        template_vars = {"link": link}
+        template_vars: TemplateVars = {"link": link}
 
         await self.send_email(
             email_address,
@@ -210,8 +218,8 @@ class Mailer:
         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
@@ -230,7 +238,7 @@ class Mailer:
             [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)
 
@@ -258,7 +266,7 @@ class Mailer:
         # 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(
@@ -289,7 +297,7 @@ class Mailer:
                 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
@@ -302,10 +310,10 @@ class Mailer:
         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,
         }
@@ -327,10 +335,10 @@ class Mailer:
         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.
 
@@ -356,7 +364,7 @@ class Mailer:
 
         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": [],
@@ -417,11 +425,11 @@ class Mailer:
 
     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.
 
@@ -442,7 +450,7 @@ class Mailer:
             after_limit=CONTEXT_AFTER,
         )
 
-        ret = {
+        ret: NotifVars = {
             "link": self._make_notif_link(notif),
             "ts": notif["received_ts"],
             "messages": [],
@@ -461,8 +469,8 @@ class Mailer:
         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.
 
@@ -494,7 +502,9 @@ class Mailer:
 
         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
@@ -504,7 +514,7 @@ class Mailer:
         # 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,
@@ -519,6 +529,8 @@ class Mailer:
             return ret
 
         msgtype = event.content.get("msgtype")
+        if not isinstance(msgtype, str):
+            msgtype = None
 
         ret["msgtype"] = msgtype
 
@@ -533,7 +545,7 @@ class Mailer:
         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.
@@ -543,8 +555,8 @@ class Mailer:
             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")
@@ -555,7 +567,7 @@ class Mailer:
             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.
@@ -570,7 +582,7 @@ class Mailer:
     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,
@@ -685,10 +697,10 @@ class Mailer:
 
     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.
@@ -718,7 +730,7 @@ class Mailer:
     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:
@@ -805,7 +817,7 @@ class Mailer:
             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.
 
diff --git a/synapse/push/push_types.py b/synapse/push/push_types.py
new file mode 100644
index 0000000000..8d16ab62ce
--- /dev/null
+++ b/synapse/push/push_types.py
@@ -0,0 +1,136 @@
+# Copyright 2021 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from typing import List, Optional
+
+from typing_extensions import TypedDict
+
+
+class EmailReason(TypedDict, total=False):
+    """
+    Information on the event that triggered the email to be sent
+
+    room_id: the ID of the room the event was sent in
+    now: timestamp in ms when the email is being sent out
+    room_name: a human-readable name for the room the event was sent in
+    received_at: the time in milliseconds at which the event was received
+    delay_before_mail_ms: the amount of time in milliseconds Synapse always waits
+            before ever emailing about a notification (to give the user a chance to respond
+            to other push or notice the window)
+    last_sent_ts: the time in milliseconds at which a notification was last sent
+            for an event in this room
+    throttle_ms: the minimum amount of time in milliseconds between two
+            notifications can be sent for this room
+    """
+
+    room_id: str
+    now: int
+    room_name: Optional[str]
+    received_at: int
+    delay_before_mail_ms: int
+    last_sent_ts: int
+    throttle_ms: int
+
+
+class MessageVars(TypedDict, total=False):
+    """
+    Details about a specific message to include in a notification
+
+    event_type: the type of the event
+    is_historical: a boolean, which is `False` if the message is the one
+                that triggered the notification, `True` otherwise
+    id: the ID of the event
+    ts: the time in milliseconds at which the event was sent
+    sender_name: the display name for the event's sender
+    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"
+    """
+
+    event_type: str
+    is_historical: bool
+    id: str
+    ts: int
+    sender_name: str
+    sender_avatar_url: Optional[str]
+    sender_hash: int
+    msgtype: Optional[str]
+    body_text_html: str
+    body_text_plain: str
+    image_url: str
+
+
+class NotifVars(TypedDict):
+    """
+    Details about an event we are about to include in a notification
+
+    link: a `matrix.to` link to the event
+    ts: the time in milliseconds at which the event was received
+    messages: a list of messages containing one message before the event, the
+              message in the event, and one message after the event.
+    """
+
+    link: str
+    ts: Optional[int]
+    messages: List[MessageVars]
+
+
+class RoomVars(TypedDict):
+    """
+    Represents a room containing events to include in the email.
+
+    title: a human-readable name for the room
+    hash: a hash of the ID of the room
+    invite: a boolean, which is `True` if the room is an invite the user hasn't
+        accepted yet, `False` otherwise
+    notifs: a list of events, or an empty list if `invite` is `True`.
+    link: a `matrix.to` link to the room
+    avator_url: url to the room's avator
+    """
+
+    title: Optional[str]
+    hash: int
+    invite: bool
+    notifs: List[NotifVars]
+    link: str
+    avatar_url: Optional[str]
+
+
+class TemplateVars(TypedDict, total=False):
+    """
+    Generic structure for passing to the email sender, can hold all the fields used in email templates.
+
+    app_name: name of the app/service this homeserver is associated with
+    server_name: name of our own homeserver
+    link: a link to include into the email to be sent
+    user_display_name: the display name for the user receiving the notification
+    unsubscribe_link: the link users can click to unsubscribe from email notifications
+    summary_text: a summary of the notification(s). The text used can be customised
+              by configuring the various settings in the `email.subjects` section of the
+              configuration file.
+    rooms: a list of rooms containing events to include in the email
+    reason: information on the event that triggered the email to be sent
+    """
+
+    app_name: str
+    server_name: str
+    link: str
+    user_display_name: str
+    unsubscribe_link: str
+    summary_text: str
+    rooms: List[RoomVars]
+    reason: EmailReason
diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py
index d957e770dc..3efdd0c920 100644
--- a/synapse/storage/databases/main/event_push_actions.py
+++ b/synapse/storage/databases/main/event_push_actions.py
@@ -16,6 +16,7 @@ import logging
 from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Union
 
 import attr
+from typing_extensions import TypedDict
 
 from synapse.metrics.background_process_metrics import wrap_as_background_process
 from synapse.storage._base import SQLBaseStore, db_to_json
@@ -37,6 +38,20 @@ DEFAULT_HIGHLIGHT_ACTION = [
 ]
 
 
+class BasePushAction(TypedDict):
+    event_id: str
+    actions: List[Union[dict, str]]
+
+
+class HttpPushAction(BasePushAction):
+    room_id: str
+    stream_ordering: int
+
+
+class EmailPushAction(HttpPushAction):
+    received_ts: Optional[int]
+
+
 def _serialize_action(actions, is_highlight):
     """Custom serializer for actions. This allows us to "compress" common actions.
 
@@ -221,7 +236,7 @@ class EventPushActionsWorkerStore(SQLBaseStore):
         min_stream_ordering: int,
         max_stream_ordering: int,
         limit: int = 20,
-    ) -> List[dict]:
+    ) -> List[HttpPushAction]:
         """Get a list of the most recent unread push actions for a given user,
         within the given stream ordering range. Called by the httppusher.
 
@@ -326,7 +341,7 @@ class EventPushActionsWorkerStore(SQLBaseStore):
         min_stream_ordering: int,
         max_stream_ordering: int,
         limit: int = 20,
-    ) -> List[dict]:
+    ) -> List[EmailPushAction]:
         """Get a list of the most recent unread push actions for a given user,
         within the given stream ordering range. Called by the emailpusher