From 06c3eec9fd1d0b33d9bfe67e3d90ee0d8b71c2bd Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 11 Aug 2026 14:55:55 +0300 Subject: [PATCH] Fix correctly returning all fields when client doesn't request particular fields --- synapse/handlers/sliding_sync/extensions.py | 20 ++++---- synapse/storage/databases/main/profile.py | 56 ++++++++++++--------- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 516a54a19c..6a74db6817 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -1193,7 +1193,7 @@ class SlidingSyncExtensionHandler: to_id=to_token.profile_updates_key, user_id=user_id, field_names=fields, - field_names_empty_means_all_fields=False if fields else True, + field_names_empty_means_all_fields=True, ) profile_user_ids = set() left_room_user_ids = { @@ -1220,16 +1220,16 @@ class SlidingSyncExtensionHandler: 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 fields and fields) - or update.user_id not in profile_user_ids - ): + if not update.affected_fields: continue - updated_user_fields.setdefault(update.user_id, set()).add(update.field_name) + for field_name in update.affected_fields: + # Skip the update if the client didn't ask for this field, or we're not + # interested in this user. + if ( + field_name not in fields and fields + ) or update.user_id not in profile_user_ids: + continue + updated_user_fields.setdefault(update.user_id, set()).add(field_name) profile_data_by_user = await self.store.get_profile_data_for_users( profile_user_ids diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 28d9e8ce83..2a7ff3fb0f 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -536,25 +536,25 @@ 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( + if field_names_empty_means_all_fields and not field_names: + field_clause = "pu.affected_fields IS NOT NULL" + field_args: list[str] = [] + else: + wanted_field_in_elems_clause, field_args = make_in_list_sql_clause( txn.database_engine, "field_names.value", field_names ) - ) - if isinstance(txn.database_engine, PostgresEngine): - # Note that if we had a GIN index on `affected_fields`, this would defeat it. - # If we decide we want one, we should consider using the `?|` operator or its - # clearer-named `jsonb_exists_any` equivalent. - all_field_names_table_expression = "jsonb_array_elements_text(pu.affected_fields) AS field_names(value)" - else: - # json_each is a table-valued function that gives `value` as one of its column names - all_field_names_table_expression = ( - "json_each(pu.affected_fields) AS field_names" - ) + if isinstance(txn.database_engine, PostgresEngine): + # Note that if we had a GIN index on `affected_fields`, this would defeat it. + # If we decide we want one, we should consider using the `?|` operator or its + # clearer-named `jsonb_exists_any` equivalent. + all_field_names_table_expression = "jsonb_array_elements_text(pu.affected_fields) AS field_names(value)" + else: + # json_each is a table-valued function that gives `value` as one of its column names + all_field_names_table_expression = ( + "json_each(pu.affected_fields) AS field_names" + ) + field_clause = f"(EXISTS (SELECT 1 FROM {all_field_names_table_expression} WHERE {wanted_field_in_elems_clause}))" user_clause = "" user_args: list[str] = [] @@ -579,7 +579,7 @@ class ProfileWorkerStore(SQLBaseStore): AND puf.user_id = ? {user_clause} AND ( - (EXISTS (SELECT 1 FROM {all_field_names_table_expression} WHERE {wanted_field_in_elems_clause})) + {field_clause} OR pu.action != ? ) ORDER BY pu.stream_id ASC @@ -589,7 +589,7 @@ class ProfileWorkerStore(SQLBaseStore): to_id, user_id, *user_args, - *wanted_field_in_elems_args, + *field_args, ProfileUpdateAction.UPDATE.value, ), ) @@ -597,18 +597,24 @@ class ProfileWorkerStore(SQLBaseStore): updates: list[ProfileUpdate] = [] for stream_id, updated_user_id, action, affected_fields_dbjson in rows: + if affected_fields_dbjson is not None: + # Get the field names that were affected by this update + if field_names_empty_means_all_fields and not field_names: + affected_fields = frozenset(db_to_json(affected_fields_dbjson)) + else: + # Only include those that intersect with the field names + # we care about + affected_fields = ( + frozenset(db_to_json(affected_fields_dbjson)) & field_names + ) + else: + affected_fields = None updates.append( ProfileUpdate( stream_id=stream_id, user_id=updated_user_id, action=action, - affected_fields=( - # Get the field names that were affected by this update - # and intersect with the field names we care about - frozenset(db_to_json(affected_fields_dbjson)) & field_names - ) - if affected_fields_dbjson is not None - else None, + affected_fields=affected_fields, ) )