Always add local users to the user directory (#10796)

It's a simplification, but one that'll help make the user directory logic easier
to follow with the other changes upcoming. It's not strictly required for those
changes, but this will help simplify the resulting logic that listens for
`m.room.member` events and generally make the logic easier to follow.

This means the config option `search_all_users` ends up controlling the
search query only, and not the data we store. The cost of doing so is an
extra row in the `user_directory` and `user_directory_search` tables for
each local user which

- belongs to no public rooms
- belongs to no private rooms of size ≥ 2

I think the cost of this will be marginal (since they'll already have entries
 in `users` and `profiles` anyway).

As a small upside, a homeserver whose directory was built with this
change can toggle `search_all_users` without having to rebuild their
directory.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

changelog.d/10796.misc

@@ -0,0 +1 @@
+Simplify the internal logic which maintains the user directory database tables.

docs/sample_config.yaml

@@ -2362,12 +2362,16 @@ user_directory:
     #enabled: false
 
     # Defines whether to search all users visible to your HS when searching
-    # the user directory, rather than limiting to users visible in public
-    # rooms. Defaults to false.
+    # the user directory. If false, search results will only contain users
+    # visible in public rooms and users sharing a room with the requester.
+    # Defaults to false.
     #
-    # If you set it true, you'll have to rebuild the user_directory search
-    # indexes, see:
-    # https://matrix-org.github.io/synapse/latest/user_directory.html
+    # NB. If you set this to true, and the last time the user_directory search
+    # indexes were (re)built was before Synapse 1.44, you'll have to
+    # rebuild the indexes in order to search through all known users.
+    # These indexes are built the first time Synapse starts; admins can
+    # manually trigger a rebuild following the instructions at
+    #     https://matrix-org.github.io/synapse/latest/user_directory.html
     #
     # Uncomment to return search results containing all known users, even if that
     # user does not share a room with the requester.

synapse/config/user_directory.py

@@ -45,12 +45,16 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
             #enabled: false
 
             # Defines whether to search all users visible to your HS when searching
-            # the user directory, rather than limiting to users visible in public
-            # rooms. Defaults to false.
+            # the user directory. If false, search results will only contain users
+            # visible in public rooms and users sharing a room with the requester.
+            # Defaults to false.
             #
-            # If you set it true, you'll have to rebuild the user_directory search
-            # indexes, see:
-            # https://matrix-org.github.io/synapse/latest/user_directory.html
+            # NB. If you set this to true, and the last time the user_directory search
+            # indexes were (re)built was before Synapse 1.44, you'll have to
+            # rebuild the indexes in order to search through all known users.
+            # These indexes are built the first time Synapse starts; admins can
+            # manually trigger a rebuild following the instructions at
+            #     https://matrix-org.github.io/synapse/latest/user_directory.html
             #
             # Uncomment to return search results containing all known users, even if that
             # user does not share a room with the requester.

synapse/handlers/deactivate_account.py

@@ -257,11 +257,8 @@ async def activate_account(self, user_id: str) -> None:
         """
         # Add the user to the directory, if necessary.
         user = UserID.from_string(user_id)
-        if self.hs.config.user_directory_search_all_users:
-            profile = await self.store.get_profileinfo(user.localpart)
-            await self.user_directory_handler.handle_local_profile_change(
-                user_id, profile
-            )
+        profile = await self.store.get_profileinfo(user.localpart)
+        await self.user_directory_handler.handle_local_profile_change(user_id, profile)
 
         # Ensure the user is not marked as erased.
         await self.store.mark_user_not_erased(user_id)

synapse/handlers/profile.py

@@ -214,11 +214,10 @@ async def set_displayname(
             target_user.localpart, displayname_to_set
         )
 
-        if self.hs.config.user_directory_search_all_users:
-            profile = await self.store.get_profileinfo(target_user.localpart)
-            await self.user_directory_handler.handle_local_profile_change(
-                target_user.to_string(), profile
-            )
+        profile = await self.store.get_profileinfo(target_user.localpart)
+        await self.user_directory_handler.handle_local_profile_change(
+            target_user.to_string(), profile
+        )
 
         await self._update_join_states(requester, target_user)
 
@@ -300,11 +299,10 @@ async def set_avatar_url(
             target_user.localpart, avatar_url_to_set
         )
 
-        if self.hs.config.user_directory_search_all_users:
-            profile = await self.store.get_profileinfo(target_user.localpart)
-            await self.user_directory_handler.handle_local_profile_change(
-                target_user.to_string(), profile
-            )
+        profile = await self.store.get_profileinfo(target_user.localpart)
+        await self.user_directory_handler.handle_local_profile_change(
+            target_user.to_string(), profile
+        )
 
         await self._update_join_states(requester, target_user)
 

synapse/handlers/register.py

@@ -295,11 +295,10 @@ async def register_user(
                 shadow_banned=shadow_banned,
             )
 
-            if self.hs.config.user_directory_search_all_users:
-                profile = await self.store.get_profileinfo(localpart)
-                await self.user_directory_handler.handle_local_profile_change(
-                    user_id, profile
-                )
+            profile = await self.store.get_profileinfo(localpart)
+            await self.user_directory_handler.handle_local_profile_change(
+                user_id, profile
+            )
 
         else:
             # autogen a sequential user ID

synapse/storage/databases/main/user_directory.py

@@ -85,19 +85,17 @@ def _make_staging_area(txn):
             self.db_pool.simple_insert_many_txn(txn, TEMP_TABLE + "_rooms", rooms)
             del rooms
 
-            # If search all users is on, get all the users we want to add.
-            if self.hs.config.user_directory_search_all_users:
-                sql = (
-                    "CREATE TABLE IF NOT EXISTS "
-                    + TEMP_TABLE
-                    + "_users(user_id TEXT NOT NULL)"
-                )
-                txn.execute(sql)
+            sql = (
+                "CREATE TABLE IF NOT EXISTS "
+                + TEMP_TABLE
+                + "_users(user_id TEXT NOT NULL)"
+            )
+            txn.execute(sql)
 
-                txn.execute("SELECT name FROM users")
-                users = [{"user_id": x[0]} for x in txn.fetchall()]
+            txn.execute("SELECT name FROM users")
+            users = [{"user_id": x[0]} for x in txn.fetchall()]
 
-                self.db_pool.simple_insert_many_txn(txn, TEMP_TABLE + "_users", users)
+            self.db_pool.simple_insert_many_txn(txn, TEMP_TABLE + "_users", users)
 
         new_pos = await self.get_max_stream_id_in_current_state_deltas()
         await self.db_pool.runInteraction(
@@ -265,13 +263,8 @@ def _get_next_batch(txn):
 
     async def _populate_user_directory_process_users(self, progress, batch_size):
         """
-        If search_all_users is enabled, add all of the users to the user directory.
+        Add all local users to the user directory.
         """
-        if not self.hs.config.user_directory_search_all_users:
-            await self.db_pool.updates._end_background_update(
-                "populate_user_directory_process_users"
-            )
-            return 1
 
         def _get_next_batch(txn):
             sql = "SELECT user_id FROM %s LIMIT %s" % (

tests/handlers/test_profile.py

@@ -16,6 +16,7 @@
 
 import synapse.types
 from synapse.api.errors import AuthError, SynapseError
+from synapse.rest import admin
 from synapse.types import UserID
 
 from tests import unittest
@@ -25,6 +26,8 @@
 class ProfileTestCase(unittest.HomeserverTestCase):
     """Tests profile management."""
 
+    servlets = [admin.register_servlets]
+
     def make_homeserver(self, reactor, clock):
         self.mock_federation = Mock()
         self.mock_registry = Mock()
@@ -46,11 +49,11 @@ def register_query_handler(query_type, handler):
     def prepare(self, reactor, clock, hs):
         self.store = hs.get_datastore()
 
-        self.frank = UserID.from_string("@1234ABCD:test")
+        self.frank = UserID.from_string("@1234abcd:test")
         self.bob = UserID.from_string("@4567:test")
         self.alice = UserID.from_string("@alice:remote")
 
-        self.get_success(self.store.create_profile(self.frank.localpart))
+        self.get_success(self.register_user(self.frank.localpart, "frankpassword"))
 
         self.handler = hs.get_profile_handler()
 

tests/rest/client/test_rooms.py

@@ -869,6 +869,12 @@ class RoomJoinRatelimitTestCase(RoomBase):
         room.register_servlets,
     ]
 
+    def prepare(self, reactor, clock, homeserver):
+        super().prepare(reactor, clock, homeserver)
+        # profile changes expect that the user is actually registered
+        user = UserID.from_string(self.user_id)
+        self.get_success(self.register_user(user.localpart, "supersecretpassword"))
+
     @unittest.override_config(
         {"rc_joins": {"local": {"per_second": 0.5, "burst_count": 3}}}
     )
@@ -898,12 +904,6 @@ def test_join_local_ratelimit_profile_change(self):
         # join in a second.
         room_ids.append(self.helper.create_room_as(self.user_id))
 
-        # Create a profile for the user, since it hasn't been done on registration.
-        store = self.hs.get_datastore()
-        self.get_success(
-            store.create_profile(UserID.from_string(self.user_id).localpart)
-        )
-
         # Update the display name for the user.
         path = "/_matrix/client/r0/profile/%s/displayname" % self.user_id
         channel = self.make_request("PUT", path, {"displayname": "John Doe"})