mirror of
https://github.com/element-hq/synapse.git
synced 2026-10-23 06:32:54 +00:00
Stabilise MSC4222: support the stable use_state_after identifiers on /sync (#20170)
Part of #19414 (Support Matrix 1.16). ⚠️ Before merging, wait for the following PRs to be merged first: - https://github.com/element-hq/synapse/pull/20081 - https://github.com/element-hq/synapse/pull/20169 - https://github.com/element-hq/synapse/pull/20171 --- Since Matrix 1.16, the [spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync) defines the stable identifiers for [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222): the `use_state_after` query parameter on `GET /_matrix/client/v3/sync`, and the `state_after` room response field: > **`use_state_after`** — `boolean` — Controls whether to receive state changes between the previous sync and the start of the timeline, or between the previous sync and the end of the timeline. If this is set to `true`, servers MUST respond with the state between the previous sync and the end of the timeline in `state_after` and MUST omit `state`. […] > > **`state_after`** — […] Required if `use_state_after` was set to true in the request, even if it is empty. Added in `v1.16` > > — [Matrix Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync) Synapse so far only implemented the unstable identifiers (`org.matrix.msc4222.use_state_after` / `org.matrix.msc4222.state_after`), gated behind the `msc4222_enabled` experimental flag and the per-user experimental features admin API. This PR: - accepts the stable `use_state_after` query parameter, unconditionally, and responds with the stable `state_after` field. The unstable query parameter is kept for a transition period; clients opting in with it get the unstable response field name back, so unstable-era clients see an unchanged wire format. If both parameters are set, the stable one wins. - removes the `msc4222_enabled` experimental flag and the `msc4222` per-user feature from the experimental features admin API, since the behaviour is no longer gated. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
This commit is contained in:
1 parent
694ad6bfa7
commit
3c46a10955
10 files changed
+94
-46
No files matched your search
@@ -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.
|
||||
@@ -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
|
||||
|
||||
@@ -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/).
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -27,8 +27,6 @@ pub enum PerUserExperimentalFeature {
|
||||
MSC3881,
|
||||
#[serde(rename = "msc3575")]
|
||||
MSC3575,
|
||||
#[serde(rename = "msc4222")]
|
||||
MSC4222,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for PerUserExperimentalFeature {
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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]:
|
||||
|
||||
Reference in new issue
Block a user