Refactoring before implementing the updated spaces summary. (#10527)

This should have no user-visible changes, but refactors some pieces of
the SpaceSummaryHandler before adding support for the updated
MSC2946.
This commit is contained in:
Patrick Cloke 2021-08-05 08:39:17 -04:00 committed by GitHub
parent a8a27b2b8b
commit 3b354faad0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 198 additions and 136 deletions

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

@ -0,0 +1 @@
Prepare for the new spaces summary endpoint (updates to [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946)).

View File

@ -1290,7 +1290,7 @@ class FederationClient(FederationBase):
) )
@attr.s(frozen=True, slots=True) @attr.s(frozen=True, slots=True, auto_attribs=True)
class FederationSpaceSummaryEventResult: class FederationSpaceSummaryEventResult:
"""Represents a single event in the result of a successful get_space_summary call. """Represents a single event in the result of a successful get_space_summary call.
@ -1299,12 +1299,13 @@ class FederationSpaceSummaryEventResult:
object attributes. object attributes.
""" """
event_type = attr.ib(type=str) event_type: str
state_key = attr.ib(type=str) room_id: str
via = attr.ib(type=Sequence[str]) state_key: str
via: Sequence[str]
# the raw data, including the above keys # the raw data, including the above keys
data = attr.ib(type=JsonDict) data: JsonDict
@classmethod @classmethod
def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryEventResult": def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryEventResult":
@ -1321,6 +1322,10 @@ class FederationSpaceSummaryEventResult:
if not isinstance(event_type, str): if not isinstance(event_type, str):
raise ValueError("Invalid event: 'event_type' must be a str") raise ValueError("Invalid event: 'event_type' must be a str")
room_id = d.get("room_id")
if not isinstance(room_id, str):
raise ValueError("Invalid event: 'room_id' must be a str")
state_key = d.get("state_key") state_key = d.get("state_key")
if not isinstance(state_key, str): if not isinstance(state_key, str):
raise ValueError("Invalid event: 'state_key' must be a str") raise ValueError("Invalid event: 'state_key' must be a str")
@ -1335,15 +1340,15 @@ class FederationSpaceSummaryEventResult:
if any(not isinstance(v, str) for v in via): if any(not isinstance(v, str) for v in via):
raise ValueError("Invalid event: 'via' must be a list of strings") raise ValueError("Invalid event: 'via' must be a list of strings")
return cls(event_type, state_key, via, d) return cls(event_type, room_id, state_key, via, d)
@attr.s(frozen=True, slots=True) @attr.s(frozen=True, slots=True, auto_attribs=True)
class FederationSpaceSummaryResult: class FederationSpaceSummaryResult:
"""Represents the data returned by a successful get_space_summary call.""" """Represents the data returned by a successful get_space_summary call."""
rooms = attr.ib(type=Sequence[JsonDict]) rooms: Sequence[JsonDict]
events = attr.ib(type=Sequence[FederationSpaceSummaryEventResult]) events: Sequence[FederationSpaceSummaryEventResult]
@classmethod @classmethod
def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryResult": def from_json_dict(cls, d: JsonDict) -> "FederationSpaceSummaryResult":

View File

@ -16,7 +16,17 @@ import itertools
import logging import logging
import re import re
from collections import deque from collections import deque
from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple from typing import (
TYPE_CHECKING,
Collection,
Dict,
Iterable,
List,
Optional,
Sequence,
Set,
Tuple,
)
import attr import attr
@ -116,20 +126,22 @@ class SpaceSummaryHandler:
max_children = max_rooms_per_space if processed_rooms else None max_children = max_rooms_per_space if processed_rooms else None
if is_in_room: if is_in_room:
room, events = await self._summarize_local_room( room_entry = await self._summarize_local_room(
requester, None, room_id, suggested_only, max_children requester, None, room_id, suggested_only, max_children
) )
events: Collection[JsonDict] = []
if room_entry:
rooms_result.append(room_entry.room)
events = room_entry.children
logger.debug( logger.debug(
"Query of local room %s returned events %s", "Query of local room %s returned events %s",
room_id, room_id,
["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events], ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
) )
if room:
rooms_result.append(room)
else: else:
fed_rooms, fed_events = await self._summarize_remote_room( fed_rooms = await self._summarize_remote_room(
queue_entry, queue_entry,
suggested_only, suggested_only,
max_children, max_children,
@ -141,12 +153,10 @@ class SpaceSummaryHandler:
# user is not permitted see. # user is not permitted see.
# #
# Filter the returned results to only what is accessible to the user. # Filter the returned results to only what is accessible to the user.
room_ids = set()
events = [] events = []
for room in fed_rooms: for room_entry in fed_rooms:
fed_room_id = room.get("room_id") room = room_entry.room
if not fed_room_id or not isinstance(fed_room_id, str): fed_room_id = room_entry.room_id
continue
# The room should only be included in the summary if: # The room should only be included in the summary if:
# a. the user is in the room; # a. the user is in the room;
@ -189,21 +199,17 @@ class SpaceSummaryHandler:
# The user can see the room, include it! # The user can see the room, include it!
if include_room: if include_room:
rooms_result.append(room) rooms_result.append(room)
room_ids.add(fed_room_id) events.extend(room_entry.children)
# All rooms returned don't need visiting again (even if the user # All rooms returned don't need visiting again (even if the user
# didn't have access to them). # didn't have access to them).
processed_rooms.add(fed_room_id) processed_rooms.add(fed_room_id)
for event in fed_events:
if event.get("room_id") in room_ids:
events.append(event)
logger.debug( logger.debug(
"Query of %s returned rooms %s, events %s", "Query of %s returned rooms %s, events %s",
room_id, room_id,
[room.get("room_id") for room in fed_rooms], [room_entry.room.get("room_id") for room_entry in fed_rooms],
["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in fed_events], ["%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 # the room we queried may or may not have been returned, but don't process
@ -283,20 +289,20 @@ class SpaceSummaryHandler:
# already done this room # already done this room
continue continue
logger.debug("Processing room %s", room_id) room_entry = await self._summarize_local_room(
room, events = await self._summarize_local_room(
None, origin, room_id, suggested_only, max_rooms_per_space None, origin, room_id, suggested_only, max_rooms_per_space
) )
processed_rooms.add(room_id) processed_rooms.add(room_id)
if room: if room_entry:
rooms_result.append(room) rooms_result.append(room_entry.room)
events_result.extend(events) events_result.extend(room_entry.children)
# add any children to the queue # add any children to the queue
room_queue.extend(edge_event["state_key"] for edge_event in events) room_queue.extend(
edge_event["state_key"] for edge_event in room_entry.children
)
return {"rooms": rooms_result, "events": events_result} return {"rooms": rooms_result, "events": events_result}
@ -307,7 +313,7 @@ class SpaceSummaryHandler:
room_id: str, room_id: str,
suggested_only: bool, suggested_only: bool,
max_children: Optional[int], max_children: Optional[int],
) -> Tuple[Optional[JsonDict], Sequence[JsonDict]]: ) -> Optional["_RoomEntry"]:
""" """
Generate a room entry and a list of event entries for a given room. Generate a room entry and a list of event entries for a given room.
@ -326,21 +332,16 @@ class SpaceSummaryHandler:
to a server-set limit. to a server-set limit.
Returns: Returns:
A tuple of: A room entry if the room should be returned. None, otherwise.
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.
""" """
if not await self._is_room_accessible(room_id, requester, origin): if not await self._is_room_accessible(room_id, requester, origin):
return None, () return None
room_entry = await self._build_room_entry(room_id) room_entry = await self._build_room_entry(room_id)
# If the room is not a space, return just the room information. # If the room is not a space, return just the room information.
if room_entry.get("room_type") != RoomTypes.SPACE: if room_entry.get("room_type") != RoomTypes.SPACE:
return room_entry, () return _RoomEntry(room_id, room_entry)
# Otherwise, look for child rooms/spaces. # Otherwise, look for child rooms/spaces.
child_events = await self._get_child_events(room_id) child_events = await self._get_child_events(room_id)
@ -363,7 +364,7 @@ class SpaceSummaryHandler:
) )
) )
return room_entry, events_result return _RoomEntry(room_id, room_entry, events_result)
async def _summarize_remote_room( async def _summarize_remote_room(
self, self,
@ -371,7 +372,7 @@ class SpaceSummaryHandler:
suggested_only: bool, suggested_only: bool,
max_children: Optional[int], max_children: Optional[int],
exclude_rooms: Iterable[str], exclude_rooms: Iterable[str],
) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]: ) -> Iterable["_RoomEntry"]:
""" """
Request room entries and a list of event entries for a given room by querying a remote server. Request room entries and a list of event entries for a given room by querying a remote server.
@ -386,11 +387,7 @@ class SpaceSummaryHandler:
Rooms IDs which do not need to be summarized. Rooms IDs which do not need to be summarized.
Returns: Returns:
A tuple of: An iterable of room entries.
An iterable of rooms.
An iterable of the sorted children events. This may be limited
to a maximum size or may include all children.
""" """
room_id = room.room_id room_id = room.room_id
logger.info("Requesting summary for %s via %s", room_id, room.via) logger.info("Requesting summary for %s via %s", room_id, room.via)
@ -414,11 +411,30 @@ class SpaceSummaryHandler:
e, e,
exc_info=logger.isEnabledFor(logging.DEBUG), exc_info=logger.isEnabledFor(logging.DEBUG),
) )
return (), () return ()
return res.rooms, tuple( # Group the events by their room.
ev.data for ev in res.events if ev.event_type == EventTypes.SpaceChild 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 _is_room_accessible( async def _is_room_accessible(
self, room_id: str, requester: Optional[str], origin: Optional[str] self, room_id: str, requester: Optional[str], origin: Optional[str]
@ -606,10 +622,21 @@ class SpaceSummaryHandler:
return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key) return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key)
@attr.s(frozen=True, slots=True) @attr.s(frozen=True, slots=True, auto_attribs=True)
class _RoomQueueEntry: class _RoomQueueEntry:
room_id = attr.ib(type=str) room_id: str
via = attr.ib(type=Sequence[str]) via: Sequence[str]
@attr.s(frozen=True, slots=True, auto_attribs=True)
class _RoomEntry:
room_id: str
# The room summary for this room.
room: JsonDict
# An iterable of the sorted, stripped children events for children of this room.
#
# This may not include all children.
children: Collection[JsonDict] = ()
def _has_valid_via(e: EventBase) -> bool: def _has_valid_via(e: EventBase) -> bool:

View File

@ -26,7 +26,7 @@ from synapse.api.constants import (
from synapse.api.errors import AuthError from synapse.api.errors import AuthError
from synapse.api.room_versions import RoomVersions from synapse.api.room_versions import RoomVersions
from synapse.events import make_event_from_dict from synapse.events import make_event_from_dict
from synapse.handlers.space_summary import _child_events_comparison_key from synapse.handlers.space_summary import _child_events_comparison_key, _RoomEntry
from synapse.rest import admin from synapse.rest import admin
from synapse.rest.client.v1 import login, room from synapse.rest.client.v1 import login, room
from synapse.server import HomeServer from synapse.server import HomeServer
@ -351,26 +351,30 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
# events before child events). # events before child events).
# Note that these entries are brief, but should contain enough info. # Note that these entries are brief, but should contain enough info.
rooms = [ return [
{ _RoomEntry(
"room_id": subspace, subspace,
"world_readable": True, {
"room_type": RoomTypes.SPACE, "room_id": subspace,
}, "world_readable": True,
{ "room_type": RoomTypes.SPACE,
"room_id": subroom, },
"world_readable": True, [
}, {
"room_id": subspace,
"state_key": subroom,
"content": {"via": [fed_hostname]},
}
],
),
_RoomEntry(
subroom,
{
"room_id": subroom,
"world_readable": True,
},
),
] ]
event_content = {"via": [fed_hostname]}
events = [
{
"room_id": subspace,
"state_key": subroom,
"content": event_content,
},
]
return rooms, events
# Add a room to the space which is on another server. # Add a room to the space which is on another server.
self._add_child(self.space, subspace, self.token) self._add_child(self.space, subspace, self.token)
@ -436,70 +440,95 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
): ):
# Note that these entries are brief, but should contain enough info. # Note that these entries are brief, but should contain enough info.
rooms = [ rooms = [
{ _RoomEntry(
"room_id": public_room, public_room,
"world_readable": False, {
"join_rules": JoinRules.PUBLIC, "room_id": public_room,
}, "world_readable": False,
{ "join_rules": JoinRules.PUBLIC,
"room_id": knock_room, },
"world_readable": False, ),
"join_rules": JoinRules.KNOCK, _RoomEntry(
}, knock_room,
{ {
"room_id": not_invited_room, "room_id": knock_room,
"world_readable": False, "world_readable": False,
"join_rules": JoinRules.INVITE, "join_rules": JoinRules.KNOCK,
}, },
{ ),
"room_id": invited_room, _RoomEntry(
"world_readable": False, not_invited_room,
"join_rules": JoinRules.INVITE, {
}, "room_id": not_invited_room,
{ "world_readable": False,
"room_id": restricted_room, "join_rules": JoinRules.INVITE,
"world_readable": False, },
"join_rules": JoinRules.MSC3083_RESTRICTED, ),
"allowed_spaces": [], _RoomEntry(
}, invited_room,
{ {
"room_id": restricted_accessible_room, "room_id": invited_room,
"world_readable": False, "world_readable": False,
"join_rules": JoinRules.MSC3083_RESTRICTED, "join_rules": JoinRules.INVITE,
"allowed_spaces": [self.room], },
}, ),
{ _RoomEntry(
"room_id": world_readable_room, restricted_room,
"world_readable": True, {
"join_rules": JoinRules.INVITE, "room_id": restricted_room,
}, "world_readable": False,
{ "join_rules": JoinRules.MSC3083_RESTRICTED,
"room_id": joined_room, "allowed_spaces": [],
"world_readable": False, },
"join_rules": JoinRules.INVITE, ),
}, _RoomEntry(
] restricted_accessible_room,
{
# Place each room in the sub-space. "room_id": restricted_accessible_room,
event_content = {"via": [fed_hostname]} "world_readable": False,
events = [ "join_rules": JoinRules.MSC3083_RESTRICTED,
{ "allowed_spaces": [self.room],
"room_id": subspace, },
"state_key": room["room_id"], ),
"content": event_content, _RoomEntry(
} world_readable_room,
for room in rooms {
"room_id": world_readable_room,
"world_readable": True,
"join_rules": JoinRules.INVITE,
},
),
_RoomEntry(
joined_room,
{
"room_id": joined_room,
"world_readable": False,
"join_rules": JoinRules.INVITE,
},
),
] ]
# Also include the subspace. # Also include the subspace.
rooms.insert( rooms.insert(
0, 0,
{ _RoomEntry(
"room_id": subspace, subspace,
"world_readable": True, {
}, "room_id": subspace,
"world_readable": True,
},
# Place each room in the sub-space.
[
{
"room_id": subspace,
"state_key": room.room_id,
"content": {"via": [fed_hostname]},
}
for room in rooms
],
),
) )
return rooms, events return rooms
# Add a room to the space which is on another server. # Add a room to the space which is on another server.
self._add_child(self.space, subspace, self.token) self._add_child(self.space, subspace, self.token)