diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index 64a36522fa..4dc4eb932b 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -15,7 +15,7 @@ enable_registration_without_verification: true bcrypt_rounds: 4 url_preview_enabled: true url_preview_ip_range_blacklist: [] -# MSC4429 Profile updates down legacy /sync +# MSC4429 and MSC4262 Profile updates down sync include_profile_updates_in_sync: true ## Registration ## diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 7654039e5c..00a31ff447 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -339,7 +339,7 @@ include_profile_data_on_invite: false ### `include_profile_updates_in_sync` *(boolean)* Use this option to include updates of other users' profiles in sync responses, for users who share rooms. -Requires an [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) compatible client, and is currently limited to legacy sync and local users only. +For legacy sync clients, requires [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) compatibility. For sliding sync clients, requires [MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262) compatibility. Note, profile updates via sync are currently limited to local users only. This feature is under development and should be used with caution on busy servers or servers which depend on `limit_profile_requests_to_users_who_share_rooms` for ensuring profile information doesn't leak across rooms. Defaults to `false`. Example configuration: diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 6a6210fcb2..a8d1719288 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -281,9 +281,11 @@ properties: Use this option to include updates of other users' profiles in sync responses, for users who share rooms. - Requires an [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) - compatible client, and is currently limited to legacy sync and local users only. - + For legacy sync clients, requires [MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429) + compatibility. For sliding sync clients, requires + [MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262) compatibility. Note, profile updates + via sync are currently limited to local users only. + This feature is under development and should be used with caution on busy servers or servers which depend on `limit_profile_requests_to_users_who_share_rooms` for ensuring profile information doesn't leak across rooms. diff --git a/synapse/config/server.py b/synapse/config/server.py index 42d43ea7f6..00b414ea30 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -585,7 +585,7 @@ class ServerConfig(Config): " 'allow_public_rooms_over_federation' is set." ) - # Whether to support MSC4429 profile updates down legacy /sync + # Whether to support MSC4429 and MSC4262 Profile updates down sync self.include_profile_updates_in_sync = config.get( "include_profile_updates_in_sync", False, diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 98efa02952..3d052bc893 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -106,7 +106,9 @@ class ProfileHandler: self._worker_locks = hs.get_worker_locks_handler() # Profile updates stream - self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._include_profile_updates_in_sync = ( + hs.config.server.include_profile_updates_in_sync + ) self._is_events_writer = ( hs.get_instance_name() in hs.config.worker.writers.events ) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 1514af8b79..08002a6708 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -129,7 +129,9 @@ class SyncRestServlet(RestServlet): self._event_serializer = hs.get_event_client_serializer() self._msc2654_enabled = hs.config.experimental.msc2654_enabled self._msc3773_enabled = hs.config.experimental.msc3773_enabled - self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._include_profile_updates_in_sync = ( + hs.config.server.include_profile_updates_in_sync + ) self._json_filter_cache: LruCache[str, bool] = LruCache( max_size=1000, @@ -358,7 +360,7 @@ class SyncRestServlet(RestServlet): if sync_result.to_device: response["to_device"] = {"events": sync_result.to_device} - if self._msc4429_enabled and sync_result.profile_updates: + if self._include_profile_updates_in_sync and sync_result.profile_updates: # FIXME: See issue https://github.com/element-hq/synapse/issues/19981 # for concerns around the current implementation of the profile # updates stream. diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 8211612e2d..35f387576f 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -269,7 +269,9 @@ class PersistEventsStore: self._clock = hs.get_clock() self._instance_name = hs.get_instance_name() self._msc4354_enabled = hs.config.experimental.msc4354_enabled - self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._include_profile_updates_in_sync = ( + hs.config.server.include_profile_updates_in_sync + ) self._ephemeral_messages_enabled = hs.config.server.enable_ephemeral_messages self.is_mine_id = hs.is_mine_id @@ -2121,7 +2123,7 @@ class PersistEventsStore: txn, {m for m in members_to_cache_bust if not self.hs.is_mine_id(m)} ) - if self._msc4429_enabled: + if self._include_profile_updates_in_sync: # Handle changes to the profile updates stream. # We've already done a bunch of work calculating the changes needed # for the sliding sync tables, so we may as well re-use that information diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index dbae6e4e8b..d32fe5cebf 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -86,7 +86,9 @@ class ProfileWorkerStore(SQLBaseStore): "populate_full_user_id_profiles", self.populate_full_user_id_profiles ) - self._msc4429_enabled = hs.config.server.include_profile_updates_in_sync + self._include_profile_updates_in_sync = ( + hs.config.server.include_profile_updates_in_sync + ) self._is_events_writer = self._instance_name in hs.config.worker.writers.events self._profile_updates_id_gen: MultiWriterIdGenerator = MultiWriterIdGenerator( db_conn=db_conn, @@ -764,7 +766,7 @@ class ProfileWorkerStore(SQLBaseStore): Returns: The profile updates stream ID that was created in this transaction """ - if self._msc4429_enabled: + if self._include_profile_updates_in_sync: assert self._is_events_writer self._check_profile_size(txn, user_id, field_name, new_value) @@ -827,7 +829,7 @@ class ProfileWorkerStore(SQLBaseStore): ), ) - if not self._msc4429_enabled: + if not self._include_profile_updates_in_sync: return None # Record updates in the profile updates stream @@ -856,7 +858,7 @@ class ProfileWorkerStore(SQLBaseStore): users profile should be pushed to the client, should they need it already even if the user hasn't actually joined the room. """ - if not self._msc4429_enabled: + if not self._include_profile_updates_in_sync: return assert self._is_events_writer @@ -904,7 +906,7 @@ class ProfileWorkerStore(SQLBaseStore): Returns: The latest stream ID created in this transaction """ - if not self._msc4429_enabled: + if not self._include_profile_updates_in_sync: return None if action == ProfileUpdateAction.UPDATE: @@ -1019,7 +1021,7 @@ class ProfileWorkerStore(SQLBaseStore): field_name: The name of the custom profile field. """ - if self._msc4429_enabled: + if self._include_profile_updates_in_sync: assert self._is_events_writer def delete_profile_field(txn: LoggingTransaction) -> int | None: @@ -1041,7 +1043,7 @@ class ProfileWorkerStore(SQLBaseStore): (f'$."{field_name}"', user_id.localpart), ) - if not self._msc4429_enabled: + if not self._include_profile_updates_in_sync: return None stream_id = self.record_profile_updates_txn( diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 3495dce866..0d9022116d 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -175,7 +175,7 @@ Changes in SCHEMA_VERSION = 93 Changes in SCHEMA_VERSION = 94 - Add `recheck` column (boolean, default true) to the `redactions` table. - MSC4242: Add state DAG tables. - - MSC4429: Track updates to user profile fields via a new stream. + - MSC4429/MSC4262: 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 index 720f958e09..e053421be3 100644 --- a/synapse/storage/schema/main/delta/94/07_profile_updates.sql +++ b/synapse/storage/schema/main/delta/94/07_profile_updates.sql @@ -12,7 +12,7 @@ -- . -- Track updates to profile fields. --- For MSC4429 legacy /sync and others. +-- For MSC4429 and MSC4262 down sync and others. -- See https://github.com/element-hq/synapse/issues/19981 for potential future directions of this table. CREATE TABLE IF NOT EXISTS profile_updates ( stream_id BIGINT NOT NULL PRIMARY KEY, diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 4b66be0102..590c7e62f5 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -199,13 +199,13 @@ class ProfileTestCase(unittest.HomeserverTestCase): ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], ] ) - def test_update_profile_does_not_update_stream_on_set_field_if_msc4429_not_enabled( + def test_update_profile_does_not_update_stream_on_set_field_if_include_profile_updates_in_sync_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.""" + if `include_profile_updates_in_sync` is not enabled.""" self.get_success( self.handler.set_field( target_user=self.frank, @@ -230,13 +230,13 @@ class ProfileTestCase(unittest.HomeserverTestCase): ["m.status", '{"text": "Holiday", "emoji": "🏖"}'], ] ) - def test_update_profile_does_not_notify_notifier_on_set_field_if_msc4429_not_enabled( + def test_update_profile_does_not_notify_notifier_on_set_field_if_include_profile_updates_in_sync_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.""" + to wake up if `include_profile_updates_in_sync` is not enabled.""" self.get_success( self.handler.set_field( target_user=self.frank, @@ -265,7 +265,8 @@ class ProfileTestCase(unittest.HomeserverTestCase): 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.""" + to wake up if the user is not in any rooms, if `include_profile_updates_in_sync` + is enabled.""" self.get_success( self.handler.set_field( target_user=self.frank, @@ -293,7 +294,7 @@ class ProfileTestCase(unittest.HomeserverTestCase): self, field_name: str, new_value: str ) -> None: """Test that profile updates get recorded in the profile updates stream if - MSC4429 is enabled.""" + `include_profile_updates_in_sync` is enabled.""" self.get_success( self.handler.set_field( target_user=self.frank, @@ -363,7 +364,7 @@ class ProfileTestCase(unittest.HomeserverTestCase): self, ) -> None: """Test that profiles updates get recorded in the 'per user' profile updates - stream tracking table, if MSC4429 is enabled.""" + stream tracking table, if `include_profile_updates_in_sync` is enabled.""" self.register_user("roger", "password") roger_token = self.login("roger", "password") self.register_user("millie", "password") @@ -503,7 +504,8 @@ class ProfileTestCase(unittest.HomeserverTestCase): 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. + the user no longer shares rooms with another user, if + `include_profile_updates_in_sync` 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 @@ -670,7 +672,7 @@ class ProfileTestCase(unittest.HomeserverTestCase): new_value: str, ) -> None: """Test that profile updates wake up the profile updates stream on profile - field updates, if MSC4429 is enabled.""" + field updates, if `include_profile_updates_in_sync` is enabled.""" self.helper.create_room_as( room_creator=self.frank.to_string(), tok=self.frank_token, diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index bfb687a6c7..5771e60533 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -1187,8 +1187,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): ) 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.""" + """Test that without `include_profile_updates_in_sync` 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), @@ -1212,8 +1212,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): @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.""" + """Test that with `include_profile_updates_in_sync` 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), @@ -1240,9 +1240,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): @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.""" + """Test that with `include_profile_updates_in_sync` 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), @@ -1304,8 +1304,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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.""" + """Test that with `include_profile_updates_in_sync` 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( @@ -1347,8 +1347,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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 @@ -1424,8 +1424,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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.""" + """Test that with `include_profile_updates_in_sync` 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( @@ -1499,9 +1499,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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") @@ -1652,7 +1652,7 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): is_initial: bool, is_lazy: bool, ) -> None: - """Test that with MSC4429 enabled any sync response + """Test that with `include_profile_updates_in_sync` 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 @@ -1742,8 +1742,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): is_initial: bool, is_lazy: bool, ) -> None: - """Test that with MSC4429 enabled any sync response always contains the users - own updates. + """Test that with `include_profile_updates_in_sync` 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. @@ -1830,8 +1830,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): is_initial: bool, is_lazy: bool, ) -> None: - """Test that with MSC4429 enabled a sync response correctly includes falsey - profile field values. + """Test that with `include_profile_updates_in_sync` enabled a sync response + correctly includes falsey profile field values. """ requester = create_requester(self.user) filter_json: dict[str, dict] = { @@ -1911,8 +1911,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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( @@ -2062,8 +2063,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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( @@ -2112,9 +2113,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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( @@ -2211,8 +2212,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): ) @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.""" + """Test that with `include_profile_updates_in_sync` 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"]} @@ -2275,8 +2276,8 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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. @@ -2440,9 +2441,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): 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. + """Test that with `include_profile_updates_in_sync` 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. @@ -2559,9 +2560,9 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase): def test_lazy_loading_cache_and_multiple_updates_to_the_same_field( self, ) -> None: - """Test that with MSC4429 enabled the incremental lazy sync response - includes an update to a field, even when the value changes back to a - value set and cached previously. + """Test that with `include_profile_updates_in_sync` enabled the incremental + lazy sync response includes an update to a field, even when the value changes + back to a value set and cached previously. """ requester = create_requester(self.user) filter_json = {