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

Commit

Permalink
Do not recurse into rooms that are not spaces.
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep committed Jun 25, 2021
1 parent 0fe2992 commit 11ac8a6
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 25 deletions.
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.
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
8 changes: 7 additions & 1 deletion 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 @@ -329,7 +330,11 @@ async def _summarize_local_room(

room_entry = await self._build_room_entry(room_id)

# look for child rooms/spaces.
# If the 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 @@ -349,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

0 comments on commit 11ac8a6

Please sign in to comment.