From e24cf423fd0493fd2726c03ae8411ba003f862f8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Mar 2021 16:52:01 +0000 Subject: [PATCH 1/7] Use an attrs object for the room queue We'll soon want to track `via`s too. --- synapse/handlers/space_summary.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 513dc0c71a02..a1799e4e25c2 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -18,6 +18,8 @@ from collections import deque from typing import TYPE_CHECKING, Iterable, List, Optional, Set +import attr + from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility from synapse.api.errors import AuthError from synapse.events import EventBase @@ -66,7 +68,7 @@ async def get_space_summary( 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 ROOMS_PER_SPACE_LIMIT. + is overridden by MAX_ROOMS_PER_SPACE. Returns: summary dict to return @@ -76,7 +78,7 @@ async def get_space_summary( await self._auth.check_user_in_room_or_world_readable(room_id, requester) # the queue of rooms to process - room_queue = deque((room_id,)) + room_queue = deque((_RoomQueueEntry(room_id),)) processed_rooms = set() # type: Set[str] @@ -86,7 +88,8 @@ async def get_space_summary( now = self._clock.time_msec() while room_queue and len(rooms_result) < MAX_ROOMS: - room_id = room_queue.popleft() + queue_entry = room_queue.popleft() + room_id = queue_entry.room_id logger.debug("Processing room %s", room_id) processed_rooms.add(room_id) @@ -191,6 +194,11 @@ async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: return (e for e in events if e.content.get("via")) +@attr.s(frozen=True, slots=True) +class _RoomQueueEntry: + room_id = attr.ib(type=str) + + def _is_suggested_child_event(edge_event: EventBase) -> bool: suggested = edge_event.content.get("suggested") if isinstance(suggested, bool) and suggested: From 1f23a8653335da0930233227ef04d82b7deb743e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Mar 2021 16:59:41 +0000 Subject: [PATCH 2/7] Factor out `_is_room_accessible` --- synapse/handlers/space_summary.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index a1799e4e25c2..c8db0f34f1fb 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -93,16 +93,7 @@ async def get_space_summary( logger.debug("Processing room %s", room_id) processed_rooms.add(room_id) - try: - await self._auth.check_user_in_room_or_world_readable( - room_id, requester - ) - except AuthError: - logger.info( - "user %s cannot view room %s, omitting from summary", - requester, - room_id, - ) + if not await self._is_room_accessible(room_id, requester): continue room_entry = await self._build_room_entry(room_id) @@ -141,6 +132,20 @@ async def get_space_summary( return {"rooms": rooms_result, "events": events_result} + async def _is_room_accessible(self, room_id: str, requester: str) -> bool: + try: + await self._auth.check_user_in_room_or_world_readable(room_id, requester) + return True + except AuthError: + pass + + logger.info( + "room %s is unpeekable and user %s is not a member, omitting from summary", + room_id, + requester, + ) + return False + async def _build_room_entry(self, room_id: str) -> JsonDict: """Generate en entry suitable for the 'rooms' list in the summary response""" stats = await self._store.get_room_with_stats(room_id) From a2ae03b0b7e4ba9c265f5fcd1af77732dde11808 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Mar 2021 17:07:48 +0000 Subject: [PATCH 3/7] Factor out _summarize_local_room --- synapse/handlers/space_summary.py | 82 ++++++++++++++++++------------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index c8db0f34f1fb..666ab3aba25a 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -16,7 +16,7 @@ import itertools import logging from collections import deque -from typing import TYPE_CHECKING, Iterable, List, Optional, Set +from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple import attr @@ -85,52 +85,64 @@ async def get_space_summary( rooms_result = [] # type: List[JsonDict] events_result = [] # type: List[JsonDict] - now = self._clock.time_msec() - while room_queue and len(rooms_result) < MAX_ROOMS: queue_entry = room_queue.popleft() room_id = queue_entry.room_id logger.debug("Processing room %s", room_id) processed_rooms.add(room_id) - if not await self._is_room_accessible(room_id, requester): - continue + # 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 None - room_entry = await self._build_room_entry(room_id) - rooms_result.append(room_entry) + rooms, events = await self._summarize_local_room( + requester, room_id, suggested_only, max_children + ) - # look for child rooms/spaces. - child_events = await self._get_child_events(room_id) + rooms_result.extend(rooms) + events_result.extend(events) - if suggested_only: - # we only care about suggested children - child_events = filter(_is_suggested_child_event, child_events) + # add any children that we haven't already processed to the queue + for edge_event in events: + if edge_event["state_key"] not in processed_rooms: + room_queue.append(_RoomQueueEntry(edge_event["state_key"])) - # 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. Otherwise, apply any client-specified - # limit, capping to our built-in limit. - if max_rooms_per_space is not None and len(processed_rooms) > 1: - max_rooms = min(MAX_ROOMS_PER_SPACE, max_rooms_per_space) - else: - max_rooms = MAX_ROOMS_PER_SPACE - - for edge_event in itertools.islice(child_events, max_rooms): - edge_room_id = edge_event.state_key - - events_result.append( - await self._event_serializer.serialize_event( - edge_event, - time_now=now, - event_format=format_event_for_client_v2, - ) - ) + return {"rooms": rooms_result, "events": events_result} + + async def _summarize_local_room( + self, + requester: str, + room_id: str, + suggested_only: bool, + max_children: Optional[int], + ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]: + if not await self._is_room_accessible(room_id, requester): + return (), () - # if we haven't yet visited the target of this link, add it to the queue - if edge_room_id not in processed_rooms: - room_queue.append(edge_room_id) + room_entry = await self._build_room_entry(room_id) - return {"rooms": rooms_result, "events": events_result} + # look for child rooms/spaces. + child_events = await self._get_child_events(room_id) + + if suggested_only: + # we only care about suggested children + child_events = filter(_is_suggested_child_event, child_events) + + if max_children is None or max_children > MAX_ROOMS_PER_SPACE: + max_children = MAX_ROOMS_PER_SPACE + + now = self._clock.time_msec() + events_result = [] # type: List[JsonDict] + for edge_event in itertools.islice(child_events, max_children): + events_result.append( + await self._event_serializer.serialize_event( + edge_event, + time_now=now, + event_format=format_event_for_client_v2, + ) + ) + return (room_entry,), events_result async def _is_room_accessible(self, room_id: str, requester: str) -> bool: try: From 93aba1713c29445708be22296065881cde6ed63d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Mar 2021 17:36:38 +0000 Subject: [PATCH 4/7] server side of federated summary API --- synapse/federation/transport/server.py | 70 +++++++++++++++++++--- synapse/handlers/space_summary.py | 83 +++++++++++++++++++++++--- 2 files changed, 137 insertions(+), 16 deletions(-) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 2cf935f38db5..8d6952bfe910 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -18,7 +18,7 @@ import functools import logging import re -from typing import Optional, Tuple, Type +from typing import TYPE_CHECKING, Container, Mapping, Optional, Sequence, Tuple, Type import synapse from synapse.api.constants import MAX_GROUP_CATEGORYID_LENGTH, MAX_GROUP_ROLEID_LENGTH @@ -29,7 +29,7 @@ FEDERATION_V1_PREFIX, FEDERATION_V2_PREFIX, ) -from synapse.http.server import JsonResource +from synapse.http.server import HttpServer, JsonResource from synapse.http.servlet import ( parse_boolean_from_args, parse_integer_from_args, @@ -44,10 +44,14 @@ whitelisted_homeserver, ) from synapse.server import HomeServer -from synapse.types import ThirdPartyInstanceID, get_domain_from_id +from synapse.types import JsonDict, ThirdPartyInstanceID, get_domain_from_id +from synapse.util.ratelimitutils import FederationRateLimiter from synapse.util.stringutils import parse_and_validate_server_name from synapse.util.versionstring import get_version_string +if TYPE_CHECKING: + import synapse.server + logger = logging.getLogger(__name__) @@ -1376,6 +1380,40 @@ async def on_PUT(self, origin, content, query, group_id): return 200, new_content +class FederationSpaceSummaryServlet(BaseFederationServlet): + PREFIX = FEDERATION_UNSTABLE_PREFIX + "/org.matrix.msc2946" + PATH = "/spaces/(?P[^/]*)" + + 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 and not isinstance(max_rooms_per_space, int): + raise SynapseError( + 400, "bad value for 'max_rooms_per_space'", Codes.BAD_JSON + ) + + return 200, await self.handler.federation_space_summary( + room_id, suggested_only, max_rooms_per_space, exclude_rooms + ) + + class RoomComplexityServlet(BaseFederationServlet): """ Indicates to other servers how complex (and therefore likely @@ -1474,18 +1512,24 @@ async def on_GET(self, origin, content, query, room_id): ) -def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=None): +def register_servlets( + hs: "synapse.server.HomeServer", + resource: HttpServer, + authenticator: Authenticator, + ratelimiter: FederationRateLimiter, + servlet_groups: Optional[Container[str]] = None, +): """Initialize and register servlet classes. Will by default register all servlets. For custom behaviour, pass in a list of servlet_groups to register. Args: - hs (synapse.server.HomeServer): homeserver - resource (JsonResource): resource class to register to - authenticator (Authenticator): authenticator to use - ratelimiter (util.ratelimitutils.FederationRateLimiter): ratelimiter to use - servlet_groups (list[str], optional): List of servlet groups to register. + hs: homeserver + resource: resource class to register to + authenticator: authenticator to use + ratelimiter: ratelimiter to use + servlet_groups: List of servlet groups to register. Defaults to ``DEFAULT_SERVLET_GROUPS``. """ if not servlet_groups: @@ -1500,6 +1544,14 @@ def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=N server_name=hs.hostname, ).register(resource) + if hs.config.experimental.spaces_enabled: + FederationSpaceSummaryServlet( + handler=hs.get_space_summary_handler(), + authenticator=authenticator, + ratelimiter=ratelimiter, + server_name=hs.hostname, + ).register(resource) + if "openid" in servlet_groups: for servletclass in OPENID_SERVLET_CLASSES: servletclass( diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 666ab3aba25a..826ded35652e 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -56,7 +56,7 @@ async def get_space_summary( max_rooms_per_space: Optional[int] = None, ) -> JsonDict: """ - Implementation of the space summary API + Implementation of the space summary C-S API Args: requester: user id of the user making this request @@ -110,9 +110,65 @@ async def get_space_summary( return {"rooms": rooms_result, "events": events_result} + async def federation_space_summary( + self, + 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: + 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(exclude_rooms) # type: Set[str] + + rooms_result = [] # type: List[JsonDict] + events_result = [] # type: List[JsonDict] + + while room_queue and len(rooms_result) < MAX_ROOMS: + room_id = room_queue.popleft() + logger.debug("Processing room %s", room_id) + processed_rooms.add(room_id) + + rooms, events = await self._summarize_local_room( + None, room_id, suggested_only, max_rooms_per_space + ) + + rooms_result.extend(rooms) + events_result.extend(events) + + # add any children that we haven't already processed to the queue + for edge_event in events: + if edge_event["state_key"] not in processed_rooms: + room_queue.append(edge_event["state_key"]) + + return {"rooms": rooms_result, "events": events_result} + async def _summarize_local_room( self, - requester: str, + requester: Optional[str], room_id: str, suggested_only: bool, max_children: Optional[int], @@ -144,12 +200,25 @@ async def _summarize_local_room( ) return (room_entry,), events_result - async def _is_room_accessible(self, room_id: str, requester: str) -> bool: - try: - await self._auth.check_user_in_room_or_world_readable(room_id, requester) + async def _is_room_accessible(self, room_id: str, requester: Optional[str]) -> bool: + # if we have an authenticated requesting user, first check if they are in the + # room + if requester: + try: + await self._auth.check_user_in_room(room_id, requester) + return True + except AuthError: + pass + + # otherwise, check if the room is peekable + hist_vis = "" + hist_vis_ev = await self._state_handler.get_current_state( + room_id, EventTypes.RoomHistoryVisibility, "" + ) + if hist_vis_ev: + hist_vis = hist_vis_ev.content.get("history_visibility") + if hist_vis == HistoryVisibility.WORLD_READABLE: return True - except AuthError: - pass logger.info( "room %s is unpeekable and user %s is not a member, omitting from summary", From 42555b15338f0ec0dee0e7758d94cab9327d3a32 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 19 Mar 2021 17:53:19 +0000 Subject: [PATCH 5/7] changelog --- changelog.d/9652.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9652.feature diff --git a/changelog.d/9652.feature b/changelog.d/9652.feature new file mode 100644 index 000000000000..2f7ccedcfbb8 --- /dev/null +++ b/changelog.d/9652.feature @@ -0,0 +1 @@ +Add initial experimental support for a "space summary" API. From 201e70a9f84308dfc1769369dd134de898811fe8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 22 Mar 2021 17:54:53 +0000 Subject: [PATCH 6/7] Address review comments --- synapse/federation/transport/server.py | 5 +---- synapse/handlers/space_summary.py | 5 ++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 8d6952bfe910..aa508bdb6ef1 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -49,9 +49,6 @@ from synapse.util.stringutils import parse_and_validate_server_name from synapse.util.versionstring import get_version_string -if TYPE_CHECKING: - import synapse.server - logger = logging.getLogger(__name__) @@ -1513,7 +1510,7 @@ async def on_GET(self, origin, content, query, room_id): def register_servlets( - hs: "synapse.server.HomeServer", + hs: HomeServer, resource: HttpServer, authenticator: Authenticator, ratelimiter: FederationRateLimiter, diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index 826ded35652e..f5ead9447f99 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -211,14 +211,13 @@ async def _is_room_accessible(self, room_id: str, requester: Optional[str]) -> b pass # otherwise, check if the room is peekable - hist_vis = "" hist_vis_ev = await self._state_handler.get_current_state( room_id, EventTypes.RoomHistoryVisibility, "" ) if hist_vis_ev: hist_vis = hist_vis_ev.content.get("history_visibility") - if hist_vis == HistoryVisibility.WORLD_READABLE: - return True + if hist_vis == HistoryVisibility.WORLD_READABLE: + return True logger.info( "room %s is unpeekable and user %s is not a member, omitting from summary", From 2fc07f43622e95c88e4e3e4aea52514dfbae844f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 22 Mar 2021 18:15:57 +0000 Subject: [PATCH 7/7] fix lint --- synapse/federation/transport/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index aa508bdb6ef1..84e39c5a468d 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -18,7 +18,7 @@ import functools import logging import re -from typing import TYPE_CHECKING, Container, Mapping, Optional, Sequence, Tuple, Type +from typing import Container, Mapping, Optional, Sequence, Tuple, Type import synapse from synapse.api.constants import MAX_GROUP_CATEGORYID_LENGTH, MAX_GROUP_ROLEID_LENGTH