From f181e541208f866ab9afdf21fe83b67c7af47993 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 22 Sep 2021 12:04:10 -0400 Subject: [PATCH 01/19] Add types to synapse.events. --- changelog.d/10998.misc | 1 + mypy.ini | 8 +- synapse/events/__init__.py | 130 ++++++++++++++-------------- synapse/handlers/room.py | 2 +- synapse/handlers/room_batch.py | 2 +- synapse/push/push_rule_evaluator.py | 6 +- synapse/rest/client/room_batch.py | 2 +- 7 files changed, 71 insertions(+), 80 deletions(-) create mode 100644 changelog.d/10998.misc diff --git a/changelog.d/10998.misc b/changelog.d/10998.misc new file mode 100644 index 000000000000..1e337bee5453 --- /dev/null +++ b/changelog.d/10998.misc @@ -0,0 +1 @@ +Add type hints to `synapse.events`. diff --git a/mypy.ini b/mypy.ini index cb4489eb3706..a71f986d4129 100644 --- a/mypy.ini +++ b/mypy.ini @@ -21,13 +21,7 @@ files = synapse/config, synapse/crypto, synapse/event_auth.py, - synapse/events/builder.py, - synapse/events/presence_router.py, - synapse/events/snapshot.py, - synapse/events/spamcheck.py, - synapse/events/third_party_rules.py, - synapse/events/utils.py, - synapse/events/validator.py, + synapse/events, synapse/federation, synapse/groups, synapse/handlers, diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 49190459c8b0..24cf835a3014 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -16,7 +16,7 @@ import abc import os -from typing import Dict, Optional, Tuple, Type +from typing import Any, Dict, Iterable, List, Optional, Tuple, Type, Union from unpaddedbase64 import encode_base64 @@ -86,7 +86,7 @@ class DefaultDictProperty(DictProperty): __slots__ = ["default"] - def __init__(self, key, default): + def __init__(self, key: str, default: Any): super().__init__(key) self.default = default @@ -111,22 +111,25 @@ def __init__(self, internal_metadata_dict: JsonDict): # in the DAG) self.outlier = False - out_of_band_membership: bool = DictProperty("out_of_band_membership") - send_on_behalf_of: str = DictProperty("send_on_behalf_of") - recheck_redaction: bool = DictProperty("recheck_redaction") - soft_failed: bool = DictProperty("soft_failed") - proactively_send: bool = DictProperty("proactively_send") - redacted: bool = DictProperty("redacted") - txn_id: str = DictProperty("txn_id") - token_id: int = DictProperty("token_id") - historical: bool = DictProperty("historical") + # Tell mypy to ignore the types of properties defined by DictProperty until + # that is properly annotated. + + out_of_band_membership: bool = DictProperty("out_of_band_membership") # type: ignore[assignment] + send_on_behalf_of: str = DictProperty("send_on_behalf_of") # type: ignore[assignment] + recheck_redaction: bool = DictProperty("recheck_redaction") # type: ignore[assignment] + soft_failed: bool = DictProperty("soft_failed") # type: ignore[assignment] + proactively_send: bool = DictProperty("proactively_send") # type: ignore[assignment] + redacted: bool = DictProperty("redacted") # type: ignore[assignment] + txn_id: str = DictProperty("txn_id") # type: ignore[assignment] + token_id: int = DictProperty("token_id") # type: ignore[assignment] + historical: bool = DictProperty("historical") # type: ignore[assignment] # XXX: These are set by StreamWorkerStore._set_before_and_after. # I'm pretty sure that these are never persisted to the database, so shouldn't # be here - before: RoomStreamToken = DictProperty("before") - after: RoomStreamToken = DictProperty("after") - order: Tuple[int, int] = DictProperty("order") + before: RoomStreamToken = DictProperty("before") # type: ignore[assignment] + after: RoomStreamToken = DictProperty("after") # type: ignore[assignment] + order: Tuple[int, int] = DictProperty("order") # type: ignore[assignment] def get_dict(self) -> JsonDict: return dict(self._dict) @@ -162,9 +165,6 @@ def need_to_check_redaction(self) -> bool: If the sender of the redaction event is allowed to redact any event due to auth rules, then this will always return false. - - Returns: - bool """ return self._dict.get("recheck_redaction", False) @@ -176,32 +176,23 @@ def is_soft_failed(self) -> bool: sent to clients. 2. They should not be added to the forward extremities (and therefore not to current state). - - Returns: - bool """ return self._dict.get("soft_failed", False) - def should_proactively_send(self): + def should_proactively_send(self) -> bool: """Whether the event, if ours, should be sent to other clients and servers. This is used for sending dummy events internally. Servers and clients can still explicitly fetch the event. - - Returns: - bool """ return self._dict.get("proactively_send", True) - def is_redacted(self): + def is_redacted(self) -> bool: """Whether the event has been redacted. This is used for efficiently checking whether an event has been marked as redacted without needing to make another database call. - - Returns: - bool """ return self._dict.get("redacted", False) @@ -241,29 +232,32 @@ def __init__( self.internal_metadata = _EventInternalMetadata(internal_metadata_dict) - auth_events = DictProperty("auth_events") - depth = DictProperty("depth") - content = DictProperty("content") - hashes = DictProperty("hashes") - origin = DictProperty("origin") - origin_server_ts = DictProperty("origin_server_ts") - prev_events = DictProperty("prev_events") - redacts = DefaultDictProperty("redacts", None) - room_id = DictProperty("room_id") - sender = DictProperty("sender") - state_key = DictProperty("state_key") - type = DictProperty("type") - user_id = DictProperty("sender") + # Tell mypy to ignore the types of properties defined by DictProperty until + # that is properly annotated. + + auth_events = DictProperty("auth_events") # type: ignore[assignment] + depth: int = DictProperty("depth") # type: ignore[assignment] + content = DictProperty("content") # type: ignore[assignment] + hashes: Dict[str, str] = DictProperty("hashes") # type: ignore[assignment] + origin: str = DictProperty("origin") # type: ignore[assignment] + origin_server_ts: int = DictProperty("origin_server_ts") # type: ignore[assignment] + prev_events = DictProperty("prev_events") # type: ignore[assignment] + redacts = DefaultDictProperty("redacts", None) # type: ignore[assignment] + room_id: str = DictProperty("room_id") # type: ignore[assignment] + sender: str = DictProperty("sender") # type: ignore[assignment] + state_key = DictProperty("state_key") # type: ignore[assignment] + type: str = DictProperty("type") # type: ignore[assignment] + user_id: str = DictProperty("sender") # type: ignore[assignment] @property def event_id(self) -> str: raise NotImplementedError() @property - def membership(self): + def membership(self) -> str: return self.content["membership"] - def is_state(self): + def is_state(self) -> bool: return hasattr(self, "state_key") and self.state_key is not None def get_dict(self) -> JsonDict: @@ -272,13 +266,13 @@ def get_dict(self) -> JsonDict: return d - def get(self, key, default=None): + def get(self, key: str, default: Optional[Any] = None) -> Any: return self._dict.get(key, default) - def get_internal_metadata_dict(self): + def get_internal_metadata_dict(self) -> JsonDict: return self.internal_metadata.get_dict() - def get_pdu_json(self, time_now=None) -> JsonDict: + def get_pdu_json(self, time_now: Optional[int] = None) -> JsonDict: pdu_json = self.get_dict() if time_now is not None and "age_ts" in pdu_json["unsigned"]: @@ -305,49 +299,49 @@ def get_templated_pdu_json(self) -> JsonDict: return template_json - def __set__(self, instance, value): + def __set__(self, instance: Any, value: Any) -> None: raise AttributeError("Unrecognized attribute %s" % (instance,)) - def __getitem__(self, field): + def __getitem__(self, field: str) -> Optional[Any]: return self._dict[field] - def __contains__(self, field): + def __contains__(self, field: str) -> bool: return field in self._dict - def items(self): + def items(self) -> List[Tuple[str, Optional[Any]]]: return list(self._dict.items()) - def keys(self): + def keys(self) -> Iterable[str]: return self._dict.keys() - def prev_event_ids(self): + def prev_event_ids(self) -> List[str]: """Returns the list of prev event IDs. The order matches the order specified in the event, though there is no meaning to it. Returns: - list[str]: The list of event IDs of this event's prev_events + The list of event IDs of this event's prev_events """ return [e for e, _ in self.prev_events] - def auth_event_ids(self): + def auth_event_ids(self) -> List[str]: """Returns the list of auth event IDs. The order matches the order specified in the event, though there is no meaning to it. Returns: - list[str]: The list of event IDs of this event's auth_events + The list of event IDs of this event's auth_events """ return [e for e, _ in self.auth_events] - def freeze(self): + def freeze(self) -> None: """'Freeze' the event dict, so it cannot be modified by accident""" # this will be a no-op if the event dict is already frozen. self._dict = freeze(self._dict) - def __str__(self): + def __str__(self) -> str: return self.__repr__() - def __repr__(self): + def __repr__(self) -> str: return "<%s event_id=%r, type=%r, state_key=%r, outlier=%s>" % ( self.__class__.__name__, self.event_id, @@ -439,7 +433,7 @@ def __init__( else: frozen_dict = event_dict - self._event_id = None + self._event_id: Optional[str] = None super().__init__( frozen_dict, @@ -451,7 +445,7 @@ def __init__( ) @property - def event_id(self): + def event_id(self) -> str: # We have to import this here as otherwise we get an import loop which # is hard to break. from synapse.crypto.event_signing import compute_event_reference_hash @@ -461,21 +455,21 @@ def event_id(self): self._event_id = "$" + encode_base64(compute_event_reference_hash(self)[1]) return self._event_id - def prev_event_ids(self): + def prev_event_ids(self) -> List[str]: """Returns the list of prev event IDs. The order matches the order specified in the event, though there is no meaning to it. Returns: - list[str]: The list of event IDs of this event's prev_events + The list of event IDs of this event's prev_events """ return self.prev_events - def auth_event_ids(self): + def auth_event_ids(self) -> List[str]: """Returns the list of auth event IDs. The order matches the order specified in the event, though there is no meaning to it. Returns: - list[str]: The list of event IDs of this event's auth_events + The list of event IDs of this event's auth_events """ return self.auth_events @@ -486,7 +480,7 @@ class FrozenEventV3(FrozenEventV2): format_version = EventFormatVersions.V3 # All events of this type are V3 @property - def event_id(self): + def event_id(self) -> str: # We have to import this here as otherwise we get an import loop which # is hard to break. from synapse.crypto.event_signing import compute_event_reference_hash @@ -499,12 +493,14 @@ def event_id(self): return self._event_id -def _event_type_from_format_version(format_version: int) -> Type[EventBase]: +def _event_type_from_format_version( + format_version: int, +) -> Type[Union[FrozenEvent, FrozenEventV2, FrozenEventV3]]: """Returns the python type to use to construct an Event object for the given event format version. Args: - format_version (int): The event format version + format_version: The event format version Returns: type: A type that can be initialized as per the initializer of diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 6f39e9446f27..f9d248d8457a 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -525,7 +525,7 @@ async def clone_existing_room( ): await self.room_member_handler.update_membership( requester, - UserID.from_string(old_event["state_key"]), + UserID.from_string(old_event.state_key), new_room_id, "ban", ratelimit=False, diff --git a/synapse/handlers/room_batch.py b/synapse/handlers/room_batch.py index 2f5a3e4d193d..072328638376 100644 --- a/synapse/handlers/room_batch.py +++ b/synapse/handlers/room_batch.py @@ -355,7 +355,7 @@ async def persist_historical_events( for (event, context) in reversed(events_to_persist): await self.event_creation_handler.handle_new_client_event( await self.create_requester_for_user_id_from_app_service( - event["sender"], app_service_requester.app_service + event.sender, app_service_requester.app_service ), event=event, context=context, diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 7a8dc63976e2..e98c8238874e 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -18,7 +18,7 @@ from typing import Any, Dict, List, Optional, Pattern, Tuple, Union from synapse.events import EventBase -from synapse.types import UserID +from synapse.types import JsonDict, UserID from synapse.util import glob_to_regex, re_word_boundary from synapse.util.caches.lrucache import LruCache @@ -222,7 +222,7 @@ def _glob_matches(glob: str, value: str, word_boundary: bool = False) -> bool: def _flatten_dict( - d: Union[EventBase, dict], + d: Union[EventBase, JsonDict], prefix: Optional[List[str]] = None, result: Optional[Dict[str, str]] = None, ) -> Dict[str, str]: @@ -233,7 +233,7 @@ def _flatten_dict( for key, value in d.items(): if isinstance(value, str): result[".".join(prefix + [key])] = value.lower() - elif hasattr(value, "items"): + elif isinstance(value, dict): _flatten_dict(value, prefix=(prefix + [key]), result=result) return result diff --git a/synapse/rest/client/room_batch.py b/synapse/rest/client/room_batch.py index 99f8156ad0ec..ab9a743bba54 100644 --- a/synapse/rest/client/room_batch.py +++ b/synapse/rest/client/room_batch.py @@ -191,7 +191,7 @@ async def on_POST( depth=inherited_depth, ) - batch_id_to_connect_to = base_insertion_event["content"][ + batch_id_to_connect_to = base_insertion_event.content[ EventContentFields.MSC2716_NEXT_BATCH_ID ] From a90954eecd2e420a8821c3d53a66275ed46f614d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 8 Oct 2021 09:55:38 -0400 Subject: [PATCH 02/19] Add type for redacts. --- synapse/events/__init__.py | 2 +- synapse/handlers/message.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 24cf835a3014..fc64509c6a29 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -242,7 +242,7 @@ def __init__( origin: str = DictProperty("origin") # type: ignore[assignment] origin_server_ts: int = DictProperty("origin_server_ts") # type: ignore[assignment] prev_events = DictProperty("prev_events") # type: ignore[assignment] - redacts = DefaultDictProperty("redacts", None) # type: ignore[assignment] + redacts: Optional[str] = DefaultDictProperty("redacts", None) # type: ignore[assignment] room_id: str = DictProperty("room_id") # type: ignore[assignment] sender: str = DictProperty("sender") # type: ignore[assignment] state_key = DictProperty("state_key") # type: ignore[assignment] diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 2e024b551f99..041d101cb3b6 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1318,6 +1318,8 @@ async def persist_and_notify_client_event( # user is actually admin or not). is_admin_redaction = False if event.type == EventTypes.Redaction: + assert event.redacts is not None + original_event = await self.store.get_event( event.redacts, redact_behaviour=EventRedactBehaviour.AS_IS, @@ -1413,6 +1415,8 @@ async def persist_and_notify_client_event( ) if event.type == EventTypes.Redaction: + assert event.redacts is not None + original_event = await self.store.get_event( event.redacts, redact_behaviour=EventRedactBehaviour.AS_IS, From 29bfdc8e6531c905d35723572fc59b28003ecdae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 8 Oct 2021 10:12:29 -0400 Subject: [PATCH 03/19] Add type hint for content. --- synapse/events/__init__.py | 2 +- synapse/handlers/message.py | 10 ++++++---- synapse/push/bulk_push_rule_evaluator.py | 4 +++- synapse/push/push_rule_evaluator.py | 4 ++-- synapse/storage/databases/main/roommember.py | 8 +++++--- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index fc64509c6a29..bd2abae97d85 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -237,7 +237,7 @@ def __init__( auth_events = DictProperty("auth_events") # type: ignore[assignment] depth: int = DictProperty("depth") # type: ignore[assignment] - content = DictProperty("content") # type: ignore[assignment] + content: JsonDict = DictProperty("content") # type: ignore[assignment] hashes: Dict[str, str] = DictProperty("hashes") # type: ignore[assignment] origin: str = DictProperty("origin") # type: ignore[assignment] origin_server_ts: int = DictProperty("origin_server_ts") # type: ignore[assignment] diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 041d101cb3b6..be29dbe1591c 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1504,11 +1504,13 @@ async def persist_and_notify_client_event( next_batch_id = event.content.get( EventContentFields.MSC2716_NEXT_BATCH_ID ) - conflicting_insertion_event_id = ( - await self.store.get_insertion_event_by_batch_id( - event.room_id, next_batch_id + conflicting_insertion_event_id = None + if next_batch_id: + conflicting_insertion_event_id = ( + await self.store.get_insertion_event_by_batch_id( + event.room_id, next_batch_id + ) ) - ) if conflicting_insertion_event_id is not None: # The current insertion event that we're processing is invalid # because an insertion event already exists in the room with the diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 0622a37ae8fd..009d8e77b05b 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -232,6 +232,8 @@ async def action_for_event_by_user( # that user, as they might not be already joined. if event.type == EventTypes.Member and event.state_key == uid: display_name = event.content.get("displayname", None) + if not isinstance(display_name, str): + display_name = None if count_as_unread: # Add an element for the current user if the event needs to be marked as @@ -268,7 +270,7 @@ def _condition_checker( evaluator: PushRuleEvaluatorForEvent, conditions: List[dict], uid: str, - display_name: str, + display_name: Optional[str], cache: Dict[str, bool], ) -> bool: for cond in conditions: diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index e98c8238874e..7f68092ec5e5 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -129,7 +129,7 @@ def __init__( self._value_cache = _flatten_dict(event) def matches( - self, condition: Dict[str, Any], user_id: str, display_name: str + self, condition: Dict[str, Any], user_id: str, display_name: Optional[str] ) -> bool: if condition["kind"] == "event_match": return self._event_match(condition, user_id) @@ -172,7 +172,7 @@ def _event_match(self, condition: dict, user_id: str) -> bool: return _glob_matches(pattern, haystack) - def _contains_display_name(self, display_name: str) -> bool: + def _contains_display_name(self, display_name: Optional[str]) -> bool: if not display_name: return False diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index ddb162a4fca1..8208e66fe697 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -569,7 +569,7 @@ async def get_users_who_share_room_with_user( async def get_joined_users_from_context( self, event: EventBase, context: EventContext - ): + ) -> Dict[str, ProfileInfo]: state_group = context.state_group if not state_group: # If state_group is None it means it has yet to be assigned a @@ -583,7 +583,9 @@ async def get_joined_users_from_context( event.room_id, state_group, current_state_ids, event=event, context=context ) - async def get_joined_users_from_state(self, room_id, state_entry): + async def get_joined_users_from_state( + self, room_id, state_entry + ) -> Dict[str, ProfileInfo]: state_group = state_entry.state_group if not state_group: # If state_group is None it means it has yet to be assigned a @@ -606,7 +608,7 @@ async def _get_joined_users_from_context( cache_context, event=None, context=None, - ): + ) -> Dict[str, ProfileInfo]: # We don't use `state_group`, it's there so that we can cache based # on it. However, it's important that it's never None, since two current_states # with a state_group of None are likely to be different. From 90251d610133924705a25f058b06868f8f20db53 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 8 Oct 2021 10:20:09 -0400 Subject: [PATCH 04/19] Remove DictProperties which vary based on sub-class. --- synapse/events/__init__.py | 10 ++++------ synapse/storage/databases/main/events.py | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index bd2abae97d85..dafab8d2a835 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -235,13 +235,11 @@ def __init__( # Tell mypy to ignore the types of properties defined by DictProperty until # that is properly annotated. - auth_events = DictProperty("auth_events") # type: ignore[assignment] depth: int = DictProperty("depth") # type: ignore[assignment] content: JsonDict = DictProperty("content") # type: ignore[assignment] hashes: Dict[str, str] = DictProperty("hashes") # type: ignore[assignment] origin: str = DictProperty("origin") # type: ignore[assignment] origin_server_ts: int = DictProperty("origin_server_ts") # type: ignore[assignment] - prev_events = DictProperty("prev_events") # type: ignore[assignment] redacts: Optional[str] = DefaultDictProperty("redacts", None) # type: ignore[assignment] room_id: str = DictProperty("room_id") # type: ignore[assignment] sender: str = DictProperty("sender") # type: ignore[assignment] @@ -321,7 +319,7 @@ def prev_event_ids(self) -> List[str]: Returns: The list of event IDs of this event's prev_events """ - return [e for e, _ in self.prev_events] + return [e for e, _ in self._dict["prev_events"]] def auth_event_ids(self) -> List[str]: """Returns the list of auth event IDs. The order matches the order @@ -330,7 +328,7 @@ def auth_event_ids(self) -> List[str]: Returns: The list of event IDs of this event's auth_events """ - return [e for e, _ in self.auth_events] + return [e for e, _ in self._dict["auth_events"]] def freeze(self) -> None: """'Freeze' the event dict, so it cannot be modified by accident""" @@ -462,7 +460,7 @@ def prev_event_ids(self) -> List[str]: Returns: The list of event IDs of this event's prev_events """ - return self.prev_events + return self._dict["prev_events"] def auth_event_ids(self) -> List[str]: """Returns the list of auth event IDs. The order matches the order @@ -471,7 +469,7 @@ def auth_event_ids(self) -> List[str]: Returns: The list of event IDs of this event's auth_events """ - return self.auth_events + return self._dict["auth_events"] class FrozenEventV3(FrozenEventV2): diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 37439f85628e..e48d4d47f842 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1790,7 +1790,7 @@ def _handle_insertion_event(self, txn: LoggingTransaction, event: EventBase): ) # Insert an edge for every prev_event connection - for prev_event_id in event.prev_events: + for prev_event_id in event.prev_event_ids(): self.db_pool.simple_insert_txn( txn, table="insertion_event_edges", From 93c91b1ef9a363ee0e529622f8fd004dc9f1a313 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 Oct 2021 14:54:13 -0400 Subject: [PATCH 05/19] Add remaining types for DictProperty & DefaultDictProperty. --- synapse/events/__init__.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index dafab8d2a835..e782e113d20b 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -45,7 +45,11 @@ class DictProperty: def __init__(self, key: str): self.key = key - def __get__(self, instance, owner=None): + def __get__( + self, + instance: Optional[Union["_EventInternalMetadata", "EventBase"]], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> Any: # if the property is accessed as a class property rather than an instance # property, return the property itself rather than the value if instance is None: @@ -65,10 +69,12 @@ def __get__(self, instance, owner=None): "'%s' has no '%s' property" % (type(instance), self.key) ) from e1.__context__ - def __set__(self, instance, v): + def __set__( + self, instance: Union["_EventInternalMetadata", "EventBase"], v: Any + ) -> None: instance._dict[self.key] = v - def __delete__(self, instance): + def __delete__(self, instance: Any) -> None: try: del instance._dict[self.key] except KeyError as e1: @@ -90,7 +96,11 @@ def __init__(self, key: str, default: Any): super().__init__(key) self.default = default - def __get__(self, instance, owner=None): + def __get__( + self, + instance: Optional[Union["_EventInternalMetadata", "EventBase"]], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> Any: if instance is None: return self return instance._dict.get(self.key, self.default) @@ -243,7 +253,7 @@ def __init__( redacts: Optional[str] = DefaultDictProperty("redacts", None) # type: ignore[assignment] room_id: str = DictProperty("room_id") # type: ignore[assignment] sender: str = DictProperty("sender") # type: ignore[assignment] - state_key = DictProperty("state_key") # type: ignore[assignment] + state_key: str = DictProperty("state_key") # type: ignore[assignment] type: str = DictProperty("type") # type: ignore[assignment] user_id: str = DictProperty("sender") # type: ignore[assignment] From 0a016914e15ef46c8cfd518f764a2b9a1b02bcc9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 Oct 2021 15:42:33 -0400 Subject: [PATCH 06/19] Newsfragment --- changelog.d/{10998.misc => 11098.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10998.misc => 11098.misc} (100%) diff --git a/changelog.d/10998.misc b/changelog.d/11098.misc similarity index 100% rename from changelog.d/10998.misc rename to changelog.d/11098.misc From 029953a16ea49157b6b59185fc11b80d9d434d1d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 Oct 2021 14:32:12 -0400 Subject: [PATCH 07/19] Proper types for __delete__. --- synapse/events/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index e782e113d20b..be37a1aac289 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -74,7 +74,9 @@ def __set__( ) -> None: instance._dict[self.key] = v - def __delete__(self, instance: Any) -> None: + def __delete__( + self, instance: Union["_EventInternalMetadata", "EventBase"] + ) -> None: try: del instance._dict[self.key] except KeyError as e1: From c980b856f1ebc4a4c33468c5e6c9a98582bb7c0d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 Oct 2021 14:56:08 -0400 Subject: [PATCH 08/19] Use Generics for DictProperty descriptors. --- synapse/events/__init__.py | 114 +++++++++++++++++++++++++------------ 1 file changed, 78 insertions(+), 36 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index be37a1aac289..4d749a2c630e 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -16,7 +16,20 @@ import abc import os -from typing import Any, Dict, Iterable, List, Optional, Tuple, Type, Union +from typing import ( + Any, + Dict, + Generic, + Iterable, + List, + Literal, + Optional, + Tuple, + Type, + TypeVar, + Union, + overload, +) from unpaddedbase64 import encode_base64 @@ -37,7 +50,10 @@ USE_FROZEN_DICTS = strtobool(os.environ.get("SYNAPSE_USE_FROZEN_DICTS", "0")) -class DictProperty: +T = TypeVar("T") + + +class DictProperty(Generic[T]): """An object property which delegates to the `_dict` within its parent object.""" __slots__ = ["key"] @@ -45,11 +61,27 @@ class DictProperty: def __init__(self, key: str): self.key = key + @overload + def __get__( + self, + instance: Literal[None], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> "DictProperty": + ... + + @overload + def __get__( + self, + instance: Union["_EventInternalMetadata", "EventBase"], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> T: + ... + def __get__( self, instance: Optional[Union["_EventInternalMetadata", "EventBase"]], owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, - ) -> Any: + ) -> Union[T, "DictProperty"]: # if the property is accessed as a class property rather than an instance # property, return the property itself rather than the value if instance is None: @@ -70,7 +102,7 @@ def __get__( ) from e1.__context__ def __set__( - self, instance: Union["_EventInternalMetadata", "EventBase"], v: Any + self, instance: Union["_EventInternalMetadata", "EventBase"], v: T ) -> None: instance._dict[self.key] = v @@ -85,7 +117,7 @@ def __delete__( ) from e1.__context__ -class DefaultDictProperty(DictProperty): +class DefaultDictProperty(DictProperty, Generic[T]): """An extension of DictProperty which provides a default if the property is not present in the parent's _dict. @@ -94,15 +126,31 @@ class DefaultDictProperty(DictProperty): __slots__ = ["default"] - def __init__(self, key: str, default: Any): + def __init__(self, key: str, default: T): super().__init__(key) self.default = default + @overload + def __get__( + self, + instance: Literal[None], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> "DefaultDictProperty": + ... + + @overload + def __get__( + self, + instance: Union["_EventInternalMetadata", "EventBase"], + owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + ) -> T: + ... + def __get__( self, instance: Optional[Union["_EventInternalMetadata", "EventBase"]], owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, - ) -> Any: + ) -> Union[T, "DefaultDictProperty"]: if instance is None: return self return instance._dict.get(self.key, self.default) @@ -123,25 +171,22 @@ def __init__(self, internal_metadata_dict: JsonDict): # in the DAG) self.outlier = False - # Tell mypy to ignore the types of properties defined by DictProperty until - # that is properly annotated. - - out_of_band_membership: bool = DictProperty("out_of_band_membership") # type: ignore[assignment] - send_on_behalf_of: str = DictProperty("send_on_behalf_of") # type: ignore[assignment] - recheck_redaction: bool = DictProperty("recheck_redaction") # type: ignore[assignment] - soft_failed: bool = DictProperty("soft_failed") # type: ignore[assignment] - proactively_send: bool = DictProperty("proactively_send") # type: ignore[assignment] - redacted: bool = DictProperty("redacted") # type: ignore[assignment] - txn_id: str = DictProperty("txn_id") # type: ignore[assignment] - token_id: int = DictProperty("token_id") # type: ignore[assignment] - historical: bool = DictProperty("historical") # type: ignore[assignment] + out_of_band_membership: DictProperty[bool] = DictProperty("out_of_band_membership") + send_on_behalf_of: DictProperty[str] = DictProperty("send_on_behalf_of") + recheck_redaction: DictProperty[bool] = DictProperty("recheck_redaction") + soft_failed: DictProperty[bool] = DictProperty("soft_failed") + proactively_send: DictProperty[bool] = DictProperty("proactively_send") + redacted: DictProperty[bool] = DictProperty("redacted") + txn_id: DictProperty[str] = DictProperty("txn_id") + token_id: DictProperty[int] = DictProperty("token_id") + historical: DictProperty[bool] = DictProperty("historical") # XXX: These are set by StreamWorkerStore._set_before_and_after. # I'm pretty sure that these are never persisted to the database, so shouldn't # be here - before: RoomStreamToken = DictProperty("before") # type: ignore[assignment] - after: RoomStreamToken = DictProperty("after") # type: ignore[assignment] - order: Tuple[int, int] = DictProperty("order") # type: ignore[assignment] + before: DictProperty[RoomStreamToken] = DictProperty("before") + after: DictProperty[RoomStreamToken] = DictProperty("after") + order: DictProperty[Tuple[int, int]] = DictProperty("order") def get_dict(self) -> JsonDict: return dict(self._dict) @@ -244,20 +289,17 @@ def __init__( self.internal_metadata = _EventInternalMetadata(internal_metadata_dict) - # Tell mypy to ignore the types of properties defined by DictProperty until - # that is properly annotated. - - depth: int = DictProperty("depth") # type: ignore[assignment] - content: JsonDict = DictProperty("content") # type: ignore[assignment] - hashes: Dict[str, str] = DictProperty("hashes") # type: ignore[assignment] - origin: str = DictProperty("origin") # type: ignore[assignment] - origin_server_ts: int = DictProperty("origin_server_ts") # type: ignore[assignment] - redacts: Optional[str] = DefaultDictProperty("redacts", None) # type: ignore[assignment] - room_id: str = DictProperty("room_id") # type: ignore[assignment] - sender: str = DictProperty("sender") # type: ignore[assignment] - state_key: str = DictProperty("state_key") # type: ignore[assignment] - type: str = DictProperty("type") # type: ignore[assignment] - user_id: str = DictProperty("sender") # type: ignore[assignment] + depth: DictProperty[int] = DictProperty("depth") + content: DictProperty[JsonDict] = DictProperty("content") + hashes: DictProperty[Dict[str, str]] = DictProperty("hashes") + origin: DictProperty[str] = DictProperty("origin") + origin_server_ts: DictProperty[int] = DictProperty("origin_server_ts") + redacts: DefaultDictProperty[Optional[str]] = DefaultDictProperty("redacts", None) + room_id: DictProperty[str] = DictProperty("room_id") + sender: DictProperty[str] = DictProperty("sender") + state_key: DictProperty[str] = DictProperty("state_key") + type: DictProperty[str] = DictProperty("type") + user_id: DictProperty[str] = DictProperty("sender") @property def event_id(self) -> str: From baa90e18e4e2b794a195324ce5de47faeec8bcae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 Oct 2021 15:15:19 -0400 Subject: [PATCH 09/19] Abstract the valid types to a separate variable. --- synapse/events/__init__.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 4d749a2c630e..ccddb9a106e9 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -53,6 +53,11 @@ T = TypeVar("T") +# DictProperty (and DefaultDictPropery) require the classes they're used with to +# have a _dict property to pull properties from. +_DictPropertyInstance = Union["_EventInternalMetadata", "EventBase"] + + class DictProperty(Generic[T]): """An object property which delegates to the `_dict` within its parent object.""" @@ -65,22 +70,22 @@ def __init__(self, key: str): def __get__( self, instance: Literal[None], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> "DictProperty": ... @overload def __get__( self, - instance: Union["_EventInternalMetadata", "EventBase"], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + instance: _DictPropertyInstance, + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> T: ... def __get__( self, - instance: Optional[Union["_EventInternalMetadata", "EventBase"]], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + instance: Optional[_DictPropertyInstance], + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> Union[T, "DictProperty"]: # if the property is accessed as a class property rather than an instance # property, return the property itself rather than the value @@ -101,14 +106,10 @@ def __get__( "'%s' has no '%s' property" % (type(instance), self.key) ) from e1.__context__ - def __set__( - self, instance: Union["_EventInternalMetadata", "EventBase"], v: T - ) -> None: + def __set__(self, instance: _DictPropertyInstance, v: T) -> None: instance._dict[self.key] = v - def __delete__( - self, instance: Union["_EventInternalMetadata", "EventBase"] - ) -> None: + def __delete__(self, instance: _DictPropertyInstance) -> None: try: del instance._dict[self.key] except KeyError as e1: @@ -134,22 +135,22 @@ def __init__(self, key: str, default: T): def __get__( self, instance: Literal[None], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> "DefaultDictProperty": ... @overload def __get__( self, - instance: Union["_EventInternalMetadata", "EventBase"], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + instance: _DictPropertyInstance, + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> T: ... def __get__( self, - instance: Optional[Union["_EventInternalMetadata", "EventBase"]], - owner: Optional[Type[Union["_EventInternalMetadata", "EventBase"]]] = None, + instance: Optional[_DictPropertyInstance], + owner: Optional[Type[_DictPropertyInstance]] = None, ) -> Union[T, "DefaultDictProperty"]: if instance is None: return self From 4ac46965de3a5b78a01cf7564cd3556ac0d1e29c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 Oct 2021 15:38:07 -0400 Subject: [PATCH 10/19] Use __setattr__ instead of __set__. --- synapse/events/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index ccddb9a106e9..5ad806487613 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -352,8 +352,8 @@ def get_templated_pdu_json(self) -> JsonDict: return template_json - def __set__(self, instance: Any, value: Any) -> None: - raise AttributeError("Unrecognized attribute %s" % (instance,)) + def __setattr__(self, key: str, value: Any) -> None: + raise AttributeError("Unrecognized attribute %s" % (key,)) def __getitem__(self, field: str) -> Optional[Any]: return self._dict[field] From 67ee1fdef988a1dcce4aa66905706b0a0eb817e7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 21 Oct 2021 15:29:02 -0400 Subject: [PATCH 11/19] Fix type errors from union of EventBase/EventBuilder. --- synapse/events/__init__.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 5ad806487613..eaa029909ae9 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -17,6 +17,7 @@ import abc import os from typing import ( + TYPE_CHECKING, Any, Dict, Generic, @@ -39,6 +40,9 @@ from synapse.util.frozenutils import freeze from synapse.util.stringutils import strtobool +if TYPE_CHECKING: + from synapse.events.builder import EventBuilder + # Whether we should use frozen_dict in FrozenEvent. Using frozen_dicts prevents # bugs where we accidentally share e.g. signature dicts. However, converting a # dict to frozen_dicts is expensive. @@ -55,7 +59,15 @@ # DictProperty (and DefaultDictPropery) require the classes they're used with to # have a _dict property to pull properties from. -_DictPropertyInstance = Union["_EventInternalMetadata", "EventBase"] +# +# TODO _DictPropertyInstance should not include EventBuilder but due to +# https://github.com/python/mypy/issues/5570 it thinks the DictProperty and +# DefaultDictProperty get applied to EventBuilder when it is in a Union with +# EventBase. This is the least invasive hack to get mypy to comply. +# +# Note that DictProperty/DefaultDictProperty cannot actually be used with +# EventBuilder as it lacks a _dict property. +_DictPropertyInstance = Union["_EventInternalMetadata", "EventBase", "EventBuilder"] class DictProperty(Generic[T]): @@ -92,6 +104,7 @@ def __get__( if instance is None: return self try: + assert isinstance(instance, (EventBase, _EventInternalMetadata)) return instance._dict[self.key] except KeyError as e1: # We want this to look like a regular attribute error (mostly so that @@ -107,9 +120,11 @@ def __get__( ) from e1.__context__ def __set__(self, instance: _DictPropertyInstance, v: T) -> None: + assert isinstance(instance, (EventBase, _EventInternalMetadata)) instance._dict[self.key] = v def __delete__(self, instance: _DictPropertyInstance) -> None: + assert isinstance(instance, (EventBase, _EventInternalMetadata)) try: del instance._dict[self.key] except KeyError as e1: @@ -154,6 +169,7 @@ def __get__( ) -> Union[T, "DefaultDictProperty"]: if instance is None: return self + assert isinstance(instance, (EventBase, _EventInternalMetadata)) return instance._dict.get(self.key, self.default) From e40eff82e7c4b7bb4a595faaeff6171e8f4f5e3e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 Oct 2021 09:25:26 -0400 Subject: [PATCH 12/19] Review comment: after freeze we return tuples, not lists. --- synapse/events/__init__.py | 9 +++++---- synapse/handlers/room_member.py | 2 +- synapse/state/__init__.py | 2 +- synapse/storage/databases/main/events.py | 5 +++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index eaa029909ae9..ba99814bf83d 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -25,6 +25,7 @@ List, Literal, Optional, + Sequence, Tuple, Type, TypeVar, @@ -383,7 +384,7 @@ def items(self) -> List[Tuple[str, Optional[Any]]]: def keys(self) -> Iterable[str]: return self._dict.keys() - def prev_event_ids(self) -> List[str]: + def prev_event_ids(self) -> Sequence[str]: """Returns the list of prev event IDs. The order matches the order specified in the event, though there is no meaning to it. @@ -392,7 +393,7 @@ def prev_event_ids(self) -> List[str]: """ return [e for e, _ in self._dict["prev_events"]] - def auth_event_ids(self) -> List[str]: + def auth_event_ids(self) -> Sequence[str]: """Returns the list of auth event IDs. The order matches the order specified in the event, though there is no meaning to it. @@ -524,7 +525,7 @@ def event_id(self) -> str: self._event_id = "$" + encode_base64(compute_event_reference_hash(self)[1]) return self._event_id - def prev_event_ids(self) -> List[str]: + def prev_event_ids(self) -> Sequence[str]: """Returns the list of prev event IDs. The order matches the order specified in the event, though there is no meaning to it. @@ -533,7 +534,7 @@ def prev_event_ids(self) -> List[str]: """ return self._dict["prev_events"] - def auth_event_ids(self) -> List[str]: + def auth_event_ids(self) -> Sequence[str]: """Returns the list of auth event IDs. The order matches the order specified in the event, though there is no meaning to it. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 74e6c7eca6b1..1e8e759a9965 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1669,7 +1669,7 @@ async def _generate_local_out_of_band_leave( # # the prev_events consist solely of the previous membership event. prev_event_ids = [previous_membership_event.event_id] - auth_event_ids = previous_membership_event.auth_event_ids() + prev_event_ids + auth_event_ids = list(previous_membership_event.auth_event_ids()) + prev_event_ids event, context = await self.event_creation_handler.create_event( requester, diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 5cf2e1257587..ec1bba8d3089 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -246,7 +246,7 @@ async def get_current_hosts_in_room(self, room_id: str) -> Set[str]: return await self.get_hosts_in_room_at_events(room_id, event_ids) async def get_hosts_in_room_at_events( - self, room_id: str, event_ids: List[str] + self, room_id: str, event_ids: Iterable[str] ) -> Set[str]: """Get the hosts that were in a room at the given event ids diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index e48d4d47f842..5a56d2fe85b5 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -24,6 +24,7 @@ Iterable, List, Optional, + Sequence, Set, Tuple, ) @@ -494,7 +495,7 @@ def _add_chain_cover_index( event_chain_id_gen: SequenceGenerator, event_to_room_id: Dict[str, str], event_to_types: Dict[str, Tuple[str, str]], - event_to_auth_chain: Dict[str, List[str]], + event_to_auth_chain: Dict[str, Sequence[str]], ) -> None: """Calculate the chain cover index for the given events. @@ -786,7 +787,7 @@ def _allocate_chain_ids( event_chain_id_gen: SequenceGenerator, event_to_room_id: Dict[str, str], event_to_types: Dict[str, Tuple[str, str]], - event_to_auth_chain: Dict[str, List[str]], + event_to_auth_chain: Dict[str, Sequence[str]], events_to_calc_chain_id_for: Set[str], chain_map: Dict[str, Tuple[int, int]], ) -> Dict[str, Tuple[int, int]]: From 0d0b707fe2c696558c45096af51998e16b319f6f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 27 Oct 2021 13:52:45 -0400 Subject: [PATCH 13/19] Lint. --- synapse/handlers/room_member.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 1e8e759a9965..08244b690d4c 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1669,7 +1669,9 @@ async def _generate_local_out_of_band_leave( # # the prev_events consist solely of the previous membership event. prev_event_ids = [previous_membership_event.event_id] - auth_event_ids = list(previous_membership_event.auth_event_ids()) + prev_event_ids + auth_event_ids = ( + list(previous_membership_event.auth_event_ids()) + prev_event_ids + ) event, context = await self.event_creation_handler.create_event( requester, From 39696d733cbb1fe213409bef51ef24ed63f0cee5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 07:58:27 -0400 Subject: [PATCH 14/19] Fix typo. Co-authored-by: David Robertson --- synapse/events/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 6ac26ca0ce0c..c13aa59ce536 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -58,7 +58,7 @@ T = TypeVar("T") -# DictProperty (and DefaultDictPropery) require the classes they're used with to +# DictProperty (and DefaultDictProperty) require the classes they're used with to # have a _dict property to pull properties from. # # TODO _DictPropertyInstance should not include EventBuilder but due to From f58609e1cec02fbac844a8e93cdd99480a3d137f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 08:00:17 -0400 Subject: [PATCH 15/19] Literal from typing extensions. --- synapse/events/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index c13aa59ce536..55d941e0861d 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -23,7 +23,6 @@ Generic, Iterable, List, - Literal, Optional, Sequence, Tuple, @@ -32,6 +31,7 @@ Union, overload, ) +from typing_extensions import Literal from unpaddedbase64 import encode_base64 From d3dd88fbbc6288178d0896421dbe4b7180caf617 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 08:01:38 -0400 Subject: [PATCH 16/19] Add note about state_key. --- synapse/events/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 55d941e0861d..c0de2808d944 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -315,6 +315,10 @@ def __init__( redacts: DefaultDictProperty[Optional[str]] = DefaultDictProperty("redacts", None) room_id: DictProperty[str] = DictProperty("room_id") sender: DictProperty[str] = DictProperty("sender") + # TODO state_key should be Optional[str], this is generally asserted in Synapse + # by calling is_state() first (which ensures this), but it is hard (not possible?) + # to properly annotate that calling is_state() asserts that state_key exists + # and is non-None. state_key: DictProperty[str] = DictProperty("state_key") type: DictProperty[str] = DictProperty("type") user_id: DictProperty[str] = DictProperty("sender") From 9ad06592a0047fd2de86806d8e171fec7e5947f5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 15:16:37 -0400 Subject: [PATCH 17/19] Properly check for fields during validation. --- synapse/events/validator.py | 2 +- synapse/handlers/federation_event.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 4d459c17f162..cf8693496846 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -55,7 +55,7 @@ def validate_new(self, event: EventBase, config: HomeServerConfig) -> None: ] for k in required: - if not hasattr(event, k): + if k not in event: raise SynapseError(400, "Event does not have key %s" % (k,)) # Check that the following keys have string values diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index e617db4c0ded..1a1cd93b1ae5 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1643,7 +1643,7 @@ async def _load_or_fetch_auth_events_for_event( event: the event whose auth_events we want Returns: - all of the events in `event.auth_events`, after deduplication + all of the events listed in `event.auth_events_ids`, after deduplication Raises: AuthError if we were unable to fetch the auth_events for any reason. From 2fdcc684302ac2ad8bd524f5b606907acafb621f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 15:25:55 -0400 Subject: [PATCH 18/19] Remove broken __setattr__. --- synapse/events/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index c0de2808d944..f7921b3eff63 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -373,9 +373,6 @@ def get_templated_pdu_json(self) -> JsonDict: return template_json - def __setattr__(self, key: str, value: Any) -> None: - raise AttributeError("Unrecognized attribute %s" % (key,)) - def __getitem__(self, field: str) -> Optional[Any]: return self._dict[field] From ac760c767256ff4eb60d9bad409e484f4bd72e34 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 Oct 2021 15:29:53 -0400 Subject: [PATCH 19/19] Lint --- synapse/events/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index f7921b3eff63..38f3cf4d330d 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -31,8 +31,8 @@ Union, overload, ) -from typing_extensions import Literal +from typing_extensions import Literal from unpaddedbase64 import encode_base64 from synapse.api.room_versions import EventFormatVersions, RoomVersion, RoomVersions