diff options
| -rw-r--r-- | changelog.d/12476.bugfix | 1 | ||||
| -rw-r--r-- | synapse/events/utils.py | 21 | ||||
| -rw-r--r-- | synapse/rest/client/room.py | 4 | ||||
| -rw-r--r-- | tests/rest/client/test_relations.py | 92 |
4 files changed, 79 insertions, 39 deletions
diff --git a/changelog.d/12476.bugfix b/changelog.d/12476.bugfix new file mode 100644 index 0000000000..9ad6a71abd --- /dev/null +++ b/changelog.d/12476.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug which incorrectly caused `GET /_matrix/client/r3/rooms/{roomId}/event/{eventId}` to return edited events rather than the original. diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 43c3241fb0..2174b4a094 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -402,6 +402,7 @@ class EventClientSerializer: *, config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, bundle_aggregations: Optional[Dict[str, "BundledAggregations"]] = None, + apply_edits: bool = True, ) -> JsonDict: """Serializes a single event. @@ -409,10 +410,10 @@ class EventClientSerializer: event: The event being serialized. time_now: The current time in milliseconds config: Event serialization config - bundle_aggregations: Whether to include the bundled aggregations for this - event. Only applies to non-state events. (State events never include - bundled aggregations.) - + bundle_aggregations: A map from event_id to the aggregations to be bundled + into the event. + apply_edits: Whether the content of the event should be modified to reflect + any replacement in `bundle_aggregations[<event_id>].replace`. Returns: The serialized event """ @@ -430,8 +431,9 @@ class EventClientSerializer: event, time_now, config, - bundle_aggregations[event.event_id], + event_aggregations, serialized_event, + apply_edits=apply_edits, ) return serialized_event @@ -470,6 +472,7 @@ class EventClientSerializer: config: SerializeEventConfig, aggregations: "BundledAggregations", serialized_event: JsonDict, + apply_edits: bool, ) -> None: """Potentially injects bundled aggregations into the unsigned portion of the serialized event. @@ -479,7 +482,8 @@ class EventClientSerializer: aggregations: The bundled aggregation to serialize. serialized_event: The serialized event which may be modified. config: Event serialization config - + apply_edits: Whether the content of the event should be modified to reflect + any replacement in `aggregations.replace`. """ serialized_aggregations = {} @@ -490,9 +494,10 @@ class EventClientSerializer: serialized_aggregations[RelationTypes.REFERENCE] = aggregations.references if aggregations.replace: - # If there is an edit, apply it to the event. + # If there is an edit, optionally apply it to the event. edit = aggregations.replace - self._apply_edit(event, serialized_event, edit) + if apply_edits: + self._apply_edit(event, serialized_event, edit) # Include information about it in the relations dict. serialized_aggregations[RelationTypes.REPLACE] = { diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 47e152c8cc..937c323176 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -669,8 +669,10 @@ class RoomEventServlet(RestServlet): ) time_now = self.clock.time_msec() + # per MSC2676, /rooms/{roomId}/event/{eventId}, should return the + # *original* event, rather than the edited version event_dict = self._event_serializer.serialize_event( - event, time_now, bundle_aggregations=aggregations + event, time_now, bundle_aggregations=aggregations, apply_edits=False ) return 200, event_dict diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 6fabada8b3..57d97bbb47 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -380,13 +380,16 @@ class RelationsTestCase(BaseRelationsTestCase): {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict ) + # /event should return the *original* event channel = self.make_request( "GET", f"/rooms/{self.room}/event/{self.parent_id}", access_token=self.user_token, ) self.assertEqual(200, channel.code, channel.json_body) - self.assertEqual(channel.json_body["content"], new_body) + self.assertEqual( + channel.json_body["content"], {"body": "Hi!", "msgtype": "m.text"} + ) assert_bundle(channel.json_body) # Request the room messages. @@ -399,6 +402,7 @@ class RelationsTestCase(BaseRelationsTestCase): assert_bundle(self._find_event_in_chunk(channel.json_body["chunk"])) # Request the room context. + # /context should return the edited event. channel = self.make_request( "GET", f"/rooms/{self.room}/context/{self.parent_id}", @@ -406,6 +410,7 @@ class RelationsTestCase(BaseRelationsTestCase): ) self.assertEqual(200, channel.code, channel.json_body) assert_bundle(channel.json_body["event"]) + self.assertEqual(channel.json_body["event"]["content"], new_body) # Request sync, but limit the timeline so it becomes limited (and includes # bundled aggregations). @@ -470,14 +475,14 @@ class RelationsTestCase(BaseRelationsTestCase): channel = self.make_request( "GET", - f"/rooms/{self.room}/event/{self.parent_id}", + f"/rooms/{self.room}/context/{self.parent_id}", access_token=self.user_token, ) self.assertEqual(200, channel.code, channel.json_body) - self.assertEqual(channel.json_body["content"], new_body) + self.assertEqual(channel.json_body["event"]["content"], new_body) - relations_dict = channel.json_body["unsigned"].get("m.relations") + relations_dict = channel.json_body["event"]["unsigned"].get("m.relations") self.assertIn(RelationTypes.REPLACE, relations_dict) m_replace_dict = relations_dict[RelationTypes.REPLACE] @@ -492,10 +497,9 @@ class RelationsTestCase(BaseRelationsTestCase): """Test that editing a reply works.""" # Create a reply to edit. + original_body = {"msgtype": "m.text", "body": "A reply!"} channel = self._send_relation( - RelationTypes.REFERENCE, - "m.room.message", - content={"msgtype": "m.text", "body": "A reply!"}, + RelationTypes.REFERENCE, "m.room.message", content=original_body ) reply = channel.json_body["event_id"] @@ -508,38 +512,54 @@ class RelationsTestCase(BaseRelationsTestCase): ) edit_event_id = channel.json_body["event_id"] + # /event returns the original event channel = self.make_request( "GET", f"/rooms/{self.room}/event/{reply}", access_token=self.user_token, ) self.assertEqual(200, channel.code, channel.json_body) + event_result = channel.json_body + self.assertDictContainsSubset(original_body, event_result["content"]) - # We expect to see the new body in the dict, as well as the reference - # metadata sill intact. - self.assertDictContainsSubset(new_body, channel.json_body["content"]) - self.assertDictContainsSubset( - { - "m.relates_to": { - "event_id": self.parent_id, - "rel_type": "m.reference", - } - }, - channel.json_body["content"], + # also check /context, which returns the *edited* event + channel = self.make_request( + "GET", + f"/rooms/{self.room}/context/{reply}", + access_token=self.user_token, ) + self.assertEqual(200, channel.code, channel.json_body) + context_result = channel.json_body["event"] - # We expect that the edit relation appears in the unsigned relations - # section. - relations_dict = channel.json_body["unsigned"].get("m.relations") - self.assertIn(RelationTypes.REPLACE, relations_dict) + # check that the relations are correct for both APIs + for result_event_dict, desc in ( + (event_result, "/event"), + (context_result, "/context"), + ): + # The reference metadata should still be intact. + self.assertDictContainsSubset( + { + "m.relates_to": { + "event_id": self.parent_id, + "rel_type": "m.reference", + } + }, + result_event_dict["content"], + desc, + ) - m_replace_dict = relations_dict[RelationTypes.REPLACE] - for key in ["event_id", "sender", "origin_server_ts"]: - self.assertIn(key, m_replace_dict) + # We expect that the edit relation appears in the unsigned relations + # section. + relations_dict = result_event_dict["unsigned"].get("m.relations") + self.assertIn(RelationTypes.REPLACE, relations_dict, desc) - self.assert_dict( - {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict - ) + m_replace_dict = relations_dict[RelationTypes.REPLACE] + for key in ["event_id", "sender", "origin_server_ts"]: + self.assertIn(key, m_replace_dict, desc) + + self.assert_dict( + {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict + ) def test_edit_thread(self) -> None: """Test that editing a thread works.""" @@ -605,19 +625,31 @@ class RelationsTestCase(BaseRelationsTestCase): ) # Request the original event. + # /event should return the original event. channel = self.make_request( "GET", f"/rooms/{self.room}/event/{self.parent_id}", access_token=self.user_token, ) self.assertEqual(200, channel.code, channel.json_body) - # The edit to the edit should be ignored. - self.assertEqual(channel.json_body["content"], new_body) + self.assertEqual( + channel.json_body["content"], {"body": "Hi!", "msgtype": "m.text"} + ) # 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) + # /context should return the event updated for the *first* edit + # (The edit to the edit should be ignored.) + channel = self.make_request( + "GET", + f"/rooms/{self.room}/context/{self.parent_id}", + access_token=self.user_token, + ) + self.assertEqual(200, channel.code, channel.json_body) + self.assertEqual(channel.json_body["event"]["content"], new_body) + m_replace_dict = relations_dict[RelationTypes.REPLACE] for key in ["event_id", "sender", "origin_server_ts"]: self.assertIn(key, m_replace_dict) |