Support spaces with > 50 rooms in the /hierarchy endpoint. (#11695)

By returning all of the m.space.child state of the space, not just
the first 50. The number of rooms returned is still capped at 50.

For the federation API this implies that the requesting server will
need to individually query for any other rooms it is not joined to.

changelog.d/11695.bugfix

@@ -0,0 +1 @@
+Fix a bug where the only the first 50 rooms from a space were returned from the `/hierarchy` API. This has existed since the introduction of the API in Synapse v1.41.0.

synapse/handlers/room_summary.py

@@ -153,6 +153,9 @@ async def get_space_summary(
         rooms_result: List[JsonDict] = []
         events_result: List[JsonDict] = []
 
+        if max_rooms_per_space is None or max_rooms_per_space > MAX_ROOMS_PER_SPACE:
+            max_rooms_per_space = MAX_ROOMS_PER_SPACE
+
         while room_queue and len(rooms_result) < MAX_ROOMS:
             queue_entry = room_queue.popleft()
             room_id = queue_entry.room_id
@@ -167,7 +170,7 @@ async def get_space_summary(
             # The client-specified max_rooms_per_space limit doesn't apply to the
             # room_id specified in the request, so we ignore it if this is the
             # first room we are processing.
-            max_children = max_rooms_per_space if processed_rooms else None
+            max_children = max_rooms_per_space if processed_rooms else MAX_ROOMS
 
             if is_in_room:
                 room_entry = await self._summarize_local_room(
@@ -395,7 +398,7 @@ async def _get_room_hierarchy(
                     None,
                     room_id,
                     suggested_only,
-                    # TODO Handle max children.
+                    # Do not limit the maximum children.
                     max_children=None,
                 )
 
@@ -525,6 +528,10 @@ async def federation_space_summary(
         rooms_result: List[JsonDict] = []
         events_result: List[JsonDict] = []
 
+        # Set a limit on the number of rooms to return.
+        if max_rooms_per_space is None or max_rooms_per_space > MAX_ROOMS_PER_SPACE:
+            max_rooms_per_space = MAX_ROOMS_PER_SPACE
+
         while room_queue and len(rooms_result) < MAX_ROOMS:
             room_id = room_queue.popleft()
             if room_id in processed_rooms:
@@ -583,7 +590,9 @@ async def get_federation_hierarchy(
 
         # Iterate through each child and potentially add it, but not its children,
         # to the response.
-        for child_room in root_room_entry.children_state_events:
+        for child_room in itertools.islice(
+            root_room_entry.children_state_events, MAX_ROOMS_PER_SPACE
+        ):
             room_id = child_room.get("state_key")
             assert isinstance(room_id, str)
             # If the room is unknown, skip it.
@@ -633,8 +642,8 @@ async def _summarize_local_room(
             suggested_only: True if only suggested children should be returned.
                 Otherwise, all children are returned.
             max_children:
-                The maximum number of children rooms to include. This is capped
-                to a server-set limit.
+                The maximum number of children rooms to include. A value of None
+                means no limit.
 
         Returns:
             A room entry if the room should be returned. None, otherwise.
@@ -656,8 +665,13 @@ async def _summarize_local_room(
             # we only care about suggested children
             child_events = filter(_is_suggested_child_event, child_events)
 
-        if max_children is None or max_children > MAX_ROOMS_PER_SPACE:
-            max_children = MAX_ROOMS_PER_SPACE
+        # TODO max_children is legacy code for the /spaces endpoint.
+        if max_children is not None:
+            child_iter: Iterable[EventBase] = itertools.islice(
+                child_events, max_children
+            )
+        else:
+            child_iter = child_events
 
         stripped_events: List[JsonDict] = [
             {
@@ -668,7 +682,7 @@ async def _summarize_local_room(
                 "sender": e.sender,
                 "origin_server_ts": e.origin_server_ts,
             }
-            for e in itertools.islice(child_events, max_children)
+            for e in child_iter
         ]
         return _RoomEntry(room_id, room_entry, stripped_events)
 

tests/handlers/test_room_summary.py

@@ -253,6 +253,38 @@ def test_simple_space(self):
         )
         self._assert_hierarchy(result, expected)
 
+    def test_large_space(self):
+        """Test a space with a large number of rooms."""
+        rooms = [self.room]
+        # Make at least 51 rooms that are part of the space.
+        for _ in range(55):
+            room = self.helper.create_room_as(self.user, tok=self.token)
+            self._add_child(self.space, room, self.token)
+            rooms.append(room)
+
+        result = self.get_success(self.handler.get_space_summary(self.user, self.space))
+        # The spaces result should have the space and the first 50 rooms in it,
+        # along with the links from space -> room for those 50 rooms.
+        expected = [(self.space, rooms[:50])] + [(room, []) for room in rooms[:49]]
+        self._assert_rooms(result, expected)
+
+        # The result should have the space and the rooms in it, along with the links
+        # from space -> room.
+        expected = [(self.space, rooms)] + [(room, []) for room in rooms]
+
+        # Make two requests to fully paginate the results.
+        result = self.get_success(
+            self.handler.get_room_hierarchy(create_requester(self.user), self.space)
+        )
+        result2 = self.get_success(
+            self.handler.get_room_hierarchy(
+                create_requester(self.user), self.space, from_token=result["next_batch"]
+            )
+        )
+        # Combine the results.
+        result["rooms"] += result2["rooms"]
+        self._assert_hierarchy(result, expected)
+
     def test_visibility(self):
         """A user not in a space cannot inspect it."""
         user2 = self.register_user("user2", "pass")