From 797b6a63fc5f8cb70d15ca0b98e871a57e712f0c Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 13 Mar 2019 01:17:51 +1100 Subject: [PATCH] fixup --- synapse/server.py | 5 ++ .../schema/delta/53/users_in_public_rooms.sql | 7 --- synapse/storage/user_directory.py | 59 ++++--------------- tests/handlers/test_user_directory.py | 21 +------ 4 files changed, 17 insertions(+), 75 deletions(-) diff --git a/synapse/server.py b/synapse/server.py index c992bbaa5f..b9549dd042 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -226,6 +226,11 @@ class HomeServer(object): logger.info("Finished setting up.") def setup_master(self): + """ + Some handlers have side effects on instantiation (like registering + background updates). This function causes them to be fetched, and + therefore instantiated, to run those side effects. + """ for i in self.REQUIRED_ON_MASTER_STARTUP: getattr(self, "get_" + i)() diff --git a/synapse/storage/schema/delta/53/users_in_public_rooms.sql b/synapse/storage/schema/delta/53/users_in_public_rooms.sql index 40adc98387..f7827ca6d2 100644 --- a/synapse/storage/schema/delta/53/users_in_public_rooms.sql +++ b/synapse/storage/schema/delta/53/users_in_public_rooms.sql @@ -26,10 +26,3 @@ CREATE TABLE IF NOT EXISTS users_in_public_rooms ( ); CREATE UNIQUE INDEX users_in_public_rooms_u_idx ON users_in_public_rooms(user_id, room_id); - --- Track what users are publicly visible -CREATE TABLE IF NOT EXISTS publicly_visible_users ( - user_id TEXT NOT NULL -); - -CREATE UNIQUE INDEX publicly_visible_users_u_idx ON publicly_visible_users(user_id); diff --git a/synapse/storage/user_directory.py b/synapse/storage/user_directory.py index 0e6619222c..8fd4fd50da 100644 --- a/synapse/storage/user_directory.py +++ b/synapse/storage/user_directory.py @@ -240,9 +240,6 @@ class UserDirectoryStore(SQLBaseStore): self._simple_delete_txn( txn, table="user_directory_search", keyvalues={"user_id": user_id} ) - self._simple_delete_txn( - txn, table="publicly_visible_users", keyvalues={"user_id": user_id} - ) self._simple_delete_txn( txn, table="users_in_public_rooms", keyvalues={"user_id": user_id} ) @@ -352,18 +349,6 @@ class UserDirectoryStore(SQLBaseStore): value_values=None, ) - # If it's a public room, also update them in publicly_visible_users. - # 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). - for user_id in user_ids: - self._simple_upsert_txn( - txn, - "publicly_visible_users", - keyvalues={"user_id": user_id}, - values={}, - ) - return self.runInteraction( "add_users_in_public_rooms", _add_users_in_public_rooms_txn ) @@ -395,20 +380,6 @@ class UserDirectoryStore(SQLBaseStore): keyvalues={"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_in_public_rooms", - keyvalues={"user_id": user_id}, - retcol="user_id", - allow_none=True, - ) - - if still_in_public is None: - self._simple_delete_txn( - txn, table="publicly_visible_users", keyvalues={"user_id": user_id} - ) - return self.runInteraction( "remove_user_who_share_room", _remove_user_who_share_room_txn ) @@ -476,7 +447,6 @@ class UserDirectoryStore(SQLBaseStore): 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 publicly_visible_users") txn.execute("DELETE FROM users_who_share_private_rooms") txn.call_after(self.get_user_in_directory.invalidate_all) @@ -583,22 +553,19 @@ class UserDirectoryStore(SQLBaseStore): """ if self.hs.config.user_directory_search_all_users: - # make s.user_id null to keep the ordering algorithm happy - join_clause = """ - CROSS JOIN (SELECT NULL as user_id) AS s - """ join_args = () where_clause = "1=1" else: - join_clause = """ - LEFT JOIN publicly_visible_users AS p USING (user_id) - LEFT JOIN ( - SELECT other_user_id AS user_id FROM users_who_share_private_rooms - WHERE user_id = ? - ) AS s USING (user_id) - """ join_args = (user_id,) - where_clause = "(s.user_id IS NOT NULL OR p.user_id IS NOT NULL)" + where_clause = """ + ( + EXISTS (select 1 from users_in_public_rooms WHERE user_id = t.user_id) + OR EXISTS ( + SELECT 1 FROM users_who_share_private_rooms + WHERE user_id = ? AND other_user_id = t.user_id + ) + ) + """ if isinstance(self.database_engine, PostgresEngine): full_query, exact_query, prefix_query = _parse_query_postgres(search_term) @@ -610,9 +577,8 @@ class UserDirectoryStore(SQLBaseStore): # search: (domain, _, display name, localpart) sql = """ SELECT d.user_id AS user_id, display_name, avatar_url - FROM user_directory_search + FROM user_directory_search as t INNER JOIN user_directory AS d USING (user_id) - %s WHERE %s AND vector @@ to_tsquery('english', ?) @@ -639,7 +605,6 @@ class UserDirectoryStore(SQLBaseStore): avatar_url IS NULL LIMIT ? """ % ( - join_clause, where_clause, ) args = join_args + (full_query, exact_query, prefix_query, limit + 1) @@ -648,9 +613,8 @@ class UserDirectoryStore(SQLBaseStore): sql = """ SELECT d.user_id AS user_id, display_name, avatar_url - FROM user_directory_search + FROM user_directory_search as t INNER JOIN user_directory AS d USING (user_id) - %s WHERE %s AND value MATCH ? @@ -660,7 +624,6 @@ class UserDirectoryStore(SQLBaseStore): avatar_url IS NULL LIMIT ? """ % ( - join_clause, where_clause, ) args = join_args + (search_query, limit + 1) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index d8248def3f..114807efc1 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -116,9 +116,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): # Check we have populated the database correctly. shares_private = self.get_users_who_share_private_rooms() public_users = self.get_users_in_public_rooms() - visible_users = self.get_publicly_visible_users() - self.assertEqual(visible_users, []) self.assertEqual( self._compress_shared(shares_private), set([(u1, u2, room), (u2, u1, room)]) ) @@ -142,9 +140,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): # Check we have removed the values. shares_private = self.get_users_who_share_private_rooms() public_users = self.get_users_in_public_rooms() - visible_users = self.get_publicly_visible_users() - self.assertEqual(visible_users, []) self.assertEqual(self._compress_shared(shares_private), set()) self.assertEqual(public_users, []) @@ -177,15 +173,6 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): retval.append((i["user_id"], i["room_id"])) return retval - def get_publicly_visible_users(self): - return self.get_success( - self.store._simple_select_onecol( - "publicly_visible_users", - None, - "user_id", - ) - ) - def get_users_who_share_private_rooms(self): return self.get_success( self.store._simple_select_list( @@ -219,11 +206,9 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): shares_private = self.get_users_who_share_private_rooms() public_users = self.get_users_in_public_rooms() - visible_users = self.get_publicly_visible_users() # Nothing updated yet self.assertEqual(shares_private, []) - self.assertEqual(visible_users, []) self.assertEqual(public_users, []) # Reset the handled users caches @@ -241,9 +226,8 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): shares_private = self.get_users_who_share_private_rooms() public_users = self.get_users_in_public_rooms() - visible_users = self.get_publicly_visible_users() - # User 1 and User 2 share public rooms + # User 1 and User 2 are in the same public room self.assertEqual( set(public_users), set([(u1, room), (u2, room)]) ) @@ -254,9 +238,6 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): set([(u1, u3, private_room), (u3, u1, private_room)]), ) - # User 1 and 2 are in public rooms - self.assertEqual(set(visible_users), set([u1, u2])) - def test_search_all_users(self): """ Search all users = True means that a user does not have to share a