Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

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

Merged
merged 18 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15458.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add column `full_user_id` to tables `profiles` and `user_filters`.
5 changes: 4 additions & 1 deletion synapse/_scripts/synapse_port_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
)
from synapse.notifier import ReplicationNotifier
from synapse.storage.database import DatabasePool, LoggingTransaction, make_conn
from synapse.storage.databases.main import PushRuleStore
from synapse.storage.databases.main import FilteringWorkerStore, PushRuleStore
from synapse.storage.databases.main.account_data import AccountDataWorkerStore
from synapse.storage.databases.main.client_ips import ClientIpBackgroundUpdateStore
from synapse.storage.databases.main.deviceinbox import DeviceInboxBackgroundUpdateStore
Expand All @@ -69,6 +69,7 @@
MediaRepositoryBackgroundUpdateStore,
)
from synapse.storage.databases.main.presence import PresenceBackgroundUpdateStore
from synapse.storage.databases.main.profile import ProfileWorkerStore
from synapse.storage.databases.main.pusher import (
PusherBackgroundUpdatesStore,
PusherWorkerStore,
Expand Down Expand Up @@ -229,6 +230,8 @@ class Store(
EndToEndRoomKeyBackgroundStore,
StatsStore,
AccountDataWorkerStore,
FilteringWorkerStore,
ProfileWorkerStore,
PushRuleStore,
PusherWorkerStore,
PusherBackgroundUpdatesStore,
Expand Down
6 changes: 2 additions & 4 deletions synapse/api/filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: UserID, 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
Expand Down
8 changes: 2 additions & 6 deletions synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ async def set_displayname(
authenticated_entity=requester.authenticated_entity,
)

await self.store.set_profile_displayname(
target_user.localpart, displayname_to_set
)
await self.store.set_profile_displayname(target_user, displayname_to_set)

profile = await self.store.get_profileinfo(target_user.localpart)
await self.user_directory_handler.handle_local_profile_change(
Expand Down Expand Up @@ -272,9 +270,7 @@ async def set_avatar_url(
target_user, authenticated_entity=requester.authenticated_entity
)

await self.store.set_profile_avatar_url(
target_user.localpart, avatar_url_to_set
)
await self.store.set_profile_avatar_url(target_user, avatar_url_to_set)

profile = await self.store.get_profileinfo(target_user.localpart)
await self.user_directory_handler.handle_local_profile_change(
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, user_filter=content
)

return 200, {"filter_id": str(filter_id)}
Expand Down
47 changes: 39 additions & 8 deletions synapse/storage/databases/main/filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,38 @@
from typing import Optional, Tuple, Union, cast

from canonicaljson import encode_canonical_json
from typing_extensions import TYPE_CHECKING

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.storage.database import (
DatabasePool,
LoggingDatabaseConnection,
LoggingTransaction,
)
from synapse.types import JsonDict, UserID
from synapse.util.caches.descriptors import cached

if TYPE_CHECKING:
from synapse.server import HomeServer


class FilteringWorkerStore(SQLBaseStore):
def __init__(
self,
database: DatabasePool,
db_conn: LoggingDatabaseConnection,
hs: "HomeServer",
):
super().__init__(database, db_conn, hs)
self.db_pool.updates.register_background_index_update(
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
"full_users_filters_unique_idx",
index_name="full_users_unique_idx",
table="user_filters",
columns=["full_user_id, filter_id"],
unique=True,
)

@cached(num_args=2)
async def get_user_filter(
self, user_localpart: str, filter_id: Union[int, str]
Expand All @@ -46,7 +69,7 @@ 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: UserID, user_filter: JsonDict) -> int:
def_json = encode_canonical_json(user_filter)

# Need an atomic transaction to SELECT the maximal ID so far then
Expand All @@ -56,24 +79,32 @@ def _do_txn(txn: LoggingTransaction) -> int:
"SELECT filter_id FROM user_filters "
"WHERE user_id = ? AND filter_json = ?"
)
txn.execute(sql, (user_localpart, bytearray(def_json)))
txn.execute(sql, (user_id.localpart, bytearray(def_json)))
filter_id_response = txn.fetchone()
if filter_id_response is not None:
return filter_id_response[0]

sql = "SELECT MAX(filter_id) FROM user_filters WHERE user_id = ?"
txn.execute(sql, (user_localpart,))
txn.execute(sql, (user_id.localpart,))
max_id = cast(Tuple[Optional[int]], txn.fetchone())[0]
if max_id is None:
filter_id = 0
else:
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_id.to_string(),
user_id.localpart,
filter_id,
bytearray(def_json),
),
)
txn.execute(sql, (user_localpart, filter_id, bytearray(def_json)))

return filter_id

Expand Down
42 changes: 35 additions & 7 deletions synapse/storage/databases/main/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,34 @@
# 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.
from typing import Optional
from typing import TYPE_CHECKING, Optional

from synapse.api.errors import StoreError
from synapse.storage._base import SQLBaseStore
from synapse.storage.database import DatabasePool, LoggingDatabaseConnection
from synapse.storage.databases.main.roommember import ProfileInfo
from synapse.types import UserID

if TYPE_CHECKING:
from synapse.server import HomeServer


class ProfileWorkerStore(SQLBaseStore):
def __init__(
self,
database: DatabasePool,
db_conn: LoggingDatabaseConnection,
hs: "HomeServer",
):
super().__init__(database, db_conn, hs)
self.db_pool.updates.register_background_index_update(
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
"profiles_full_user_id_key_idx",
index_name="profiles_full_user_id_key",
table="profiles",
columns=["full_user_id"],
unique=True,
)

async def get_profileinfo(self, user_localpart: str) -> ProfileInfo:
try:
profile = await self.db_pool.simple_select_one(
Expand Down Expand Up @@ -54,28 +74,36 @@ 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: UserID) -> None:
user_localpart = 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.to_string()},
desc="create_profile",
)

async def set_profile_displayname(
self, user_localpart: str, new_displayname: Optional[str]
self, user_id: UserID, new_displayname: Optional[str]
) -> None:
user_localpart = 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.to_string(),
},
desc="set_profile_displayname",
)

async def set_profile_avatar_url(
self, user_localpart: str, new_avatar_url: Optional[str]
self, user_id: UserID, new_avatar_url: Optional[str]
) -> None:
user_localpart = 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.to_string()},
desc="set_profile_avatar_url",
)

Expand Down
4 changes: 2 additions & 2 deletions synapse/storage/databases/main/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion synapse/storage/schema/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

SCHEMA_VERSION = 75 # remember to update the list below when updating
SCHEMA_VERSION = 76 # 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
Expand Down Expand Up @@ -97,6 +97,9 @@
`local_current_membership` & `room_memberships`) is now being populated for new
rows. When the background job to populate historical rows lands this will
become the compat schema version.

Changes in SCHEMA_VERSION = 76:
- Adds a full_user_id column to tables profiles and user_filters.
"""


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* 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;

-- Make sure the column has a unique constraint, mirroring the `profiles_user_id_key`
-- constraint.
INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (7501, 'profiles_full_user_id_key_idx', '{}');
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* 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.
INSERT INTO background_updates (ordering, update_name, progress_json) VALUES (7502, 'full_users_filters_unique_idx', '{}');
16 changes: 9 additions & 7 deletions tests/api/test_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
from synapse.api.filtering import Filter
from synapse.api.presence import UserPresenceState
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.types import JsonDict, UserID
from synapse.util import Clock
from synapse.util.frozenutils import freeze

from tests import unittest
from tests.events.test_utils import MockEvent

user_id = UserID.from_string("@test_user:test")
user2_id = UserID.from_string("@test_user2:test")
user_localpart = "test_user"


Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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")
Expand All @@ -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(
Expand Down Expand Up @@ -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
)
)

Expand All @@ -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
)
)

Expand Down
Loading