diff options
| -rw-r--r-- | synapse/handlers/user_directory.py | 41 | ||||
| -rw-r--r-- | synapse/storage/schema/delta/53/user_share.sql | 3 | ||||
| -rw-r--r-- | synapse/storage/schema/delta/53/users_in_public_rooms.sql | 28 | ||||
| -rw-r--r-- | synapse/storage/user_directory.py | 34 | ||||
| -rw-r--r-- | tests/handlers/test_user_directory.py | 12 |
5 files changed, 114 insertions, 4 deletions
diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index c21da8343a..fc45123d0c 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -64,6 +64,10 @@ class UserDirectoryHandler(object): # This is a set of user_id's we've inserted already self.initially_handled_users = set() + self.register_background_update_handler( + "users_in_public_rooms_initial", self._populate_users_in_public_rooms + ) + # The current position in the current_state_delta stream self.pos = None @@ -77,6 +81,41 @@ class UserDirectoryHandler(object): # we start populating the user directory self.clock.call_later(0, self.notify_new_event) + @defer.inlineCallbacks + def _populate_users_in_public_rooms(self, progress, batch_size): + """ + Populate the users_in_public_rooms table with the contents of the + users_who_share_public_rooms table. + """ + + def _fetch(txn): + sql = "SELECT DISTINCT other_user_id FROM users_who_share_public_rooms" + txn.execute(sql) + return txn.fetchall() + + users = yield self.store.runInteraction( + "populate_users_in_public_rooms_fetch", _fetch + ) + + if users: + + def _fill(txn): + self._simple_upsert_many_txn( + txn, + table="users_in_public_rooms", + key_names=["user_id"], + key_values=users, + value_names=(), + value_values=None, + ) + + users = yield self.store.runInteraction( + "populate_users_in_public_rooms_fill", _fill + ) + + yield self._end_background_update("users_in_public_rooms_initial") + defer.returnValue(1) + def search_users(self, user_id, search_term, limit): """Searches for users in directory @@ -231,7 +270,7 @@ class UserDirectoryHandler(object): unhandled_users = user_ids - self.initially_handled_users yield self.store.add_profiles_to_user_dir( - {user_id: users_with_profile[user_id] for user_id in unhandled_users}, + {user_id: users_with_profile[user_id] for user_id in unhandled_users} ) self.initially_handled_users |= unhandled_users diff --git a/synapse/storage/schema/delta/53/user_share.sql b/synapse/storage/schema/delta/53/user_share.sql index 14424ded0c..5831b1a6f8 100644 --- a/synapse/storage/schema/delta/53/user_share.sql +++ b/synapse/storage/schema/delta/53/user_share.sql @@ -16,9 +16,6 @@ -- Old disused version of the tables below. DROP TABLE IF EXISTS users_who_share_rooms; --- This is no longer used because it's duplicated by the users_who_share_public_rooms -DROP TABLE IF EXISTS users_in_public_rooms; - -- Tables keeping track of what users share rooms. This is a map of local users -- to local or remote users, per room. Remote users cannot be in the user_id -- column, only the other_user_id column. There are two tables, one for public diff --git a/synapse/storage/schema/delta/53/users_in_public_rooms.sql b/synapse/storage/schema/delta/53/users_in_public_rooms.sql new file mode 100644 index 0000000000..bd57fd778b --- /dev/null +++ b/synapse/storage/schema/delta/53/users_in_public_rooms.sql @@ -0,0 +1,28 @@ +/* Copyright 2019 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- We don't need the old version of this table. +DROP TABLE IF EXISTS users_in_public_rooms; + +-- Track what users are in public rooms. +CREATE TABLE IF NOT EXISTS users_in_public_rooms ( + user_id TEXT NOT NULL +); + +CREATE UNIQUE INDEX users_in_public_rooms_u_idx ON users_in_public_rooms(user_id); + +-- Fill the table. +INSERT INTO background_updates (update_name, progress_json) VALUES + ('users_in_public_rooms_initial', '{}'); diff --git a/synapse/storage/user_directory.py b/synapse/storage/user_directory.py index 2317d22ed6..8f40277b50 100644 --- a/synapse/storage/user_directory.py +++ b/synapse/storage/user_directory.py @@ -242,6 +242,9 @@ class UserDirectoryStore(SQLBaseStore): txn, table="user_directory_search", keyvalues={"user_id": user_id} ) self._simple_delete_txn( + txn, table="users_in_public_rooms", keyvalues={"user_id": user_id} + ) + self._simple_delete_txn( txn, table="users_who_share_public_rooms", keyvalues={"user_id": user_id}, @@ -339,6 +342,21 @@ class UserDirectoryStore(SQLBaseStore): value_names=(), value_values=None, ) + + # If it's a public room, also update them in users_in_public_rooms. + # We don't look before they're in the table before we do it, as it's + # more efficient to simply have Postgres do that (one UPSERT vs one + # SELECT and maybe one INSERT). + if not share_private: + for user_id in set([x[1] for x in user_id_tuples]): + self._simple_upsert_txn( + txn, + "users_in_public_rooms", + keyvalues={"user_id": user_id}, + values={}, + desc="add_user_as_in_public_room", + ) + for user_id, other_user_id in user_id_tuples: txn.call_after( self.get_users_who_share_room_from_dir.invalidate, (user_id,) @@ -379,6 +397,21 @@ class UserDirectoryStore(SQLBaseStore): table="users_who_share_public_rooms", keyvalues={"other_user_id": user_id, "room_id": room_id}, ) + + # Are the users still in a public room after we deleted them from this one? + still_in_public = self._simple_select_one_onecol_txn( + txn, + "users_who_share_public_rooms", + keyvalues={"other_user_id": user_id}, + retcol="other_user_id", + allow_none=True, + ) + + if still_in_public is None: + self._simple_delete_txn( + txn, table="users_in_public_rooms", keyvalues={"user_id": user_id} + ) + txn.call_after( self.get_users_who_share_room_from_dir.invalidate, (user_id,) ) @@ -452,6 +485,7 @@ class UserDirectoryStore(SQLBaseStore): def _delete_all_from_user_dir_txn(txn): txn.execute("DELETE FROM user_directory") txn.execute("DELETE FROM user_directory_search") + txn.execute("DELETE FROM users_in_public_rooms") txn.execute("DELETE FROM users_who_share_public_rooms") txn.execute("DELETE FROM users_who_share_private_rooms") txn.call_after(self.get_user_in_directory.invalidate_all) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index a16a2dc67b..0e0ac0a48b 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -121,6 +121,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): self.assertEqual( self._compress_shared(shares_private), set([(u1, u2, room), (u2, u1, room)]) ) + self.assertEqual(set(public_users), set([u1, u2])) # We get one search result when searching for user2 by user1. s = self.get_success(self.handler.search_users(u1, "user2", 10)) @@ -140,9 +141,11 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): # Check we have removed the values. shares_public = self.get_users_who_share_public_rooms() shares_private = self.get_users_who_share_private_rooms() + public_users = self.get_users_in_public_rooms() self.assertEqual(shares_public, []) self.assertEqual(self._compress_shared(shares_private), set()) + self.assertEqual(public_users, [u1]) # User1 now gets no search results for any of the other users. s = self.get_success(self.handler.search_users(u1, "user2", 10)) @@ -160,6 +163,15 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): r.add((i["user_id"], i["other_user_id"], i["room_id"])) return r + def get_users_in_public_rooms(self): + return self.get_success( + self.store._simple_select_list( + "users_in_public_rooms", + None, + ["user_id"], + ) + ) + def get_users_who_share_public_rooms(self): return self.get_success( self.store._simple_select_list( |