From 8eb7bb975eed0250aa8be5e8fb70c586cbff6b37 Mon Sep 17 00:00:00 2001 From: Mathieu Velten Date: Wed, 12 Jul 2023 11:09:13 +0200 Subject: [PATCH] Mark get_user_in_directory private since only used in tests (#15884) --- changelog.d/15884.misc | 1 + .../storage/databases/main/user_directory.py | 9 +-------- tests/handlers/test_user_directory.py | 18 +++++++++--------- tests/rest/admin/test_user.py | 6 +++--- 4 files changed, 14 insertions(+), 20 deletions(-) create mode 100644 changelog.d/15884.misc diff --git a/changelog.d/15884.misc b/changelog.d/15884.misc new file mode 100644 index 0000000000..8e73a9a6cd --- /dev/null +++ b/changelog.d/15884.misc @@ -0,0 +1 @@ +Mark `get_user_in_directory` private since it is only used in tests. Also remove the cache from it. diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index b0a06baf4f..924022c95c 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -62,7 +62,6 @@ from synapse.types import ( get_domain_from_id, get_localpart_from_id, ) -from synapse.util.caches.descriptors import cached logger = logging.getLogger(__name__) @@ -771,9 +770,6 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore): # This should be unreachable. raise Exception("Unrecognized database engine") - for p in profiles: - txn.call_after(self.get_user_in_directory.invalidate, (p.user_id,)) - async def add_users_who_share_private_room( self, room_id: str, user_id_tuples: Iterable[Tuple[str, str]] ) -> None: @@ -831,14 +827,12 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore): txn.execute(f"{truncate} user_directory_search") txn.execute(f"{truncate} users_in_public_rooms") txn.execute(f"{truncate} users_who_share_private_rooms") - txn.call_after(self.get_user_in_directory.invalidate_all) await self.db_pool.runInteraction( "delete_all_from_user_dir", _delete_all_from_user_dir_txn ) - @cached() - async def get_user_in_directory(self, user_id: str) -> Optional[Mapping[str, str]]: + async def _get_user_in_directory(self, user_id: str) -> Optional[Mapping[str, str]]: return await self.db_pool.simple_select_one( table="user_directory", keyvalues={"user_id": user_id}, @@ -900,7 +894,6 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore): table="users_who_share_private_rooms", keyvalues={"other_user_id": user_id}, ) - txn.call_after(self.get_user_in_directory.invalidate, (user_id,)) await self.db_pool.runInteraction( "remove_from_user_dir", _remove_from_user_dir_txn diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 15a7dc6818..9785dd698b 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -356,7 +356,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): support_user_id, ProfileInfo("I love support me", None) ) ) - profile = self.get_success(self.store.get_user_in_directory(support_user_id)) + profile = self.get_success(self.store._get_user_in_directory(support_user_id)) self.assertIsNone(profile) display_name = "display_name" @@ -364,7 +364,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): self.get_success( self.handler.handle_local_profile_change(regular_user_id, profile_info) ) - profile = self.get_success(self.store.get_user_in_directory(regular_user_id)) + profile = self.get_success(self.store._get_user_in_directory(regular_user_id)) assert profile is not None self.assertTrue(profile["display_name"] == display_name) @@ -383,7 +383,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): ) # profile is in directory - profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + profile = self.get_success(self.store._get_user_in_directory(r_user_id)) assert profile is not None self.assertTrue(profile["display_name"] == display_name) @@ -392,7 +392,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): self.get_success(self.handler.handle_local_user_deactivated(r_user_id)) # profile is not in directory - profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + profile = self.get_success(self.store._get_user_in_directory(r_user_id)) self.assertIsNone(profile) # update profile after deactivation @@ -401,7 +401,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): ) # profile is furthermore not in directory - profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + profile = self.get_success(self.store._get_user_in_directory(r_user_id)) self.assertIsNone(profile) def test_handle_local_profile_change_with_appservice_user(self) -> None: @@ -411,7 +411,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): ) # profile is not in directory - profile = self.get_success(self.store.get_user_in_directory(as_user_id)) + profile = self.get_success(self.store._get_user_in_directory(as_user_id)) self.assertIsNone(profile) # update profile @@ -421,13 +421,13 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): ) # profile is still not in directory - profile = self.get_success(self.store.get_user_in_directory(as_user_id)) + profile = self.get_success(self.store._get_user_in_directory(as_user_id)) self.assertIsNone(profile) def test_handle_local_profile_change_with_appservice_sender(self) -> None: # profile is not in directory profile = self.get_success( - self.store.get_user_in_directory(self.appservice.sender) + self.store._get_user_in_directory(self.appservice.sender) ) self.assertIsNone(profile) @@ -441,7 +441,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): # profile is still not in directory profile = self.get_success( - self.store.get_user_in_directory(self.appservice.sender) + self.store._get_user_in_directory(self.appservice.sender) ) self.assertIsNone(profile) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index a17a1bb1d8..6f7b4bf642 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -2472,7 +2472,7 @@ class UserRestTestCase(unittest.HomeserverTestCase): """ # is in user directory - profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + profile = self.get_success(self.store._get_user_in_directory(self.other_user)) assert profile is not None self.assertTrue(profile["display_name"] == "User") @@ -2489,7 +2489,7 @@ class UserRestTestCase(unittest.HomeserverTestCase): self.assertTrue(channel.json_body["deactivated"]) # is not in user directory - profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + profile = self.get_success(self.store._get_user_in_directory(self.other_user)) self.assertIsNone(profile) # Set new displayname user @@ -2506,7 +2506,7 @@ class UserRestTestCase(unittest.HomeserverTestCase): self.assertEqual("Foobar", channel.json_body["displayname"]) # is not in user directory - profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + profile = self.get_success(self.store._get_user_in_directory(self.other_user)) self.assertIsNone(profile) def test_reactivate_user(self) -> None: