From b5ad7da2b92b5047aeabaaca85230565fdce7c27 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 09:42:24 +0100 Subject: [PATCH 01/22] Skip over existing rows --- .../databases/main/events_bg_updates.py | 8 +- tests/storage/test_sliding_sync_tables.py | 130 ------------------ 2 files changed, 5 insertions(+), 133 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 4209100a5ca..275fd21a490 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1888,9 +1888,10 @@ def _find_memberships_to_update_txn( e.instance_name, e.outlier FROM local_current_membership AS c + LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) LEFT JOIN rooms AS r ON (c.room_id = r.room_id) - WHERE (c.room_id, c.user_id) > (?, ?) + WHERE (c.room_id, c.user_id) > (?, ?) AND m.user_id IS NULL ORDER BY c.room_id ASC, c.user_id ASC LIMIT ? """, @@ -1919,9 +1920,10 @@ def _find_memberships_to_update_txn( e.instance_name, e.outlier FROM local_current_membership AS c + LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) - WHERE event_stream_ordering > ? - ORDER BY event_stream_ordering ASC + WHERE c.event_stream_ordering > ? AND m.user_id IS NULL + ORDER BY c.event_stream_ordering ASC LIMIT ? """, (last_event_stream_ordering, batch_size), diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 621f46fff82..22383548fff 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -4221,136 +4221,6 @@ def test_membership_snapshots_background_update_forgotten_missing(self) -> None: ), ) - def test_membership_snapshots_background_update_forgotten_partial(self) -> None: - """ - Test an existing `sliding_sync_membership_snapshots` row is updated with the - latest `forgotten` status after the background update passes over it. - """ - 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") - - room_id = self.helper.create_room_as(user2_id, tok=user2_tok) - - # 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) - - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id) - ) - - # Forget the room - channel = self.make_request( - "POST", - f"/_matrix/client/r0/rooms/{room_id}/forget", - content={}, - access_token=user1_tok, - ) - self.assertEqual(channel.code, 200, channel.result) - - # Clean-up the `sliding_sync_joined_rooms` table as if the forgotten status - # never made it into the table. - self.get_success( - self.store.db_pool.simple_update( - table="sliding_sync_membership_snapshots", - keyvalues={"room_id": room_id}, - updatevalues={"forgotten": 0}, - desc="sliding_sync_membership_snapshots.test_membership_snapshots_background_update_forgotten_partial", - ) - ) - - # We should see the partial row that we made in preparation for the test. - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - { - (room_id, user1_id), - (room_id, user2_id), - }, - exact=True, - ) - user1_snapshot = _SlidingSyncMembershipSnapshotResult( - room_id=room_id, - user_id=user1_id, - sender=user1_id, - membership_event_id=state_map[(EventTypes.Member, user1_id)].event_id, - membership=Membership.LEAVE, - event_stream_ordering=state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.stream_ordering, - has_known_state=True, - room_type=None, - room_name=None, - is_encrypted=False, - tombstone_successor_room_id=None, - # Room is *not* forgotten because of our test preparation - forgotten=False, - ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user1_id)), - user1_snapshot, - ) - user2_snapshot = _SlidingSyncMembershipSnapshotResult( - room_id=room_id, - user_id=user2_id, - sender=user2_id, - membership_event_id=state_map[(EventTypes.Member, user2_id)].event_id, - membership=Membership.JOIN, - event_stream_ordering=state_map[ - (EventTypes.Member, user2_id) - ].internal_metadata.stream_ordering, - has_known_state=True, - room_type=None, - room_name=None, - is_encrypted=False, - tombstone_successor_room_id=None, - ) - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user2_id)), - user2_snapshot, - ) - - # Insert and run the background update. - self.get_success( - self.store.db_pool.simple_insert( - "background_updates", - { - "update_name": _BackgroundUpdates.SLIDING_SYNC_MEMBERSHIP_SNAPSHOTS_BG_UPDATE, - "progress_json": "{}", - }, - ) - ) - self.store.db_pool.updates._all_done = False - self.wait_for_background_updates() - - # Make sure the table is populated - sliding_sync_membership_snapshots_results = ( - self._get_sliding_sync_membership_snapshots() - ) - self.assertIncludes( - set(sliding_sync_membership_snapshots_results.keys()), - { - (room_id, user1_id), - (room_id, user2_id), - }, - exact=True, - ) - # Forgotten status is now updated - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user1_id)), - attr.evolve(user1_snapshot, forgotten=True), - ) - # Holds the info according to the current state when the user joined - self.assertEqual( - sliding_sync_membership_snapshots_results.get((room_id, user2_id)), - user2_snapshot, - ) - class SlidingSyncTablesCatchUpBackgroundUpdatesTestCase(SlidingSyncTablesTestCaseBase): """ From 269dc55f64b20ce33267efda539eb4527800dd65 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 09:58:13 +0100 Subject: [PATCH 02/22] Handle missing previous memberships --- .../databases/main/events_bg_updates.py | 72 ++++++++++++------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 275fd21a490..6598e103f12 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1967,7 +1967,7 @@ def _find_memberships_to_update_txn( def _find_previous_membership_txn( txn: LoggingTransaction, room_id: str, user_id: str, event_id: str - ) -> Tuple[str, str]: + ) -> Optional[Tuple[str, str]]: # Find the previous invite/knock event before the leave event # # Here are some notes on how we landed on this query: @@ -2013,8 +2013,13 @@ def _find_previous_membership_txn( ) row = txn.fetchone() - # We should see a corresponding previous invite/knock event - assert row is not None + if row is None: + # Generally we should have an invite or knock event for leaves + # that are outliers, however this may not always be the case + # (e.g. a local user got kicked but the kick event got pulled in + # as an outlier). + return None + event_id, membership = row return event_id, membership @@ -2140,8 +2145,8 @@ def _find_previous_membership_txn( elif membership in (Membership.INVITE, Membership.KNOCK) or ( membership in (Membership.LEAVE, Membership.BAN) and is_outlier ): - invite_or_knock_event_id = membership_event_id - invite_or_knock_membership = membership + invite_or_knock_event_id = None + invite_or_knock_membership = None # If the event is an `out_of_band_membership` (special case of # `outlier`), we never had historical state so we have to pull from @@ -2150,35 +2155,52 @@ def _find_previous_membership_txn( # membership (i.e. the room shouldn't disappear if your using the # `is_encrypted` filter and you leave). if membership in (Membership.LEAVE, Membership.BAN) and is_outlier: - ( - invite_or_knock_event_id, - invite_or_knock_membership, - ) = await self.db_pool.runInteraction( + previous_membership = await self.db_pool.runInteraction( "sliding_sync_membership_snapshots_bg_update._find_previous_membership", _find_previous_membership_txn, room_id, user_id, membership_event_id, ) + if previous_membership is not None: + ( + invite_or_knock_event_id, + invite_or_knock_membership, + ) = previous_membership + else: + invite_or_knock_event_id = membership_event_id + invite_or_knock_membership = membership - # Pull from the stripped state on the invite/knock event - invite_or_knock_event = await self.get_event(invite_or_knock_event_id) - - raw_stripped_state_events = None - if invite_or_knock_membership == Membership.INVITE: - invite_room_state = invite_or_knock_event.unsigned.get( - "invite_room_state" - ) - raw_stripped_state_events = invite_room_state - elif invite_or_knock_membership == Membership.KNOCK: - knock_room_state = invite_or_knock_event.unsigned.get( - "knock_room_state" + if ( + invite_or_knock_event_id is not None + and invite_or_knock_membership is not None + ): + # Pull from the stripped state on the invite/knock event + invite_or_knock_event = await self.get_event( + invite_or_knock_event_id ) - raw_stripped_state_events = knock_room_state - sliding_sync_membership_snapshots_insert_map = PersistEventsStore._get_sliding_sync_insert_values_from_stripped_state( - raw_stripped_state_events - ) + raw_stripped_state_events = None + if invite_or_knock_membership == Membership.INVITE: + invite_room_state = invite_or_knock_event.unsigned.get( + "invite_room_state" + ) + raw_stripped_state_events = invite_room_state + elif invite_or_knock_membership == Membership.KNOCK: + knock_room_state = invite_or_knock_event.unsigned.get( + "knock_room_state" + ) + raw_stripped_state_events = knock_room_state + + sliding_sync_membership_snapshots_insert_map = PersistEventsStore._get_sliding_sync_insert_values_from_stripped_state( + raw_stripped_state_events + ) + else: + # We couldn't find any state for the membership, so we just have to + # leave it as empty. + sliding_sync_membership_snapshots_insert_map = { + "has_known_state": False + } # We should have some insert values for each room, even if no # stripped state is on the event because we still want to record From 01860e1e2f06e5e157c8d07cc64c8d86aaf1f545 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 09:58:49 +0100 Subject: [PATCH 03/22] Don't overwrite forgotten flag --- synapse/storage/databases/main/events_bg_updates.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 6598e103f12..48b1ed30a00 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -2324,12 +2324,13 @@ def _fill_table_txn(txn: LoggingTransaction) -> None: UPDATE sliding_sync_membership_snapshots SET forgotten = (SELECT forgotten FROM room_memberships WHERE event_id = ?) - WHERE room_id = ? and user_id = ? + WHERE room_id = ? and user_id = ? AND membership_event_id = ? """, ( membership_event_id, room_id, user_id, + membership_event_id, ), ) From f71dd25a4a76cf25c58035dd737f5f151f4fd58a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 10:33:39 +0100 Subject: [PATCH 04/22] 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 48b1ed30a00..b665072ba4b 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1892,10 +1892,11 @@ def _find_memberships_to_update_txn( INNER JOIN events AS e USING (event_id) 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 + 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 @@ -1923,10 +1924,11 @@ def _find_memberships_to_update_txn( LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) WHERE c.event_stream_ordering > ? AND m.user_id IS NULL + 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 8140ca353d8c505fcac541409aea5f8559aa6067 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 13:23:14 +0100 Subject: [PATCH 05/22] 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 22383548fff..951978432f2 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -3312,10 +3312,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, ) @@ -3697,16 +3693,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, @@ -3761,37 +3753,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( @@ -3808,28 +3774,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( @@ -4025,10 +3969,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, ) @@ -4117,7 +4057,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) @@ -4186,7 +4126,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[ @@ -4625,7 +4565,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 @@ -4646,6 +4586,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, From 4f3333b996c5cf9d8e9c0235263d43bc2de76a0b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 14:19:59 +0100 Subject: [PATCH 06/22] Ignore invites to rooms with unknown room version --- .../databases/main/events_bg_updates.py | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index b665072ba4b..1919155cf6a 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1865,7 +1865,18 @@ async def _sliding_sync_membership_snapshots_bg_update( def _find_memberships_to_update_txn( txn: LoggingTransaction, ) -> List[ - Tuple[str, Optional[str], str, str, str, str, int, Optional[str], bool] + Tuple[ + str, + Optional[str], + Optional[str], + str, + str, + str, + str, + int, + Optional[str], + bool, + ] ]: # Fetch the set of event IDs that we want to update @@ -1880,6 +1891,7 @@ def _find_memberships_to_update_txn( SELECT c.room_id, r.room_id, + r.room_version, c.user_id, e.sender, c.event_id, @@ -1912,7 +1924,8 @@ def _find_memberships_to_update_txn( """ SELECT c.room_id, - c.room_id, + r.room_id, + r.room_version, c.user_id, e.sender, c.event_id, @@ -1923,6 +1936,7 @@ def _find_memberships_to_update_txn( FROM local_current_membership AS c LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) + LEFT JOIN rooms AS r ON (c.room_id = r.room_id) WHERE c.event_stream_ordering > ? AND m.user_id IS NULL AND (c.membership != ? OR c.user_id != e.sender) ORDER BY c.event_stream_ordering ASC @@ -1936,7 +1950,16 @@ def _find_memberships_to_update_txn( memberships_to_update_rows = cast( List[ Tuple[ - str, Optional[str], str, str, str, str, int, Optional[str], bool + str, + Optional[str], + Optional[str], + str, + str, + str, + str, + int, + Optional[str], + bool, ] ], txn.fetchall(), @@ -2036,6 +2059,7 @@ def _find_previous_membership_txn( for ( room_id, room_id_from_rooms_table, + room_version_id, user_id, sender, membership_event_id, @@ -2054,6 +2078,13 @@ def _find_previous_membership_txn( Membership.BAN, ) + if ( + room_version_id is not None + and room_version_id not in KNOWN_ROOM_VERSIONS + ): + # Ignore rooms with unknown room versions (these were experimental rooms). + continue + # There are some old out-of-band memberships (before # https://github.com/matrix-org/synapse/issues/6983) where we don't have the # corresponding room stored in the `rooms` table`. We have a `FOREIGN KEY` @@ -2344,6 +2375,7 @@ def _fill_table_txn(txn: LoggingTransaction) -> None: ( room_id, _room_id_from_rooms_table, + _room_version_id, user_id, _sender, _membership_event_id, From 4369e94bd1ec7fa577a20a64f89afd6a8ff51bec Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 16:35:54 +0100 Subject: [PATCH 07/22] Ignore nulls in room names --- synapse/storage/databases/main/events.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index d423d80efa7..ba56f2d2564 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1978,7 +1978,9 @@ def _get_sliding_sync_insert_values_from_state_map( elif state_key == (EventTypes.Name, ""): room_name = event.content.get(EventContentFields.ROOM_NAME) # Scrutinize JSON values - if room_name is None or isinstance(room_name, str): + if room_name is None or ( + isinstance(room_name, str) and "\0" not in room_name + ): sliding_sync_insert_map["room_name"] = room_name elif state_key == (EventTypes.Tombstone, ""): successor_room_id = event.content.get( From 330e6145bdc014956a7f8deb251156b9f0a7e6fe Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 2 Sep 2024 16:42:24 +0100 Subject: [PATCH 08/22] Ignore nulls in invite state --- synapse/storage/databases/main/events.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index ba56f2d2564..2dd589c513e 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -2070,6 +2070,12 @@ def _get_sliding_sync_insert_values_from_stripped_state( else None ) + if ( + sliding_sync_insert_map["room_name"] is not None + and "\0" in sliding_sync_insert_map["room_name"] + ): + sliding_sync_insert_map.pop("room_name") + # Find the tombstone_successor_room_id # Note: This isn't one of the stripped state events according to the spec # but seems like there is no reason not to support this kind of thing. From 037cb109c0322cb2f1f3d6baf3248cb0dc7ae556 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 3 Sep 2024 08:10:58 +0100 Subject: [PATCH 09/22] Handle the case where there is a missing `room_membership` row --- synapse/storage/databases/main/events_bg_updates.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 1919155cf6a..9386431cee4 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -2356,11 +2356,15 @@ def _fill_table_txn(txn: LoggingTransaction) -> None: """ UPDATE sliding_sync_membership_snapshots SET - forgotten = (SELECT forgotten FROM room_memberships WHERE event_id = ?) - WHERE room_id = ? and user_id = ? AND membership_event_id = ? + forgotten = m.forgotten + FROM room_memberships AS m + WHERE sliding_sync_membership_snapshots.room_id = ? + AND sliding_sync_membership_snapshots.user_id = ? + AND membership_event_id = ? + AND membership_event_id = m.event_id + AND m.event_id IS NOT NULL """, ( - membership_event_id, room_id, user_id, membership_event_id, From 20542f061856030ed4bd1836e8fa2fffba3f2bed Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 3 Sep 2024 12:43:15 +0100 Subject: [PATCH 10/22] Newsfile --- changelog.d/17652.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17652.misc diff --git a/changelog.d/17652.misc b/changelog.d/17652.misc new file mode 100644 index 00000000000..756918e2b21 --- /dev/null +++ b/changelog.d/17652.misc @@ -0,0 +1 @@ +Pre-populate room data used in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint for quick filtering/sorting. From 0a7f41cfd32f9992bdab2bd8b402c093eeb77997 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 14:18:11 +0100 Subject: [PATCH 11/22] Comment why we strip null bytes --- synapse/storage/databases/main/events.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 2dd589c513e..4bd30abc830 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1977,7 +1977,8 @@ def _get_sliding_sync_insert_values_from_state_map( sliding_sync_insert_map["is_encrypted"] = is_encrypted elif state_key == (EventTypes.Name, ""): room_name = event.content.get(EventContentFields.ROOM_NAME) - # Scrutinize JSON values + # Scrutinize JSON values. We ignore values with nulls as + # postgres doesn't allow null bytes in text columns. if room_name is None or ( isinstance(room_name, str) and "\0" not in room_name ): @@ -2074,6 +2075,8 @@ def _get_sliding_sync_insert_values_from_stripped_state( sliding_sync_insert_map["room_name"] is not None and "\0" in sliding_sync_insert_map["room_name"] ): + # We ignore values with nulls as + # postgres doesn't allow null bytes in text columns. sliding_sync_insert_map.pop("room_name") # Find the tombstone_successor_room_id From 47bfb1b622e42c75133be299e0c03d628dac72fa Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 14:22:12 +0100 Subject: [PATCH 12/22] Only skip rows if we have the same event ID --- .../storage/databases/main/events_bg_updates.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 9386431cee4..78d0050868d 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1886,6 +1886,10 @@ def _find_memberships_to_update_txn( # the corresponding room stored in the `rooms` table`. We use `LEFT JOIN # rooms AS r USING (room_id)` to find the rooms missing from `rooms` and # insert a row for them below. + # + # We skip over rows which we've already handled, i.e. have a matching row + # in `sliding_sync_membership_snapshots` with the same room, user and + # event ID. txn.execute( """ SELECT @@ -1903,7 +1907,8 @@ def _find_memberships_to_update_txn( LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) 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 + 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 ? @@ -1920,6 +1925,10 @@ def _find_memberships_to_update_txn( # `c.room_id` is duplicated to make it match what we're doing in the # `initial_phase`. But we can avoid doing the extra `rooms` table join # because we can assume all of these new events won't have this problem. + # + # We skip over rows which we've already handled, i.e. have a matching row + # in `sliding_sync_membership_snapshots` with the same room, user and + # event ID. txn.execute( """ SELECT @@ -1937,7 +1946,8 @@ def _find_memberships_to_update_txn( LEFT JOIN sliding_sync_membership_snapshots AS m USING (room_id, user_id) INNER JOIN events AS e USING (event_id) LEFT JOIN rooms AS r ON (c.room_id = r.room_id) - WHERE c.event_stream_ordering > ? AND m.user_id IS NULL + 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 ? From 2a48840ce9188f9c0b5de4f874d35ad9cfac9fef Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 14:22:58 +0100 Subject: [PATCH 13/22] Update comment on room versions --- synapse/storage/databases/main/events_bg_updates.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 78d0050868d..af401882fe5 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -2092,7 +2092,8 @@ def _find_previous_membership_txn( room_version_id is not None and room_version_id not in KNOWN_ROOM_VERSIONS ): - # Ignore rooms with unknown room versions (these were experimental rooms). + # Ignore rooms with unknown room versions (these were + # experimental rooms, that we no longer support). continue # There are some old out-of-band memberships (before From 5f04c2f267d30fe415f39a2ded8decb2d9dbea87 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 14:25:10 +0100 Subject: [PATCH 14/22] Don't inherit values --- .../storage/databases/main/events_bg_updates.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index af401882fe5..8afc9c11213 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -2183,9 +2183,12 @@ def _find_previous_membership_txn( # in the events table though. We'll just say that we don't # know the state for these rooms and continue on with our # day. - sliding_sync_membership_snapshots_insert_map["has_known_state"] = ( - False - ) + sliding_sync_membership_snapshots_insert_map = { + "has_known_state": False, + "room_type": None, + "room_name": None, + "is_encrypted": False, + } elif membership in (Membership.INVITE, Membership.KNOCK) or ( membership in (Membership.LEAVE, Membership.BAN) and is_outlier ): @@ -2243,7 +2246,10 @@ def _find_previous_membership_txn( # We couldn't find any state for the membership, so we just have to # leave it as empty. sliding_sync_membership_snapshots_insert_map = { - "has_known_state": False + "has_known_state": False, + "room_type": None, + "room_name": None, + "is_encrypted": False, } # We should have some insert values for each room, even if no From b7782194ccef43b8a00cf31908f7d7cdacad8c13 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 14:29:32 +0100 Subject: [PATCH 15/22] Add note on why we're skipping old left rooms --- .../storage/databases/main/events_bg_updates.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 8afc9c11213..fe03a172cc2 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1879,6 +1879,15 @@ def _find_memberships_to_update_txn( ] ]: # Fetch the set of event IDs that we want to update + # + # We skip over rows which we've already handled, i.e. have a + # matching row in `sliding_sync_membership_snapshots` with the same + # room, user and event ID. + # + # We also ignore rooms that the user has left themselves (i.e. not + # kicked). This is to avoid having to port lots of old rooms that we + # will never send down sliding sync (as we exclude such rooms from + # initial syncs). if initial_phase: # There are some old out-of-band memberships (before @@ -1886,10 +1895,6 @@ def _find_memberships_to_update_txn( # the corresponding room stored in the `rooms` table`. We use `LEFT JOIN # rooms AS r USING (room_id)` to find the rooms missing from `rooms` and # insert a row for them below. - # - # We skip over rows which we've already handled, i.e. have a matching row - # in `sliding_sync_membership_snapshots` with the same room, user and - # event ID. txn.execute( """ SELECT @@ -1925,10 +1930,6 @@ def _find_memberships_to_update_txn( # `c.room_id` is duplicated to make it match what we're doing in the # `initial_phase`. But we can avoid doing the extra `rooms` table join # because we can assume all of these new events won't have this problem. - # - # We skip over rows which we've already handled, i.e. have a matching row - # in `sliding_sync_membership_snapshots` with the same room, user and - # event ID. txn.execute( """ SELECT From 1679ba09572b3cf3e5a7984e5995494b84009083 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 5 Sep 2024 15:26:40 +0100 Subject: [PATCH 16/22] Fix old sqlite versions --- .../databases/main/events_bg_updates.py | 55 +++++++++++++------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index fe03a172cc2..7b272e4761c 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -44,6 +44,7 @@ from synapse.storage.databases.main.events_worker import DatabaseCorruptionError from synapse.storage.databases.main.state_deltas import StateDeltasStore from synapse.storage.databases.main.stream import StreamWorkerStore +from synapse.storage.engines import PostgresEngine from synapse.storage.types import Cursor from synapse.types import JsonDict, RoomStreamToken, StateMap, StrCollection from synapse.types.handlers import SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES @@ -2370,24 +2371,42 @@ def _fill_table_txn(txn: LoggingTransaction) -> None: ) # We need to find the `forgotten` value during the transaction because # we can't risk inserting stale data. - txn.execute( - """ - UPDATE sliding_sync_membership_snapshots - SET - forgotten = m.forgotten - FROM room_memberships AS m - WHERE sliding_sync_membership_snapshots.room_id = ? - AND sliding_sync_membership_snapshots.user_id = ? - AND membership_event_id = ? - AND membership_event_id = m.event_id - AND m.event_id IS NOT NULL - """, - ( - room_id, - user_id, - membership_event_id, - ), - ) + if isinstance(txn.database_engine, PostgresEngine): + txn.execute( + """ + UPDATE sliding_sync_membership_snapshots + SET + forgotten = m.forgotten + FROM room_memberships AS m + WHERE sliding_sync_membership_snapshots.room_id = ? + AND sliding_sync_membership_snapshots.user_id = ? + AND membership_event_id = ? + AND membership_event_id = m.event_id + AND m.event_id IS NOT NULL + """, + ( + room_id, + user_id, + membership_event_id, + ), + ) + else: + # SQLite doesn't support UPDATE FROM before 3.33.0, so we do + # this via sub-selects. + txn.execute( + """ + UPDATE sliding_sync_membership_snapshots + SET + forgotten = (SELECT forgotten FROM room_memberships WHERE event_id = ?) + WHERE room_id = ? and user_id = ? AND membership_event_id = ? + """, + ( + membership_event_id, + room_id, + user_id, + membership_event_id, + ), + ) await self.db_pool.runInteraction( "sliding_sync_membership_snapshots_bg_update", _fill_table_txn From 1963f184b5be8c3953e5a78db2e686067dd2b2e7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 10:26:40 +0100 Subject: [PATCH 17/22] Update synapse/storage/databases/main/events.py Co-authored-by: Eric Eastwood --- synapse/storage/databases/main/events.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 4bd30abc830..c4994a5dc81 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1980,7 +1980,10 @@ def _get_sliding_sync_insert_values_from_state_map( # Scrutinize JSON values. We ignore values with nulls as # postgres doesn't allow null bytes in text columns. if room_name is None or ( - isinstance(room_name, str) and "\0" not in room_name + isinstance(room_name, str) + # We ignore values with null bytes as Postgres doesn't allow them in + # text columns. + and "\0" not in room_name ): sliding_sync_insert_map["room_name"] = room_name elif state_key == (EventTypes.Tombstone, ""): From ec6b2d5554243e8fa51fac37381580a2d1f11429 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 10:26:46 +0100 Subject: [PATCH 18/22] Update synapse/storage/databases/main/events.py Co-authored-by: Eric Eastwood --- synapse/storage/databases/main/events.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index c4994a5dc81..4899c5d3a71 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -2074,12 +2074,12 @@ def _get_sliding_sync_insert_values_from_stripped_state( else None ) + # Check for null bytes in the room name. We have to ignore values with + # null bytes as Postgres doesn't allow them in text columns. if ( sliding_sync_insert_map["room_name"] is not None and "\0" in sliding_sync_insert_map["room_name"] ): - # We ignore values with nulls as - # postgres doesn't allow null bytes in text columns. sliding_sync_insert_map.pop("room_name") # Find the tombstone_successor_room_id From 46804dd2668421a31433c135d1f409e65b7873fd Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 15:53:33 +0100 Subject: [PATCH 19/22] Also ignore room types with null bytes --- synapse/storage/databases/main/events.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 4899c5d3a71..912747579aa 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1967,7 +1967,12 @@ def _get_sliding_sync_insert_values_from_state_map( if state_key == (EventTypes.Create, ""): room_type = event.content.get(EventContentFields.ROOM_TYPE) # Scrutinize JSON values - if room_type is None or isinstance(room_type, str): + if room_type is None or ( + isinstance(room_type, str) + # We ignore values with null bytes as Postgres doesn't allow them in + # text columns. + and "\0" not in room_type + ): sliding_sync_insert_map["room_type"] = room_type elif state_key == (EventTypes.RoomEncryption, ""): encryption_algorithm = event.content.get( @@ -2074,14 +2079,21 @@ def _get_sliding_sync_insert_values_from_stripped_state( else None ) - # Check for null bytes in the room name. We have to ignore values with - # null bytes as Postgres doesn't allow them in text columns. + # Check for null bytes in the room name and type. We have to + # ignore values with null bytes as Postgres doesn't allow them + # in text columns. if ( sliding_sync_insert_map["room_name"] is not None and "\0" in sliding_sync_insert_map["room_name"] ): sliding_sync_insert_map.pop("room_name") + if ( + sliding_sync_insert_map["room_type"] is not None + and "\0" in sliding_sync_insert_map["room_type"] + ): + sliding_sync_insert_map.pop("room_type") + # Find the tombstone_successor_room_id # Note: This isn't one of the stripped state events according to the spec # but seems like there is no reason not to support this kind of thing. From 06173a31154bfe303bbf284b363a839287b96818 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Sep 2024 16:03:58 +0100 Subject: [PATCH 20/22] Also ignore null bytes in tombstone room IDs --- synapse/storage/databases/main/events.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 42386176b95..c0b7d8107d7 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -2009,7 +2009,12 @@ def _get_sliding_sync_insert_values_from_state_map( EventContentFields.TOMBSTONE_SUCCESSOR_ROOM ) # Scrutinize JSON values - if successor_room_id is None or isinstance(successor_room_id, str): + if successor_room_id is None or ( + isinstance(successor_room_id, str) + # We ignore values with null bytes as Postgres doesn't allow them in + # text columns. + and "\0" not in successor_room_id + ): sliding_sync_insert_map["tombstone_successor_room_id"] = ( successor_room_id ) @@ -2121,6 +2126,12 @@ def _get_sliding_sync_insert_values_from_stripped_state( else None ) + if ( + sliding_sync_insert_map["tombstone_successor_room_id"] is not None + and "\0" in sliding_sync_insert_map["tombstone_successor_room_id"] + ): + sliding_sync_insert_map.pop("tombstone_successor_room_id") + else: # No stripped state provided sliding_sync_insert_map["has_known_state"] = False From e0e7a8b24a82c2dca42cfb4691904568ddb50438 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 11 Sep 2024 14:49:39 +0100 Subject: [PATCH 21/22] Revert "Ignore leave events for bg updates" This reverts commit f71dd25a4a76cf25c58035dd737f5f151f4fd58a. --- synapse/storage/databases/main/events_bg_updates.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index a5d04c719fa..743200471b6 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1927,11 +1927,10 @@ 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, Membership.LEAVE, batch_size), + (last_room_id, last_user_id, batch_size), ) elif last_event_stream_ordering is not None: # It's important to sort by `event_stream_ordering` *ascending* (oldest to @@ -1962,11 +1961,10 @@ 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, Membership.LEAVE, batch_size), + (last_event_stream_ordering, batch_size), ) else: raise Exception("last_event_stream_ordering should not be None") From cf4e6cb8137319f08389305a9ea4461b9a956b8f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 11 Sep 2024 14:49:43 +0100 Subject: [PATCH 22/22] Revert "Fix tests for leaves" This reverts commit 8140ca353d8c505fcac541409aea5f8559aa6067. --- tests/storage/test_sliding_sync_tables.py | 73 ++++++++++++++++++++--- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/tests/storage/test_sliding_sync_tables.py b/tests/storage/test_sliding_sync_tables.py index 6cd9b64442f..61dccc8077d 100644 --- a/tests/storage/test_sliding_sync_tables.py +++ b/tests/storage/test_sliding_sync_tables.py @@ -3507,6 +3507,10 @@ 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, ) @@ -3888,12 +3892,16 @@ def test_membership_snapshots_background_update_remote_invite_rejections_and_ret # Reject the remote invites. # Also try retracting a remote invite. - self.helper.leave(room_id_unknown_state, user1_id, tok=user1_tok) + room_id_unknown_state_leave_event_response = 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, ) - self.helper.leave(room_id_with_info, user1_id, tok=user1_tok) + room_id_with_info_leave_event_response = 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, @@ -3948,11 +3956,37 @@ 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( @@ -3969,6 +4003,28 @@ 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( @@ -4164,6 +4220,10 @@ 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, ) @@ -4252,7 +4312,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=user2_tok) + self.helper.leave(room_id, user1_id, tok=user1_tok) state_map = self.get_success( self.storage_controllers.state.get_current_state(room_id) @@ -4321,7 +4381,7 @@ def test_membership_snapshots_background_update_forgotten_missing(self) -> None: _SlidingSyncMembershipSnapshotResult( room_id=room_id, user_id=user1_id, - sender=user2_id, + sender=user1_id, membership_event_id=state_map[(EventTypes.Member, user1_id)].event_id, membership=Membership.LEAVE, event_stream_ordering=state_map[ @@ -4760,7 +4820,7 @@ def test_membership_snapshots_background_update_catch_up_membership_change( ) # User2 leaves the room - self.helper.leave(room_id, user2_id, tok=user1_tok) + self.helper.leave(room_id, user2_id, tok=user2_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 @@ -4781,9 +4841,6 @@ 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,