diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index 1f9dcc9957..7ea55bb230 100644 --- a/synapse/handlers/sliding_sync.py +++ b/synapse/handlers/sliding_sync.py @@ -21,6 +21,15 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +# Everything except `Membership.LEAVE` +MEMBERSHIP_TO_DISPLAY_IN_SYNC = ( + Membership.INVITE, + Membership.JOIN, + Membership.KNOCK, + Membership.BAN, +) + + class SlidingSyncConfig(SlidingSyncBody): user: UserID device_id: str @@ -213,36 +222,42 @@ class SlidingSyncHandler: """ user_id = user.to_string() - room_for_user_list = await self.store.get_rooms_for_local_user_where_membership_is( - user_id=user_id, - # List everything except `Membership.LEAVE` - membership_list=( - Membership.INVITE, - Membership.JOIN, - Membership.KNOCK, - Membership.BAN, - ), - excluded_rooms=self.rooms_to_exclude_globally, + # First grab the current rooms for the user + room_for_user_list = ( + await self.store.get_rooms_for_local_user_where_membership_is( + user_id=user_id, + membership_list=Membership.LIST, + excluded_rooms=self.rooms_to_exclude_globally, + ) ) + + # If the user has never joined any rooms before, we can just return an empty list + if not room_for_user_list: + return {} + + # Our working list of rooms that can show up in the sync response + sync_room_id_set = { + room_for_user.room_id + for room_for_user in room_for_user_list + if room_for_user.membership in MEMBERSHIP_TO_DISPLAY_IN_SYNC + } + + # Find the stream_ordering of the latest room membership event for the user + # which will mark the spot we queried up to. max_stream_ordering_from_room_list = max( room_for_user.stream_ordering for room_for_user in room_for_user_list ) - # Our working list of rooms that can show up in the sync response - sync_room_id_set = { - room_for_user.room_id for room_for_user in room_for_user_list - } - # If our `to_token` is already the same or ahead of the latest room membership - # for the user, we can just straight up return the room list (nothing has + # for the user, we can just straight-up return the room list (nothing has # changed) if max_stream_ordering_from_room_list <= to_token.room_key.stream: return sync_room_id_set - # We assume the `from_token` is before the `to_token` + # We assume the `from_token` is before or at-least equal to the `to_token` assert ( - from_token is None or from_token.room_key.stream < to_token.room_key.stream - ), f"{from_token.room_key.stream if from_token else None} < {to_token.room_key.stream}" + from_token is None or from_token.room_key.stream <= to_token.room_key.stream + ), f"{from_token.room_key.stream if from_token else None} <= {to_token.room_key.stream}" # We assume the `from_token`/`to_token` is before the `max_stream_ordering_from_room_list` assert ( @@ -253,7 +268,7 @@ class SlidingSyncHandler: to_token.room_key.stream < max_stream_ordering_from_room_list ), f"{to_token.room_key.stream} < {max_stream_ordering_from_room_list}" - # Since we fetched the users room list at some point in time after the to/from + # Since we fetched the users room list at some point in time after the from/to # tokens, we need to revert some membership changes to match the point in time # of the `to_token`. # @@ -262,7 +277,8 @@ class SlidingSyncHandler: # - 2b) Add back rooms that the user left after the `to_token` membership_change_events = await self.store.get_membership_changes_for_user( user_id, - # Start from the `from_token` if given, otherwise from the `to_token` + # Start from the `from_token` if given, otherwise from the `to_token` so we + # can still do the 2) fixups. from_key=from_token.room_key if from_token else to_token.room_key, to_key=RoomStreamToken(stream=max_stream_ordering_from_room_list), excluded_rooms=self.rooms_to_exclude_globally, @@ -270,7 +286,7 @@ class SlidingSyncHandler: # Assemble a list of the last membership events in some given ranges. Someone # could have left and joined multiple times during the given range but we only - # care about end-result. + # care about end-result so we grab the last one. last_membership_change_by_room_id_in_from_to_range: Dict[str, EventBase] = {} last_membership_change_by_room_id_after_to_token: Dict[str, EventBase] = {} for event in membership_change_events: diff --git a/tests/handlers/test_sliding_sync.py b/tests/handlers/test_sliding_sync.py index 4659dd84e6..21d3f1db06 100644 --- a/tests/handlers/test_sliding_sync.py +++ b/tests/handlers/test_sliding_sync.py @@ -32,7 +32,30 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): self.store = self.hs.get_datastores().main self.event_sources = hs.get_event_sources() + def test_no_rooms(self) -> None: + """ + Test when the user has never joined any rooms before + """ + user1_id = self.register_user("user1", "pass") + # user1_tok = self.login(user1_id, "pass") + + now_token = self.event_sources.get_current_token() + + room_id_results = self.get_success( + self.sliding_sync_handler.get_sync_room_ids_for_user( + UserID.from_string(user1_id), + from_token=now_token, + to_token=now_token, + ) + ) + + self.assertEqual(room_id_results, {}) + def test_get_newly_joined_room(self) -> None: + """ + Test that rooms that the user has newly_joined show up. newly_joined is when you + join after the `from_token` and <= `to_token`. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -53,6 +76,9 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): self.assertEqual(room_id_results, {room_id}) def test_get_already_joined_room(self) -> None: + """ + Test that rooms that the user is already joined show up. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -71,6 +97,10 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): self.assertEqual(room_id_results, {room_id}) def test_get_invited_banned_knocked_room(self) -> None: + """ + Test that rooms that the user is invited to, banned from, and knocked on show + up. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") user2_id = self.register_user("user2", "pass") @@ -129,6 +159,11 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): ) def test_only_newly_left_rooms_show_up(self) -> None: + """ + Test that newly_left rooms still show up in the sync response but rooms that + were left before the `from_token` don't show up. See condition "1)" comments in + the `get_sync_room_ids_for_user` method. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -136,20 +171,105 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) self.helper.leave(room_id1, user1_id, tok=user1_tok) - before_room_token = self.event_sources.get_current_token() + after_room1_token = self.event_sources.get_current_token() # Leave during the from_token/to_token range (newly_left) room_id2 = self.helper.create_room_as(user1_id, tok=user1_tok) self.helper.leave(room_id1, user1_id, tok=user1_tok) - after_room_token = self.event_sources.get_current_token() + after_room2_token = self.event_sources.get_current_token() room_id_results = self.get_success( self.sliding_sync_handler.get_sync_room_ids_for_user( UserID.from_string(user1_id), - from_token=before_room_token, - to_token=after_room_token, + from_token=after_room1_token, + to_token=after_room2_token, ) ) + # Only the newly_left room should show up self.assertEqual(room_id_results, {room_id2}) + + def test_no_joins_after_to_token(self) -> None: + """ + Rooms we join after the `to_token` should not show up. See condition "2b)" + comments in the `get_sync_room_ids_for_user()` method. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + before_room1_token = self.event_sources.get_current_token() + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + after_room1_token = self.event_sources.get_current_token() + + # Room join after after our `to_token` shouldn't show up + room_id2 = self.helper.create_room_as(user1_id, tok=user1_tok) + _ = room_id2 + + room_id_results = self.get_success( + self.sliding_sync_handler.get_sync_room_ids_for_user( + UserID.from_string(user1_id), + from_token=before_room1_token, + to_token=after_room1_token, + ) + ) + + self.assertEqual(room_id_results, {room_id1}) + + def test_join_during_range_and_left_room_after_to_token(self) -> None: + """ + Room still shows up if we left the room but we were joined during the + from_token/to_token. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + before_room1_token = self.event_sources.get_current_token() + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + after_room1_token = self.event_sources.get_current_token() + + # Leave the room after we already have our tokens + self.helper.leave(room_id1, user1_id, tok=user1_tok) + + room_id_results = self.get_success( + self.sliding_sync_handler.get_sync_room_ids_for_user( + UserID.from_string(user1_id), + from_token=before_room1_token, + to_token=after_room1_token, + ) + ) + + # We should still see the room because we were joined during the + # from_token/to_token time period. + self.assertEqual(room_id_results, {room_id1}) + + def test_join_before_range_and_left_room_after_to_token(self) -> None: + """ + Room still shows up if we left the room but we were joined before the + `from_token` so it should show up + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + after_room1_token = self.event_sources.get_current_token() + + # Leave the room after we already have our tokens + self.helper.leave(room_id1, user1_id, tok=user1_tok) + + room_id_results = self.get_success( + self.sliding_sync_handler.get_sync_room_ids_for_user( + UserID.from_string(user1_id), + from_token=after_room1_token, + to_token=after_room1_token, + ) + ) + + # We should still see the room because we were joined during the + # from_token/to_token time period. + self.assertEqual(room_id_results, {room_id1})