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

Consistently exclude from user_directory, round 2 #10960

Merged
merged 12 commits into from
Oct 4, 2021
Merged

Consistently exclude from user_directory, round 2 #10960

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b9638c9
Introduce `should_include_local_users_in_dir`
Sep 30, 2021
3f969d1
Introduce register_appservice_user to HomeserverTestCase
Sep 30, 2021
76751aa
Check appservice profile changes are ignored
Sep 30, 2021
010314d
test population excludes users it should
Sep 30, 2021
a8a12a5
Extra assertion in test_initial
Sep 30, 2021
8b614d7
Changelog
Sep 30, 2021
8c41d2e
Improve docstring of `register_appservice_user`
Oct 1, 2021
667ec6e
Use access_token parameter
Oct 1, 2021
c8bb561
Assert 200 response!
Oct 1, 2021
aeeb697
Add tests which interrogate room sharing tables
Oct 1, 2021
c090ab6
Correctly exclude from users_who_share_private_room
Oct 1, 2021
3dc823f
`setup` is not a verb
Oct 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 10 additions & 15 deletions synapse/storage/databases/main/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@

logger = logging.getLogger(__name__)


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how these crept in. Possibly I accidentally formatted in pycharm (ctrl-alt-l) and black was happy with the change?

TEMP_TABLE = "_temp_populate_user_directory"


class UserDirectoryBackgroundUpdateStore(StateDeltasStore):

# How many records do we calculate before sending it to
# add_users_who_share_private_rooms?
SHARE_PRIVATE_WORKING_SET = 500
Expand Down Expand Up @@ -235,13 +233,16 @@ def _get_next_batch(
)

users_with_profile = await self.get_users_in_room_with_profiles(room_id)
# Throw away users excluded from the directory.
users_with_profile = {
user_id: profile
for user_id, profile in users_with_profile.items()
if not self.hs.is_mine_id(user_id)
or await self.should_include_local_user_in_dir(user_id)
}

# Update each user in the user directory.
for user_id, profile in users_with_profile.items():
if self.hs.is_mine_id(
user_id
) and not await self.should_include_local_user_in_dir(user_id):
continue
await self.update_profile_in_user_dir(
user_id, profile.display_name, profile.avatar_url
)
Expand All @@ -250,24 +251,19 @@ def _get_next_batch(

if is_public:
for user_id in users_with_profile:
if self.hs.is_mine_id(
user_id
) and not await self.should_include_local_user_in_dir(user_id):
continue

to_insert.add(user_id)

if to_insert:
await self.add_users_in_public_rooms(room_id, to_insert)
to_insert.clear()
else:
for user_id in users_with_profile:
# We want the set of pairs (L, M) where L and M are
# in `users_with_profile` and L is local.
# Do so by looking for the local user L first.
if not self.hs.is_mine_id(user_id):
continue

if not await self.should_include_local_user_in_dir(user_id):
continue

for other_user_id in users_with_profile:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a hard-to-spot oversight right below here on -269.

We don't check to see if other_user_id ought to be excluded from the directory. We'll gleefully add them to users_who_share_private_rooms. This isn't a user-visible problem---if all of the logic elsewhere is correct, then other_user_id won't be in the user_directory table and so won't show up in searches. But it's a frustrating inconsistency.

Instead I'm going to filter out the excluded users early on, right before we assign a value to users_with_profile.

if user_id == other_user_id:
continue
Expand Down Expand Up @@ -562,7 +558,6 @@ async def update_user_directory_stream_pos(self, stream_id: Optional[int]) -> No


class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):

# How many records do we calculate before sending it to
# add_users_who_share_private_rooms?
SHARE_PRIVATE_WORKING_SET = 500
Expand Down