dandri/synapse
1
0 Fork 0
mirror of https://github.com/element-hq/synapse.git synced 2026-10-23 13:03:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix new state saved with the last local user's leave missing from state_after (#20314)

Browse Source 
> [!NOTE]
> This PR will be rebased on `develop` once
https://github.com/element-hq/synapse/pull/20081 is merged

This PR is a follow up to
https://github.com/element-hq/synapse/pull/20081 and is based on its
code.
The scope of that original PR is limited to recovering the leave event
missing from `state_after`.

I noticed that state events from the leave event batch would only be
recovered if they were previously defined.

That made me add a pre-condition to the test: the topic had to be set
first.


https://github.com/element-hq/synapse/blob/bccf1679aed0a5d86de87165c548f8c459758055/tests/handlers/test_sync.py#L1096-L1099

This PR attempts to recover new state events that only appeared first in
the leave batch.

### 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: Fefaine <fefaine@gmail.com>
Co-authored-by: Quentin Gliech <quenting@element.io>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Co-authored-by: Eric Eastwood <erice@element.io>
This commit is contained in:
Paul ChobertFefaineQuentin GliechEric EastwoodEric Eastwood
 authored and GitHub committed 2026-10-19 09:25:23 +00:00
1 parent 26a92c7111
commit 7d44c81d5c
3 files changed
+37 -22
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

changelog.d/20314.bugfix
+1
View File
@@ -0,0 +1 @@
Fix new state saved with the last local user's leave being missing from `state_after` ([MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222)) on incremental `/sync`.
synapse/handlers/sync.py
+17 -11
View File
@@ -1738,11 +1738,12 @@ class SyncHandler:
# any earlier delta for it is stale. That batch ends the
# timeline, so we look up the keys in the timeline in state
# groups, which still have the state at `end_token`.
# - has no delta at all if it is new.
# TODO: Recover it.
# - has no delta at all if it is new, so we also look up all the
# keys in the timeline (see below).
# Keys that only changed earlier keep their earlier delta, and
# other keys need no update.
cleared_state_keys: set[tuple[str, str]] = set()
own_membership_key = (EventTypes.Member, sync_config.user.to_string())
state_keys_to_look_up: set[tuple[str, str]] = set()
for delta in deltas:
key = (delta.event_type, delta.state_key)
if delta.event_id is None:
@@ -1750,11 +1751,8 @@ class SyncHandler:
# - it is in the timeline (see above).
# - OR it is the user's own membership, in case a filter
# removed their leave from the timeline.
if key in timeline_state or key == (
EventTypes.Member,
sync_config.user.to_string(),
):
cleared_state_keys.add(key)
if key in timeline_state or key == own_membership_key:
state_keys_to_look_up.add(key)
continue
# Note that deltas are in stream ordering, so if there are
@@ -1762,13 +1760,21 @@ class SyncHandler:
# the latest one.
delta_state_ids[key] = delta.event_id
changed_keys.add(key)
cleared_state_keys.discard(key)
state_keys_to_look_up.discard(key)
if cleared_state_keys:
if own_membership_key in state_keys_to_look_up:
# The server left the room in this window. It only wrote deltas
# for the keys that were in the room state before the batch that
# made it leave, so the keys that batch added have no delta at
# all. Look up all the keys in the timeline: the ones with a delta
# are already in the set.
state_keys_to_look_up.update(timeline_state)
if state_keys_to_look_up:
state_at_end = await self._state_storage_controller.get_state_ids_at(
room_id,
stream_position=end_token,
state_filter=StateFilter.from_types(cleared_state_keys),
state_filter=StateFilter.from_types(state_keys_to_look_up),
# Don't wait for the full state. Partial state has every state
# event except remote memberships, and any remote membership
# we look up here is from an event in the timeline, which we
tests/handlers/test_sync.py
+19 -11
View File
@@ -1093,18 +1093,26 @@ class SyncTestCase(tests.unittest.HomeserverTestCase):
{("m.room.member", alice): leave_event},
)
@parameterized.expand(
[(True,), (False,)],
name_func=lambda func, num, p: (
f"{func.__name__}_{'existing' if p.args[0] else 'new'}_topic"
),
)
def test_state_after_leave_last_local_user_with_state_change_persisted_together(
self,
self, topic_existed: bool
) -> None:
"""When another state event is persisted in the same batch as the last local
user's leave, both the leave and that state event must appear in state_after
on an incremental sync.
on an incremental sync, whether or not the room already had that state.
This is to make sure we play nicely with this behavior: When the server leaves a
room, it will insert new rows with `event_id = null` into the
`current_state_delta_stream` table for all current state.
Neither the leave nor the state event persisted with it gets a row of its own,
only the `event_id = null` rows of the state they replace.
Neither the leave nor the state event persisted with it gets a row of its own:
the leave only gets the `event_id = null` row of the leaving user's membership,
and the state event only gets one if the room already had that state, so the
two cases are recovered differently.
"""
if not self.use_state_after:
self.skipTest("Only relevant for `state_after` (MSC4222)")
@@ -1115,9 +1123,10 @@ class SyncTestCase(tests.unittest.HomeserverTestCase):
alice_requester = create_requester(alice)
room_id = self.helper.create_room_as(alice, tok=alice_tok)
self.helper.send_state(
room_id, EventTypes.Topic, {"topic": "before leaving"}, tok=alice_tok
)
if topic_existed:
self.helper.send_state(
room_id, EventTypes.Topic, {"topic": "before leaving"}, tok=alice_tok
)
# Sync up to get a since_token.
initial_sync_result = self.get_success(
@@ -1128,10 +1137,9 @@ class SyncTestCase(tests.unittest.HomeserverTestCase):
)
)
# Alice changes the topic and leaves, with both events persisted in one
# batch. She is the last local user, so the server clears
# current_state_events for this room, and neither event gets a delta of its
# own.
# Alice sets the topic and leaves, with both events persisted in one batch.
# She is the last local user, so the server clears current_state_events for
# this room, and neither event gets a delta of its own.
self.get_success(
self.hs.get_event_creation_handler().create_and_send_new_client_events(
requester=alice_requester,