OpenCloud
/
MatrixSynapse
mirror of https://github.com/matrix-org/synapse
1
0
Fork 0
Code Issues Releases Wiki Activity

Implement MSC3925: changes to bundling of edits (#14811) 

Two parts to this:

 * Bundle the whole of the replacement with any edited events. This is backwards-compatible so I haven't put it behind a flag.
 * Optionally, inhibit server-side replacement of edited events. This has scope to break things, so it is currently disabled by default.
pull/14816/head
Richard van der Hoff 2023-01-10 16:31:28 +00:00 committed by GitHub
parent f417fb84b8
commit 06ab64f201
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 159 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/14811.feature Normal file
View File

@ -0,0 +1 @@
Per [MSC3925](https://github.com/matrix-org/matrix-spec-proposals/pull/3925), bundle the whole of the replacement with any edited events, and optionally inhibit server-side replacement.

3
synapse/config/experimental.py
View File

@ -139,3 +139,6 @@ class ExperimentalConfig(Config):
# MSC3391: Removing account data.
self.msc3391_enabled = experimental.get("msc3391_enabled", False)
# MSC3925: do not replace events with their edits
self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False)

31
synapse/events/utils.py
View File

@ -403,6 +403,14 @@ class EventClientSerializer:
clients.
"""
def __init__(self, inhibit_replacement_via_edits: bool = False):
"""
Args:
inhibit_replacement_via_edits: If this is set to True, then events are
never replaced by their edits.
"""
self._inhibit_replacement_via_edits = inhibit_replacement_via_edits
def serialize_event(
self,
event: Union[JsonDict, EventBase],
@ -422,6 +430,8 @@ class EventClientSerializer:
into the event.
apply_edits: Whether the content of the event should be modified to reflect
any replacement in `bundle_aggregations[<event_id>].replace`.
See also the `inhibit_replacement_via_edits` constructor arg: if that is
set to True, then this argument is ignored.
Returns:
The serialized event
"""
@ -495,7 +505,8 @@ class EventClientSerializer:
again for additional events in a recursive manner.
serialized_event: The serialized event which may be modified.
apply_edits: Whether the content of the event should be modified to reflect
any replacement in `aggregations.replace`.
any replacement in `aggregations.replace` (subject to the
`inhibit_replacement_via_edits` constructor arg).
"""
# We have already checked that aggregations exist for this event.
@ -518,15 +529,21 @@ class EventClientSerializer:
if event_aggregations.replace:
# If there is an edit, optionally apply it to the event.
edit = event_aggregations.replace
if apply_edits:
if apply_edits and not self._inhibit_replacement_via_edits:
self._apply_edit(event, serialized_event, edit)
# Include information about it in the relations dict.
serialized_aggregations[RelationTypes.REPLACE] = {
"event_id": edit.event_id,
"origin_server_ts": edit.origin_server_ts,
"sender": edit.sender,
}
#
# Matrix spec v1.5 (https://spec.matrix.org/v1.5/client-server-api/#server-side-aggregation-of-mreplace-relationships)
# said that we should only include the `event_id`, `origin_server_ts` and
# `sender` of the edit; however MSC3925 proposes extending it to the whole
# of the edit, which is what we do here.
serialized_aggregations[RelationTypes.REPLACE] = self.serialize_event(
edit,
time_now,
config=config,
apply_edits=False,
)
# Include any threaded replies to this event.
if event_aggregations.thread:

2
synapse/server.py
View File

@ -743,7 +743,7 @@ class HomeServer(metaclass=abc.ABCMeta):
@cache_in_self
def get_event_client_serializer(self) -> EventClientSerializer:
return EventClientSerializer()
return EventClientSerializer(self.config.experimental.msc3925_inhibit_edit)
@cache_in_self
def get_password_policy_handler(self) -> PasswordPolicyHandler:

185
tests/rest/client/test_relations.py
View File

@ -30,6 +30,7 @@ from tests import unittest
from tests.server import FakeChannel
from tests.test_utils import make_awaitable
from tests.test_utils.event_injection import inject_event
from tests.unittest import override_config
class BaseRelationsTestCase(unittest.HomeserverTestCase):
@ -355,30 +356,67 @@ class RelationsTestCase(BaseRelationsTestCase):
self.assertEqual(200, channel.code, channel.json_body)
self.assertNotIn("m.relations", channel.json_body["unsigned"])
def test_edit(self) -> None:
"""Test that a simple edit works."""
def _assert_edit_bundle(
self, event_json: JsonDict, edit_event_id: str, edit_event_content: JsonDict
) -> None:
"""
Assert that the given event has a correctly-serialised edit event in its
bundled aggregations
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body},
)
edit_event_id = channel.json_body["event_id"]
def assert_bundle(event_json: JsonDict) -> None:
"""Assert the expected values of the bundled aggregations."""
Args:
event_json: the serialised event to be checked
edit_event_id: the ID of the edit event that we expect to be bundled
edit_event_content: the content of that event, excluding the 'm.relates_to`
property
"""
relations_dict = event_json["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"]:
for key in [
"event_id",
"sender",
"origin_server_ts",
"content",
"type",
"unsigned",
]:
self.assertIn(key, m_replace_dict)
expected_edit_content = {
"m.relates_to": {
"event_id": event_json["event_id"],
"rel_type": "m.replace",
}
}
expected_edit_content.update(edit_event_content)
self.assert_dict(
{"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict
{
"event_id": edit_event_id,
"sender": self.user_id,
"content": expected_edit_content,
"type": "m.room.message",
},
m_replace_dict,
)
def test_edit(self) -> None:
"""Test that a simple edit works."""
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
edit_event_content = {
"msgtype": "m.text",
"body": "foo",
"m.new_content": new_body,
}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content=edit_event_content,
)
edit_event_id = channel.json_body["event_id"]
# /event should return the *original* event
channel = self.make_request(
"GET",
@ -389,7 +427,7 @@ class RelationsTestCase(BaseRelationsTestCase):
self.assertEqual(
channel.json_body["content"], {"body": "Hi!", "msgtype": "m.text"}
)
assert_bundle(channel.json_body)
self._assert_edit_bundle(channel.json_body, edit_event_id, edit_event_content)
# Request the room messages.
channel = self.make_request(
@ -398,7 +436,11 @@ class RelationsTestCase(BaseRelationsTestCase):
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
assert_bundle(self._find_event_in_chunk(channel.json_body["chunk"]))
self._assert_edit_bundle(
self._find_event_in_chunk(channel.json_body["chunk"]),
edit_event_id,
edit_event_content,
)
# Request the room context.
# /context should return the edited event.
@ -408,7 +450,9 @@ class RelationsTestCase(BaseRelationsTestCase):
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
assert_bundle(channel.json_body["event"])
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
self.assertEqual(channel.json_body["event"]["content"], new_body)
# Request sync, but limit the timeline so it becomes limited (and includes
@ -420,7 +464,11 @@ class RelationsTestCase(BaseRelationsTestCase):
self.assertEqual(200, channel.code, channel.json_body)
room_timeline = channel.json_body["rooms"]["join"][self.room]["timeline"]
self.assertTrue(room_timeline["limited"])
assert_bundle(self._find_event_in_chunk(room_timeline["events"]))
self._assert_edit_bundle(
self._find_event_in_chunk(room_timeline["events"]),
edit_event_id,
edit_event_content,
)
# Request search.
channel = self.make_request(
@ -437,7 +485,45 @@ class RelationsTestCase(BaseRelationsTestCase):
"results"
]
]
assert_bundle(self._find_event_in_chunk(chunk))
self._assert_edit_bundle(
self._find_event_in_chunk(chunk),
edit_event_id,
edit_event_content,
)
@override_config({"experimental_features": {"msc3925_inhibit_edit": True}})
def test_edit_inhibit_replace(self) -> None:
"""
If msc3925_inhibit_edit is enabled, then the original event should not be
replaced.
"""
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
edit_event_content = {
"msgtype": "m.text",
"body": "foo",
"m.new_content": new_body,
}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content=edit_event_content,
)
edit_event_id = channel.json_body["event_id"]
# /context should return the *original* event.
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"], {"body": "Hi!", "msgtype": "m.text"}
)
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
def test_multi_edit(self) -> None:
"""Test that multiple edits, including attempts by people who
@ -455,10 +541,15 @@ class RelationsTestCase(BaseRelationsTestCase):
)
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
edit_event_content = {
"msgtype": "m.text",
"body": "foo",
"m.new_content": new_body,
}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body},
content=edit_event_content,
)
edit_event_id = channel.json_body["event_id"]
@ -480,16 +571,8 @@ class RelationsTestCase(BaseRelationsTestCase):
self.assertEqual(200, channel.code, channel.json_body)
self.assertEqual(channel.json_body["event"]["content"], new_body)
relations_dict = channel.json_body["event"]["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
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
def test_edit_reply(self) -> None:
@ -502,11 +585,15 @@ class RelationsTestCase(BaseRelationsTestCase):
)
reply = channel.json_body["event_id"]
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
edit_event_content = {
"msgtype": "m.text",
"body": "foo",
"m.new_content": {"msgtype": "m.text", "body": "I've been edited!"},
}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body},
content=edit_event_content,
parent_id=reply,
)
edit_event_id = channel.json_body["event_id"]
@ -549,28 +636,22 @@ class RelationsTestCase(BaseRelationsTestCase):
# 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)
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
self._assert_edit_bundle(
result_event_dict, edit_event_id, edit_event_content
)
def test_edit_edit(self) -> None:
"""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={
edit_event_content = {
"msgtype": "m.text",
"body": "Wibble",
"m.new_content": new_body,
},
}
channel = self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
content=edit_event_content,
)
edit_event_id = channel.json_body["event_id"]
@ -599,8 +680,7 @@ class RelationsTestCase(BaseRelationsTestCase):
)
# 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)
self._assert_edit_bundle(channel.json_body, edit_event_id, edit_event_content)
# /context should return the event updated for the *first* edit
# (The edit to the edit should be ignored.)
@ -611,13 +691,8 @@ class RelationsTestCase(BaseRelationsTestCase):
)
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)
self.assert_dict(
{"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
# Directly requesting the edit should not have the edit to the edit applied.