diff --git a/changelog.d/15193.bugfix b/changelog.d/15193.bugfix
new file mode 100644
index 0000000000..ca781e9631
--- /dev/null
+++ b/changelog.d/15193.bugfix
@@ -0,0 +1 @@
+Stop applying edits when bundling aggregations, per [MSC3925](https://github.com/matrix-org/matrix-spec-proposals/pull/3925).
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index 9c58cee2c8..489f2601ac 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -166,9 +166,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)
-
# MSC3873: Disambiguate event_match keys.
self.msc3873_escape_event_match_key = experimental.get(
"msc3873_escape_event_match_key", False
diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index eaa6cad4af..45f46949a1 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -39,7 +39,6 @@ from synapse.api.constants import (
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import RoomVersion
from synapse.types import JsonDict
-from synapse.util.frozenutils import unfreeze
from . import EventBase
@@ -403,14 +402,6 @@ 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],
@@ -418,7 +409,6 @@ class EventClientSerializer:
*,
config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG,
bundle_aggregations: Optional[Dict[str, "BundledAggregations"]] = None,
- apply_edits: bool = True,
) -> JsonDict:
"""Serializes a single event.
@@ -428,10 +418,7 @@ class EventClientSerializer:
config: Event serialization config
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`.
- See also the `inhibit_replacement_via_edits` constructor arg: if that is
- set to True, then this argument is ignored.
+
Returns:
The serialized event
"""
@@ -450,38 +437,10 @@ class EventClientSerializer:
config,
bundle_aggregations,
serialized_event,
- apply_edits=apply_edits,
)
return serialized_event
- def _apply_edit(
- self, orig_event: EventBase, serialized_event: JsonDict, edit: EventBase
- ) -> None:
- """Replace the content, preserving existing relations of the serialized event.
-
- Args:
- orig_event: The original event.
- serialized_event: The original event, serialized. This is modified.
- edit: The event which edits the above.
- """
-
- # 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 = orig_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)
-
def _inject_bundled_aggregations(
self,
event: EventBase,
@@ -489,7 +448,6 @@ class EventClientSerializer:
config: SerializeEventConfig,
bundled_aggregations: Dict[str, "BundledAggregations"],
serialized_event: JsonDict,
- apply_edits: bool,
) -> None:
"""Potentially injects bundled aggregations into the unsigned portion of the serialized event.
@@ -504,9 +462,6 @@ class EventClientSerializer:
While serializing the bundled aggregations this map may be searched
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` (subject to the
- `inhibit_replacement_via_edits` constructor arg).
"""
# We have already checked that aggregations exist for this event.
@@ -522,11 +477,6 @@ class EventClientSerializer:
] = event_aggregations.references
if event_aggregations.replace:
- # If there is an edit, optionally apply it to the event.
- edit = event_aggregations.replace
- 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.
#
# Matrix spec v1.5 (https://spec.matrix.org/v1.5/client-server-api/#server-side-aggregation-of-mreplace-relationships)
@@ -534,10 +484,7 @@ class EventClientSerializer:
# `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,
+ event_aggregations.replace, time_now, config=config
)
# Include any threaded replies to this event.
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 45aee3d3fe..c5af07816a 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -818,7 +818,7 @@ class RoomEventServlet(RestServlet):
# 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, apply_edits=False
+ event, time_now, bundle_aggregations=aggregations
)
return 200, event_dict
diff --git a/synapse/server.py b/synapse/server.py
index a7c32e9a60..df80fc1beb 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -743,7 +743,7 @@ class HomeServer(metaclass=abc.ABCMeta):
@cache_in_self
def get_event_client_serializer(self) -> EventClientSerializer:
- return EventClientSerializer(self.config.experimental.msc3925_inhibit_edit)
+ return EventClientSerializer()
@cache_in_self
def get_password_policy_handler(self) -> PasswordPolicyHandler:
diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py
index a8a0a16141..fbbbcb23f1 100644
--- a/tests/rest/client/test_relations.py
+++ b/tests/rest/client/test_relations.py
@@ -30,7 +30,6 @@ 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):
@@ -403,7 +402,7 @@ class RelationsTestCase(BaseRelationsTestCase):
def test_edit(self) -> None:
"""Test that a simple edit works."""
-
+ orig_body = {"body": "Hi!", "msgtype": "m.text"}
new_body = {"msgtype": "m.text", "body": "I've been edited!"}
edit_event_content = {
"msgtype": "m.text",
@@ -424,9 +423,7 @@ class RelationsTestCase(BaseRelationsTestCase):
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
- self.assertEqual(
- channel.json_body["content"], {"body": "Hi!", "msgtype": "m.text"}
- )
+ self.assertEqual(channel.json_body["content"], orig_body)
self._assert_edit_bundle(channel.json_body, edit_event_id, edit_event_content)
# Request the room messages.
@@ -443,7 +440,7 @@ class RelationsTestCase(BaseRelationsTestCase):
)
# Request the room context.
- # /context should return the edited event.
+ # /context should return the event.
channel = self.make_request(
"GET",
f"/rooms/{self.room}/context/{self.parent_id}",
@@ -453,7 +450,7 @@ class RelationsTestCase(BaseRelationsTestCase):
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
- self.assertEqual(channel.json_body["event"]["content"], new_body)
+ self.assertEqual(channel.json_body["event"]["content"], orig_body)
# Request sync, but limit the timeline so it becomes limited (and includes
# bundled aggregations).
@@ -491,45 +488,11 @@ class RelationsTestCase(BaseRelationsTestCase):
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
shouldn't be allowed, are correctly handled.
"""
-
+ orig_body = orig_body = {"body": "Hi!", "msgtype": "m.text"}
self._send_relation(
RelationTypes.REPLACE,
"m.room.message",
@@ -570,7 +533,7 @@ class RelationsTestCase(BaseRelationsTestCase):
)
self.assertEqual(200, channel.code, channel.json_body)
- self.assertEqual(channel.json_body["event"]["content"], new_body)
+ self.assertEqual(channel.json_body["event"]["content"], orig_body)
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
@@ -642,6 +605,7 @@ class RelationsTestCase(BaseRelationsTestCase):
def test_edit_edit(self) -> None:
"""Test that an edit cannot be edited."""
+ orig_body = {"body": "Hi!", "msgtype": "m.text"}
new_body = {"msgtype": "m.text", "body": "Initial edit"}
edit_event_content = {
"msgtype": "m.text",
@@ -675,14 +639,12 @@ class RelationsTestCase(BaseRelationsTestCase):
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
- self.assertEqual(
- channel.json_body["content"], {"body": "Hi!", "msgtype": "m.text"}
- )
+ self.assertEqual(channel.json_body["content"], orig_body)
# The relations information should not include the edit to the edit.
self._assert_edit_bundle(channel.json_body, edit_event_id, edit_event_content)
- # /context should return the event updated for the *first* edit
+ # /context should return the bundled edit for the *first* edit
# (The edit to the edit should be ignored.)
channel = self.make_request(
"GET",
@@ -690,7 +652,7 @@ class RelationsTestCase(BaseRelationsTestCase):
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
- self.assertEqual(channel.json_body["event"]["content"], new_body)
+ self.assertEqual(channel.json_body["event"]["content"], orig_body)
self._assert_edit_bundle(
channel.json_body["event"], edit_event_id, edit_event_content
)
@@ -1287,7 +1249,6 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
thread_summary = relations_dict[RelationTypes.THREAD]
self.assertIn("latest_event", thread_summary)
latest_event_in_thread = thread_summary["latest_event"]
- self.assertEqual(latest_event_in_thread["content"]["body"], "I've been edited!")
# The latest event in the thread should have the edit appear under the
# bundled aggregations.
self.assertDictContainsSubset(
|
