From 52797dca58dfac10507dad0ca7642e1f680f302b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 16:40:56 -0500 Subject: [PATCH 1/8] Bust the cache when the room encryption is updated Make sure we get up-to-date information from `get_sliding_sync_rooms_for_user(...)` --- synapse/storage/_base.py | 1 + synapse/storage/databases/main/cache.py | 6 ++ synapse/storage/databases/main/roommember.py | 5 +- tests/storage/test_roommember.py | 74 ++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index d22160b85c5..1f894d15bb4 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -135,6 +135,7 @@ def _invalidate_state_caches( self._attempt_to_invalidate_cache("get_partial_current_state_ids", (room_id,)) self._attempt_to_invalidate_cache("get_room_type", (room_id,)) self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) + self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) def _invalidate_state_caches_all(self, room_id: str) -> None: """Invalidates caches that are based on the current state, but does diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index b0e30daee5a..7e529b5cc8d 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -275,6 +275,9 @@ def _process_event_stream_row(self, token: int, row: EventsStreamRow) -> None: self._attempt_to_invalidate_cache( "get_room_encryption", (data.room_id,) ) + self._attempt_to_invalidate_cache( + "get_sliding_sync_rooms_for_user", None + ) elif data.type == EventTypes.Create: self._attempt_to_invalidate_cache("get_room_type", (data.room_id,)) elif row.type == EventsStreamAllStateRow.TypeId: @@ -285,6 +288,7 @@ def _process_event_stream_row(self, token: int, row: EventsStreamRow) -> None: self._attempt_to_invalidate_cache("get_rooms_for_user", None) self._attempt_to_invalidate_cache("get_room_type", (data.room_id,)) self._attempt_to_invalidate_cache("get_room_encryption", (data.room_id,)) + self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) else: raise Exception("Unknown events stream row type %s" % (row.type,)) @@ -364,6 +368,7 @@ def _invalidate_caches_for_event( self._attempt_to_invalidate_cache("get_room_type", (room_id,)) elif etype == EventTypes.RoomEncryption: self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) + self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) if relates_to: self._attempt_to_invalidate_cache( @@ -483,6 +488,7 @@ def _invalidate_caches_for_room(self, room_id: str) -> None: self._attempt_to_invalidate_cache("get_room_version_id", (room_id,)) self._attempt_to_invalidate_cache("get_room_type", (room_id,)) self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) + self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) self._attempt_to_invalidate_cache("_get_max_event_pos", (room_id,)) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 8df760e8a64..ea672fe8ab8 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1393,6 +1393,9 @@ async def get_sliding_sync_rooms_for_user( def get_sliding_sync_rooms_for_user_txn( txn: LoggingTransaction, ) -> Dict[str, RoomsForUserSlidingSync]: + # XXX: If you return any new `COALESCE(...)` fields using + # `sliding_sync_joined_rooms`, make sure to bust the + # `get_sliding_sync_rooms_for_user` cache in the appropriate places. sql = """ SELECT m.room_id, m.sender, m.membership, m.membership_event_id, r.room_version, @@ -1415,7 +1418,7 @@ def get_sliding_sync_rooms_for_user_txn( room_version_id=row[4], event_pos=PersistedEventPosition(row[5], row[6]), room_type=row[7], - is_encrypted=row[8], + is_encrypted=bool(row[8]), ) for row in txn } diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 330fea0e624..70a473f5a2e 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -19,12 +19,14 @@ # [This file includes modifications made by New Vector Limited] # # +import attr import logging from typing import List, Optional, Tuple, cast from twisted.test.proto_helpers import MemoryReactor from synapse.api.constants import EventContentFields, EventTypes, JoinRules, Membership +from synapse.types import PersistedEventPosition from synapse.api.room_versions import RoomVersions from synapse.rest import admin from synapse.rest.admin import register_servlets_for_client_rest_resource @@ -34,6 +36,12 @@ from synapse.storage.roommember import MemberSummary from synapse.types import UserID, create_requester from synapse.util import Clock +from synapse.storage.roommember import ( + MemberSummary, + ProfileInfo, + RoomsForUser, + RoomsForUserSlidingSync, +) from tests import unittest from tests.server import TestHomeServer @@ -810,3 +818,69 @@ def test_can_rerun_update(self) -> None: # Now let's actually drive the updates to completion self.wait_for_background_updates() + + +class GetSlidingSyncRoomsForUserTestCase(unittest.HomeserverTestCase): + """ + Test `get_sliding_sync_rooms_for_user()` + """ + + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.store = hs.get_datastores().main + self.storage_controllers = hs.get_storage_controllers() + + def test_joined_up_to_date_is_encrypted(self) -> None: + """ + Make sure we get up-to-date `is_encrypted` status for a joined room + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_version = RoomVersions.V10 + room_id = self.helper.create_room_as( + user1_id, tok=user1_tok, room_version=room_version.identifier + ) + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id) + ) + + room1_for_user = RoomsForUserSlidingSync( + room_id=room_id, + sender=user1_id, + membership=Membership.JOIN, + event_id=state_map[(EventTypes.Member, user1_id)].event_id, + room_version_id=room_version.identifier, + event_pos=PersistedEventPosition( + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.instance_name, + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.stream_ordering, + ), + room_type=None, + is_encrypted=False, + ) + self.assertDictEqual( + self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), + {room_id: room1_for_user}, + ) + + # Update the encryption status + self.helper.send_state( + room_id, + EventTypes.RoomEncryption, + {EventContentFields.ENCRYPTION_ALGORITHM: "m.megolm.v1.aes-sha2"}, + tok=user1_tok, + ) + + self.assertDictEqual( + self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), + {room_id: attr.evolve(room1_for_user, is_encrypted=True)}, + ) From a384147b0f6d200832bf5ada890c44eec4991985 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 16:59:58 -0500 Subject: [PATCH 2/8] Add other tests --- synapse/storage/databases/main/roommember.py | 3 +- tests/storage/test_roommember.py | 121 +++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index ea672fe8ab8..734b0b3770a 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1395,7 +1395,8 @@ def get_sliding_sync_rooms_for_user_txn( ) -> Dict[str, RoomsForUserSlidingSync]: # XXX: If you return any new `COALESCE(...)` fields using # `sliding_sync_joined_rooms`, make sure to bust the - # `get_sliding_sync_rooms_for_user` cache in the appropriate places. + # `get_sliding_sync_rooms_for_user` cache in the appropriate places (and add + # tests). sql = """ SELECT m.room_id, m.sender, m.membership, m.membership_event_id, r.room_version, diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 70a473f5a2e..43ecbf3b102 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -22,6 +22,8 @@ import attr import logging from typing import List, Optional, Tuple, cast +from parameterized import parameterized, parameterized_class + from twisted.test.proto_helpers import MemoryReactor @@ -835,6 +837,125 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = hs.get_datastores().main self.storage_controllers = hs.get_storage_controllers() + @parameterized.expand( + [ + (Membership.JOIN,), + (Membership.INVITE,), + (Membership.LEAVE,), + (Membership.BAN,), + ] + ) + def test_updated_membership(self, test_membership: str) -> None: + """ + Make sure we get all new membership + """ + 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_version = RoomVersions.V10 + room_id1 = self.helper.create_room_as( + user2_id, tok=user2_tok, room_version=room_version.identifier + ) + room_id2 = self.helper.create_room_as( + user2_id, tok=user2_tok, room_version=room_version.identifier + ) + + # User1 joins room1 + self.helper.join(room_id1, user1_id, tok=user1_tok) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + room1_for_user = RoomsForUserSlidingSync( + room_id=room_id1, + sender=user1_id, + membership=Membership.JOIN, + event_id=state_map[(EventTypes.Member, user1_id)].event_id, + room_version_id=room_version.identifier, + event_pos=PersistedEventPosition( + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.instance_name, + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.stream_ordering, + ), + room_type=None, + is_encrypted=False, + ) + self.assertDictEqual( + self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), + {room_id1: room1_for_user}, + ) + + test_sender: str + if test_membership == Membership.JOIN: + # User1 joins room2 + self.helper.join(room_id2, user1_id, tok=user1_tok) + test_sender = user1_id + elif test_membership == Membership.INVITE: + # User1 is invited to room2 + self.helper.invite(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) + test_sender = user2_id + elif test_membership == Membership.LEAVE: + # Kick user1 from the rooms (leave) + self.helper.change_membership( + room=room_id2, + src=user2_id, + targ=user1_id, + tok=user2_tok, + membership=Membership.LEAVE, + extra_data={ + "reason": "Bad manners", + }, + ) + test_sender = user2_id + elif test_membership == Membership.BAN: + # User1 is banned from room2 + self.helper.ban(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) + test_sender = user2_id + else: + raise AssertionError("Unknown test_membership") + + # Check for the updated membership + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id2) + ) + self.assertDictEqual( + self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), + { + room_id1: room1_for_user, + room_id2: RoomsForUserSlidingSync( + room_id=room_id2, + sender=test_sender, + membership=test_membership, + event_id=state_map[(EventTypes.Member, user1_id)].event_id, + room_version_id=room_version.identifier, + event_pos=PersistedEventPosition( + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.instance_name, + state_map[ + (EventTypes.Member, user1_id) + ].internal_metadata.stream_ordering, + ), + room_type=None, + is_encrypted=False, + ), + }, + ) + + # def test_forgotten_up_to_date(self) -> None: + # """ + # Make sure we see up-to-date forgotten rooms + # """ + # TODO + + # Because `room_type` can only be set on room creation, it's always up-to-date once + # inserted. + def test_joined_up_to_date_is_encrypted(self) -> None: """ Make sure we get up-to-date `is_encrypted` status for a joined room From e0fc7ec33f2de362c58aaabf17f904e71ca251be Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:08:48 -0500 Subject: [PATCH 3/8] Use rest-layer test --- .../client/sliding_sync/test_sliding_sync.py | 143 ++++++++------ tests/storage/test_roommember.py | 185 ------------------ 2 files changed, 88 insertions(+), 240 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index 930cb5ef450..15ae5681f83 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -722,43 +722,37 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None: self.helper.join(space_room_id, user1_id, tok=user1_tok) # Make an initial Sliding Sync request - channel = self.make_request( - "POST", - self.sync_endpoint, - { - "lists": { - "all-list": { - "ranges": [[0, 99]], - "required_state": [], - "timeline_limit": 0, - "filters": {}, - }, - "foo-list": { - "ranges": [[0, 99]], - "required_state": [], - "timeline_limit": 1, - "filters": { - "is_encrypted": True, - "room_types": [RoomTypes.SPACE], - }, + sync_body = { + "lists": { + "all-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + "filters": {}, + }, + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 1, + "filters": { + "is_encrypted": True, + "room_types": [RoomTypes.SPACE], }, - } - }, - access_token=user1_tok, - ) - self.assertEqual(channel.code, 200, channel.json_body) - from_token = channel.json_body["pos"] + }, + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) # Make sure the response has the lists we requested self.assertListEqual( - list(channel.json_body["lists"].keys()), + list(response_body["lists"].keys()), ["all-list", "foo-list"], - channel.json_body["lists"].keys(), + response_body["lists"].keys(), ) # Make sure the lists have the correct rooms self.assertListEqual( - list(channel.json_body["lists"]["all-list"]["ops"]), + list(response_body["lists"]["all-list"]["ops"]), [ { "op": "SYNC", @@ -768,7 +762,7 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None: ], ) self.assertListEqual( - list(channel.json_body["lists"]["foo-list"]["ops"]), + list(response_body["lists"]["foo-list"]["ops"]), [ { "op": "SYNC", @@ -783,35 +777,30 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None: self.helper.leave(space_room_id, user2_id, tok=user2_tok) # Make an incremental Sliding Sync request - channel = self.make_request( - "POST", - self.sync_endpoint + f"?pos={from_token}", - { - "lists": { - "all-list": { - "ranges": [[0, 99]], - "required_state": [], - "timeline_limit": 0, - "filters": {}, - }, - "foo-list": { - "ranges": [[0, 99]], - "required_state": [], - "timeline_limit": 1, - "filters": { - "is_encrypted": True, - "room_types": [RoomTypes.SPACE], - }, + sync_body = { + "lists": { + "all-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + "filters": {}, + }, + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 1, + "filters": { + "is_encrypted": True, + "room_types": [RoomTypes.SPACE], }, - } - }, - access_token=user1_tok, - ) - self.assertEqual(channel.code, 200, channel.json_body) + }, + } + } + response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) # Make sure the lists have the correct rooms even though we `newly_left` self.assertListEqual( - list(channel.json_body["lists"]["all-list"]["ops"]), + list(response_body["lists"]["all-list"]["ops"]), [ { "op": "SYNC", @@ -821,7 +810,7 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None: ], ) self.assertListEqual( - list(channel.json_body["lists"]["foo-list"]["ops"]), + list(response_body["lists"]["foo-list"]["ops"]), [ { "op": "SYNC", @@ -831,6 +820,50 @@ def test_filter_regardless_of_membership_server_left_room(self) -> None: ], ) + def test_filter_is_encrypted_up_to_date(self) -> None: + """ + Make sure we get up-to-date `is_encrypted` status for a joined room + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + "filters": { + "is_encrypted": True, + }, + }, + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + set(), + exact=True, + ) + + # Update the encryption status + self.helper.send_state( + room_id, + EventTypes.RoomEncryption, + {EventContentFields.ENCRYPTION_ALGORITHM: "m.megolm.v1.aes-sha2"}, + tok=user1_tok, + ) + + # We should see the room now because it's encrypted + response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id}, + exact=True, + ) + def test_sort_list(self) -> None: """ Test that the `lists` are sorted by `stream_ordering` diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 43ecbf3b102..303116c6fd0 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -820,188 +820,3 @@ def test_can_rerun_update(self) -> None: # Now let's actually drive the updates to completion self.wait_for_background_updates() - - -class GetSlidingSyncRoomsForUserTestCase(unittest.HomeserverTestCase): - """ - Test `get_sliding_sync_rooms_for_user()` - """ - - servlets = [ - admin.register_servlets, - login.register_servlets, - room.register_servlets, - ] - - def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: - self.store = hs.get_datastores().main - self.storage_controllers = hs.get_storage_controllers() - - @parameterized.expand( - [ - (Membership.JOIN,), - (Membership.INVITE,), - (Membership.LEAVE,), - (Membership.BAN,), - ] - ) - def test_updated_membership(self, test_membership: str) -> None: - """ - Make sure we get all new membership - """ - 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_version = RoomVersions.V10 - room_id1 = self.helper.create_room_as( - user2_id, tok=user2_tok, room_version=room_version.identifier - ) - room_id2 = self.helper.create_room_as( - user2_id, tok=user2_tok, room_version=room_version.identifier - ) - - # User1 joins room1 - self.helper.join(room_id1, user1_id, tok=user1_tok) - - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id1) - ) - room1_for_user = RoomsForUserSlidingSync( - room_id=room_id1, - sender=user1_id, - membership=Membership.JOIN, - event_id=state_map[(EventTypes.Member, user1_id)].event_id, - room_version_id=room_version.identifier, - event_pos=PersistedEventPosition( - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.instance_name, - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.stream_ordering, - ), - room_type=None, - is_encrypted=False, - ) - self.assertDictEqual( - self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), - {room_id1: room1_for_user}, - ) - - test_sender: str - if test_membership == Membership.JOIN: - # User1 joins room2 - self.helper.join(room_id2, user1_id, tok=user1_tok) - test_sender = user1_id - elif test_membership == Membership.INVITE: - # User1 is invited to room2 - self.helper.invite(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) - test_sender = user2_id - elif test_membership == Membership.LEAVE: - # Kick user1 from the rooms (leave) - self.helper.change_membership( - room=room_id2, - src=user2_id, - targ=user1_id, - tok=user2_tok, - membership=Membership.LEAVE, - extra_data={ - "reason": "Bad manners", - }, - ) - test_sender = user2_id - elif test_membership == Membership.BAN: - # User1 is banned from room2 - self.helper.ban(room_id2, src=user2_id, targ=user1_id, tok=user2_tok) - test_sender = user2_id - else: - raise AssertionError("Unknown test_membership") - - # Check for the updated membership - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id2) - ) - self.assertDictEqual( - self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), - { - room_id1: room1_for_user, - room_id2: RoomsForUserSlidingSync( - room_id=room_id2, - sender=test_sender, - membership=test_membership, - event_id=state_map[(EventTypes.Member, user1_id)].event_id, - room_version_id=room_version.identifier, - event_pos=PersistedEventPosition( - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.instance_name, - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.stream_ordering, - ), - room_type=None, - is_encrypted=False, - ), - }, - ) - - # def test_forgotten_up_to_date(self) -> None: - # """ - # Make sure we see up-to-date forgotten rooms - # """ - # TODO - - # Because `room_type` can only be set on room creation, it's always up-to-date once - # inserted. - - def test_joined_up_to_date_is_encrypted(self) -> None: - """ - Make sure we get up-to-date `is_encrypted` status for a joined room - """ - user1_id = self.register_user("user1", "pass") - user1_tok = self.login(user1_id, "pass") - - room_version = RoomVersions.V10 - room_id = self.helper.create_room_as( - user1_id, tok=user1_tok, room_version=room_version.identifier - ) - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id) - ) - - room1_for_user = RoomsForUserSlidingSync( - room_id=room_id, - sender=user1_id, - membership=Membership.JOIN, - event_id=state_map[(EventTypes.Member, user1_id)].event_id, - room_version_id=room_version.identifier, - event_pos=PersistedEventPosition( - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.instance_name, - state_map[ - (EventTypes.Member, user1_id) - ].internal_metadata.stream_ordering, - ), - room_type=None, - is_encrypted=False, - ) - self.assertDictEqual( - self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), - {room_id: room1_for_user}, - ) - - # Update the encryption status - self.helper.send_state( - room_id, - EventTypes.RoomEncryption, - {EventContentFields.ENCRYPTION_ALGORITHM: "m.megolm.v1.aes-sha2"}, - tok=user1_tok, - ) - - self.assertDictEqual( - self.get_success(self.store.get_sliding_sync_rooms_for_user(user1_id)), - {room_id: attr.evolve(room1_for_user, is_encrypted=True)}, - ) From f0f06ead42993b72083da28aeac4789e895fb026 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:10:57 -0500 Subject: [PATCH 4/8] Fix lints --- tests/storage/test_roommember.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 303116c6fd0..330fea0e624 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -19,16 +19,12 @@ # [This file includes modifications made by New Vector Limited] # # -import attr import logging from typing import List, Optional, Tuple, cast -from parameterized import parameterized, parameterized_class - from twisted.test.proto_helpers import MemoryReactor from synapse.api.constants import EventContentFields, EventTypes, JoinRules, Membership -from synapse.types import PersistedEventPosition from synapse.api.room_versions import RoomVersions from synapse.rest import admin from synapse.rest.admin import register_servlets_for_client_rest_resource @@ -38,12 +34,6 @@ from synapse.storage.roommember import MemberSummary from synapse.types import UserID, create_requester from synapse.util import Clock -from synapse.storage.roommember import ( - MemberSummary, - ProfileInfo, - RoomsForUser, - RoomsForUserSlidingSync, -) from tests import unittest from tests.server import TestHomeServer From 3152d5c6becceaa2ce45ffc942a99314f9e1daa9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:13:52 -0500 Subject: [PATCH 5/8] Add changelog --- changelog.d/17692.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17692.bugfix diff --git a/changelog.d/17692.bugfix b/changelog.d/17692.bugfix new file mode 100644 index 00000000000..84e0754a99f --- /dev/null +++ b/changelog.d/17692.bugfix @@ -0,0 +1 @@ +Make sure we get up-to-date state information when using the new Sliding Sync tables to derive room membership. From 62b1745981437feb563d9502d2d552bc9be4fc14 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:26:56 -0500 Subject: [PATCH 6/8] Bust cache when we forget --- synapse/storage/databases/main/cache.py | 2 +- synapse/storage/databases/main/roommember.py | 7 ++- .../client/sliding_sync/test_sliding_sync.py | 48 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index 7e529b5cc8d..49569039156 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -482,13 +482,13 @@ def _invalidate_caches_for_room(self, room_id: str) -> None: self._attempt_to_invalidate_cache( "get_current_hosts_in_room_ordered", (room_id,) ) + self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) self._attempt_to_invalidate_cache("did_forget", None) self._attempt_to_invalidate_cache("get_forgotten_rooms_for_user", None) self._attempt_to_invalidate_cache("_get_membership_from_event_id", None) self._attempt_to_invalidate_cache("get_room_version_id", (room_id,)) self._attempt_to_invalidate_cache("get_room_type", (room_id,)) self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) - self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) self._attempt_to_invalidate_cache("_get_max_event_pos", (room_id,)) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 734b0b3770a..feaa33144b4 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1348,6 +1348,9 @@ def f(txn: LoggingTransaction) -> None: self._invalidate_cache_and_stream( txn, self.get_forgotten_rooms_for_user, (user_id,) ) + self._invalidate_cache_and_stream( + txn, self.get_sliding_sync_rooms_for_user, (user_id,) + ) await self.db_pool.runInteraction("forget_membership", f) @@ -1393,8 +1396,8 @@ async def get_sliding_sync_rooms_for_user( def get_sliding_sync_rooms_for_user_txn( txn: LoggingTransaction, ) -> Dict[str, RoomsForUserSlidingSync]: - # XXX: If you return any new `COALESCE(...)` fields using - # `sliding_sync_joined_rooms`, make sure to bust the + # XXX: If you use any new columns that can change (like from + # `sliding_sync_joined_rooms` or `forgotten`), make sure to bust the # `get_sliding_sync_rooms_for_user` cache in the appropriate places (and add # tests). sql = """ diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index 15ae5681f83..9e23dbe5222 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -864,6 +864,54 @@ def test_filter_is_encrypted_up_to_date(self) -> None: exact=True, ) + def test_forgotten_up_to_date(self) -> None: + """ + Make sure we get up-to-date `forgotten` status for 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") + + room_id = self.helper.create_room_as(user2_id, tok=user2_tok) + + # User1 is banned from the room (was never in the room) + self.helper.ban(room_id, src=user2_id, targ=user1_id, tok=user2_tok) + + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 99]], + "required_state": [], + "timeline_limit": 0, + "filters": {}, + }, + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + {room_id}, + exact=True, + ) + + # User1 forgets 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) + + # We should no longer see the forgotten room + response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) + self.assertIncludes( + set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]), + set(), + exact=True, + ) + def test_sort_list(self) -> None: """ Test that the `lists` are sorted by `stream_ordering` From cb17f3951f6516cd98733a0d7af11d38a623b89d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:57:25 -0500 Subject: [PATCH 7/8] Make sure we bust cache wherever we already bust the cache for `get_rooms_for_user(...)` --- synapse/storage/databases/main/cache.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index 49569039156..4c2305b894d 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -45,6 +45,7 @@ from synapse.storage.util.id_generators import MultiWriterIdGenerator from synapse.util.caches.descriptors import CachedFunction from synapse.util.iterutils import batch_iter +from synapse.storage.databases.main.events import SLIDING_SYNC_RELEVANT_STATE_SET if TYPE_CHECKING: from synapse.server import HomeServer @@ -271,15 +272,20 @@ def _process_event_stream_row(self, token: int, row: EventsStreamRow) -> None: self._attempt_to_invalidate_cache( "get_rooms_for_user", (data.state_key,) ) + self._attempt_to_invalidate_cache( + "get_sliding_sync_rooms_for_user", None + ) elif data.type == EventTypes.RoomEncryption: self._attempt_to_invalidate_cache( "get_room_encryption", (data.room_id,) ) + elif data.type == EventTypes.Create: + self._attempt_to_invalidate_cache("get_room_type", (data.room_id,)) + + if (data.type, data.state_key) in SLIDING_SYNC_RELEVANT_STATE_SET: self._attempt_to_invalidate_cache( "get_sliding_sync_rooms_for_user", None ) - elif data.type == EventTypes.Create: - self._attempt_to_invalidate_cache("get_room_type", (data.room_id,)) elif row.type == EventsStreamAllStateRow.TypeId: assert isinstance(data, EventsStreamAllStateRow) # Similar to the above, but the entire caches are invalidated. This is @@ -368,6 +374,8 @@ def _invalidate_caches_for_event( self._attempt_to_invalidate_cache("get_room_type", (room_id,)) elif etype == EventTypes.RoomEncryption: self._attempt_to_invalidate_cache("get_room_encryption", (room_id,)) + + if (etype, state_key) in SLIDING_SYNC_RELEVANT_STATE_SET: self._attempt_to_invalidate_cache("get_sliding_sync_rooms_for_user", None) if relates_to: From 260974ccefcd00266a8ff8b1a7c33a7f5a5a1be8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 10 Sep 2024 17:58:53 -0500 Subject: [PATCH 8/8] Fix lints --- synapse/storage/databases/main/cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/cache.py b/synapse/storage/databases/main/cache.py index 4c2305b894d..37c865a8e75 100644 --- a/synapse/storage/databases/main/cache.py +++ b/synapse/storage/databases/main/cache.py @@ -41,11 +41,11 @@ LoggingDatabaseConnection, LoggingTransaction, ) +from synapse.storage.databases.main.events import SLIDING_SYNC_RELEVANT_STATE_SET from synapse.storage.engines import PostgresEngine from synapse.storage.util.id_generators import MultiWriterIdGenerator from synapse.util.caches.descriptors import CachedFunction from synapse.util.iterutils import batch_iter -from synapse.storage.databases.main.events import SLIDING_SYNC_RELEVANT_STATE_SET if TYPE_CHECKING: from synapse.server import HomeServer