diff --git a/changelog.d/19556.feature b/changelog.d/19556.feature new file mode 100644 index 00000000000..bcb6c5c983c --- /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/start_for_complement.sh b/docker/complement/conf/start_for_complement.sh index e0d30abed36..9d8fd93334f 100755 --- a/docker/complement/conf/start_for_complement.sh +++ b/docker/complement/conf/start_for_complement.sh @@ -60,12 +60,13 @@ if [[ -n "$SYNAPSE_COMPLEMENT_USE_WORKERS" ]]; then federation_inbound, \ federation_reader, \ federation_sender, \ + profile_updates, \ synchrotron, \ client_reader, \ appservice, \ pusher, \ device_lists:2, \ - stream_writers=account_data+presence+receipts+to_device+typing" + stream_writers=account_data+presence+profile_updates+receipts+to_device+typing" fi log "Workers requested: $SYNAPSE_WORKER_TYPES" diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index e829292aca9..64a36522fa4 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/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 26c8556eff4..895c287ce9d 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -269,7 +269,10 @@ "^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/(join|invite|leave|ban|unban|kick)$", "^/_matrix/client/(api/v1|r0|v3|unstable)/join/", "^/_matrix/client/(api/v1|r0|v3|unstable)/knock/", - "^/_matrix/client/(api/v1|r0|v3|unstable)/profile/", + # The [^/] differentiates this endpoint from + # `ProfileRestFieldsServlet`, which we want to instead go to the + # `profile_updates` worker below. + "^/_matrix/client/(api/v1|r0|v3|unstable)/profile/[^/]+", ], "shared_extra_conf": {}, "worker_extra_conf": "", @@ -308,6 +311,15 @@ "shared_extra_conf": {}, "worker_extra_conf": "", }, + "profile_updates": { + "app": "synapse.app.generic_worker", + "listener_resources": ["client", "replication"], + "endpoint_patterns": [ + "^/_matrix/client/(unstable/uk.tcpip.msc4133|api/v1|r0|v3|unstable)/profile/.+/" + ], + "shared_extra_conf": {}, + "worker_extra_conf": "", + }, "device_lists": { "app": "synapse.app.generic_worker", "listener_resources": ["client", "replication"], @@ -517,6 +529,7 @@ def add_worker_roles_to_shared_config( "typing", "push_rules", "thread_subscriptions", + "profile_updates", } # Worker-type specific sharding config. Now a single worker can fulfill multiple diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 92eca4a7ffd..f1e5595bf53 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -318,6 +318,16 @@ 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. +Note, requires an MSC4429 compatible client, and is currently limited to legacy sync and local users only. 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`. @@ -4486,6 +4496,8 @@ This setting has the following sub-options: * `device_lists` (string): Name of a worker assigned to the `device_lists` stream. +* `profile_updates` (string): Name of a worker assigned to the `profile_updates` stream. + Example configuration: ```yaml stream_writers: diff --git a/docs/workers.md b/docs/workers.md index 92b606607ca..7401a4190b2 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -577,6 +577,7 @@ configured as stream writer for the `quarantined_media_changes` stream: ^/_synapse/admin/v1/quarantine_media/.*$ + #### Restrict outbound federation traffic to a specific set of workers The diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index ef9ecb067ce..b59d61e5f9c 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 max_event_delay_ms: Option, + 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 db8bca82d68..5bf36fa4e9f 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 0e345b7b69d..a3c792d1354 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -235,6 +235,17 @@ 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. + + Note, requires an MSC4429 compatible client, and is currently limited to + legacy sync and local users only. + default: false + examples: + - true allow_public_rooms_without_auth: type: boolean description: @@ -5541,6 +5552,9 @@ properties: device_lists: type: string description: Name of a worker assigned to the `device_lists` stream. + profile_updates: + type: string + description: Name of a worker assigned to the `profile_updates` stream. default: {} examples: - events: worker1 diff --git a/scripts-dev/check_schema_delta.py b/scripts-dev/check_schema_delta.py index 12ed5d258c9..7f2500ec05b 100755 --- a/scripts-dev/check_schema_delta.py +++ b/scripts-dev/check_schema_delta.py @@ -14,6 +14,11 @@ 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 cca87d42a9e..d8e553f446f 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/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 0b8a289d92d..f4f598d27c7 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -917,6 +917,10 @@ def alter_table(txn: LoggingTransaction) -> None: "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 acac0573340..a5ee617344b 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -410,6 +410,56 @@ 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. + These actions are used to determine how profiles, and what data is included in the + sync responses, depending on field updates and room membership changes. + """ + + 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 profile 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 9b47c20437b..cbae9133c66 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -123,6 +123,13 @@ "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 @@ "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 @@ def __init__(self, hs: "HomeServer", filter_json: JsonMapping): self.event_fields = filter_json.get("event_fields", []) self.event_format = filter_json.get("event_format", "client") + self.profile_fields: 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 ca94c224ea5..ffd4ef5ab89 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -545,6 +545,12 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: " 'allow_public_rooms_over_federation' is set." ) + # Whether to support MSC4299 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 fb7378bfc81..a05843d900e 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -142,6 +142,9 @@ 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. + profile_updates: The instances that write to the profile updates stream. quarantined_media_changes: The instances that write to the quarantined media changes stream. """ @@ -179,7 +182,11 @@ 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, + ) + profile_updates: list[str] = attr.ib( + default=[MAIN_PROCESS_INSTANCE_NAME], converter=_instance_to_list_converter, ) quarantined_media_changes: list[str] = attr.ib( @@ -361,8 +368,7 @@ def read_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 +377,9 @@ def read_config( "receipts", "presence", "push_rules", + "device_lists", + "thread_subscriptions", + "profile_updates", ): instances = _instance_to_list_converter(getattr(self.writers, stream)) for instance in instances: @@ -421,6 +430,16 @@ def read_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." + ) + + if len(self.writers.profile_updates) == 0: + raise ConfigError( + "Must specify at least one instance to handle `profile_updates` messages." + ) + self.events_shard_config = RoutableShardedWorkerHandlingConfig( self.writers.events ) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 9ec00d55ad3..7e31abf0e8b 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -172,7 +172,7 @@ async def deactivate_account( # Note that displayname and avatar URL may persist as historical state events # 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( + await self._profile_handler.dispatch_delete_profile_upon_deactivation( user, requester, by_admin ) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index c3886795b66..e55ace6f1b5 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -34,6 +34,11 @@ StoreError, SynapseError, ) +from synapse.replication.http.profile import ( + ReplicationProfileDeleteField, + ReplicationProfileDeleteUponDeactivation, + ReplicationProfileSetField, +) from synapse.storage.databases.main.media_repository import LocalMedia, RemoteMedia from synapse.storage.roommember import ProfileInfo from synapse.types import ( @@ -42,6 +47,7 @@ JsonValue, Requester, ScheduledTask, + StreamKeyType, TaskStatus, UserID, create_requester, @@ -75,6 +81,8 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() # nb must be called this for @cached self.store = hs.get_datastores().main self.hs = hs + self._notifier = hs.get_notifier() + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync self.federation = hs.get_federation_client() hs.get_federation_registry().register_query_handler( @@ -98,6 +106,55 @@ def __init__(self, hs: "HomeServer"): self._update_join_states_task, UPDATE_JOIN_STATES_ACTION_NAME ) self._worker_locks = hs.get_worker_locks_handler() + self._is_profile_worker = ( + hs.get_instance_name() in hs.config.worker.writers.profile_updates + ) + self._delete_profile_field_client = ReplicationProfileDeleteField.make_client( + self.hs + ) + self._delete_profile_upon_deactivation_client = ( + ReplicationProfileDeleteUponDeactivation.make_client(self.hs) + ) + self._set_profile_field_client = ReplicationProfileSetField.make_client(self.hs) + self._profile_updates_writer_instance = ( + self.hs.config.worker.writers.profile_updates[0] + ) + + async def get_targets_for_profile_updates( + self, user_id: UserID + ) -> dict[str, set[str]]: + """ + Get targets for the profile updates stream. + + Collects the rooms and users that are in the same rooms, and thus would be + interested in profile changes of our user. This will always include the user + themselves, even if the user is not in any rooms. + + Args: + user_id: The user ID that is updating a profile + + Returns: + Dictionary containing two sets, one for `rooms`, one for `users` + """ + if not self._msc4429_enabled: + return { + "rooms": set(), + "users": set(), + } + + room_ids = await self.store.get_rooms_for_user(user_id.to_string()) + if not room_ids: + return { + "rooms": set(), + "users": {user_id.to_string()}, + } + + return { + "rooms": set(room_ids), + "users": await self.store.get_local_users_who_share_room_with_user( + user_id.to_string() + ), + } async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDict: """ @@ -191,13 +248,14 @@ async def get_displayname(self, target_user: UserID) -> str | None: async def set_displayname( self, + *, target_user: UserID, requester: Requester, new_displayname: str, - *, + profile_update_target_user_ids: set[str], by_admin: bool = False, propagate: bool = True, - ) -> None: + ) -> int | None: """Set the displayname of a user Preconditions: @@ -207,12 +265,19 @@ async def set_displayname( 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. + profile_update_target_user_ids: User id's to trigger profile updates stream + updates for. 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 that was just inserted. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -252,7 +317,12 @@ async def set_displayname( 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_update_target_user_ids, + ) profile = await self.store.get_profileinfo(target_user) @@ -267,6 +337,8 @@ async def set_displayname( 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 +374,14 @@ async def get_avatar_url(self, target_user: UserID) -> str | None: async def set_avatar_url( self, + *, target_user: UserID, requester: Requester, new_avatar_url: str, - *, + profile_update_target_user_ids: set[str], by_admin: bool = False, propagate: bool = True, - ) -> None: + ) -> int | None: """Set a new avatar URL for a user. Preconditions: @@ -318,12 +391,19 @@ async def set_avatar_url( 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. + profile_update_target_user_ids: User ID's to trigger profile update stream + updates for. 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 that was just inserted. """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this homeserver") @@ -361,7 +441,12 @@ async def set_avatar_url( 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_update_target_user_ids, + ) profile = await self.store.get_profileinfo(target_user) @@ -376,6 +461,120 @@ async def set_avatar_url( if propagate: await self._update_join_states(requester, target_user) + return stream_id + + async def user_left_room(self, user_id: UserID, room_id: str) -> None: + """ + A user left a room. We now: + + * Add a row to `profile_updates` stating that the user left a room. + * Check if this user no longer shares any rooms with certain users. + * Insert a row for each of those users into `profile_updates_per_user`. + * Remove any previous profile update stream rows concerning + this user. This is done to stop leaking any updates to users who no longer + share a room. + * Now, when any of those users sync, the sync code will check + `profile_updates` and see that the user left a room. And thus a "clear + this user's profile" instruction will be sent down to the client. + + If that is the case, + """ + if not self._msc4429_enabled: + return + # We should never be called without being a profile worker + assert self._is_profile_worker + + user_id_str = user_id.to_string() + + users_in_left_room = set(await self.store.get_local_users_in_room(room_id)) + users_in_left_room.discard(user_id_str) + if not users_in_left_room: + return + + users_still_sharing_rooms = await self.store.do_users_share_a_room( + user_id_str, users_in_left_room + ) + + users_to_update = users_in_left_room - users_still_sharing_rooms + if users_to_update: + stream_id = await self.store.record_profile_updates_for_user_left_room( + user_id=user_id, + users_to_update=users_to_update, + ) + if stream_id: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=set(room_id), + ) + + async def user_joined_room(self, user_id: UserID, room_id: str) -> None: + """ + A user joined a room. We now: + + * Add a row to `profile_updates` stating that the user joined a room. + * Get list of users in that room. + * Insert a row for each of those users into `profile_updates_per_user`. + * Now, when any of those users sync, the sync code will check + `profile_updates` and see that the user joined a room. Thus, we can + include the users full profile in the case that we need to do so. + + If that is the case, + """ + if not self._msc4429_enabled: + return + # We should never be called without being a profile worker + assert self._is_profile_worker + + user_id_str = user_id.to_string() + + users_in_room = set(await self.store.get_local_users_in_room(room_id)) + users_in_room.discard(user_id_str) + if not users_in_room: + return + + if users_in_room: + stream_id = await self.store.record_profile_updates_for_user_joined_room( + user_id=user_id, + users_to_update=users_in_room, + ) + if stream_id: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=set(room_id), + ) + + async def dispatch_delete_profile_upon_deactivation( + self, + target_user: UserID, + requester: Requester, + by_admin: bool = False, + ) -> None: + """ + Dispatch profile deletion upon deactivation to the right instance that + can write to profiles, or handle it ourselves, if we're a profile worker. + + Args: + target_user: User ID who'se profile is being deactivated. + requester: The requesting user. + by_admin: Whether the action is being done by an admin. + """ + if self._is_profile_worker: + await self.delete_profile_upon_deactivation( + target_user=target_user, + requester=requester, + by_admin=by_admin, + ) + else: + # Offload to the right worker via http replication + await self._delete_profile_upon_deactivation_client( + instance_name=self._profile_updates_writer_instance, + user_id=target_user.to_string(), + requester=requester, + by_admin=by_admin, + ) + async def delete_profile_upon_deactivation( self, target_user: UserID, @@ -406,7 +605,40 @@ async def delete_profile_upon_deactivation( # have it. raise AuthError(400, "Cannot remove another user's profile") - await self.store.delete_profile(target_user) + profile_updates: list[str] = [] + profile_update_targets: dict = {"rooms": set(), "users": set()} + + if self._msc4429_enabled: + current_profile = await self.store.get_profileinfo(target_user) + + if current_profile.display_name is not None: + profile_updates.append(ProfileFields.DISPLAYNAME) + if current_profile.avatar_url is not None: + profile_updates.append(ProfileFields.AVATAR_URL) + + custom_fields = await self.store.get_profile_fields(target_user) + for field_name in custom_fields.keys(): + profile_updates.append(field_name) + + if profile_updates: + profile_update_targets = await self.get_targets_for_profile_updates( + target_user + ) + # Discard ourselves as we're deactivating this account + profile_update_targets["users"].discard(target_user.to_string()) + + # Record the profile delete and update the profile updates stream + stream_id = await self.store.delete_profile( + user_id=target_user, + field_names=profile_updates, + target_users=profile_update_targets["users"], + ) + if stream_id and profile_update_targets["rooms"]: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=profile_update_targets["rooms"], + ) await self._third_party_rules.on_profile_update( target_user.to_string(), @@ -415,6 +647,50 @@ async def delete_profile_upon_deactivation( 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 = False, + ) -> 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_profile_worker: + 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 +768,7 @@ async def check_avatar_size_and_mime_type(self, mxc: str) -> bool: 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 +806,72 @@ async def get_profile_field( 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.""" + profile_update_targets = await self.get_targets_for_profile_updates(target_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, + profile_update_target_user_ids=profile_update_targets["users"], + ) + 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, + profile_update_target_user_ids=profile_update_targets["users"], + ) + 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, + profile_update_target_user_ids=profile_update_targets["users"], + ) + + if stream_id and profile_update_targets["rooms"]: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=profile_update_targets["rooms"], + ) + + async def set_profile_field( + self, + *, + target_user: UserID, + requester: Requester, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + profile_update_target_user_ids: set[str], + by_admin: bool = False, + ) -> int | None: """Set a new profile field for a user. Preconditions: @@ -546,11 +879,15 @@ async def set_profile_field( 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. field_name: The name of the profile field to update. new_value: The new field value for this user. + profile_update_target_user_ids: User ID's to trigger profile update stream + updates for. by_admin: Whether this change was made by an administrator. """ if not self.hs.is_mine(target_user): @@ -559,7 +896,12 @@ async def set_profile_field( 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, + profile_update_target_user_ids, + ) # Custom fields do not propagate into the user directory *or* rooms. profile = await self.store.get_profileinfo(target_user) @@ -567,6 +909,48 @@ async def set_profile_field( 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_profile_worker: + 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 +966,8 @@ async def delete_profile_field( 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 +980,10 @@ async def delete_profile_field( 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) + profile_update_targets = await self.get_targets_for_profile_updates(target_user) + stream_id = await self.store.delete_profile_field( + target_user, field_name, profile_update_targets["users"] + ) # Custom fields do not propagate into the user directory *or* rooms. profile = await self.store.get_profileinfo(target_user) @@ -602,6 +991,13 @@ async def delete_profile_field( target_user.to_string(), profile, by_admin, deactivation=False ) + if stream_id: + self._notifier.on_new_event( + StreamKeyType.PROFILE_UPDATES, + stream_id, + rooms=profile_update_targets["rooms"], + ) + 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 5152d0b522e..61279d88bb3 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -53,6 +53,7 @@ from synapse.logging import opentracing from synapse.logging.opentracing import SynapseTags, set_tag, tag_args, trace from synapse.metrics import SERVER_NAME_LABEL, event_processing_positions +from synapse.replication.http.profile import ReplicationProfileUserRoomMembershipChange from synapse.replication.http.push import ReplicationCopyPusherRestServlet from synapse.storage.databases.main.state_deltas import StateDelta from synapse.storage.invite_rule import InviteRule @@ -205,6 +206,16 @@ def __init__(self, hs: "HomeServer"): ) self._push_writer = hs.config.worker.writers.push_rules[0] self._copy_push_client = ReplicationCopyPusherRestServlet.make_client(hs) + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._is_profile_worker = ( + hs.get_instance_name() in hs.config.worker.writers.profile_updates + ) + self._profile_updates_writer_instance = ( + self.hs.config.worker.writers.profile_updates[0] + ) + self._profile_user_room_membership_change_client = ( + ReplicationProfileUserRoomMembershipChange.make_client(self.hs) + ) def _on_user_joined_room(self, event_id: str, room_id: str) -> None: """Notify the rate limiter that a room join has occurred. @@ -526,6 +537,45 @@ async def _local_membership_update( ) if prev_member_event.membership == Membership.JOIN: await self._user_left_room(target, room_id) + if self._msc4429_enabled: + # Notify the profile handler. We only want to do this once + # in a multi-worker setup, so we can't listen on the dispatched + # event above. + if self._is_profile_worker: + await self.profile_handler.user_left_room( + target, room_id + ) + else: + # Offload to the right worker via http replication + await self._profile_user_room_membership_change_client( + instance_name=self._profile_updates_writer_instance, + user_id=target.to_string(), + room_id=room_id, + membership=Membership.LEAVE, + ) + + elif self._msc4429_enabled and event.membership == Membership.JOIN: + prev_member_event = None + if prev_member_event_id: + prev_member_event = await self.store.get_event( + prev_member_event_id + ) + if ( + not prev_member_event + or prev_member_event.membership == Membership.LEAVE + ): + # Notify the profile handler. We only want to do this once + # in a multi-worker setup, so we can't dispatch a hook to all workers. + if self._is_profile_worker: + await self.profile_handler.user_joined_room(target, room_id) + else: + # Offload to the right worker via http replication + await self._profile_user_room_membership_change_client( + instance_name=self._profile_updates_writer_instance, + user_id=target.to_string(), + room_id=room_id, + membership=Membership.JOIN, + ) break except PartialStateConflictError as e: @@ -1529,18 +1579,54 @@ async def send_membership_event( ratelimit=ratelimit, ) - if event.membership == Membership.LEAVE: - prev_state_ids = await context.get_prev_state_ids( - StateFilter.from_types([(EventTypes.Member, event.state_key)]) - ) - prev_member_event_id = prev_state_ids.get( - (EventTypes.Member, event.state_key), None - ) + prev_state_ids = await context.get_prev_state_ids( + StateFilter.from_types([(EventTypes.Member, event.state_key)]) + ) + prev_member_event_id = prev_state_ids.get( + (EventTypes.Member, event.state_key), None + ) + if event.membership == Membership.LEAVE: if prev_member_event_id: prev_member_event = await self.store.get_event(prev_member_event_id) if prev_member_event.membership == Membership.JOIN: await self._user_left_room(target_user, room_id) + if self._msc4429_enabled: + # Notify the profile handler. We only want to do this once + # in a multi-worker setup, so we can't listen on the dispatched + # event above. + if self._is_profile_worker: + await self.profile_handler.user_left_room( + target_user, room_id + ) + else: + # Offload to the right worker via http replication + await self._profile_user_room_membership_change_client( + instance_name=self._profile_updates_writer_instance, + user_id=target_user.to_string(), + room_id=room_id, + membership=Membership.LEAVE, + ) + elif self._msc4429_enabled and event.membership == Membership.JOIN: + prev_member_event = None + if prev_member_event_id: + prev_member_event = await self.store.get_event(prev_member_event_id) + if ( + not prev_member_event + or prev_member_event.membership == Membership.LEAVE + ): + # Notify the profile handler. We only want to do this once + # in a multi-worker setup, so we can't dispatch a hook to all workers. + if self._is_profile_worker: + await self.profile_handler.user_joined_room(target_user, room_id) + else: + # Offload to the right worker via http replication + await self._profile_user_room_membership_change_client( + instance_name=self._profile_updates_writer_instance, + user_id=target_user.to_string(), + room_id=room_id, + membership=Membership.JOIN, + ) async def _can_guest_join(self, partial_current_state_ids: StateMap[str]) -> bool: """ diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index bb5ca329e00..f9d94757115 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -530,10 +530,11 @@ async def complete_sso_login_request( 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 @@ def is_allowed_mime_type(content_type: str) -> bool: ) # 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 9ecfe0da0f2..2706a817098 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -18,6 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # +import hashlib import itertools import logging from typing import ( @@ -37,6 +38,7 @@ EventContentFields, EventTypes, Membership, + ProfileUpdateAction, StickyEvent, ) from synapse.api.filtering import FilterCollection @@ -64,6 +66,7 @@ DeviceListUpdates, JsonDict, JsonMapping, + JsonValue, MultiWriterStreamToken, MutableStateMap, Requester, @@ -104,10 +107,18 @@ # 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 + SyncRequestKey = tuple[Any, ...] @@ -224,6 +235,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 +251,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 +274,7 @@ def __bool__(self) -> bool: or self.knocked or self.archived or self.account_data + or self.profile_updates or self.to_device or self.device_lists ) @@ -275,6 +290,7 @@ def empty( next_batch=next_batch, presence=[], account_data=[], + profile_updates={}, joined=[], invited=[], knocked=[], @@ -291,6 +307,7 @@ def __init__(self, hs: "HomeServer"): 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 +346,30 @@ def __init__(self, hs: "HomeServer"): max_len=0, expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE, ) + # ExpiringCache((User, Device)) + # -> LruCache( + # sha256(Other User ID + Field Name + Field value) -> bool + # ) + self.lazy_loaded_profile_fields_cache: ExpiringCache[ + tuple[str, str | None], LruCache[str, bool] + ] = 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 combination + of the user + field name + value, all hashed into sha256, an existing value + indicating the field has recently been sent. The boolean value does not hold + other significance. A missing cache entry means "we have not sent this user + + field name + value combo to the syncing user". + + 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 @@ -1036,6 +1077,34 @@ def get_lazy_loaded_members_cache( 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[str, bool]: + """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 combination + of the user + field name + value, all hashed into sha256, an existing value + indicating the field has recently been sent. The boolean value does not hold + other significance. A missing cache entry means "we have not sent this user + + field name + value combo to the syncing user". + + 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. + """ + cache: LruCache[str, bool] | 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, @@ -1853,10 +1922,18 @@ async def generate_sync_result( } ) + # 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, @@ -2121,6 +2198,254 @@ async def _generate_sync_entry_for_account_data( 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: + # Skip the update if there is no field update (a joined or left room + # action), the client didn't ask for this field, or we're not + # interested in this user. + if ( + not update.field_name + or update.field_name not in profile_fields + or update.user_id not in users + ): + continue + updated_user_fields.setdefault(update.user_id, set()).add( + update.field_name + ) + + # 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) + + 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 value to check against is an md5 of a string of + # user ID + field name + value, which ensures if the value + # changes, we'll miss the cache, thus sending the field update + # to the syncing user. + cache_value = hashlib.sha256( + f"{other_user_id}-{field_name}-{profile_data.get(field_name)}".encode( + "utf8" + ) + ).hexdigest() + if cache.get(cache_value) is None: + 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, + True, + ) + 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", @@ -3137,6 +3462,7 @@ class SyncResultBuilder: # The following mirror the fields in a sync response presence account_data + profile_updates joined invited knocked @@ -3155,6 +3481,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 11312530285..16dc168d84b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -2025,10 +2025,11 @@ async def set_displayname( 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 6a057ac09fa..e24d0ef5a25 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -528,6 +528,7 @@ def on_new_event( 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 68cc6ce1fc6..d934ef80678 100644 --- a/synapse/replication/http/__init__.py +++ b/synapse/replication/http/__init__.py @@ -30,6 +30,7 @@ login, membership, presence, + profile, push, register, send_events, @@ -59,6 +60,7 @@ def register_servlets(self, hs: "HomeServer") -> None: 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 00000000000..62b87e242cb --- /dev/null +++ b/synapse/replication/http/profile.py @@ -0,0 +1,266 @@ +# +# 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.api.constants import Membership +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 ReplicationProfileUserRoomMembershipChange(ReplicationEndpoint): + """Store user profile update action regarding membership changes. + + The POST looks like: + + POST /_synapse/replication/profile_user_room_membership_change/ + + { + "room_id": "!1234:domain.tld", + "membership": "join | leave" + } + + 200 OK + + {} + """ + + NAME = "profile_user_room_membership_change" + 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, + room_id: str, + membership: str, + ) -> JsonDict: + assert membership in (Membership.JOIN, Membership.LEAVE) + return { + "room_id": room_id, + "membership": membership, + } + + async def _handle_request( # type: ignore[override] + self, request: Request, content: JsonDict, user_id: str + ) -> tuple[int, JsonDict]: + assert content["membership"] in (Membership.JOIN, Membership.LEAVE) + if content["membership"] == Membership.JOIN: + await self._profile_handler.user_joined_room( + user_id=UserID.from_string(user_id), + room_id=content["room_id"], + ) + else: + await self._profile_handler.user_left_room( + user_id=UserID.from_string(user_id), + room_id=content["room_id"], + ) + + return (200, {}) + + +class ReplicationProfileSetField(ReplicationEndpoint): + """Update a profile field for a user. + + The POST looks like: + + POST /_synapse/replication/profile_set_field/ + + { + "target_user": "@user:hs", + "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 { + "target_user": user_id, + "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/ + + { + "target_user": "@user:hs", + "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 { + "target_user": user_id, + "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, {}) + + +class ReplicationProfileDeleteUponDeactivation(ReplicationEndpoint): + """Delete a profile for a user during deactivation. + + The POST looks like: + + POST /_synapse/replication/profile_delete_upon_deactivation/ + + { + "target_user": "@user:hs", + "requester": "@admin:hs", + "by_admin": true + } + + 200 OK + + {} + """ + + NAME = "profile_delete_upon_deactivation" + 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, + by_admin: bool, + ) -> JsonDict: + return { + "target_user": user_id, + "requester": requester.user.to_string(), + "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_upon_deactivation( + target_user=UserID.from_string(user_id), + requester=create_requester(content["requester"]), + by_admin=content["by_admin"], + ) + + return (200, {}) + + +def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: + ReplicationProfileUserRoomMembershipChange(hs).register(http_server) + ReplicationProfileSetField(hs).register(http_server) + ReplicationProfileDeleteField(hs).register(http_server) + ReplicationProfileDeleteUponDeactivation(hs).register(http_server) diff --git a/synapse/replication/tcp/client.py b/synapse/replication/tcp/client.py index bc7e46d4c92..00a24db749b 100644 --- a/synapse/replication/tcp/client.py +++ b/synapse/replication/tcp/client.py @@ -44,6 +44,7 @@ UnPartialStatedRoomStream, ) from synapse.replication.tcp.streams._base import ( + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, ) @@ -265,6 +266,23 @@ async def on_rdata( 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 users room ID's 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 ad9fed72dd8..65befb77064 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -67,6 +67,7 @@ ) from synapse.replication.tcp.streams._base import ( DeviceListsStream, + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, ) @@ -218,6 +219,12 @@ def __init__(self, hs: "HomeServer"): continue + if isinstance(stream, ProfileUpdatesStream): + if hs.get_instance_name() in hs.config.worker.writers.profile_updates: + 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 e41573cf689..e657822da70 100644 --- a/synapse/replication/tcp/streams/__init__.py +++ b/synapse/replication/tcp/streams/__init__.py @@ -37,6 +37,7 @@ DeviceListsStream, PresenceFederationStream, PresenceStream, + ProfileUpdatesStream, PushersStream, PushRulesStream, QuarantinedMediaStream, @@ -70,6 +71,7 @@ ToDeviceStream, FederationStream, AccountDataStream, + ProfileUpdatesStream, StickyEventsStream, ThreadSubscriptionsStream, UnPartialStatedRoomStream, @@ -94,6 +96,7 @@ "ToDeviceStream", "FederationStream", "AccountDataStream", + "ProfileUpdatesStream", "StickyEventsStream", "ThreadSubscriptionsStream", "UnPartialStatedRoomStream", diff --git a/synapse/replication/tcp/streams/_base.py b/synapse/replication/tcp/streams/_base.py index a73f767add2..86242b23413 100644 --- a/synapse/replication/tcp/streams/_base.py +++ b/synapse/replication/tcp/streams/_base.py @@ -31,8 +31,9 @@ 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,57 @@ async def _update_function( return rows, rows[-1][0], len(updates) == limit +@attr.s(slots=True, auto_attribs=True) +class ProfileUpdatesStreamRow: + """Stream to inform workers about profile updates.""" + + 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 constant. + """ + field_name: str | None + """The profile field that was updated, see https://spec.matrix.org/unstable/client-server-api/#profiles. + This can be None if `action` is not 'update'. + """ + + +class ProfileUpdatesStream(_StreamFromIdGen): + """A user profile field was changed.""" + + 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, field_name), + ) + for stream_id, user_id, action, field_name 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 1bd48e18cc8..edcb585295c 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -28,7 +28,7 @@ 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 @@ async def on_PUT( 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 @@ async def on_PUT( ) 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 @@ async def on_PUT( ) 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 c2ec5b36114..dc632822c1f 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -58,7 +58,7 @@ def _read_propagate(hs: "HomeServer", request: SynapseRequest) -> bool: class ProfileRestServlet(RestServlet): - PATTERNS = client_patterns("/profile/(?P[^/]*)", v1=True) + PATTERNS = client_patterns("/profile/(?P[^/]*)$", v1=True) CATEGORY = "Event sending requests" def __init__(self, hs: "HomeServer"): @@ -109,6 +109,9 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.profile_handler = hs.get_profile_handler() self.auth = hs.get_auth() + self._is_profile_worker = ( + hs.get_instance_name() in hs.config.worker.writers.profile_updates + ) if hs.config.experimental.msc4133_enabled: self.PATTERNS.append( re.compile( @@ -146,7 +149,9 @@ async def on_GET( 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 +209,14 @@ async def on_PUT( 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 +262,21 @@ async def on_DELETE( 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 +289,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 962317dedbe..af302ea7e34 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -123,6 +123,7 @@ def __init__(self, hs: "HomeServer"): 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,12 @@ async def encode_response( if sync_result.to_device: response["to_device"] = {"events": sync_result.to_device} + if self._msc4429_enabled and sync_result.profile_updates: + 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 01db41bcfac..cc5717b103d 100644 --- a/synapse/rest/synapse/mas/users.py +++ b/synapse/rest/synapse/mas/users.py @@ -19,6 +19,7 @@ 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 @@ async def _async_render_POST( ) 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 @@ async def _async_render_POST( 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 @@ async def _async_render_POST( 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 @@ async def _async_render_POST( 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/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 68548434a95..36dc6d9f612 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -19,13 +19,15 @@ # # import json -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING, Collection, Iterable, cast +import attr from canonicaljson import encode_canonical_json -from synapse.api.constants import ProfileFields +from synapse.api.constants import 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, make_in_list_sql_clause from synapse.storage.database import ( DatabasePool, LoggingDatabaseConnection, @@ -33,6 +35,7 @@ ) 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 if TYPE_CHECKING: @@ -43,6 +46,16 @@ 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 + field_name: str | None + + class ProfileWorkerStore(SQLBaseStore): def __init__( self, @@ -52,6 +65,7 @@ def __init__( ): 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 +79,24 @@ def __init__( "populate_full_user_id_profiles", self.populate_full_user_id_profiles ) + self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._can_write_to_profile_updates = ( + self._instance_name in hs.config.worker.writers.profile_updates + ) + 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.profile_updates, + ) + async def populate_full_user_id_profiles( self, progress: JsonDict, batch_size: int ) -> int: @@ -152,6 +184,13 @@ def _final_batch(txn: LoggingTransaction, lower_bound_id: str) -> None: 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 +249,9 @@ async def get_profile_avatar_url(self, user_id: UserID) -> str | None: 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,7 +263,9 @@ async def get_profile_field(self, user_id: UserID, field_name: str) -> JsonValue 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}"' @@ -240,7 +283,9 @@ def get_profile_field(txn: LoggingTransaction) -> JsonValue: # 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 @@ -257,7 +302,9 @@ def get_profile_field(txn: LoggingTransaction) -> JsonValue: ) # 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. @@ -291,6 +338,248 @@ async def get_profile_fields(self, user_id: UserID) -> dict[str, str]: 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, 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, str | None]]: + sql = """ + SELECT + stream_id, user_id, action, field_name + FROM profile_updates + WHERE + ? < stream_id AND stream_id <= ? + ORDER BY stream_id ASC LIMIT ? + """ + txn.execute(sql, (from_id, to_id, limit)) + return cast(list[tuple[int, str, str, str | None]], txn.fetchall()) + + 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: Iterable[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 + """ + if from_id >= to_id: + return [] + + field_names = list(field_names) + if not field_names: + return [] + + def _get_profile_updates_for_fields_txn( + txn: LoggingTransaction, + ) -> list[ProfileUpdate]: + clause, args = make_in_list_sql_clause( + txn.database_engine, "field_name", field_names + ) + sql = ( + "SELECT stream_id, user_id, action, field_name" + " FROM profile_updates" + f" WHERE ? < stream_id AND stream_id <= ? AND ({clause}" + " OR action != ?) " + " ORDER BY stream_id ASC" + ) + txn.execute(sql, (from_id, to_id, *args, ProfileUpdateAction.UPDATE.value)) + rows = cast(list[tuple[int, str, str, str | None]], txn.fetchall()) + + updates: list[ProfileUpdate] = [] + for stream_id, user_id, action, field_name in rows: + updates.append( + ProfileUpdate( + stream_id=stream_id, + user_id=user_id, + action=action, + field_name=field_name, + ) + ) + + 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]: + field_clause, field_args = make_in_list_sql_clause( + txn.database_engine, "pu.field_name", 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. + sql = f""" + SELECT pu.stream_id, pu.user_id, pu.action, pu.field_name + 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 ({field_clause} OR pu.action != ?) + ORDER BY pu.stream_id ASC + """ + + txn.execute( + sql, + ( + from_id, + to_id, + user_id, + *user_args, + *field_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, field_name in rows: + updates.append( + ProfileUpdate( + stream_id=stream_id, + user_id=updated_user_id, + action=action, + field_name=field_name, + ) + ) + + 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 automatically convert JSON to + # Python objects + 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 +601,7 @@ def _check_profile_size( 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,89 +661,46 @@ def _check_profile_size( 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], + target_users: set[str], + ) -> 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. - """ - user_localpart = user_id.localpart + 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 + target_users: Users to trigger a profile update stream row for - 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.db_pool.simple_upsert_txn( - txn, - table="profiles", - keyvalues={"user_id": user_localpart}, - values={ - "displayname": new_displayname, - "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. + Returns: + The profile updates stream ID that was created in this transaction """ - user_localpart = user_id.localpart + if self._msc4429_enabled: + assert self._can_write_to_profile_updates - 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._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={ - "avatar_url": new_avatar_url, + field_name: new_value, "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 @@ -462,10 +708,10 @@ def set_profile_field(txn: LoggingTransaction) -> None: # 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 - """ + 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, @@ -482,10 +728,10 @@ def set_profile_field(txn: LoggingTransaction) -> None: # 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(?)) - """ + 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}"' @@ -502,9 +748,139 @@ def set_profile_field(txn: LoggingTransaction) -> None: ), ) - await self.db_pool.runInteraction("set_profile_field", set_profile_field) + if not self._msc4429_enabled: + return 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], + target_users=target_users, + ) + + return stream_id - async def delete_profile_field(self, user_id: UserID, field_name: str) -> None: + def _record_profile_updates_txn( + self, + *, + txn: LoggingTransaction, + user_id: UserID, + action: ProfileUpdateAction, + field_names: list[str] | None, + target_users: set[str], + ) -> int | None: + """ + Record updates into the profile updates stream tables. + + 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 + target_users: Set of users to create profile update stream rows for + + 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 + + # Record the profile update + inserted_ts = self.clock.time_msec() + if field_names: + stream_ids = self._profile_updates_id_gen.get_next_mult_txn( + txn, len(field_names) + ) + values: list[tuple[int, str, str, str, str | None, int]] = [ + ( + stream_id, + self._instance_name, + user_id.to_string(), + action.value, + field_name, + inserted_ts, + ) + for stream_id, field_name in zip(stream_ids, field_names) + ] + else: + stream_ids = [self._profile_updates_id_gen.get_next_txn(txn)] + values = [ + ( + stream_ids[0], + self._instance_name, + user_id.to_string(), + action.value, + None, + inserted_ts, + ) + ] + self.db_pool.simple_insert_many_txn( + txn, + table="profile_updates", + keys=[ + "stream_id", + "instance_name", + "user_id", + "action", + "field_name", + "inserted_ts", + ], + values=values, + ) + + # Add per user tracking rows pointing to the latest stream ID + inserted_ts = self.clock.time_msec() + per_user_values = [ + (stream_ids[-1], user_id, inserted_ts) for user_id in target_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_ids[-1] + + async def set_profile_field( + self, + user_id: UserID, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], + target_users: set[str], + ) -> 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. + target_users: Set of users to trigger profile updates for. + """ + return await self.db_pool.runInteraction( + "set_profile_field", + self._set_profile_field_txn, + user_id, + field_name, + new_value, + target_users, + ) + + async def delete_profile_field( + self, user_id: UserID, field_name: str, target_users: set[str] + ) -> int | None: """ Remove a custom profile field for a user. @@ -513,7 +889,10 @@ async def delete_profile_field(self, user_id: UserID, field_name: str) -> None: field_name: The name of the custom profile field. """ - def delete_profile_field(txn: LoggingTransaction) -> None: + if self._msc4429_enabled: + assert self._can_write_to_profile_updates + + def delete_profile_field(txn: LoggingTransaction) -> int | None: if isinstance(self.database_engine, PostgresEngine): sql = """ UPDATE profiles SET fields = fields - ? @@ -534,18 +913,238 @@ def delete_profile_field(txn: LoggingTransaction) -> None: (f'$."{field_name}"', user_id.localpart), ) - await self.db_pool.runInteraction("delete_profile_field", delete_profile_field) + if not self._msc4429_enabled: + return None + + stream_id = self._record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.UPDATE, + field_names=[field_name], + target_users=target_users, + ) + return stream_id + + return await self.db_pool.runInteraction( + "delete_profile_field", delete_profile_field + ) - async def delete_profile(self, user_id: UserID) -> None: + async def delete_profile( + self, + user_id: UserID, + field_names: list[str], + target_users: set[str], + ) -> int | 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. + field_names: List of profile fields the user had. + target_users: List of users who share rooms and are interested in + profile updates for this user. + + Returns: + Latest stream ID for the profile updates stream. """ - 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) -> int | None: + # Delete the profile + txn.execute( + """ + DELETE FROM profiles + WHERE full_user_id = ? + """, + (user_id.to_string(),), + ) + # Update the profile updates stream + stream_id = self._record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.UPDATE, + field_names=field_names, + target_users=target_users, + ) + return stream_id + + return await self.db_pool.runInteraction( + "delete_profile", + _delete_profile_txn, + ) + + async def record_profile_updates_for_user_left_room( + self, + user_id: UserID, + users_to_update: set[str], + ) -> int | None: + """ + Record updates into the profile updates stream for when a user leaves + the last room with a set of users. + + Doing this clears all old rows from the `profile_updates_per_user` table, + to avoid exposing any profile field changes past the point of not being + in any common rooms with the user. + + Args: + user_id: The user who left the last common room with a set of users. + users_to_update: The set of users who no longer share rooms with the user. + + Returns: + Stream ID for the profile update stream update. + """ + if not users_to_update: + return None + assert self._can_write_to_profile_updates + + def _record_profile_updates_for_user_left_room_txn( + txn: LoggingTransaction, + ) -> int | None: + # First clear the previous rows from the table + txn.execute( + """ + SELECT stream_id FROM profile_updates + WHERE user_id = ? + """, + (user_id.to_string(),), + ) + res = txn.fetchall() + if res: + stream_ids = [row[0] for row in res] + + user_clause, user_args = make_in_list_sql_clause( + txn.database_engine, + "user_id", + users_to_update, + ) + stream_id_clause, stream_id_args = make_in_list_sql_clause( + txn.database_engine, + "stream_id", + stream_ids, + ) + txn.execute( + f""" + DELETE FROM profile_updates_per_user + WHERE {user_clause} + AND {stream_id_clause} + """, + (*user_args, *stream_id_args), + ) + + # Now record the "left room" action in the stream + stream_id = self._record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.LEFT_ROOM, + field_names=[], + target_users=users_to_update, + ) + return stream_id + + return await self.db_pool.runInteraction( + "record_profile_updates_for_user_left_room", + _record_profile_updates_for_user_left_room_txn, + ) + + async def record_profile_updates_for_user_joined_room( + self, + user_id: UserID, + users_to_update: set[str], + ) -> int | None: + """ + Record updates into the profile updates stream for when a user joins a room. + + Args: + user_id: The user who joined a room. + users_to_update: A set of the other users in the room. + + Returns: + Stream ID for the profile update stream update. + """ + if not users_to_update: + return None + assert self._can_write_to_profile_updates + + def _record_profile_updates_for_user_joined_room_txn( + txn: LoggingTransaction, + ) -> int | None: + stream_id = self._record_profile_updates_txn( + txn=txn, + user_id=user_id, + action=ProfileUpdateAction.JOINED_ROOM, + field_names=[], + target_users=users_to_update, + ) + return stream_id + + return await self.db_pool.runInteraction( + "record_profile_updates_for_user_joined_room", + _record_profile_updates_for_user_joined_room_txn, + ) + + async def clear_profile_updates_for_user( + self, user_id: UserID, users_to_remove: set[str] + ) -> None: + """ + Clear all the ProfileUpdateAction rows from the + `profile_updates_per_user` table from a particular user for + a list of target users. This could be needed for example to ensure when + a user leaves the last shared room with another user, that we clear all + the old updates to ensure we don't leak any field updates to the other user. + + This does not remove the stream ID row from `profile_updates` as it is + likely other per user rows may refer to it. Our automatic pruning of old + stream ID's will kick in later and clean up potential orphan `profile_updates` + table rows. + + Args: + user_id: The user's ID. + users_to_remove: List of users to remove per user rows for. + + Returns: + None + """ + assert self._can_write_to_profile_updates + if not users_to_remove: + return + + def _clear_profile_updates_for_user_txn( + txn: LoggingTransaction, + ) -> None: + sql = """ + SELECT stream_id FROM profile_updates + WHERE user_id = ? + """ + + txn.execute(sql, (user_id.to_string(),)) + res = txn.fetchall() + if not res: + return + + stream_ids = [row[0] for row in res] + + user_clause, user_args = make_in_list_sql_clause( + txn.database_engine, + "user_id", + users_to_remove, + ) + stream_id_clause, stream_id_args = make_in_list_sql_clause( + txn.database_engine, + "stream_id", + stream_ids, + ) + txn.execute( + f""" + DELETE FROM profile_updates_per_user + WHERE {user_clause} + AND {stream_id_clause} + """, + (*user_args, *stream_id_args), + ) + + await self.db_pool.runInteraction( + "clear_profile_updates_for_user", + _clear_profile_updates_for_user_txn, ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 736f3e4c781..8c69266d510 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -982,6 +982,20 @@ async def get_users_who_share_room_with_user(self, user_id: str) -> set[str]: 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 1afc6d0b2a6..3495dce866a 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -175,6 +175,7 @@ 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 00000000000..b641a965362 --- /dev/null +++ b/synapse/storage/schema/main/delta/94/07_profile_updates.sql @@ -0,0 +1,52 @@ +-- +-- 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. +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, + + -- Profile field name that has been updated, + -- see https://spec.matrix.org/unstable/client-server-api/#profiles + -- This is only required if "action" is "update" + field_name TEXT NULL, + + -- Unix timestamp for debugging purposes + inserted_ts BIGINT NOT NULL +); + +CREATE INDEX IF NOT EXISTS profile_updates_by_user ON profile_updates (user_id, stream_id); +CREATE INDEX IF NOT EXISTS profile_updates_by_field ON profile_updates (field_name, stream_id); + +-- Track which local users should receive each profile update. +CREATE TABLE IF NOT EXISTS profile_updates_per_user ( + id $%AUTO_INCREMENT_PRIMARY_KEY%$, + + -- 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. Used to determine when to cull rows (to prevent the table + -- from growing indefinitely). + inserted_ts BIGINT NOT NULL +); + +CREATE INDEX IF NOT EXISTS profile_updates_per_user_by_user_stream ON profile_updates_per_user (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 00000000000..9abf79b68de --- /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 36490fcb355..24120eb7362 100644 --- a/synapse/streams/events.py +++ b/synapse/streams/events.py @@ -86,6 +86,7 @@ def get_current_token(self) -> StreamToken: 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 @@ def get_current_token(self) -> StreamToken: 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 @@ async def bound_future_token(self, token: StreamToken) -> StreamToken: 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 a6fc806701d..ef99d575546 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -1095,6 +1095,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) @@ -1102,7 +1103,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. @@ -1118,6 +1119,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: @@ -1181,6 +1183,7 @@ class StreamToken: quarantined_media_key: MultiWriterStreamToken = attr.ib( validator=attr.validators.instance_of(MultiWriterStreamToken) ) + profile_updates_key: int _SEPARATOR = "_" START: ClassVar["StreamToken"] @@ -1211,6 +1214,7 @@ async def from_string(cls, store: "DataStore", string: str) -> "StreamToken": thread_subscriptions_key, sticky_events_key, quarantined_media_key, + profile_updates_key, ) = keys return cls( @@ -1231,6 +1235,7 @@ async def from_string(cls, store: "DataStore", string: str) -> "StreamToken": quarantined_media_key=await MultiWriterStreamToken.parse( store, quarantined_media_key ), + profile_updates_key=int(profile_updates_key), ) except CancelledError: raise @@ -1256,6 +1261,7 @@ async def to_string(self, store: "DataStore") -> str: str(self.thread_subscriptions_key), str(self.sticky_events_key), await self.quarantined_media_key.to_string(store), + str(self.profile_updates_key), ] ) @@ -1329,6 +1335,7 @@ def get_field( StreamKeyType.UN_PARTIAL_STATED_ROOMS, StreamKeyType.THREAD_SUBSCRIPTIONS, StreamKeyType.STICKY_EVENTS, + StreamKeyType.PROFILE_UPDATES, ], ) -> int: ... @@ -1384,9 +1391,10 @@ def __str__(self) -> str: 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})" ) @@ -1404,6 +1412,7 @@ def __str__(self) -> str: 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 f8b4098c714..fe5c3ff67fe 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 @@ def test_reactivation_recreates_profile(self) -> None: # 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 @@ def test_reactivation_without_erasure_keeps_profile(self) -> None: """ 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 561b45827fd..9a876f5aefc 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -26,12 +26,13 @@ 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.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 @@ -62,8 +63,10 @@ def register_query_handler( 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 +86,17 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: 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 +104,11 @@ def test_get_my_name(self) -> None: 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 +119,11 @@ def test_set_my_name(self) -> None: # 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 +134,11 @@ def test_set_my_name(self) -> None: # 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 +150,11 @@ def test_update_room_membership_on_set_displayname(self) -> None: """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 +172,11 @@ def test_update_room_membership_on_set_displayname(self) -> None: 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 +187,540 @@ def test_update_room_membership_on_set_displayname(self) -> None: ) 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, + field_name=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", + field_name=None, + ), + ProfileUpdate( + stream_id=4, + user_id=self.frank.to_string(), + action="update", + field_name="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", + field_name="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", + field_name=None, + ), + ProfileUpdate( + stream_id=3, + user_id="@millie:test", + action="joined_room", + field_name=None, + ), + ProfileUpdate( + stream_id=4, + user_id=self.frank.to_string(), + action="update", + field_name="m.status", + ), + ], + ) + + @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", + field_name=None, + ), + ProfileUpdate( + stream_id=5, + user_id=self.frank.to_string(), + action="update", + field_name="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", + field_name="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", + field_name=None, + ), + ProfileUpdate( + stream_id=6, + user_id=self.frank.to_string(), + action="left_room", + field_name=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", + field_name=None, + ), + ProfileUpdate( + stream_id=7, + user_id="@gracie:test", + action="left_room", + field_name=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", + field_name="m.status", + ), + ], + ) + + @override_config({"include_profile_updates_in_sync": True}) + def test_display_name_change_does_not_generate_join_action_rows_for_profile_updates( + self, + ) -> None: + """Test that changing ones display name doesn't generate "joined_room" + profile updates to the stream, due to room state membership event changes, + if MSC4429 is enabled. + """ + 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={"displayname"}, + ) + ) + # Starting situation + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=2, + user_id="@roger:test", + action="joined_room", + field_name=None, + ), + ], + ) + + # Change rogers display name + self.get_success( + self.handler.set_field( + target_user=UserID.from_string("@roger:test"), + requester=synapse.types.create_requester("@roger:test"), + field_name="displayname", + new_value="Roger new name", + ) + ) + + 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={"displayname"}, + ) + ) + # Only the displayname change should appear, no extra "room join" due to + # membership state event changes. + self.assertEqual( + per_user_updates, + [ + ProfileUpdate( + stream_id=2, + user_id="@roger:test", + action="joined_room", + field_name=None, + ), + ProfileUpdate( + stream_id=3, + user_id="@roger:test", + action="update", + field_name="displayname", + ), + ], + ) + + @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 +741,11 @@ async def slow_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: ): 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 +772,11 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: """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 +813,11 @@ async def potentially_slow_update_membership( ): 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 +883,14 @@ async def potentially_slow_update_membership( @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", + target_users=set(), + ) + ) self.assertEqual( (self.get_success(self.store.get_profile_displayname(self.frank))), @@ -329,16 +899,22 @@ def test_set_my_name_if_disabled(self) -> None: # 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 +937,11 @@ def test_incoming_fed_query(self) -> None: 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 +959,33 @@ def test_incoming_fed_query(self) -> None: 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, + target_users=set(), + ) + ) + self.get_success( + self.store.set_profile_field( + user_id=self.frank, + field_name=ProfileFields.AVATAR_URL, + new_value="http://my.server/me.png", + target_users=set(), + ) ) profile = self.get_success(self.handler.get_profile(self.frank.to_string())) @@ -398,10 +994,11 @@ def test_get_profile_empty_displayname(self) -> None: 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 +1009,11 @@ def test_set_my_avatar(self) -> None: # 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 +1024,11 @@ def test_set_my_avatar(self) -> None: # 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 +1040,12 @@ def test_set_my_avatar(self) -> None: 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 +1055,11 @@ def test_set_my_avatar_if_disabled(self) -> None: # 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 0db7f30b1f8..182ff7a8fc3 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -25,7 +25,7 @@ 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 @@ async def get_or_create_user( 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 d2b2523321b..49e1682c706 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.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.server import HomeServer from synapse.types import ( JsonDict, + JsonValue, MultiWriterStreamToken, RoomStreamToken, StreamKeyType, @@ -55,6 +57,8 @@ 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,1398 @@ 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": "🏊"}, + target_users=set(), + ) + ) + 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( + 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. + """ + # 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), + ) + ) + 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), + ) + ) + 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, + ) + + 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 c4e4170c6f9..4deb3c29f41 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -2549,7 +2549,7 @@ def test_timestamp_to_event(self) -> None: 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 @@ def test_topo_token_is_accepted(self) -> None: 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 bce199c564f..14be9389ab5 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -39,6 +39,7 @@ EventContentFields, EventTypes, LoginType, + ProfileFields, UserTypes, ) from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError @@ -943,18 +944,27 @@ def test_order_by(self) -> None: # 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", + target_users=set(), ) ) 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", + target_users=set(), ) ) 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", + target_users=set(), ) ) @@ -1546,8 +1556,11 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: # 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", + target_users=set(), ) ) self.get_success( @@ -1679,7 +1692,12 @@ def test_deactivate_user_erase_true_avatar_nonnull_but_empty(self) -> None: """ # 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="", + target_users=set(), + ) ) # Check we can still erase them. @@ -2758,8 +2776,11 @@ def test_deactivate_user(self) -> None: # 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", + target_users=set(), ) ) self.get_success( diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index 158d7f33f0d..86fab2fe254 100644 --- a/tests/rest/client/test_account.py +++ b/tests/rest/client/test_account.py @@ -30,7 +30,7 @@ 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 @@ -522,13 +522,19 @@ def test_deactivate_erase_account(self) -> None: # 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 @@ -580,9 +586,21 @@ def test_deactivate_erase_account_with_disabled_profile_changes(self) -> None: # 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", + target_users=set(), + ) + ) + self.get_success( + store.set_profile_field( + user_id=user_id, + field_name=ProfileFields.AVATAR_URL, + new_value="http://test/Kermit.jpg", + target_users=set(), + ) ) # Verify it is set diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 8d4892ae91f..793a66f3434 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2248,7 +2248,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: 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) ) @@ -2259,7 +2259,7 @@ def test_topo_token_is_accepted(self) -> None: 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 6f44761bb8b..be6ff52fcc8 100644 --- a/tests/rest/synapse/mas/test_users.py +++ b/tests/rest/synapse/mas/test_users.py @@ -17,6 +17,7 @@ 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,11 @@ def test_query_user(self) -> None: ) ) 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", + target_users=set(), ) ) @@ -729,7 +732,14 @@ def test_user_deactivation_removes_custom_profile_fields(self) -> None: 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", + target_users=set(), + ) + ) # 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 7b5774b8c1c..235e4ed6783 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,12 @@ def test_get_users_paginate(self) -> None: 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, + target_users=set(), + ) ) users, total = self.get_success( diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index dbaf2986975..c099841267a 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,14 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: 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", + target_users=set(), + ) + ) self.assertEqual( "Frank", @@ -47,7 +55,14 @@ def test_displayname(self) -> None: ) # 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, + target_users=set(), + ) + ) self.assertIsNone( self.get_success(self.store.get_profile_displayname(self.u_frank)) @@ -57,7 +72,12 @@ def test_avatar_url(self) -> None: 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", + target_users=set(), + ) ) self.assertEqual( @@ -66,7 +86,14 @@ def test_avatar_url(self) -> None: ) # 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, + target_users=set(), + ) + ) self.assertIsNone( self.get_success(self.store.get_profile_avatar_url(self.u_frank))