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

Commit

Permalink
Mark get_user_in_directory private since only used in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Mathieu Velten committed Jul 5, 2023
1 parent 4cf9f92 commit 725b844
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 20 deletions.
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

0 comments on commit 725b844

Please sign in to comment.