Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add forward compatibility for the redacts property. #16013

Merged
merged 6 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions synapse/events/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,14 +475,14 @@ def serialize_event(
if config.as_client_event:
d = config.event_format(d)

# If the event is a redaction, copy the redacts field from the content to
# top-level for backwards compatibility.
if (
e.type == EventTypes.Redaction
and e.room_version.updated_redaction_rules
and e.redacts is not None
):
d["redacts"] = e.redacts
# If the event is a redaction, either copy the redacts field from the content to
# top-level for backwards compatibility OR from the redacts field to the content
# field for forwards compatibility.
if e.type == EventTypes.Redaction and e.redacts is not None:
if e.room_version.updated_redaction_rules:
d["redacts"] = e.redacts
else:
d["content"]["redacts"] = e.redacts
clokep marked this conversation as resolved.
Show resolved Hide resolved

only_event_fields = config.only_event_fields
if only_event_fields:
Expand Down
67 changes: 50 additions & 17 deletions tests/rest/client/test_redactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
# limitations under the License.
from typing import List, Optional

from parameterized import parameterized

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes, RelationTypes
from synapse.api.room_versions import RoomVersions
from synapse.api.room_versions import RoomVersion, RoomVersions
from synapse.rest import admin
from synapse.rest.client import login, room, sync
from synapse.server import HomeServer
Expand Down Expand Up @@ -569,50 +571,81 @@ def test_redact_relations_txn_id_reuse(self) -> None:
self.assertIn("body", event_dict["content"], event_dict)
self.assertEqual("I'm in a thread!", event_dict["content"]["body"])

def test_content_redaction(self) -> None:
"""MSC2174 moved the redacts property to the content."""
@parameterized.expand(
[
# Tuples of:
# Room version
# Boolean: True if the redaction event content should include the event ID.
# Boolean: true if the resulting redaction event is expected to include the
# event ID in the content.
(RoomVersions.V10, False, False),
(RoomVersions.V11, True, True),
(RoomVersions.V11, False, True),
Comment on lines +582 to +583
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a moment to see that the first bool controls what the test does, and the second bool defines what the test asserts. (I thought they were both controlling assertions, which didn't make sense given that V11 appears twice.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion on how to make it clearer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine; any action is on me to read more carefully.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add a description above the array since parameters.expand() tends to always be a bit opaque.

]
)
def test_redaction_content(
self, room_version: RoomVersion, include_content: bool, expect_content: bool
) -> None:
"""
Room version 11 moved the redacts property to the content.

Ensure that the event gets created properly and that the Client-Server
API servers the proper backwards-compatible version.
"""
# Create a room with the newer room version.
room_id = self.helper.create_room_as(
self.mod_user_id,
tok=self.mod_access_token,
room_version=RoomVersions.V11.identifier,
room_version=room_version.identifier,
)

# Create an event.
b = self.helper.send(room_id=room_id, tok=self.mod_access_token)
event_id = b["event_id"]

# Attempt to redact it with a bogus event ID.
self._redact_event(
# Ensure the event ID in the URL and the content must match.
if include_content:
self._redact_event(
self.mod_access_token,
room_id,
event_id,
expect_code=400,
content={"redacts": "foo"},
)

# Redact it for real.
result = self._redact_event(
self.mod_access_token,
room_id,
event_id,
expect_code=400,
content={"redacts": "foo"},
content={"redacts": event_id} if include_content else {},
)

# Redact it for real.
self._redact_event(self.mod_access_token, room_id, event_id)
redaction_event_id = result["event_id"]

# Sync the room, to get the id of the create event
timeline = self._sync_room_timeline(self.mod_access_token, room_id)
redact_event = timeline[-1]
self.assertEqual(redact_event["type"], EventTypes.Redaction)
# The redacts key should be in the content.
# The redacts key should be in the content and the redacts keys.
self.assertEquals(redact_event["content"]["redacts"], event_id)

# It should also be copied as the top-level redacts field for backwards
# compatibility.
self.assertEquals(redact_event["redacts"], event_id)

# But it isn't actually part of the event.
def get_event(txn: LoggingTransaction) -> JsonDict:
return db_to_json(
main_datastore._fetch_event_rows(txn, [event_id])[event_id].json
main_datastore._fetch_event_rows(txn, [redaction_event_id])[
redaction_event_id
].json
)

main_datastore = self.hs.get_datastores().main
event_json = self.get_success(
main_datastore.db_pool.runInteraction("get_event", get_event)
)
self.assertNotIn("redacts", event_json)
self.assertEquals(event_json["type"], EventTypes.Redaction)
if expect_content:
self.assertNotIn("redacts", event_json)
self.assertEquals(event_json["content"]["redacts"], event_id)
else:
self.assertEquals(event_json["redacts"], event_id)
self.assertNotIn("redacts", event_json["content"])