In legacy sync, lazy-load the memberships of the senders of MSC4354 Sticky Events. (#20362)

Part of: MSC4354 (Sticky Events, Experimental Feature tracking issue
#19409)

When a user is the sender of a sticky event that comes down sync, that
user is now in scope for having their lazy-loaded membership sent down
in sync.

---------

Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>

changelog.d/20362.feature

@@ -0,0 +1 @@
+In legacy sync, lazy-load the memberships of the senders of [MSC4354 Sticky Events](https://github.com/matrix-org/matrix-spec-proposals/pull/4354).

synapse/handlers/sync.py

@@ -291,6 +291,17 @@ class JoinedSyncResult:
             # else in the result, we don't need to send it.
         )
 
+    def lazy_loading_members_user_ids(self) -> set[str]:
+        """
+        Return the set of User IDs that are in-scope for lazy-loading in this
+        joined room sync result.
+
+        This is the set of all the senders of timeline events and MSC4354 sticky events.
+        """
+        return {e.event.sender for e in self.timeline.events} | {
+            e.event.sender for e in self.sticky
+        }
+
 
 @attr.s(slots=True, frozen=True, auto_attribs=True)
 class ArchivedSyncResult:
@@ -1253,6 +1264,7 @@ class SyncHandler:
         self,
         room_id: str,
         batch: TimelineBatch,
+        sticky_events: Sequence[FilteredEvent],
         sync_config: SyncConfig,
         since_token: StreamToken | None,
         end_token: StreamToken,
@@ -1318,8 +1330,11 @@ class SyncHandler:
 
                 # Membership events to fetch that can be found in the room state, or in
                 # the case of partial state rooms, the auth events of timeline events.
-                members_to_fetch = set()
+                #
+                # Fetch the members of MSC4354 sticky events
+                members_to_fetch = {event.event.sender for event in sticky_events}
                 first_event_by_sender_map = {}
+
                 for event in batch.events:
                     # Build the map from user IDs to the first timeline event they sent.
                     if event.event.sender not in first_event_by_sender_map:
@@ -1796,15 +1811,15 @@ class SyncHandler:
         if is_linear_timeline and not batch.limited:
             state_ids: StateMap[str] = {}
             if lazy_load_members:
-                if members_to_fetch and batch.events:
+                if members_to_fetch:
                     # We're lazy-loading, so the client might need some more
-                    # member events to understand the events in this timeline.
+                    # member events to understand the events in this timeline and/or sticky section.
                     # So we fish out all the member events corresponding to the
-                    # timeline here. The caller will then dedupe any redundant
+                    # timeline and/or sticky section here. The caller will then dedupe any redundant
                     # ones.
 
-                    state_ids = (
-                        await self._state_storage_controller.get_state_ids_for_event(
+                    if batch.events:
+                        state_ids = await self._state_storage_controller.get_state_ids_for_event(
                             batch.events[0].event.event_id,
                             # we only want members!
                             state_filter=StateFilter.from_types(
@@ -1813,7 +1828,23 @@ class SyncHandler:
                             ),
                             await_full_state=False,
                         )
-                    )
+                    else:
+                        # If we don't have any events in the timeline then this means we have members
+                        # to fetch for the sake of sticky events.
+                        # It's still the case that we should return the state at the 'start' of the timeline,
+                        # which in this case is the same as the end, so use the `end_token` to get state.
+                        state_ids = (
+                            await self._state_storage_controller.get_state_ids_at(
+                                room_id,
+                                end_token,
+                                # we only want members!
+                                state_filter=StateFilter.from_types(
+                                    (EventTypes.Member, member)
+                                    for member in members_to_fetch
+                                ),
+                                await_full_state=False,
+                            )
+                        )
             # A linear, non-gappy timeline means the only changes to the `state`
             # are lazy-loaded members.
             return state_ids, set()
@@ -2535,9 +2566,9 @@ class SyncHandler:
                     include_users.add(invited.invite.sender)
             # joined
             for joined in sync_result_builder.joined:
-                for timeline_event in joined.timeline.events:
-                    if self._is_mine_id(timeline_event.event.sender):
-                        include_users.add(timeline_event.event.sender)
+                for sender in joined.lazy_loading_members_user_ids():
+                    if self._is_mine_id(sender):
+                        include_users.add(sender)
             # knocked
             for knocked in sync_result_builder.knocked:
                 if self._is_mine_id(knocked.knock.sender):
@@ -3494,10 +3525,34 @@ class SyncHandler:
             ):
                 return
 
+            sticky_events: list[FilteredEvent] = []
+            if room_builder.rtype == "joined" and sticky_event_ids:
+                # As per MSC4354:
+                # Remove sticky events that are already in the timeline, else we will needlessly duplicate
+                # events.
+                # There is no purpose in including sticky events in the sticky section if they're already in
+                # the timeline, as either way the client becomes aware of them.
+                # This is particularly important given the risk of sticky events spam since
+                # anyone can send sticky events, so halving the bandwidth on average for each sticky
+                # event is helpful.
+                timeline_event_id_set = {ev.event.event_id for ev in batch.events}
+                # Must preserve sticky event stream order
+                sticky_event_ids = [
+                    e for e in sticky_event_ids if e not in timeline_event_id_set
+                ]
+                if sticky_event_ids:
+                    # Fetch and filter the sticky events
+                    sticky_events = await filter_and_transform_events_for_client(
+                        self._storage_controllers,
+                        sync_result_builder.sync_config.user.to_string(),
+                        await self.store.get_events_as_list(sticky_event_ids),
+                    )
+
             if not room_builder.out_of_band:
                 state = await self.compute_state_delta(
                     room_id,
                     batch,
+                    sticky_events,
                     sync_config,
                     since_token,
                     room_builder.end_token,
@@ -3538,28 +3593,7 @@ class SyncHandler:
 
             if room_builder.rtype == "joined":
                 unread_notifications: dict[str, int] = {}
-                sticky_events: list[FilteredEvent] = []
-                if sticky_event_ids:
-                    # As per MSC4354:
-                    # Remove sticky events that are already in the timeline, else we will needlessly duplicate
-                    # events.
-                    # There is no purpose in including sticky events in the sticky section if they're already in
-                    # the timeline, as either way the client becomes aware of them.
-                    # This is particularly important given the risk of sticky events spam since
-                    # anyone can send sticky events, so halving the bandwidth on average for each sticky
-                    # event is helpful.
-                    timeline_event_id_set = {ev.event.event_id for ev in batch.events}
-                    # Must preserve sticky event stream order
-                    sticky_event_ids = [
-                        e for e in sticky_event_ids if e not in timeline_event_id_set
-                    ]
-                    if sticky_event_ids:
-                        # Fetch and filter the sticky events
-                        sticky_events = await filter_and_transform_events_for_client(
-                            self._storage_controllers,
-                            sync_result_builder.sync_config.user.to_string(),
-                            await self.store.get_events_as_list(sticky_event_ids),
-                        )
+
                 room_sync = JoinedSyncResult(
                     room_id=room_id,
                     timeline=batch,

tests/rest/client/test_sync_sticky_events.py

@@ -48,6 +48,20 @@ A filter that excludes message events, so that the sticky events come down
 the sticky section rather than the timeline section.
 """
 
+NO_TIMELINE_FILTER_WITH_LAZY_LOADING = json.dumps(
+    {
+        "room": {
+            "timeline": {"types": ["io.element.example"]},
+            "state": {"lazy_load_members": True},
+        }
+    }
+)
+"""
+A filter that excludes message events, so that the sticky events come down
+the sticky section rather than the timeline section.
+This one also has lazy-loading members enabled.
+"""
+
 
 class SyncStickyEventsTestCase(unittest.HomeserverTestCase):
     """
@@ -675,3 +689,80 @@ class SyncStickyEventsTestCase(unittest.HomeserverTestCase):
         # Finally, expect an empty incremental sync at the end
         sync4 = self._do_sync(since=sync3.next_batch, filter_json=NO_TIMELINE_FILTER)
         self.assertEqual(sync4.sticky_event_ids, [])
+
+    def test_sticky_event_sender_membership_is_lazy_loaded(self) -> None:
+        """
+        Test that, when lazy-loading members, receiving a sticky event in the sticky
+        section causes the membership of the sticky event's sender to be sent down
+        in the room's state.
+        """
+        # Give the room a name to avoid heroes interfering with the test
+        self.helper.send_state(
+            self.room_id, EventTypes.Name, {"name": "A room"}, tok=self.token
+        )
+
+        user2_id = self.register_user("user2", "pass")
+        user2_token = self.login(user2_id, "pass")
+        self.helper.join(self.room_id, user2_id, tok=user2_token)
+        user3_id = self.register_user("user3", "pass")
+        user3_token = self.login(user3_id, "pass")
+        self.helper.join(self.room_id, user3_id, tok=user3_token)
+
+        # Have user2 send a sticky event
+        user2_sticky_event_id = self.helper.send_sticky_event(
+            self.room_id,
+            EventTypes.Message,
+            duration=Duration(minutes=1),
+            content={"body": "sticky from user2", "msgtype": "m.text"},
+            tok=user2_token,
+        )["event_id"]
+
+        # Initial sync:
+        # Expect to get user2's sticky event and user2's lazy-loaded membership
+        channel = self.make_request(
+            "GET",
+            f"/sync?filter={quote(NO_TIMELINE_FILTER_WITH_LAZY_LOADING)}",
+            access_token=self.token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
+        room_entry = channel.json_body["rooms"]["join"][self.room_id]
+        self.assertEqual(
+            [e["event_id"] for e in room_entry["msc4354_sticky"]["events"]],
+            [user2_sticky_event_id],
+        )
+        state_members = {
+            e["state_key"]
+            for e in room_entry["state"]["events"]
+            if e["type"] == EventTypes.Member
+        }
+        self.assertEqual(state_members, {self.user_id, user2_id})
+        since = channel.json_body["next_batch"]
+
+        # Now have user3 send a sticky event
+        user3_sticky_event_id = self.helper.send_sticky_event(
+            self.room_id,
+            EventTypes.Message,
+            duration=Duration(minutes=1),
+            content={"body": "sticky from user3", "msgtype": "m.text"},
+            tok=user3_token,
+        )["event_id"]
+
+        # Incremental sync:
+        # Expect to get user3's sticky event and user3's membership.
+        channel = self.make_request(
+            "GET",
+            f"/sync?filter={quote(NO_TIMELINE_FILTER_WITH_LAZY_LOADING)}&since={since}",
+            access_token=self.token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
+        room_entry = channel.json_body["rooms"]["join"][self.room_id]
+        self.assertEqual(
+            [e["event_id"] for e in room_entry["msc4354_sticky"]["events"]],
+            [user3_sticky_event_id],
+        )
+        state_members = {
+            e["state_key"]
+            for e in room_entry["state"]["events"]
+            if e["type"] == EventTypes.Member
+        }
+        self.assertEqual(state_members, {user3_id})