diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 5805ccfd55..34d96df8ba 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -1113,9 +1113,13 @@ class SlidingSyncExtensionHandler: # in initial sync. continue per_user_updates: dict[str, JsonValue | dict[str, JsonValue]] = {} - for field_name in fields: - if field_name in profile_data.keys(): - per_user_updates[field_name] = profile_data[field_name] + # Include the fields the client asked for, or all, if not specified + if fields: + user_fields = set(profile_data.keys()).intersection(fields) + else: + user_fields = set(profile_data.keys()) + for field_name in user_fields: + per_user_updates[field_name] = profile_data[field_name] if per_user_updates: response[profile_user_id] = { @@ -1150,12 +1154,7 @@ class SlidingSyncExtensionHandler: return None user_id = sync_config.user.to_string() - - if not profiles_request.fields: - return SlidingSyncResult.Extensions.ProfilesExtension( - users={}, - ) - fields = set(profiles_request.fields) + fields = set(profiles_request.fields) if profiles_request.fields else set() response: dict[str, JsonDict | None] = {} @@ -1173,7 +1172,8 @@ class SlidingSyncExtensionHandler: from_id=from_token.stream_token.profile_updates_key, to_id=to_token.profile_updates_key, user_id=user_id, - field_names=set(fields), + field_names=fields, + field_names_empty_means_all_fields=False if fields else True, ) profile_user_ids = set() updated_users = { @@ -1192,7 +1192,7 @@ class SlidingSyncExtensionHandler: # interested in this user. if ( not update.field_name - or update.field_name not in fields + or (update.field_name not in fields and fields) or update.user_id not in profile_user_ids ): continue @@ -1219,9 +1219,12 @@ class SlidingSyncExtensionHandler: if is_lazy: # TODO lazy cache - # Include all the fields the client asked for - fields = set(profile_data.keys()).intersection(fields) - for field_name in fields: + # Include the fields the client asked for, or all, if not specified + if fields: + user_fields = set(profile_data.keys()).intersection(fields) + else: + user_fields = set(profile_data.keys()) + for field_name in user_fields: per_user_updates[field_name] = profile_data.get(field_name) else: # Include only the diff, unless the user recently joined, @@ -1229,14 +1232,14 @@ class SlidingSyncExtensionHandler: # 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 = ( + user_fields = ( fields # TODO joined_room_user_ids if profile_user_id in [] else set(updated_user_fields.get(profile_user_id, [])) ) - fields = set(profile_data.keys()).intersection(fields) - for field_name in fields: + user_fields = set(profile_data.keys()).intersection(user_fields) + for field_name in user_fields: per_user_updates[field_name] = profile_data[field_name] if per_user_updates: diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 05faad5b26..28d9e8ce83 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -501,6 +501,7 @@ class ProfileWorkerStore(SQLBaseStore): to_id: int, user_id: str, field_names: Set[str], + field_names_empty_means_all_fields: bool = False, include_users: set[str] | None = None, ) -> list[ProfileUpdate]: """Get profile update markers for a user in a stream range. @@ -515,6 +516,8 @@ class ProfileWorkerStore(SQLBaseStore): 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. + field_names_empty_means_all_fields: If `field_names` is empty and this + is `True`, include all field names. Defaults to `False`. include_users: If given, only include updates for these user IDs. Returns: @@ -523,7 +526,7 @@ class ProfileWorkerStore(SQLBaseStore): if from_id >= to_id: return [] - if len(field_names) == 0: + if not field_names_empty_means_all_fields and len(field_names) == 0: return [] if include_users is not None and len(include_users) == 0: @@ -533,6 +536,9 @@ class ProfileWorkerStore(SQLBaseStore): def _get_profile_updates_for_user_and_fields_txn( txn: LoggingTransaction, ) -> list[ProfileUpdate]: + # TODO merge conflict, handle + # https://github.com/element-hq/synapse/pull/20003/commits/88f1990a39c05eb8833057a63e432c5815dc15f6 + # ie optional fields wanted_field_in_elems_clause, wanted_field_in_elems_args = ( make_in_list_sql_clause( txn.database_engine, "field_names.value", field_names diff --git a/tests/rest/client/sliding_sync/test_extension_profiles.py b/tests/rest/client/sliding_sync/test_extension_profiles.py index 5fe98872ee..829af6daf4 100644 --- a/tests/rest/client/sliding_sync/test_extension_profiles.py +++ b/tests/rest/client/sliding_sync/test_extension_profiles.py @@ -271,7 +271,10 @@ class SlidingSyncProfilesTestCase(SlidingSyncBase): } response_body, from_token = self.do_sync(sync_body, tok=self.tok) if is_initial: - response_body["extensions"].get("org.matrix.msc4262.profiles") + # Nothing returned since we didn't ask for the updated field + self.assertIsNone( + response_body["extensions"].get("org.matrix.msc4262.profiles") + ) if not is_initial: self.get_success( @@ -284,5 +287,76 @@ class SlidingSyncProfilesTestCase(SlidingSyncBase): ) # Make an incremental Sliding Sync request response_body, _ = self.do_sync(sync_body, since=from_token, tok=self.tok) + # Nothing returned since we didn't ask for the updated field + self.assertIsNone( + response_body["extensions"].get("org.matrix.msc4262.profiles") + ) - response_body["extensions"].get("org.matrix.msc4262.profiles") + @parameterized.expand( + [ + True, + False, + ] + ) + @override_config({"include_profile_updates_in_sync": True}) + def test_all_fields_returned_if_no_fields_specified(self, is_initial: bool) -> None: + """ + Test that profile extension response returns all profile fields if we didn't + request any particular fields in initial and incremental sync. + """ + 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="field", + new_value="value", + ) + ) + # Make an initial Sliding Sync request with the profiles extension enabled + sync_body = { + "lists": {}, + "extensions": { + "org.matrix.msc4262.profiles": { + "enabled": True, + }, + }, + } + response_body, from_token = self.do_sync(sync_body, tok=self.tok) + if is_initial: + # As this is an initial sync, we get all profile fields + self.assertEqual( + response_body["extensions"]["org.matrix.msc4262.profiles"]["users"][ + "@other_user:test" + ], + { + "updated": { + "avatar_url": None, + "displayname": "other_user", + "field": "value", + } + }, + ) + + if not 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="field", + new_value="value", + ) + ) + # Make an incremental Sliding Sync request + response_body, _ = self.do_sync(sync_body, since=from_token, tok=self.tok) + # As this is an incremental sync, we only get actual updates back + self.assertEqual( + response_body["extensions"]["org.matrix.msc4262.profiles"]["users"][ + "@other_user:test" + ], + { + "updated": { + "field": "value", + } + }, + )