From 48f4e95d18fd308bb9b47219fb2786b010fa22d2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:47:07 -0500 Subject: [PATCH 1/7] Rename some variables. --- synapse/events/utils.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 6fa631aa1d4d..7e16e867c51f 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -425,12 +425,12 @@ async def serialize_event( ) if annotations.chunk: - r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.ANNOTATION] = annotations.to_dict() + relations = serialized_event["unsigned"].setdefault("m.relations", {}) + relations[RelationTypes.ANNOTATION] = annotations.to_dict() if references.chunk: - r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.REFERENCE] = references.to_dict() + relations = serialized_event["unsigned"].setdefault("m.relations", {}) + relations[RelationTypes.REFERENCE] = references.to_dict() edit = None if event.type == EventTypes.Message: @@ -449,15 +449,15 @@ async def serialize_event( serialized_event["content"] = edit_content.get("m.new_content", {}) # Check for existing relations - relations = event.content.get("m.relates_to") - if relations: + relates_to = event.content.get("m.relates_to") + if relates_to: # Keep the relations, ensuring we use a dict copy of the original - serialized_event["content"]["m.relates_to"] = relations.copy() + serialized_event["content"]["m.relates_to"] = relates_to.copy() else: serialized_event["content"].pop("m.relates_to", None) - r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.REPLACE] = { + relations = serialized_event["unsigned"].setdefault("m.relations", {}) + relations[RelationTypes.REPLACE] = { "event_id": edit.event_id, "origin_server_ts": edit.origin_server_ts, "sender": edit.sender, @@ -470,8 +470,10 @@ async def serialize_event( latest_thread_event, ) = await self.store.get_thread_summary(event_id) if latest_thread_event: - r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.THREAD] = { + relations = serialized_event["unsigned"].setdefault( + "m.relations", {} + ) + relations[RelationTypes.THREAD] = { # Don't bundle aggregations as this could recurse forever. "latest_event": await self.serialize_event( latest_thread_event, time_now, bundle_aggregations=False From 43c4f52acdc84d094686283d49ea055bb751b044 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:49:44 -0500 Subject: [PATCH 2/7] Minor refactoring to reduce code duplication. --- synapse/events/utils.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 7e16e867c51f..e484d1aad2a7 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -424,12 +424,12 @@ async def serialize_event( event_id, RelationTypes.REFERENCE, direction="f" ) + relations = {} + if annotations.chunk: - relations = serialized_event["unsigned"].setdefault("m.relations", {}) relations[RelationTypes.ANNOTATION] = annotations.to_dict() if references.chunk: - relations = serialized_event["unsigned"].setdefault("m.relations", {}) relations[RelationTypes.REFERENCE] = references.to_dict() edit = None @@ -456,7 +456,6 @@ async def serialize_event( else: serialized_event["content"].pop("m.relates_to", None) - relations = serialized_event["unsigned"].setdefault("m.relations", {}) relations[RelationTypes.REPLACE] = { "event_id": edit.event_id, "origin_server_ts": edit.origin_server_ts, @@ -470,9 +469,6 @@ async def serialize_event( latest_thread_event, ) = await self.store.get_thread_summary(event_id) if latest_thread_event: - relations = serialized_event["unsigned"].setdefault( - "m.relations", {} - ) relations[RelationTypes.THREAD] = { # Don't bundle aggregations as this could recurse forever. "latest_event": await self.serialize_event( @@ -481,6 +477,12 @@ async def serialize_event( "count": thread_count, } + # If any bundled relations were found, include them. + if relations: + serialized_event["unsigned"].setdefault("m.relations", {}).update( + relations + ) + return serialized_event async def serialize_events( From edcc683bc3f34672c0912336aa9ddfe5238d49cc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:58:59 -0500 Subject: [PATCH 3/7] Move bundle calculation to a separate method. --- synapse/events/utils.py | 142 ++++++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 65 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index e484d1aad2a7..ddeb6bd628ac 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -398,7 +398,7 @@ async def serialize_event( """Serializes a single event. Args: - event + event: The event being serialized. time_now: The current time in milliseconds bundle_aggregations: Whether to bundle in related events **kwargs: Arguments to pass to `serialize_event` @@ -410,7 +410,6 @@ async def serialize_event( if not isinstance(event, EventBase): return event - event_id = event.event_id serialized_event = serialize_event(event, time_now, **kwargs) # If MSC1849 is enabled then we need to look if there are any relations @@ -419,72 +418,85 @@ async def serialize_event( if not event.internal_metadata.is_redacted() and ( self._msc1849_enabled and bundle_aggregations ): - annotations = await self.store.get_aggregation_groups_for_event(event_id) - references = await self.store.get_relations_for_event( - event_id, RelationTypes.REFERENCE, direction="f" - ) - - relations = {} - - if annotations.chunk: - relations[RelationTypes.ANNOTATION] = annotations.to_dict() - - if references.chunk: - relations[RelationTypes.REFERENCE] = references.to_dict() - - edit = None - if event.type == EventTypes.Message: - edit = await self.store.get_applicable_edit(event_id) - - if edit: - # If there is an edit replace the content, preserving existing - # relations. - - # Ensure we take copies of the edit content, otherwise we risk modifying - # the original event. - edit_content = edit.content.copy() - - # Unfreeze the event content if necessary, so that we may modify it below - edit_content = unfreeze(edit_content) - serialized_event["content"] = edit_content.get("m.new_content", {}) - - # Check for existing relations - relates_to = event.content.get("m.relates_to") - if relates_to: - # Keep the relations, ensuring we use a dict copy of the original - serialized_event["content"]["m.relates_to"] = relates_to.copy() - else: - serialized_event["content"].pop("m.relates_to", None) - - relations[RelationTypes.REPLACE] = { - "event_id": edit.event_id, - "origin_server_ts": edit.origin_server_ts, - "sender": edit.sender, - } - - # If this event is the start of a thread, include a summary of the replies. - if self._msc3440_enabled: - ( - thread_count, - latest_thread_event, - ) = await self.store.get_thread_summary(event_id) - if latest_thread_event: - relations[RelationTypes.THREAD] = { - # Don't bundle aggregations as this could recurse forever. - "latest_event": await self.serialize_event( - latest_thread_event, time_now, bundle_aggregations=False - ), - "count": thread_count, - } - - # If any bundled relations were found, include them. - if relations: - serialized_event["unsigned"].setdefault("m.relations", {}).update( - relations - ) + await self._injected_bundled_relations(event, time_now, serialized_event) return serialized_event + async def _injected_bundled_relations( + self, event: EventBase, time_now: int, serialized_event: JsonDict + ) -> None: + """Potentially injects bundled relations into the unsigned portion of the serialized event. + + Args: + event: The event being serialized. + time_now: The current time in milliseconds + serialized_event: The serialized event which may be modified. + + """ + event_id = event.event_id + + annotations = await self.store.get_aggregation_groups_for_event(event_id) + references = await self.store.get_relations_for_event( + event_id, RelationTypes.REFERENCE, direction="f" + ) + + relations = {} + + if annotations.chunk: + relations[RelationTypes.ANNOTATION] = annotations.to_dict() + + if references.chunk: + relations[RelationTypes.REFERENCE] = references.to_dict() + + edit = None + if event.type == EventTypes.Message: + edit = await self.store.get_applicable_edit(event_id) + + if edit: + # If there is an edit replace the content, preserving existing + # relations. + + # Ensure we take copies of the edit content, otherwise we risk modifying + # the original event. + edit_content = edit.content.copy() + + # Unfreeze the event content if necessary, so that we may modify it below + edit_content = unfreeze(edit_content) + serialized_event["content"] = edit_content.get("m.new_content", {}) + + # Check for existing relations + relates_to = event.content.get("m.relates_to") + if relates_to: + # Keep the relations, ensuring we use a dict copy of the original + serialized_event["content"]["m.relates_to"] = relates_to.copy() + else: + serialized_event["content"].pop("m.relates_to", None) + + relations[RelationTypes.REPLACE] = { + "event_id": edit.event_id, + "origin_server_ts": edit.origin_server_ts, + "sender": edit.sender, + } + + # If this event is the start of a thread, include a summary of the replies. + if self._msc3440_enabled: + ( + thread_count, + latest_thread_event, + ) = await self.store.get_thread_summary(event_id) + if latest_thread_event: + relations[RelationTypes.THREAD] = { + # Don't bundle aggregations as this could recurse forever. + "latest_event": await self.serialize_event( + latest_thread_event, time_now, bundle_aggregations=False + ), + "count": thread_count, + } + + # If any bundled relations were found, include them. + if relations: + serialized_event["unsigned"].setdefault("m.relations", {}).update(relations) + async def serialize_events( self, events: Iterable[Union[JsonDict, EventBase]], time_now: int, **kwargs: Any ) -> List[JsonDict]: From 833533b67ec58a1bfc4e25238b71ac3ddb549dcb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 12:40:17 -0500 Subject: [PATCH 4/7] Move definitions closer to use. --- synapse/events/utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index ddeb6bd628ac..e52a1e5a4e75 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -435,16 +435,16 @@ async def _injected_bundled_relations( """ event_id = event.event_id - annotations = await self.store.get_aggregation_groups_for_event(event_id) - references = await self.store.get_relations_for_event( - event_id, RelationTypes.REFERENCE, direction="f" - ) - + # The bundled relations to include. relations = {} + annotations = await self.store.get_aggregation_groups_for_event(event_id) if annotations.chunk: relations[RelationTypes.ANNOTATION] = annotations.to_dict() + references = await self.store.get_relations_for_event( + event_id, RelationTypes.REFERENCE, direction="f" + ) if references.chunk: relations[RelationTypes.REFERENCE] = references.to_dict() From 70d5b9d8254244f5dd4cb909106dd9d7360cb10f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 12:40:47 -0500 Subject: [PATCH 5/7] More consistently refer to bundled relations. --- synapse/events/utils.py | 10 +++++----- synapse/handlers/events.py | 2 +- synapse/handlers/message.py | 2 +- synapse/rest/admin/rooms.py | 4 ++-- synapse/rest/client/relations.py | 6 +++--- synapse/rest/client/room.py | 2 +- synapse/rest/client/sync.py | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index e52a1e5a4e75..48f77785acf3 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -392,7 +392,7 @@ async def serialize_event( self, event: Union[JsonDict, EventBase], time_now: int, - bundle_aggregations: bool = True, + bundle_relations: bool = True, **kwargs: Any, ) -> JsonDict: """Serializes a single event. @@ -400,7 +400,7 @@ async def serialize_event( Args: event: The event being serialized. time_now: The current time in milliseconds - bundle_aggregations: Whether to bundle in related events + bundle_relations: Whether to bundle in related events **kwargs: Arguments to pass to `serialize_event` Returns: @@ -416,7 +416,7 @@ async def serialize_event( # we need to bundle in with the event. # Do not bundle relations if the event has been redacted if not event.internal_metadata.is_redacted() and ( - self._msc1849_enabled and bundle_aggregations + self._msc1849_enabled and bundle_relations ): await self._injected_bundled_relations(event, time_now, serialized_event) @@ -486,9 +486,9 @@ async def _injected_bundled_relations( ) = await self.store.get_thread_summary(event_id) if latest_thread_event: relations[RelationTypes.THREAD] = { - # Don't bundle aggregations as this could recurse forever. + # Don't bundle relations as this could recurse forever. "latest_event": await self.serialize_event( - latest_thread_event, time_now, bundle_aggregations=False + latest_thread_event, time_now, bundle_relations=False ), "count": thread_count, } diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index 1f64534a8a78..b4ff935546c8 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -124,7 +124,7 @@ async def get_stream( as_client_event=as_client_event, # We don't bundle "live" events, as otherwise clients # will end up double counting annotations. - bundle_aggregations=False, + bundle_relations=False, ) chunk = { diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 22dd4cf5fd3f..95b4fad3c68b 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -252,7 +252,7 @@ async def get_state_events( now, # We don't bother bundling aggregations in when asked for state # events, as clients won't use them. - bundle_aggregations=False, + bundle_relations=False, ) return events diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 5b8ec1e5ca89..a89dda1ba5b2 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -448,7 +448,7 @@ async def on_GET( now, # We don't bother bundling aggregations in when asked for state # events, as clients won't use them. - bundle_aggregations=False, + bundle_relations=False, ) ret = {"state": room_state} @@ -778,7 +778,7 @@ async def on_GET( results["state"], time_now, # No need to bundle aggregations for state events - bundle_aggregations=False, + bundle_relations=False, ) return 200, results diff --git a/synapse/rest/client/relations.py b/synapse/rest/client/relations.py index 184cfbe19626..45e9f1dd9022 100644 --- a/synapse/rest/client/relations.py +++ b/synapse/rest/client/relations.py @@ -224,17 +224,17 @@ async def on_GET( ) now = self.clock.time_msec() - # We set bundle_aggregations to False when retrieving the original + # We set bundle_relations to False when retrieving the original # event because we want the content before relations were applied to # it. original_event = await self._event_serializer.serialize_event( - event, now, bundle_aggregations=False + event, now, bundle_relations=False ) # Similarly, we don't allow relations to be applied to relations, so we # return the original relations without any aggregations on top of them # here. serialized_events = await self._event_serializer.serialize_events( - events, now, bundle_aggregations=False + events, now, bundle_relations=False ) return_value = pagination_chunk.to_dict() diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 03a353d53c2c..955d4e8641fe 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -719,7 +719,7 @@ async def on_GET( results["state"], time_now, # No need to bundle aggregations for state events - bundle_aggregations=False, + bundle_relations=False, ) return 200, results diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 8c0fdb194004..b6a24857320b 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -522,7 +522,7 @@ def serialize(events: Iterable[EventBase]) -> Awaitable[List[JsonDict]]: time_now=time_now, # We don't bundle "live" events, as otherwise clients # will end up double counting annotations. - bundle_aggregations=False, + bundle_relations=False, token_id=token_id, event_format=event_formatter, only_event_fields=only_fields, From 658e47f90122933f3d38bd88e0f3a5ae00c653e6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 12:51:49 -0500 Subject: [PATCH 6/7] Clarify comment. --- synapse/events/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 48f77785acf3..e5967c995e8a 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -1,4 +1,5 @@ # Copyright 2014-2016 OpenMarket Ltd +# 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. @@ -400,7 +401,8 @@ async def serialize_event( Args: event: The event being serialized. time_now: The current time in milliseconds - bundle_relations: Whether to bundle in related events + bundle_relations: Whether to include the bundled relations for this + event. **kwargs: Arguments to pass to `serialize_event` Returns: From c160558f885a102d034214ea800c252afb30ba75 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 12:58:00 -0500 Subject: [PATCH 7/7] Newsfragment --- changelog.d/11408.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11408.misc diff --git a/changelog.d/11408.misc b/changelog.d/11408.misc new file mode 100644 index 000000000000..55ed064672c6 --- /dev/null +++ b/changelog.d/11408.misc @@ -0,0 +1 @@ +Refactor including the bundled relations when serializing an event.