From b03dc0a9978837ffc0bd258bb90638c4efc5c678 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 10:33:39 +0100 Subject: [PATCH 1/2] Ignore leave events for bg updates --- synapse/storage/databases/main/events_bg_updates.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 743200471b6..a5d04c719fa 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1927,10 +1927,11 @@ def _find_memberships_to_update_txn( LEFT JOIN rooms AS r ON (c.room_id = r.room_id) WHERE (c.room_id, c.user_id) > (?, ?) AND (m.user_id IS NULL OR c.event_id != m.membership_event_id) + AND (c.membership != ? OR c.user_id != e.sender) ORDER BY c.room_id ASC, c.user_id ASC LIMIT ? """, - (last_room_id, last_user_id, batch_size), + (last_room_id, last_user_id, Membership.LEAVE, batch_size), ) elif last_event_stream_ordering is not None: # It's important to sort by `event_stream_ordering` *ascending* (oldest to @@ -1961,10 +1962,11 @@ def _find_memberships_to_update_txn( LEFT JOIN rooms AS r ON (c.room_id = r.room_id) WHERE c.event_stream_ordering > ? AND (m.user_id IS NULL OR c.event_id != m.membership_event_id) + AND (c.membership != ? OR c.user_id != e.sender) ORDER BY c.event_stream_ordering ASC LIMIT ? """, - (last_event_stream_ordering, batch_size), + (last_event_stream_ordering, Membership.LEAVE, batch_size), ) else: raise Exception("last_event_stream_ordering should not be None") From bbcd1e9927e375a9a838fa6efc371c6793401e4d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 13:23:14 +0100 Subject: [PATCH 2/2] Fix tests for leaves --- tests/storage/test_sliding_sync_tables.py | 73 +++-------------------- 1 file changed, 8 insertions(+), 65 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 61dccc8077d..6cd9b64442f 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -3507,10 +3507,6 @@ def test_membership_snapshots_background_update_local_invite(self) -> None: (room_id_no_info, user1_id), (room_id_with_info, user1_id), (space_room_id, user1_id), - # The leave memberships for user2 - (room_id_no_info, user2_id), - (room_id_with_info, user2_id), - (space_room_id, user2_id), }, exact=True, ) @@ -3892,16 +3888,12 @@ def test_membership_snapshots_background_update_remote_invite_rejections_and_ret # Reject the remote invites. # Also try retracting a remote invite. - room_id_unknown_state_leave_event_response = self.helper.leave( - room_id_unknown_state, user1_id, tok=user1_tok - ) + self.helper.leave(room_id_unknown_state, user1_id, tok=user1_tok) room_id_no_info_leave_event = self._retract_remote_invite_for_user( user_id=user1_id, remote_room_id=room_id_no_info, ) - room_id_with_info_leave_event_response = self.helper.leave( - room_id_with_info, user1_id, tok=user1_tok - ) + self.helper.leave(room_id_with_info, user1_id, tok=user1_tok) space_room_id_leave_event = self._retract_remote_invite_for_user( user_id=user1_id, remote_room_id=space_room_id, @@ -3956,37 +3948,11 @@ def test_membership_snapshots_background_update_remote_invite_rejections_and_ret set(sliding_sync_membership_snapshots_results.keys()), { # The invite memberships for user1 - (room_id_unknown_state, user1_id), (room_id_no_info, user1_id), - (room_id_with_info, user1_id), (space_room_id, user1_id), }, exact=True, ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get( - (room_id_unknown_state, user1_id) - ), - _SlidingSyncMembershipSnapshotResult( - room_id=room_id_unknown_state, - user_id=user1_id, - sender=user1_id, - membership_event_id=room_id_unknown_state_leave_event_response[ - "event_id" - ], - membership=Membership.LEAVE, - event_stream_ordering=self.get_success( - self.store.get_position_for_event( - room_id_unknown_state_leave_event_response["event_id"] - ) - ).stream, - has_known_state=False, - room_type=None, - room_name=None, - is_encrypted=False, - tombstone_successor_room_id=None, - ), - ) self.assertEqual( sliding_sync_membership_snapshots_results.get((room_id_no_info, user1_id)), _SlidingSyncMembershipSnapshotResult( @@ -4003,28 +3969,6 @@ def test_membership_snapshots_background_update_remote_invite_rejections_and_ret tombstone_successor_room_id=None, ), ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get( - (room_id_with_info, user1_id) - ), - _SlidingSyncMembershipSnapshotResult( - room_id=room_id_with_info, - user_id=user1_id, - sender=user1_id, - membership_event_id=room_id_with_info_leave_event_response["event_id"], - membership=Membership.LEAVE, - event_stream_ordering=self.get_success( - self.store.get_position_for_event( - room_id_with_info_leave_event_response["event_id"] - ) - ).stream, - has_known_state=True, - room_type=None, - room_name="my super duper room", - is_encrypted=True, - tombstone_successor_room_id=None, - ), - ) self.assertEqual( sliding_sync_membership_snapshots_results.get((space_room_id, user1_id)), _SlidingSyncMembershipSnapshotResult( @@ -4220,10 +4164,6 @@ def test_membership_snapshots_background_update_historical_state( (room_id_no_info, user1_id), (room_id_with_info, user1_id), (space_room_id, user1_id), - # The leave memberships for user2 - (room_id_no_info, user2_id), - (room_id_with_info, user2_id), - (space_room_id, user2_id), }, exact=True, ) @@ -4312,7 +4252,7 @@ def test_membership_snapshots_background_update_forgotten_missing(self) -> None: # User1 joins the room self.helper.join(room_id, user1_id, tok=user1_tok) # User1 leaves the room (we have to leave in order to forget the room) - self.helper.leave(room_id, user1_id, tok=user1_tok) + self.helper.leave(room_id, user1_id, tok=user2_tok) state_map = self.get_success( self.storage_controllers.state.get_current_state(room_id) @@ -4381,7 +4321,7 @@ def test_membership_snapshots_background_update_forgotten_missing(self) -> None: _SlidingSyncMembershipSnapshotResult( room_id=room_id, user_id=user1_id, - sender=user1_id, + sender=user2_id, membership_event_id=state_map[(EventTypes.Member, user1_id)].event_id, membership=Membership.LEAVE, event_stream_ordering=state_map[ @@ -4820,7 +4760,7 @@ def test_membership_snapshots_background_update_catch_up_membership_change( ) # User2 leaves the room - self.helper.leave(room_id, user2_id, tok=user2_tok) + self.helper.leave(room_id, user2_id, tok=user1_tok) # Make sure all of the background updates have finished before we start the # catch-up. Even though it should work fine if the other background update is @@ -4841,6 +4781,9 @@ def test_membership_snapshots_background_update_catch_up_membership_change( keyvalues={"room_id": room_id, "user_id": user2_id}, updatevalues={ # Reset everything back to the value before user2 left the room + "sender": sliding_sync_membership_snapshots_results_before_membership_changes[ + (room_id, user2_id) + ].sender, "membership": sliding_sync_membership_snapshots_results_before_membership_changes[ (room_id, user2_id) ].membership,