From 4b8aa8c1e39eb92f5dc39e6bcfdcecdbf952e584 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 6 Jun 2024 17:16:16 -0500 Subject: [PATCH] Add `spaces` filtering to Sliding Sync `/sync` Based on [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575): Sliding Sync --- changelog.d/17248.feature | 1 + synapse/handlers/room_summary.py | 18 ++- synapse/handlers/sliding_sync.py | 45 +++++- tests/handlers/test_sliding_sync.py | 209 +++++++++++++++++++++++++++- 4 files changed, 263 insertions(+), 10 deletions(-) create mode 100644 changelog.d/17248.feature diff --git a/changelog.d/17248.feature b/changelog.d/17248.feature new file mode 100644 index 0000000000..acd3a86371 --- /dev/null +++ b/changelog.d/17248.feature @@ -0,0 +1 @@ +Add `spaces` filtering to experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint. diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 720459f1e7..1f747953c9 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -452,11 +452,15 @@ class RoomSummaryHandler: return _RoomEntry(room_id, room_entry) # Otherwise, look for child rooms/spaces. - child_events = await self._get_child_events(room_id) + space_child_events = await self._get_space_child_events(room_id) + # Sort the results for stability. + space_child_events = sorted( + space_child_events, key=_child_events_comparison_key + ) if suggested_only: # we only care about suggested children - child_events = filter(_is_suggested_child_event, child_events) + space_child_events = filter(_is_suggested_child_event, space_child_events) stripped_events: List[JsonDict] = [ { @@ -466,7 +470,7 @@ class RoomSummaryHandler: "sender": e.sender, "origin_server_ts": e.origin_server_ts, } - for e in child_events + for e in space_child_events ] return _RoomEntry(room_id, room_entry, stripped_events) @@ -763,9 +767,9 @@ class RoomSummaryHandler: return room_entry - async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: + async def _get_space_child_events(self, room_id: str) -> Iterable[EventBase]: """ - Get the child events for a given room. + Get the space child events for a given room. The returned results are sorted for stability. @@ -791,7 +795,9 @@ class RoomSummaryHandler: # filter out any events without a "via" (which implies it has been redacted), # and order to ensure we return stable results. - return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key) + filtered_events = filter(_has_valid_via, events) + + return filtered_events async def get_room_summary( self, diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index 08c6aadff6..a4a1dfc9b2 100644 --- a/synapse/handlers/sliding_sync.py +++ b/synapse/handlers/sliding_sync.py @@ -19,7 +19,7 @@ # import logging from enum import Enum -from typing import TYPE_CHECKING, AbstractSet, Dict, Final, List, Optional, Tuple +from typing import TYPE_CHECKING, AbstractSet, Dict, Final, List, Optional, Set, Tuple import attr from immutabledict import immutabledict @@ -226,9 +226,12 @@ class SlidingSyncHandler: def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.store = hs.get_datastores().main + self.storage_controllers = hs.get_storage_controllers() self.auth_blocking = hs.get_auth_blocking() self.notifier = hs.get_notifier() self.event_sources = hs.get_event_sources() + self.room_summary_handler = hs.get_room_summary_handler() + self.rooms_to_exclude_globally = hs.config.server.rooms_to_exclude_from_sync async def wait_for_sync_for_user( @@ -624,7 +627,9 @@ class SlidingSyncHandler: """ user_id = user.to_string() - # TODO: Apply filters + # TODO: Re-order filters so that the easiest, most likely to eliminate rooms, + # are first. This way when people use multiple filters, we can eliminate rooms + # and do less work for the subsequent filters. # # TODO: Exclude partially stated rooms unless the `required_state` has # `["m.room.member", "$LAZY"]` @@ -658,8 +663,42 @@ class SlidingSyncHandler: # Only non-DM rooms please filtered_room_id_set = filtered_room_id_set.difference(dm_room_id_set) + # Filter the room based on the space they belong to according to `m.space.child` + # state events. If multiple spaces are present, a room can be part of any one of + # the listed spaces (OR'd). if filters.spaces: - raise NotImplementedError() + # Only use spaces that we're joined to to avoid leaking private space + # information that the user is not part of. We could probably allow + # public spaces here but the spec says "joined" only. + joined_space_room_ids = set() + for space_room_id in set(filters.spaces): + # TODO: Is there a good method to look up all space rooms at once? (N+1 query problem) + is_user_in_room = await self.store.check_local_user_in_room( + user_id=user.to_string(), room_id=space_room_id + ) + + if is_user_in_room: + joined_space_room_ids.add(space_room_id) + + # Flatten the child rooms in the spaces + space_child_room_ids: Set[str] = set() + for space_room_id in joined_space_room_ids: + space_child_events = ( + await self.room_summary_handler._get_space_child_events( + space_room_id + ) + ) + space_child_room_ids.update( + event.state_key for event in space_child_events + ) + # TODO: The spec says that if the child room has a `m.room.tombstone` + # event, we should recursively navigate until we find the latest room + # and include those IDs (although this point is under scrutiny). + + # Only rooms in the spaces please + filtered_room_id_set = filtered_room_id_set.intersection( + space_child_room_ids + ) if filters.is_encrypted: raise NotImplementedError() diff --git a/tests/handlers/test_sliding_sync.py b/tests/handlers/test_sliding_sync.py index 220683b9d6..ce03f87efa 100644 --- a/tests/handlers/test_sliding_sync.py +++ b/tests/handlers/test_sliding_sync.py @@ -18,11 +18,19 @@ # # import logging +from typing import List, Optional from unittest.mock import patch from twisted.test.proto_helpers import MemoryReactor -from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules, Membership +from synapse.api.constants import ( + AccountDataTypes, + EventContentFields, + EventTypes, + JoinRules, + Membership, + RoomTypes, +) from synapse.api.room_versions import RoomVersions from synapse.rest import admin from synapse.rest.client import knock, login, room @@ -1189,6 +1197,32 @@ class FilterRoomsTestCase(HomeserverTestCase): return room_id + def _add_space_child( + self, + space_id: str, + room_id: str, + token: str, + order: Optional[str] = None, + via: Optional[List[str]] = None, + ) -> None: + """ + Helper to add a child room to a space. + """ + + if via is None: + via = [self.hs.hostname] + + content: JsonDict = {"via": via} + if order is not None: + content["order"] = order + self.helper.send_state( + space_id, + event_type=EventTypes.SpaceChild, + body=content, + tok=token, + state_key=room_id, + ) + def test_filter_dm_rooms(self) -> None: """ Test filter for DM rooms @@ -1270,3 +1304,176 @@ class FilterRoomsTestCase(HomeserverTestCase): ) self.assertEqual(filtered_room_ids, {room_id}) + + def test_filter_space_rooms(self) -> None: + """ + Test `filter.spaces` for rooms in spaces + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + space_a = self.helper.create_room_as( + user1_id, + tok=user1_tok, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + space_b = self.helper.create_room_as( + user1_id, + tok=user1_tok, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + space_c = self.helper.create_room_as( + user1_id, + tok=user1_tok, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + + room_id1 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to space_a + self._add_space_child(space_a, room_id1, user1_tok) + + room_id2 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to space_a and space_b + self._add_space_child(space_a, room_id2, user1_tok) + self._add_space_child(space_c, room_id2, user1_tok) + + room_id3 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to all spaces + self._add_space_child(space_a, room_id3, user1_tok) + self._add_space_child(space_b, room_id3, user1_tok) + self._add_space_child(space_c, room_id3, user1_tok) + + room_id4 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to space_c + self._add_space_child(space_c, room_id3, user1_tok) + + room_not_in_space1 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + + # TODO: Better way to avoid the circular import? (see + # https://github.com/element-hq/synapse/pull/17187#discussion_r1619492779) + from synapse.handlers.sliding_sync import SlidingSyncConfig + + filters = SlidingSyncConfig.SlidingSyncList.Filters( + spaces=[ + space_a, + space_b, + ], + ) + + # Try filtering the rooms + filtered_room_ids = self.get_success( + self.sliding_sync_handler.filter_rooms( + UserID.from_string(user1_id), + { + # a + room_id1, + # a, c + room_id2, + # a, b, c + room_id3, + # c + room_id4, + # not in any space + room_not_in_space1, + }, + filters, + ) + ) + + self.assertEqual(filtered_room_ids, {room_id1, room_id2, room_id3}) + + def test_filter_only_joined_spaces(self) -> None: + """ + Test `filter.spaces` to make sure the filter only takes into account spaces we + are joined to. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + + # space_a created by user1 + space_a = self.helper.create_room_as( + user1_id, + tok=user1_tok, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + # space_b created by user2 + space_b = self.helper.create_room_as( + user2_id, + tok=user2_tok, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + + room_id1 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to space_a + self._add_space_child(space_a, room_id1, user1_tok) + + room_id2 = self.helper.create_room_as( + user1_id, + is_public=False, + tok=user1_tok, + ) + # Add to space_b + self._add_space_child(space_b, room_id2, user2_tok) + + # TODO: Better way to avoid the circular import? (see + # https://github.com/element-hq/synapse/pull/17187#discussion_r1619492779) + from synapse.handlers.sliding_sync import SlidingSyncConfig + + filters = SlidingSyncConfig.SlidingSyncList.Filters( + spaces=[ + space_a, + space_b, + ], + ) + + # Try filtering the rooms + filtered_room_ids = self.get_success( + self.sliding_sync_handler.filter_rooms( + UserID.from_string(user1_id), + { + # a + room_id1, + # b (but user1 isn't in space_b) + room_id2, + }, + filters, + ) + ) + + self.assertEqual(filtered_room_ids, {room_id1})