forked from MirrorHub/synapse
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.
This commit is contained in:
parent
d3cf0730f8
commit
8e57584a58
1
changelog.d/11695.bugfix
Normal file
1
changelog.d/11695.bugfix
Normal file
|
@ -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.
|
|
@ -153,6 +153,9 @@ class RoomSummaryHandler:
|
||||||
rooms_result: List[JsonDict] = []
|
rooms_result: List[JsonDict] = []
|
||||||
events_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:
|
while room_queue and len(rooms_result) < MAX_ROOMS:
|
||||||
queue_entry = room_queue.popleft()
|
queue_entry = room_queue.popleft()
|
||||||
room_id = queue_entry.room_id
|
room_id = queue_entry.room_id
|
||||||
|
@ -167,7 +170,7 @@ class RoomSummaryHandler:
|
||||||
# The client-specified max_rooms_per_space limit doesn't apply to the
|
# 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
|
# room_id specified in the request, so we ignore it if this is the
|
||||||
# first room we are processing.
|
# 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:
|
if is_in_room:
|
||||||
room_entry = await self._summarize_local_room(
|
room_entry = await self._summarize_local_room(
|
||||||
|
@ -395,7 +398,7 @@ class RoomSummaryHandler:
|
||||||
None,
|
None,
|
||||||
room_id,
|
room_id,
|
||||||
suggested_only,
|
suggested_only,
|
||||||
# TODO Handle max children.
|
# Do not limit the maximum children.
|
||||||
max_children=None,
|
max_children=None,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -525,6 +528,10 @@ class RoomSummaryHandler:
|
||||||
rooms_result: List[JsonDict] = []
|
rooms_result: List[JsonDict] = []
|
||||||
events_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:
|
while room_queue and len(rooms_result) < MAX_ROOMS:
|
||||||
room_id = room_queue.popleft()
|
room_id = room_queue.popleft()
|
||||||
if room_id in processed_rooms:
|
if room_id in processed_rooms:
|
||||||
|
@ -583,7 +590,9 @@ class RoomSummaryHandler:
|
||||||
|
|
||||||
# Iterate through each child and potentially add it, but not its children,
|
# Iterate through each child and potentially add it, but not its children,
|
||||||
# to the response.
|
# 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")
|
room_id = child_room.get("state_key")
|
||||||
assert isinstance(room_id, str)
|
assert isinstance(room_id, str)
|
||||||
# If the room is unknown, skip it.
|
# If the room is unknown, skip it.
|
||||||
|
@ -633,8 +642,8 @@ class RoomSummaryHandler:
|
||||||
suggested_only: True if only suggested children should be returned.
|
suggested_only: True if only suggested children should be returned.
|
||||||
Otherwise, all children are returned.
|
Otherwise, all children are returned.
|
||||||
max_children:
|
max_children:
|
||||||
The maximum number of children rooms to include. This is capped
|
The maximum number of children rooms to include. A value of None
|
||||||
to a server-set limit.
|
means no limit.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
A room entry if the room should be returned. None, otherwise.
|
A room entry if the room should be returned. None, otherwise.
|
||||||
|
@ -656,8 +665,13 @@ class RoomSummaryHandler:
|
||||||
# we only care about suggested children
|
# we only care about suggested children
|
||||||
child_events = filter(_is_suggested_child_event, child_events)
|
child_events = filter(_is_suggested_child_event, child_events)
|
||||||
|
|
||||||
if max_children is None or max_children > MAX_ROOMS_PER_SPACE:
|
# TODO max_children is legacy code for the /spaces endpoint.
|
||||||
max_children = MAX_ROOMS_PER_SPACE
|
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] = [
|
stripped_events: List[JsonDict] = [
|
||||||
{
|
{
|
||||||
|
@ -668,7 +682,7 @@ class RoomSummaryHandler:
|
||||||
"sender": e.sender,
|
"sender": e.sender,
|
||||||
"origin_server_ts": e.origin_server_ts,
|
"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)
|
return _RoomEntry(room_id, room_entry, stripped_events)
|
||||||
|
|
||||||
|
|
|
@ -253,6 +253,38 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
|
||||||
)
|
)
|
||||||
self._assert_hierarchy(result, expected)
|
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):
|
def test_visibility(self):
|
||||||
"""A user not in a space cannot inspect it."""
|
"""A user not in a space cannot inspect it."""
|
||||||
user2 = self.register_user("user2", "pass")
|
user2 = self.register_user("user2", "pass")
|
||||||
|
|
Loading…
Reference in a new issue