diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index 8774a42b19..c675e76afb 100644 --- a/synapse/handlers/sliding_sync.py +++ b/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() diff --git a/synapse/rest/client/models.py b/synapse/rest/client/models.py index eb725cf1c1..5433ed91ef 100644 --- a/synapse/rest/client/models.py +++ b/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( diff --git a/tests/handlers/test_sliding_sync.py b/tests/handlers/test_sliding_sync.py index 8860bf2a0d..2ca4a48605 100644 --- a/tests/handlers/test_sliding_sync.py +++ b/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})