Finish up stripped_state for invite rooms

See https://github.com/element-hq/synapse/pull/17320#discussion_r1646581077

synapse/handlers/sliding_sync.py

@@ -26,6 +26,7 @@ from immutabledict import immutabledict
 from synapse.api.constants import AccountDataTypes, Direction, EventTypes, Membership
 from synapse.events import EventBase
 from synapse.events.utils import strip_event
+from synapse.handlers.relations import BundledAggregations
 from synapse.storage.roommember import RoomsForUser
 from synapse.types import (
     JsonDict,
@@ -756,6 +757,7 @@ class SlidingSyncHandler:
 
         # Assemble the list of timeline events
         timeline_events: Optional[List[EventBase]] = None
+        bundled_aggregations: Optional[Dict[str, BundledAggregations]] = None
         limited: Optional[bool] = None
         prev_batch_token: Optional[StreamToken] = None
         num_live: Optional[int] = None
@@ -848,7 +850,9 @@ class SlidingSyncHandler:
                 filter_send_to_client=True,
             )
             # TODO: Filter out `EventTypes.CallInvite` in public rooms,
-            # see https://github.com/element-hq/synapse/pull/16908#discussion_r1651598029
+            # see https://github.com/element-hq/synapse/issues/17359
+
+            # TODO: Handle timeline gaps (`get_timeline_gaps()`)
 
             # Determine how many "live" events we have (events within the given token range).
             #
@@ -878,6 +882,15 @@ class SlidingSyncHandler:
                         # this more with a binary search (bisect).
                         break
 
+            # If the timeline is `limited=True`, the client does not have all events
+            # necessary to calculate aggregations themselves.
+            if limited:
+                bundled_aggregations = (
+                    await self.relations_handler.get_bundled_aggregations(
+                        timeline_events, user.to_string()
+                    )
+                )
+
             # Update the `prev_batch_token` to point to the position that allows us to
             # keep paginating backwards from the oldest event we return in the timeline.
             prev_batch_token = prev_batch_token.copy_and_replace(
@@ -907,18 +920,6 @@ class SlidingSyncHandler:
 
             stripped_state.append(strip_event(invite_or_knock_event))
 
-        # TODO: Handle timeline gaps (`get_timeline_gaps()`)
-
-        # If the timeline is `limited=True`, the client does not have all events
-        # necessary to calculate aggregations themselves.
-        bundled_aggregations = None
-        if limited and timeline_events is not None:
-            bundled_aggregations = (
-                await self.relations_handler.get_bundled_aggregations(
-                    timeline_events, user.to_string()
-                )
-            )
-
         return SlidingSyncResult.RoomResult(
             # TODO: Dummy value
             name=None,

synapse/types/handlers/__init__.py

@@ -203,6 +203,7 @@ class SlidingSyncResult:
         timeline_events: Optional[List[EventBase]]
         bundled_aggregations: Optional[Dict[str, "BundledAggregations"]]
         is_dm: bool
+        # Optional because it's only relevant to invite/knock rooms
         stripped_state: Optional[List[JsonDict]]
         # Only optional because it won't be included for invite/knock rooms with `stripped_state`
         prev_batch: Optional[StreamToken]

tests/rest/client/test_sync.py

@@ -1836,9 +1836,12 @@ class SlidingSyncTestCase(unittest.HomeserverTestCase):
 
     def test_rooms_invite_shared_history_initial_sync(self) -> None:
         """
-        Test that `rooms` we are invited to have some stripped `invite_state` and that
-        we can't see any timeline events because the history visiblity is `shared` and
-        we haven't joined the room yet.
+        Test that `rooms` we are invited to have some stripped `invite_state` during an
+        initial sync.
+
+        This is an `invite` room so we should only have `stripped_state` (no `timeline`)
+        but we also shouldn't see any timeline events because the history visiblity is
+        `shared` and we haven't joined the room yet.
         """
         user1_id = self.register_user("user1", "pass")
         user1_tok = self.login(user1_id, "pass")
@@ -1936,9 +1939,10 @@ class SlidingSyncTestCase(unittest.HomeserverTestCase):
 
     def test_rooms_invite_shared_history_incremental_sync(self) -> None:
         """
-        Test that `rooms` we are invited to have some stripped `invite_state` 
-        
-        This is an `invite` room so we should only have `stripped_state` (no timeline)
+        Test that `rooms` we are invited to have some stripped `invite_state` during an
+        incremental sync.
+
+        This is an `invite` room so we should only have `stripped_state` (no `timeline`)
         but we also shouldn't see any timeline events because the history visiblity is
         `shared` and we haven't joined the room yet.
         """
@@ -2046,9 +2050,14 @@ class SlidingSyncTestCase(unittest.HomeserverTestCase):
 
     def test_rooms_invite_world_readable_history_initial_sync(self) -> None:
         """
-        Test that `rooms` we are invited to have some stripped `invite_state` and that
-        we can't see any timeline events because the history visiblity is `shared` and
-        we haven't joined the room yet.
+        Test that `rooms` we are invited to have some stripped `invite_state` during an
+        initial sync.
+
+        This is an `invite` room so we should only have `stripped_state` (no `timeline`)
+        but depending on the semantics we decide, we could potentially see some
+        historical events before/after the `from_token` because the history is
+        `world_readable`. Same situation for events after the `from_token` if the
+        history visibility was set to `invited`.
         """
         user1_id = self.register_user("user1", "pass")
         user1_tok = self.login(user1_id, "pass")
@@ -2160,6 +2169,135 @@ class SlidingSyncTestCase(unittest.HomeserverTestCase):
             channel.json_body["rooms"][room_id1]["invite_state"],
         )
 
+    def test_rooms_invite_world_readable_history_incremental_sync(self) -> None:
+        """
+        Test that `rooms` we are invited to have some stripped `invite_state` during an
+        incremental sync.
+
+        This is an `invite` room so we should only have `stripped_state` (no `timeline`)
+        but depending on the semantics we decide, we could potentially see some
+        historical events before/after the `from_token` because the history is
+        `world_readable`. Same situation for events after the `from_token` if the
+        history visibility was set to `invited`.
+        """
+        user1_id = self.register_user("user1", "pass")
+        user1_tok = self.login(user1_id, "pass")
+        user1 = UserID.from_string(user1_id)
+        user2_id = self.register_user("user2", "pass")
+        user2_tok = self.login(user2_id, "pass")
+        user2 = UserID.from_string(user2_id)
+
+        room_id1 = self.helper.create_room_as(
+            user2_id,
+            tok=user2_tok,
+            extra_content={
+                "preset": "public_chat",
+                "initial_state": [
+                    {
+                        "content": {
+                            "history_visibility": HistoryVisibility.WORLD_READABLE
+                        },
+                        "state_key": "",
+                        "type": EventTypes.RoomHistoryVisibility,
+                    }
+                ],
+            },
+        )
+        # Ensure we're testing with a room with `world_readable` history visibility
+        # which means events are visible to anyone even without membership.
+        history_visibility_response = self.helper.get_state(
+            room_id1, EventTypes.RoomHistoryVisibility, tok=user2_tok
+        )
+        self.assertEqual(
+            history_visibility_response.get("history_visibility"),
+            HistoryVisibility.WORLD_READABLE,
+        )
+
+        self.helper.send(room_id1, "activity before invite1", tok=user2_tok)
+        self.helper.send(room_id1, "activity before invite2", tok=user2_tok)
+        self.helper.invite(room_id1, src=user2_id, targ=user1_id, tok=user2_tok)
+        self.helper.send(room_id1, "activity after invite3", tok=user2_tok)
+        self.helper.send(room_id1, "activity after invite4", tok=user2_tok)
+
+        from_token = self.event_sources.get_current_token()
+
+        self.helper.send(room_id1, "activity after token5", tok=user2_tok)
+        self.helper.send(room_id1, "activity after toekn6", tok=user2_tok)
+
+        # Make the Sliding Sync request
+        channel = self.make_request(
+            "POST",
+            self.sync_endpoint
+            + f"?pos={self.get_success(
+            from_token.to_string(self.store)
+        )}",
+            {
+                "lists": {
+                    "foo-list": {
+                        "ranges": [[0, 1]],
+                        "required_state": [],
+                        # Large enough to see the latest events and before the invite
+                        "timeline_limit": 4,
+                    }
+                }
+            },
+            access_token=user1_tok,
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+        # `timeline` is omitted for `invite` rooms with `stripped_state`
+        self.assertIsNone(
+            channel.json_body["rooms"][room_id1].get("timeline"),
+            channel.json_body["rooms"][room_id1],
+        )
+        # `num_live` is omitted for `invite` rooms with `stripped_state` (no timeline anyway)
+        self.assertIsNone(
+            channel.json_body["rooms"][room_id1].get("num_live"),
+            channel.json_body["rooms"][room_id1],
+        )
+        # `limited` is omitted for `invite` rooms with `stripped_state` (no timeline anyway)
+        self.assertIsNone(
+            channel.json_body["rooms"][room_id1].get("limited"),
+            channel.json_body["rooms"][room_id1],
+        )
+        # `prev_batch` is omitted for `invite` rooms with `stripped_state` (no timeline anyway)
+        self.assertIsNone(
+            channel.json_body["rooms"][room_id1].get("prev_batch"),
+            channel.json_body["rooms"][room_id1],
+        )
+        # We should have some `stripped_state` so the potential joiner can identify the
+        # room (we don't care about the order).
+        self.assertCountEqual(
+            channel.json_body["rooms"][room_id1]["invite_state"],
+            [
+                {
+                    "content": {"creator": user2_id, "room_version": "10"},
+                    "sender": user2_id,
+                    "state_key": "",
+                    "type": "m.room.create",
+                },
+                {
+                    "content": {"join_rule": "public"},
+                    "sender": user2_id,
+                    "state_key": "",
+                    "type": "m.room.join_rules",
+                },
+                {
+                    "content": {"displayname": user2.localpart, "membership": "join"},
+                    "sender": user2_id,
+                    "state_key": user2_id,
+                    "type": "m.room.member",
+                },
+                {
+                    "content": {"displayname": user1.localpart, "membership": "invite"},
+                    "sender": user2_id,
+                    "state_key": user1_id,
+                    "type": "m.room.member",
+                },
+            ],
+            channel.json_body["rooms"][room_id1]["invite_state"],
+        )
+
     def test_rooms_ban_initial_sync(self) -> None:
         """
         Test that `rooms` we are banned from in an intial sync only allows us to see