Remove the unstable /spaces endpoint. (#12073)

...and various code supporting it.

The /spaces endpoint was from an old version of MSC2946 and included
both a Client-Server and Server-Server API. Note that the unstable
/hierarchy endpoint (from the final version of MSC2946) is not yet
removed.

changelog.d/12073.removal

@@ -0,0 +1 @@
+Remove the unstable `/spaces` endpoint from [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946).

docs/workers.md

@@ -212,7 +212,6 @@ information.
     ^/_matrix/federation/v1/user/devices/
     ^/_matrix/federation/v1/get_groups_publicised$
     ^/_matrix/key/v2/query
-    ^/_matrix/federation/unstable/org.matrix.msc2946/spaces/
     ^/_matrix/federation/(v1|unstable/org.matrix.msc2946)/hierarchy/
 
     # Inbound federation transaction request
@@ -225,7 +224,6 @@ information.
     ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/context/.*$
     ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/members$
     ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/state$
-    ^/_matrix/client/unstable/org.matrix.msc2946/rooms/.*/spaces$
     ^/_matrix/client/(v1|unstable/org.matrix.msc2946)/rooms/.*/hierarchy$
     ^/_matrix/client/unstable/im.nheko.summary/rooms/.*/summary$
     ^/_matrix/client/(r0|v3|unstable)/account/3pid$

synapse/federation/federation_client.py

@@ -1362,61 +1362,6 @@ class FederationClient(FederationBase):
         # server doesn't give it to us.
         return None
 
-    async def get_space_summary(
-        self,
-        destinations: Iterable[str],
-        room_id: str,
-        suggested_only: bool,
-        max_rooms_per_space: Optional[int],
-        exclude_rooms: List[str],
-    ) -> "FederationSpaceSummaryResult":
-        """
-        Call other servers to get a summary of the given space
-
-
-        Args:
-            destinations: The remote servers. We will try them in turn, omitting any
-                that have been blacklisted.
-
-            room_id: ID of the space to be queried
-
-            suggested_only:  If true, ask the remote server to only return children
-                with the "suggested" flag set
-
-            max_rooms_per_space: A limit on the number of children to return for each
-                space
-
-            exclude_rooms: A list of room IDs to tell the remote server to skip
-
-        Returns:
-            a parsed FederationSpaceSummaryResult
-
-        Raises:
-            SynapseError if we were unable to get a valid summary from any of the
-               remote servers
-        """
-
-        async def send_request(destination: str) -> FederationSpaceSummaryResult:
-            res = await self.transport_layer.get_space_summary(
-                destination=destination,
-                room_id=room_id,
-                suggested_only=suggested_only,
-                max_rooms_per_space=max_rooms_per_space,
-                exclude_rooms=exclude_rooms,
-            )
-
-            try:
-                return FederationSpaceSummaryResult.from_json_dict(res)
-            except ValueError as e:
-                raise InvalidResponseError(str(e))
-
-        return await self._try_destination_list(
-            "fetch space summary",
-            destinations,
-            send_request,
-            failover_on_unknown_endpoint=True,
-        )
-
     async def get_room_hierarchy(
         self,
         destinations: Iterable[str],
@@ -1488,10 +1433,8 @@ class FederationClient(FederationBase):
             if any(not isinstance(e, dict) for e in children_state):
                 raise InvalidResponseError("Invalid event in 'children_state' list")
             try:
-                [
+                for child_state in children_state:
-                    FederationSpaceSummaryEventResult.from_json_dict(e)
+                    _validate_hierarchy_event(child_state)
-                    for e in children_state
-                ]
             except ValueError as e:
                 raise InvalidResponseError(str(e))
 
@@ -1513,62 +1456,12 @@ class FederationClient(FederationBase):
 
             return room, children_state, children, inaccessible_children
 
-        try:
         result = await self._try_destination_list(
             "fetch room hierarchy",
             destinations,
             send_request,
             failover_on_unknown_endpoint=True,
         )
-        except SynapseError as e:
-            # If an unexpected error occurred, re-raise it.
-            if e.code != 502:
-                raise
-
-            logger.debug(
-                "Couldn't fetch room hierarchy, falling back to the spaces API"
-            )
-
-            # Fallback to the old federation API and translate the results if
-            # no servers implement the new API.
-            #
-            # The algorithm below is a bit inefficient as it only attempts to
-            # parse information for the requested room, but the legacy API may
-            # return additional layers.
-            legacy_result = await self.get_space_summary(
-                destinations,
-                room_id,
-                suggested_only,
-                max_rooms_per_space=None,
-                exclude_rooms=[],
-            )
-
-            # Find the requested room in the response (and remove it).
-            for _i, room in enumerate(legacy_result.rooms):
-                if room.get("room_id") == room_id:
-                    break
-            else:
-                # The requested room was not returned, nothing we can do.
-                raise
-            requested_room = legacy_result.rooms.pop(_i)
-
-            # Find any children events of the requested room.
-            children_events = []
-            children_room_ids = set()
-            for event in legacy_result.events:
-                if event.room_id == room_id:
-                    children_events.append(event.data)
-                    children_room_ids.add(event.state_key)
-
-            # Find the children rooms.
-            children = []
-            for room in legacy_result.rooms:
-                if room.get("room_id") in children_room_ids:
-                    children.append(room)
-
-            # It isn't clear from the response whether some of the rooms are
-            # not accessible.
-            result = (requested_room, children_events, children, ())
 
         # Cache the result to avoid fetching data over federation every time.
         self._get_room_hierarchy_cache[(room_id, suggested_only)] = result
@@ -1710,26 +1603,8 @@ class TimestampToEventResponse:
         return cls(event_id, origin_server_ts, d)
 
 
-@attr.s(frozen=True, slots=True, auto_attribs=True)
+def _validate_hierarchy_event(d: JsonDict) -> None:
-class FederationSpaceSummaryEventResult:
+    """Validate an event within the result of a /hierarchy request
-    """Represents a single event in the result of a successful get_space_summary call.
-
-    It's essentially just a serialised event object, but we do a bit of parsing and
-    validation in `from_json_dict` and store some of the validated properties in
-    object attributes.
-    """
-
-    event_type: str
-    room_id: str
-    state_key: str
-    via: Sequence[str]
-
-    # the raw data, including the above keys
-    data: JsonDict
-
-    @classmethod
-    def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryEventResult":
-        """Parse an event within the result of a /spaces/ request
 
     Args:
         d: json object to be parsed
@@ -1759,40 +1634,3 @@ class FederationSpaceSummaryEventResult:
         raise ValueError("Invalid event: 'via' must be a list")
     if any(not isinstance(v, str) for v in via):
         raise ValueError("Invalid event: 'via' must be a list of strings")
-
-        return cls(event_type, room_id, state_key, via, d)
-
-
-@attr.s(frozen=True, slots=True, auto_attribs=True)
-class FederationSpaceSummaryResult:
-    """Represents the data returned by a successful get_space_summary call."""
-
-    rooms: List[JsonDict]
-    events: Sequence[FederationSpaceSummaryEventResult]
-
-    @classmethod
-    def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryResult":
-        """Parse the result of a /spaces/ request
-
-        Args:
-            d: json object to be parsed
-
-        Raises:
-            ValueError if d is not a valid /spaces/ response
-        """
-        rooms = d.get("rooms")
-        if not isinstance(rooms, List):
-            raise ValueError("'rooms' must be a list")
-        if any(not isinstance(r, dict) for r in rooms):
-            raise ValueError("Invalid room in 'rooms' list")
-
-        events = d.get("events")
-        if not isinstance(events, Sequence):
-            raise ValueError("'events' must be a list")
-        if any(not isinstance(e, dict) for e in events):
-            raise ValueError("Invalid event in 'events' list")
-        parsed_events = [
-            FederationSpaceSummaryEventResult.from_json_dict(e) for e in events
-        ]
-
-        return cls(rooms, parsed_events)

synapse/federation/transport/client.py

@@ -1179,39 +1179,6 @@ class TransportLayerClient:
 
         return await self.client.get_json(destination=destination, path=path)
 
-    async def get_space_summary(
-        self,
-        destination: str,
-        room_id: str,
-        suggested_only: bool,
-        max_rooms_per_space: Optional[int],
-        exclude_rooms: List[str],
-    ) -> JsonDict:
-        """
-        Args:
-            destination: The remote server
-            room_id: The room ID to ask about.
-            suggested_only: if True, only suggested rooms will be returned
-            max_rooms_per_space: an optional limit to the number of children to be
-               returned per space
-            exclude_rooms: a list of any rooms we can skip
-        """
-        # TODO When switching to the stable endpoint, use GET instead of POST.
-        path = _create_path(
-            FEDERATION_UNSTABLE_PREFIX, "/org.matrix.msc2946/spaces/%s", room_id
-        )
-
-        params = {
-            "suggested_only": suggested_only,
-            "exclude_rooms": exclude_rooms,
-        }
-        if max_rooms_per_space is not None:
-            params["max_rooms_per_space"] = max_rooms_per_space
-
-        return await self.client.post_json(
-            destination=destination, path=path, data=params
-        )
-
     async def get_room_hierarchy(
         self, destination: str, room_id: str, suggested_only: bool
     ) -> JsonDict:

synapse/federation/transport/server/federation.py

@@ -624,81 +624,6 @@ class FederationVersionServlet(BaseFederationServlet):
         )
 
 
-class FederationSpaceSummaryServlet(BaseFederationServlet):
-    PREFIX = FEDERATION_UNSTABLE_PREFIX + "/org.matrix.msc2946"
-    PATH = "/spaces/(?P<room_id>[^/]*)"
-
-    def __init__(
-        self,
-        hs: "HomeServer",
-        authenticator: Authenticator,
-        ratelimiter: FederationRateLimiter,
-        server_name: str,
-    ):
-        super().__init__(hs, authenticator, ratelimiter, server_name)
-        self.handler = hs.get_room_summary_handler()
-
-    async def on_GET(
-        self,
-        origin: str,
-        content: Literal[None],
-        query: Mapping[bytes, Sequence[bytes]],
-        room_id: str,
-    ) -> Tuple[int, JsonDict]:
-        suggested_only = parse_boolean_from_args(query, "suggested_only", default=False)
-
-        max_rooms_per_space = parse_integer_from_args(query, "max_rooms_per_space")
-        if max_rooms_per_space is not None and max_rooms_per_space < 0:
-            raise SynapseError(
-                400,
-                "Value for 'max_rooms_per_space' must be a non-negative integer",
-                Codes.BAD_JSON,
-            )
-
-        exclude_rooms = parse_strings_from_args(query, "exclude_rooms", default=[])
-
-        return 200, await self.handler.federation_space_summary(
-            origin, room_id, suggested_only, max_rooms_per_space, exclude_rooms
-        )
-
-    # TODO When switching to the stable endpoint, remove the POST handler.
-    async def on_POST(
-        self,
-        origin: str,
-        content: JsonDict,
-        query: Mapping[bytes, Sequence[bytes]],
-        room_id: str,
-    ) -> Tuple[int, JsonDict]:
-        suggested_only = content.get("suggested_only", False)
-        if not isinstance(suggested_only, bool):
-            raise SynapseError(
-                400, "'suggested_only' must be a boolean", Codes.BAD_JSON
-            )
-
-        exclude_rooms = content.get("exclude_rooms", [])
-        if not isinstance(exclude_rooms, list) or any(
-            not isinstance(x, str) for x in exclude_rooms
-        ):
-            raise SynapseError(400, "bad value for 'exclude_rooms'", Codes.BAD_JSON)
-
-        max_rooms_per_space = content.get("max_rooms_per_space")
-        if max_rooms_per_space is not None:
-            if not isinstance(max_rooms_per_space, int):
-                raise SynapseError(
-                    400, "bad value for 'max_rooms_per_space'", Codes.BAD_JSON
-                )
-            if max_rooms_per_space < 0:
-                raise SynapseError(
-                    400,
-                    "Value for 'max_rooms_per_space' must be a non-negative integer",
-                    Codes.BAD_JSON,
-                )
-
-        return 200, await self.handler.federation_space_summary(
-            origin, room_id, suggested_only, max_rooms_per_space, exclude_rooms
-        )
-
-
 class FederationRoomHierarchyServlet(BaseFederationServlet):
     PATH = "/hierarchy/(?P<room_id>[^/]*)"
 
@@ -826,7 +751,6 @@ FEDERATION_SERVLET_CLASSES: Tuple[Type[BaseFederationServlet], ...] = (
     On3pidBindServlet,
     FederationVersionServlet,
     RoomComplexityServlet,
-    FederationSpaceSummaryServlet,
     FederationRoomHierarchyServlet,
     FederationRoomHierarchyUnstableServlet,
     FederationV1SendKnockServlet,

synapse/handlers/room_summary.py

@@ -15,7 +15,6 @@
 import itertools
 import logging
 import re
-from collections import deque
 from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Sequence, Set, Tuple
 
 import attr
@@ -107,153 +106,6 @@ class RoomSummaryHandler:
             "get_room_hierarchy",
         )
 
-    async def get_space_summary(
-        self,
-        requester: str,
-        room_id: str,
-        suggested_only: bool = False,
-        max_rooms_per_space: Optional[int] = None,
-    ) -> JsonDict:
-        """
-        Implementation of the space summary C-S API
-
-        Args:
-            requester:  user id of the user making this request
-
-            room_id: room id to start the summary at
-
-            suggested_only: whether we should only return children with the "suggested"
-                flag set.
-
-            max_rooms_per_space: an optional limit on the number of child rooms we will
-                return. This does not apply to the root room (ie, room_id), and
-                is overridden by MAX_ROOMS_PER_SPACE.
-
-        Returns:
-            summary dict to return
-        """
-        # First of all, check that the room is accessible.
-        if not await self._is_local_room_accessible(room_id, requester):
-            raise AuthError(
-                403,
-                "User %s not in room %s, and room previews are disabled"
-                % (requester, room_id),
-            )
-
-        # the queue of rooms to process
-        room_queue = deque((_RoomQueueEntry(room_id, ()),))
-
-        # rooms we have already processed
-        processed_rooms: Set[str] = set()
-
-        # events we have already processed. We don't necessarily have their event ids,
-        # so instead we key on (room id, state key)
-        processed_events: Set[Tuple[str, str]] = set()
-
-        rooms_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:
-            queue_entry = room_queue.popleft()
-            room_id = queue_entry.room_id
-            if room_id in processed_rooms:
-                # already done this room
-                continue
-
-            logger.debug("Processing room %s", room_id)
-
-            is_in_room = await self._store.is_host_joined(room_id, self._server_name)
-
-            # 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
-            # first room we are processing.
-            max_children = max_rooms_per_space if processed_rooms else MAX_ROOMS
-
-            if is_in_room:
-                room_entry = await self._summarize_local_room(
-                    requester, None, room_id, suggested_only, max_children
-                )
-
-                events: Sequence[JsonDict] = []
-                if room_entry:
-                    rooms_result.append(room_entry.room)
-                    events = room_entry.children_state_events
-
-                logger.debug(
-                    "Query of local room %s returned events %s",
-                    room_id,
-                    ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
-                )
-            else:
-                fed_rooms = await self._summarize_remote_room(
-                    queue_entry,
-                    suggested_only,
-                    max_children,
-                    exclude_rooms=processed_rooms,
-                )
-
-                # The results over federation might include rooms that the we,
-                # as the requesting server, are allowed to see, but the requesting
-                # user is not permitted see.
-                #
-                # Filter the returned results to only what is accessible to the user.
-                events = []
-                for room_entry in fed_rooms:
-                    room = room_entry.room
-                    fed_room_id = room_entry.room_id
-
-                    # The user can see the room, include it!
-                    if await self._is_remote_room_accessible(
-                        requester, fed_room_id, room
-                    ):
-                        # Before returning to the client, remove the allowed_room_ids
-                        # and allowed_spaces keys.
-                        room.pop("allowed_room_ids", None)
-                        room.pop("allowed_spaces", None)  # historical
-
-                        rooms_result.append(room)
-                        events.extend(room_entry.children_state_events)
-
-                    # All rooms returned don't need visiting again (even if the user
-                    # didn't have access to them).
-                    processed_rooms.add(fed_room_id)
-
-                logger.debug(
-                    "Query of %s returned rooms %s, events %s",
-                    room_id,
-                    [room_entry.room.get("room_id") for room_entry in fed_rooms],
-                    ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
-                )
-
-            # the room we queried may or may not have been returned, but don't process
-            # it again, anyway.
-            processed_rooms.add(room_id)
-
-            # XXX: is it ok that we blindly iterate through any events returned by
-            #   a remote server, whether or not they actually link to any rooms in our
-            #   tree?
-            for ev in events:
-                # remote servers might return events we have already processed
-                # (eg, Dendrite returns inward pointers as well as outward ones), so
-                # we need to filter them out, to avoid returning duplicate links to the
-                # client.
-                ev_key = (ev["room_id"], ev["state_key"])
-                if ev_key in processed_events:
-                    continue
-                events_result.append(ev)
-
-                # add the child to the queue. we have already validated
-                # that the vias are a list of server names.
-                room_queue.append(
-                    _RoomQueueEntry(ev["state_key"], ev["content"]["via"])
-                )
-                processed_events.add(ev_key)
-
-        return {"rooms": rooms_result, "events": events_result}
-
     async def get_room_hierarchy(
         self,
         requester: Requester,
@@ -398,8 +250,6 @@ class RoomSummaryHandler:
                     None,
                     room_id,
                     suggested_only,
-                    # Do not limit the maximum children.
-                    max_children=None,
                 )
 
             # Otherwise, attempt to use information for federation.
@@ -488,74 +338,6 @@ class RoomSummaryHandler:
 
         return result
 
-    async def federation_space_summary(
-        self,
-        origin: str,
-        room_id: str,
-        suggested_only: bool,
-        max_rooms_per_space: Optional[int],
-        exclude_rooms: Iterable[str],
-    ) -> JsonDict:
-        """
-        Implementation of the space summary Federation API
-
-        Args:
-            origin: The server requesting the spaces summary.
-
-            room_id: room id to start the summary at
-
-            suggested_only: whether we should only return children with the "suggested"
-                flag set.
-
-            max_rooms_per_space: an optional limit on the number of child rooms we will
-                return. Unlike the C-S API, this applies to the root room (room_id).
-                It is clipped to MAX_ROOMS_PER_SPACE.
-
-            exclude_rooms: a list of rooms to skip over (presumably because the
-                calling server has already seen them).
-
-        Returns:
-            summary dict to return
-        """
-        # the queue of rooms to process
-        room_queue = deque((room_id,))
-
-        # the set of rooms that we should not walk further. Initialise it with the
-        # excluded-rooms list; we will add other rooms as we process them so that
-        # we do not loop.
-        processed_rooms: Set[str] = set(exclude_rooms)
-
-        rooms_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:
-            room_id = room_queue.popleft()
-            if room_id in processed_rooms:
-                # already done this room
-                continue
-
-            room_entry = await self._summarize_local_room(
-                None, origin, room_id, suggested_only, max_rooms_per_space
-            )
-
-            processed_rooms.add(room_id)
-
-            if room_entry:
-                rooms_result.append(room_entry.room)
-                events_result.extend(room_entry.children_state_events)
-
-                # add any children to the queue
-                room_queue.extend(
-                    edge_event["state_key"]
-                    for edge_event in room_entry.children_state_events
-                )
-
-        return {"rooms": rooms_result, "events": events_result}
-
     async def get_federation_hierarchy(
         self,
         origin: str,
@@ -579,7 +361,7 @@ class RoomSummaryHandler:
             The JSON hierarchy dictionary.
         """
         root_room_entry = await self._summarize_local_room(
-            None, origin, requested_room_id, suggested_only, max_children=None
+            None, origin, requested_room_id, suggested_only
         )
         if root_room_entry is None:
             # Room is inaccessible to the requesting server.
@@ -600,7 +382,7 @@ class RoomSummaryHandler:
                 continue
 
             room_entry = await self._summarize_local_room(
-                None, origin, room_id, suggested_only, max_children=0
+                None, origin, room_id, suggested_only, include_children=False
             )
             # If the room is accessible, include it in the results.
             #
@@ -626,7 +408,7 @@ class RoomSummaryHandler:
         origin: Optional[str],
         room_id: str,
         suggested_only: bool,
-        max_children: Optional[int],
+        include_children: bool = True,
     ) -> Optional["_RoomEntry"]:
         """
         Generate a room entry and a list of event entries for a given room.
@@ -641,9 +423,8 @@ class RoomSummaryHandler:
             room_id: The room ID to summarize.
             suggested_only: True if only suggested children should be returned.
                 Otherwise, all children are returned.
-            max_children:
+            include_children:
-                The maximum number of children rooms to include. A value of None
+                Whether to include the events of any children.
-                means no limit.
 
         Returns:
             A room entry if the room should be returned. None, otherwise.
@@ -653,9 +434,8 @@ class RoomSummaryHandler:
 
         room_entry = await self._build_room_entry(room_id, for_federation=bool(origin))
 
-        # If the room is not a space or the children don't matter, return just
+        # If the room is not a space return just the room information.
-        # the room information.
+        if room_entry.get("room_type") != RoomTypes.SPACE or not include_children:
-        if room_entry.get("room_type") != RoomTypes.SPACE or max_children == 0:
             return _RoomEntry(room_id, room_entry)
 
         # Otherwise, look for child rooms/spaces.
@@ -665,14 +445,6 @@ class RoomSummaryHandler:
             # we only care about suggested children
             child_events = filter(_is_suggested_child_event, child_events)
 
-        # TODO max_children is legacy code for the /spaces endpoint.
-        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] = [
             {
                 "type": e.type,
@@ -682,80 +454,10 @@ class RoomSummaryHandler:
                 "sender": e.sender,
                 "origin_server_ts": e.origin_server_ts,
             }
-            for e in child_iter
+            for e in child_events
         ]
         return _RoomEntry(room_id, room_entry, stripped_events)
 
-    async def _summarize_remote_room(
-        self,
-        room: "_RoomQueueEntry",
-        suggested_only: bool,
-        max_children: Optional[int],
-        exclude_rooms: Iterable[str],
-    ) -> Iterable["_RoomEntry"]:
-        """
-        Request room entries and a list of event entries for a given room by querying a remote server.
-
-        Args:
-            room: The room to summarize.
-            suggested_only: True if only suggested children should be returned.
-                Otherwise, all children are returned.
-            max_children:
-                The maximum number of children rooms to include. This is capped
-                to a server-set limit.
-            exclude_rooms:
-                Rooms IDs which do not need to be summarized.
-
-        Returns:
-            An iterable of room entries.
-        """
-        room_id = room.room_id
-        logger.info("Requesting summary for %s via %s", room_id, room.via)
-
-        # we need to make the exclusion list json-serialisable
-        exclude_rooms = list(exclude_rooms)
-
-        via = itertools.islice(room.via, MAX_SERVERS_PER_SPACE)
-        try:
-            res = await self._federation_client.get_space_summary(
-                via,
-                room_id,
-                suggested_only=suggested_only,
-                max_rooms_per_space=max_children,
-                exclude_rooms=exclude_rooms,
-            )
-        except Exception as e:
-            logger.warning(
-                "Unable to get summary of %s via federation: %s",
-                room_id,
-                e,
-                exc_info=logger.isEnabledFor(logging.DEBUG),
-            )
-            return ()
-
-        # Group the events by their room.
-        children_by_room: Dict[str, List[JsonDict]] = {}
-        for ev in res.events:
-            if ev.event_type == EventTypes.SpaceChild:
-                children_by_room.setdefault(ev.room_id, []).append(ev.data)
-
-        # Generate the final results.
-        results = []
-        for fed_room in res.rooms:
-            fed_room_id = fed_room.get("room_id")
-            if not fed_room_id or not isinstance(fed_room_id, str):
-                continue
-
-            results.append(
-                _RoomEntry(
-                    fed_room_id,
-                    fed_room,
-                    children_by_room.get(fed_room_id, []),
-                )
-            )
-
-        return results
-
     async def _summarize_remote_room_hierarchy(
         self, room: "_RoomQueueEntry", suggested_only: bool
     ) -> Tuple[Optional["_RoomEntry"], Dict[str, JsonDict], Set[str]]:
@@ -958,9 +660,8 @@ class RoomSummaryHandler:
         ):
             return True
 
-        # Check if the user is a member of any of the allowed spaces
+        # Check if the user is a member of any of the allowed rooms from the response.
-        # from the response.
+        allowed_rooms = room.get("allowed_room_ids")
-        allowed_rooms = room.get("allowed_room_ids") or room.get("allowed_spaces")
         if allowed_rooms and isinstance(allowed_rooms, list):
             if await self._event_auth_handler.is_user_in_rooms(
                 allowed_rooms, requester
@@ -1028,8 +729,6 @@ class RoomSummaryHandler:
                 )
                 if allowed_rooms:
                     entry["allowed_room_ids"] = allowed_rooms
-                    # TODO Remove this key once the API is stable.
-                    entry["allowed_spaces"] = allowed_rooms
 
         # Filter out Nones – rather omit the field altogether
         room_entry = {k: v for k, v in entry.items() if v is not None}
@@ -1094,7 +793,7 @@ class RoomSummaryHandler:
                 room_id,
                 # Suggested-only doesn't matter since no children are requested.
                 suggested_only=False,
-                max_children=0,
+                include_children=False,
             )
 
             if not room_entry:

synapse/rest/client/room.py

@@ -1141,73 +1141,6 @@ class TimestampLookupRestServlet(RestServlet):
         }
 
 
-class RoomSpaceSummaryRestServlet(RestServlet):
-    PATTERNS = (
-        re.compile(
-            "^/_matrix/client/unstable/org.matrix.msc2946"
-            "/rooms/(?P<room_id>[^/]*)/spaces$"
-        ),
-    )
-
-    def __init__(self, hs: "HomeServer"):
-        super().__init__()
-        self._auth = hs.get_auth()
-        self._room_summary_handler = hs.get_room_summary_handler()
-
-    async def on_GET(
-        self, request: SynapseRequest, room_id: str
-    ) -> Tuple[int, JsonDict]:
-        requester = await self._auth.get_user_by_req(request, allow_guest=True)
-
-        max_rooms_per_space = parse_integer(request, "max_rooms_per_space")
-        if max_rooms_per_space is not None and max_rooms_per_space < 0:
-            raise SynapseError(
-                400,
-                "Value for 'max_rooms_per_space' must be a non-negative integer",
-                Codes.BAD_JSON,
-            )
-
-        return 200, await self._room_summary_handler.get_space_summary(
-            requester.user.to_string(),
-            room_id,
-            suggested_only=parse_boolean(request, "suggested_only", default=False),
-            max_rooms_per_space=max_rooms_per_space,
-        )
-
-    # TODO When switching to the stable endpoint, remove the POST handler.
-    async def on_POST(
-        self, request: SynapseRequest, room_id: str
-    ) -> Tuple[int, JsonDict]:
-        requester = await self._auth.get_user_by_req(request, allow_guest=True)
-        content = parse_json_object_from_request(request)
-
-        suggested_only = content.get("suggested_only", False)
-        if not isinstance(suggested_only, bool):
-            raise SynapseError(
-                400, "'suggested_only' must be a boolean", Codes.BAD_JSON
-            )
-
-        max_rooms_per_space = content.get("max_rooms_per_space")
-        if max_rooms_per_space is not None:
-            if not isinstance(max_rooms_per_space, int):
-                raise SynapseError(
-                    400, "'max_rooms_per_space' must be an integer", Codes.BAD_JSON
-                )
-            if max_rooms_per_space < 0:
-                raise SynapseError(
-                    400,
-                    "Value for 'max_rooms_per_space' must be a non-negative integer",
-                    Codes.BAD_JSON,
-                )
-
-        return 200, await self._room_summary_handler.get_space_summary(
-            requester.user.to_string(),
-            room_id,
-            suggested_only=suggested_only,
-            max_rooms_per_space=max_rooms_per_space,
-        )
-
-
 class RoomHierarchyRestServlet(RestServlet):
     PATTERNS = (
         re.compile(
@@ -1301,7 +1234,6 @@ def register_servlets(
     RoomRedactEventRestServlet(hs).register(http_server)
     RoomTypingRestServlet(hs).register(http_server)
     RoomEventContextServlet(hs).register(http_server)
-    RoomSpaceSummaryRestServlet(hs).register(http_server)
     RoomHierarchyRestServlet(hs).register(http_server)
     if hs.config.experimental.msc3266_enabled:
         RoomSummaryRestServlet(hs).register(http_server)

tests/handlers/test_room_summary.py

@@ -157,35 +157,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             state_key=room_id,
         )
 
-    def _assert_rooms(
-        self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]]
-    ) -> None:
-        """
-        Assert that the expected room IDs and events are in the response.
-
-        Args:
-            result: The result from the API call.
-            rooms_and_children: An iterable of tuples where each tuple is:
-                The expected room ID.
-                The expected IDs of any children rooms.
-        """
-        room_ids = []
-        children_ids = []
-        for room_id, children in rooms_and_children:
-            room_ids.append(room_id)
-            if children:
-                children_ids.extend([(room_id, child_id) for child_id in children])
-        self.assertCountEqual(
-            [room.get("room_id") for room in result["rooms"]], room_ids
-        )
-        self.assertCountEqual(
-            [
-                (event.get("room_id"), event.get("state_key"))
-                for event in result["events"]
-            ],
-            children_ids,
-        )
-
     def _assert_hierarchy(
         self, result: JsonDict, rooms_and_children: Iterable[Tuple[str, Iterable[str]]]
     ) -> None:
@@ -251,11 +222,9 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
 
     def test_simple_space(self):
         """Test a simple space with a single room."""
-        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.
         expected = [(self.space, [self.room]), (self.room, ())]
-        self._assert_rooms(result, expected)
 
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(self.user), self.space)
@@ -271,12 +240,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             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]
@@ -300,10 +263,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         token2 = self.login("user2", "pass")
 
         # The user can see the space since it is publicly joinable.
-        result = self.get_success(self.handler.get_space_summary(user2, self.space))
         expected = [(self.space, [self.room]), (self.room, ())]
-        self._assert_rooms(result, expected)
-
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(user2), self.space)
         )
@@ -316,7 +276,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             body={"join_rule": JoinRules.INVITE},
             tok=self.token,
         )
-        self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)
         self.get_failure(
             self.handler.get_room_hierarchy(create_requester(user2), self.space),
             AuthError,
@@ -329,9 +288,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             body={"history_visibility": HistoryVisibility.WORLD_READABLE},
             tok=self.token,
         )
-        result = self.get_success(self.handler.get_space_summary(user2, self.space))
-        self._assert_rooms(result, expected)
-
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(user2), self.space)
         )
@@ -344,7 +300,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             body={"history_visibility": HistoryVisibility.JOINED},
             tok=self.token,
         )
-        self.get_failure(self.handler.get_space_summary(user2, self.space), AuthError)
         self.get_failure(
             self.handler.get_room_hierarchy(create_requester(user2), self.space),
             AuthError,
@@ -353,19 +308,12 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         # Join the space and results should be returned.
         self.helper.invite(self.space, targ=user2, tok=self.token)
         self.helper.join(self.space, user2, tok=token2)
-        result = self.get_success(self.handler.get_space_summary(user2, self.space))
-        self._assert_rooms(result, expected)
-
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(user2), self.space)
         )
         self._assert_hierarchy(result, expected)
 
         # Attempting to view an unknown room returns the same error.
-        self.get_failure(
-            self.handler.get_space_summary(user2, "#not-a-space:" + self.hs.hostname),
-            AuthError,
-        )
         self.get_failure(
             self.handler.get_room_hierarchy(
                 create_requester(user2), "#not-a-space:" + self.hs.hostname
@@ -496,7 +444,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
 
         # Join the space.
         self.helper.join(self.space, user2, tok=token2)
-        result = self.get_success(self.handler.get_space_summary(user2, self.space))
         expected = [
             (
                 self.space,
@@ -520,7 +467,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             (world_readable_room, ()),
             (joined_room, ()),
         ]
-        self._assert_rooms(result, expected)
 
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(user2), self.space)
@@ -554,8 +500,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         self._add_child(subspace, self.room, token=self.token)
         self._add_child(subspace, room2, self.token)
 
-        result = self.get_success(self.handler.get_space_summary(self.user, self.space))
-
         # The result should include each room a single time and each link.
         expected = [
             (self.space, [self.room, room2, subspace]),
@@ -563,7 +507,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             (subspace, [subroom, self.room, room2]),
             (subroom, ()),
         ]
-        self._assert_rooms(result, expected)
 
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(self.user), self.space)
@@ -728,10 +671,8 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             )
         )
 
-        result = self.get_success(self.handler.get_space_summary(self.user, self.space))
         # The result should have only the space, along with a link from space -> room.
         expected = [(self.space, [self.room])]
-        self._assert_rooms(result, expected)
 
         result = self.get_success(
             self.handler.get_room_hierarchy(create_requester(self.user), self.space)
@@ -775,41 +716,18 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             "world_readable": True,
         }
 
-        async def summarize_remote_room(
-            _self, room, suggested_only, max_children, exclude_rooms
-        ):
-            return [
-                requested_room_entry,
-                _RoomEntry(
-                    subroom,
-                    {
-                        "room_id": subroom,
-                        "world_readable": True,
-                    },
-                ),
-            ]
-
         async def summarize_remote_room_hierarchy(_self, room, suggested_only):
             return requested_room_entry, {subroom: child_room}, set()
 
         # Add a room to the space which is on another server.
         self._add_child(self.space, subspace, self.token)
 
-        with mock.patch(
-            "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room",
-            new=summarize_remote_room,
-        ):
-            result = self.get_success(
-                self.handler.get_space_summary(self.user, self.space)
-            )
-
         expected = [
             (self.space, [self.room, subspace]),
             (self.room, ()),
             (subspace, [subroom]),
             (subroom, ()),
         ]
-        self._assert_rooms(result, expected)
 
         with mock.patch(
             "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy",
@@ -881,7 +799,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
                     "room_id": restricted_room,
                     "world_readable": False,
                     "join_rules": JoinRules.RESTRICTED,
-                    "allowed_spaces": [],
+                    "allowed_room_ids": [],
                 },
             ),
             (
@@ -890,7 +808,7 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
                     "room_id": restricted_accessible_room,
                     "world_readable": False,
                     "join_rules": JoinRules.RESTRICTED,
-                    "allowed_spaces": [self.room],
+                    "allowed_room_ids": [self.room],
                 },
             ),
             (
@@ -929,30 +847,12 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-        async def summarize_remote_room(
-            _self, room, suggested_only, max_children, exclude_rooms
-        ):
-            return [subspace_room_entry] + [
-                # A copy is made of the room data since the allowed_spaces key
-                # is removed.
-                _RoomEntry(child_room[0], dict(child_room[1]))
-                for child_room in children_rooms
-            ]
-
         async def summarize_remote_room_hierarchy(_self, room, suggested_only):
             return subspace_room_entry, dict(children_rooms), set()
 
         # Add a room to the space which is on another server.
         self._add_child(self.space, subspace, self.token)
 
-        with mock.patch(
-            "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room",
-            new=summarize_remote_room,
-        ):
-            result = self.get_success(
-                self.handler.get_space_summary(self.user, self.space)
-            )
-
         expected = [
             (self.space, [self.room, subspace]),
             (self.room, ()),
@@ -976,7 +876,6 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             (world_readable_room, ()),
             (joined_room, ()),
         ]
-        self._assert_rooms(result, expected)
 
         with mock.patch(
             "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy",
@@ -1010,31 +909,17 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
             },
         )
 
-        async def summarize_remote_room(
-            _self, room, suggested_only, max_children, exclude_rooms
-        ):
-            return [fed_room_entry]
-
         async def summarize_remote_room_hierarchy(_self, room, suggested_only):
             return fed_room_entry, {}, set()
 
         # Add a room to the space which is on another server.
         self._add_child(self.space, fed_room, self.token)
 
-        with mock.patch(
-            "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room",
-            new=summarize_remote_room,
-        ):
-            result = self.get_success(
-                self.handler.get_space_summary(self.user, self.space)
-            )
-
         expected = [
             (self.space, [self.room, fed_room]),
             (self.room, ()),
             (fed_room, ()),
         ]
-        self._assert_rooms(result, expected)
 
         with mock.patch(
             "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy",