From 0593bd50c82e6a5ad7bd2d81b4c79513347252d0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 7 Oct 2021 14:47:40 -0400 Subject: [PATCH 01/10] Include aggregations of events in the /relations API. --- synapse/rest/client/relations.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/synapse/rest/client/relations.py b/synapse/rest/client/relations.py index 184cfbe19626..16b247c3c1e6 100644 --- a/synapse/rest/client/relations.py +++ b/synapse/rest/client/relations.py @@ -230,12 +230,9 @@ async def on_GET( original_event = await self._event_serializer.serialize_event( event, now, bundle_aggregations=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 - ) + # For any relations applying to the original event they need their + # aggregations applied to them. + serialized_events = await self._event_serializer.serialize_events(events, now) return_value = pagination_chunk.to_dict() return_value["chunk"] = serialized_events From d44219d8ab95175892a592db389d4fe1cdce1b8e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 9 Nov 2021 12:59:36 -0500 Subject: [PATCH 02/10] Newsfragment --- changelog.d/11284.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11284.feature diff --git a/changelog.d/11284.feature b/changelog.d/11284.feature new file mode 100644 index 000000000000..9aad331f1fe8 --- /dev/null +++ b/changelog.d/11284.feature @@ -0,0 +1 @@ +Include bundled relations in response to the `/relations` API, per [MSC3440](https://github.com/matrix-org/matrix-doc/pull/3440). From 05c2fa1aaf55cd9504d4933c2eab0b4da382017e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 13:48:41 -0500 Subject: [PATCH 03/10] Add test for bundled relations when requesting /relations. --- tests/rest/client/test_relations.py | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 78c2fb86b983..906a2b1ccc10 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -461,6 +461,52 @@ def test_aggregation_get_event(self): }, ) + def test_aggregation_get_event_for_thread(self): + """Test that threads get bundled relations included when directly requested.""" + channel = self._send_relation(RelationTypes.THREAD, "m.room.test") + self.assertEquals(200, channel.code, channel.json_body) + thread_id = channel.json_body["event_id"] + + # Annotate the annotation. + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "a", parent_id=thread_id + ) + self.assertEquals(200, channel.code, channel.json_body) + + channel = self.make_request( + "GET", + f"/rooms/{self.room}/event/{thread_id}", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertEquals( + channel.json_body["unsigned"].get("m.relations"), + { + RelationTypes.ANNOTATION: { + "chunk": [{"count": 1, "key": "a", "type": "m.reaction"}] + }, + }, + ) + + # It should also be included when the entire thread is requested. + channel = self.make_request( + "GET", + f"/_matrix/client/unstable/rooms/{self.room}/relations/{self.parent_id}?limit=1", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertEqual(len(channel.json_body["chunk"]), 1) + + thread_message = channel.json_body["chunk"][0] + self.assertEquals( + thread_message["unsigned"].get("m.relations"), + { + RelationTypes.ANNOTATION: { + "chunk": [{"count": 1, "key": "a", "type": "m.reaction"}] + }, + }, + ) + def test_edit(self): """Test that a simple edit works.""" From 78757f881f3fc28cf9b28baf4324a332dd4b876b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:47:07 -0500 Subject: [PATCH 04/10] 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 041e6c08ffec0483880b8947ce5b12a6a3aa1011 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:49:44 -0500 Subject: [PATCH 05/10] 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 3dab787be90fdd12743df39668dc836aa53552bd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 11:58:59 -0500 Subject: [PATCH 06/10] 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..b5bf43556d24 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 unsigend 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 ee686830b7ab9e1439a90fe1d9d9f10c41dec435 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 13:48:48 -0500 Subject: [PATCH 07/10] Do not bundle relations for annotations and edits. --- synapse/events/utils.py | 8 +++- tests/rest/client/test_relations.py | 72 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index b5bf43556d24..e1af196a2ddd 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -433,6 +433,13 @@ async def _injected_bundled_relations( serialized_event: The serialized event which may be modified. """ + # Do not bundle aggregations for an event which represents an edit or annotation. + relates_to = event.content.get("m.relates_to") + if isinstance(relates_to, (dict, frozendict)): + relation_type = relates_to.get("rel_type") + if relation_type in (RelationTypes.ANNOTATION, RelationTypes.REPLACE): + return + event_id = event.event_id annotations = await self.store.get_aggregation_groups_for_event(event_id) @@ -465,7 +472,6 @@ async def _injected_bundled_relations( 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() diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 906a2b1ccc10..9e9ad6225955 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -461,6 +461,28 @@ def test_aggregation_get_event(self): }, ) + def test_aggregation_get_event_for_annotation(self): + """Test that annotations do not get bundled relations included + when directly requested. + """ + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + annotation_id = channel.json_body["event_id"] + + # Annotate the annotation. + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "a", parent_id=annotation_id + ) + self.assertEquals(200, channel.code, channel.json_body) + + channel = self.make_request( + "GET", + f"/rooms/{self.room}/event/{annotation_id}", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertIsNone(channel.json_body["unsigned"].get("m.relations")) + def test_aggregation_get_event_for_thread(self): """Test that threads get bundled relations included when directly requested.""" channel = self._send_relation(RelationTypes.THREAD, "m.room.test") @@ -653,6 +675,56 @@ def test_edit_reply(self): {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict ) + def test_edit_edit(self): + """Test that an edit cannot be edited.""" + new_body = {"msgtype": "m.text", "body": "Initial edit"} + channel = self._send_relation( + RelationTypes.REPLACE, + "m.room.message", + content={ + "msgtype": "m.text", + "body": "Wibble", + "m.new_content": new_body, + }, + ) + self.assertEquals(200, channel.code, channel.json_body) + edit_event_id = channel.json_body["event_id"] + + # Edit the edit event. + channel = self._send_relation( + RelationTypes.REPLACE, + "m.room.message", + content={ + "msgtype": "m.text", + "body": "foo", + "m.new_content": {"msgtype": "m.text", "body": "Ignored edit"}, + }, + parent_id=edit_event_id, + ) + self.assertEquals(200, channel.code, channel.json_body) + + # Request the original event. + channel = self.make_request( + "GET", + "/rooms/%s/event/%s" % (self.room, self.parent_id), + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + # The edit to the edit should be ignored. + self.assertEquals(channel.json_body["content"], new_body) + + # The relations information should not include the edit to the edit. + relations_dict = channel.json_body["unsigned"].get("m.relations") + self.assertIn(RelationTypes.REPLACE, relations_dict) + + m_replace_dict = relations_dict[RelationTypes.REPLACE] + for key in ["event_id", "sender", "origin_server_ts"]: + self.assertIn(key, m_replace_dict) + + self.assert_dict( + {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict + ) + def test_relations_redaction_redacts_edits(self): """Test that edits of an event are redacted when the original event is redacted. From 17a61a8a3a9eac49056c89bc62176700c7d2d22e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 11:09:49 -0500 Subject: [PATCH 08/10] Fix typo. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/events/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index e1af196a2ddd..910b45bd17eb 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -425,7 +425,7 @@ async def serialize_event( async def _injected_bundled_relations( self, event: EventBase, time_now: int, serialized_event: JsonDict ) -> None: - """Potentially injects bundled relations into the unsigend portion of the serialized event. + """Potentially injects bundled relations into the unsigned portion of the serialized event. Args: event: The event being serialized. From 7cede76a9667d7e0dd88f9f013a5a88892d6663d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 Nov 2021 06:54:56 -0500 Subject: [PATCH 09/10] Clarified comments. --- synapse/events/utils.py | 3 ++- synapse/rest/client/relations.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index fed51548fdf0..05219a9dd03a 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -435,7 +435,8 @@ async def _injected_bundled_relations( serialized_event: The serialized event which may be modified. """ - # Do not bundle aggregations for an event which represents an edit or annotation. + # Do not bundle relations for an event which represents an edit or an + # annotation. It does not make sense for them to have related events. relates_to = event.content.get("m.relates_to") if isinstance(relates_to, (dict, frozendict)): relation_type = relates_to.get("rel_type") diff --git a/synapse/rest/client/relations.py b/synapse/rest/client/relations.py index a0760b46748e..b1a3304849c6 100644 --- a/synapse/rest/client/relations.py +++ b/synapse/rest/client/relations.py @@ -230,8 +230,8 @@ async def on_GET( original_event = await self._event_serializer.serialize_event( event, now, bundle_relations=False ) - # For any relations applying to the original event they need their - # aggregations applied to them. + # The relations returned for the requested event do include their + # bundled relations. serialized_events = await self._event_serializer.serialize_events(events, now) return_value = pagination_chunk.to_dict() From c89bf96814e2afc2dee85635542f3d74a6472919 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 29 Nov 2021 10:38:51 -0500 Subject: [PATCH 10/10] Clarify changelog. --- changelog.d/11284.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/11284.feature b/changelog.d/11284.feature index 9aad331f1fe8..cbaa5a988cfb 100644 --- a/changelog.d/11284.feature +++ b/changelog.d/11284.feature @@ -1 +1 @@ -Include bundled relations in response to the `/relations` API, per [MSC3440](https://github.com/matrix-org/matrix-doc/pull/3440). +When returning relation events from the `/relations` API, bundle any relations of those relations into the result, per updates to [MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675).