diff --git a/changelog.d/20170.feature b/changelog.d/20170.feature new file mode 100644 index 0000000000..aa271eb45f --- /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 251018e1d2..3cf8615b75 100644 --- a/docker/complement/conf/workers-shared-extra.yaml.j2 +++ b/docker/complement/conf/workers-shared-extra.yaml.j2 @@ -145,8 +145,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 e32728e56d..ef1b58c9ba 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 aba29046db..c9f4ffc635 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -71,7 +71,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 b339d7748c..c099d6f501 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 ca67f1bf56..2e3782392e 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -260,9 +260,6 @@ class ExperimentalConfig(Config): # TODO: Remove the flag and the legacy rules after 2026-12-01. self.msc4210_enabled: bool = experimental.get("msc4210_enabled", True) - # 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 ef1da33a8c..ccd860cc60 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -232,6 +232,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 c91c5b6a49..ae0e60ad3e 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 83aa00d0b5..bf4b2c660a 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -175,11 +175,12 @@ class SyncRestServlet(RestServlet): 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 ) @@ -216,6 +217,7 @@ class SyncRestServlet(RestServlet): device_id, last_ignore_accdata_streampos, use_state_after, + use_unstable_state_after_name, ) if filter_id is None: @@ -253,6 +255,7 @@ class SyncRestServlet(RestServlet): 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 @@ -634,7 +637,12 @@ class SyncRestServlet(RestServlet): # 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 20fdacdc01..fe6a1739e9 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -34,7 +34,6 @@ from synapse.api.constants import ( RelationTypes, ) from synapse.events import EventBase -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, RoomStreamToken, StreamKeyType, StreamToken @@ -775,6 +774,70 @@ class SyncCacheTestCase(unittest.HomeserverTestCase): 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. @@ -1322,12 +1385,6 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): self.bob = self.register_user("bob", "password") self.bob_tok = self.login("bob", "password") - self.get_success( - self.store.set_features_for_user( - self.alice, {ExperimentalFeature.MSC4222: True} - ) - ) - # Named room, so that hero calculation doesn't inject current # membership state into the response and confuse the assertions. self.room_id = self.helper.create_room_as( @@ -1346,7 +1403,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): sync_filter: JsonDict = {"room": {"timeline": {"limit": timeline_limit}}} if lazy_load_members: sync_filter["room"]["state"] = {"lazy_load_members": True} - return f"/sync?filter={json.dumps(sync_filter)}&org.matrix.msc4222.use_state_after=true" + return f"/sync?filter={json.dumps(sync_filter)}&use_state_after=true" def _sync(self, sync_url: str, since: str | None = None) -> JsonDict: url = sync_url if since is None else f"{sync_url}&since={since}" @@ -1363,7 +1420,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): return [e["event_id"] for e in room["timeline"]["events"]] def _state_after_ids(self, room: JsonDict) -> list[str]: - return [e["event_id"] for e in room["org.matrix.msc4222.state_after"]["events"]] + return [e["event_id"] for e in room["state_after"]["events"]] def _persist_batch(self) -> tuple[EventBase, EventBase]: return self.get_success( @@ -1384,7 +1441,7 @@ class SyncStateAfterTimelineStateTestCase(unittest.HomeserverTestCase): self.store.get_partial_current_state_ids(self.room_id) ) current_ids = set(current_state.values()) - for event in room["org.matrix.msc4222.state_after"]["events"]: + for event in room["state_after"]["events"]: self.assertIn( event["event_id"], current_ids, @@ -1583,9 +1640,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 @@ -1602,11 +1656,6 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): 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( @@ -1635,7 +1684,7 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): } } ) - 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) @@ -1665,7 +1714,7 @@ class SyncStateAfterArchivedRoomTestCase(unittest.HomeserverTestCase): 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( @@ -1708,12 +1757,6 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): self.bob = self.register_user("bob", "password") self.bob_tok = self.login("bob", "password") - self.get_success( - hs.get_datastores().main.set_features_for_user( - self.alice, {ExperimentalFeature.MSC4222: True} - ) - ) - # Named room, so that hero calculation doesn't inject current # membership state into the response and confuse the assertions. self.room_id = self.helper.create_room_as( @@ -1748,7 +1791,7 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): } url = f"/sync?filter={json.dumps(sync_filter)}&timeout={timeout_ms}" if use_state_after: - url += "&org.matrix.msc4222.use_state_after=true" + url += "&use_state_after=true" if since is not None: url += f"&since={since}" return self.make_request( @@ -1786,7 +1829,7 @@ class SyncLazyLoadedMembersCacheTestCase(unittest.HomeserverTestCase): self, response: JsonDict, use_state_after: bool ) -> list[JsonDict]: """Extract the list of state events from the test room in a sync response.""" - key = "org.matrix.msc4222.state_after" if use_state_after else "state" + key = "state_after" if use_state_after else "state" return self._room(response)[key]["events"] def _state_ids(self, response: JsonDict, use_state_after: bool) -> list[str]: