diff --git a/changelog.d/20170.feature b/changelog.d/20170.feature new file mode 100644 index 00000000000..aa271eb45fe --- /dev/null +++ b/changelog.d/20170.feature @@ -0,0 +1 @@ +Stabilise [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): support the stable `use_state_after` query parameter and `state_after` response field on `/sync`, as per Matrix v1.16, and remove the `msc4222_enabled` experimental config flag. diff --git a/docker/complement/conf/workers-shared-extra.yaml.j2 b/docker/complement/conf/workers-shared-extra.yaml.j2 index 4dc4eb932b7..6018c1adadd 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -141,8 +141,6 @@ experimental_features: msc4306_enabled: true # Sticky Events msc4354_enabled: true - # `/sync` `state_after` - msc4222_enabled: true server_notices: system_mxid_localpart: _server diff --git a/docs/admin_api/experimental_features.md b/docs/admin_api/experimental_features.md index e32728e56d3..ef1b58c9ba0 100644 --- a/docs/admin_api/experimental_features.md +++ b/docs/admin_api/experimental_features.md @@ -5,7 +5,6 @@ basis. The currently supported features are: - [MSC3881](https://github.com/matrix-org/matrix-spec-proposals/pull/3881): enable remotely toggling push notifications for another client - [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575): enable experimental sliding sync support -- [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): adding `state_after` to sync v2 To use it, you will need to authenticate by providing an `access_token` for a server admin: see [Admin API](../usage/administration/admin_api/). diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index db001971072..aee7a41aa4c 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -70,7 +70,6 @@ pub struct ExperimentalConfig { pub msc4306_enabled: bool, pub msc4169_enabled: bool, pub msc4354_enabled: bool, - pub msc4222_enabled: bool, pub msc4491_enabled: bool, pub msc4143_enabled: bool, pub msc4446_enabled: bool, diff --git a/rust/src/storage/store.rs b/rust/src/storage/store.rs index b339d7748c5..c099d6f5011 100644 --- a/rust/src/storage/store.rs +++ b/rust/src/storage/store.rs @@ -27,8 +27,6 @@ pub enum PerUserExperimentalFeature { MSC3881, #[serde(rename = "msc3575")] MSC3575, - #[serde(rename = "msc4222")] - MSC4222, } impl std::fmt::Display for PerUserExperimentalFeature { diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 1c2f021322b..54ca2af1a4b 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -256,9 +256,6 @@ def read_config( # MSC4210: Remove legacy mentions self.msc4210_enabled: bool = experimental.get("msc4210_enabled", False) - # MSC4222: Adding `state_after` to sync v2 - self.msc4222_enabled: bool = experimental.get("msc4222_enabled", False) - # MSC4076: Add `disable_badge_count`` to pusher configuration self.msc4076_enabled: bool = experimental.get("msc4076_enabled", False) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 049726a97e9..9ff00caf364 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -139,6 +139,14 @@ class SyncConfig: is_guest: bool device_id: str | None use_state_after: bool + # Whether the client opted in to `state_after` via the unstable + # `org.matrix.msc4222.use_state_after` query parameter rather than the + # stable one, in which case the unstable response field name is used too. + # Only relevant when `use_state_after` is True. + # + # FIXME(unstable_state_after): Remove support for the unstable identifiers after 2027-09-01 + # (to allow some time for the ecosystem to adapt to the stable identifiers) + use_unstable_state_after_name: bool = False @attr.s(slots=True, frozen=True, auto_attribs=True) diff --git a/synapse/rest/admin/experimental_features.py b/synapse/rest/admin/experimental_features.py index c91c5b6a495..ae0e60ad3eb 100644 --- a/synapse/rest/admin/experimental_features.py +++ b/synapse/rest/admin/experimental_features.py @@ -43,15 +43,12 @@ class ExperimentalFeature(str, Enum): MSC3881 = "msc3881" MSC3575 = "msc3575" - MSC4222 = "msc4222" def is_globally_enabled(self, config: "HomeServerConfig") -> bool: if self is ExperimentalFeature.MSC3881: return config.experimental.msc3881_enabled if self is ExperimentalFeature.MSC3575: return config.experimental.msc3575_enabled - if self is ExperimentalFeature.MSC4222: - return config.experimental.msc4222_enabled assert_never(self) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 08002a6708a..3d658d9211d 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -174,11 +174,12 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: filter_id = parse_string(request, "filter") full_state = parse_boolean(request, "full_state", default=False) - use_state_after = False - if await self.store.is_feature_enabled( - user.to_string(), ExperimentalFeature.MSC4222 - ): - use_state_after = parse_boolean( + use_state_after = parse_boolean(request, "use_state_after", default=False) + use_unstable_state_after_name = False + # FIXME(unstable_state_after): Remove support for the unstable identifiers after 2027-09-01 + # (to allow some time for the ecosystem to adapt to the stable identifiers) + if not use_state_after: + use_state_after = use_unstable_state_after_name = parse_boolean( request, "org.matrix.msc4222.use_state_after", default=False ) @@ -215,6 +216,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: device_id, last_ignore_accdata_streampos, use_state_after, + use_unstable_state_after_name, ) if filter_id is None: @@ -252,6 +254,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: is_guest=requester.is_guest, device_id=device_id, use_state_after=use_state_after, + use_unstable_state_after_name=use_unstable_state_after_name, ) since_token = None @@ -633,7 +636,12 @@ async def encode_room( # We either include a `state` or `state_after` field depending on # whether the client has opted in to the newer `state_after` behavior. if sync_config.use_state_after: - state_key_name = "org.matrix.msc4222.state_after" + # Clients which opted in via the unstable MSC4222 query parameter + # get the unstable field name back, for the transition period. + if sync_config.use_unstable_state_after_name: + state_key_name = "org.matrix.msc4222.state_after" + else: + state_key_name = "state_after" else: state_key_name = "state" diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 039aea4d781..f942a2acab1 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -33,7 +33,6 @@ ReceiptTypes, RelationTypes, ) -from synapse.rest.admin.experimental_features import ExperimentalFeature from synapse.rest.client import devices, knock, login, read_marker, receipts, room, sync from synapse.server import HomeServer from synapse.types import JsonDict @@ -767,6 +766,70 @@ def test_noop_sync_does_not_tightloop(self) -> None: self.assertEqual(channel.code, 200, channel.json_body) +# FIXME(unstable_state_after): Remove this whole test case after 2027-09-01 +# and we drop support for the unstable variant of `state_after` +class SyncStateAfterTestCase(unittest.HomeserverTestCase): + """ + Tests for the `use_state_after` opt-in on `/sync` (MSC4222, stable as of + Matrix v1.16): the variant of the `state_after` field in the response should + match the stable/unstable query parameter the client opted in with. + """ + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + sync.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.user_id = self.register_user("user", "password") + self.access_token = self.login(self.user_id, "password") + self.room_id = self.helper.create_room_as(self.user_id, tok=self.access_token) + + def _sync_room_keys(self, query_string: str) -> set[str]: + """Perform a sync with the given query string and return the keys of + our room's response object.""" + channel = self.make_request( + "GET", + f"/sync{query_string}", + access_token=self.access_token, + ) + self.assertEqual(channel.code, 200, channel.json_body) + return set(channel.json_body["rooms"]["join"][self.room_id].keys()) + + def test_state_without_opt_in(self) -> None: + """By default, room state comes down under the `state` key.""" + room_keys = self._sync_room_keys("") + self.assertIn("state", room_keys) + self.assertNotIn("state_after", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + def test_state_after_stable_name(self) -> None: + """Opting in with the stable query parameter gives the stable field name.""" + room_keys = self._sync_room_keys("?use_state_after=true") + self.assertIn("state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + def test_state_after_unstable_name(self) -> None: + """Opting in with the unstable query parameter gives the unstable field + name, for the transition period.""" + room_keys = self._sync_room_keys("?org.matrix.msc4222.use_state_after=true") + self.assertIn("org.matrix.msc4222.state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("state_after", room_keys) + + def test_state_after_both_names(self) -> None: + """If a client opts in with both query parameters, the stable field name wins.""" + room_keys = self._sync_room_keys( + "?use_state_after=true&org.matrix.msc4222.use_state_after=true" + ) + self.assertIn("state_after", room_keys) + self.assertNotIn("state", room_keys) + self.assertNotIn("org.matrix.msc4222.state_after", room_keys) + + class DeviceListSyncTestCase(unittest.HomeserverTestCase): """ Tests regarding device list (`device_lists`) changes. @@ -1286,9 +1349,6 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): sync.register_servlets, ] - def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: - self.store = hs.get_datastores().main - def test_archived_room_state_after_not_newer_than_leave(self) -> None: """`state_after` for a left room must be the state at the end of that room's timeline, i.e. at the user's leave point — never state from @@ -1305,11 +1365,6 @@ def test_archived_room_state_after_not_newer_than_leave(self) -> None: bob = self.register_user("bob", "password") bob_tok = self.login("bob", "password") - # Opt Alice in to MSC4222. - self.get_success( - self.store.set_features_for_user(alice, {ExperimentalFeature.MSC4222: True}) - ) - # Name the room to avoid heroes: those come from the *current* # summary — a separate leak path from the one under test. room_id = self.helper.create_room_as( @@ -1338,7 +1393,7 @@ def test_archived_room_state_after_not_newer_than_leave(self) -> None: } } ) - sync_url = f"/sync?filter={sync_filter}&org.matrix.msc4222.use_state_after=true" + sync_url = f"/sync?filter={sync_filter}&use_state_after=true" # Initial sync. channel = self.make_request("GET", sync_url, access_token=alice_tok) @@ -1368,7 +1423,7 @@ def test_archived_room_state_after_not_newer_than_leave(self) -> None: self.assertEqual(channel.code, 200, channel.result) left_room = channel.json_body["rooms"]["leave"][room_id] - state_after_events = left_room["org.matrix.msc4222.state_after"]["events"] + state_after_events = left_room["state_after"]["events"] # Post-leave state must not appear in `state_after`. self.assertNotIn(