Fix `/room/.../event/...` to return the *original* event after any edits (#12476)
This is what the MSC (now) requires. Fixes https://github.com/matrix-org/synapse/issues/10310.
This commit is contained in:
parent
798deb3a10
commit
b80bb7e452
|
@ -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.
|
|
@ -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] = {
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue