Add experimental support for MSC4222#17888
Merged
erikjohnston merged 11 commits intodevelopfrom Nov 5, 2024
Merged
Conversation
This doesn't actually change what we return in `state_after` yet.
4 tasks
Member
anoadragon453
left a comment
anoadragon453
left a comment
There was a problem hiding this comment.
A couple questions, but on the whole looks good.
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
anoadragon453
approved these changes
Nov 5, 2024
Member
anoadragon453
left a comment
anoadragon453
left a comment
There was a problem hiding this comment.
Thanks for the changes!
erikjohnston
added a commit
that referenced
this pull request
Nov 8, 2024
There was a bug that meant we would return the full state of the room on incremental syncs when using lazy loaded members and there were no entries in the timeline. This was due to trying to use `state_filter or state_filter.all()` as a short hand for handling `None` case, however `state_filter` implements `__bool__` so if the state filter was empty it would be set to full. c.f. MSC4222 and #17888
3 tasks
5 tasks
sandhose
pushed a commit
that referenced
this pull request
Mar 3, 2026
*This PR was originally only to enable [MSC4222](matrix-org/matrix-spec-proposals#4222) Complement tests (`/sync` `state_after`) but after merging the [fix PR](#19463), we discovered that while the tests pass locally, [fail in CI](#19460 (comment)). To unblock the RC, we decided to revert the fix PR (see #19474 (comment) for more info). To better ensure tests actually pass in CI, we're re-introducing the fix here in the same PR that we enable the tests in.* --- Fix `/sync` missing membership in `state_after`. This applies to any scenario where the first membership has a different `sender` compared to the `state_key` and then the second membership has the same `sender`/`state_key`. Like someone inviting another person and then them joining. Or someone being kicked and then they leave. This bug has been present since the MSC4222 implementation was introduced into the codebase (#17888). --- Fix #19455 Fix element-hq/customer-success#656 I have a feeling, this might also fix these issues (will close and see how people report back): Fix #18182 Fix #19478 ### Testing strategy Complement tests: matrix-org/complement#842 We will need #19460 to merge in order to enable the Complement tests in Synapse but this PR should be merged first so they pass in the first place. I've tested locally that the Complement tests pass with this fix. ### Dev notes [MSC4222](matrix-org/matrix-spec-proposals#4222) has already been merged into the spec and is already part of Matrix v1.16 but we haven't [stabilized support in Synapse yet](#19414). --- In the same ballpark: - #19455 - #17050 - #17430 - #16940 - #18182 - #18793 - #19478 --- Docker builds preferring remote image over the local image we just built, #19460 (comment) `containerd` image store (storage driver, driver type) -> #19475 ### Todo - [x] Wait for #19463 to merge so the Complement tests all pass - [x] Wait for #19475 to merge ### 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: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basically, if the client sets a special query param on
/syncv2 instead of responding withstateat the start of the timeline, we instead respond withstate_afterat the end of the timeline.We do this by using the
current_state_delta_streamtable, which is actually reliable, rather than messing around with "state at" points on the timeline.c.f. MSC4222
Reviewable commit-by-commit.