Consistently exclude from user_directory (#10960)

* Introduce `should_include_local_users_in_dir`

We exclude three kinds of local users from the user_directory tables. At
present we don't consistently exclude all three in the same places. This
commit introduces a new function to gather those exclusion conditions
together. Because we have to handle local and remote users in different
ways, I've made that function only consider the case of remote users.
It's the caller's responsibility to make the local versus remote
distinction clear and correct.

A test fixup is required. The test now hits a path which makes db
queries against the users table. The expected rows were missing, because
we were using a dummy user that hadn't actually been registered.

We also add new test cases to covert the exclusion logic.

----

By my reading this makes these changes:

* When an app service user registers or changes their profile, they will
  _not_ be added to the user directory. (Previously only support and
  deactivated users were excluded). This is consistent with the logic that
  rebuilds the user directory. See also [the discussion
  here](https://github.com/matrix-org/synapse/pull/10914#discussion_r716859548).
* When rebuilding the directory, exclude support and disabled users from
  room sharing tables. Previously only appservice users were excluded.
* Exclude all three categories of local users when rebuilding the
  directory. Previously `_populate_user_directory_process_users` didn't do
  any exclusion.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
This commit is contained in:
David Robertson 2021-10-04 12:45:51 +01:00 committed by GitHub
parent a0f48ee89d
commit f7b034a24b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 409 additions and 57 deletions

View file

@ -132,12 +132,7 @@ class UserDirectoryHandler(StateDeltasHandler):
# FIXME(#3714): We should probably do this in the same worker as all
# the other changes.
# Support users are for diagnostics and should not appear in the user directory.
is_support = await self.store.is_support_user(user_id)
# When change profile information of deactivated user it should not appear in the user directory.
is_deactivated = await self.store.get_user_deactivated_status(user_id)
if not (is_support or is_deactivated):
if await self.store.should_include_local_user_in_dir(user_id):
await self.store.update_profile_in_user_dir(
user_id, profile.display_name, profile.avatar_url
)
@ -229,8 +224,10 @@ class UserDirectoryHandler(StateDeltasHandler):
else:
logger.debug("Server is still in room: %r", room_id)
is_support = await self.store.is_support_user(state_key)
if not is_support:
include_in_dir = not self.is_mine_id(
state_key
) or await self.store.should_include_local_user_in_dir(state_key)
if include_in_dir:
if change is MatchChange.no_change:
# Handle any profile changes
await self._handle_profile_change(
@ -356,13 +353,7 @@ class UserDirectoryHandler(StateDeltasHandler):
# First, if they're our user then we need to update for every user
if self.is_mine_id(user_id):
is_appservice = self.store.get_if_app_services_interested_in_user(
user_id
)
# We don't care about appservice users.
if not is_appservice:
if await self.store.should_include_local_user_in_dir(user_id):
for other_user_id in other_users_in_room:
if user_id == other_user_id:
continue
@ -374,10 +365,10 @@ class UserDirectoryHandler(StateDeltasHandler):
if user_id == other_user_id:
continue
is_appservice = self.store.get_if_app_services_interested_in_user(
include_other_user = self.is_mine_id(
other_user_id
)
if self.is_mine_id(other_user_id) and not is_appservice:
) and await self.store.should_include_local_user_in_dir(other_user_id)
if include_other_user:
to_insert.add((other_user_id, user_id))
if to_insert: