diff options
| -rw-r--r-- | changelog.d/12633.bugfix | 1 | ||||
| -rw-r--r-- | synapse/handlers/relations.py | 38 | ||||
| -rw-r--r-- | tests/rest/client/test_relations.py | 31 |
3 files changed, 50 insertions, 20 deletions
diff --git a/changelog.d/12633.bugfix b/changelog.d/12633.bugfix new file mode 100644 index 0000000000..32332acd9a --- /dev/null +++ b/changelog.d/12633.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse v1.53.0 where bundled aggregations for annotations/edits were incorrectly calculated. diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py index cec5740fbd..c2754ec918 100644 --- a/synapse/handlers/relations.py +++ b/synapse/handlers/relations.py @@ -364,21 +364,29 @@ class RelationsHandler: The results may include additional events which are related to the requested events. """ - # De-duplicate events by ID to handle the same event requested multiple times. - # - # State events do not get bundled aggregations. - events_by_id = { - event.event_id: event for event in events if not event.is_state() - } - + # De-duplicated events by ID to handle the same event requested multiple times. + events_by_id = {} # A map of event ID to the relation in that event, if there is one. relations_by_id: Dict[str, str] = {} - for event_id, event in events_by_id.items(): + for event in events: + # State events do not get bundled aggregations. + if event.is_state(): + continue + relates_to = event.content.get("m.relates_to") + relation_type = None if isinstance(relates_to, collections.abc.Mapping): relation_type = relates_to.get("rel_type") - if isinstance(relation_type, str): - relations_by_id[event_id] = relation_type + # An event which is a replacement (ie edit) or annotation (ie, + # reaction) may not have any other event related to it. + if relation_type in (RelationTypes.ANNOTATION, RelationTypes.REPLACE): + continue + + # The event should get bundled aggregations. + events_by_id[event.event_id] = event + # Track the event's relation information for later. + if isinstance(relation_type, str): + relations_by_id[event.event_id] = relation_type # event ID -> bundled aggregation in non-serialized form. results: Dict[str, BundledAggregations] = {} @@ -413,16 +421,6 @@ class RelationsHandler: # Fetch other relations per event. for event in events_by_id.values(): - # An event which is a replacement (ie edit) or annotation (ie, reaction) - # may not have any other event related to it. - # - # XXX This is buggy, see https://github.com/matrix-org/synapse/issues/12566 - if relations_by_id.get(event.event_id) in ( - RelationTypes.ANNOTATION, - RelationTypes.REPLACE, - ): - continue - # Fetch any annotations (ie, reactions) to bundle with this event. annotations = await self.get_annotations_for_event( event.event_id, event.room_id, ignored_users=ignored_users diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 33ce9471b3..27dee8f697 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -620,6 +620,19 @@ class RelationsTestCase(BaseRelationsTestCase): {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict ) + # Directly requesting the edit should not have the edit to the edit applied. + channel = self.make_request( + "GET", + f"/rooms/{self.room}/event/{edit_event_id}", + access_token=self.user_token, + ) + self.assertEqual(200, channel.code, channel.json_body) + self.assertEqual("Wibble", channel.json_body["content"]["body"]) + self.assertIn("m.new_content", channel.json_body["content"]) + + # The relations information should not include the edit to the edit. + self.assertNotIn("m.relations", channel.json_body["unsigned"]) + def test_unknown_relations(self) -> None: """Unknown relations should be accepted.""" channel = self._send_relation("m.relation.test", "m.room.test") @@ -984,6 +997,24 @@ class BundledAggregationsTestCase(BaseRelationsTestCase): self._test_bundled_aggregations(RelationTypes.ANNOTATION, assert_annotations, 7) + def test_annotation_to_annotation(self) -> None: + """Any relation to an annotation should be ignored.""" + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + event_id = channel.json_body["event_id"] + self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "b", parent_id=event_id + ) + + # Fetch the initial annotation event to see if it has bundled aggregations. + channel = self.make_request( + "GET", + f"/_matrix/client/v3/rooms/{self.room}/event/{event_id}", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + # The first annotationt should not have any bundled aggregations. + self.assertNotIn("m.relations", channel.json_body["unsigned"]) + def test_reference(self) -> None: """ Test that references get correctly bundled. |