Files
synapse/tests/storage/test_profile.py
T
Paul Chobert bd21823818 Fix 500 error when setting a custom profile field for a user with no profile row (#20172)
Follow-up to #20149, which fixed the 500 when *getting* a profile field
for a user with no `profiles` row. The same crash was still reachable
when *setting* one via `PUT /_matrix/client/v3/profile/{userId}/{field}`
(as server admin). This PR splits that case in two:

* **The user exists but has no `profiles` row** (e.g. profile erased
upon deactivation):
* Before: `500 M_UNKNOWN` (`TypeError: cannot unpack non-sequence
NoneType` in the profile size check).
  * After: `200`, the profile row is recreated with the field set.
* **The user does not exist at all**:
  * Before: `500 M_UNKNOWN` (same crash).
* After: `404 M_NOT_FOUND`, without conjuring up an orphan profile row.

Fixing the crash also surfaced a latent SQLite-only bug where a field
set on a freshly created profile row was stored under the wrong key,
making it 404 on `GET` right after a successful `PUT`.



### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
2026-09-07 10:34:30 +02:00

200 lines
6.5 KiB
Python

#
# This file is licensed under the Affero General Public License (AGPL) version 3.
#
# Copyright 2014-2021 The Matrix.org Foundation C.I.C.
# Copyright (C) 2023 New Vector, Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# See the GNU Affero General Public License for more details:
# <https://www.gnu.org/licenses/agpl-3.0.html>.
#
# Originally licensed under the Apache License, Version 2.0:
# <http://www.apache.org/licenses/LICENSE-2.0>.
#
# [This file includes modifications made by New Vector Limited]
#
#
from http import HTTPStatus
from twisted.internet.testing import MemoryReactor
from synapse.api.constants import ProfileFields
from synapse.api.errors import StoreError
from synapse.server import HomeServer
from synapse.storage.database import LoggingTransaction
from synapse.storage.engines import PostgresEngine
from synapse.types import UserID
from synapse.util.clock import Clock
from tests import unittest
class ProfileStoreTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.store = hs.get_datastores().main
self.u_frank = UserID.from_string("@frank:test")
def test_displayname(self) -> None:
self.get_success(self.store.create_profile(self.u_frank))
self.get_success(
self.store.set_profile_field(
user_id=self.u_frank,
field_name=ProfileFields.DISPLAYNAME,
new_value="Frank",
)
)
self.assertEqual(
"Frank",
(self.get_success(self.store.get_profile_displayname(self.u_frank))),
)
# test set to None
self.get_success(
self.store.set_profile_field(
user_id=self.u_frank,
field_name=ProfileFields.DISPLAYNAME,
new_value=None,
)
)
self.assertIsNone(
self.get_success(self.store.get_profile_displayname(self.u_frank))
)
def test_avatar_url(self) -> None:
self.get_success(self.store.create_profile(self.u_frank))
self.get_success(
self.store.set_profile_field(
user_id=self.u_frank,
field_name=ProfileFields.AVATAR_URL,
new_value="http://my.site/here",
)
)
self.assertEqual(
"http://my.site/here",
(self.get_success(self.store.get_profile_avatar_url(self.u_frank))),
)
# test set to None
self.get_success(
self.store.set_profile_field(
user_id=self.u_frank,
field_name=ProfileFields.AVATAR_URL,
new_value=None,
)
)
self.assertIsNone(
self.get_success(self.store.get_profile_avatar_url(self.u_frank))
)
def test_get_profile_field_without_profile(self) -> None:
"""
Getting a custom profile field for a user that has no row in the
`profiles` table at all should raise a 404.
Regression test (we previously would trigger an unhandled exception).
Can happen for users whose profile was erased upon deactivation.
"""
f = self.get_failure(
self.store.get_profile_field(self.u_frank, "org.example.field"),
StoreError,
)
self.assertEqual(f.value.code, HTTPStatus.NOT_FOUND)
def test_set_profile_field_without_profile(self) -> None:
"""
Setting a custom profile field for a user that has no row in the
`profiles` table at all should create the row and store the field.
Regression test (we previously would trigger an unhandled exception in
the profile size check, and then store the field under a wrong key on
SQLite). Can happen for users whose profile was erased upon
deactivation.
"""
self.get_success(
self.store.set_profile_field(
user_id=self.u_frank,
field_name="org.example.field",
new_value="test",
)
)
self.assertEqual(
"test",
self.get_success(
self.store.get_profile_field(self.u_frank, "org.example.field")
),
)
def test_profiles_bg_migration(self) -> None:
"""
Test background job that copies entries from column user_id to full_user_id, adding
the hostname in the process.
"""
updater = self.hs.get_datastores().main.db_pool.updates
# drop the constraint so we can insert nulls in full_user_id to populate the test
if isinstance(self.store.database_engine, PostgresEngine):
def f(txn: LoggingTransaction) -> None:
txn.execute(
"ALTER TABLE profiles DROP CONSTRAINT full_user_id_not_null"
)
self.get_success(self.store.db_pool.runInteraction("", f))
for i in range(70):
self.get_success(
self.store.db_pool.simple_insert(
"profiles",
{"user_id": f"hello{i:02}"},
)
)
# re-add the constraint so that when it's validated it actually exists
if isinstance(self.store.database_engine, PostgresEngine):
def f(txn: LoggingTransaction) -> None:
txn.execute(
"ALTER TABLE profiles ADD CONSTRAINT full_user_id_not_null CHECK (full_user_id IS NOT NULL) NOT VALID"
)
self.get_success(self.store.db_pool.runInteraction("", f))
self.get_success(
self.store.db_pool.simple_insert(
"background_updates",
values={
"update_name": "populate_full_user_id_profiles",
"progress_json": "{}",
},
)
)
self.get_success(
updater.run_background_updates(False),
)
expected_values = []
for i in range(70):
expected_values.append((f"@hello{i:02}:{self.hs.hostname}",))
res = self.get_success(
self.store.db_pool.execute(
"", "SELECT full_user_id from profiles ORDER BY full_user_id"
)
)
self.assertEqual(len(res), len(expected_values))
self.assertEqual(res, expected_values)