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

Fixes a regression from 8b309adb436c162510ed1402f33b8741d71fc058 (#11660)
and b65acead428653b988351ae8d7b22127a22039cd (#11752) where events which
themselves were an edit or an annotation could have bundled aggregations calculated,
which is not allowed.

3 files changed, 50 insertions, 20 deletions

diff --git a/changelog.d/12633.bugfix b/changelog.d/12633.bugfix
new file mode 100644
index 0000000000..32332acd9a
--- /dev/null
+++ b/changelog.d/12633.bugfix
@@ -0,0 +1 @@
+Fix a bug introduced in Synapse v1.53.0 where bundled aggregations for annotations/edits were incorrectly calculated.
diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py
index cec5740fbd..c2754ec918 100644
--- a/synapse/handlers/relations.py
+++ b/synapse/handlers/relations.py
@@ -364,21 +364,29 @@ class RelationsHandler:
             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] = {}
@@ -413,16 +421,6 @@ class RelationsHandler:
 
         # 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
diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py
index 33ce9471b3..27dee8f697 100644
--- a/tests/rest/client/test_relations.py
+++ b/tests/rest/client/test_relations.py
@@ -620,6 +620,19 @@ class RelationsTestCase(BaseRelationsTestCase):
             {"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")
@@ -984,6 +997,24 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
 
         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.