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

Previously m.child.room events in non-space rooms would be
treated as part of the room graph, but this is no longer
supported.
This commit is contained in:
Patrick Cloke 2021-06-29 12:00:04 -04:00 committed by GitHub
parent 7647b0337f
commit f55836929d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 26 deletions

1
changelog.d/10256.misc Normal file
View File

@ -0,0 +1 @@
Improve the performance of the spaces summary endpoint by only recursing into spaces (and not rooms in general).

View File

@ -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: class RoomEncryptionAlgorithms:
MEGOLM_V1_AES_SHA2 = "m.megolm.v1.aes-sha2" MEGOLM_V1_AES_SHA2 = "m.megolm.v1.aes-sha2"
DEFAULT = MEGOLM_V1_AES_SHA2 DEFAULT = MEGOLM_V1_AES_SHA2

View File

@ -25,6 +25,7 @@ from synapse.api.constants import (
EventTypes, EventTypes,
HistoryVisibility, HistoryVisibility,
Membership, Membership,
RoomTypes,
) )
from synapse.events import EventBase from synapse.events import EventBase
from synapse.events.utils import format_event_for_client_v2 from synapse.events.utils import format_event_for_client_v2
@ -318,7 +319,8 @@ class SpaceSummaryHandler:
Returns: Returns:
A tuple of: 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 An iterable of the sorted children events. This may be limited
to a maximum size or may include all children. to a maximum size or may include all children.
@ -328,7 +330,11 @@ class SpaceSummaryHandler:
room_entry = await self._build_room_entry(room_id) 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) child_events = await self._get_child_events(room_id)
if suggested_only: if suggested_only:
@ -348,6 +354,7 @@ class SpaceSummaryHandler:
event_format=format_event_for_client_v2, event_format=format_event_for_client_v2,
) )
) )
return room_entry, events_result return room_entry, events_result
async def _summarize_remote_room( async def _summarize_remote_room(

View File

@ -14,6 +14,7 @@
from typing import Any, Iterable, Optional, Tuple from typing import Any, Iterable, Optional, Tuple
from unittest import mock from unittest import mock
from synapse.api.constants import EventContentFields, RoomTypes
from synapse.api.errors import AuthError from synapse.api.errors import AuthError
from synapse.handlers.space_summary import _child_events_comparison_key from synapse.handlers.space_summary import _child_events_comparison_key
from synapse.rest import admin from synapse.rest import admin
@ -97,9 +98,21 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
self.hs = hs self.hs = hs
self.handler = self.hs.get_space_summary_handler() self.handler = self.hs.get_space_summary_handler()
# Create a user.
self.user = self.register_user("user", "pass") self.user = self.register_user("user", "pass")
self.token = self.login("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: def _add_child(self, space_id: str, room_id: str, token: str) -> None:
"""Add a child room to a space.""" """Add a child room to a space."""
self.helper.send_state( self.helper.send_state(
@ -128,43 +141,32 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
def test_simple_space(self): def test_simple_space(self):
"""Test a simple space with a single room.""" """Test a simple space with a single room."""
space = self.helper.create_room_as(self.user, tok=self.token) result = self.get_success(self.handler.get_space_summary(self.user, self.space))
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))
# The result should have the space and the room in it, along with a link # The result should have the space and the room in it, along with a link
# from space -> room. # from space -> room.
self._assert_rooms(result, [space, room]) self._assert_rooms(result, [self.space, self.room])
self._assert_events(result, [(space, room)]) self._assert_events(result, [(self.space, self.room)])
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."""
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") user2 = self.register_user("user2", "pass")
token2 = self.login("user2", "pass") token2 = self.login("user2", "pass")
# The user cannot see the space. # 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. # Joining the room causes it to be visible.
self.helper.join(space, user2, tok=token2) self.helper.join(self.space, user2, tok=token2)
result = self.get_success(self.handler.get_space_summary(user2, space)) 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. # The result should only have the space, but includes the link to the room.
self._assert_rooms(result, [space]) self._assert_rooms(result, [self.space])
self._assert_events(result, [(space, room)]) self._assert_events(result, [(self.space, self.room)])
def test_world_readable(self): def test_world_readable(self):
"""A world-readable room is visible to everyone.""" """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( self.helper.send_state(
space, self.space,
event_type="m.room.history_visibility", event_type="m.room.history_visibility",
body={"history_visibility": "world_readable"}, body={"history_visibility": "world_readable"},
tok=self.token, tok=self.token,
@ -173,6 +175,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
user2 = self.register_user("user2", "pass") user2 = self.register_user("user2", "pass")
# The space should be visible, as well as the link to the room. # The space should be visible, as well as the link to the room.
result = self.get_success(self.handler.get_space_summary(user2, space)) result = self.get_success(self.handler.get_space_summary(user2, self.space))
self._assert_rooms(result, [space]) self._assert_rooms(result, [self.space])
self._assert_events(result, [(space, room)]) self._assert_events(result, [(self.space, self.room)])

View File

@ -52,6 +52,7 @@ class RestHelper:
room_version: str = None, room_version: str = None,
tok: str = None, tok: str = None,
expect_code: int = 200, expect_code: int = 200,
extra_content: Optional[Dict] = None,
) -> str: ) -> str:
""" """
Create a room. Create a room.
@ -72,7 +73,7 @@ class RestHelper:
temp_id = self.auth_user_id temp_id = self.auth_user_id
self.auth_user_id = room_creator self.auth_user_id = room_creator
path = "/_matrix/client/r0/createRoom" path = "/_matrix/client/r0/createRoom"
content = {} content = extra_content or {}
if not is_public: if not is_public:
content["visibility"] = "private" content["visibility"] = "private"
if room_version: if room_version: