fixup

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
@@ -241,9 +241,6 @@ class UserDirectoryStore(SQLBaseStore):
                 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}
             )
             self._simple_delete_txn(
@@ -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