From b8eae59cdce40b3a270ffaf15c2fcb570d64d302 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 09:58:49 -0500
Subject: [PATCH 01/13] Rename bundle_relations to bundle_aggregations.

---
 synapse/events/utils.py          | 38 ++++++++++++++++----------------
 synapse/handlers/events.py       |  2 +-
 synapse/handlers/message.py      |  2 +-
 synapse/rest/admin/rooms.py      |  4 ++--
 synapse/rest/client/relations.py |  6 ++---
 synapse/rest/client/room.py      |  2 +-
 synapse/rest/client/sync.py      |  2 +-
 7 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 05219a9dd03a..f340166cc58d 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -393,7 +393,7 @@ async def serialize_event(
         self,
         event: Union[JsonDict, EventBase],
         time_now: int,
-        bundle_relations: bool = True,
+        bundle_aggregations: bool = True,
         **kwargs: Any,
     ) -> JsonDict:
         """Serializes a single event.
@@ -401,7 +401,7 @@ async def serialize_event(
         Args:
             event: The event being serialized.
             time_now: The current time in milliseconds
-            bundle_relations: Whether to include the bundled relations for this
+            bundle_aggregations: Whether to include the bundled aggregations for this
                 event.
             **kwargs: Arguments to pass to `serialize_event`
 
@@ -416,18 +416,18 @@ async def serialize_event(
 
         # If MSC1849 is enabled then we need to look if there are any relations
         # we need to bundle in with the event.
-        # Do not bundle relations if the event has been redacted
+        # Do not bundle aggregations if the event has been redacted
         if not event.internal_metadata.is_redacted() and (
-            self._msc1849_enabled and bundle_relations
+            self._msc1849_enabled and bundle_aggregations
         ):
-            await self._injected_bundled_relations(event, time_now, serialized_event)
+            await self._injected_bundled_aggregations(event, time_now, serialized_event)
 
         return serialized_event
 
-    async def _injected_bundled_relations(
+    async def _injected_bundled_aggregations(
         self, event: EventBase, time_now: int, serialized_event: JsonDict
     ) -> None:
-        """Potentially injects bundled relations into the unsigned portion of the serialized event.
+        """Potentially injects bundled aggregations into the unsigned portion of the serialized event.
 
         Args:
             event: The event being serialized.
@@ -435,7 +435,7 @@ async def _injected_bundled_relations(
             serialized_event: The serialized event which may be modified.
 
         """
-        # Do not bundle relations for an event which represents an edit or an
+        # Do not bundle aggregations for an event which represents an edit or an
         # annotation. It does not make sense for them to have related events.
         relates_to = event.content.get("m.relates_to")
         if isinstance(relates_to, (dict, frozendict)):
@@ -445,18 +445,18 @@ async def _injected_bundled_relations(
 
         event_id = event.event_id
 
-        # The bundled relations to include.
-        relations = {}
+        # The bundled aggregations to include.
+        aggregations = {}
 
         annotations = await self.store.get_aggregation_groups_for_event(event_id)
         if annotations.chunk:
-            relations[RelationTypes.ANNOTATION] = annotations.to_dict()
+            aggregations[RelationTypes.ANNOTATION] = annotations.to_dict()
 
         references = await self.store.get_relations_for_event(
             event_id, RelationTypes.REFERENCE, direction="f"
         )
         if references.chunk:
-            relations[RelationTypes.REFERENCE] = references.to_dict()
+            aggregations[RelationTypes.REFERENCE] = references.to_dict()
 
         edit = None
         if event.type == EventTypes.Message:
@@ -482,7 +482,7 @@ async def _injected_bundled_relations(
             else:
                 serialized_event["content"].pop("m.relates_to", None)
 
-            relations[RelationTypes.REPLACE] = {
+            aggregations[RelationTypes.REPLACE] = {
                 "event_id": edit.event_id,
                 "origin_server_ts": edit.origin_server_ts,
                 "sender": edit.sender,
@@ -495,17 +495,17 @@ async def _injected_bundled_relations(
                 latest_thread_event,
             ) = await self.store.get_thread_summary(event_id)
             if latest_thread_event:
-                relations[RelationTypes.THREAD] = {
-                    # Don't bundle relations as this could recurse forever.
+                aggregations[RelationTypes.THREAD] = {
+                    # Don't bundle aggregations as this could recurse forever.
                     "latest_event": await self.serialize_event(
-                        latest_thread_event, time_now, bundle_relations=False
+                        latest_thread_event, time_now, bundle_aggregations=False
                     ),
                     "count": thread_count,
                 }
 
-        # If any bundled relations were found, include them.
-        if relations:
-            serialized_event["unsigned"].setdefault("m.relations", {}).update(relations)
+        # If any bundled aggregations were found, include them.
+        if aggregations:
+            serialized_event["unsigned"].setdefault("m.relations", {}).update(aggregations)
 
     async def serialize_events(
         self, events: Iterable[Union[JsonDict, EventBase]], time_now: int, **kwargs: Any
diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py
index b4ff935546c8..1f64534a8a78 100644
--- a/synapse/handlers/events.py
+++ b/synapse/handlers/events.py
@@ -124,7 +124,7 @@ async def get_stream(
                 as_client_event=as_client_event,
                 # We don't bundle "live" events, as otherwise clients
                 # will end up double counting annotations.
-                bundle_relations=False,
+                bundle_aggregations=False,
             )
 
             chunk = {
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 95b4fad3c68b..22dd4cf5fd3f 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -252,7 +252,7 @@ async def get_state_events(
             now,
             # We don't bother bundling aggregations in when asked for state
             # events, as clients won't use them.
-            bundle_relations=False,
+            bundle_aggregations=False,
         )
         return events
 
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 6bbc5510f070..300a84f64344 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -454,7 +454,7 @@ async def on_GET(
             now,
             # We don't bother bundling aggregations in when asked for state
             # events, as clients won't use them.
-            bundle_relations=False,
+            bundle_aggregations=False,
         )
         ret = {"state": room_state}
 
@@ -792,7 +792,7 @@ async def on_GET(
             results["state"],
             time_now,
             # No need to bundle aggregations for state events
-            bundle_relations=False,
+            bundle_aggregations=False,
         )
 
         return HTTPStatus.OK, results
diff --git a/synapse/rest/client/relations.py b/synapse/rest/client/relations.py
index b1a3304849c6..ae954c571f7d 100644
--- a/synapse/rest/client/relations.py
+++ b/synapse/rest/client/relations.py
@@ -224,14 +224,14 @@ async def on_GET(
         )
 
         now = self.clock.time_msec()
-        # We set bundle_relations to False when retrieving the original
+        # We set bundle_aggregations to False when retrieving the original
         # event because we want the content before relations were applied to
         # it.
         original_event = await self._event_serializer.serialize_event(
-            event, now, bundle_relations=False
+            event, now, bundle_aggregations=False
         )
         # The relations returned for the requested event do include their
-        # bundled relations.
+        # bundled aggregations.
         serialized_events = await self._event_serializer.serialize_events(events, now)
 
         return_value = pagination_chunk.to_dict()
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 99f303c88e34..ea9889936325 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -719,7 +719,7 @@ async def on_GET(
             results["state"],
             time_now,
             # No need to bundle aggregations for state events
-            bundle_relations=False,
+            bundle_aggregations=False,
         )
 
         return 200, results
diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index b6a24857320b..8c0fdb194004 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -522,7 +522,7 @@ def serialize(events: Iterable[EventBase]) -> Awaitable[List[JsonDict]]:
                 time_now=time_now,
                 # We don't bundle "live" events, as otherwise clients
                 # will end up double counting annotations.
-                bundle_relations=False,
+                bundle_aggregations=False,
                 token_id=token_id,
                 event_format=event_formatter,
                 only_event_fields=only_fields,

From 278321d22967ad66c63f4ffad511efeddca907be Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 10:11:49 -0500
Subject: [PATCH 02/13] Clarify comments about when to not bundle aggregations.

---
 synapse/handlers/events.py       | 3 +--
 synapse/handlers/message.py      | 3 +--
 synapse/rest/admin/rooms.py      | 5 ++---
 synapse/rest/client/relations.py | 5 ++---
 synapse/rest/client/room.py      | 2 +-
 5 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py
index 1f64534a8a78..32b0254c5f08 100644
--- a/synapse/handlers/events.py
+++ b/synapse/handlers/events.py
@@ -122,8 +122,7 @@ async def get_stream(
                 events,
                 time_now,
                 as_client_event=as_client_event,
-                # We don't bundle "live" events, as otherwise clients
-                # will end up double counting annotations.
+                # Don't bundle aggregations as this is a deprecated API.
                 bundle_aggregations=False,
             )
 
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 22dd4cf5fd3f..e77a7c721ea4 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -250,8 +250,7 @@ async def get_state_events(
         events = await self._event_serializer.serialize_events(
             room_state.values(),
             now,
-            # We don't bother bundling aggregations in when asked for state
-            # events, as clients won't use them.
+            # No need to bundle aggregations for state events.
             bundle_aggregations=False,
         )
         return events
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 300a84f64344..89716bf8140d 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -452,8 +452,7 @@ async def on_GET(
         room_state = await self._event_serializer.serialize_events(
             events.values(),
             now,
-            # We don't bother bundling aggregations in when asked for state
-            # events, as clients won't use them.
+            # No need to bundle aggregations for state events.
             bundle_aggregations=False,
         )
         ret = {"state": room_state}
@@ -791,7 +790,7 @@ async def on_GET(
         results["state"] = await self._event_serializer.serialize_events(
             results["state"],
             time_now,
-            # No need to bundle aggregations for state events
+            # No need to bundle aggregations for state events.
             bundle_aggregations=False,
         )
 
diff --git a/synapse/rest/client/relations.py b/synapse/rest/client/relations.py
index ae954c571f7d..fc4e6921c5e6 100644
--- a/synapse/rest/client/relations.py
+++ b/synapse/rest/client/relations.py
@@ -224,9 +224,8 @@ async def on_GET(
         )
 
         now = self.clock.time_msec()
-        # We set bundle_aggregations to False when retrieving the original
-        # event because we want the content before relations were applied to
-        # it.
+        # Do not bundle aggregations when retrieving the original event because
+        # we want the content before relations are applied to it.
         original_event = await self._event_serializer.serialize_event(
             event, now, bundle_aggregations=False
         )
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index ea9889936325..e89121024ac7 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -718,7 +718,7 @@ async def on_GET(
         results["state"] = await self._event_serializer.serialize_events(
             results["state"],
             time_now,
-            # No need to bundle aggregations for state events
+            # No need to bundle aggregations for state events.
             bundle_aggregations=False,
         )
 

From 6e0475d99604fe617496c3836cac42fe33fc4e0f Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 10:12:05 -0500
Subject: [PATCH 03/13] Bundle aggregations for limited /sync requests.

---
 synapse/rest/client/sync.py | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 8c0fdb194004..15989181964d 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -516,13 +516,13 @@ async def encode_room(
             The room, encoded in our response format
         """
 
-        def serialize(events: Iterable[EventBase]) -> Awaitable[List[JsonDict]]:
+        def serialize(
+            events: Iterable[EventBase], bundle_aggregations: bool
+        ) -> Awaitable[List[JsonDict]]:
             return self._event_serializer.serialize_events(
                 events,
                 time_now=time_now,
-                # We don't bundle "live" events, as otherwise clients
-                # will end up double counting annotations.
-                bundle_aggregations=False,
+                bundle_aggregations=bundle_aggregations,
                 token_id=token_id,
                 event_format=event_formatter,
                 only_event_fields=only_fields,
@@ -544,8 +544,13 @@ def serialize(events: Iterable[EventBase]) -> Awaitable[List[JsonDict]]:
                     event.room_id,
                 )
 
-        serialized_state = await serialize(state_events)
-        serialized_timeline = await serialize(timeline_events)
+        # No need to bundle aggregations for state events.
+        serialized_state = await serialize(state_events, bundle_aggregations=False)
+        # Only bundle aggregations if the room is limited, as clients could be
+        # missing events.
+        serialized_timeline = await serialize(
+            timeline_events, bundle_aggregations=not room.timeline.limited
+        )
 
         account_data = room.account_data
 

From 4a1e9212bca941548be0ee1acfcfc47f68574b8f Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 10:21:17 -0500
Subject: [PATCH 04/13] Do not bundle aggregations for initial_sync.

---
 synapse/events/utils.py          |  4 +++-
 synapse/handlers/initial_sync.py | 30 ++++++++++++++++++++++++------
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index f340166cc58d..8a85b6ff691c 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -505,7 +505,9 @@ async def _injected_bundled_aggregations(
 
         # If any bundled aggregations were found, include them.
         if aggregations:
-            serialized_event["unsigned"].setdefault("m.relations", {}).update(aggregations)
+            serialized_event["unsigned"].setdefault("m.relations", {}).update(
+                aggregations
+            )
 
     async def serialize_events(
         self, events: Iterable[Union[JsonDict, EventBase]], time_now: int, **kwargs: Any
diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py
index d4e45561555c..1aff7930a6f3 100644
--- a/synapse/handlers/initial_sync.py
+++ b/synapse/handlers/initial_sync.py
@@ -165,7 +165,11 @@ async def handle_room(event: RoomsForUser) -> None:
 
                 invite_event = await self.store.get_event(event.event_id)
                 d["invite"] = await self._event_serializer.serialize_event(
-                    invite_event, time_now, as_client_event
+                    invite_event,
+                    time_now,
+                    # No need to bundle aggregations for state events.
+                    bundle_aggregations=False,
+                    as_client_event=as_client_event,
                 )
 
             rooms_ret.append(d)
@@ -216,7 +220,11 @@ async def handle_room(event: RoomsForUser) -> None:
                 d["messages"] = {
                     "chunk": (
                         await self._event_serializer.serialize_events(
-                            messages, time_now=time_now, as_client_event=as_client_event
+                            messages,
+                            time_now=time_now,
+                            # Don't bundle aggregations as this is a deprecated API.
+                            bundle_aggregations=False,
+                            as_client_event=as_client_event,
                         )
                     ),
                     "start": await start_token.to_string(self.store),
@@ -226,6 +234,8 @@ async def handle_room(event: RoomsForUser) -> None:
                 d["state"] = await self._event_serializer.serialize_events(
                     current_state.values(),
                     time_now=time_now,
+                    # No need to bundle aggregations for state events.
+                    bundle_aggregations=False,
                     as_client_event=as_client_event,
                 )
 
@@ -366,14 +376,18 @@ async def _room_initial_sync_parted(
             "room_id": room_id,
             "messages": {
                 "chunk": (
-                    await self._event_serializer.serialize_events(messages, time_now)
+                    # Don't bundle aggregations as this is a deprecated API.
+                    await self._event_serializer.serialize_events(
+                        messages, time_now, bundle_aggregations=False
+                    )
                 ),
                 "start": await start_token.to_string(self.store),
                 "end": await end_token.to_string(self.store),
             },
             "state": (
+                # No need to bundle aggregations for state events.
                 await self._event_serializer.serialize_events(
-                    room_state.values(), time_now
+                    room_state.values(), time_now, bundle_aggregations=False
                 )
             ),
             "presence": [],
@@ -392,8 +406,9 @@ async def _room_initial_sync_joined(
 
         # TODO: These concurrently
         time_now = self.clock.time_msec()
+        # No need to bundle aggregations for state events.
         state = await self._event_serializer.serialize_events(
-            current_state.values(), time_now
+            current_state.values(), time_now, bundle_aggregations=False
         )
 
         now_token = self.hs.get_event_sources().get_current_token()
@@ -467,7 +482,10 @@ async def get_receipts() -> List[JsonDict]:
             "room_id": room_id,
             "messages": {
                 "chunk": (
-                    await self._event_serializer.serialize_events(messages, time_now)
+                    # Don't bundle aggregations as this is a deprecated API.
+                    await self._event_serializer.serialize_events(
+                        messages, time_now, bundle_aggregations=False
+                    )
                 ),
                 "start": await start_token.to_string(self.store),
                 "end": await end_token.to_string(self.store),

From d599b05a7ca686ca0d3eec90a93f9a51403703f0 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 10:26:32 -0500
Subject: [PATCH 05/13] Consistently do not bundle aggregations for state
 events.

---
 synapse/handlers/pagination.py | 6 +++++-
 synapse/handlers/search.py     | 3 ++-
 synapse/rest/client/sync.py    | 4 ++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py
index cd64142735de..d6e50ffa68d6 100644
--- a/synapse/handlers/pagination.py
+++ b/synapse/handlers/pagination.py
@@ -554,7 +554,11 @@ async def get_messages(
 
         if state:
             chunk["state"] = await self._event_serializer.serialize_events(
-                state, time_now, as_client_event=as_client_event
+                state,
+                time_now,
+                as_client_event=as_client_event,
+                # No need to bundle aggregations for state events.
+                bundle_aggregations=False,
             )
 
         return chunk
diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py
index ab7eaab2fb56..145396a60fed 100644
--- a/synapse/handlers/search.py
+++ b/synapse/handlers/search.py
@@ -457,8 +457,9 @@ async def search(
         if state_results:
             s = {}
             for room_id, state_events in state_results.items():
+                # No need to bundle aggregations for state events.
                 s[room_id] = await self._event_serializer.serialize_events(
-                    state_events, time_now
+                    state_events, time_now, bundle_aggregations=False
                 )
 
             rooms_cat_res["state"] = s
diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 15989181964d..291b0f4d92e3 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -391,6 +391,8 @@ async def encode_invited(
             invite = await self._event_serializer.serialize_event(
                 room.invite,
                 time_now,
+                # No need to bundle aggregations for state events.
+                bundle_aggregations=False,
                 token_id=token_id,
                 event_format=event_formatter,
                 include_stripped_room_state=True,
@@ -427,6 +429,8 @@ async def encode_knocked(
             knock = await self._event_serializer.serialize_event(
                 room.knock,
                 time_now,
+                # No need to bundle aggregations for state events.
+                bundle_aggregations=False,
                 token_id=token_id,
                 event_format=event_formatter,
                 include_stripped_room_state=True,

From 313e272dc60db5226a6e48962ac853dd8a575d1b Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 1 Dec 2021 11:29:51 -0500
Subject: [PATCH 06/13] Newsfragment

---
 changelog.d/11478.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/11478.bugfix

diff --git a/changelog.d/11478.bugfix b/changelog.d/11478.bugfix
new file mode 100644
index 000000000000..0ed5622cb48a
--- /dev/null
+++ b/changelog.d/11478.bugfix
@@ -0,0 +1 @@
+Include bundled relations during a limited `/sync` request, per [MSC2675](/~https://github.com/matrix-org/matrix-doc/pull/2675).

From a48bd2ad88a1685633fdccf735ac27bf489cecb1 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 2 Dec 2021 14:23:30 -0500
Subject: [PATCH 07/13] Never inlude bundled aggregations for state events.

---
 synapse/events/utils.py          | 10 +++++++---
 synapse/handlers/initial_sync.py |  2 --
 synapse/rest/client/sync.py      |  4 ----
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 8a85b6ff691c..84968ce6ff7c 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -416,9 +416,13 @@ async def serialize_event(
 
         # If MSC1849 is enabled then we need to look if there are any relations
         # we need to bundle in with the event.
-        # Do not bundle aggregations if the event has been redacted
-        if not event.internal_metadata.is_redacted() and (
-            self._msc1849_enabled and bundle_aggregations
+        # Do not bundle aggregations if the event has been redacted or if the event
+        # is a state event.
+        if (
+            self._msc1849_enabled
+            and bundle_aggregations
+            and not event.is_state()
+            and not event.internal_metadata.is_redacted()
         ):
             await self._injected_bundled_aggregations(event, time_now, serialized_event)
 
diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py
index 1aff7930a6f3..133817264ef5 100644
--- a/synapse/handlers/initial_sync.py
+++ b/synapse/handlers/initial_sync.py
@@ -167,8 +167,6 @@ async def handle_room(event: RoomsForUser) -> None:
                 d["invite"] = await self._event_serializer.serialize_event(
                     invite_event,
                     time_now,
-                    # No need to bundle aggregations for state events.
-                    bundle_aggregations=False,
                     as_client_event=as_client_event,
                 )
 
diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 291b0f4d92e3..15989181964d 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -391,8 +391,6 @@ async def encode_invited(
             invite = await self._event_serializer.serialize_event(
                 room.invite,
                 time_now,
-                # No need to bundle aggregations for state events.
-                bundle_aggregations=False,
                 token_id=token_id,
                 event_format=event_formatter,
                 include_stripped_room_state=True,
@@ -429,8 +427,6 @@ async def encode_knocked(
             knock = await self._event_serializer.serialize_event(
                 room.knock,
                 time_now,
-                # No need to bundle aggregations for state events.
-                bundle_aggregations=False,
                 token_id=token_id,
                 event_format=event_formatter,
                 include_stripped_room_state=True,

From a11ef5bc18d2ae3100e0bda67fdb8af907d53749 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 2 Dec 2021 15:12:07 -0500
Subject: [PATCH 08/13] Add tests.

---
 synapse/rest/client/sync.py         |   2 +-
 tests/rest/client/test_relations.py | 135 ++++++++++++++++++++--------
 2 files changed, 97 insertions(+), 40 deletions(-)

diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 15989181964d..803cb9bca474 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -549,7 +549,7 @@ def serialize(
         # Only bundle aggregations if the room is limited, as clients could be
         # missing events.
         serialized_timeline = await serialize(
-            timeline_events, bundle_aggregations=not room.timeline.limited
+            timeline_events, bundle_aggregations=room.timeline.limited
         )
 
         account_data = room.account_data
diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py
index b494da51387b..397c12c2a6c5 100644
--- a/tests/rest/client/test_relations.py
+++ b/tests/rest/client/test_relations.py
@@ -19,7 +19,7 @@
 
 from synapse.api.constants import EventTypes, RelationTypes
 from synapse.rest import admin
-from synapse.rest.client import login, register, relations, room
+from synapse.rest.client import login, register, relations, room, sync
 
 from tests import unittest
 from tests.server import FakeChannel
@@ -29,6 +29,7 @@ class RelationsTestCase(unittest.HomeserverTestCase):
     servlets = [
         relations.register_servlets,
         room.register_servlets,
+        sync.register_servlets,
         login.register_servlets,
         register.register_servlets,
         admin.register_servlets_for_client_rest_resource,
@@ -454,11 +455,9 @@ def test_aggregation_must_be_annotation(self):
         self.assertEquals(400, channel.code, channel.json_body)
 
     @unittest.override_config({"experimental_features": {"msc3440_enabled": True}})
-    def test_aggregation_get_event(self):
-        """Test that annotations, references, and threads get correctly bundled when
-        getting the parent event.
-        """
-
+    def test_bundled_aggregations(self):
+        """Test that annotations, references, and threads get correctly bundled."""
+        # Setup by sending a variety of relations.
         channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")
         self.assertEquals(200, channel.code, channel.json_body)
 
@@ -485,49 +484,107 @@ def test_aggregation_get_event(self):
         self.assertEquals(200, channel.code, channel.json_body)
         thread_2 = channel.json_body["event_id"]
 
-        channel = self.make_request(
-            "GET",
-            "/rooms/%s/event/%s" % (self.room, self.parent_id),
-            access_token=self.user_token,
-        )
-        self.assertEquals(200, channel.code, channel.json_body)
+        def assert_bundle(actual):
+            """Assert the expected values of the bundled aggregations."""
 
-        self.assertEquals(
-            channel.json_body["unsigned"].get("m.relations"),
-            {
-                RelationTypes.ANNOTATION: {
+            # Ensure the fields are as expected.
+            self.assertCountEqual(
+                actual.keys(),
+                (
+                    RelationTypes.ANNOTATION,
+                    RelationTypes.REFERENCE,
+                    RelationTypes.THREAD,
+                ),
+            )
+
+            # Check the values of each field.
+            self.assertEquals(
+                {
                     "chunk": [
                         {"type": "m.reaction", "key": "a", "count": 2},
                         {"type": "m.reaction", "key": "b", "count": 1},
                     ]
                 },
-                RelationTypes.REFERENCE: {
-                    "chunk": [{"event_id": reply_1}, {"event_id": reply_2}]
-                },
-                RelationTypes.THREAD: {
-                    "count": 2,
-                    "latest_event": {
-                        "age": 100,
-                        "content": {
-                            "m.relates_to": {
-                                "event_id": self.parent_id,
-                                "rel_type": RelationTypes.THREAD,
-                            }
-                        },
-                        "event_id": thread_2,
-                        "origin_server_ts": 1600,
-                        "room_id": self.room,
-                        "sender": self.user_id,
-                        "type": "m.room.test",
-                        "unsigned": {"age": 100},
-                        "user_id": self.user_id,
+                actual[RelationTypes.ANNOTATION],
+            )
+
+            self.assertEquals(
+                {"chunk": [{"event_id": reply_1}, {"event_id": reply_2}]},
+                actual[RelationTypes.REFERENCE],
+            )
+
+            self.assertEquals(
+                2,
+                actual[RelationTypes.THREAD].get("count"),
+            )
+            # The latest thread event has some fields that don't matter.
+            self.assert_dict(
+                {
+                    "content": {
+                        "m.relates_to": {
+                            "event_id": self.parent_id,
+                            "rel_type": RelationTypes.THREAD,
+                        }
                     },
+                    "event_id": thread_2,
+                    "room_id": self.room,
+                    "sender": self.user_id,
+                    "type": "m.room.test",
+                    "user_id": self.user_id,
                 },
-            },
+                actual[RelationTypes.THREAD].get("latest_event"),
+            )
+
+        def _find_and_assert_event(events):
+            """
+            Find the parent event in a chunk of events and assert that it has the proper bundled aggregations.
+            """
+            for event in events:
+                if event["event_id"] == self.parent_id:
+                    break
+            else:
+                raise AssertionError(f"Event {self.parent_id} not found in chunk")
+            assert_bundle(event["unsigned"].get("m.relations"))
+
+        # Request the event directly.
+        channel = self.make_request(
+            "GET",
+            f"/rooms/{self.room}/event/{self.parent_id}",
+            access_token=self.user_token,
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+        assert_bundle(channel.json_body["unsigned"].get("m.relations"))
+
+        # Request the room messages.
+        channel = self.make_request(
+            "GET",
+            f"/rooms/{self.room}/messages?dir=b",
+            access_token=self.user_token,
         )
+        self.assertEquals(200, channel.code, channel.json_body)
+        _find_and_assert_event(channel.json_body["chunk"])
+
+        # Request the room context.
+        channel = self.make_request(
+            "GET",
+            f"/rooms/{self.room}/context/{self.parent_id}",
+            access_token=self.user_token,
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+        assert_bundle(channel.json_body["event"]["unsigned"].get("m.relations"))
+
+        # Request sync.
+        channel = self.make_request("GET", "/sync", access_token=self.user_token)
+        self.assertEquals(200, channel.code, channel.json_body)
+        room_timeline = channel.json_body["rooms"]["join"][self.room]["timeline"]
+        self.assertTrue(room_timeline["limited"])
+        _find_and_assert_event(room_timeline["events"])
+
+        # Note that /relations is tested separately in test_aggregation_get_event_for_thread
+        # since it needs different data configured.
 
     def test_aggregation_get_event_for_annotation(self):
-        """Test that annotations do not get bundled relations included
+        """Test that annotations do not get bundled aggregations included
         when directly requested.
         """
         channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")
@@ -549,7 +606,7 @@ def test_aggregation_get_event_for_annotation(self):
         self.assertIsNone(channel.json_body["unsigned"].get("m.relations"))
 
     def test_aggregation_get_event_for_thread(self):
-        """Test that threads get bundled relations included when directly requested."""
+        """Test that threads get bundled aggregations included when directly requested."""
         channel = self._send_relation(RelationTypes.THREAD, "m.room.test")
         self.assertEquals(200, channel.code, channel.json_body)
         thread_id = channel.json_body["event_id"]

From b7359c78babd613a40fb22f86a239ff865850aaa Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Fri, 3 Dec 2021 10:43:50 -0500
Subject: [PATCH 09/13] Clarify comments / newsfile.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 changelog.d/11478.bugfix | 2 +-
 synapse/events/utils.py  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/changelog.d/11478.bugfix b/changelog.d/11478.bugfix
index 0ed5622cb48a..5f02636f50e8 100644
--- a/changelog.d/11478.bugfix
+++ b/changelog.d/11478.bugfix
@@ -1 +1 @@
-Include bundled relations during a limited `/sync` request, per [MSC2675](/~https://github.com/matrix-org/matrix-doc/pull/2675).
+Include bundled relation aggregations during a limited `/sync` request, per [MSC2675](/~https://github.com/matrix-org/matrix-doc/pull/2675).
diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 84968ce6ff7c..476d67a581b4 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -414,8 +414,8 @@ async def serialize_event(
 
         serialized_event = serialize_event(event, time_now, **kwargs)
 
-        # If MSC1849 is enabled then we need to look if there are any relations
-        # we need to bundle in with the event.
+        # If MSC1849 is enabled then we need to look if there are any relation
+        # aggregations we need to bundle in with the event.
         # Do not bundle aggregations if the event has been redacted or if the event
         # is a state event.
         if (

From 2f409daa8cb1834191bd5632ad3feab1584846df Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Fri, 3 Dec 2021 11:10:17 -0500
Subject: [PATCH 10/13] Require args with defaults to be passed as kw-only.

---
 synapse/events/utils.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 476d67a581b4..bffcb65df19b 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -306,6 +306,7 @@ def format_event_for_client_v2_without_room_id(d: JsonDict) -> JsonDict:
 def serialize_event(
     e: Union[JsonDict, EventBase],
     time_now_ms: int,
+    *,
     as_client_event: bool = True,
     event_format: Callable[[JsonDict], JsonDict] = format_event_for_client_v1,
     token_id: Optional[str] = None,
@@ -393,6 +394,7 @@ async def serialize_event(
         self,
         event: Union[JsonDict, EventBase],
         time_now: int,
+        *,
         bundle_aggregations: bool = True,
         **kwargs: Any,
     ) -> JsonDict:

From e98973d3f51b503b5c44f25965292770b619097f Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Mon, 6 Dec 2021 08:04:05 -0500
Subject: [PATCH 11/13] Clarify comment.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 synapse/rest/client/sync.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 803cb9bca474..da16cd7e6b67 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -546,8 +546,8 @@ def serialize(
 
         # No need to bundle aggregations for state events.
         serialized_state = await serialize(state_events, bundle_aggregations=False)
-        # Only bundle aggregations if the room is limited, as clients could be
-        # missing events.
+        # Don't bother to bundle aggregations if the timeline is unlimited, as
+        # clients will have all the necessary information.
         serialized_timeline = await serialize(
             timeline_events, bundle_aggregations=room.timeline.limited
         )

From 2e76df3b023604f2b4e2d77eea895663d62ea35a Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 6 Dec 2021 08:10:10 -0500
Subject: [PATCH 12/13] Only override bundled aggregations behavior where it
 differs from defaults.

---
 synapse/events/utils.py          | 13 ++++++++-----
 synapse/handlers/initial_sync.py |  8 +++++---
 synapse/handlers/message.py      |  7 +------
 synapse/handlers/pagination.py   |  6 +-----
 synapse/handlers/search.py       |  3 +--
 synapse/rest/admin/rooms.py      | 12 ++----------
 synapse/rest/client/room.py      |  5 +----
 synapse/rest/client/sync.py      | 17 ++++++-----------
 8 files changed, 25 insertions(+), 46 deletions(-)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index bffcb65df19b..d7f5f4493689 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -404,7 +404,7 @@ async def serialize_event(
             event: The event being serialized.
             time_now: The current time in milliseconds
             bundle_aggregations: Whether to include the bundled aggregations for this
-                event.
+                event. Only applies to non-state events.
             **kwargs: Arguments to pass to `serialize_event`
 
         Returns:
@@ -416,10 +416,13 @@ async def serialize_event(
 
         serialized_event = serialize_event(event, time_now, **kwargs)
 
-        # If MSC1849 is enabled then we need to look if there are any relation
-        # aggregations we need to bundle in with the event.
-        # Do not bundle aggregations if the event has been redacted or if the event
-        # is a state event.
+        # Check if there are any bundled aggregations to include with the event.
+        #
+        # Do not bundle aggregations if any of the following at true:
+        #
+        # * Support is disabled via the configuration or the caller.
+        # * The event is a state event.
+        # * The event has been redacted.
         if (
             self._msc1849_enabled
             and bundle_aggregations
diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py
index 133817264ef5..9cd21e7f2b3c 100644
--- a/synapse/handlers/initial_sync.py
+++ b/synapse/handlers/initial_sync.py
@@ -167,6 +167,8 @@ async def handle_room(event: RoomsForUser) -> None:
                 d["invite"] = await self._event_serializer.serialize_event(
                     invite_event,
                     time_now,
+                    # Don't bundle aggregations as this is a deprecated API.
+                    bundle_aggregations=False,
                     as_client_event=as_client_event,
                 )
 
@@ -232,7 +234,7 @@ async def handle_room(event: RoomsForUser) -> None:
                 d["state"] = await self._event_serializer.serialize_events(
                     current_state.values(),
                     time_now=time_now,
-                    # No need to bundle aggregations for state events.
+                    # Don't bundle aggregations as this is a deprecated API.
                     bundle_aggregations=False,
                     as_client_event=as_client_event,
                 )
@@ -383,7 +385,7 @@ async def _room_initial_sync_parted(
                 "end": await end_token.to_string(self.store),
             },
             "state": (
-                # No need to bundle aggregations for state events.
+                # Don't bundle aggregations as this is a deprecated API.
                 await self._event_serializer.serialize_events(
                     room_state.values(), time_now, bundle_aggregations=False
                 )
@@ -404,7 +406,7 @@ async def _room_initial_sync_joined(
 
         # TODO: These concurrently
         time_now = self.clock.time_msec()
-        # No need to bundle aggregations for state events.
+        # Don't bundle aggregations as this is a deprecated API.
         state = await self._event_serializer.serialize_events(
             current_state.values(), time_now, bundle_aggregations=False
         )
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index e77a7c721ea4..87f671708c4e 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -247,12 +247,7 @@ async def get_state_events(
                 room_state = room_state_events[membership_event_id]
 
         now = self.clock.time_msec()
-        events = await self._event_serializer.serialize_events(
-            room_state.values(),
-            now,
-            # No need to bundle aggregations for state events.
-            bundle_aggregations=False,
-        )
+        events = await self._event_serializer.serialize_events(room_state.values(), now)
         return events
 
     async def get_joined_members(self, requester: Requester, room_id: str) -> dict:
diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py
index d6e50ffa68d6..cd64142735de 100644
--- a/synapse/handlers/pagination.py
+++ b/synapse/handlers/pagination.py
@@ -554,11 +554,7 @@ async def get_messages(
 
         if state:
             chunk["state"] = await self._event_serializer.serialize_events(
-                state,
-                time_now,
-                as_client_event=as_client_event,
-                # No need to bundle aggregations for state events.
-                bundle_aggregations=False,
+                state, time_now, as_client_event=as_client_event
             )
 
         return chunk
diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py
index 145396a60fed..ab7eaab2fb56 100644
--- a/synapse/handlers/search.py
+++ b/synapse/handlers/search.py
@@ -457,9 +457,8 @@ async def search(
         if state_results:
             s = {}
             for room_id, state_events in state_results.items():
-                # No need to bundle aggregations for state events.
                 s[room_id] = await self._event_serializer.serialize_events(
-                    state_events, time_now, bundle_aggregations=False
+                    state_events, time_now
                 )
 
             rooms_cat_res["state"] = s
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 89716bf8140d..669ab44a4599 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -449,12 +449,7 @@ async def on_GET(
         event_ids = await self.store.get_current_state_ids(room_id)
         events = await self.store.get_events(event_ids.values())
         now = self.clock.time_msec()
-        room_state = await self._event_serializer.serialize_events(
-            events.values(),
-            now,
-            # No need to bundle aggregations for state events.
-            bundle_aggregations=False,
-        )
+        room_state = await self._event_serializer.serialize_events(events.values(), now)
         ret = {"state": room_state}
 
         return HTTPStatus.OK, ret
@@ -788,10 +783,7 @@ async def on_GET(
             results["events_after"], time_now
         )
         results["state"] = await self._event_serializer.serialize_events(
-            results["state"],
-            time_now,
-            # No need to bundle aggregations for state events.
-            bundle_aggregations=False,
+            results["state"], time_now
         )
 
         return HTTPStatus.OK, results
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index d7eb97edf3f3..f48e2e6ca248 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -716,10 +716,7 @@ async def on_GET(
             results["events_after"], time_now
         )
         results["state"] = await self._event_serializer.serialize_events(
-            results["state"],
-            time_now,
-            # No need to bundle aggregations for state events.
-            bundle_aggregations=False,
+            results["state"], time_now
         )
 
         return 200, results
diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index da16cd7e6b67..88e4f5e0630f 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -516,13 +516,13 @@ async def encode_room(
             The room, encoded in our response format
         """
 
-        def serialize(
-            events: Iterable[EventBase], bundle_aggregations: bool
-        ) -> Awaitable[List[JsonDict]]:
+        def serialize(events: Iterable[EventBase]) -> Awaitable[List[JsonDict]]:
             return self._event_serializer.serialize_events(
                 events,
                 time_now=time_now,
-                bundle_aggregations=bundle_aggregations,
+                # Don't bother to bundle aggregations if the timeline is unlimited,
+                # as clients will have all the necessary information.
+                bundle_aggregations=room.timeline.limited,
                 token_id=token_id,
                 event_format=event_formatter,
                 only_event_fields=only_fields,
@@ -544,13 +544,8 @@ def serialize(
                     event.room_id,
                 )
 
-        # No need to bundle aggregations for state events.
-        serialized_state = await serialize(state_events, bundle_aggregations=False)
-        # Don't bother to bundle aggregations if the timeline is unlimited, as
-        # clients will have all the necessary information.
-        serialized_timeline = await serialize(
-            timeline_events, bundle_aggregations=room.timeline.limited
-        )
+        serialized_state = await serialize(state_events)
+        serialized_timeline = await serialize(timeline_events)
 
         account_data = room.account_data
 

From 4ea1d51f6f96265f90e5bf6644808a2c1810036e Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 6 Dec 2021 10:21:32 -0500
Subject: [PATCH 13/13] Clarify comment.

---
 synapse/events/utils.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index d7f5f4493689..84ef69df679b 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -404,7 +404,8 @@ async def serialize_event(
             event: The event being serialized.
             time_now: The current time in milliseconds
             bundle_aggregations: Whether to include the bundled aggregations for this
-                event. Only applies to non-state events.
+                event. Only applies to non-state events. (State events never include
+                bundled aggregations.)
             **kwargs: Arguments to pass to `serialize_event`
 
         Returns: