Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Do not recurse into non-spaces in the spaces summary. #10256

Merged
merged 3 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10256.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve the performance of the spaces summary endpoint by only recursing into spaces (and not rooms in general).
6 changes: 6 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ class EventContentFields:
)


class RoomTypes:
"""Understood values of the room_type field of m.room.create events."""

SPACE = "m.space"


class RoomEncryptionAlgorithms:
MEGOLM_V1_AES_SHA2 = "m.megolm.v1.aes-sha2"
DEFAULT = MEGOLM_V1_AES_SHA2
Expand Down
11 changes: 9 additions & 2 deletions synapse/handlers/space_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
EventTypes,
HistoryVisibility,
Membership,
RoomTypes,
)
from synapse.events import EventBase
from synapse.events.utils import format_event_for_client_v2
Expand Down Expand Up @@ -318,7 +319,8 @@ async def _summarize_local_room(

Returns:
A tuple of:
An iterable of a single value of the room.
The room information, if the room should be returned to the
user. None, otherwise.

An iterable of the sorted children events. This may be limited
to a maximum size or may include all children.
Expand All @@ -328,7 +330,11 @@ async def _summarize_local_room(

room_entry = await self._build_room_entry(room_id)

# look for child rooms/spaces.
# If the room is not a space, return just the room information.
if room_entry.get("room_type") != RoomTypes.SPACE:
return room_entry, ()

# Otherwise, look for child rooms/spaces.
child_events = await self._get_child_events(room_id)

if suggested_only:
Expand All @@ -348,6 +354,7 @@ async def _summarize_local_room(
event_format=format_event_for_client_v2,
)
)

return room_entry, events_result

async def _summarize_remote_room(
Expand Down
48 changes: 25 additions & 23 deletions tests/handlers/test_space_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from typing import Any, Iterable, Optional, Tuple
from unittest import mock

from synapse.api.constants import EventContentFields, RoomTypes
from synapse.api.errors import AuthError
from synapse.handlers.space_summary import _child_events_comparison_key
from synapse.rest import admin
Expand Down Expand Up @@ -97,9 +98,21 @@ def prepare(self, reactor, clock, hs: HomeServer):
self.hs = hs
self.handler = self.hs.get_space_summary_handler()

# Create a user.
self.user = self.register_user("user", "pass")
self.token = self.login("user", "pass")

# Create a space and a child room.
self.space = self.helper.create_room_as(
self.user,
tok=self.token,
extra_content={
"creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}
},
)
self.room = self.helper.create_room_as(self.user, tok=self.token)
self._add_child(self.space, self.room, self.token)

def _add_child(self, space_id: str, room_id: str, token: str) -> None:
"""Add a child room to a space."""
self.helper.send_state(
Expand Down Expand Up @@ -128,43 +141,32 @@ def _assert_events(

def test_simple_space(self):
"""Test a simple space with a single room."""
space = self.helper.create_room_as(self.user, tok=self.token)
room = self.helper.create_room_as(self.user, tok=self.token)
self._add_child(space, room, self.token)

result = self.get_success(self.handler.get_space_summary(self.user, space))
result = self.get_success(self.handler.get_space_summary(self.user, self.space))
# The result should have the space and the room in it, along with a link
# from space -> room.
self._assert_rooms(result, [space, room])
self._assert_events(result, [(space, room)])
self._assert_rooms(result, [self.space, self.room])
self._assert_events(result, [(self.space, self.room)])

def test_visibility(self):
"""A user not in a space cannot inspect it."""
space = self.helper.create_room_as(self.user, tok=self.token)
room = self.helper.create_room_as(self.user, tok=self.token)
self._add_child(space, room, self.token)

user2 = self.register_user("user2", "pass")
token2 = self.login("user2", "pass")

# The user cannot see the space.
self.get_failure(self.handler.get_space_summary(user2, space), AuthError)
self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)

# Joining the room causes it to be visible.
self.helper.join(space, user2, tok=token2)
result = self.get_success(self.handler.get_space_summary(user2, space))
self.helper.join(self.space, user2, tok=token2)
result = self.get_success(self.handler.get_space_summary(user2, self.space))

# The result should only have the space, but includes the link to the room.
self._assert_rooms(result, [space])
self._assert_events(result, [(space, room)])
self._assert_rooms(result, [self.space])
self._assert_events(result, [(self.space, self.room)])

def test_world_readable(self):
"""A world-readable room is visible to everyone."""
space = self.helper.create_room_as(self.user, tok=self.token)
room = self.helper.create_room_as(self.user, tok=self.token)
self._add_child(space, room, self.token)
self.helper.send_state(
space,
self.space,
event_type="m.room.history_visibility",
body={"history_visibility": "world_readable"},
tok=self.token,
Expand All @@ -173,6 +175,6 @@ def test_world_readable(self):
user2 = self.register_user("user2", "pass")

# The space should be visible, as well as the link to the room.
result = self.get_success(self.handler.get_space_summary(user2, space))
self._assert_rooms(result, [space])
self._assert_events(result, [(space, room)])
result = self.get_success(self.handler.get_space_summary(user2, self.space))
self._assert_rooms(result, [self.space])
self._assert_events(result, [(self.space, self.room)])
3 changes: 2 additions & 1 deletion tests/rest/client/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def create_room_as(
room_version: str = None,
tok: str = None,
expect_code: int = 200,
extra_content: Optional[Dict] = None,
) -> str:
"""
Create a room.
Expand All @@ -72,7 +73,7 @@ def create_room_as(
temp_id = self.auth_user_id
self.auth_user_id = room_creator
path = "/_matrix/client/r0/createRoom"
content = {}
content = extra_content or {}
if not is_public:
content["visibility"] = "private"
if room_version:
Expand Down