diff --git a/changelog.d/19556.feature b/changelog.d/19556.feature new file mode 100644 index 0000000000..bcb6c5c983 --- /dev/null +++ b/changelog.d/19556.feature @@ -0,0 +1,2 @@ +Add optional support for [MSC4429: Profile Updates for Legacy Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4429). +Currently defaults to not enabled, and is limited to local users only for the sync results. \ No newline at end of file diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index e829292aca..64a36522fa 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -15,6 +15,8 @@ enable_registration_without_verification: true bcrypt_rounds: 4 url_preview_enabled: true url_preview_ip_range_blacklist: [] +# MSC4429 Profile updates down legacy /sync +include_profile_updates_in_sync: true ## Registration ## diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 510bc5f8c9..3fb961d6f8 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -336,6 +336,17 @@ Example configuration: include_profile_data_on_invite: false ``` --- +### `include_profile_updates_in_sync` + +*(boolean)* Use this option to include updates of other users' profiles in sync responses, for users who share rooms. +Requires an [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) compatible client, and is currently limited to legacy sync and local users only. +This feature is under development and should be used with caution on busy servers or servers which depend on `limit_profile_requests_to_users_who_share_rooms` for ensuring profile information doesn't leak across rooms. Defaults to `false`. + +Example configuration: +```yaml +include_profile_updates_in_sync: true +``` +--- ### `allow_public_rooms_without_auth` *(boolean)* If set to true, removes the need for authentication to access the server's public rooms directory through the client API, meaning that anyone can query the room directory. Defaults to `false`. diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index d79d12a83a..1c97373c27 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -48,6 +48,7 @@ pub struct AuthConfig { #[derive(FromPyObject, Clone)] pub struct ServerConfig { pub msc4140_enabled: bool, + pub include_profile_updates_in_sync: bool, } #[derive(FromPyObject, Clone)] diff --git a/rust/src/handlers/versions.rs b/rust/src/handlers/versions.rs index 25da9d23fc..5d35b052bd 100644 --- a/rust/src/handlers/versions.rs +++ b/rust/src/handlers/versions.rs @@ -257,6 +257,9 @@ pub struct UnstableFeatureMap { /// MSC4380: Invite blocking #[serde(rename = "org.matrix.msc4380.stable")] msc4380: bool, + /// MSC4429: Profile updates for legacy /sync. + #[serde(rename = "org.matrix.msc4429")] + msc4429: bool, /// MSC4445: Sync timeline order #[serde(rename = "org.matrix.msc4445.initial_sync_timeline_topological_ordering")] msc4445_initial_sync_timeline_topological_ordering: bool, @@ -316,6 +319,7 @@ pub fn synapse_config_to_global_unstable_feature_map( msc4169: config.experimental.msc4169_enabled, msc4354: config.experimental.msc4354_enabled, msc4380: true, + msc4429: config.server.include_profile_updates_in_sync, msc4445_initial_sync_timeline_topological_ordering: true, msc4491_enabled: config.experimental.msc4491_enabled, msc4143_enabled: config.experimental.msc4143_enabled, diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index c2153f3f2a..6e7880a900 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -275,6 +275,21 @@ properties: default: true examples: - false + include_profile_updates_in_sync: + type: boolean + description: >- + Use this option to include updates of other users' profiles in sync responses, + for users who share rooms. + + Requires an [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) + compatible client, and is currently limited to legacy sync and local users only. + + This feature is under development and should be used with caution on busy servers or + servers which depend on `limit_profile_requests_to_users_who_share_rooms` for ensuring + profile information doesn't leak across rooms. + default: false + examples: + - true allow_public_rooms_without_auth: type: boolean description: diff --git a/scripts-dev/check_schema_delta.py b/scripts-dev/check_schema_delta.py index 12ed5d258c..7f2500ec05 100755 --- a/scripts-dev/check_schema_delta.py +++ b/scripts-dev/check_schema_delta.py @@ -14,6 +14,11 @@ import sqlglot.expressions SCHEMA_FILE_REGEX = re.compile(r"^synapse/storage/schema/(.*)/delta/(.*)/(.*)$") +# Keep this in sync with synapse.storage.engines._base. The CI job for this +# script deliberately installs only its lightweight parsing dependencies, so we +# avoid importing Synapse here. +AUTO_INCREMENT_PRIMARY_KEYPLACEHOLDER = "$%AUTO_INCREMENT_PRIMARY_KEY%$" + # The base branch we want to check against. We use the main development branch # on the assumption that is what we are developing against. DEVELOP_BRANCH = "develop" @@ -81,7 +86,7 @@ def main(force_colors: bool) -> None: bad_delta_files = [] changed_delta_files = [] for diff in diffs: - if diff.b_path is None: + if diff.deleted_file or diff.b_path is None: # We don't lint deleted files. continue @@ -196,6 +201,10 @@ def check_schema_delta(delta_files: list[str], force_colors: bool) -> bool: ) return True + delta_contents = _replace_auto_increment_primary_key_placeholder( + delta_contents, sql_lang + ) + statements = sqlglot.parse(delta_contents, read=sql_lang) for statement in statements: @@ -244,5 +253,18 @@ def check_schema_delta(delta_files: list[str], force_colors: bool) -> bool: return success +def _replace_auto_increment_primary_key_placeholder( + delta_contents: str, sql_lang: str +) -> str: + """Replace Synapse's auto-increment PK placeholder with parseable SQL.""" + + if sql_lang == "sqlite": + replacement = "INTEGER PRIMARY KEY AUTOINCREMENT" + else: + replacement = "BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY" + + return delta_contents.replace(AUTO_INCREMENT_PRIMARY_KEYPLACEHOLDER, replacement) + + if __name__ == "__main__": main() diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index cca87d42a9..d8e553f446 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -286,6 +286,7 @@ main() { ./tests/msc4155 ./tests/msc4306 ./tests/msc4222 + ./tests/msc4429 ) # Export the list of test packages as a space-separated environment variable, so other diff --git a/scripts-dev/mypy_synapse_plugin.py b/scripts-dev/mypy_synapse_plugin.py index 7fe4d6cd86..ac1fee7f67 100644 --- a/scripts-dev/mypy_synapse_plugin.py +++ b/scripts-dev/mypy_synapse_plugin.py @@ -45,6 +45,7 @@ from mypy.types import ( AnyType, CallableType, Instance, + LiteralType, NoneType, Options, TupleType, @@ -813,6 +814,10 @@ def is_cacheable( if isinstance(rt, AnyType): return True, ("may be mutable" if verbose else None) + elif isinstance(rt, LiteralType): + # Literal[True] etc + return True, None + elif isinstance(rt, Instance): if ( rt.type.fullname in IMMUTABLE_VALUE_TYPES diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 0b8a289d92..f4f598d27c 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -917,6 +917,10 @@ class Porter: "quarantined_media_id_seq", [("quarantined_media_changes", "stream_id")], ) + await self._setup_sequence( + "profile_updates_sequence", + [("profile_updates", "stream_id")], + ) # Step 3. Get tables. self.progress.set_state("Fetching tables") diff --git a/synapse/api/constants.py b/synapse/api/constants.py index acac057334..041a7f284a 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -410,6 +410,57 @@ class ProfileFields: AVATAR_URL: Final = "avatar_url" +class ProfileUpdateAction(str, enum.Enum): + """ + Enum representing the action of a row in the profile updates stream tables. + The action determines whether a profile field update has occurred, or whether + something else has happened that the sync code should know about, for example + a user joining or leaving a room. + """ + + JOINED_ROOM = "joined_room" + """ + This profile update row action represents a user joining a room. + + When gathering an incremental sync non-lazy response for profile updates, + we always include the full profile of users who have joined a room the syncing + user is a member of, where full profile means all the current profile values the + client asked for, regardless of whether they have changed recently. This ensures + that clients have profiles re-populated for any users who have recently left + shared rooms. + + A scenario example would be as follows: + + * Alice leaves a room with Bob + * Bob's client clears all profile fields from Alice + * Alice joins a room with Bob + * Bob's client does an incremental non-lazy sync + + At the end of the flow Bob should receive all the profile fields the client + is interested in, not just the potential diff, which non-lazy incremental sync + normally includes. This update action currently has no meaning for sync responses + that are not incremental and non-lazy. + """ + LEFT_ROOM = "left_room" + """ + This profile update row action represents a user leaving a room. + + Clients will want to know when they no longer share rooms with a user. This + profile action row allows the sync code to deliver a `null` response for those + profiles, so clients can clear their cache containing the users profile data + they are no longer interested in. + """ + UPDATE = "update" + """ + This profile update row action represents a user updating a profile field. + + Depending on the type of sync (initial/incremental, lazy/non-lazy), either the + diff of profile field updates or all the current profile fields are included + in the sync response. In the latter case the profile update action row signifies + a change, but the client may still get fields that have not changed. + """ + + class StickyEventField(TypedDict): """ Dict content of the `sticky` part of an event. diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 9b47c20437..cbae9133c6 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -123,6 +123,13 @@ USER_FILTER_SCHEMA = { "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"}, @@ -130,6 +137,9 @@ USER_FILTER_SCHEMA = { "room": {"$ref": "#/definitions/room_filter"}, "event_format": {"type": "string", "enum": ["client", "federation"]}, "event_fields": {"type": "array", "items": {"type": "string"}}, + "org.matrix.msc4429.profile_fields": { + "$ref": "#/definitions/profile_fields_filter" + }, }, "additionalProperties": True, # Allow new fields for forward compatibility } @@ -217,6 +227,13 @@ class FilterCollection: self.event_fields = filter_json.get("event_fields", []) self.event_format = filter_json.get("event_format", "client") + self.profile_fields: set[str] = set() + if hs.config.server.include_profile_updates_in_sync: + profile_fields_filter = filter_json.get("org.matrix.msc4429.profile_fields") + + if isinstance(profile_fields_filter, Mapping): + self.profile_fields = set(profile_fields_filter.get("ids", [])) + def __repr__(self) -> str: return "" % (json.dumps(self._filter_json),) diff --git a/synapse/config/server.py b/synapse/config/server.py index f071040c6e..42d43ea7f6 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -585,6 +585,12 @@ class ServerConfig(Config): " 'allow_public_rooms_over_federation' is set." ) + # Whether to support MSC4429 profile updates down legacy /sync + self.include_profile_updates_in_sync = config.get( + "include_profile_updates_in_sync", + False, + ) + # Check if the legacy "restrict_public_rooms_to_local_users" flag is set. This # flag is now obsolete but we need to check it for backward-compatibility. if config.get("restrict_public_rooms_to_local_users", False): diff --git a/synapse/config/workers.py b/synapse/config/workers.py index fb7378bfc8..c92534b799 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -127,9 +127,10 @@ class WriterLocations: """Specifies the instances that write various streams. Attributes: - events: The instances that write to the event, backfill and `sticky_events` streams. - (`sticky_events` is written to during event persistence so must be handled by the - same stream writers.) + events: The instances that write to the `event`, `backfill`, `sticky_events` and + `profile_updates` streams. + (`sticky_events` and `profile_updates` are written to during event + persistence so must be handled by the same stream writers.) typing: The instances that write to the typing stream. Currently can only be a single instance. to_device: The instances that write to the to_device stream. Currently @@ -142,6 +143,8 @@ class WriterLocations: push_rules: The instances that write to the push stream. Currently can only be a single instance. device_lists: The instances that write to the device list stream. + thread_subscriptions: The instances that write to the thread subscriptions + stream. quarantined_media_changes: The instances that write to the quarantined media changes stream. """ @@ -179,7 +182,7 @@ class WriterLocations: converter=_instance_to_list_converter, ) thread_subscriptions: list[str] = attr.ib( - default=["master"], + default=[MAIN_PROCESS_INSTANCE_NAME], converter=_instance_to_list_converter, ) quarantined_media_changes: list[str] = attr.ib( @@ -361,8 +364,7 @@ class WorkerConfig(Config): writers = config.get("stream_writers") or {} self.writers = WriterLocations(**writers) - # Check that the configured writers for events and typing also appears in - # `instance_map`. + # Check that the configured writers also appear in `instance_map`. for stream in ( "events", "typing", @@ -371,6 +373,8 @@ class WorkerConfig(Config): "receipts", "presence", "push_rules", + "device_lists", + "thread_subscriptions", ): instances = _instance_to_list_converter(getattr(self.writers, stream)) for instance in instances: @@ -421,6 +425,11 @@ class WorkerConfig(Config): "Must specify at least one instance to handle `device_lists` messages." ) + if len(self.writers.thread_subscriptions) == 0: + raise ConfigError( + "Must specify at least one instance to handle `thread_subscriptions` messages." + ) + self.events_shard_config = RoutableShardedWorkerHandlingConfig( self.writers.events ) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 9ec00d55ad..34596ade16 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -173,7 +173,9 @@ class DeactivateAccountHandler: # in rooms, but these cases behave like message history, following # https://spec.matrix.org/v1.17/client-server-api/#post_matrixclientv3accountdeactivate await self._profile_handler.delete_profile_upon_deactivation( - user, requester, by_admin + target_user=user, + requester=requester, + by_admin=by_admin, ) logger.info("Marking %s as erased", user_id) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index c3886795b6..5f2cec2366 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -34,6 +34,10 @@ from synapse.api.errors import ( StoreError, SynapseError, ) +from synapse.replication.http.profile import ( + ReplicationProfileDeleteField, + ReplicationProfileSetField, +) from synapse.storage.databases.main.media_repository import LocalMedia, RemoteMedia from synapse.storage.roommember import ProfileInfo from synapse.types import ( @@ -42,6 +46,7 @@ from synapse.types import ( JsonValue, Requester, ScheduledTask, + StreamKeyType, TaskStatus, UserID, create_requester, @@ -75,6 +80,7 @@ class ProfileHandler: 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.federation = hs.get_federation_client() hs.get_federation_registry().register_query_handler( @@ -99,6 +105,17 @@ class ProfileHandler: ) self._worker_locks = hs.get_worker_locks_handler() + # Profile updates stream + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._is_events_writer = ( + hs.get_instance_name() in hs.config.worker.writers.events + ) + self._delete_profile_field_client = ReplicationProfileDeleteField.make_client( + self.hs + ) + self._set_profile_field_client = ReplicationProfileSetField.make_client(self.hs) + self._profile_updates_writer_instance = self.hs.config.worker.writers.events[0] + async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDict: """ Get a user's profile as a JSON dictionary. @@ -191,13 +208,13 @@ class ProfileHandler: async def set_displayname( self, + *, target_user: UserID, requester: Requester, new_displayname: str, - *, by_admin: bool = False, propagate: bool = True, - ) -> None: + ) -> int | None: """Set the displayname of a user Preconditions: @@ -207,12 +224,17 @@ class ProfileHandler: updates into rooms, which could cause rooms to be accidentally joined after the deactivated user has left them. + FIXME: This precondition seems to lack a test. + Args: target_user: the user whose displayname is to be changed. requester: The user attempting to make this change. new_displayname: The displayname to give this user. by_admin: Whether this change was made by an administrator. propagate: Whether this change also applies to the user's membership events. + + Returns: + Stream ID of the profile updates stream row that was just inserted. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -252,7 +274,11 @@ class ProfileHandler: authenticated_entity=requester.authenticated_entity, ) - await self.store.set_profile_displayname(target_user, displayname_to_set) + stream_id = await self.store.set_profile_field( + target_user, + ProfileFields.DISPLAYNAME, + displayname_to_set, + ) profile = await self.store.get_profileinfo(target_user) @@ -267,6 +293,8 @@ class ProfileHandler: if propagate: await self._update_join_states(requester, target_user) + return stream_id + async def get_avatar_url(self, target_user: UserID) -> str | None: """ Fetch a user's avatar URL from their profile. @@ -302,13 +330,13 @@ class ProfileHandler: async def set_avatar_url( self, + *, target_user: UserID, requester: Requester, new_avatar_url: str, - *, by_admin: bool = False, propagate: bool = True, - ) -> None: + ) -> int | None: """Set a new avatar URL for a user. Preconditions: @@ -318,12 +346,17 @@ class ProfileHandler: updates into rooms, which could cause rooms to be accidentally joined after the deactivated user has left them. + FIXME: This precondition seems to lack a test. + Args: target_user: the user whose avatar URL is to be changed. requester: The user attempting to make this change. new_avatar_url: The avatar URL to give this user. by_admin: Whether this change was made by an administrator. propagate: Whether this change also applies to the user's membership events. + + Returns: + Stream ID of the profile updates stream row that was just inserted. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -361,7 +394,11 @@ class ProfileHandler: target_user, authenticated_entity=requester.authenticated_entity ) - await self.store.set_profile_avatar_url(target_user, avatar_url_to_set) + stream_id = await self.store.set_profile_field( + target_user, + ProfileFields.AVATAR_URL, + avatar_url_to_set, + ) profile = await self.store.get_profileinfo(target_user) @@ -376,6 +413,8 @@ class ProfileHandler: if propagate: await self._update_join_states(requester, target_user) + return stream_id + async def delete_profile_upon_deactivation( self, target_user: UserID, @@ -394,6 +433,9 @@ class ProfileHandler: **leave** the room on the user's behalf, so there's no point sending new join events into rooms to propagate the profile deletion. See the `users_pending_deactivation` table and the associated user parter loop. + + Profile update streams are NOT updated in any way; this happens when the + event persister processes the room leave events triggered elsewhere as above. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -406,7 +448,10 @@ class ProfileHandler: # have it. raise AuthError(400, "Cannot remove another user's profile") - await self.store.delete_profile(target_user) + # Record the profile delete + await self.store.delete_profile( + user_id=target_user, + ) await self._third_party_rules.on_profile_update( target_user.to_string(), @@ -415,6 +460,50 @@ class ProfileHandler: deactivation=True, ) + async def dispatch_set_profile_field( + self, + *, + target_user: UserID, + requester: Requester, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + by_admin: bool = False, + propagate: bool = True, + ) -> None: + """ + Dispatch setting a profile field value. This either happens in the same + instance, if configured for profile updates, or via replication in the + right instance. + + Args: + target_user: the user whose profile field is to be changed. + requester: The user attempting to make this change. + field_name: The field name to update. + new_value: New value for the profile field. + by_admin: Whether this change was made by an administrator. + propagate: Whether this change also applies to the user's membership events. + """ + if self._is_events_writer: + await self.set_field( + target_user=target_user, + requester=requester, + field_name=field_name, + new_value=new_value, + by_admin=by_admin, + propagate=propagate, + ) + else: + # Offload to the right worker via http replication + await self._set_profile_field_client( + instance_name=self._profile_updates_writer_instance, + user_id=target_user.to_string(), + requester=requester, + field_name=field_name, + new_value=new_value, + by_admin=by_admin, + propagate=propagate, + ) + @cached() async def check_avatar_size_and_mime_type(self, mxc: str) -> bool: """Check that the size and content type of the avatar at the given MXC URI are @@ -492,7 +581,7 @@ class ProfileHandler: async def get_profile_field( self, target_user: UserID, field_name: str - ) -> JsonValue: + ) -> JsonValue | dict[str, JsonValue]: """ Fetch a user's profile from the database for local users and over federation for remote users. @@ -530,15 +619,68 @@ class ProfileHandler: return result.get(field_name) - async def set_profile_field( + async def set_field( self, + *, target_user: UserID, requester: Requester, field_name: str, - new_value: JsonValue, - *, + new_value: JsonValue | dict[str, JsonValue], by_admin: bool = False, + propagate: bool = True, ) -> None: + """Wrapper function for setting any profile field for a user.""" + if field_name == ProfileFields.DISPLAYNAME: + if not isinstance(new_value, str): + raise SynapseError( + 400, "'displayname' must be a string", errcode=Codes.INVALID_PARAM + ) + stream_id = await self.set_displayname( + target_user=target_user, + requester=requester, + new_displayname=new_value, + by_admin=by_admin, + propagate=propagate, + ) + elif field_name == ProfileFields.AVATAR_URL: + if not isinstance(new_value, str): + raise SynapseError( + 400, "'avatar_url' must be a string", errcode=Codes.INVALID_PARAM + ) + stream_id = await self.set_avatar_url( + target_user=target_user, + requester=requester, + new_avatar_url=new_value, + by_admin=by_admin, + propagate=propagate, + ) + else: + stream_id = await self.set_profile_field( + target_user=target_user, + requester=requester, + field_name=field_name, + new_value=new_value, + by_admin=by_admin, + ) + + if stream_id is not None: + room_ids = await self.store.get_rooms_for_user(target_user.to_string()) + if room_ids: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=room_ids, + ) + + async def set_profile_field( + self, + *, + target_user: UserID, + requester: Requester, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + by_admin: bool = False, + ) -> int | None: """Set a new profile field for a user. Preconditions: @@ -546,6 +688,8 @@ class ProfileHandler: notify modules about the change whilst claiming it is not related to user deactivation. + FIXME: This precondition seems to lack a test. + Args: target_user: the user whose profile is to be changed. requester: The user attempting to make this change. @@ -559,7 +703,11 @@ class ProfileHandler: if not by_admin and target_user != requester.user: raise AuthError(403, "Cannot set another user's profile") - await self.store.set_profile_field(target_user, field_name, new_value) + stream_id = await self.store.set_profile_field( + 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) @@ -567,6 +715,48 @@ class ProfileHandler: target_user.to_string(), profile, by_admin, deactivation=False ) + return stream_id + + async def dispatch_delete_profile_field( + self, + *, + target_user: UserID, + requester: Requester, + field_name: str, + by_admin: bool = False, + ) -> None: + """ + Dispatch deleting a profile field value. This either happens in the same + instance, if configured for profile updates, or via replication in the + right instance. + + To delete a displayname / avatar_uri, use the `dispatch_set_profile_field` + method, using an empty string as the value. + + Args: + target_user: the user whose profile field is to be changed. + requester: The user attempting to make this change. + field_name: The field name to update. + by_admin: Whether this change was made by an administrator. + """ + assert field_name not in (ProfileFields.DISPLAYNAME, ProfileFields.AVATAR_URL) + if self._is_events_writer: + await self.delete_profile_field( + target_user=target_user, + requester=requester, + field_name=field_name, + by_admin=by_admin, + ) + else: + # Offload to the right worker via http replication + await self._delete_profile_field_client( + instance_name=self._profile_updates_writer_instance, + user_id=target_user.to_string(), + requester=requester, + field_name=field_name, + by_admin=by_admin, + ) + async def delete_profile_field( self, target_user: UserID, @@ -582,6 +772,8 @@ class ProfileHandler: notify modules about the change whilst claiming it is not related to user deactivation. + FIXME: This precondition seems to lack a test. + Args: target_user: the user whose profile is to be changed. requester: The user attempting to make this change. @@ -594,7 +786,10 @@ class ProfileHandler: if not by_admin and target_user != requester.user: raise AuthError(400, "Cannot set another user's profile") - await self.store.delete_profile_field(target_user, field_name) + stream_id = await self.store.delete_profile_field( + target_user, + field_name, + ) # Custom fields do not propagate into the user directory *or* rooms. profile = await self.store.get_profileinfo(target_user) @@ -602,6 +797,15 @@ class ProfileHandler: target_user.to_string(), profile, by_admin, deactivation=False ) + if stream_id: + room_ids = await self.store.get_rooms_for_user(target_user.to_string()) + if room_ids: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=room_ids, + ) + async def on_profile_query(self, args: JsonDict) -> JsonDict: """Handles federation profile query requests.""" diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 5152d0b522..4f3a63f8bc 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1536,7 +1536,6 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): prev_member_event_id = prev_state_ids.get( (EventTypes.Member, event.state_key), None ) - if prev_member_event_id: prev_member_event = await self.store.get_event(prev_member_event_id) if prev_member_event.membership == Membership.JOIN: diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index bb5ca329e0..f9d9475711 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -530,10 +530,11 @@ class SsoHandler: user_id, authenticated_entity=user_id, ) - await self._profile_handler.set_displayname( - user_id_obj, - requester, - attributes.display_name, + await self._profile_handler.dispatch_set_profile_field( + target_user=user_id_obj, + requester=requester, + field_name=ProfileFields.DISPLAYNAME, + new_value=attributes.display_name, by_admin=True, ) if attributes.picture: @@ -842,10 +843,11 @@ class SsoHandler: ) # save it as user avatar - await self._profile_handler.set_avatar_url( - uid, - create_requester(uid), - str(avatar_mxc_url), + await self._profile_handler.dispatch_set_profile_field( + target_user=uid, + requester=create_requester(uid), + field_name=ProfileFields.AVATAR_URL, + new_value=str(avatar_mxc_url), ) logger.info("successfully saved the user avatar") diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index a05d6c6e59..943105415a 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -18,8 +18,11 @@ # [This file includes modifications made by New Vector Limited] # # +import hashlib import itertools +import json import logging +import os from typing import ( TYPE_CHECKING, AbstractSet, @@ -37,6 +40,7 @@ from synapse.api.constants import ( EventContentFields, EventTypes, Membership, + ProfileUpdateAction, StickyEvent, ) from synapse.api.filtering import FilterCollection @@ -64,6 +68,7 @@ from synapse.types import ( DeviceListUpdates, JsonDict, JsonMapping, + JsonValue, MultiWriterStreamToken, MutableStateMap, Requester, @@ -104,10 +109,25 @@ non_empty_sync_counter = Counter( # client for no more than 30 minutes. LAZY_LOADED_MEMBERS_CACHE_MAX_AGE = 30 * 60 * 1000 +# Store the cache that tracks which lazy-loaded profile fields have been sent to a given +# client for no more than 30 minutes. +LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE = 30 * 60 * 1000 + # Remember the last 100 members we sent to a client for the purposes of # avoiding redundantly sending the same lazy-loaded members to the client LAZY_LOADED_MEMBERS_CACHE_MAX_SIZE = 100 +# Remember the last 100 profile field updates we sent to a client for the purposes of +# avoiding redundantly sending the same lazy-loaded full profiles to the client +LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_SIZE = 100 + +# The digest size for the lazy loaded profile fields cache. +LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_SIZE = 16 + +# A random key generated on server startup, for the lazy loaded profile fields cache. +# Since this is a per-process cache, we don't care if the key is different per process. +LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_KEY = os.urandom(32) + SyncRequestKey = tuple[Any, ...] @@ -224,6 +244,7 @@ class SyncResult: next_batch: Token for the next sync presence: List of presence events for the user. account_data: List of account_data events for the user. + profile_updates: Map of user_id to profile field updates for that user. joined: JoinedSyncResult for each joined room. invited: InvitedSyncResult for each invited room. knocked: KnockedSyncResult for each knocked on room. @@ -239,6 +260,8 @@ class SyncResult: next_batch: StreamToken presence: list[UserPresenceState] account_data: list[JsonDict] + # user ID -> {profile field -> value | null if unset } + profile_updates: dict[str, dict[str, JsonValue | dict[str, JsonValue]] | None] joined: list[JoinedSyncResult] invited: list[InvitedSyncResult] knocked: list[KnockedSyncResult] @@ -260,6 +283,7 @@ class SyncResult: or self.knocked or self.archived or self.account_data + or self.profile_updates or self.to_device or self.device_lists ) @@ -275,6 +299,7 @@ class SyncResult: next_batch=next_batch, presence=[], account_data=[], + profile_updates={}, joined=[], invited=[], knocked=[], @@ -291,6 +316,7 @@ class SyncHandler: self.server_name = hs.hostname self.hs_config = hs.config self.store = hs.get_datastores().main + self._is_mine_id = hs.is_mine_id self.notifier = hs.get_notifier() self.presence_handler = hs.get_presence_handler() self._relations_handler = hs.get_relations_handler() @@ -329,6 +355,29 @@ class SyncHandler: max_len=0, expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE, ) + # ExpiringCache((User, Device)) + # -> LruCache( + # blake2b(Other User ID + Field Name) -> blake2b(Field value) + # ) + self.lazy_loaded_profile_fields_cache: ExpiringCache[ + tuple[str, str | None], LruCache[bytes, bytes] + ] = ExpiringCache( + cache_name="lazy_loaded_profile_fields_cache", + server_name=self.server_name, + hs=hs, + clock=self.clock, + max_len=0, + expiry_ms=LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE, + ) + """This cache contains fields and values we have sent to clients as profile + updates, for a particular user + device combo. The cache entry is a blake2b hash + of the user + field name, with the value being a blake2b hash of the field value. + If the field value changes for a particular user, the hash will change + and the cache will be missed. + + We don't manually remove entries from this cache, though it may be ignored + in cases where the sync must send the field down to the client. + """ self.rooms_to_exclude_globally = hs.config.server.rooms_to_exclude_from_sync @@ -1023,6 +1072,8 @@ class SyncHandler: def get_lazy_loaded_members_cache( self, cache_key: tuple[str, str | None] ) -> LruCache[str, str]: + # FIXME: This cache may be subject to losing members in the case that + # a sync is interrupted and retried, see https://github.com/element-hq/synapse/issues/19978 cache: LruCache[str, str] | None = self.lazy_loaded_members_cache.get(cache_key) if cache is None: logger.debug("creating LruCache for %r", cache_key) @@ -1036,6 +1087,35 @@ class SyncHandler: logger.debug("found LruCache for %r", cache_key) return cache + def get_lazy_loaded_profile_fields_cache( + self, cache_key: tuple[str, str | None] + ) -> LruCache[bytes, bytes]: + """This cache contains fields and values we have sent to clients as profile + updates, for a particular user + device combo. The cache entry is a blake2b hash + of the user + field name, with the value being a blake2b hash of the field value. + If the field value changes for a particular user, the hash will change + and the cache will be missed. + + We don't manually remove entries from this cache, though it may be ignored + in cases where the sync must send the field down to the client. + """ + # FIXME: This cache may be subject to losing field updates in the case that + # a sync is interrupted and retried, see https://github.com/element-hq/synapse/issues/19978 + cache: LruCache[bytes, bytes] | None = ( + self.lazy_loaded_profile_fields_cache.get(cache_key) + ) + if cache is None: + logger.debug("creating LruCache for %r", cache_key) + cache = LruCache( + max_size=LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_SIZE, + clock=self.clock, + server_name=self.server_name, + ) + self.lazy_loaded_profile_fields_cache[cache_key] = cache + else: + logger.debug("found LruCache for %r", cache_key) + return cache + async def compute_state_delta( self, room_id: str, @@ -1863,10 +1943,18 @@ class SyncHandler: } ) + # Note, this needs to be after we collect `joined`, `invited`, `knocked` and + # `archived` sync results since we want to utilize the work we did to collect + # events in those responses as a basis for which users to include profiles + # for when lazy loading. + if self.hs_config.server.include_profile_updates_in_sync: + await self._generate_sync_entry_for_profile_updates(sync_result_builder) + logger.debug("Sync response calculation complete") return SyncResult( presence=sync_result_builder.presence, account_data=sync_result_builder.account_data, + profile_updates=sync_result_builder.profile_updates, joined=sync_result_builder.joined, invited=sync_result_builder.invited, knocked=sync_result_builder.knocked, @@ -2131,6 +2219,272 @@ class SyncHandler: sync_result_builder.account_data = account_data_for_user + async def _generate_initial_sync_entry_for_profile_updates( + self, + *, + user_id: str, + sync_result_builder: "SyncResultBuilder", + profile_fields: set[str], + include_users: set[str] | None, + ) -> None: + """ + Build an initial sync entry for profile updates and attach it to the + given `sync_result_builder`. + + Note: Currently, only profile updates of local users are generated. + + Args: + user_id: The Matrix ID of the user to generate the sync entry for. + sync_result_builder: + profile_fields: The list of field IDs to filter for. + include_users: List of users profiles to include in the sync response, + for when we have calculated a list of users in our lazy loading + sync and want to only return those. + """ + # Currently, limited to only local profiles, so filter remote servers out + user_ids = await self.store.get_local_users_who_share_room_with_user(user_id) + # Ensure we're in the list even if we don't belong to any rooms + user_ids.add(user_id) + if include_users: + # Filter down to selected included users + user_ids = {user_id for user_id in user_ids if user_id in include_users} + + if not user_ids: + return + + profile_data_by_user = await self.store.get_profile_data_for_users(user_ids) + + # Serialise the profile updates into the sync response format. + profile_updates: dict[ + str, dict[str, JsonValue | dict[str, JsonValue]] | None + ] = {} + for other_user_id in user_ids: + profile_data = profile_data_by_user.get(other_user_id) + if profile_data is None: + # Don't generate anything for users with no profile data + # in initial sync. + continue + + per_user_updates: dict[str, JsonValue | dict[str, JsonValue]] = {} + for field_name in profile_fields: + if field_name in profile_data.keys(): + per_user_updates[field_name] = profile_data[field_name] + + if per_user_updates: + profile_updates[other_user_id] = per_user_updates + + if profile_updates: + sync_result_builder.profile_updates = profile_updates + + async def _generate_sync_entry_for_profile_updates( + self, sync_result_builder: "SyncResultBuilder" + ) -> None: + """ + Build a sync entry for profile updates and attach it to the given + `sync_result_builder`. + + Currently only local profiles updates will be included in the sync response. + + Args: + sync_result_builder: + """ + sync_config = sync_result_builder.sync_config + profile_fields = sync_config.filter_collection.profile_fields + if not profile_fields: + return + + user_id = sync_config.user.to_string() + since_token = sync_result_builder.since_token + now_token = sync_result_builder.now_token + + sync_config = sync_result_builder.sync_config + lazy_load_members = sync_config.filter_collection.lazy_load_members() + include_users = None + if lazy_load_members: + # Collect members from the existing `sync_result_builder` data. + # Ensure we filter out any remove users until we support profile + # updates for federated users. + include_users = set() + # invited + for invited in sync_result_builder.invited: + if self._is_mine_id(invited.invite.sender): + include_users.add(invited.invite.sender) + # joined + for joined in sync_result_builder.joined: + for timeline_event in joined.timeline.events: + if self._is_mine_id(timeline_event.event.sender): + include_users.add(timeline_event.event.sender) + # knocked + for knocked in sync_result_builder.knocked: + if self._is_mine_id(knocked.knock.sender): + include_users.add(knocked.knock.sender) + # archived + for archived in sync_result_builder.archived: + for timeline_event in archived.timeline.events: + if self._is_mine_id(timeline_event.event.sender): + include_users.add(timeline_event.event.sender) + + if since_token is None: + await self._generate_initial_sync_entry_for_profile_updates( + user_id=user_id, + sync_result_builder=sync_result_builder, + profile_fields=profile_fields, + include_users=include_users, + ) + return + + updates = await self.store.get_profile_updates_for_user_and_fields( + from_id=since_token.profile_updates_key, + to_id=now_token.profile_updates_key, + user_id=user_id, + field_names=profile_fields, + ) + + left_room_user_ids = { + update.user_id + for update in updates + if update.action == ProfileUpdateAction.LEFT_ROOM.value + } + joined_room_user_ids = { + update.user_id + for update in updates + if update.action == ProfileUpdateAction.JOINED_ROOM.value + } + users = set() + updated_users = { + update.user_id + for update in updates + if update.action == ProfileUpdateAction.UPDATE.value + } + # Add any users in the timeline, if we collected them due to lazy loading + if include_users: + users.update(include_users) + # Add users with updates + users.update(updated_users) + # Add any newly joined users + users.update(joined_room_user_ids) + + if not users and not left_room_user_ids: + return + + # Serialise the profile updates into the sync response format. + # user ID -> {profile field -> value | null if unset } + profile_updates: dict[ + str, dict[str, JsonValue | dict[str, JsonValue]] | None + ] = {} + + # Process field updates and users who have events in the sync response + if users: + updated_user_fields: dict[str, set[str]] = {} + # Set fields from updates + for update in updates: + if ( + # Skip the update if there is no field update (a joined or left room action), + update.action != ProfileUpdateAction.UPDATE + or update.affected_fields is None + # or if the client isn't interested in any of the fields + or update.affected_fields.isdisjoint(profile_fields) + # or we're not interested in this user. + or update.user_id not in users + ): + continue + updated_user_fields.setdefault(update.user_id, set()).update( + # Add any fields that were affected and that we're interested in + update.affected_fields & profile_fields + ) + + # Note: there's a small race condition here where a profile update may + # occur between fetching `now_token` above and reaching this step. In + # that case, the profile information will be newer than `now_token`. + # This is fine, as users will generally always want the latest profile + # information. However, it does mean that on the next sync, the same + # profile update will come down a second time. + # + # Hopefully clients can just filter these out. + profile_data_by_user = await self.store.get_profile_data_for_users(users) + + # Note, we've already collected field updates above via `updates`, + # outside of events in the timeline when lazy loading. When lazy loading, + # we're already always sending the fields that have changed, regardless + # of the lazy loading cache. + for other_user_id in users: + profile_data = profile_data_by_user.get(other_user_id) + if profile_data is None: + # No profile data for this user, just return a blank dictionary + # in incremental sync, telling the clients to remove all profile + # information for this user. + profile_updates[other_user_id] = None + continue + + per_user_updates: dict[str, JsonValue | dict[str, JsonValue]] = {} + if include_users and other_user_id in include_users: + # Include all the fields the client asked for, as this user + # has events in a lazy loaded sync response, except for + # fields we've recently sent in a previous lazy loaded sync response + fields = set(profile_data.keys()).intersection(profile_fields) + for field_name in fields: + cache_key = ( + sync_config.user.to_string(), + sync_config.device_id, + ) + cache = self.get_lazy_loaded_profile_fields_cache(cache_key) + # Only send this users field if we haven't recently sent it. + # Our cache contains previously set values as pairs of + # blake2b(other_used_id + field_name) -> blake2b(value), + # which ensures if the value changes, we'll miss the cache, + # thus sending the field update to the syncing user. + cache_value = hashlib.blake2b( + f"{other_user_id}-{field_name}".encode("utf8"), + key=LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_KEY, + digest_size=LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_SIZE, + ).digest() + value_hash = hashlib.blake2b( + json.dumps( + [ + profile_data.get(field_name), + ], + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf8"), + key=LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_KEY, + digest_size=LAZY_LOADED_PROFILE_FIELDS_CACHE_DIGEST_SIZE, + ).digest() + if cache.get(cache_value) != value_hash: + per_user_updates[field_name] = profile_data.get(field_name) + # Update our cache to indicate this user/field combo + # has been recently sent. + cache.set( + cache_value, + value_hash, + ) + else: + # Include only the diff, unless the user recently joined, + # then send all the fields the client asked for. + # We don't use a cache here as for non-lazy sync we always + # send changes and/or fields the client asked for, if relevant + # as above joined condition. + fields = ( + profile_fields + if other_user_id in joined_room_user_ids + else set(updated_user_fields.get(other_user_id, [])) + ) + fields = set(profile_data.keys()).intersection(fields) + for field_name in fields: + per_user_updates[field_name] = profile_data[field_name] + + if per_user_updates: + profile_updates[other_user_id] = per_user_updates + + # Process left rooms + if left_room_user_ids: + for other_user_id in left_room_user_ids: + # Return an empty dictionary to the client + profile_updates[other_user_id] = None + + if profile_updates: + sync_result_builder.profile_updates = profile_updates + async def _generate_sync_entry_for_presence( self, sync_result_builder: "SyncResultBuilder", @@ -3147,6 +3501,7 @@ class SyncResultBuilder: # The following mirror the fields in a sync response presence account_data + profile_updates joined invited knocked @@ -3165,6 +3520,9 @@ class SyncResultBuilder: presence: list[UserPresenceState] = attr.Factory(list) account_data: list[JsonDict] = attr.Factory(list) + profile_updates: dict[str, dict[str, JsonValue | dict[str, JsonValue]] | None] = ( + attr.Factory(dict) + ) joined: list[JoinedSyncResult] = attr.Factory(list) invited: list[InvitedSyncResult] = attr.Factory(list) knocked: list[KnockedSyncResult] = attr.Factory(list) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 48963b8d83..1be0e9fd91 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -2042,10 +2042,11 @@ class ModuleApi: deactivation, ) - await self._hs.get_profile_handler().set_displayname( + await self._hs.get_profile_handler().dispatch_set_profile_field( target_user=user_id, requester=requester, - new_displayname=new_displayname, + field_name=ProfileFields.DISPLAYNAME, + new_value=new_displayname, by_admin=True, ) diff --git a/synapse/notifier.py b/synapse/notifier.py index 6a057ac09f..e24d0ef5a2 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -528,6 +528,7 @@ class Notifier: StreamKeyType.UN_PARTIAL_STATED_ROOMS, StreamKeyType.THREAD_SUBSCRIPTIONS, StreamKeyType.STICKY_EVENTS, + StreamKeyType.PROFILE_UPDATES, ], new_token: int, users: Collection[str | UserID] | None = None, diff --git a/synapse/replication/http/__init__.py b/synapse/replication/http/__init__.py index 68cc6ce1fc..d934ef8067 100644 --- a/synapse/replication/http/__init__.py +++ b/synapse/replication/http/__init__.py @@ -30,6 +30,7 @@ from synapse.replication.http import ( login, membership, presence, + profile, push, register, send_events, @@ -59,6 +60,7 @@ class ReplicationRestResource(JsonResource): push.register_servlets(hs, self) state.register_servlets(hs, self) devices.register_servlets(hs, self) + profile.register_servlets(hs, self) # The following can't currently be instantiated on workers. if hs.config.worker.worker_app is None: diff --git a/synapse/replication/http/profile.py b/synapse/replication/http/profile.py new file mode 100644 index 0000000000..dddc36477f --- /dev/null +++ b/synapse/replication/http/profile.py @@ -0,0 +1,150 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 Element Creations, 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: +# . +# + + +import logging +from typing import TYPE_CHECKING + +from twisted.web.server import Request + +from synapse.http.server import HttpServer +from synapse.replication.http._base import ReplicationEndpoint +from synapse.synapse_rust.types import Requester +from synapse.types import JsonDict, JsonValue, UserID, create_requester + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class ReplicationProfileSetField(ReplicationEndpoint): + """Update a profile field for a user. + + The POST looks like: + + POST /_synapse/replication/profile_set_field/ + + { + "requester": "@admin:hs", + "field_name": "displayname", + "new_value": "Alice", + "by_admin": true, + "propagate": false + } + + 200 OK + + {} + """ + + NAME = "profile_set_field" + PATH_ARGS = ("user_id",) + METHOD = "POST" + CACHE = False + + def __init__(self, hs: "HomeServer"): + super().__init__(hs) + + self._profile_handler = hs.get_profile_handler() + + @staticmethod + async def _serialize_payload( # type: ignore[override] + user_id: str, + requester: Requester, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + by_admin: bool, + propagate: bool, + ) -> JsonDict: + return { + "requester": requester.user.to_string(), + "field_name": field_name, + "new_value": new_value, + "by_admin": by_admin, + "propagate": propagate, + } + + async def _handle_request( # type: ignore[override] + self, request: Request, content: JsonDict, user_id: str + ) -> tuple[int, JsonDict]: + await self._profile_handler.set_field( + target_user=UserID.from_string(user_id), + requester=create_requester(content["requester"]), + field_name=content["field_name"], + new_value=content["new_value"], + by_admin=content["by_admin"], + propagate=content["propagate"], + ) + + return (200, {}) + + +class ReplicationProfileDeleteField(ReplicationEndpoint): + """Delete a profile field for a user. + + The POST looks like: + + POST /_synapse/replication/profile_delete_field/ + + { + "requester": "@admin:hs", + "field_name": "displayname", + "by_admin": true + } + + 200 OK + + {} + """ + + NAME = "profile_delete_field" + PATH_ARGS = ("user_id",) + METHOD = "POST" + CACHE = False + + def __init__(self, hs: "HomeServer"): + super().__init__(hs) + + self._profile_handler = hs.get_profile_handler() + + @staticmethod + async def _serialize_payload( # type: ignore[override] + user_id: str, + requester: Requester, + field_name: str, + by_admin: bool, + ) -> JsonDict: + return { + "requester": requester.user.to_string(), + "field_name": field_name, + "by_admin": by_admin, + } + + async def _handle_request( # type: ignore[override] + self, request: Request, content: JsonDict, user_id: str + ) -> tuple[int, JsonDict]: + await self._profile_handler.delete_profile_field( + target_user=UserID.from_string(user_id), + requester=create_requester(content["requester"]), + field_name=content["field_name"], + by_admin=content["by_admin"], + ) + + return (200, {}) + + +def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: + ReplicationProfileSetField(hs).register(http_server) + ReplicationProfileDeleteField(hs).register(http_server) diff --git a/synapse/replication/tcp/client.py b/synapse/replication/tcp/client.py index bc7e46d4c9..c0896b83e7 100644 --- a/synapse/replication/tcp/client.py +++ b/synapse/replication/tcp/client.py @@ -44,6 +44,7 @@ from synapse.replication.tcp.streams import ( UnPartialStatedRoomStream, ) from synapse.replication.tcp.streams._base import ( + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, ) @@ -265,6 +266,23 @@ class ReplicationDataHandler: token, users=[row.user_id for row in rows], ) + elif stream_name == ProfileUpdatesStream.NAME: + updated_user_ids = {row.user_id for row in rows} + if updated_user_ids: + room_ids: set[str] = set() + # Get all the rooms of the updated users, dict of + # User ID -> [Room ID] + users_and_rooms = await self.store.get_rooms_for_users(updated_user_ids) + # Loop through each user's room IDs and add to our set of rooms + for user_room_ids in users_and_rooms.values(): + room_ids.update(user_room_ids) + + if room_ids: + self.notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + token, + rooms=room_ids, + ) elif stream_name == StickyEventsStream.NAME: self.notifier.on_new_event( StreamKeyType.STICKY_EVENTS, diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index ad9fed72dd..1d0586abe0 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -67,6 +67,7 @@ from synapse.replication.tcp.streams import ( ) from synapse.replication.tcp.streams._base import ( DeviceListsStream, + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, ) @@ -218,6 +219,12 @@ class ReplicationCommandHandler: continue + if isinstance(stream, ProfileUpdatesStream): + if hs.get_instance_name() in hs.config.worker.writers.events: + self._streams_to_replicate.append(stream) + + continue + if isinstance(stream, StickyEventsStream): if hs.get_instance_name() in hs.config.worker.writers.events: self._streams_to_replicate.append(stream) diff --git a/synapse/replication/tcp/streams/__init__.py b/synapse/replication/tcp/streams/__init__.py index e41573cf68..e657822da7 100644 --- a/synapse/replication/tcp/streams/__init__.py +++ b/synapse/replication/tcp/streams/__init__.py @@ -37,6 +37,7 @@ from synapse.replication.tcp.streams._base import ( DeviceListsStream, PresenceFederationStream, PresenceStream, + ProfileUpdatesStream, PushersStream, PushRulesStream, QuarantinedMediaStream, @@ -70,6 +71,7 @@ STREAMS_MAP = { ToDeviceStream, FederationStream, AccountDataStream, + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, UnPartialStatedRoomStream, @@ -94,6 +96,7 @@ __all__ = [ "ToDeviceStream", "FederationStream", "AccountDataStream", + "ProfileUpdatesStream", "StickyEventsStream", "ThreadSubscriptionsStream", "UnPartialStatedRoomStream", diff --git a/synapse/replication/tcp/streams/_base.py b/synapse/replication/tcp/streams/_base.py index a73f767add..3d7b0ec3d1 100644 --- a/synapse/replication/tcp/streams/_base.py +++ b/synapse/replication/tcp/streams/_base.py @@ -31,8 +31,9 @@ from typing import ( import attr -from synapse.api.constants import AccountDataTypes +from synapse.api.constants import AccountDataTypes, ProfileUpdateAction from synapse.replication.http.streams import ReplicationGetStreamUpdates +from synapse.types import UserID if TYPE_CHECKING: from synapse.server import HomeServer @@ -765,6 +766,80 @@ class ThreadSubscriptionsStream(_StreamFromIdGen): return rows, rows[-1][0], len(updates) == limit +def _convert_affected_fields( + wire: list[str] | frozenset[str] | None, +) -> frozenset[str] | None: + return ( + frozenset(wire) + if wire is not None and not isinstance(wire, frozenset) + else None + ) + + +@attr.s(slots=True, auto_attribs=True) +class ProfileUpdatesStreamRow: + """Profile update stream row detailing what the profile update changes.""" + + user_id: UserID + """The full user ID with the profile update.""" + action: ProfileUpdateAction + """The action, either 'update' for a field update, 'left_room' if the user left + a room or `joined_room` if the user joined a room, see ProfileUpdateAction enum. + """ + affected_fields: frozenset[str] | None = attr.ib( + # Convert list back to frozenset from wire format + converter=_convert_affected_fields + ) + """Names of the profile fields that were added, updated or removed, see https://spec.matrix.org/unstable/client-server-api/#profiles. + This is None if `action` is not `update`. + """ + + +class ProfileUpdatesStream(_StreamFromIdGen): + """Stream to inform users about profile updates.""" + + # FIXME: See issue https://github.com/element-hq/synapse/issues/19981 + # for concerns around the current implementation of the profile + # updates stream. + + NAME = "profile_updates" + ROW_TYPE = ProfileUpdatesStreamRow + + def __init__(self, hs: "HomeServer"): + self.store = hs.get_datastores().main + super().__init__( + hs.get_instance_name(), + self._update_function, + self.store._profile_updates_id_gen, + ) + + async def _update_function( + self, instance_name: str, from_token: int, to_token: int, limit: int + ) -> StreamUpdateResult: + updates = await self.store.get_updated_profile_updates( + from_id=from_token, to_id=to_token, limit=limit + ) + rows = [ + ( + stream_id, + # These are the args to `ProfileUpdatesStreamRow` + ( + user_id, + action, + # Must convert `field_names` to a list for transport over the wire + # It will be reconstructed as a frozenset on the other end + list(field_names) if field_names is not None else None, + ), + ) + for stream_id, user_id, action, field_names in updates + ] + + if not rows: + return [], to_token, False + + return rows, rows[-1][0], len(updates) == limit + + @attr.s(slots=True, auto_attribs=True) class StickyEventsStreamRow: """Stream to inform workers about changes to sticky events.""" diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 53d1f2d366..ddb4483789 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -28,7 +28,7 @@ from typing import TYPE_CHECKING import attr from pydantic import StrictBool, StrictInt, StrictStr -from synapse.api.constants import Direction +from synapse.api.constants import Direction, ProfileFields from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.servlet import ( RestServlet, @@ -366,8 +366,12 @@ class UserRestServletV2(UserRestServletV2Get): if user: # modify user if "displayname" in body: - await self.profile_handler.set_displayname( - target_user, requester, body["displayname"], by_admin=True + await self.profile_handler.dispatch_set_profile_field( + target_user=target_user, + requester=requester, + field_name=ProfileFields.DISPLAYNAME, + new_value=body["displayname"], + by_admin=True, ) if threepids is not None: @@ -415,8 +419,12 @@ class UserRestServletV2(UserRestServletV2Get): ) if "avatar_url" in body: - await self.profile_handler.set_avatar_url( - target_user, requester, body["avatar_url"], by_admin=True + await self.profile_handler.dispatch_set_profile_field( + target_user=target_user, + requester=requester, + field_name=ProfileFields.AVATAR_URL, + new_value=body["avatar_url"], + by_admin=True, ) if "admin" in body: @@ -523,8 +531,12 @@ class UserRestServletV2(UserRestServletV2Get): ) if "avatar_url" in body and isinstance(body["avatar_url"], str): - await self.profile_handler.set_avatar_url( - target_user, requester, body["avatar_url"], by_admin=True + await self.profile_handler.dispatch_set_profile_field( + target_user=target_user, + requester=requester, + field_name=ProfileFields.AVATAR_URL, + new_value=body["avatar_url"], + by_admin=True, ) user_info_dict = await self.admin_handler.get_user(target_user) diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 360a83f39c..4431aa2b2c 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -146,7 +146,9 @@ class ProfileFieldRestServlet(RestServlet): await self.profile_handler.check_profile_query_allowed(user, requester_user) if field_name == ProfileFields.DISPLAYNAME: - field_value: JsonValue = await self.profile_handler.get_displayname(user) + field_value: ( + JsonValue | dict[str, JsonValue] + ) = await self.profile_handler.get_displayname(user) elif field_name == ProfileFields.AVATAR_URL: field_value = await self.profile_handler.get_avatar_url(user) else: @@ -204,18 +206,14 @@ class ProfileFieldRestServlet(RestServlet): Codes.USER_ACCOUNT_SUSPENDED, ) - if field_name == ProfileFields.DISPLAYNAME: - await self.profile_handler.set_displayname( - user, requester, new_value, by_admin=is_admin, propagate=propagate - ) - elif field_name == ProfileFields.AVATAR_URL: - await self.profile_handler.set_avatar_url( - user, requester, new_value, by_admin=is_admin, propagate=propagate - ) - else: - await self.profile_handler.set_profile_field( - user, requester, field_name, new_value, by_admin=is_admin - ) + await self.profile_handler.dispatch_set_profile_field( + target_user=user, + requester=requester, + field_name=field_name, + new_value=new_value, + by_admin=is_admin, + propagate=propagate, + ) return 200, {} @@ -261,17 +259,21 @@ class ProfileFieldRestServlet(RestServlet): Codes.USER_ACCOUNT_SUSPENDED, ) - if field_name == ProfileFields.DISPLAYNAME: - await self.profile_handler.set_displayname( - user, requester, "", by_admin=is_admin, propagate=propagate - ) - elif field_name == ProfileFields.AVATAR_URL: - await self.profile_handler.set_avatar_url( - user, requester, "", by_admin=is_admin, propagate=propagate + if field_name in (ProfileFields.DISPLAYNAME, ProfileFields.AVATAR_URL): + await self.profile_handler.dispatch_set_profile_field( + target_user=user, + requester=requester, + field_name=field_name, + new_value="", + by_admin=is_admin, + propagate=propagate, ) else: - await self.profile_handler.delete_profile_field( - user, requester, field_name, by_admin=is_admin + await self.profile_handler.dispatch_delete_profile_field( + target_user=user, + requester=requester, + field_name=field_name, + by_admin=is_admin, ) return 200, {} @@ -284,8 +286,9 @@ class UnstableProfileFieldRestServlet(ProfileFieldRestServlet): def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: - # The specific field endpoint *must* appear before the generic profile endpoint. ProfileFieldRestServlet(hs).register(http_server) - ProfileRestServlet(hs).register(http_server) + if hs.config.experimental.msc4133_enabled: UnstableProfileFieldRestServlet(hs).register(http_server) + + ProfileRestServlet(hs).register(http_server) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 962317dedb..4e437c5e98 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -123,6 +123,7 @@ class SyncRestServlet(RestServlet): self._event_serializer = hs.get_event_client_serializer() self._msc2654_enabled = hs.config.experimental.msc2654_enabled self._msc3773_enabled = hs.config.experimental.msc3773_enabled + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync self._json_filter_cache: LruCache[str, bool] = LruCache( max_size=1000, @@ -351,6 +352,15 @@ class SyncRestServlet(RestServlet): if sync_result.to_device: response["to_device"] = {"events": sync_result.to_device} + if self._msc4429_enabled and sync_result.profile_updates: + # FIXME: See issue https://github.com/element-hq/synapse/issues/19981 + # for concerns around the current implementation of the profile + # updates stream. + response["org.matrix.msc4429.users"] = { + user_id: {"profile_updates": updates} + for user_id, updates in sync_result.profile_updates.items() + } + if sync_result.device_lists.changed: response["device_lists"]["changed"] = list(sync_result.device_lists.changed) if sync_result.device_lists.left: diff --git a/synapse/rest/synapse/mas/users.py b/synapse/rest/synapse/mas/users.py index 01db41bcfa..cc5717b103 100644 --- a/synapse/rest/synapse/mas/users.py +++ b/synapse/rest/synapse/mas/users.py @@ -19,6 +19,7 @@ from typing import TYPE_CHECKING, Any, TypedDict from pydantic import StrictBool, StrictStr, model_validator +from synapse.api.constants import ProfileFields from synapse.api.errors import NotFoundError, SynapseError from synapse.http.servlet import ( parse_and_validate_json_object_from_request, @@ -162,18 +163,18 @@ class MasProvisionUserResource(MasBaseResource): ) else: created = False + new_displayname = None if body.unset_displayname: - await self.profile_handler.set_displayname( - target_user=user_id, - requester=requester, - new_displayname="", - by_admin=True, - ) + new_displayname = "" elif body.set_displayname is not None: - await self.profile_handler.set_displayname( + new_displayname = body.set_displayname + + if new_displayname is not None: + await self.profile_handler.dispatch_set_profile_field( target_user=user_id, requester=requester, - new_displayname=body.set_displayname, + field_name=ProfileFields.DISPLAYNAME, + new_value=new_displayname, by_admin=True, ) @@ -221,18 +222,18 @@ class MasProvisionUserResource(MasBaseResource): if body.locked is not None: await self.store.set_user_locked_status(user_id.to_string(), body.locked) + new_avatar_url_value = None if body.unset_avatar_url: - await self.profile_handler.set_avatar_url( - target_user=user_id, - requester=requester, - new_avatar_url="", - by_admin=True, - ) + new_avatar_url_value = "" elif body.set_avatar_url is not None: - await self.profile_handler.set_avatar_url( + new_avatar_url_value = body.set_avatar_url + + if new_avatar_url_value is not None: + await self.profile_handler.dispatch_set_profile_field( target_user=user_id, requester=requester, - new_avatar_url=body.set_avatar_url, + field_name=ProfileFields.AVATAR_URL, + new_value=new_avatar_url_value, by_admin=True, ) @@ -380,10 +381,11 @@ class MasSetDisplayNameResource(MasBaseResource): requester = create_requester(user_id=user_id) - await self.profile_handler.set_displayname( + await self.profile_handler.dispatch_set_profile_field( target_user=requester.user, requester=requester, - new_displayname=body.displayname, + field_name=ProfileFields.DISPLAYNAME, + new_value=body.displayname, by_admin=True, ) @@ -424,10 +426,11 @@ class MasUnsetDisplayNameResource(MasBaseResource): requester = create_requester(user_id=user_id) - await self.profile_handler.set_displayname( + await self.profile_handler.dispatch_set_profile_field( target_user=requester.user, requester=requester, - new_displayname="", + field_name=ProfileFields.DISPLAYNAME, + new_value="", by_admin=True, ) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 8eeea20967..1df7f70b71 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -218,12 +218,18 @@ class SQLBaseStore(metaclass=ABCMeta): self.external_cached_functions[cache_name] = func -def db_to_json(db_content: memoryview | bytes | bytearray | str) -> Any: +def db_to_json( + db_content: memoryview | bytes | bytearray | str | dict[str, Any] | list[Any], +) -> Any: """ Take some data from a database row and return a JSON-decoded object. Args: db_content: The JSON-encoded contents from the database. + Supports TEXT columns, as well as JSON/JSONB columns containing lists or objects. + Note that psycopg will decode JSON/JSONB automatically but SQLite doesn't have + such a data type (and returns the text verbatim), so this function can help + paper over the difference. Returns: The object decoded from JSON. @@ -238,6 +244,13 @@ def db_to_json(db_content: memoryview | bytes | bytearray | str) -> Any: if isinstance(db_content, (bytes, bytearray)): db_content = db_content.decode("utf8") + if isinstance(db_content, (dict, list)): + # psycopg2 has already decoded this JSON or JSONB value + # Maybe we should be splitting this case out to a separate helper where + # we expect JSON/JSONB columns and switch behaviour based on + # the database driver + return db_content + try: return json_decoder.decode(db_content) except Exception: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index d92bbeeae3..8211612e2d 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -42,6 +42,7 @@ from synapse.api.constants import ( EventContentFields, EventTypes, Membership, + ProfileUpdateAction, RelationTypes, ) from synapse.api.errors import PartialStateConflictError @@ -76,6 +77,7 @@ from synapse.types import ( MutableStateMap, StateMap, StrCollection, + UserID, ) from synapse.types.handlers import SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES from synapse.types.state import StateFilter @@ -267,6 +269,7 @@ class PersistEventsStore: self._clock = hs.get_clock() self._instance_name = hs.get_instance_name() self._msc4354_enabled = hs.config.experimental.msc4354_enabled + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync self._ephemeral_messages_enabled = hs.config.server.enable_ephemeral_messages self.is_mine_id = hs.is_mine_id @@ -2118,6 +2121,129 @@ class PersistEventsStore: txn, {m for m in members_to_cache_bust if not self.hs.is_mine_id(m)} ) + if self._msc4429_enabled: + # Handle changes to the profile updates stream. + # We've already done a bunch of work calculating the changes needed + # for the sliding sync tables, so we may as well re-use that information + # here to avoid parsing the state delta again, and handling various + # edge cases. + # FIXME: See issue https://github.com/element-hq/synapse/issues/19981 + # for concerns around the current implementation of the profile + # updates stream. + profile_update_additions = { + c.user_id + for c in sliding_sync_table_changes.to_insert_membership_snapshots + if self.hs.is_mine_id(c.user_id) + # FIXME: Ideally we would filter out JOIN -> JOIN. See note below. + and c.membership == Membership.JOIN + } + profile_update_leaves = { + c.user_id + for c in sliding_sync_table_changes.to_insert_membership_snapshots + if self.hs.is_mine_id(c.user_id) + # Any transition from JOIN to something else counts as a leave here. + # Even the 'invalid' transitions might effectively happen due to + # state resolution. + and c.membership != Membership.JOIN + } | ( + # We also need to consider users that get fully state reset out of the room. + # These should be treated as 'leave' + set(sliding_sync_table_changes.to_delete_membership_snapshots) + ) + + if profile_update_additions: + # Write the profile updates for additions to the room, from either + # a join, knock, invite, etc. + # FIXME this will add rows also when a display name changes due to + # the facts that `sliding_sync_table_changes` contains a JOIN + # membership event in that case. We should aim to filter these + # unnecessary rows out, as we're also generating an UPDATE profile + # update action row for the actual display name change itself. + # See https://github.com/element-hq/synapse/issues/19981 + self.store.record_profile_updates_for_user_joined_room_txn( + txn=txn, + room_id=room_id, + joined_users=profile_update_additions, + ) + if profile_update_leaves: + # Write the profile updates for LEAVE events + for user_id in profile_update_leaves: + self._record_profile_updates_for_user_left_room_txn( + txn=txn, + user_id=UserID.from_string(user_id), + room_id=room_id, + ) + + def _record_profile_updates_for_user_left_room_txn( + self, + txn: LoggingTransaction, + user_id: UserID, + room_id: str, + ) -> None: + """ + Record updates into the profile updates stream for when a user leaves a room. + + If this was the last shared room with a set of users, clear all old rows from + the `profile_updates_per_user` table relating to those users, to avoid exposing + any profile field changes past the point of not being in any common rooms with + the user. + + Currently, updates are only recorded for local users. + + Note, this method lives here in the events store file due to the profile + store not having access to the membership store (which the events store does), + which we need to re-use the `do_users_share_a_room_txn` method there. + + Args: + user_id: The user who left the room. + room_id: The room that was left. + """ + # Get the local members of the room + room_members = self.db_pool.simple_select_onecol_txn( + txn=txn, + table="local_current_membership", + retcol="user_id", + keyvalues={ + "membership": Membership.JOIN, + "room_id": room_id, + }, + ) + # For each user check if we still share rooms + users_sharing_rooms = self.store.do_users_share_a_room_txn( + txn=txn, + user_id=user_id.to_string(), + other_user_ids=set(room_members), + ) + users_no_longer_sharing_rooms = set(room_members) - set( + users_sharing_rooms.keys() + ) + + # First clear the previous rows from the table + user_clause, user_args = make_in_list_sql_clause( + txn.database_engine, + "user_id", + users_no_longer_sharing_rooms, + ) + txn.execute( + f""" + DELETE FROM profile_updates_per_user + WHERE {user_clause} + AND stream_id IN ( + SELECT stream_id FROM profile_updates WHERE user_id = ? + ) + """, + (*user_args, user_id.to_string()), + ) + + # Now record the "left room" action in the stream + self.store.record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.LEFT_ROOM, + field_names=[], + target_users=users_no_longer_sharing_rooms, + ) + @classmethod def _get_relevant_sliding_sync_current_state_event_ids_txn( cls, txn: LoggingTransaction, room_id: str diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 68548434a9..05faad5b26 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -19,13 +19,21 @@ # # import json -from typing import TYPE_CHECKING, cast +from collections.abc import Set +from typing import TYPE_CHECKING, Collection, cast +import attr from canonicaljson import encode_canonical_json -from synapse.api.constants import ProfileFields +from synapse.api.constants import ( + EventTypes, + Membership, + ProfileFields, + ProfileUpdateAction, +) from synapse.api.errors import Codes, StoreError -from synapse.storage._base import SQLBaseStore +from synapse.replication.tcp.streams._base import ProfileUpdatesStream +from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause from synapse.storage.database import ( DatabasePool, LoggingDatabaseConnection, @@ -33,7 +41,9 @@ from synapse.storage.database import ( ) from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.engines import PostgresEngine, Sqlite3Engine +from synapse.storage.util.id_generators import MultiWriterIdGenerator from synapse.types import JsonDict, JsonValue, UserID +from synapse.util.json import json_encoder if TYPE_CHECKING: from synapse.server import HomeServer @@ -43,6 +53,16 @@ if TYPE_CHECKING: MAX_PROFILE_SIZE = 65536 +@attr.s(slots=True, frozen=True, auto_attribs=True) +class ProfileUpdate: + """An update to a user's profile.""" + + stream_id: int + user_id: str + action: str + affected_fields: frozenset[str] | None + + class ProfileWorkerStore(SQLBaseStore): def __init__( self, @@ -52,6 +72,7 @@ class ProfileWorkerStore(SQLBaseStore): ): super().__init__(database, db_conn, hs) self.server_name: str = hs.hostname + self._instance_name: str = hs.get_instance_name() self.database_engine = database.engine self.db_pool.updates.register_background_index_update( "profiles_full_user_id_key_idx", @@ -65,6 +86,22 @@ class ProfileWorkerStore(SQLBaseStore): "populate_full_user_id_profiles", self.populate_full_user_id_profiles ) + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._is_events_writer = self._instance_name in hs.config.worker.writers.events + self._profile_updates_id_gen: MultiWriterIdGenerator = MultiWriterIdGenerator( + db_conn=db_conn, + db=database, + notifier=hs.get_replication_notifier(), + stream_name="profile_updates", + server_name=self.server_name, + instance_name=self._instance_name, + tables=[ + ("profile_updates", "instance_name", "stream_id"), + ], + sequence_name="profile_updates_sequence", + writers=hs.config.worker.writers.events, + ) + async def populate_full_user_id_profiles( self, progress: JsonDict, batch_size: int ) -> int: @@ -76,13 +113,15 @@ class ProfileWorkerStore(SQLBaseStore): lower_bound_id = progress.get("lower_bound_id", "") def _get_last_id(txn: LoggingTransaction) -> str | None: - sql = """ - SELECT user_id FROM profiles - WHERE user_id > ? - ORDER BY user_id - LIMIT 1 OFFSET 1000 - """ - txn.execute(sql, (lower_bound_id,)) + txn.execute( + """ + SELECT user_id FROM profiles + WHERE user_id > ? + ORDER BY user_id + LIMIT 1 OFFSET 1000 + """, + (lower_bound_id,), + ) res = txn.fetchone() if res: upper_bound_id = res[0] @@ -93,21 +132,22 @@ class ProfileWorkerStore(SQLBaseStore): def _process_batch( txn: LoggingTransaction, lower_bound_id: str, upper_bound_id: str ) -> None: - sql = """ - UPDATE profiles - SET full_user_id = '@' || user_id || ? - WHERE ? < user_id AND user_id <= ? AND full_user_id IS NULL - """ - txn.execute(sql, (f":{self.server_name}", lower_bound_id, upper_bound_id)) + txn.execute( + """ + UPDATE profiles + SET full_user_id = '@' || user_id || ? + WHERE ? < user_id AND user_id <= ? AND full_user_id IS NULL + """, + (f":{self.server_name}", lower_bound_id, upper_bound_id), + ) def _final_batch(txn: LoggingTransaction, lower_bound_id: str) -> None: - sql = """ - UPDATE profiles - SET full_user_id = '@' || user_id || ? - WHERE ? < user_id AND full_user_id IS NULL - """ txn.execute( - sql, + """ + UPDATE profiles + SET full_user_id = '@' || user_id || ? + WHERE ? < user_id AND full_user_id IS NULL + """, ( f":{self.server_name}", lower_bound_id, @@ -115,10 +155,11 @@ class ProfileWorkerStore(SQLBaseStore): ) if isinstance(self.database_engine, PostgresEngine): - sql = """ - ALTER TABLE profiles VALIDATE CONSTRAINT full_user_id_not_null - """ - txn.execute(sql) + txn.execute( + """ + ALTER TABLE profiles VALIDATE CONSTRAINT full_user_id_not_null + """, + ) upper_bound_id = await self.db_pool.runInteraction( "populate_full_user_id_profiles", _get_last_id @@ -152,6 +193,13 @@ class ProfileWorkerStore(SQLBaseStore): return 50 + def process_replication_position( + self, stream_name: str, instance_name: str, token: int + ) -> None: + if stream_name == ProfileUpdatesStream.NAME: + self._profile_updates_id_gen.advance(instance_name, token) + super().process_replication_position(stream_name, instance_name, token) + async def get_profileinfo(self, user_id: UserID) -> ProfileInfo: """ Fetch the display name and avatar URL of a user. @@ -210,7 +258,9 @@ class ProfileWorkerStore(SQLBaseStore): desc="get_profile_avatar_url", ) - async def get_profile_field(self, user_id: UserID, field_name: str) -> JsonValue: + async def get_profile_field( + self, user_id: UserID, field_name: str + ) -> JsonValue | dict[str, JsonValue]: """ Get a custom profile field for a user. @@ -222,42 +272,46 @@ class ProfileWorkerStore(SQLBaseStore): The string value if the field exists, otherwise raises 404. """ - def get_profile_field(txn: LoggingTransaction) -> JsonValue: + def get_profile_field( + txn: LoggingTransaction, + ) -> JsonValue | dict[str, JsonValue]: # This will error if field_name has double quotes in it, but that's not # possible due to the grammar. field_path = f'$."{field_name}"' if isinstance(self.database_engine, PostgresEngine): - sql = """ - SELECT JSONB_PATH_EXISTS(fields, ?), JSONB_EXTRACT_PATH(fields, ?) - FROM profiles - WHERE user_id = ? - """ txn.execute( - sql, + """ + SELECT JSONB_PATH_EXISTS(fields, ?), JSONB_EXTRACT_PATH(fields, ?) + FROM profiles + WHERE user_id = ? + """, (field_path, field_name, user_id.localpart), ) # Test exists first since value being None is used for both # missing and a null JSON value. - exists, value = cast(tuple[bool, JsonValue], txn.fetchone()) + exists, value = cast( + tuple[bool, JsonValue | dict[str, JsonValue]], txn.fetchone() + ) if not exists: raise StoreError(404, "No row found") return value else: - sql = """ - SELECT JSON_TYPE(fields, ?), JSON_EXTRACT(fields, ?) - FROM profiles - WHERE user_id = ? - """ txn.execute( - sql, + """ + SELECT JSON_TYPE(fields, ?), JSON_EXTRACT(fields, ?) + FROM profiles + WHERE user_id = ? + """, (field_path, field_path, user_id.localpart), ) # If value_type is None, then the value did not exist. - value_type, value = cast(tuple[str | None, JsonValue], txn.fetchone()) + value_type, value = cast( + tuple[str | None, JsonValue | dict[str, JsonValue]], txn.fetchone() + ) if not value_type: raise StoreError(404, "No row found") # If value_type is object or array, then need to deserialize the JSON. @@ -285,12 +339,321 @@ class ProfileWorkerStore(SQLBaseStore): retcol="fields", desc="get_profile_fields", ) - # The SQLite driver doesn't automatically convert JSON to - # Python objects + # The SQLite driver doesn't have a JSON datatype. if isinstance(self.database_engine, Sqlite3Engine) and result: result = json.loads(result) return result or {} + def get_max_profile_updates_stream_id(self) -> int: + """Get the current maximum stream_id for profile updates.""" + return self._profile_updates_id_gen.get_current_token() + + def get_profile_updates_stream_id_generator(self) -> MultiWriterIdGenerator: + return self._profile_updates_id_gen + + async def get_updated_profile_updates( + self, *, from_id: int, to_id: int, limit: int + ) -> list[tuple[int, str, str, frozenset[str] | None]]: + """Get updates to profile updates between two stream IDs. + + Bounds: from_id < ... <= to_id + + Args: + from_id: The starting stream ID (exclusive) + to_id: The ending stream ID (inclusive) + limit: The maximum number of rows to return + + Returns: + list of tuples representing stream_id, user_id, action and field_name + """ + if from_id >= to_id: + return [] + + def _get_updated_profile_updates_txn( + txn: LoggingTransaction, + ) -> list[tuple[int, str, str, frozenset[str] | None]]: + txn.execute( + """ + SELECT + stream_id, user_id, action, affected_fields + FROM profile_updates + WHERE + ? < stream_id AND stream_id <= ? + ORDER BY stream_id ASC LIMIT ? + """, + (from_id, to_id, limit), + ) + + return [ + ( + stream_id, + user_id, + action, + ( + # affected_fields is a JSON array, turn it to a frozenset[str] + frozenset(db_to_json(affected_fields)) + if affected_fields is not None + else None + ), + ) + for stream_id, user_id, action, affected_fields in txn + ] + + return await self.db_pool.runInteraction( + "get_updated_profile_updates", _get_updated_profile_updates_txn + ) + + async def get_profile_updates_for_fields( + self, + *, + from_id: int, + to_id: int, + field_names: Set[str], + ) -> list[ProfileUpdate]: + """Get profile update markers for the given fields in a stream range. + + Bounds: from_id < ... <= to_id + + Args: + from_id: The starting stream ID (exclusive) + to_id: The ending stream ID (inclusive) + field_names: List of field names to filter against. + + Returns: + list of ProfileUpdates update rows + The `affected_fields` entry in the ProfileUpdates will be filtered. + """ + if from_id >= to_id: + return [] + + if not field_names: + return [] + + def _get_profile_updates_for_fields_txn( + txn: LoggingTransaction, + ) -> list[ProfileUpdate]: + wanted_field_in_elems_clause, wanted_field_in_elems_args = ( + make_in_list_sql_clause( + txn.database_engine, "field_names.value", field_names + ) + ) + + if isinstance(txn.database_engine, PostgresEngine): + # Note that if we had a GIN index on `affected_fields`, this would defeat it. + # If we decide we want one, we should consider using the `?|` operator or its + # clearer-named `jsonb_exists_any` equivalent. + all_field_names_table_expression = ( + "jsonb_array_elements_text(affected_fields) AS field_names(value)" + ) + else: + # json_each is a table-valued function that gives `value` as one of its column names + all_field_names_table_expression = ( + "json_each(affected_fields) AS field_names" + ) + + txn.execute( + f""" + SELECT stream_id, user_id, action, affected_fields + FROM profile_updates + WHERE ? < stream_id AND stream_id <= ? + AND ( + (EXISTS (SELECT 1 FROM {all_field_names_table_expression} WHERE {wanted_field_in_elems_clause})) + OR action != ? + ) + ORDER BY stream_id ASC + """, + ( + from_id, + to_id, + *wanted_field_in_elems_args, + ProfileUpdateAction.UPDATE.value, + ), + ) + rows = cast(list[tuple[int, str, str, str | None]], txn.fetchall()) + + updates: list[ProfileUpdate] = [] + for stream_id, user_id, action, affected_fields_dbjson in rows: + updates.append( + ProfileUpdate( + stream_id=stream_id, + user_id=user_id, + action=action, + affected_fields=( + # Get the field names that were affected by this update + # and intersect with the field names we care about + frozenset(db_to_json(affected_fields_dbjson)) & field_names + ) + if affected_fields_dbjson is not None + else None, + ) + ) + + return updates + + return await self.db_pool.runInteraction( + "get_profile_updates_for_fields", _get_profile_updates_for_fields_txn + ) + + async def get_profile_updates_for_user_and_fields( + self, + *, + from_id: int, + to_id: int, + user_id: str, + field_names: Set[str], + include_users: set[str] | None = None, + ) -> list[ProfileUpdate]: + """Get profile update markers for a user in a stream range. + + The returned profile update rows are restricted to those with a + corresponding `profile_updates_per_user` row for the syncing user. + + Bounds: from_id < ... <= to_id + + Args: + from_id: The starting stream ID (exclusive). + to_id: The ending stream ID (inclusive). + user_id: The full user ID to filter on. + field_names: Set of field names to filter update actions against. + include_users: If given, only include updates for these user IDs. + + Returns: + A list of ProfileUpdates update rows. + """ + if from_id >= to_id: + return [] + + if len(field_names) == 0: + return [] + + if include_users is not None and len(include_users) == 0: + # All updates have been filtered out by lazy-loading. + return [] + + def _get_profile_updates_for_user_and_fields_txn( + txn: LoggingTransaction, + ) -> list[ProfileUpdate]: + wanted_field_in_elems_clause, wanted_field_in_elems_args = ( + make_in_list_sql_clause( + txn.database_engine, "field_names.value", field_names + ) + ) + + if isinstance(txn.database_engine, PostgresEngine): + # Note that if we had a GIN index on `affected_fields`, this would defeat it. + # If we decide we want one, we should consider using the `?|` operator or its + # clearer-named `jsonb_exists_any` equivalent. + all_field_names_table_expression = "jsonb_array_elements_text(pu.affected_fields) AS field_names(value)" + else: + # json_each is a table-valued function that gives `value` as one of its column names + all_field_names_table_expression = ( + "json_each(pu.affected_fields) AS field_names" + ) + + user_clause = "" + user_args: list[str] = [] + if include_users is not None: + # Filter out rows that aren't in `include_users`, if defined. + # This is only relevant when lazy-loading. + user_clause, user_args = make_in_list_sql_clause( + txn.database_engine, "pu.user_id", include_users + ) + user_clause = f"AND {user_clause}" + + # Retrieve profile updates where there's a corresponding row in + # `profile_updates_per_user` within the given `stream_id` bounds + # and the `user_id` and `field_names` match. + txn.execute( + f""" + SELECT pu.stream_id, pu.user_id, pu.action, pu.affected_fields + FROM profile_updates AS pu + INNER JOIN profile_updates_per_user AS puf + ON pu.stream_id = puf.stream_id + WHERE ? < pu.stream_id AND pu.stream_id <= ? + AND puf.user_id = ? + {user_clause} + AND ( + (EXISTS (SELECT 1 FROM {all_field_names_table_expression} WHERE {wanted_field_in_elems_clause})) + OR pu.action != ? + ) + ORDER BY pu.stream_id ASC + """, + ( + from_id, + to_id, + user_id, + *user_args, + *wanted_field_in_elems_args, + ProfileUpdateAction.UPDATE.value, + ), + ) + rows = cast(list[tuple[int, str, str, str | None]], txn.fetchall()) + + updates: list[ProfileUpdate] = [] + for stream_id, updated_user_id, action, affected_fields_dbjson in rows: + updates.append( + ProfileUpdate( + stream_id=stream_id, + user_id=updated_user_id, + action=action, + affected_fields=( + # Get the field names that were affected by this update + # and intersect with the field names we care about + frozenset(db_to_json(affected_fields_dbjson)) & field_names + ) + if affected_fields_dbjson is not None + else None, + ) + ) + + return updates + + return await self.db_pool.runInteraction( + "get_profile_updates_for_user_and_fields", + _get_profile_updates_for_user_and_fields_txn, + ) + + async def get_profile_data_for_users( + self, user_ids: Collection[str] + ) -> dict[str, dict[str, JsonValue | dict[str, JsonValue]]]: + """Fetch displayname/avatar_url/custom fields for a list of users. + + Currently, this returns only local users as the `profiles` table only + tracks local users. + + Args: + user_ids: List of user IDs to filter against. + + Returns: + Dictionary of displayname/avatar_url/custom fields for a list of users. + """ + if not user_ids: + return {} + + rows = await self.db_pool.simple_select_many_batch( + table="profiles", + column="full_user_id", + iterable=user_ids, + retcols=("full_user_id", "displayname", "avatar_url", "fields"), + desc="get_profile_data_for_users", + ) + + results: dict[str, dict[str, JsonValue | dict[str, JsonValue]]] = {} + for full_user_id, displayname, avatar_url, fields in rows: + user_fields = fields or {} + # The SQLite driver doesn't have a JSON datatype. + if isinstance(self.database_engine, Sqlite3Engine) and fields: + user_fields = json.loads(fields) + base_fields = { + ProfileFields.DISPLAYNAME: displayname, + ProfileFields.AVATAR_URL: avatar_url, + } + user_fields.update(base_fields) + + results[full_user_id] = user_fields + + return results + async def create_profile(self, user_id: UserID) -> None: """ Create a blank profile for a user, if one does not already exist. @@ -312,7 +675,7 @@ class ProfileWorkerStore(SQLBaseStore): txn: LoggingTransaction, user_id: UserID, new_field_name: str, - new_value: JsonValue, + new_value: JsonValue | dict[str, JsonValue], ) -> None: # For each entry there are 4 quotes (2 each for key and value), 1 colon, # and 1 comma. @@ -372,103 +735,57 @@ class ProfileWorkerStore(SQLBaseStore): if total_bytes > MAX_PROFILE_SIZE: raise StoreError(400, "Profile too large", Codes.PROFILE_TOO_LARGE) - async def set_profile_displayname( - self, user_id: UserID, new_displayname: str | None - ) -> None: + def _set_profile_field_txn( + self, + txn: LoggingTransaction, + user_id: UserID, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + ) -> int | None: """ - Set the display name of a user. + Wrapper function to set a profile field value and write to the profile + update stream tables in one transaction. Args: - user_id: The user's ID. - new_displayname: The new display name. If this is None, the user's display - name is removed. + txn: The transaction to use + user_id: The user to set the profile field for + field_name: The field to set the value for + new_value: New value for the profile field + + Returns: + The profile updates stream ID that was created in this transaction """ - user_localpart = user_id.localpart + if self._msc4429_enabled: + assert self._is_events_writer - def set_profile_displayname(txn: LoggingTransaction) -> None: - if new_displayname is not None: - self._check_profile_size( - txn, user_id, ProfileFields.DISPLAYNAME, new_displayname - ) + self._check_profile_size(txn, user_id, field_name, new_value) + if field_name in (ProfileFields.DISPLAYNAME, ProfileFields.AVATAR_URL): self.db_pool.simple_upsert_txn( txn, table="profiles", - keyvalues={"user_id": user_localpart}, + keyvalues={"user_id": user_id.localpart}, values={ - "displayname": new_displayname, + field_name: new_value, "full_user_id": user_id.to_string(), }, ) - - await self.db_pool.runInteraction( - "set_profile_displayname", set_profile_displayname - ) - - async def set_profile_avatar_url( - self, user_id: UserID, new_avatar_url: str | None - ) -> None: - """ - Set the avatar of a user. - - Args: - user_id: The user's ID. - new_avatar_url: The new avatar URL. If this is None, the user's avatar is - removed. - """ - user_localpart = user_id.localpart - - def set_profile_avatar_url(txn: LoggingTransaction) -> None: - if new_avatar_url is not None: - self._check_profile_size( - txn, user_id, ProfileFields.AVATAR_URL, new_avatar_url - ) - - self.db_pool.simple_upsert_txn( - txn, - table="profiles", - keyvalues={"user_id": user_localpart}, - values={ - "avatar_url": new_avatar_url, - "full_user_id": user_id.to_string(), - }, - ) - - await self.db_pool.runInteraction( - "set_profile_avatar_url", set_profile_avatar_url - ) - - async def set_profile_field( - self, user_id: UserID, field_name: str, new_value: JsonValue - ) -> None: - """ - Set a custom profile field for a user. - - Args: - user_id: The user's ID. - field_name: The name of the custom profile field. - new_value: The value of the custom profile field. - """ - - # Encode to canonical JSON. - canonical_value = encode_canonical_json(new_value) - - def set_profile_field(txn: LoggingTransaction) -> None: - self._check_profile_size(txn, user_id, field_name, new_value) + else: + # Encode to canonical JSON. + canonical_value = encode_canonical_json(new_value) if isinstance(self.database_engine, PostgresEngine): from psycopg2.extras import Json # Note that the || jsonb operator is not recursive, any duplicate # keys will be taken from the second value. - sql = """ - INSERT INTO profiles (user_id, full_user_id, fields) VALUES (?, ?, JSON_BUILD_OBJECT(?, ?::jsonb)) - ON CONFLICT (user_id) - DO UPDATE SET full_user_id = EXCLUDED.full_user_id, fields = COALESCE(profiles.fields, '{}'::jsonb) || EXCLUDED.fields - """ - txn.execute( - sql, + """ + INSERT INTO profiles + (user_id, full_user_id, fields) VALUES (?, ?, JSON_BUILD_OBJECT(?, ?::jsonb)) + ON CONFLICT (user_id) + DO UPDATE SET full_user_id = EXCLUDED.full_user_id, fields = COALESCE(profiles.fields, '{}'::jsonb) || EXCLUDED.fields + """, ( user_id.localpart, user_id.to_string(), @@ -479,19 +796,18 @@ class ProfileWorkerStore(SQLBaseStore): ), ) else: - # You may be tempted to use json_patch instead of providing the parameters - # twice, but that recursively merges objects instead of replacing. - sql = """ - INSERT INTO profiles (user_id, full_user_id, fields) VALUES (?, ?, JSON_OBJECT(?, JSON(?))) - ON CONFLICT (user_id) - DO UPDATE SET full_user_id = EXCLUDED.full_user_id, fields = JSON_SET(COALESCE(profiles.fields, '{}'), ?, JSON(?)) - """ # This will error if field_name has double quotes in it, but that's not # possible due to the grammar. json_field_name = f'$."{field_name}"' txn.execute( - sql, + # You may be tempted to use json_patch instead of providing the parameters + # twice, but that recursively merges objects instead of replacing. + """ + INSERT INTO profiles (user_id, full_user_id, fields) VALUES (?, ?, JSON_OBJECT(?, JSON(?))) + ON CONFLICT (user_id) + DO UPDATE SET full_user_id = EXCLUDED.full_user_id, fields = JSON_SET(COALESCE(profiles.fields, '{}'), ?, JSON(?)) + """, ( user_id.localpart, user_id.to_string(), @@ -502,9 +818,190 @@ class ProfileWorkerStore(SQLBaseStore): ), ) - await self.db_pool.runInteraction("set_profile_field", set_profile_field) + if not self._msc4429_enabled: + return None - async def delete_profile_field(self, user_id: UserID, field_name: str) -> None: + # Record updates in the profile updates stream + stream_id = self.record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.UPDATE, + field_names=[field_name], + ) + + return stream_id + + def record_profile_updates_for_user_joined_room_txn( + self, *, txn: LoggingTransaction, room_id: str, joined_users: set[str] + ) -> None: + """ + Record profile updates for membership additions to a room. + + Currently, updates are only recorded for local users. + + Args: + txn: The transaction to use. + room_id: The room ID concerned. + joined_users: A list of users who have "joined" the room, which here also + means "invited" or "knocked", as in either case we consider that the + users profile should be pushed to the client, should they need it + already even if the user hasn't actually joined the room. + """ + if not self._msc4429_enabled: + return + + assert self._is_events_writer + + # Ensure we're working with local users only + users = {user_id for user_id in joined_users if self.hs.is_mine_id(user_id)} + + # Record the profile updates for each user + for user_id in users: + self.record_profile_updates_txn( + txn=txn, + user_id=UserID.from_string(user_id), + action=ProfileUpdateAction.JOINED_ROOM, + field_names=None, + user_rooms={room_id}, + ) + + def record_profile_updates_txn( + self, + *, + txn: LoggingTransaction, + user_id: UserID, + action: ProfileUpdateAction, + field_names: Collection[str] | None, + user_rooms: set[str] | None = None, + target_users: set[str] | None = None, + ) -> int | None: + """ + Record updates into the profile updates stream tables. + + Currently, updates are only recorded for local users. + + Args: + txn: Transaction to use + user_id: User ID that made the profile update + action: The profile update action, either `update`, `left_room` or + `joined_room` + field_names: A list of fields that were set, if ProfileUpdateAction.UPDATE + user_rooms: Optionally, a set of rooms that the update concerns. If not + given, a database lookup will be done to fetch all the users rooms. + target_users: Optionally, set of users to create profile update stream rows + for. If not given, a database lookup will be done based on `user_rooms`, + or if that is not set, the result of the rooms lookup. + + Returns: + The latest stream ID created in this transaction + """ + if not self._msc4429_enabled: + return None + + if action == ProfileUpdateAction.UPDATE: + assert field_names + else: + assert not field_names + + if not target_users: + if not user_rooms: + rows = self.db_pool.simple_select_onecol_txn( + txn=txn, + table="current_state_events", + keyvalues={ + "type": EventTypes.Member, + "membership": Membership.JOIN, + "state_key": user_id.to_string(), + }, + retcol="room_id", + ) + user_rooms = set(rows) + + rows = self.db_pool.simple_select_many_txn( + txn=txn, + table="local_current_membership", + column="room_id", + iterable=user_rooms, + retcols=("user_id",), + keyvalues={ + "membership": Membership.JOIN, + }, + ) + target_users = {row[0] for row in rows} + + # Ensure we only write updates for local users + users = {user for user in target_users if self.hs.is_mine_id(user)} + + if action in (ProfileUpdateAction.JOINED_ROOM, ProfileUpdateAction.LEFT_ROOM): + users.discard(user_id.to_string()) + if not users: + # No point writing an update for ourselves, if a membership change and no + # other users interested + return None + elif action == ProfileUpdateAction.UPDATE: + # Always include ourselves when updating field values + users.add(user_id.to_string()) + + # Record the profile update + inserted_ts = self.clock.time_msec() + stream_id = self._profile_updates_id_gen.get_next_txn(txn) + + self.db_pool.simple_insert_txn( + txn, + table="profile_updates", + values={ + "stream_id": stream_id, + "instance_name": self._instance_name, + "user_id": user_id.to_string(), + "action": action.value, + "affected_fields": json_encoder.encode(sorted(field_names)) + if field_names + else None, + "inserted_ts": inserted_ts, + }, + ) + + # Add per user tracking rows for each generated stream ID + per_user_values = [(stream_id, user_id, inserted_ts) for user_id in users] + self.db_pool.simple_insert_many_txn( + txn, + table="profile_updates_per_user", + keys=[ + "stream_id", + "user_id", + "inserted_ts", + ], + values=per_user_values, + ) + return stream_id + + async def set_profile_field( + self, + user_id: UserID, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + ) -> int | None: + """ + Set a custom profile field for a user. + + Args: + user_id: The user's ID. + field_name: The name of the custom profile field. + new_value: The value of the custom profile field. + """ + return await self.db_pool.runInteraction( + "set_profile_field", + self._set_profile_field_txn, + user_id, + field_name, + new_value, + ) + + async def delete_profile_field( + self, + user_id: UserID, + field_name: str, + ) -> int | None: """ Remove a custom profile field for a user. @@ -513,39 +1010,68 @@ class ProfileWorkerStore(SQLBaseStore): field_name: The name of the custom profile field. """ - def delete_profile_field(txn: LoggingTransaction) -> None: + if self._msc4429_enabled: + assert self._is_events_writer + + def delete_profile_field(txn: LoggingTransaction) -> int | None: if isinstance(self.database_engine, PostgresEngine): - sql = """ - UPDATE profiles SET fields = fields - ? - WHERE user_id = ? - """ txn.execute( - sql, + """ + UPDATE profiles SET fields = fields - ? + WHERE user_id = ? + """, (field_name, user_id.localpart), ) else: - sql = """ - UPDATE profiles SET fields = json_remove(fields, ?) - WHERE user_id = ? - """ txn.execute( - sql, + """ + UPDATE profiles SET fields = json_remove(fields, ?) + WHERE user_id = ? + """, # This will error if field_name has double quotes in it. (f'$."{field_name}"', user_id.localpart), ) - await self.db_pool.runInteraction("delete_profile_field", delete_profile_field) + if not self._msc4429_enabled: + return None - async def delete_profile(self, user_id: UserID) -> None: + stream_id = self.record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.UPDATE, + field_names=[field_name], + ) + return stream_id + + return await self.db_pool.runInteraction( + "delete_profile_field", delete_profile_field + ) + + async def delete_profile( + self, + user_id: UserID, + ) -> None: """ - Deletes an entire user profile, including displayname, avatar_url and all custom fields. - Used at user deactivation when erasure is requested. + Deletes an entire user profile, including displayname, avatar_url and all + custom fields. Used at user deactivation when erasure is requested. + + Args: + user_id: User ID whose profile is going to be deleted. """ - await self.db_pool.simple_delete( - desc="delete_profile", - table="profiles", - keyvalues={"full_user_id": user_id.to_string()}, + def _delete_profile_txn(txn: LoggingTransaction) -> None: + # Delete the profile + txn.execute( + """ + DELETE FROM profiles + WHERE full_user_id = ? + """, + (user_id.to_string(),), + ) + + await self.db_pool.runInteraction( + "delete_profile", + _delete_profile_txn, ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 667ad1ace8..499b114cb6 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -25,6 +25,7 @@ from typing import ( AbstractSet, Collection, Iterable, + Literal, Mapping, Sequence, cast, @@ -844,63 +845,111 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore): @cached(max_entries=10000) async def does_pair_of_users_share_a_room( - self, user_id: str, other_user_id: str + self, + user_id: str, + other_user_id: str, + exclude_room_ids: list[str] | None = None, ) -> bool: raise NotImplementedError() - @cachedList( - cached_method_name="does_pair_of_users_share_a_room", list_name="other_user_ids" - ) - async def _do_users_share_a_room( - self, user_id: str, other_user_ids: Collection[str] - ) -> Mapping[str, bool | None]: + def do_users_share_a_room_txn( + self, + txn: LoggingTransaction, + user_id: str, + other_user_ids: Collection[str], + exclude_room_id: str | None = None, + ) -> dict[str, Literal[True]]: """Return mapping from user ID to whether they share a room with the given user. - Note: `None` and `False` are equivalent and mean they don't share a - room. + Optionally, exclude a room when querying the database. + + Users sharing rooms get `True` returned, users who don't are omitted from the return. + (This is for friendliness with `cachedList` on `_do_users_share_a_room`) """ + state_key_clause, state_key_args = make_in_list_sql_clause( + self.database_engine, "state_key", other_user_ids + ) + # Build SQL args based on whether we are excluding a room ID or not + exclude_room_id_clause = "" + exclude_room_id_args: tuple[str, ...] = () - def do_users_share_a_room_txn( - txn: LoggingTransaction, user_ids: Collection[str] - ) -> dict[str, bool]: - clause, args = make_in_list_sql_clause( - self.database_engine, "state_key", user_ids - ) + if exclude_room_id: + exclude_room_id_clause = "AND room_id != ?" + exclude_room_id_args = (exclude_room_id,) - # This query works by fetching both the list of rooms for the target - # user and the set of other users, and then checking if there is any - # overlap. - sql = f""" + # This query works by fetching both the list of rooms for the target + # user and the set of other users, and then checking if there is any + # overlap. + txn.execute( + f""" SELECT DISTINCT b.state_key FROM ( SELECT room_id FROM current_state_events - WHERE type = 'm.room.member' AND membership = 'join' AND state_key = ? + WHERE type = 'm.room.member' + AND membership = 'join' + AND state_key = ? + {exclude_room_id_clause} ) AS a INNER JOIN ( SELECT room_id, state_key FROM current_state_events - WHERE type = 'm.room.member' AND membership = 'join' AND {clause} - ) AS b using (room_id) - """ + WHERE type = 'm.room.member' + AND membership = 'join' + AND {state_key_clause} + {exclude_room_id_clause} + ) AS b USING (room_id) + """, + ( + user_id, + *exclude_room_id_args, + *state_key_args, + *exclude_room_id_args, + ), + ) + return {u: True for (u,) in txn} - txn.execute(sql, (user_id, *args)) - return {u: True for (u,) in txn} + @cachedList( + cached_method_name="does_pair_of_users_share_a_room", + list_name="other_user_ids", + ) + async def _do_users_share_a_room( + self, + user_id: str, + other_user_ids: Collection[str], + exclude_room_id: str | None = None, + ) -> Mapping[str, Literal[True] | None]: + """Return mapping from user ID to whether they share a room with the + given user. + + Optionally, exclude a room when querying the database. + + This returns `True` for users that share a room and `None` for users that don't. + (This is because of the `cachedList` annotation.) + """ to_return = {} for batch_user_ids in batch_iter(other_user_ids, 1000): res = await self.db_pool.runInteraction( - "do_users_share_a_room", do_users_share_a_room_txn, batch_user_ids + "do_users_share_a_room", + self.do_users_share_a_room_txn, + user_id, + batch_user_ids, + exclude_room_id, ) to_return.update(res) return to_return async def do_users_share_a_room( - self, user_id: str, other_user_ids: Collection[str] + self, + user_id: str, + other_user_ids: Collection[str], + exclude_room_id: str | None = None, ) -> set[str]: """Return the set of users who share a room with the first users""" - - user_dict = await self._do_users_share_a_room(user_id, other_user_ids) + user_dict = await self._do_users_share_a_room( + user_id, other_user_ids, exclude_room_id + ) return {u for u, share_room in user_dict.items() if share_room} @@ -992,6 +1041,20 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore): return user_who_share_room + async def get_local_users_who_share_room_with_user(self, user_id: str) -> set[str]: + """Returns the set of local users who share a room with `user_id`. + + This also includes the `user_id` themselves. + """ + room_ids = await self.get_rooms_for_user(user_id) + + user_who_share_room: set[str] = set() + for room_id in room_ids: + user_ids = await self.get_local_users_in_room(room_id) + user_who_share_room.update(user_ids) + + return user_who_share_room + @cached(cache_context=True, iterable=True) async def get_mutual_rooms_between_users( self, user_ids: frozenset[str], cache_context: _CacheContext diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 1afc6d0b2a..3495dce866 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -175,6 +175,7 @@ Changes in SCHEMA_VERSION = 93 Changes in SCHEMA_VERSION = 94 - Add `recheck` column (boolean, default true) to the `redactions` table. - MSC4242: Add state DAG tables. + - MSC4429: Track updates to user profile fields via a new stream. """ diff --git a/synapse/storage/schema/main/delta/94/07_profile_updates.sql b/synapse/storage/schema/main/delta/94/07_profile_updates.sql new file mode 100644 index 0000000000..720f958e09 --- /dev/null +++ b/synapse/storage/schema/main/delta/94/07_profile_updates.sql @@ -0,0 +1,60 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2026 Element Creations 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: +-- . + +-- Track updates to profile fields. +-- For MSC4429 legacy /sync and others. +-- See https://github.com/element-hq/synapse/issues/19981 for potential future directions of this table. +CREATE TABLE IF NOT EXISTS profile_updates ( + stream_id BIGINT NOT NULL PRIMARY KEY, + instance_name TEXT NOT NULL, + + -- The full user ID + user_id TEXT NOT NULL, + + -- Profile action that has happened, see ProfileUpdateAction enum. + action TEXT NOT NULL, + + -- JSON array of the profile field names that have been + -- added, updated or removed in this update. + -- See https://spec.matrix.org/unstable/client-server-api/#profiles + -- This is only present if `action` is `update`. + -- + -- We support multiple field updates at once because it is easy to foresee features + -- involving multiple fields (where getting the illusion of a torn write might be harmful), + -- as well as synchronisation over federation being likely to lead to multiple field changes + -- at once. + affected_fields JSONB NULL, + + -- Unix timestamp (milliseconds) for debugging purposes + inserted_ts BIGINT NOT NULL +); + +CREATE INDEX IF NOT EXISTS profile_updates_by_user ON profile_updates (user_id, stream_id); + +-- We aren't creating a GIN index on `affected_fields` at this time because we don't expect +-- field names to be very selective and therefore an index might not be that useful. + +-- Track which local users should receive each profile update. +CREATE TABLE IF NOT EXISTS profile_updates_per_user ( + -- Stream ID reference to `profile_updates` + stream_id BIGINT NOT NULL REFERENCES profile_updates (stream_id), + + -- The full user ID of the local user that should receive the profile update. + user_id TEXT NOT NULL, + + -- Unix timestamp (milliseconds). Used to determine when to prune rows (to prevent the table + -- from growing indefinitely). + inserted_ts BIGINT NOT NULL, + + PRIMARY KEY (user_id, stream_id) +); diff --git a/synapse/storage/schema/main/delta/94/07_profile_updates_seq.sql.postgres b/synapse/storage/schema/main/delta/94/07_profile_updates_seq.sql.postgres new file mode 100644 index 0000000000..9abf79b68d --- /dev/null +++ b/synapse/storage/schema/main/delta/94/07_profile_updates_seq.sql.postgres @@ -0,0 +1,18 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2026 Element Creations 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: +-- . + +CREATE SEQUENCE profile_updates_sequence; +-- Synapse streams start at 2, because the default position is 1 +-- so any item inserted at position 1 is ignored. +-- We have to use nextval not START WITH 2, see https://github.com/element-hq/synapse/issues/18712 +SELECT nextval('profile_updates_sequence'); diff --git a/synapse/streams/events.py b/synapse/streams/events.py index 36490fcb35..24120eb736 100644 --- a/synapse/streams/events.py +++ b/synapse/streams/events.py @@ -86,6 +86,7 @@ class EventSources: thread_subscriptions_key = self.store.get_max_thread_subscriptions_stream_id() sticky_events_key = self.store.get_max_sticky_events_stream_id() quarantined_media_key = self.store.get_quarantined_media_stream_token() + profile_updates_key = self.store.get_max_profile_updates_stream_id() token = StreamToken( room_key=self.sources.room.get_current_key(), @@ -102,6 +103,7 @@ class EventSources: thread_subscriptions_key=thread_subscriptions_key, sticky_events_key=sticky_events_key, quarantined_media_key=quarantined_media_key, + profile_updates_key=profile_updates_key, ) return token @@ -131,6 +133,7 @@ class EventSources: StreamKeyType.THREAD_SUBSCRIPTIONS: self.store.get_thread_subscriptions_stream_id_generator(), StreamKeyType.STICKY_EVENTS: self.store.get_sticky_events_stream_id_generator(), StreamKeyType.QUARANTINED_MEDIA: self.store.get_quarantined_media_stream_id_generator(), + StreamKeyType.PROFILE_UPDATES: self.store.get_profile_updates_stream_id_generator(), } for _, key in StreamKeyType.__members__.items(): diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index b42893e39d..7516847303 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -1137,6 +1137,7 @@ class StreamKeyType(Enum): THREAD_SUBSCRIPTIONS = "thread_subscriptions_key" STICKY_EVENTS = "sticky_events_key" QUARANTINED_MEDIA = "quarantined_media_key" + PROFILE_UPDATES = "profile_updates_key" @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -1144,7 +1145,7 @@ class StreamToken: """A collection of keys joined together by underscores in the following order and which represent the position in their respective streams. - ex. `s2633508_17_338_6732159_1082514_541479_274711_265584_1_379_4242_4141_4343` + ex. `s2633508_17_338_6732159_1082514_541479_274711_265584_1_379_4242_4141_4343_4444` 1. `room_key`: `s2633508` which is a `RoomStreamToken` - `RoomStreamToken`'s can also look like `t426-2633508` or `m56~2.58~3.59` - See the docstring for `RoomStreamToken` for more details. @@ -1160,6 +1161,7 @@ class StreamToken: 11. `thread_subscriptions_key`: 4242 12. `sticky_events_key`: 4141 13. `quarantined_media_key`: 4343 + 14. `profile_updates_key`: 4444 You can see how many of these keys correspond to the various fields in a "/sync" response: @@ -1223,6 +1225,7 @@ class StreamToken: quarantined_media_key: MultiWriterStreamToken = attr.ib( validator=attr.validators.instance_of(MultiWriterStreamToken) ) + profile_updates_key: int _SEPARATOR = "_" START: ClassVar["StreamToken"] @@ -1253,6 +1256,7 @@ class StreamToken: thread_subscriptions_key, sticky_events_key, quarantined_media_key, + profile_updates_key, ) = keys return cls( @@ -1273,6 +1277,7 @@ class StreamToken: quarantined_media_key=await MultiWriterStreamToken.parse( store, quarantined_media_key ), + profile_updates_key=int(profile_updates_key), ) except CancelledError: raise @@ -1298,6 +1303,7 @@ class StreamToken: str(self.thread_subscriptions_key), str(self.sticky_events_key), await self.quarantined_media_key.to_string(store), + str(self.profile_updates_key), ] ) @@ -1371,6 +1377,7 @@ class StreamToken: StreamKeyType.UN_PARTIAL_STATED_ROOMS, StreamKeyType.THREAD_SUBSCRIPTIONS, StreamKeyType.STICKY_EVENTS, + StreamKeyType.PROFILE_UPDATES, ], ) -> int: ... @@ -1426,9 +1433,10 @@ class StreamToken: f"typing: {self.typing_key}, receipt: {self.receipt_key}, " f"account_data: {self.account_data_key}, push_rules: {self.push_rules_key}, " f"to_device: {self.to_device_key}, device_list: {self.device_list_key}, " - f"groups: {self.groups_key}, un_partial_stated_rooms: {self.un_partial_stated_rooms_key}," - f"thread_subscriptions: {self.thread_subscriptions_key}, sticky_events: {self.sticky_events_key}" - f"quarantined_media: {self.quarantined_media_key})" + f"groups: {self.groups_key}, un_partial_stated_rooms: {self.un_partial_stated_rooms_key}, " + f"thread_subscriptions: {self.thread_subscriptions_key}, sticky_events: {self.sticky_events_key}, " + f"quarantined_media: {self.quarantined_media_key}, " + f"profile_updates: {self.profile_updates_key})" ) @@ -1446,6 +1454,7 @@ StreamToken.START = StreamToken( thread_subscriptions_key=0, sticky_events_key=0, quarantined_media_key=MultiWriterStreamToken(stream=0), + profile_updates_key=0, ) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index f8b4098c71..fe5c3ff67f 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -21,7 +21,13 @@ from twisted.internet.testing import MemoryReactor -from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules, Membership +from synapse.api.constants import ( + AccountDataTypes, + EventTypes, + JoinRules, + Membership, + ProfileFields, +) from synapse.push.rulekinds import PRIORITY_CLASS_MAP from synapse.rest import admin from synapse.rest.client import account, login, room @@ -515,8 +521,12 @@ class DeactivateAccountTestCase(HomeserverTestCase): # Setting a display name now works again. user = UserID.from_string(self.user) self.get_success( - self.hs.get_profile_handler().set_displayname( - user, create_requester(user), "Reactivated", by_admin=True + self.hs.get_profile_handler().set_field( + target_user=user, + requester=create_requester(user), + field_name=ProfileFields.DISPLAYNAME, + new_value="Reactivated", + by_admin=True, ) ) self.assertEqual( @@ -530,8 +540,12 @@ class DeactivateAccountTestCase(HomeserverTestCase): """ user = UserID.from_string(self.user) self.get_success( - self.hs.get_profile_handler().set_displayname( - user, create_requester(user), "Original", by_admin=True + self.hs.get_profile_handler().set_field( + target_user=user, + requester=create_requester(user), + field_name=ProfileFields.DISPLAYNAME, + new_value="Original", + by_admin=True, ) ) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 561b45827f..c53c04fbc8 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -26,12 +26,17 @@ from parameterized import parameterized from twisted.internet.testing import MemoryReactor import synapse.types -from synapse.api.constants import EventTypes +from synapse.api.constants import ( + EventTypes, + ProfileFields, + ProfileUpdateAction, +) from synapse.api.errors import AuthError, SynapseError from synapse.rest import admin -from synapse.rest.client import login, room +from synapse.rest.client import knock, login, room from synapse.server import HomeServer -from synapse.types import JsonDict, UserID +from synapse.storage.databases.main.profile import ProfileUpdate +from synapse.types import JsonDict, StreamKeyType, UserID from synapse.types.state import StateFilter from synapse.util.clock import Clock from synapse.util.duration import Duration @@ -48,6 +53,7 @@ class ProfileTestCase(unittest.HomeserverTestCase): admin.register_servlets, login.register_servlets, room.register_servlets, + knock.register_servlets, ] def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: @@ -62,8 +68,10 @@ class ProfileTestCase(unittest.HomeserverTestCase): self.query_handlers[query_type] = handler self.mock_registry.register_query_handler = register_query_handler + self.mock_hs_notifier = Mock() hs = self.setup_test_homeserver( + notifier=self.mock_hs_notifier, federation_client=self.mock_federation, federation_server=Mock(), federation_registry=self.mock_registry, @@ -83,9 +91,17 @@ class ProfileTestCase(unittest.HomeserverTestCase): self.frank_token = self.login(self.frank.localpart, "frankpassword") self.handler = hs.get_profile_handler() + self.on_new_event = self.mock_hs_notifier.on_new_event def test_get_my_name(self) -> None: - self.get_success(self.store.set_profile_displayname(self.frank, "Frank")) + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", + ) + ) displayname = self.get_success(self.handler.get_displayname(self.frank)) @@ -93,8 +109,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): def test_set_my_name(self) -> None: self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ) ) @@ -105,8 +124,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Set displayname again self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", ) ) @@ -117,8 +139,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Set displayname to an empty string self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "" + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="", ) ) @@ -130,8 +155,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): """Test that `set_displayname` updates membership events in rooms.""" self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", ) ) @@ -149,8 +177,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): self.assertEqual(membership[state_tuple].content["displayname"], "Frank") self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ) ) @@ -161,12 +192,511 @@ class ProfileTestCase(unittest.HomeserverTestCase): ) self.assertEqual(membership[state_tuple].content["displayname"], "Frank Jr.") + @parameterized.expand( + [ + ["displayname", "Frank"], + ["avatar_url", "mxc://foobar"], + ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], + ] + ) + def test_update_profile_does_not_update_stream_on_set_field_if_msc4429_not_enabled( + self, + field_name: str, + new_value: str, + ) -> None: + """Test that profile updates don't get recorded in the profile updates stream + if MSC4429 is not enabled.""" + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value=new_value, + ) + ) + updates = self.get_success( + self.store.get_updated_profile_updates( + from_id=1, + to_id=2, + limit=1, + ) + ) + self.assertEqual(len(updates), 0) + + @parameterized.expand( + [ + ["displayname", "Frank"], + ["avatar_url", "mxc://foobar"], + ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], + ] + ) + def test_update_profile_does_not_notify_notifier_on_set_field_if_msc4429_not_enabled( + self, + field_name: str, + new_value: str, + ) -> None: + """Test that profile updates do not cause the profile updates stream notifier + to wake up if MSC4429 is not enabled.""" + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value=new_value, + ) + ) + + calls_found = [ + call + for call in self.on_new_event.mock_calls + if call.args[0] == StreamKeyType.PROFILE_UPDATES + ] + self.assertEqual(len(calls_found), 0) + + @parameterized.expand( + [ + ["displayname", "Frank"], + ["avatar_url", "mxc://foobar"], + ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_update_profile_does_not_notify_notifier_on_set_field_if_user_not_in_rooms( + self, field_name: str, new_value: str + ) -> None: + """Test that profile updates do not cause the profile updates stream notifier + to wake up if the user is not in any rooms, if MSC4429 is enabled.""" + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value=new_value, + ) + ) + calls_found = [ + call + for call in self.on_new_event.mock_calls + if call.args[0] == StreamKeyType.PROFILE_UPDATES + ] + self.assertEqual(len(calls_found), 0) + + @parameterized.expand( + [ + ["displayname", "Frank"], + ["avatar_url", "mxc://foobar"], + ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_update_profile_updates_stream_on_set_field( + self, field_name: str, new_value: str + ) -> None: + """Test that profile updates get recorded in the profile updates stream if + MSC4429 is enabled.""" + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value=new_value, + ) + ) + updates = self.get_success( + self.store.get_updated_profile_updates( + from_id=1, + to_id=2, + limit=1, + ) + ) + self.assertEqual( + updates[0], + ( + 2, + "@1234abcd:test", + ProfileUpdateAction.UPDATE.value, + {field_name}, + ), + ) + + fields_updates = self.get_success( + self.store.get_profile_updates_for_fields( + from_id=1, + to_id=2, + field_names={field_name}, + ) + ) + self.assertEqual( + fields_updates[0], + ProfileUpdate( + stream_id=2, + user_id="@1234abcd:test", + action=ProfileUpdateAction.UPDATE.value, + affected_fields=frozenset({field_name}), + ), + ) + + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value="", + ) + ) + delete_updates = self.get_success( + self.store.get_updated_profile_updates( + from_id=2, + to_id=3, + limit=1, + ) + ) + self.assertEqual( + delete_updates[0], + (3, "@1234abcd:test", ProfileUpdateAction.UPDATE.value, {field_name}), + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_update_profile_set_field_writes_to_per_user_profile_tracking_table( + self, + ) -> None: + """Test that profiles updates get recorded in the 'per user' profile updates + stream tracking table, if MSC4429 is enabled.""" + self.register_user("roger", "password") + roger_token = self.login("roger", "password") + self.register_user("millie", "password") + millie_token = self.login("millie", "password") + room_id = self.helper.create_room_as( + room_creator=self.frank.to_string(), + tok=self.frank_token, + ) + self.helper.join(room_id, "@roger:test", tok=roger_token) + self.helper.join(room_id, "@millie:test", tok=millie_token) + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name="m.status", + new_value='{"text": "Holiday"}', + ) + ) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@roger:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=3, + user_id="@millie:test", + action="joined_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=4, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@millie:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=4, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id=self.frank.to_string(), + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=2, + user_id="@roger:test", + action="joined_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=3, + user_id="@millie:test", + action="joined_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=4, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_membership_addition_to_room_adds_the_right_join_action_to_profile_streams( + self, + ) -> None: + """Test that a membership event, which adds a user as joined to a room, + adds the relevant joined action to the profile update stream tables. + + Here we consider join, knock and invite to all be additions to the room + list of members for answering the question "which profiles should we send + information about to clients based on memberships appearing". + """ + self.register_user("roger", "password") + roger_token = self.login("roger", "password") + room_id = self.helper.create_room_as( + room_creator=self.frank.to_string(), + tok=self.frank_token, + ) + + self.helper.join(room_id, "@roger:test", tok=roger_token) + + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id=self.frank.to_string(), + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=2, + user_id="@roger:test", + action="joined_room", + affected_fields=None, + ), + ], + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_previous_profile_updates_stream_rows_cleared_if_no_longer_sharing_a_room( + self, + ) -> None: + """Test that previous profile update stream rows are removed for a user if + the user no longer shares rooms with another user, if MSC4429 is enabled. + + This test ensures that when a user leaves a room, we clear all old profile + update rows of users who the user no longer shares rooms with, to avoid + leaking any further profile field updates from those users. + """ + self.register_user("roger", "password") + roger_token = self.login("roger", "password") + self.register_user("millie", "password") + millie_token = self.login("millie", "password") + self.register_user("gracie", "password") + gracie_token = self.login("gracie", "password") + room_id = self.helper.create_room_as( + room_creator=self.frank.to_string(), + tok=self.frank_token, + ) + room_with_millie_id = self.helper.create_room_as( + room_creator=self.frank.to_string(), + tok=self.frank_token, + ) + self.helper.join(room_id, "@roger:test", tok=roger_token) + self.helper.join(room_with_millie_id, "@millie:test", tok=millie_token) + self.helper.join(room_id, "@gracie:test", tok=gracie_token) + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name="m.status", + new_value='{"text": "Holiday"}', + ) + ) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@roger:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=4, + user_id="@gracie:test", + action="joined_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=5, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@millie:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=5, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + + # Make frank leave room and verify only the "left room" + gracies join exists + # for roger + self.helper.leave(room_id, self.frank.to_string(), tok=self.frank_token) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@roger:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=4, + user_id="@gracie:test", + action="joined_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=6, + user_id=self.frank.to_string(), + action="left_room", + affected_fields=None, + ), + ], + ) + # Make gracie leave room and verify only the "left room"'s + self.helper.leave(room_id, "@gracie:test", tok=gracie_token) + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@roger:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=6, + user_id=self.frank.to_string(), + action="left_room", + affected_fields=None, + ), + ProfileUpdate( + stream_id=7, + user_id="@gracie:test", + action="left_room", + affected_fields=None, + ), + ], + ) + + # Sanity check we didn't clear any rows for millie + per_user_updates = self.get_success( + self.store.get_profile_updates_for_user_and_fields( + from_id=0, + to_id=10, + user_id="@millie:test", + field_names={"m.status"}, + ) + ) + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=5, + user_id=self.frank.to_string(), + action="update", + affected_fields=frozenset({"m.status"}), + ), + ], + ) + + @parameterized.expand( + [ + ["displayname", "Frank"], + ["avatar_url", "mxc://foobar"], + ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_update_profile_notifies_notifier_on_set_field( + self, + field_name: str, + new_value: str, + ) -> None: + """Test that profile updates wake up the profile updates stream on profile + field updates, if MSC4429 is enabled.""" + self.helper.create_room_as( + room_creator=self.frank.to_string(), + tok=self.frank_token, + ) + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value=new_value, + ) + ) + calls_found = [ + call + for call in self.on_new_event.mock_calls + if call.args[0] == StreamKeyType.PROFILE_UPDATES + ] + self.assertEqual(len(calls_found), 1) + def test_background_update_room_membership_on_set_displayname(self) -> None: """Test that `set_displayname` returns immediately and that room membership updates are still done in background.""" self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", ) ) @@ -187,8 +717,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): ): state_tuple = (EventTypes.Member, self.frank.to_string()) self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ) ) @@ -215,8 +748,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): """Test that room membership updates triggered by changing the avatar or the display name are resumed after a restart.""" self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", ) ) @@ -253,8 +789,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): ): state_tuple = (EventTypes.Member, self.frank.to_string()) self.get_success( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ) ) @@ -320,7 +859,13 @@ class ProfileTestCase(unittest.HomeserverTestCase): @override_config({"enable_set_displayname": False}) 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, "Frank")) + self.get_success( + self.store.set_profile_field( + user_id=self.frank, + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", + ) + ) self.assertEqual( (self.get_success(self.store.get_profile_displayname(self.frank))), @@ -329,16 +874,22 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Setting displayname a second time is forbidden self.get_failure( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ), SynapseError, ) def test_set_my_name_noauth(self) -> None: self.get_failure( - self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.bob), "Frank Jr." + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.bob), + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank Jr.", ), AuthError, ) @@ -361,8 +912,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): self.store.create_profile(UserID.from_string("@caroline:test")) ) self.get_success( - self.store.set_profile_displayname( - UserID.from_string("@caroline:test"), "Caroline" + self.handler.set_field( + target_user=UserID.from_string("@caroline:test"), + requester=synapse.types.create_requester("@caroline:test"), + field_name=ProfileFields.DISPLAYNAME, + new_value="Caroline", ) ) @@ -380,16 +934,31 @@ class ProfileTestCase(unittest.HomeserverTestCase): def test_get_my_avatar(self) -> None: self.get_success( - self.store.set_profile_avatar_url(self.frank, "http://my.server/me.png") + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/me.png", + ) ) avatar_url = self.get_success(self.handler.get_avatar_url(self.frank)) self.assertEqual("http://my.server/me.png", avatar_url) def test_get_profile_empty_displayname(self) -> None: - self.get_success(self.store.set_profile_displayname(self.frank, None)) self.get_success( - self.store.set_profile_avatar_url(self.frank, "http://my.server/me.png") + self.store.set_profile_field( + user_id=self.frank, + field_name=ProfileFields.DISPLAYNAME, + new_value=None, + ) + ) + self.get_success( + self.store.set_profile_field( + user_id=self.frank, + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/me.png", + ) ) profile = self.get_success(self.handler.get_profile(self.frank.to_string())) @@ -398,10 +967,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): def test_set_my_avatar(self) -> None: self.get_success( - self.handler.set_avatar_url( - self.frank, - synapse.types.create_requester(self.frank), - "http://my.server/pic.gif", + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/pic.gif", ) ) @@ -412,10 +982,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Set avatar again self.get_success( - self.handler.set_avatar_url( - self.frank, - synapse.types.create_requester(self.frank), - "http://my.server/me.png", + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/me.png", ) ) @@ -426,10 +997,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Set avatar to an empty string self.get_success( - self.handler.set_avatar_url( - self.frank, - synapse.types.create_requester(self.frank), - "", + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="", ) ) @@ -441,7 +1013,12 @@ class ProfileTestCase(unittest.HomeserverTestCase): 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, "http://my.server/me.png") + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/me.png", + ) ) self.assertEqual( @@ -451,10 +1028,11 @@ class ProfileTestCase(unittest.HomeserverTestCase): # Set avatar a second time is forbidden self.get_failure( - self.handler.set_avatar_url( - self.frank, - synapse.types.create_requester(self.frank), - "http://my.server/pic.gif", + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/pic.gif", ), SynapseError, ) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 0db7f30b1f..182ff7a8fc 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -25,7 +25,7 @@ from unittest.mock import AsyncMock, Mock from twisted.internet.testing import MemoryReactor from synapse.api.auth.internal import InternalAuth -from synapse.api.constants import UserTypes +from synapse.api.constants import ProfileFields, UserTypes from synapse.api.errors import ( CodeMessageException, Codes, @@ -824,8 +824,12 @@ class RegistrationTestCase(unittest.HomeserverTestCase): if displayname is not None: # logger.info("setting user display name: %s -> %s", user_id, displayname) - await self.hs.get_profile_handler().set_displayname( - user, requester, displayname, by_admin=True + await self.hs.get_profile_handler().set_field( + target_user=user, + requester=requester, + field_name=ProfileFields.DISPLAYNAME, + new_value=displayname, + by_admin=True, ) return user_id, token diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index d2b2523321..bfb687a6c7 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -18,7 +18,7 @@ # # from http import HTTPStatus -from typing import Collection, ContextManager +from typing import Collection, ContextManager, cast from unittest.mock import AsyncMock, Mock, patch from parameterized import parameterized, parameterized_class @@ -33,6 +33,7 @@ from synapse.api.room_versions import RoomVersion, RoomVersions from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.handlers.sync import ( + LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE, SyncConfig, SyncRequestKey, SyncResult, @@ -43,6 +44,7 @@ from synapse.rest.client import knock, login, room from synapse.server import HomeServer from synapse.types import ( JsonDict, + JsonValue, MultiWriterStreamToken, RoomStreamToken, StreamKeyType, @@ -55,6 +57,8 @@ from synapse.util.duration import Duration import tests.unittest import tests.utils from tests.test_utils.event_builders import make_test_pdu_event +from tests.test_utils.event_injection import inject_member_event +from tests.unittest import override_config _request_key = 0 @@ -1152,6 +1156,1543 @@ def generate_sync_config( ) +class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): + """Tests Sync Handler for profile updates.""" + + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + self.sync_handler = self.hs.get_sync_handler() + self.profile_handler = self.hs.get_profile_handler() + self.store = self.hs.get_datastores().main + self.user = self.register_user("user", "password") + self.tok = self.login("user", "password") + self.other_user = self.register_user("other_user", "password") + self.other_tok = self.login("other_user", "password") + self.joined_room = self.helper.create_room_as(self.user, tok=self.tok) + self.get_success( + self.store.set_profile_field( + user_id=UserID.from_string(self.user), + field_name="m.status", + new_value={"text": "Swimming in the Great Lakes!", "emoji": "🏊"}, + ) + ) + self.helper.join( + room=self.joined_room, user=self.other_user, tok=self.other_tok + ) + + def test_initial_sync_no_profile_updates_if_not_enabled(self) -> None: + """Test that without MSC4429 enabled the initial sync response does not + contain any profile updates.""" + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + self.user, + ), + request_key=generate_request_key(), + ) + ) + self.assertEqual(initial_result.profile_updates, {}) + + @override_config({"include_profile_updates_in_sync": True}) + def test_initial_sync_no_profile_updates_if_not_filtered_for(self) -> None: + """Test that with MSC4429 enabled the initial sync response does not + contain any profile updates, if fields are not filtered for.""" + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + ), + request_key=generate_request_key(), + ) + ) + self.assertEqual( + initial_result.profile_updates, + {}, + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_initial_sync_responds_with_tracked_profile_updates(self) -> None: + """Test that with MSC4429 enabled the initial sync response does + contain profile updates for users who share rooms, for the fields the + client requests. This response should include our syncing user.""" + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + # Also set a field the client doesn't want + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="displayname", + new_value="New displayname", + ) + ) + + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": {"ids": ["m.status"]} + }, + ), + ), + request_key=generate_request_key(), + ) + ) + assert initial_result.profile_updates[self.user] is not None + assert initial_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + initial_result.profile_updates["@other_user:test"]["m.status"], + {"text": "On holiday", "emoji": "🏖"}, + ) + self.assertFalse( + "displayname" in initial_result.profile_updates["@other_user:test"].keys(), + ) + self.assertCountEqual( + initial_result.profile_updates.keys(), + [ + self.user, + "@other_user:test", + ], + ) + + @parameterized.expand( + [ + True, + False, + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_initial_sync_does_not_include_untracked_users_profile_updates( + self, is_lazy: bool + ) -> None: + """Test that with MSC4429 enabled the initial sync response does not + contain profile updates for users who do not share rooms.""" + third_user = self.register_user("third_user", "password") + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + + requester = create_requester(self.user) + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + } + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertIsNone(initial_result.profile_updates.get(third_user)) + + @override_config({"include_profile_updates_in_sync": True}) + def test_initial_sync_lazy_loading_responds_with_only_profiles_with_events( + self, + ) -> None: + """Test that with MSC4429 enabled the initial sync lazy loading response does + contain profile updates for events in the timeline. + + This test ensures lazy loading sync only returns profiles that we also have + events for in the sync response. The second room in this test has the most + recent events from "third_user" and thus we don't get the profile of + "other_user" down the line, who is in the the same rooms as the syncer, + but not in the second room. + """ + third_user = self.register_user("third_user", "password") + third_tok = self.login("third_user", "password") + self.helper.join( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + + requester = create_requester(self.user) + + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + # Check that lazy-loading filters out profile updates as well on initial sync. + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="m.status", + new_value={"text": "On fire", "emoji": "🔥"}, + ) + ) + self.helper.send_messages( + room_id=self.joined_room, num_events=1, tok=self.other_tok + ) + self.helper.send_messages( + room_id=self.joined_room, num_events=10, tok=third_tok + ) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + }, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + # Only third_user is returned, as lazy loading filters out the events from + # the other users + self.assertCountEqual( + initial_result.profile_updates.keys(), + [ + "@third_user:test", + ], + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_sends_down_profile_update_diffs( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental sync response does + contain profile update diffs.""" + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + } + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + # Set a field the client didn't ask for + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="uninterestingfield", + new_value="Content", + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + } + }, + ), + ), + request_key=generate_request_key(), + ) + ) + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["m.status"], + {"text": "On holiday", "emoji": "🏖"}, + ) + # We only send diffs in incremental sync for profile field updates + self.assertFalse( + "displayname" + in incremental_result.profile_updates["@other_user:test"].keys(), + ) + # The client didn't ask for this field + self.assertFalse( + "uninterestingfield" + in incremental_result.profile_updates["@other_user:test"].keys(), + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_does_not_filter_profile_updates_when_lazy_loading( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental sync lazy loading response + does contain profile updates even if the user would be filtered out by lazy + loading. + """ + third_user = self.register_user("third_user", "password") + third_tok = self.login("third_user", "password") + self.helper.join( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname"] + } + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="m.status", + new_value={"text": "On fire", "emoji": "🔥"}, + ) + ) + self.helper.send_messages( + room_id=self.joined_room, num_events=1, tok=self.other_tok + ) + self.helper.send_messages( + room_id=self.joined_room, num_events=10, tok=third_tok + ) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="uninterestingfield", + new_value="Content", + ) + ) + # Join a federated user to the room + self.get_success( + inject_member_event( + self.hs, + self.joined_room, + "@federateduser:federatedhs", + "join", + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname"] + }, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + + # Ensure our federated user is filtered out, even though they have an + # event in the joined room timeline + self.assertFalse( + "@federateduser:federatedhs" in incremental_result.profile_updates.keys() + ) + + # Lazy loading only filters initial sync profile updates. Incremental syncs + # should include all tracked profile updates for the syncing user. + self.assertCountEqual( + incremental_result.profile_updates.keys(), + [ + "@other_user:test", + "@third_user:test", + ], + ) + assert incremental_result.profile_updates["@other_user:test"] is not None + + # This is a field update, so should be here + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["m.status"], + {"text": "On holiday", "emoji": "🏖"}, + ) + + # We don't have events for this user in this response, so their full profile + # is not included + self.assertFalse( + "displayname" + in incremental_result.profile_updates["@other_user:test"].keys(), + ) + assert incremental_result.profile_updates["@third_user:test"] is not None + + # This user has events in the timeline, thus the fields the client asked for + # are included + self.assertEqual( + incremental_result.profile_updates["@third_user:test"]["m.status"], + {"text": "On fire", "emoji": "🔥"}, + ) + self.assertFalse( + "uninterestingfield" + in incremental_result.profile_updates["@third_user:test"].keys(), + ) + self.assertEqual( + incremental_result.profile_updates["@third_user:test"]["displayname"], + "third_user", + ) + + @parameterized.expand( + [ + [True, True], + [False, False], + [True, False], + [False, True], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_sync_filters_out_profile_updates_from_federated_users( + self, + is_initial: bool, + is_lazy: bool, + ) -> None: + """Test that with MSC4429 enabled any sync response + doesn't contain federated users even if there are timeline events from them. + """ + # Join a federated user to the room, causing a membership event into + # the joined rooms sync response + self.get_success( + inject_member_event( + self.hs, + self.joined_room, + "@federateduser1:federatedhs", + "join", + ) + ) + requester = create_requester(self.user) + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + }, + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # Ensure our federated user is filtered out, even though they have an + # event in the joined room timeline + self.assertFalse( + "@federateduser1:federatedhs" in initial_result.profile_updates.keys() + ) + if not is_initial: + # Join another federated user to the room, causing a membership event into + # the joined rooms sync response + self.get_success( + inject_member_event( + self.hs, + self.joined_room, + "@federateduser2:federatedhs", + "join", + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + + # Ensure our federated user is filtered out, even though they have an + # event in the joined room timeline + self.assertFalse( + "@federateduser2:federatedhs" + in incremental_result.profile_updates.keys() + ) + + @parameterized.expand( + [ + [True, True], + [False, False], + [True, False], + [False, True], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_sync_response_always_includes_the_user_themselves( + self, + is_initial: bool, + is_lazy: bool, + ) -> None: + """Test that with MSC4429 enabled any sync response always contains the users + own updates. + + This test is made with a user that is not in any rooms, to prove our code + to collect interested users from the profile updates always collect the user. + """ + third_user = self.register_user("third_user", "password") + requester = create_requester(third_user) + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": {"ids": ["field"]}, + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + if is_initial: + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="field", + new_value="Content", + ) + ) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=third_user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + if is_initial: + assert initial_result.profile_updates["@third_user:test"] is not None + self.assertEqual( + initial_result.profile_updates["@third_user:test"]["field"], + "Content", + ) + else: + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="field", + new_value="Content", + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=third_user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + assert incremental_result.profile_updates["@third_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@third_user:test"]["field"], + "Content", + ) + + @parameterized.expand( + [ + [True, True], + [False, False], + [True, False], + [False, True], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_sync_profile_updates_works_correctly_with_falsey_values( + self, + is_initial: bool, + is_lazy: bool, + ) -> None: + """Test that with MSC4429 enabled a sync response correctly includes falsey + profile field values. + """ + requester = create_requester(self.user) + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": {"ids": ["falseyvaluefield"]}, + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + for value in [False, 0, "", [], {}, None]: + if is_initial: + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="falseyvaluefield", + new_value=cast(JsonValue | dict[str, JsonValue], value), + ) + ) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + if is_initial: + assert initial_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + initial_result.profile_updates["@other_user:test"][ + "falseyvaluefield" + ], + value, + ) + else: + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="falseyvaluefield", + new_value=cast(JsonValue | dict[str, JsonValue], value), + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + assert ( + incremental_result.profile_updates["@other_user:test"] is not None + ) + self.assertEqual( + incremental_result.profile_updates["@other_user:test"][ + "falseyvaluefield" + ], + value, + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_lazy_loading_cache_filters_recently_sent_profiles_and_fields( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental sync lazy loading response + filters out unchanged profiles or fields we have recently sent to the client. + """ + requester = create_requester(self.user) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="sooninterestingfield", + new_value="Content", + ) + ) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.helper.send_messages( + room_id=self.joined_room, + num_events=1, + tok=self.other_tok, + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + }, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + # Lazy loading incremental sync should include profiles from events + self.assertCountEqual( + incremental_result.profile_updates.keys(), + [ + "@other_user:test", + ], + ) + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + set(incremental_result.profile_updates["@other_user:test"].keys()), + {"avatar_url", "displayname"}, + ) + + # If we have more events from the other_user, and do another lazy sync, + # we don't expect the full profile to be sent again due to our cache. + self.helper.send_messages( + room_id=self.joined_room, num_events=1, tok=self.other_tok + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + }, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertCountEqual( + incremental_result.profile_updates.keys(), + [], + ) + # However, if we again add an event, we do expect any fields the client didn't + # previously ask for to be there. + self.helper.send_messages( + room_id=self.joined_room, num_events=1, tok=self.other_tok + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": [ + "m.status", + "displayname", + "avatar_url", + "sooninterestingfield", + ] + }, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertCountEqual( + incremental_result.profile_updates.keys(), + [ + "@other_user:test", + ], + ) + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + set(incremental_result.profile_updates["@other_user:test"].keys()), + {"sooninterestingfield"}, + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_sends_down_null_profile_if_user_no_longer_sharing_rooms( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental sync response + includes a 'null' for users who are no longer sharing rooms. + """ + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + } + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.helper.leave( + room=self.joined_room, user=self.other_user, tok=self.other_tok + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["m.status", "displayname", "avatar_url"] + } + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertIsNone( + incremental_result.profile_updates["@other_user:test"], + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_sends_down_all_requested_fields_for_users_who_have_joined( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental sync response + includes all the requested fields of a user who has joined a room with the + syncing user. + """ + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["displayname", "avatar_url"] + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["displayname", "avatar_url"] + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + + third_user = self.register_user("third_user", "password") + third_tok = self.login("third_user", "password") + self.helper.join( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + # Set a status field we don't except to see in sync + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(third_user), + requester=create_requester(third_user), + field_name="m.status", + new_value={"text": "On fire", "emoji": "🔥"}, + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": { + "ids": ["displayname", "avatar_url"] + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + assert incremental_result.profile_updates["@third_user:test"] is not None + self.assertCountEqual( + incremental_result.profile_updates.keys(), + [third_user], + ) + self.assertEqual( + incremental_result.profile_updates["@third_user:test"]["displayname"], + "third_user", + ) + self.assertIsNone( + incremental_result.profile_updates["@third_user:test"]["avatar_url"], + ) + self.assertFalse( + "m.status" in incremental_result.profile_updates["@third_user:test"].keys(), + ) + + @parameterized.expand( + [ + True, + False, + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_includes_own_profile_updates(self, is_lazy: bool) -> None: + """Test that with MSC4429 enabled the incremental sync response includes + ones own profile updates.""" + requester = create_requester(self.user) + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": {"ids": ["m.status", "avatar_url"]} + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.user), + requester=requester, + field_name="m.status", + new_value={"text": "On holiday", "emoji": "🏖"}, + ) + ) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + assert incremental_result.profile_updates["@user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@user:test"]["m.status"], + {"text": "On holiday", "emoji": "🏖"}, + ) + # We didn't ask for displayname + self.assertFalse( + "displayname" in incremental_result.profile_updates["@user:test"].keys(), + ) + + @parameterized.expand([[True, False], [True, True], [False, False], [False, True]]) + @override_config({"include_profile_updates_in_sync": True}) + def test_incremental_sync_join_leave_join_leave_includes_user_joining_and_leaving( + self, + eager_sync: bool, + is_lazy: bool, + ) -> None: + """Test that with MSC4429 enabled the incremental sync response + correctly handles multiple join / leave / join / leave in a row. + + In the first variant we sync and check after each iteration of join/leave. + In the second variant we only sync at the end of all the join/leaves. + We do both of these as lazy and not-lazy variants. + + This test checks that for a syncing user that is joining and leaving, a member + of the room gets the right profile information down the line. + """ + # Use third_user for this test as other_user is already joined + third_user = self.register_user("third_user", "password") + third_tok = self.login("third_user", "password") + + filter_json: dict[str, dict] = { + "org.matrix.msc4429.profile_fields": {"ids": ["displayname", "avatar_url"]} + } + if is_lazy: + filter_json["room"] = { + "state": { + "lazy_load_members": True, + }, + } + + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # Join the room + self.helper.join( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + next_token = initial_result.next_batch + if eager_sync: + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=next_token, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # We expect there to be the users profile + self.assertIsNotNone( + incremental_result.profile_updates["@third_user:test"], + ) + next_token = incremental_result.next_batch + # Leave the room + self.helper.leave( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + if eager_sync: + # Ensure we don't get caught by the cache + self.reactor.advance((LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE / 1000) + 1) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=next_token, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # We expect there to be a null profile + self.assertIsNone( + incremental_result.profile_updates["@third_user:test"], + ) + next_token = incremental_result.next_batch + # Join the room + self.helper.join( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + if eager_sync: + # Ensure we don't get caught by the cache + self.reactor.advance((LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE / 1000) + 1) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=next_token, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # We expect there to be the users profile + self.assertIsNotNone( + incremental_result.profile_updates["@third_user:test"], + ) + next_token = incremental_result.next_batch + # Leave the room + self.helper.leave( + room=self.joined_room, + user=third_user, + tok=third_tok, + ) + # Ensure we don't get caught by the cache + self.reactor.advance((LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE / 1000) + 1) + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=next_token, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # The end result should be null profile + self.assertIsNone( + incremental_result.profile_updates["@third_user:test"], + ) + + @parameterized.expand( + [ + ["string value", "new string value"], + [True, False], + [None, "not None"], + [[], ["with item"]], + [{}, {"key": "value"}], + [{"foo": "bar"}, {"bar": "foo"}], + [42, 42.24], + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_profile_updates_dont_get_silenced_by_cache( + self, + value: str | bool | list | dict | int | float | None, + new_value: str | bool | list | dict | int | float | None, + ) -> None: + """Test that with MSC4429 enabled the incremental lazy sync response + includes all the profile update changes for the user, even if the profile + field has been recently sent and is in our lazy loading cache. + + Parameterize across different types of potential value types that profile + field updates could have to ensure robustness. + """ + requester = create_requester(self.user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": {"ids": ["field"]}, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertFalse( + "@other_user:test" in initial_result.profile_updates, + ) + + # Update the field + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="field", + new_value=cast(JsonValue | dict[str, JsonValue], value), + ) + ) + # Also send an event + self.helper.send(self.joined_room, "Foo", tok=self.other_tok) + # Sync + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": {"ids": ["field"]}, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + # We should have the field change in our sync response. + # It will also be added to the lazy loading cache, so the same field value + # isn't sent again immediately. + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["field"], + value, + ) + + # Update the field again, busting our cache + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="field", + new_value=cast(JsonValue | dict[str, JsonValue], new_value), + ) + ) + # Also send an event + self.helper.send(self.joined_room, "Foo", tok=self.other_tok) + # Sync + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json={ + "org.matrix.msc4429.profile_fields": {"ids": ["field"]}, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + }, + ), + ), + request_key=generate_request_key(), + ) + ) + # Even though the field was added to the lazy loading members cache, + # it should come through as an update, as the field value changed. + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["field"], + new_value, + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_lazy_loading_cache_and_multiple_updates_to_the_same_field( + self, + ) -> None: + """Test that with MSC4429 enabled the incremental lazy sync response + includes an update to a field, even when the value changes back to a + value set and cached previously. + """ + requester = create_requester(self.user) + filter_json = { + "org.matrix.msc4429.profile_fields": {"ids": ["field"]}, + "room": { + "state": { + "lazy_load_members": True, + }, + }, + } + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + self.assertFalse( + "@other_user:test" in initial_result.profile_updates, + ) + + # Update the field + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="field", + new_value="value", + ) + ) + # Also send an event + self.helper.send(self.joined_room, "Foo", tok=self.other_tok) + # Sync + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=initial_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # We should have the field change in our sync response. + # It will also be added to the lazy loading cache, so the same field value + # isn't sent again immediately. + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["field"], + "value", + ) + + # Update the field again, busting our cache + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="field", + new_value="new value", + ) + ) + # Also send an event + self.helper.send(self.joined_room, "Foo", tok=self.other_tok) + # Sync + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # Even though the field was added to the lazy loading members cache, + # it should come through as an update, as the field value changed. + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["field"], + "new value", + ) + + # Update the field again, but to the previous value + self.get_success( + self.profile_handler.set_field( + target_user=UserID.from_string(self.other_user), + requester=create_requester(self.other_user), + field_name="field", + new_value="value", + ) + ) + # Also send an event + self.helper.send(self.joined_room, "Foo", tok=self.other_tok) + # Sync + incremental_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + since_token=incremental_result.next_batch, + sync_config=generate_sync_config( + user_id=self.user, + filter_collection=FilterCollection( + hs=self.hs, + filter_json=filter_json, + ), + ), + request_key=generate_request_key(), + ) + ) + # Even though we've quite recently sent down this value, we should still + # see it again as it is a change + assert incremental_result.profile_updates["@other_user:test"] is not None + self.assertEqual( + incremental_result.profile_updates["@other_user:test"]["field"], + "value", + ) + + class SyncStateAfterTestCase(tests.unittest.HomeserverTestCase): """Tests Sync Handler state behavior when using `use_state_after.""" diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index c4e4170c6f..4deb3c29f4 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -2549,7 +2549,7 @@ class RoomMessagesTestCase(unittest.HomeserverTestCase): def test_topo_token_is_accepted(self) -> None: """Test Topo Token is accepted.""" - token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0" + token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/_synapse/admin/v1/rooms/%s/messages?from=%s" % (self.room_id, token), @@ -2563,7 +2563,7 @@ class RoomMessagesTestCase(unittest.HomeserverTestCase): def test_stream_token_is_accepted_for_fwd_pagianation(self) -> None: """Test that stream token is accepted for forward pagination.""" - token = "s0_0_0_0_0_0_0_0_0_0_0_0_0" + token = "s0_0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/_synapse/admin/v1/rooms/%s/messages?from=%s" % (self.room_id, token), diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index bce199c564..e2e133a85e 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -39,6 +39,7 @@ from synapse.api.constants import ( EventContentFields, EventTypes, LoginType, + ProfileFields, UserTypes, ) from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError @@ -943,18 +944,24 @@ class UsersListTestCase(unittest.HomeserverTestCase): # 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( - UserID.from_string("@user1:test"), "mxc://url3" + self.store.set_profile_field( + user_id=UserID.from_string("@user1:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://url3", ) ) self.get_success( - self.store.set_profile_avatar_url( - UserID.from_string("@user2:test"), "mxc://url2" + self.store.set_profile_field( + user_id=UserID.from_string("@user2:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://url2", ) ) self.get_success( - self.store.set_profile_avatar_url( - UserID.from_string("@admin:test"), "mxc://url1" + self.store.set_profile_field( + user_id=UserID.from_string("@admin:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://url1", ) ) @@ -1546,8 +1553,10 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase): # set attributes for user self.get_success( - self.store.set_profile_avatar_url( - UserID.from_string("@user:test"), "mxc://servername/mediaid" + self.store.set_profile_field( + user_id=UserID.from_string("@user:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://servername/mediaid", ) ) self.get_success( @@ -1679,7 +1688,11 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase): """ # Patch `self.other_user` to have an empty string as their avatar. self.get_success( - self.store.set_profile_avatar_url(UserID.from_string("@user:test"), "") + self.store.set_profile_field( + user_id=UserID.from_string("@user:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="", + ) ) # Check we can still erase them. @@ -2758,8 +2771,10 @@ class UserRestTestCase(unittest.HomeserverTestCase): # set attributes for user self.get_success( - self.store.set_profile_avatar_url( - UserID.from_string("@user:test"), "mxc://servername/mediaid" + self.store.set_profile_field( + user_id=UserID.from_string("@user:test"), + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://servername/mediaid", ) ) self.get_success( diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index c0494c606f..8ffac644b6 100644 --- a/tests/rest/client/test_account.py +++ b/tests/rest/client/test_account.py @@ -30,7 +30,7 @@ from twisted.internet.interfaces import IReactorTCP from twisted.internet.testing import MemoryReactor import synapse.rest.admin -from synapse.api.constants import LoginType, Membership +from synapse.api.constants import LoginType, Membership, ProfileFields from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.appservice import ApplicationService from synapse.rest import admin @@ -520,13 +520,19 @@ class DeactivateTestCase(unittest.HomeserverTestCase): # Set some profile data that can be checked for after the user is erased self.get_success( - profile_handler.set_displayname( - user_id, create_requester(user_id), "Kermit the Frog" + profile_handler.set_field( + target_user=user_id, + requester=create_requester(user_id), + field_name=ProfileFields.DISPLAYNAME, + new_value="Kermit the Frog", ) ) self.get_success( - profile_handler.set_avatar_url( - user_id, create_requester(user_id), "http://test/Kermit.jpg" + profile_handler.set_field( + target_user=user_id, + requester=create_requester(user_id), + field_name=ProfileFields.AVATAR_URL, + new_value="http://test/Kermit.jpg", ) ) # Verify it is set @@ -578,9 +584,19 @@ class DeactivateTestCase(unittest.HomeserverTestCase): # Can not use the profile handler to set a display name when it is disabled. Use # the database directly store = self.hs.get_datastores().main - self.get_success(store.set_profile_displayname(user_id, "Kermit the Frog")) self.get_success( - store.set_profile_avatar_url(user_id, "http://test/Kermit.jpg") + store.set_profile_field( + user_id=user_id, + field_name=ProfileFields.DISPLAYNAME, + new_value="Kermit the Frog", + ) + ) + self.get_success( + store.set_profile_field( + user_id=user_id, + field_name=ProfileFields.AVATAR_URL, + new_value="http://test/Kermit.jpg", + ) ) # Verify it is set diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 05d42a3d44..a6f5043c92 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2252,7 +2252,7 @@ class RoomMessageListTestCase(RoomBase): self.room_id = self.helper.create_room_as(self.user_id) def test_topo_token_is_accepted(self) -> None: - token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0" + token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/rooms/%s/messages?access_token=x&from=%s" % (self.room_id, token) ) @@ -2263,7 +2263,7 @@ class RoomMessageListTestCase(RoomBase): self.assertTrue("end" in channel.json_body) def test_stream_token_is_accepted_for_fwd_pagianation(self) -> None: - token = "s0_0_0_0_0_0_0_0_0_0_0_0_0" + token = "s0_0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/rooms/%s/messages?access_token=x&from=%s" % (self.room_id, token) ) diff --git a/tests/rest/synapse/mas/test_users.py b/tests/rest/synapse/mas/test_users.py index 6f44761bb8..9804f0195d 100644 --- a/tests/rest/synapse/mas/test_users.py +++ b/tests/rest/synapse/mas/test_users.py @@ -17,6 +17,7 @@ from parameterized import parameterized from twisted.internet.testing import MemoryReactor +from synapse.api.constants import ProfileFields from synapse.api.errors import StoreError from synapse.appservice import ApplicationService from synapse.server import HomeServer @@ -54,9 +55,10 @@ class MasQueryUserResource(BaseTestCase): ) ) self.get_success( - store.set_profile_avatar_url( + store.set_profile_field( user_id=alice, - new_avatar_url="mxc://example.com/avatar", + field_name=ProfileFields.AVATAR_URL, + new_value="mxc://example.com/avatar", ) ) @@ -729,7 +731,13 @@ class MasDeleteUserResource(BaseTestCase): store = self.hs.get_datastores().main # Add custom profile field - self.get_success(store.set_profile_field(alice, "io.element.example", "hello")) + self.get_success( + store.set_profile_field( + user_id=alice, + field_name="io.element.example", + new_value="hello", + ) + ) # Ensure we're testing what we think we are: # check the user has profile data at the start of the test diff --git a/tests/storage/test_main.py b/tests/storage/test_main.py index 7b5774b8c1..dc98876930 100644 --- a/tests/storage/test_main.py +++ b/tests/storage/test_main.py @@ -18,8 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # - - +from synapse.api.constants import ProfileFields from synapse.types import UserID from tests import unittest @@ -38,7 +37,11 @@ class DataStoreTestCase(unittest.HomeserverTestCase): self.get_success(self.store.register_user(self.user.to_string(), "pass")) self.get_success(self.store.create_profile(self.user)) self.get_success( - self.store.set_profile_displayname(self.user, self.displayname) + self.store.set_profile_field( + user_id=self.user, + field_name=ProfileFields.DISPLAYNAME, + new_value=self.displayname, + ) ) users, total = self.get_success( diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index dbaf298697..b65af28962 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -21,6 +21,7 @@ from twisted.internet.testing import MemoryReactor +from synapse.api.constants import ProfileFields from synapse.server import HomeServer from synapse.storage.database import LoggingTransaction from synapse.storage.engines import PostgresEngine @@ -39,7 +40,13 @@ class ProfileStoreTestCase(unittest.HomeserverTestCase): def test_displayname(self) -> None: self.get_success(self.store.create_profile(self.u_frank)) - self.get_success(self.store.set_profile_displayname(self.u_frank, "Frank")) + self.get_success( + self.store.set_profile_field( + user_id=self.u_frank, + field_name=ProfileFields.DISPLAYNAME, + new_value="Frank", + ) + ) self.assertEqual( "Frank", @@ -47,7 +54,13 @@ class ProfileStoreTestCase(unittest.HomeserverTestCase): ) # test set to None - self.get_success(self.store.set_profile_displayname(self.u_frank, 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)) @@ -57,7 +70,11 @@ class ProfileStoreTestCase(unittest.HomeserverTestCase): self.get_success(self.store.create_profile(self.u_frank)) self.get_success( - self.store.set_profile_avatar_url(self.u_frank, "http://my.site/here") + self.store.set_profile_field( + user_id=self.u_frank, + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.site/here", + ) ) self.assertEqual( @@ -66,7 +83,13 @@ class ProfileStoreTestCase(unittest.HomeserverTestCase): ) # test set to None - self.get_success(self.store.set_profile_avatar_url(self.u_frank, 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))