-
Notifications
You must be signed in to change notification settings - Fork 512
Experimental support for MSC4429: Profile Updates for Legacy Sync #19556
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
base: develop
Are you sure you want to change the base?
Changes from all commits
632c83e
d0b616d
63cfbe7
e45045d
d9adfd8
30e8737
cfb7034
5b4d505
86572c6
3221271
f27ca70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Implement experimental support for [MSC4429: Profile Updates for Legacy Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4429). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,34 @@ each upgrade are complete before moving on to the next upgrade, to avoid | |
| stacking them up. You can monitor the currently running background updates with | ||
| [the Admin API](usage/administration/admin_api/background_updates.html#status). | ||
|
|
||
| # Upgrading to v1.151.0 | ||
|
|
||
| ## Profile Updates Stream Writer Workers | ||
|
|
||
| This version of Synapse adds a new `profile_updates` writer stream. The | ||
| following endpoints may now only be handled by either the main process, or a | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this wording is confusing; if the main process isn't designated as a stream writer anymore then you can't use the main process for that. The current wording implies |
||
| worker that is designed a "profile_updates" writer. If you are already routing | ||
| the following endpoints to a worker: | ||
|
|
||
| ``` | ||
| /_matrix/client/(api/v1|r0|v3)/profile/<user_id>/(<field_name>?) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't ideal, as it breaks backwards compat for people. I reckon you'll see quite a lot of breakage if we require this. What we do elsewhere is to route requests that go to the wrong worker to the right worker internally via http replication endpoint. |
||
| /_matrix/client/unstable/uk.tcpip.msc4133/profile/<user_id>/(<field_name>?) | ||
| ``` | ||
|
|
||
| those worker(s) need to be marked as a stream writer for the `profile_updates` | ||
| stream in the shared config, using the | ||
| [`stream_writers`](https://element-hq.github.io/synapse/v1.151/usage/configuration/config_documentation.html#stream_writers) | ||
| config option: | ||
|
|
||
| ```yaml | ||
| stream_writers: | ||
| profile_updates: worker1 | ||
| ``` | ||
|
|
||
| as well as included in the | ||
| [`instance_map`](https://element-hq.github.io/synapse/v1.151/usage/configuration/config_documentation.html#instance_map) | ||
| config option. | ||
|
|
||
| # Upgrading to v1.150.0 | ||
|
|
||
| ## Removal of the `systemd` pip extra | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,13 +123,24 @@ | |
| "filter": FILTER_SCHEMA, | ||
| "room_filter": ROOM_FILTER_SCHEMA, | ||
| "room_event_filter": ROOM_EVENT_FILTER_SCHEMA, | ||
| "profile_fields_filter": { | ||
| "type": "object", | ||
| "properties": { | ||
| "ids": {"type": "array", "items": {"type": "string"}}, | ||
| }, | ||
| "additionalProperties": True, | ||
| }, | ||
| }, | ||
| "properties": { | ||
| "presence": {"$ref": "#/definitions/filter"}, | ||
| "account_data": {"$ref": "#/definitions/filter"}, | ||
| "room": {"$ref": "#/definitions/room_filter"}, | ||
| "event_format": {"type": "string", "enum": ["client", "federation"]}, | ||
| "event_fields": {"type": "array", "items": {"type": "string"}}, | ||
| "profile_fields": {"$ref": "#/definitions/profile_fields_filter"}, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fairly sure this shouldn't be included as we should use unstable prefixed |
||
| "org.matrix.msc4429.profile_fields": { | ||
| "$ref": "#/definitions/profile_fields_filter" | ||
| }, | ||
| }, | ||
| "additionalProperties": True, # Allow new fields for forward compatibility | ||
| } | ||
|
|
@@ -217,6 +228,20 @@ def __init__(self, hs: "HomeServer", filter_json: JsonMapping): | |
| self.event_fields = filter_json.get("event_fields", []) | ||
| self.event_format = filter_json.get("event_format", "client") | ||
|
|
||
| self.profile_fields: list[str] = [] | ||
| if hs.config.experimental.msc4429_enabled: | ||
| profile_fields_filter = filter_json.get("profile_fields") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto here; should use unstable prefix |
||
| if profile_fields_filter is None: | ||
| profile_fields_filter = filter_json.get( | ||
| "org.matrix.msc4429.profile_fields" | ||
| ) | ||
|
|
||
| if isinstance(profile_fields_filter, Mapping): | ||
| ids = profile_fields_filter.get("ids", []) | ||
| if ids is None: | ||
| ids = [] | ||
| self.profile_fields = list(ids) | ||
|
|
||
| def __repr__(self) -> str: | ||
| return "<FilterCollection %s>" % (json.dumps(self._filter_json),) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -563,6 +563,9 @@ def read_config( | |
| # MSC4133: Custom profile fields | ||
| self.msc4133_enabled: bool = experimental.get("msc4133_enabled", False) | ||
|
|
||
| # MSC4429: Profile updates for legacy /sync | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add an Experimental Feature Tracking issue (see Tracked lines later): https://element-hq.github.io/synapse/latest/development/experimental_features.html#tracking-issues-for-experimental-features |
||
| self.msc4429_enabled: bool = experimental.get("msc4429_enabled", False) | ||
|
|
||
| # MSC4143: Matrix RTC Transport using Livekit Backend | ||
| self.msc4143_enabled: bool = experimental.get("msc4143_enabled", False) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ | |
| JsonValue, | ||
| Requester, | ||
| ScheduledTask, | ||
| StreamKeyType, | ||
| TaskStatus, | ||
| UserID, | ||
| create_requester, | ||
|
|
@@ -75,6 +76,8 @@ def __init__(self, hs: "HomeServer"): | |
| self.clock = hs.get_clock() # nb must be called this for @cached | ||
| self.store = hs.get_datastores().main | ||
| self.hs = hs | ||
| self._notifier = hs.get_notifier() | ||
| self._msc4429_enabled = hs.config.experimental.msc4429_enabled | ||
|
|
||
| self.federation = hs.get_federation_client() | ||
| hs.get_federation_registry().register_query_handler( | ||
|
|
@@ -99,6 +102,24 @@ def __init__(self, hs: "HomeServer"): | |
| ) | ||
| self._worker_locks = hs.get_worker_locks_handler() | ||
|
|
||
| async def _notify_profile_update(self, user_id: UserID, stream_id: int) -> None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would benefit from docstring. Not at first glance sure why this is pulled out from |
||
| room_ids = await self.store.get_rooms_for_user(user_id.to_string()) | ||
| if not room_ids: | ||
| return | ||
|
|
||
| self._notifier.on_new_event( | ||
| StreamKeyType.PROFILE_UPDATES, stream_id, rooms=room_ids | ||
| ) | ||
|
|
||
| async def _record_profile_updates( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would benefit from docstring |
||
| self, user_id: UserID, updates: list[tuple[str, JsonValue | None]] | ||
| ) -> None: | ||
| if not self._msc4429_enabled or not updates: | ||
| return | ||
|
|
||
| stream_id = await self.store.add_profile_updates(user_id, updates) | ||
| await self._notify_profile_update(user_id, stream_id) | ||
|
|
||
| async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDict: | ||
| """ | ||
| Get a user's profile as a JSON dictionary. | ||
|
|
@@ -253,6 +274,9 @@ async def set_displayname( | |
| ) | ||
|
|
||
| await self.store.set_profile_displayname(target_user, displayname_to_set) | ||
| await self._record_profile_updates( | ||
| target_user, [(ProfileFields.DISPLAYNAME, displayname_to_set)] | ||
| ) | ||
|
|
||
| profile = await self.store.get_profileinfo(target_user) | ||
|
|
||
|
|
@@ -362,6 +386,9 @@ async def set_avatar_url( | |
| ) | ||
|
|
||
| await self.store.set_profile_avatar_url(target_user, avatar_url_to_set) | ||
| await self._record_profile_updates( | ||
| target_user, [(ProfileFields.AVATAR_URL, avatar_url_to_set)] | ||
| ) | ||
|
|
||
| profile = await self.store.get_profileinfo(target_user) | ||
|
|
||
|
|
@@ -406,6 +433,8 @@ async def delete_profile_upon_deactivation( | |
| # have it. | ||
| raise AuthError(400, "Cannot remove another user's profile") | ||
|
|
||
| profile_updates: list[tuple[str, JsonValue | None]] = [] | ||
| current_profile: ProfileInfo | None = None | ||
| if not by_admin: | ||
| current_profile = await self.store.get_profileinfo(target_user) | ||
| if not self.hs.config.registration.enable_set_displayname: | ||
|
|
@@ -428,7 +457,21 @@ async def delete_profile_upon_deactivation( | |
| Codes.FORBIDDEN, | ||
| ) | ||
|
|
||
| if self._msc4429_enabled: | ||
| if current_profile is None: | ||
| current_profile = await self.store.get_profileinfo(target_user) | ||
|
|
||
| if current_profile.display_name is not None: | ||
| profile_updates.append((ProfileFields.DISPLAYNAME, None)) | ||
| if current_profile.avatar_url is not None: | ||
| profile_updates.append((ProfileFields.AVATAR_URL, None)) | ||
|
|
||
| custom_fields = await self.store.get_profile_fields(target_user) | ||
| for field_name in custom_fields.keys(): | ||
| profile_updates.append((field_name, None)) | ||
|
|
||
| await self.store.delete_profile(target_user) | ||
| await self._record_profile_updates(target_user, profile_updates) | ||
|
|
||
| await self._third_party_rules.on_profile_update( | ||
| target_user.to_string(), | ||
|
|
@@ -582,6 +625,7 @@ async def set_profile_field( | |
| raise AuthError(403, "Cannot set another user's profile") | ||
|
|
||
| await self.store.set_profile_field(target_user, field_name, new_value) | ||
| await self._record_profile_updates(target_user, [(field_name, new_value)]) | ||
|
|
||
| # Custom fields do not propagate into the user directory *or* rooms. | ||
| profile = await self.store.get_profileinfo(target_user) | ||
|
|
@@ -617,6 +661,7 @@ async def delete_profile_field( | |
| raise AuthError(400, "Cannot set another user's profile") | ||
|
|
||
| await self.store.delete_profile_field(target_user, field_name) | ||
| await self._record_profile_updates(target_user, [(field_name, None)]) | ||
|
|
||
| # Custom fields do not propagate into the user directory *or* rooms. | ||
| profile = await self.store.get_profileinfo(target_user) | ||
|
|
||
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.
one thing that isn't very obvious from the outside of this PR is that this is all only for local users, correct? I think it would be good to signpost that better throughout