Skip to content

Revert "Add the ability to exclude remote users in user directory search results (#18300)" #18388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3nprob wants to merge 1 commit into from
Closed

Revert "Add the ability to exclude remote users in user directory search results (#18300)" #18388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion changelog.d/18300.feature
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions docs/usage/configuration/config_documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4095,7 +4095,6 @@ This option has the following sub-options:
* `prefer_local_users`: Defines whether to prefer local users in search query results.
If set to true, local users are more likely to appear above remote users when searching the
user directory. Defaults to false.
* `exclude_remote_users`: If set to true, the search will only return local users. Defaults to false.
* `show_locked_users`: Defines whether to show locked users in search query results. Defaults to false.

Example configuration:
Expand All @@ -4104,7 +4103,6 @@ user_directory:
enabled: false
search_all_users: true
prefer_local_users: true
exclude_remote_users: false
show_locked_users: true
```
---
Expand Down
3 changes: 0 additions & 3 deletions synapse/config/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.user_directory_search_all_users = user_directory_config.get(
"search_all_users", False
)
self.user_directory_exclude_remote_users = user_directory_config.get(
"exclude_remote_users", False
)
self.user_directory_search_prefer_local_users = user_directory_config.get(
"prefer_local_users", False
)
Expand Down
3 changes: 0 additions & 3 deletions synapse/handlers/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ def __init__(self, hs: "HomeServer"):
self.is_mine_id = hs.is_mine_id
self.update_user_directory = hs.config.worker.should_update_user_directory
self.search_all_users = hs.config.userdirectory.user_directory_search_all_users
self.exclude_remote_users = (
hs.config.userdirectory.user_directory_exclude_remote_users
)
self.show_locked_users = hs.config.userdirectory.show_locked_users
self._spam_checker_module_callbacks = hs.get_module_api_callbacks().spam_checker
self._hs = hs
Expand Down
18 changes: 4 additions & 14 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 @@ -1037,11 +1037,11 @@ async def search_user_dir(
}
"""

join_args: Tuple[str, ...] = (user_id,)

if self.hs.config.userdirectory.user_directory_search_all_users:
join_args = (user_id,)
where_clause = "user_id != ?"
else:
join_args = (user_id,)
where_clause = """
(
EXISTS (select 1 from users_in_public_rooms WHERE user_id = t.user_id)
Expand All @@ -1055,14 +1055,6 @@ async def search_user_dir(
if not show_locked_users:
where_clause += " AND (u.locked IS NULL OR u.locked = FALSE)"

# Adjust the JOIN type based on the exclude_remote_users flag (the users
# table only contains local users so an inner join is a good way to
# to exclude remote users)
if self.hs.config.userdirectory.user_directory_exclude_remote_users:
join_type = "JOIN"
else:
join_type = "LEFT JOIN"

# We allow manipulating the ranking algorithm by injecting statements
# based on config options.
additional_ordering_statements = []
Expand Down Expand Up @@ -1094,7 +1086,7 @@ async def search_user_dir(
SELECT d.user_id AS user_id, display_name, avatar_url
FROM matching_users as t
INNER JOIN user_directory AS d USING (user_id)
%(join_type)s users AS u ON t.user_id = u.name
LEFT JOIN users AS u ON t.user_id = u.name
WHERE
%(where_clause)s
ORDER BY
Expand Down Expand Up @@ -1123,7 +1115,6 @@ async def search_user_dir(
""" % {
"where_clause": where_clause,
"order_case_statements": " ".join(additional_ordering_statements),
"join_type": join_type,
}
args = (
(full_query,)
Expand Down Expand Up @@ -1151,7 +1142,7 @@ async def search_user_dir(
SELECT d.user_id AS user_id, display_name, avatar_url
FROM user_directory_search as t
INNER JOIN user_directory AS d USING (user_id)
%(join_type)s users AS u ON t.user_id = u.name
LEFT JOIN users AS u ON t.user_id = u.name
WHERE
%(where_clause)s
AND value MATCH ?
Expand All @@ -1164,7 +1155,6 @@ async def search_user_dir(
""" % {
"where_clause": where_clause,
"order_statements": " ".join(additional_ordering_statements),
"join_type": join_type,
}
args = join_args + (search_query,) + ordering_arguments + (limit + 1,)
else:
Expand Down
61 changes: 0 additions & 61 deletions tests/handlers/test_user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,67 +992,6 @@ def test_prefer_local_users(self) -> None:
[self.assertIn(user, local_users) for user in received_user_id_ordering[:3]]
[self.assertIn(user, remote_users) for user in received_user_id_ordering[3:]]

@override_config(
{
"user_directory": {
"enabled": True,
"search_all_users": True,
"exclude_remote_users": True,
}
}
)
def test_exclude_remote_users(self) -> None:
"""Tests that only local users are returned when
user_directory.exclude_remote_users is True.
"""

# Create a room and few users to test the directory with
searching_user = self.register_user("searcher", "password")
searching_user_tok = self.login("searcher", "password")

room_id = self.helper.create_room_as(
searching_user,
room_version=RoomVersions.V1.identifier,
tok=searching_user_tok,
)

# Create a few local users and join them to the room
local_user_1 = self.register_user("user_xxxxx", "password")
local_user_2 = self.register_user("user_bbbbb", "password")
local_user_3 = self.register_user("user_zzzzz", "password")

self._add_user_to_room(room_id, RoomVersions.V1, local_user_1)
self._add_user_to_room(room_id, RoomVersions.V1, local_user_2)
self._add_user_to_room(room_id, RoomVersions.V1, local_user_3)

# Create a few "remote" users and join them to the room
remote_user_1 = "@user_aaaaa:remote_server"
remote_user_2 = "@user_yyyyy:remote_server"
remote_user_3 = "@user_ccccc:remote_server"
self._add_user_to_room(room_id, RoomVersions.V1, remote_user_1)
self._add_user_to_room(room_id, RoomVersions.V1, remote_user_2)
self._add_user_to_room(room_id, RoomVersions.V1, remote_user_3)

local_users = [local_user_1, local_user_2, local_user_3]
remote_users = [remote_user_1, remote_user_2, remote_user_3]

# The local searching user searches for the term "user", which other users have
# in their user id
results = self.get_success(
self.handler.search_users(searching_user, "user", 20)
)["results"]
received_user_ids = [result["user_id"] for result in results]

for user in local_users:
self.assertIn(
user, received_user_ids, f"Local user {user} not found in results"
)

for user in remote_users:
self.assertNotIn(
user, received_user_ids, f"Remote user {user} should not be in results"
)

def _add_user_to_room(
self,
room_id: str,
Expand Down
Loading