N + 1: Add column full_user_id to tables profiles and user_filters.

changelog.d/15457.misc

@@ -0,0 +1 @@
+Add column `full_user_id` to tables `profiles` and `user_filters`.

synapse/api/filtering.py

@@ -170,11 +170,9 @@ async def get_user_filter(
         result = await self.store.get_user_filter(user_localpart, filter_id)
         return FilterCollection(self._hs, result)
 
-    def add_user_filter(
-        self, user_localpart: str, user_filter: JsonDict
-    ) -> Awaitable[int]:
+    def add_user_filter(self, user_id: str, user_filter: JsonDict) -> Awaitable[int]:
         self.check_valid_filter(user_filter)
-        return self.store.add_user_filter(user_localpart, user_filter)
+        return self.store.add_user_filter(user_id, user_filter)
 
     # TODO(paul): surely we should probably add a delete_user_filter or
     #   replace_user_filter at some point? There's no REST API specified for

synapse/handlers/profile.py

@@ -179,7 +179,7 @@ async def set_displayname(
             )
 
         await self.store.set_profile_displayname(
-            target_user.localpart, displayname_to_set
+            target_user.to_string(), displayname_to_set
         )
 
         profile = await self.store.get_profileinfo(target_user.localpart)
@@ -273,7 +273,7 @@ async def set_avatar_url(
             )
 
         await self.store.set_profile_avatar_url(
-            target_user.localpart, avatar_url_to_set
+            target_user.to_string(), avatar_url_to_set
         )
 
         profile = await self.store.get_profileinfo(target_user.localpart)

synapse/rest/client/filter.py

@@ -94,7 +94,7 @@ async def on_POST(
         set_timeline_upper_limit(content, self.hs.config.server.filter_timeline_limit)
 
         filter_id = await self.filtering.add_user_filter(
-            user_localpart=target_user.localpart, user_filter=content
+            user_id=target_user.to_string(), user_filter=content
         )
 
         return 200, {"filter_id": str(filter_id)}

synapse/storage/databases/main/filtering.py

@@ -20,7 +20,7 @@
 from synapse.api.errors import Codes, StoreError, SynapseError
 from synapse.storage._base import SQLBaseStore, db_to_json
 from synapse.storage.database import LoggingTransaction
-from synapse.types import JsonDict
+from synapse.types import JsonDict, UserID
 from synapse.util.caches.descriptors import cached
 
 
@@ -46,8 +46,9 @@ async def get_user_filter(
 
         return db_to_json(def_json)
 
-    async def add_user_filter(self, user_localpart: str, user_filter: JsonDict) -> int:
+    async def add_user_filter(self, user_id: str, user_filter: JsonDict) -> int:
         def_json = encode_canonical_json(user_filter)
+        user_localpart = UserID.from_string(user_id).localpart
 
         # Need an atomic transaction to SELECT the maximal ID so far then
         # INSERT a new one
@@ -70,10 +71,10 @@ def _do_txn(txn: LoggingTransaction) -> int:
                 filter_id = max_id + 1
 
             sql = (
-                "INSERT INTO user_filters (user_id, filter_id, filter_json)"
-                "VALUES(?, ?, ?)"
+                "INSERT INTO user_filters (full_user_id, user_id, filter_id, filter_json)"
+                "VALUES(?, ?, ?, ?)"
             )
-            txn.execute(sql, (user_localpart, filter_id, bytearray(def_json)))
+            txn.execute(sql, (user_id, user_localpart, filter_id, bytearray(def_json)))
 
             return filter_id
 

synapse/storage/databases/main/profile.py

@@ -16,6 +16,7 @@
 from synapse.api.errors import StoreError
 from synapse.storage._base import SQLBaseStore
 from synapse.storage.databases.main.roommember import ProfileInfo
+from synapse.types import UserID
 
 
 class ProfileWorkerStore(SQLBaseStore):
@@ -54,28 +55,33 @@ async def get_profile_avatar_url(self, user_localpart: str) -> Optional[str]:
             desc="get_profile_avatar_url",
         )
 
-    async def create_profile(self, user_localpart: str) -> None:
+    async def create_profile(self, user_id: str) -> None:
+        user_localpart = UserID.from_string(user_id).localpart
         await self.db_pool.simple_insert(
-            table="profiles", values={"user_id": user_localpart}, desc="create_profile"
+            table="profiles",
+            values={"user_id": user_localpart, "full_user_id": user_id},
+            desc="create_profile",
         )
 
     async def set_profile_displayname(
-        self, user_localpart: str, new_displayname: Optional[str]
+        self, user_id: str, new_displayname: Optional[str]
     ) -> None:
+        user_localpart = UserID.from_string(user_id).localpart
         await self.db_pool.simple_upsert(
             table="profiles",
             keyvalues={"user_id": user_localpart},
-            values={"displayname": new_displayname},
+            values={"displayname": new_displayname, "full_user_id": user_id},
             desc="set_profile_displayname",
         )
 
     async def set_profile_avatar_url(
-        self, user_localpart: str, new_avatar_url: Optional[str]
+        self, user_id: str, new_avatar_url: Optional[str]
     ) -> None:
+        user_localpart = UserID.from_string(user_id).localpart
         await self.db_pool.simple_upsert(
             table="profiles",
             keyvalues={"user_id": user_localpart},
-            values={"avatar_url": new_avatar_url},
+            values={"avatar_url": new_avatar_url, "full_user_id": user_id},
             desc="set_profile_avatar_url",
         )
 

synapse/storage/databases/main/registration.py

@@ -2414,8 +2414,8 @@ def _register_user(
             # *obviously* the 'profiles' table uses localpart for user_id
             # while everything else uses the full mxid.
             txn.execute(
-                "INSERT INTO profiles(user_id, displayname) VALUES (?,?)",
-                (user_id_obj.localpart, create_profile_with_displayname),
+                "INSERT INTO profiles(full_user_id, user_id, displayname) VALUES (?,?,?)",
+                (user_id, user_id_obj.localpart, create_profile_with_displayname),
             )
 
         if self.hs.config.stats.stats_enabled:

synapse/storage/schema/__init__.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-SCHEMA_VERSION = 74  # remember to update the list below when updating
+SCHEMA_VERSION = 75  # remember to update the list below when updating
 """Represents the expectations made by the codebase about the database schema
 
 This should be incremented whenever the codebase changes its requirements on the
@@ -91,6 +91,9 @@
     - A query on `event_stream_ordering` column has now been disambiguated (i.e. the
       codebase can handle the `current_state_events`, `local_current_memberships` and
       `room_memberships` tables having an `event_stream_ordering` column).
+
+Changes in SCHEMA_VERSION = 75:
+    - Adds a full_user_id column to tables profiles and user_filters
 """
 
 

synapse/storage/schema/main/delta/75/01_add_profiles_full_user_id_column.sql.postgres

@@ -0,0 +1,21 @@
+/* Copyright 2023 The Matrix.org Foundation C.I.C
+ *
+ * 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.
+ */
+
+ALTER TABLE profiles ADD COLUMN full_user_id TEXT;
+
+-- Add a new constraint on the new column, mirroring the `profiles_user_id_key`
+-- constraint.
+ALTER TABLE ONLY profiles
+    ADD CONSTRAINT profiles_full_user_id_key UNIQUE (full_user_id);

synapse/storage/schema/main/delta/75/01_add_profiles_full_user_id_column.sql.sqlite

@@ -0,0 +1,40 @@
+/* Copyright 2023 The Matrix.org Foundation C.I.C
+ *
+ * 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.
+ */
+
+ALTER TABLE profiles ADD COLUMN full_user_id TEXT;
+
+-- Add a new constraint on the new column, mirroring the `user_id` constraint.
+--
+-- SQLite doesn't support modifying constraints on an existing table, so it must be
+-- recreated.
+CREATE TABLE profiles_new(
+    full_user_id TEXT,
+    user_id TEXT NOT NULL,
+    displayname TEXT,
+    avatar_url TEXT,
+    UNIQUE (full_user_id),
+    UNIQUE (user_id)
+);
+
+-- Copy the data.
+INSERT INTO profiles_new (full_user_id, user_id, displayname, avatar_url)
+    SELECT NULL, user_id, displayname, avatar_url
+    FROM profiles;
+
+-- Drop the old table.
+DROP TABLE profiles;
+
+-- Rename the table.
+ALTER TABLE profiles_new RENAME TO profiles;

synapse/storage/schema/main/delta/75/02_add_user_filters_full_user_id_column.sql

@@ -0,0 +1,23 @@
+/* Copyright 2023 The Matrix.org Foundation C.I.C
+ *
+ * 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.
+ */
+
+ALTER TABLE user_filters ADD COLUMN full_user_id TEXT;
+
+-- Add a unique index on the new column, mirroring the `user_filters_unique` unique
+-- index.
+CREATE UNIQUE INDEX full_user_filters_unique ON user_filters (full_user_id, filter_id);
+-- NB: This will lock the table for writes while the index is being built.
+--     There are around 4,000,000 user_filters on matrix.org so we expect this to take
+--     a couple of seconds at most.

tests/api/test_filtering.py

@@ -33,6 +33,8 @@
 from tests import unittest
 from tests.events.test_utils import MockEvent
 
+user_id = "@test_user:test"
+user2_id = "@test_user2:test"
 user_localpart = "test_user"
 
 
@@ -437,7 +439,7 @@ def test_filter_presence_match(self) -> None:
         user_filter_json = {"presence": {"senders": ["@foo:bar"]}}
         filter_id = self.get_success(
             self.datastore.add_user_filter(
-                user_localpart=user_localpart, user_filter=user_filter_json
+                user_id=user_id, user_filter=user_filter_json
             )
         )
         presence_states = [
@@ -467,7 +469,7 @@ def test_filter_presence_no_match(self) -> None:
 
         filter_id = self.get_success(
             self.datastore.add_user_filter(
-                user_localpart=user_localpart + "2", user_filter=user_filter_json
+                user_id=user2_id, user_filter=user_filter_json
             )
         )
         presence_states = [
@@ -495,7 +497,7 @@ def test_filter_room_state_match(self) -> None:
         user_filter_json = {"room": {"state": {"types": ["m.*"]}}}
         filter_id = self.get_success(
             self.datastore.add_user_filter(
-                user_localpart=user_localpart, user_filter=user_filter_json
+                user_id=user_id, user_filter=user_filter_json
             )
         )
         event = MockEvent(sender="@foo:bar", type="m.room.topic", room_id="!foo:bar")
@@ -514,7 +516,7 @@ def test_filter_room_state_no_match(self) -> None:
         user_filter_json = {"room": {"state": {"types": ["m.*"]}}}
         filter_id = self.get_success(
             self.datastore.add_user_filter(
-                user_localpart=user_localpart, user_filter=user_filter_json
+                user_id=user_id, user_filter=user_filter_json
             )
         )
         event = MockEvent(
@@ -598,7 +600,7 @@ def test_add_filter(self) -> None:
 
         filter_id = self.get_success(
             self.filtering.add_user_filter(
-                user_localpart=user_localpart, user_filter=user_filter_json
+                user_id=user_id, user_filter=user_filter_json
             )
         )
 
@@ -619,7 +621,7 @@ def test_get_filter(self) -> None:
 
         filter_id = self.get_success(
             self.datastore.add_user_filter(
-                user_localpart=user_localpart, user_filter=user_filter_json
+                user_id=user_id, user_filter=user_filter_json
             )
         )
 

tests/handlers/test_profile.py

@@ -67,7 +67,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
 
     def test_get_my_name(self) -> None:
         self.get_success(
-            self.store.set_profile_displayname(self.frank.localpart, "Frank")
+            self.store.set_profile_displayname(self.frank.to_string(), "Frank")
         )
 
         displayname = self.get_success(self.handler.get_displayname(self.frank))
@@ -122,7 +122,7 @@ def test_set_my_name_if_disabled(self) -> None:
 
         # Setting displayname for the first time is allowed
         self.get_success(
-            self.store.set_profile_displayname(self.frank.localpart, "Frank")
+            self.store.set_profile_displayname(self.frank.to_string(), "Frank")
         )
 
         self.assertEqual(
@@ -166,8 +166,10 @@ def test_get_other_name(self) -> None:
         )
 
     def test_incoming_fed_query(self) -> None:
-        self.get_success(self.store.create_profile("caroline"))
-        self.get_success(self.store.set_profile_displayname("caroline", "Caroline"))
+        self.get_success(self.store.create_profile("@caroline:test"))
+        self.get_success(
+            self.store.set_profile_displayname("@caroline:test", "Caroline")
+        )
 
         response = self.get_success(
             self.query_handlers["profile"](
@@ -184,7 +186,7 @@ def test_incoming_fed_query(self) -> None:
     def test_get_my_avatar(self) -> None:
         self.get_success(
             self.store.set_profile_avatar_url(
-                self.frank.localpart, "http://my.server/me.png"
+                self.frank.to_string(), "http://my.server/me.png"
             )
         )
         avatar_url = self.get_success(self.handler.get_avatar_url(self.frank))
@@ -238,7 +240,7 @@ def test_set_my_avatar_if_disabled(self) -> None:
         # Setting displayname for the first time is allowed
         self.get_success(
             self.store.set_profile_avatar_url(
-                self.frank.localpart, "http://my.server/me.png"
+                self.frank.to_string(), "http://my.server/me.png"
             )
         )
 

tests/rest/admin/test_user.py

@@ -802,9 +802,9 @@ def test_order_by(self) -> None:
 
         # Set avatar URL to all users, that no user has a NULL value to avoid
         # different sort order between SQlite and PostreSQL
-        self.get_success(self.store.set_profile_avatar_url("user1", "mxc://url3"))
-        self.get_success(self.store.set_profile_avatar_url("user2", "mxc://url2"))
-        self.get_success(self.store.set_profile_avatar_url("admin", "mxc://url1"))
+        self.get_success(self.store.set_profile_avatar_url("@user1:test", "mxc://url3"))
+        self.get_success(self.store.set_profile_avatar_url("@user2:test", "mxc://url2"))
+        self.get_success(self.store.set_profile_avatar_url("@admin:test", "mxc://url1"))
 
         # order by default (name)
         self._order_test([self.admin_user, user1, user2], None)
@@ -1127,7 +1127,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
 
         # set attributes for user
         self.get_success(
-            self.store.set_profile_avatar_url("user", "mxc://servername/mediaid")
+            self.store.set_profile_avatar_url("@user:test", "mxc://servername/mediaid")
         )
         self.get_success(
             self.store.user_add_threepid("@user:test", "email", "[email protected]", 0, 0)
@@ -1257,7 +1257,7 @@ def test_deactivate_user_erase_true_avatar_nonnull_but_empty(self) -> None:
         Reproduces #12257.
         """
         # Patch `self.other_user` to have an empty string as their avatar.
-        self.get_success(self.store.set_profile_avatar_url("user", ""))
+        self.get_success(self.store.set_profile_avatar_url("@user:test", ""))
 
         # Check we can still erase them.
         channel = self.make_request(
@@ -2311,7 +2311,7 @@ def test_deactivate_user(self) -> None:
 
         # set attributes for user
         self.get_success(
-            self.store.set_profile_avatar_url("user", "mxc://servername/mediaid")
+            self.store.set_profile_avatar_url("@user:test", "mxc://servername/mediaid")
         )
         self.get_success(
             self.store.user_add_threepid("@user:test", "email", "[email protected]", 0, 0)