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

Edits/annotations should not have any bundled aggregations calculated. #12633

Merged
merged 4 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions changelog.d/12633.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug introduced in Synapse v1.53.0 where bundled aggregations for annotations/edits were incorrectly calculated.
38 changes: 18 additions & 20 deletions synapse/handlers/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,21 +364,29 @@ async def get_bundled_aggregations(
The results may include additional events which are related to the
requested events.
"""
# De-duplicate events by ID to handle the same event requested multiple times.
#
# State events do not get bundled aggregations.
events_by_id = {
event.event_id: event for event in events if not event.is_state()
}

# De-duplicated events by ID to handle the same event requested multiple times.
events_by_id = {}
# A map of event ID to the relation in that event, if there is one.
relations_by_id: Dict[str, str] = {}
for event_id, event in events_by_id.items():
for event in events:
# State events do not get bundled aggregations.
if event.is_state():
continue

relates_to = event.content.get("m.relates_to")
relation_type = None
if isinstance(relates_to, collections.abc.Mapping):
relation_type = relates_to.get("rel_type")
if isinstance(relation_type, str):
relations_by_id[event_id] = relation_type
# An event which is a replacement (ie edit) or annotation (ie,
# reaction) may not have any other event related to it.
if relation_type in (RelationTypes.ANNOTATION, RelationTypes.REPLACE):
continue

# The event should get bundled aggregations.
events_by_id[event.event_id] = event
# Track the event's relation information for later.
if isinstance(relation_type, str):
relations_by_id[event.event_id] = relation_type

# event ID -> bundled aggregation in non-serialized form.
results: Dict[str, BundledAggregations] = {}
Expand Down Expand Up @@ -413,16 +421,6 @@ async def get_bundled_aggregations(

# Fetch other relations per event.
for event in events_by_id.values():
# An event which is a replacement (ie edit) or annotation (ie, reaction)
# may not have any other event related to it.
#
# XXX This is buggy, see https://github.com/matrix-org/synapse/issues/12566
if relations_by_id.get(event.event_id) in (
RelationTypes.ANNOTATION,
RelationTypes.REPLACE,
):
continue

# Fetch any annotations (ie, reactions) to bundle with this event.
annotations = await self.get_annotations_for_event(
event.event_id, event.room_id, ignored_users=ignored_users
Expand Down
31 changes: 31 additions & 0 deletions tests/rest/client/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,19 @@ def test_edit_edit(self) -> None:
{"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict
)

# Directly requesting the edit should not have the edit to the edit applied.
channel = self.make_request(
"GET",
f"/rooms/{self.room}/event/{edit_event_id}",
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
self.assertEqual("Wibble", channel.json_body["content"]["body"])
self.assertIn("m.new_content", channel.json_body["content"])

# The relations information should not include the edit to the edit.
self.assertNotIn("m.relations", channel.json_body["unsigned"])

def test_unknown_relations(self) -> None:
"""Unknown relations should be accepted."""
channel = self._send_relation("m.relation.test", "m.room.test")
Expand Down Expand Up @@ -984,6 +997,24 @@ def assert_annotations(bundled_aggregations: JsonDict) -> None:

self._test_bundled_aggregations(RelationTypes.ANNOTATION, assert_annotations, 7)

def test_annotation_to_annotation(self) -> None:
"""Any relation to an annotation should be ignored."""
channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")
event_id = channel.json_body["event_id"]
self._send_relation(
RelationTypes.ANNOTATION, "m.reaction", "b", parent_id=event_id
)

# Fetch the initial annotation event to see if it has bundled aggregations.
channel = self.make_request(
"GET",
f"/_matrix/client/v3/rooms/{self.room}/event/{event_id}",
access_token=self.user_token,
)
self.assertEquals(200, channel.code, channel.json_body)
# The first annotationt should not have any bundled aggregations.
self.assertNotIn("m.relations", channel.json_body["unsigned"])

def test_reference(self) -> None:
"""
Test that references get correctly bundled.
Expand Down