Include bundled aggregations for the latest event in a thread

changelog.d/12273.bugfix

@@ -0,0 +1 @@
+Fix a bug introduced in Synapse v1.48.0 the latest event provided in bundled aggregations where it did not include bundled aggregations.

synapse/events/utils.py

@@ -32,13 +32,13 @@
 from synapse.api.constants import EventContentFields, EventTypes, RelationTypes
 from synapse.api.errors import Codes, SynapseError
 from synapse.api.room_versions import RoomVersion
+from synapse.handlers.relations import BundledAggregations
 from synapse.types import JsonDict
 from synapse.util.frozenutils import unfreeze
 
 from . import EventBase
 
 if TYPE_CHECKING:
-    from synapse.handlers.relations import BundledAggregations
     from synapse.server import HomeServer
 
 
@@ -428,13 +428,12 @@ def serialize_event(
 
         # Check if there are any bundled aggregations to include with the event.
         if bundle_aggregations:
-            event_aggregations = bundle_aggregations.get(event.event_id)
-            if event_aggregations:
+            if event.event_id in bundle_aggregations:
                 self._inject_bundled_aggregations(
                     event,
                     time_now,
                     config,
-                    bundle_aggregations[event.event_id],
+                    bundle_aggregations,
                     serialized_event,
                 )
 
@@ -472,7 +471,7 @@ def _inject_bundled_aggregations(
         event: EventBase,
         time_now: int,
         config: SerializeEventConfig,
-        aggregations: "BundledAggregations",
+        bundled_aggregations: Dict[str, "BundledAggregations"],
         serialized_event: JsonDict,
     ) -> None:
         """Potentially injects bundled aggregations into the unsigned portion of the serialized event.
@@ -487,15 +486,22 @@ def _inject_bundled_aggregations(
         """
         serialized_aggregations = {}
 
-        if aggregations.annotations:
-            serialized_aggregations[RelationTypes.ANNOTATION] = aggregations.annotations
+        # The aggregations must exist for this event.
+        event_aggregations = bundled_aggregations[event.event_id]
 
-        if aggregations.references:
-            serialized_aggregations[RelationTypes.REFERENCE] = aggregations.references
+        if event_aggregations.annotations:
+            serialized_aggregations[
+                RelationTypes.ANNOTATION
+            ] = event_aggregations.annotations
 
-        if aggregations.replace:
+        if event_aggregations.references:
+            serialized_aggregations[
+                RelationTypes.REFERENCE
+            ] = event_aggregations.references
+
+        if event_aggregations.replace:
             # If there is an edit, apply it to the event.
-            edit = aggregations.replace
+            edit = event_aggregations.replace
             self._apply_edit(event, serialized_event, edit)
 
             # Include information about it in the relations dict.
@@ -506,18 +512,16 @@ def _inject_bundled_aggregations(
             }
 
         # If this event is the start of a thread, include a summary of the replies.
-        if aggregations.thread:
-            thread = aggregations.thread
+        if event_aggregations.thread:
+            thread = event_aggregations.thread
 
             # Don't bundle aggregations as this could recurse forever.
-            serialized_latest_event = serialize_event(
-                thread.latest_event, time_now, config=config
+            serialized_latest_event = self.serialize_event(
+                thread.latest_event,
+                time_now,
+                config=config,
+                bundle_aggregations=bundled_aggregations,
             )
-            # Manually apply an edit, if one exists.
-            if thread.latest_edit:
-                self._apply_edit(
-                    thread.latest_event, serialized_latest_event, thread.latest_edit
-                )
 
             thread_summary = {
                 "latest_event": serialized_latest_event,

synapse/handlers/relations.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-from typing import TYPE_CHECKING, Dict, Iterable, Optional, cast
+from typing import TYPE_CHECKING, Dict, Iterable, Optional, Tuple, cast
 
 import attr
 from frozendict import frozendict
@@ -34,8 +34,6 @@
 class _ThreadAggregation:
     # The latest event in the thread.
     latest_event: EventBase
-    # The latest edit to the latest event in the thread.
-    latest_edit: Optional[EventBase]
     # The total number of events in the thread.
     count: int
     # True if the current user has sent an event to the thread.
@@ -153,7 +151,7 @@ async def get_relations(
 
     async def _get_bundled_aggregation_for_event(
         self, event: EventBase, user_id: str
-    ) -> Optional[BundledAggregations]:
+    ) -> Tuple[Optional[JsonDict], Optional[JsonDict]]:
         """Generate bundled aggregations for an event.
 
         Note that this does not use a cache, but depends on cached methods.
@@ -173,32 +171,27 @@ async def _get_bundled_aggregation_for_event(
         if isinstance(relates_to, (dict, frozendict)):
             relation_type = relates_to.get("rel_type")
             if relation_type in (RelationTypes.ANNOTATION, RelationTypes.REPLACE):
-                return None
+                return None, None
 
         event_id = event.event_id
         room_id = event.room_id
 
-        # The bundled aggregations to include, a mapping of relation type to a
-        # type-specific value. Some types include the direct return type here
-        # while others need more processing during serialization.
-        aggregations = BundledAggregations()
-
         annotations = await self._main_store.get_aggregation_groups_for_event(
             event_id, room_id
         )
+        serialized_annotations = None
         if annotations.chunk:
-            aggregations.annotations = await annotations.to_dict(
-                cast("DataStore", self)
-            )
+            serialized_annotations = await annotations.to_dict(cast("DataStore", self))
 
         references = await self._main_store.get_relations_for_event(
             event_id, event, room_id, RelationTypes.REFERENCE, direction="f"
         )
+        serialized_references = None
         if references.chunk:
-            aggregations.references = await references.to_dict(cast("DataStore", self))
+            serialized_references = await references.to_dict(cast("DataStore", self))
 
         # Store the bundled aggregations in the event metadata for later use.
-        return aggregations
+        return serialized_annotations, serialized_references
 
     async def get_bundled_aggregations(
         self, events: Iterable[EventBase], user_id: str
@@ -210,8 +203,13 @@ async def get_bundled_aggregations(
             user_id: The user requesting the bundled aggregations.
 
         Returns:
-            A map of event ID to the bundled aggregation for the event. Not all
-            events may have bundled aggregations in the results.
+            A map of event ID to the bundled aggregation for the event.
+
+            Not all requested events may exist in the results (if they don't have
+            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.
         #
@@ -223,42 +221,61 @@ async def get_bundled_aggregations(
         # event ID -> bundled aggregation in non-serialized form.
         results: Dict[str, BundledAggregations] = {}
 
-        # Fetch other relations per event.
-        for event in events_by_id.values():
-            event_result = await self._get_bundled_aggregation_for_event(event, user_id)
-            if event_result:
-                results[event.event_id] = event_result
-
-        # Fetch any edits (but not for redacted events).
-        edits = await self._main_store.get_applicable_edits(
-            [
-                event_id
-                for event_id, event in events_by_id.items()
-                if not event.internal_metadata.is_redacted()
-            ]
-        )
-        for event_id, edit in edits.items():
-            results.setdefault(event_id, BundledAggregations()).replace = edit
+        # Threads are special as the latest event of a thread might cause additional
+        # events to be fetched. Thus, we check those first!
 
-        # Fetch thread summaries.
+        # Fetch thread summaries (but only for the directly requested events).
+        #
+        # Note that you can't have threads off of other related events, but it is
+        # possible for a malicious homeserver to inject them anyway.
         summaries = await self._main_store.get_thread_summaries(events_by_id.keys())
         # Only fetch participated for a limited selection based on what had
         # summaries.
         participated = await self._main_store.get_threads_participated(
-            [event_id for event_id, summary in summaries.items() if summary], user_id
+            [event_id for event_id, summary in summaries.items() if summary],
+            user_id,
         )
         for event_id, summary in summaries.items():
             if summary:
-                thread_count, latest_thread_event, edit = summary
+                thread_count, latest_thread_event = summary
+
+                # If the latest event in a thread is not already being fetched,
+                # add it to the events. (Note that we don't mo
+                if (
+                    latest_thread_event
+                    and latest_thread_event.event_id not in events_by_id
+                ):
+                    events_by_id[latest_thread_event.event_id] = latest_thread_event
+
                 results.setdefault(
                     event_id, BundledAggregations()
                 ).thread = _ThreadAggregation(
                     latest_event=latest_thread_event,
-                    latest_edit=edit,
                     count=thread_count,
                     # If there's a thread summary it must also exist in the
                     # participated dictionary.
                     current_user_participated=participated[event_id],
                 )
 
+        # Fetch other relations per event.
+        for event in events_by_id.values():
+            annotations, references = await self._get_bundled_aggregation_for_event(
+                event, user_id
+            )
+            if annotations or references:
+                aggregations = results.setdefault(event.event_id, BundledAggregations())
+                aggregations.annotations = annotations
+                aggregations.references = references
+
+        # Fetch any edits (but not for redacted events).
+        edits = await self._main_store.get_applicable_edits(
+            [
+                event_id
+                for event_id, event in events_by_id.items()
+                if not event.internal_metadata.is_redacted()
+            ]
+        )
+        for event_id, edit in edits.items():
+            results.setdefault(event_id, BundledAggregations()).replace = edit
+
         return results

synapse/storage/databases/main/relations.py

@@ -444,7 +444,7 @@ def get_thread_summary(self, event_id: str) -> Optional[Tuple[int, EventBase]]:
     @cachedList(cached_method_name="get_thread_summary", list_name="event_ids")
     async def get_thread_summaries(
         self, event_ids: Collection[str]
-    ) -> Dict[str, Optional[Tuple[int, EventBase, Optional[EventBase]]]]:
+    ) -> Dict[str, Optional[Tuple[int, EventBase]]]:
         """Get the number of threaded replies, the latest reply (if any), and the latest edit for that reply for the given event.
 
         Args:
@@ -457,7 +457,6 @@ async def get_thread_summaries(
             Each summary is a tuple of:
                 The number of events in the thread.
                 The most recent event in the thread.
-                The most recent edit to the most recent event in the thread, if applicable.
         """
 
         def _get_thread_summaries_txn(
@@ -555,9 +554,6 @@ def _get_thread_summaries_txn(
 
         latest_events = await self.get_events(latest_event_ids.values())  # type: ignore[attr-defined]
 
-        # Check to see if any of those events are edited.
-        latest_edits = await self.get_applicable_edits(latest_event_ids.values())
-
         # Map to the event IDs to the thread summary.
         #
         # There might not be a summary due to there not being a thread or
@@ -568,8 +564,7 @@ def _get_thread_summaries_txn(
 
             summary = None
             if latest_event:
-                latest_edit = latest_edits.get(latest_event_id)
-                summary = (counts[parent_event_id], latest_event, latest_edit)
+                summary = (counts[parent_event_id], latest_event)
             summaries[parent_event_id] = summary
 
         return summaries