Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Mark get_user_in_directory private since only used in tests #15884

Merged
merged 1 commit into from Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions 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.
9 changes: 1 addition & 8 deletions synapse/storage/databases/main/user_directory.py
Expand Up @@ -62,7 +62,6 @@
get_domain_from_id,
get_localpart_from_id,
)
from synapse.util.caches.descriptors import cached

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -771,9 +770,6 @@ def _update_profiles_in_user_dir_txn(
# 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:
Expand Down Expand Up @@ -831,14 +827,12 @@ def _delete_all_from_user_dir_txn(txn: LoggingTransaction) -> None:
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},
Expand Down Expand Up @@ -900,7 +894,6 @@ def _remove_from_user_dir_txn(txn: LoggingTransaction) -> None:
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
Expand Down
18 changes: 9 additions & 9 deletions tests/handlers/test_user_directory.py
Expand Up @@ -356,15 +356,15 @@ def test_handle_local_profile_change_with_support_user(self) -> None:
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"

profile_info = ProfileInfo(avatar_url="avatar_url", display_name=display_name)
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)

Expand All @@ -383,7 +383,7 @@ def test_handle_local_profile_change_with_deactivated_user(self) -> None:
)

# 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)

Expand All @@ -392,7 +392,7 @@ def test_handle_local_profile_change_with_deactivated_user(self) -> None:
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
Expand All @@ -401,7 +401,7 @@ def test_handle_local_profile_change_with_deactivated_user(self) -> None:
)

# 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:
Expand All @@ -411,7 +411,7 @@ def test_handle_local_profile_change_with_appservice_user(self) -> None:
)

# 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
Expand All @@ -421,13 +421,13 @@ def test_handle_local_profile_change_with_appservice_user(self) -> None:
)

# 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)

Expand All @@ -441,7 +441,7 @@ def test_handle_local_profile_change_with_appservice_sender(self) -> None:

# 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)

Expand Down
6 changes: 3 additions & 3 deletions tests/rest/admin/test_user.py
Expand Up @@ -2472,7 +2472,7 @@ def test_change_name_deactivate_user_user_directory(self) -> None:
"""

# 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")

Expand All @@ -2489,7 +2489,7 @@ def test_change_name_deactivate_user_user_directory(self) -> None:
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
Expand All @@ -2506,7 +2506,7 @@ def test_change_name_deactivate_user_user_directory(self) -> None:
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:
Expand Down