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.

5 files changed, 43 insertions, 26 deletions

diff --git a/changelog.d/10256.misc b/changelog.d/10256.misc
new file mode 100644
index 0000000000..adef12fcb9
--- /dev/null
+++ b/changelog.d/10256.misc
@@ -0,0 +1 @@
+Improve the performance of the spaces summary endpoint by only recursing into spaces (and not rooms in general).
diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 414e4c019a..8363c2bb0f 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -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
diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py
index 17fc47ce16..266f369883 100644
--- a/synapse/handlers/space_summary.py
+++ b/synapse/handlers/space_summary.py
@@ -25,6 +25,7 @@ from synapse.api.constants import (
     EventTypes,
     HistoryVisibility,
     Membership,
+    RoomTypes,
 )
 from synapse.events import EventBase
 from synapse.events.utils import format_event_for_client_v2
@@ -318,7 +319,8 @@ class SpaceSummaryHandler:
 
         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.
@@ -328,7 +330,11 @@ class SpaceSummaryHandler:
 
         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:
@@ -348,6 +354,7 @@ class SpaceSummaryHandler:
                     event_format=format_event_for_client_v2,
                 )
             )
+
         return room_entry, events_result
 
     async def _summarize_remote_room(
diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py
index 131d362ccc..9771d3fb3b 100644
--- a/tests/handlers/test_space_summary.py
+++ b/tests/handlers/test_space_summary.py
@@ -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
@@ -97,9 +98,21 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         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(
@@ -128,43 +141,32 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
 
     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,
@@ -173,6 +175,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         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)])
diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/v1/utils.py
index ed55a640af..69798e95c3 100644
--- a/tests/rest/client/v1/utils.py
+++ b/tests/rest/client/v1/utils.py
@@ -52,6 +52,7 @@ class RestHelper:
         room_version: str = None,
         tok: str = None,
         expect_code: int = 200,
+        extra_content: Optional[Dict] = None,
     ) -> str:
         """
         Create a room.
@@ -72,7 +73,7 @@ class RestHelper:
         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: