-
Notifications
You must be signed in to change notification settings - Fork 364
Add plain-text handling for rich-text topics as per MSC3765 #18195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Johannes Marbach <[email protected]>
4bdb12d
to
4dbca25
Compare
@@ -757,6 +757,59 @@ def test_post_room_initial_state(self) -> None: | |||
assert channel.resource_usage is not None | |||
self.assertEqual(37, channel.resource_usage.db_txn_count) | |||
|
|||
def test_post_room_topic(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would benefit more if we converted this to a set of Complement tests that any homeserver implementation could test against.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I've opened matrix-org/complement#788 for this. Do you want me to remove the tests here in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can leave them here ⏩
All comments except for the Complement tests addressed. I haven't used pydantic before so this part is most probably utter crap. 🙈 Pydantic v2 has an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go. Just want to have you look at my changes before shipping to ensure I'm not overlooking something.
Thanks and sorry you had to patch my mess up yourself. |
@@ -757,6 +757,59 @@ def test_post_room_initial_state(self) -> None: | |||
assert channel.resource_usage is not None | |||
self.assertEqual(37, channel.resource_usage.db_txn_count) | |||
|
|||
def test_post_room_topic(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can leave them here ⏩
Thanks for the contribution and being willing to jump to robust Pydantic parsing @Johennes 🐏 |
Tested on NetBSD 9 amd64 (federation, multiple clients) # Synapse 1.135.0 (2025-08-01) ## Features - Add `recaptcha_private_key_path` and `recaptcha_public_key_path` config option. ([\#17984](element-hq/synapse#17984), [\#18684](element-hq/synapse#18684)) - Add plain-text handling for rich-text topics as per [MSC3765](matrix-org/matrix-spec-proposals#3765). ([\#18195](element-hq/synapse#18195)) - If enabled by the user, server admins will see [soft failed](https://spec.matrix.org/v1.13/server-server-api/#soft-failure) events over the Client-Server API. ([\#18238](element-hq/synapse#18238)) - Add experimental support for [MSC4277: Harmonizing the reporting endpoints](matrix-org/matrix-spec-proposals#4277). ([\#18263](element-hq/synapse#18263)) - Add ability to limit amount of media uploaded by a user in a given time period. ([\#18527](element-hq/synapse#18527)) - Enable workers to write directly to the device lists stream and handle device list updates, reducing load on the main process. ([\#18581](element-hq/synapse#18581)) - Support arbitrary profile fields. Contributed by @clokep. ([\#18635](element-hq/synapse#18635)) - Advertise support for Matrix v1.12. ([\#18647](element-hq/synapse#18647)) - Add an option to issue redactions as an admin user via the [admin redaction endpoint](https://element-hq.github.io/synapse/latest/admin_api/user_admin_api.html#redact-all-the-events-of-a-user). ([\#18671](element-hq/synapse#18671)) - Add experimental and incomplete support for [MSC4306: Thread Subscriptions](https://github.com/matrix-org/matrix-spec-proposals/blob/rei/msc_thread_subscriptions/proposals/4306-thread-subscriptions.md). ([\#18674](element-hq/synapse#18674)) - Include `event_id` when getting state with `?format=event`. Contributed by @tulir @ Beeper. ([\#18675](element-hq/synapse#18675))
This implements matrix-org/matrix-spec-proposals#3765 which is already merged and, therefore, can use stable identifiers.
For
/publicRooms
and/hierarchy
, the topic is read from the eponymous field of thecurrent_state_events
table. Rather than introduce further columns in this table, I changed the insertion / update logic to write the plain-text topic from the rich topic into the existing field. This will not take effect for existing rooms unless their topic is changed. However, existing rooms shouldn't have rich topics to begin with.Similarly, for server-side search, I changed the insertion logic of the
event_search
table to prefer the value from the rich topic. Again, existing events shouldn't have rich topics and, therefore, don't need to be migrated in the table.Spec doc: https://spec.matrix.org/v1.15/client-server-api/#mroomtopic
Part of supporting Matrix v1.15: https://spec.matrix.org/v1.15/client-server-api/#mroomtopic
Pull Request Checklist
EventStore
toEventWorkerStore
.".code blocks
.(run the linters)