Add tests for DM filter

synapse/handlers/sliding_sync.py

@@ -509,7 +509,10 @@ class SlidingSyncHandler:
         filtered_room_id_set = set(room_id_set)
 
         # Filter for Direct-Message (DM) rooms
-        if filters.is_dm:
+        if filters.is_dm is not None:
+            # We're using global account data (`m.direct`) instead of checking for
+            # `is_direct` on the membership event because that only appears for the
+            # initee membership event. Account data is set by the client though.
             dm_map = await self.store.get_global_account_data_by_type_for_user(
                 user_id, AccountDataTypes.DIRECT
             )
@@ -518,13 +521,18 @@ class SlidingSyncHandler:
             dm_room_id_set = set()
             if dm_map:
                 for room_ids in dm_map.values():
-                    from typing_extensions import reveal_type
+                    # Account data should be a list of room IDs. Ignore anything else
+                    if isinstance(room_ids, list):
+                        for room_id in room_ids:
+                            if isinstance(room_id, str):
+                                dm_room_id_set.add(room_id)
 
-                    logger.warn("type(room_ids): %s", type(room_ids))
-                    reveal_type(room_ids)
-                    dm_room_id_set.update(room_ids)
-
-            filtered_room_id_set = filtered_room_id_set.intersection(dm_room_id_set)
+            if filters.is_dm:
+                # Only DM rooms please
+                filtered_room_id_set = filtered_room_id_set.intersection(dm_room_id_set)
+            else:
+                # Only non-DM rooms please
+                filtered_room_id_set = filtered_room_id_set.difference(dm_room_id_set)
 
         if filters.spaces:
             raise NotImplementedError()

synapse/rest/client/models.py

@@ -167,7 +167,7 @@ class SlidingSyncBody(RequestBodyModel):
             timeline_limit: int
         else:
             timeline_limit: conint(le=1000, strict=True)  # type: ignore[valid-type]
-        include_old_rooms: Optional[IncludeOldRooms]
+        include_old_rooms: Optional[IncludeOldRooms] = None
 
     class SlidingSyncList(CommonRoomParameters):
         """
@@ -238,42 +238,42 @@ class SlidingSyncBody(RequestBodyModel):
         """
 
         class Filters(RequestBodyModel):
-            is_dm: Optional[StrictBool]
-            spaces: Optional[List[StrictStr]]
-            is_encrypted: Optional[StrictBool]
-            is_invite: Optional[StrictBool]
-            room_types: Optional[List[Union[StrictStr, None]]]
-            not_room_types: Optional[List[StrictStr]]
-            room_name_like: Optional[StrictStr]
-            tags: Optional[List[StrictStr]]
-            not_tags: Optional[List[StrictStr]]
+            is_dm: Optional[StrictBool] = None
+            spaces: Optional[List[StrictStr]] = None
+            is_encrypted: Optional[StrictBool] = None
+            is_invite: Optional[StrictBool] = None
+            room_types: Optional[List[Union[StrictStr, None]]] = None
+            not_room_types: Optional[List[StrictStr]] = None
+            room_name_like: Optional[StrictStr] = None
+            tags: Optional[List[StrictStr]] = None
+            not_tags: Optional[List[StrictStr]] = None
 
         # mypy workaround via https://github.com/pydantic/pydantic/issues/156#issuecomment-1130883884
         if TYPE_CHECKING:
-            ranges: Optional[List[Tuple[int, int]]]
+            ranges: Optional[List[Tuple[int, int]]] = None
         else:
-            ranges: Optional[List[Tuple[conint(ge=0, strict=True), conint(ge=0, strict=True)]]]  # type: ignore[valid-type]
-        sort: Optional[List[StrictStr]]
+            ranges: Optional[List[Tuple[conint(ge=0, strict=True), conint(ge=0, strict=True)]]] = None  # type: ignore[valid-type]
+        sort: Optional[List[StrictStr]] = None
         slow_get_all_rooms: Optional[StrictBool] = False
         include_heroes: Optional[StrictBool] = False
-        filters: Optional[Filters]
-        bump_event_types: Optional[List[StrictStr]]
+        filters: Optional[Filters] = None
+        bump_event_types: Optional[List[StrictStr]] = None
 
     class RoomSubscription(CommonRoomParameters):
         pass
 
     class Extension(RequestBodyModel):
         enabled: Optional[StrictBool] = False
-        lists: Optional[List[StrictStr]]
-        rooms: Optional[List[StrictStr]]
+        lists: Optional[List[StrictStr]] = None
+        rooms: Optional[List[StrictStr]] = None
 
     # mypy workaround via https://github.com/pydantic/pydantic/issues/156#issuecomment-1130883884
     if TYPE_CHECKING:
-        lists: Optional[Dict[str, SlidingSyncList]]
+        lists: Optional[Dict[str, SlidingSyncList]] = None
     else:
-        lists: Optional[Dict[constr(max_length=64, strict=True), SlidingSyncList]]  # type: ignore[valid-type]
-    room_subscriptions: Optional[Dict[StrictStr, RoomSubscription]]
-    extensions: Optional[Dict[StrictStr, Extension]]
+        lists: Optional[Dict[constr(max_length=64, strict=True), SlidingSyncList]] = None  # type: ignore[valid-type]
+    room_subscriptions: Optional[Dict[StrictStr, RoomSubscription]] = None
+    extensions: Optional[Dict[StrictStr, Extension]] = None
 
     @validator("lists")
     def lists_length_check(

tests/handlers/test_sliding_sync.py

@@ -1,19 +1,17 @@
+import logging
+
 from twisted.test.proto_helpers import MemoryReactor
 
-from synapse.api.constants import EventTypes, JoinRules
+from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules
 from synapse.api.room_versions import RoomVersions
 from synapse.rest import admin
 from synapse.rest.client import knock, login, room
 from synapse.server import HomeServer
 from synapse.types import JsonDict, UserID
 from synapse.util import Clock
-from synapse.handlers.sliding_sync import SlidingSyncConfig
 
 from tests.unittest import HomeserverTestCase
 
-
-import logging
-
 logger = logging.getLogger(__name__)
 
 
@@ -561,41 +559,132 @@ class FilterRoomsTestCase(HomeserverTestCase):
         self.sliding_sync_handler = self.hs.get_sliding_sync_handler()
         self.store = self.hs.get_datastores().main
 
-    def test_TODO(self) -> None:
+    def create_dm_room(
+        self,
+        inviter_user_id: str,
+        inviter_tok: str,
+        invitee_user_id: str,
+        invitee_tok: str,
+    ) -> str:
         """
-        Test TODO
+        Create DM room as the "inviter" and invite the "invitee" user to the room. The
+        "invitee" user also will join the room. The `m.direct` account data will be set
+        for both users.
+        """
+
+        # Create a room and send an invite the other user
+        room_id = self.helper.create_room_as(
+            inviter_user_id,
+            is_public=False,
+            tok=inviter_tok,
+        )
+        self.helper.invite(
+            room_id,
+            src=inviter_user_id,
+            targ=invitee_user_id,
+            tok=inviter_tok,
+            extra_data={"is_direct": True},
+        )
+        # Person that was invited joins the room
+        self.helper.join(room_id, invitee_user_id, tok=invitee_tok)
+
+        # Mimic the client setting the room as a direct message in the global account
+        # data
+        self.get_success(
+            self.store.add_account_data_for_user(
+                invitee_user_id,
+                AccountDataTypes.DIRECT,
+                {inviter_user_id: [room_id]},
+            )
+        )
+        self.get_success(
+            self.store.add_account_data_for_user(
+                inviter_user_id,
+                AccountDataTypes.DIRECT,
+                {invitee_user_id: [room_id]},
+            )
+        )
+
+        return room_id
+
+    def test_filter_dm_rooms(self) -> None:
+        """
+        Test filter for DM rooms
         """
         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")
 
-        # Create a room and send an invite to the other user
+        # Create a normal room
         room_id = self.helper.create_room_as(
-            user2_id,
+            user1_id,
             is_public=False,
-            tok=user2_tok,
+            tok=user1_tok,
         )
-        self.helper.invite(
-            room_id,
-            src=user2_id,
-            targ=user1_id,
-            tok=user2_tok,
-            extra_data={"is_direct": True},
+
+        # Create a DM room
+        dm_room_id = self.create_dm_room(
+            inviter_user_id=user1_id,
+            inviter_tok=user1_tok,
+            invitee_user_id=user2_id,
+            invitee_tok=user2_tok,
         )
-        # Accept the invite
-        self.helper.join(room_id, user1_id, 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(
             is_dm=True,
         )
 
+        # Try filtering the rooms
         filtered_room_ids = self.get_success(
             self.sliding_sync_handler.filter_rooms(
-                UserID.from_string(user1_id), {room_id}, filters
+                UserID.from_string(user1_id), {room_id, dm_room_id}, filters
             )
         )
 
-        logger.warn("filtered_room_ids %s", filtered_room_ids)
+        self.assertEqual(filtered_room_ids, {dm_room_id})
+
+    def test_filter_non_dm_rooms(self) -> None:
+        """
+        Test filter for non-DM rooms
+        """
+        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")
+
+        # Create a normal room
+        room_id = self.helper.create_room_as(
+            user1_id,
+            is_public=False,
+            tok=user1_tok,
+        )
+
+        # Create a DM room
+        dm_room_id = self.create_dm_room(
+            inviter_user_id=user1_id,
+            inviter_tok=user1_tok,
+            invitee_user_id=user2_id,
+            invitee_tok=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(
+            is_dm=False,
+        )
+
+        # Try filtering the rooms
+        filtered_room_ids = self.get_success(
+            self.sliding_sync_handler.filter_rooms(
+                UserID.from_string(user1_id), {room_id, dm_room_id}, filters
+            )
+        )
 
         self.assertEqual(filtered_room_ids, {room_id})