Merge branch 'develop' into madlittlemods/msc3575-sliding-sync-filter-spaces2

Conflicts:
	synapse/handlers/sliding_sync.py
	tests/handlers/test_sliding_sync.py

changelog.d/17280.feature

@@ -0,0 +1 @@
+Add `spaces` filtering to experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint.

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,

synapse/handlers/sliding_sync.py

@@ -18,7 +18,7 @@
 #
 #
 import logging
-from typing import TYPE_CHECKING, AbstractSet, Dict, List, Optional
+from typing import TYPE_CHECKING, AbstractSet, Dict, List, Optional, Set
 
 from immutabledict import immutabledict
 
@@ -57,9 +57,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(
@@ -483,8 +486,10 @@ 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"]`
 
@@ -520,8 +525,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()

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.handlers.sliding_sync import SlidingSyncConfig
 from synapse.rest import admin
@@ -1191,6 +1199,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.is_dm` for DM rooms
@@ -1244,3 +1278,170 @@ class FilterRoomsTestCase(HomeserverTestCase):
         )
 
         self.assertEqual(falsy_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,
+        )
+
+        after_rooms_token = self.event_sources.get_current_token()
+
+        # 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,
+                },
+                SlidingSyncConfig.SlidingSyncList.Filters(
+                    spaces=[
+                        space_a,
+                        space_b,
+                    ],
+                ),
+                after_rooms_token,
+            )
+        )
+
+        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)
+
+        after_rooms_token = self.event_sources.get_current_token()
+
+        # 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,
+                },
+                SlidingSyncConfig.SlidingSyncList.Filters(
+                    spaces=[
+                        space_a,
+                        space_b,
+                    ],
+                ),
+                after_rooms_token,
+            )
+        )
+
+        self.assertEqual(filtered_room_ids, {room_id1})