WIP Make fields optional [merge conflicted fix later]

If initial, return all fields. If incremental, return updates.

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:

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

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",
+                    }
+                },
+            )