From 50605880120237b5722b45a19578395d8c37a0b5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 12 Jun 2024 18:18:20 -0500 Subject: [PATCH] Fix newly_left not being added back if we returned early (when `membership_snapshot_token.is_before_or_eq(to_token.room_key)`) Wasn't caught in the tests because the test was wrong --- synapse/handlers/sliding_sync.py | 26 ++++++++++---------- tests/handlers/test_sliding_sync.py | 38 +++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index c84e7d265d..21cf16e008 100644 --- a/synapse/handlers/sliding_sync.py +++ b/synapse/handlers/sliding_sync.py @@ -317,12 +317,6 @@ class SlidingSyncHandler: instance_map=immutabledict(instance_to_max_stream_ordering_map), ) - # 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 - # changed) - if membership_snapshot_token.is_before_or_eq(to_token.room_key): - return sync_room_id_set - # Since we fetched the users room list at some point in time after the from/to # tokens, we need to revert/rewind some membership changes to match the point in # time of the `to_token`. In particular, we need to make these fixups: @@ -342,14 +336,20 @@ class SlidingSyncHandler: # 1) Fetch membership changes that fall in the range from `to_token` up to # `membership_snapshot_token` - membership_change_events_after_to_token = ( - await self.store.get_membership_changes_for_user( - user_id, - from_key=to_token.room_key, - to_key=membership_snapshot_token, - excluded_rooms=self.rooms_to_exclude_globally, + # + # If our `to_token` is already the same or ahead of the latest room membership + # for the user, we don't need to do any "2)" fix-ups and can just straight-up + # use the room list from the snapshot as a base (nothing has changed) + membership_change_events_after_to_token = [] + if not membership_snapshot_token.is_before_or_eq(to_token.room_key): + membership_change_events_after_to_token = ( + await self.store.get_membership_changes_for_user( + user_id, + from_key=to_token.room_key, + to_key=membership_snapshot_token, + excluded_rooms=self.rooms_to_exclude_globally, + ) ) - ) # 1) 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 diff --git a/tests/handlers/test_sliding_sync.py b/tests/handlers/test_sliding_sync.py index bd1ca8f4ef..5dfd2d671a 100644 --- a/tests/handlers/test_sliding_sync.py +++ b/tests/handlers/test_sliding_sync.py @@ -20,6 +20,8 @@ import logging from unittest.mock import patch +from parameterized import parameterized + from twisted.test.proto_helpers import MemoryReactor from synapse.api.constants import EventTypes, JoinRules, Membership @@ -326,7 +328,7 @@ class GetSyncRoomIdsForUserTestCase(HomeserverTestCase): # 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) + self.helper.leave(room_id2, user1_id, tok=user1_tok) after_room2_token = self.event_sources.get_current_token() @@ -1177,14 +1179,15 @@ class SortRoomsTestCase(HomeserverTestCase): [room_id2, room_id1], ) - # @parameterized.expand( - # [ - # (Membership.INVITE,), - # (Membership.KNOCK,), - # (Membership.INVITE,), - # ] - # ) - def test_activity_after_invite(self) -> None: + @parameterized.expand( + [ + (Membership.LEAVE,), + (Membership.INVITE,), + (Membership.KNOCK,), + (Membership.BAN,), + ] + ) + def test_activity_after_xxx(self, room1_membership: Membership) -> None: """ When someone is invited to a room, they shouldn't take anything into account after the invite. """ @@ -1198,12 +1201,27 @@ class SortRoomsTestCase(HomeserverTestCase): # Create the rooms as user2 so we can have user1 with a clean slate to work from # and join in whatever order we need for the tests. room_id1 = self.helper.create_room_as(user2_id, tok=user2_tok, is_public=True) + if room1_membership == Membership.KNOCK: + self.helper.send_state( + room_id1, + EventTypes.JoinRules, + {"join_rule": JoinRules.KNOCK}, + tok=user2_tok, + ) room_id2 = self.helper.create_room_as(user2_id, tok=user2_tok, is_public=True) room_id3 = self.helper.create_room_as(user2_id, tok=user2_tok, is_public=True) # Here is the activity with user1 that will determine the sort of the rooms self.helper.join(room_id3, user1_id, tok=user1_tok) - self.helper.invite(room_id1, src=user2_id, targ=user1_id, tok=user2_tok) + if room1_membership == Membership.LEAVE: + self.helper.join(room_id1, user1_id, tok=user1_tok) + self.helper.leave(room_id1, user1_id, tok=user1_tok) + elif room1_membership == Membership.INVITE: + self.helper.invite(room_id1, src=user2_id, targ=user1_id, tok=user2_tok) + elif room1_membership == Membership.KNOCK: + self.helper.knock(room_id1, user1_id, tok=user1_tok) + elif room1_membership == Membership.BAN: + self.helper.ban(room_id1, user1_id, user2_id) self.helper.join(room_id2, user1_id, tok=user1_tok) # Activity before the token but the user is only invited to this room so it