Add MSC4222 /sync state_after test for initial sync lazy-loading room members#842
Conversation
This test is used to try to reproduce element-hq/synapse#19455 (comment) but found that it seems to the rigth thing.
| // FIXME: Some implementations haven't stabilized yet (Synapse) so we'll keep this | ||
| // here until then. | ||
| query["org.matrix.msc4222.use_state_after"] = []string{"true"} |
| // | ||
| // We're specifically testing the scenario where a new "DM" is created and the other person | ||
| // joins without speaking yet. | ||
| t.Run("Initial sync with lazy-loading room members -> private room `state_after` includes all members from timeline", func(t *testing.T) { |
There was a problem hiding this comment.
Seems like the room being private and the other member being invited makes a difference - I've updated your complement PR to make it like that and it now seems to fail
I've updated things so we have both tests (for public and private)
| // Try looking up the stable variant `state_after` first, then fallback to the | ||
| // unstable version | ||
| roomStateAfterResStable := joinedRoomRes.Get("state_after.events"); | ||
| roomStateAfterResUnstable := joinedRoomRes.Get("org\\.matrix\\.msc4222\\.state_after.events"); | ||
| var roomStateAfterRes gjson.Result | ||
| if roomStateAfterResStable.Exists() { | ||
| roomStateAfterRes = roomStateAfterResStable | ||
| } else if roomStateAfterResUnstable.Exists() { | ||
| roomStateAfterRes = roomStateAfterResUnstable | ||
| } |
There was a problem hiding this comment.
Not a great piece of code but since we only do this in one spot I'm tempted to leave it rather than create a bad abstraction.
And it's unclear how things like this should work once MSC's have stabilized but homeservers haven't yet.
| "state": { "lazy_load_members": true } | ||
| } | ||
| }` | ||
| testInitialSyncStateAfterIncludesTimelineSenders(t, alice, roomID, expectedSendersFromTimeline, syncFilter) |
There was a problem hiding this comment.
For public rooms, things appear to work fine in Synapse ✅ :
Example rooms.join.<room_id> part of the initial sync response: (includes m.room.member events for both alice and bob in timeline and org.matrix.msc4222.state_after fields)
{"timeline":{"events":[{"type":"m.room.create","sender":"@user-1-alice:hs1","content":{"room_version":"10","creator":"@user-1-alice:hs1"},"state_key":"","origin_server_ts":1770930300664,"unsigned":{"membership":"leave","age":618},"event_id":"$qp-zP4IHGRfkc0IVa_kN6tX_sBJyV0ffkTF3RaWey4c"},{"type":"m.room.member","sender":"@user-1-alice:hs1","content":{"displayname":"user-1-alice","membership":"join"},"state_key":"@user-1-alice:hs1","origin_server_ts":1770930300805,"unsigned":{"membership":"join","age":477},"event_id":"$59iC8AUKcjIZHIxjq7HglPsxDZBzGcKy1WP5g48FSRI"},{"type":"m.room.power_levels","sender":"@user-1-alice:hs1","content":{"users":{"@user-1-alice:hs1":100},"users_default":0,"events":{"m.room.name":50,"m.room.power_levels":100,"m.room.history_visibility":100,"m.room.canonical_alias":50,"m.room.avatar":50,"m.room.tombstone":100,"m.room.server_acl":100,"m.room.encryption":100},"events_default":0,"state_default":50,"ban":50,"kick":50,"redact":50,"invite":50,"historical":100,"m.call.invite":50},"state_key":"","origin_server_ts":1770930300916,"unsigned":{"membership":"join","age":366},"event_id":"$gbu10XtbHx4qEN0Zwho3k248Sho2n5SWlR29ISc5FKw"},{"type":"m.room.join_rules","sender":"@user-1-alice:hs1","content":{"join_rule":"public"},"state_key":"","origin_server_ts":1770930300921,"unsigned":{"membership":"join","age":361},"event_id":"$dWnerY4XQ-eVhLmGmy9prqMgRwIF5E8n5TOYkAmAT4k"},{"type":"m.room.history_visibility","sender":"@user-1-alice:hs1","content":{"history_visibility":"shared"},"state_key":"","origin_server_ts":1770930300921,"unsigned":{"membership":"join","age":361},"event_id":"$3fe6abisM3FkfrWK9CDRlV32dOzmk2kb9FaG8wEDOjs"},{"type":"m.room.member","sender":"@user-2-bob:hs1","content":{"displayname":"user-2-bob","membership":"join"},"state_key":"@user-2-bob:hs1","origin_server_ts":1770930301142,"unsigned":{"membership":"join","age":140},"event_id":"$Ug1lAQ7BCQANKhBtSfPrRnImxzaZOyFyi6P2qVjtfok"}],"prev_batch":"s7_2_0_1_1_1_1_3_0_1_1_1","limited":false},"org.matrix.msc4222.state_after":{"events":[{"type":"m.room.create","sender":"@user-1-alice:hs1","content":{"room_version":"10","creator":"@user-1-alice:hs1"},"state_key":"","origin_server_ts":1770930300664,"unsigned":{"age":618},"event_id":"$qp-zP4IHGRfkc0IVa_kN6tX_sBJyV0ffkTF3RaWey4c"},{"type":"m.room.history_visibility","sender":"@user-1-alice:hs1","content":{"history_visibility":"shared"},"state_key":"","origin_server_ts":1770930300921,"unsigned":{"age":361},"event_id":"$3fe6abisM3FkfrWK9CDRlV32dOzmk2kb9FaG8wEDOjs"},{"type":"m.room.join_rules","sender":"@user-1-alice:hs1","content":{"join_rule":"public"},"state_key":"","origin_server_ts":1770930300921,"unsigned":{"age":361},"event_id":"$dWnerY4XQ-eVhLmGmy9prqMgRwIF5E8n5TOYkAmAT4k"},{"type":"m.room.member","sender":"@user-1-alice:hs1","content":{"displayname":"user-1-alice","membership":"join"},"state_key":"@user-1-alice:hs1","origin_server_ts":1770930300805,"unsigned":{"age":477},"event_id":"$59iC8AUKcjIZHIxjq7HglPsxDZBzGcKy1WP5g48FSRI"},{"type":"m.room.member","sender":"@user-2-bob:hs1","content":{"displayname":"user-2-bob","membership":"join"},"state_key":"@user-2-bob:hs1","origin_server_ts":1770930301142,"unsigned":{"age":140},"event_id":"$Ug1lAQ7BCQANKhBtSfPrRnImxzaZOyFyi6P2qVjtfok"},{"type":"m.room.power_levels","sender":"@user-1-alice:hs1","content":{"users":{"@user-1-alice:hs1":100},"users_default":0,"events":{"m.room.name":50,"m.room.power_levels":100,"m.room.history_visibility":100,"m.room.canonical_alias":50,"m.room.avatar":50,"m.room.tombstone":100,"m.room.server_acl":100,"m.room.encryption":100},"events_default":0,"state_default":50,"ban":50,"kick":50,"redact":50,"invite":50,"historical":100,"m.call.invite":50},"state_key":"","origin_server_ts":1770930300916,"unsigned":{"age":366},"event_id":"$gbu10XtbHx4qEN0Zwho3k248Sho2n5SWlR29ISc5FKw"}]},"account_data":{"events":[]},"ephemeral":{"events":[]},"unread_notifications":{"notification_count":0,"highlight_count":0},"summary":{"m.joined_member_count":2,"m.invited_member_count":0,"m.heroes":["@user-2-bob:hs1"]}}| "state": { "lazy_load_members": true } | ||
| } | ||
| }` | ||
| testInitialSyncStateAfterIncludesTimelineSenders(t, alice, roomID, expectedSendersFromTimeline, syncFilter) |
There was a problem hiding this comment.
For private rooms, things currently fail in Synapse ❌. Reproduces element-hq/synapse#19455
Example rooms.join.<room_id> part of the initial sync response: (missing membership event for bob in the state_after)
{"next_batch":"s9_3_0_1_1_1_1_3_0_1_1_1","account_data":{"events":[{"type":"m.push_rules","content":{"global":{"underride":[{"conditions":[{"kind":"event_match","key":"type","pattern":"m.call.invite"}],"actions":["notify",{"set_tweak":"sound","value":"ring"},{"set_tweak":"highlight","value":false}],"rule_id":".m.rule.call","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.encrypted"},{"kind":"room_member_count","is":"2"}],"actions":["notify",{"set_tweak":"sound","value":"default"},{"set_tweak":"highlight","value":false}],"rule_id":".m.rule.encrypted_room_one_to_one","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.message"},{"kind":"room_member_count","is":"2"}],"actions":["notify",{"set_tweak":"sound","value":"default"},{"set_tweak":"highlight","value":false}],"rule_id":".m.rule.room_one_to_one","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.message"}],"actions":["notify",{"set_tweak":"highlight","value":false}],"rule_id":".m.rule.message","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.encrypted"}],"actions":["notify",{"set_tweak":"highlight","value":false}],"rule_id":".m.rule.encrypted","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"im.vector.modular.widgets"},{"kind":"event_match","key":"content.type","pattern":"jitsi"},{"kind":"event_match","key":"state_key","pattern":"*"}],"actions":["notify",{"set_tweak":"highlight","value":false}],"rule_id":".im.vector.jitsi","default":true,"enabled":true},{"conditions":[{"kind":"room_member_count","is":"2"},{"kind":"event_match","key":"type","pattern":"org.matrix.msc3381.poll.start"}],"actions":["notify",{"set_tweak":"sound","value":"default"}],"rule_id":".org.matrix.msc3930.rule.poll_start_one_to_one","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"org.matrix.msc3381.poll.start"}],"actions":["notify"],"rule_id":".org.matrix.msc3930.rule.poll_start","default":true,"enabled":true},{"conditions":[{"kind":"room_member_count","is":"2"},{"kind":"event_match","key":"type","pattern":"org.matrix.msc3381.poll.end"}],"actions":["notify",{"set_tweak":"sound","value":"default"}],"rule_id":".org.matrix.msc3930.rule.poll_end_one_to_one","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"org.matrix.msc3381.poll.end"}],"actions":["notify"],"rule_id":".org.matrix.msc3930.rule.poll_end","default":true,"enabled":true}],"sender":[],"room":[],"postcontent":[{"conditions":[{"kind":"io.element.msc4306.thread_subscription","subscribed":false}],"actions":[],"rule_id":".io.element.msc4306.rule.unsubscribed_thread","default":true,"enabled":true},{"conditions":[{"kind":"io.element.msc4306.thread_subscription","subscribed":true}],"actions":["notify",{"set_tweak":"sound","value":"default"}],"rule_id":".io.element.msc4306.rule.subscribed_thread","default":true,"enabled":true}],"content":[{"actions":["notify",{"set_tweak":"highlight"},{"set_tweak":"sound","value":"default"}],"rule_id":".m.rule.contains_user_name","default":true,"pattern":"user-1-alice","enabled":true}],"override":[{"conditions":[],"actions":[],"rule_id":".m.rule.master","default":true,"enabled":false},{"conditions":[{"kind":"event_match","key":"content.msgtype","pattern":"m.notice"}],"actions":[],"rule_id":".m.rule.suppress_notices","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"},{"kind":"event_match","key":"content.membership","pattern":"invite"},{"kind":"event_match","key":"state_key","pattern":"@user-1-alice:hs1"}],"actions":["notify",{"set_tweak":"highlight","value":false},{"set_tweak":"sound","value":"default"}],"rule_id":".m.rule.invite_for_me","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"}],"actions":[],"rule_id":".m.rule.member_event","default":true,"enabled":true},{"conditions":[{"kind":"event_property_contains","key":"content.m\\.mentions.user_ids","value":"@user-1-alice:hs1"}],"actions":["notify",{"set_tweak":"highlight"},{"set_tweak":"sound","value":"default"}],"rule_id":".m.rule.is_user_mention","default":true,"enabled":true},{"conditions":[{"kind":"contains_display_name"}],"actions":["notify",{"set_tweak":"highlight"},{"set_tweak":"sound","value":"default"}],"rule_id":".m.rule.contains_display_name","default":true,"enabled":true},{"conditions":[{"kind":"event_property_is","key":"content.m\\.mentions.room","value":true},{"kind":"sender_notification_permission","key":"room"}],"actions":["notify",{"set_tweak":"highlight"}],"rule_id":".m.rule.is_room_mention","default":true,"enabled":true},{"conditions":[{"kind":"sender_notification_permission","key":"room"},{"kind":"event_match","key":"content.body","pattern":"@room"}],"actions":["notify",{"set_tweak":"highlight"}],"rule_id":".m.rule.roomnotif","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.tombstone"},{"kind":"event_match","key":"state_key","pattern":""}],"actions":["notify",{"set_tweak":"highlight"}],"rule_id":".m.rule.tombstone","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.reaction"}],"actions":[],"rule_id":".m.rule.reaction","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.server_acl"},{"kind":"event_match","key":"state_key","pattern":""}],"actions":[],"rule_id":".m.rule.room.server_acl","default":true,"enabled":true},{"conditions":[{"kind":"event_property_is","key":"content.m\\.relates_to.rel_type","value":"m.replace"}],"actions":[],"rule_id":".m.rule.suppress_edits","default":true,"enabled":true},{"conditions":[{"kind":"event_match","key":"type","pattern":"org.matrix.msc3381.poll.response"}],"actions":[],"rule_id":".org.matrix.msc3930.rule.poll_response","default":true,"enabled":true}]}}}]},"presence":{"events":[{"type":"m.presence","sender":"@user-1-alice:hs1","content":{"presence":"online","last_active_ago":3,"currently_active":true}},{"type":"m.presence","sender":"@user-2-bob:hs1","content":{"presence":"online","last_active_ago":179,"currently_active":true}}]},"device_one_time_keys_count":{"signed_curve25519":0},"device_unused_fallback_key_types":[],"rooms":{"join":{"!gdBDkMRnmIypMjFmBh:hs1":{"timeline":{"events":[{"type":"m.room.create","sender":"@user-1-alice:hs1","content":{"room_version":"10","creator":"@user-1-alice:hs1"},"state_key":"","origin_server_ts":1771007574475,"unsigned":{"membership":"leave","age":824},"event_id":"$y5d9TkIOmtx_Bgs6emLOkuAsMeKGF8BdBNTfb_MOyVQ"},{"type":"m.room.member","sender":"@user-1-alice:hs1","content":{"displayname":"user-1-alice","membership":"join"},"state_key":"@user-1-alice:hs1","origin_server_ts":1771007574619,"unsigned":{"membership":"join","age":680},"event_id":"$1_a1Y1rddZC9qQuILgeZymyTfGS9hQia5dYj82h1O_I"},{"type":"m.room.power_levels","sender":"@user-1-alice:hs1","content":{"users":{"@user-1-alice:hs1":100},"users_default":0,"events":{"m.room.name":50,"m.room.power_levels":100,"m.room.history_visibility":100,"m.room.canonical_alias":50,"m.room.avatar":50,"m.room.tombstone":100,"m.room.server_acl":100,"m.room.encryption":100},"events_default":0,"state_default":50,"ban":50,"kick":50,"redact":50,"invite":0,"historical":100},"state_key":"","origin_server_ts":1771007574733,"unsigned":{"membership":"join","age":566},"event_id":"$qc1MGLT23L35gJCnFGTV3QkL7SmvODwe6PSFqFo3x9I"},{"type":"m.room.join_rules","sender":"@user-1-alice:hs1","content":{"join_rule":"invite"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"membership":"join","age":561},"event_id":"$KVG2BvhxOJw3wN0iTdOmqV7jONh44dokydRrATbrCvI"},{"type":"m.room.history_visibility","sender":"@user-1-alice:hs1","content":{"history_visibility":"shared"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"membership":"join","age":561},"event_id":"$JdttXi3VDucYC7HcGAw0OTBwm2K6mJn_7GfkewcHEhc"},{"type":"m.room.guest_access","sender":"@user-1-alice:hs1","content":{"guest_access":"can_join"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"membership":"join","age":561},"event_id":"$cgkV_w3r4_piAlf4g1X8kg7ba6yh37eBdQuXLWWyxzE"},{"type":"m.room.member","sender":"@user-1-alice:hs1","content":{"displayname":"user-2-bob","membership":"invite"},"state_key":"@user-2-bob:hs1","origin_server_ts":1771007574962,"unsigned":{"membership":"join","age":337},"event_id":"$rzywv46JfX2c30FWTTRPBhEMDmnWwOrtabxycJPOHuE"},{"type":"m.room.member","sender":"@user-2-bob:hs1","content":{"displayname":"user-2-bob","membership":"join"},"state_key":"@user-2-bob:hs1","origin_server_ts":1771007575156,"unsigned":{"replaces_state":"$rzywv46JfX2c30FWTTRPBhEMDmnWwOrtabxycJPOHuE","prev_content":{"displayname":"user-2-bob","membership":"invite"},"prev_sender":"@user-1-alice:hs1","membership":"join","age":143},"event_id":"$iMb1zckAPk3WERBUuMNJRIS32F-q3nUbUK_TdYcNTv4"}],"prev_batch":"s9_3_0_1_1_1_1_3_0_1_1_1","limited":false},"org.matrix.msc4222.state_after":{"events":[{"type":"m.room.create","sender":"@user-1-alice:hs1","content":{"room_version":"10","creator":"@user-1-alice:hs1"},"state_key":"","origin_server_ts":1771007574475,"unsigned":{"age":824},"event_id":"$y5d9TkIOmtx_Bgs6emLOkuAsMeKGF8BdBNTfb_MOyVQ"},{"type":"m.room.guest_access","sender":"@user-1-alice:hs1","content":{"guest_access":"can_join"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"age":561},"event_id":"$cgkV_w3r4_piAlf4g1X8kg7ba6yh37eBdQuXLWWyxzE"},{"type":"m.room.history_visibility","sender":"@user-1-alice:hs1","content":{"history_visibility":"shared"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"age":561},"event_id":"$JdttXi3VDucYC7HcGAw0OTBwm2K6mJn_7GfkewcHEhc"},{"type":"m.room.join_rules","sender":"@user-1-alice:hs1","content":{"join_rule":"invite"},"state_key":"","origin_server_ts":1771007574738,"unsigned":{"age":561},"event_id":"$KVG2BvhxOJw3wN0iTdOmqV7jONh44dokydRrATbrCvI"},{"type":"m.room.member","sender":"@user-1-alice:hs1","content":{"displayname":"user-1-alice","membership":"join"},"state_key":"@user-1-alice:hs1","origin_server_ts":1771007574619,"unsigned":{"age":680},"event_id":"$1_a1Y1rddZC9qQuILgeZymyTfGS9hQia5dYj82h1O_I"},{"type":"m.room.power_levels","sender":"@user-1-alice:hs1","content":{"users":{"@user-1-alice:hs1":100},"users_default":0,"events":{"m.room.name":50,"m.room.power_levels":100,"m.room.history_visibility":100,"m.room.canonical_alias":50,"m.room.avatar":50,"m.room.tombstone":100,"m.room.server_acl":100,"m.room.encryption":100},"events_default":0,"state_default":50,"ban":50,"kick":50,"redact":50,"invite":0,"historical":100},"state_key":"","origin_server_ts":1771007574733,"unsigned":{"age":566},"event_id":"$qc1MGLT23L35gJCnFGTV3QkL7SmvODwe6PSFqFo3x9I"}]},"account_data":{"events":[]},"ephemeral":{"events":[]},"unread_notifications":{"notification_count":0,"highlight_count":0},"summary":{"m.joined_member_count":2,"m.invited_member_count":0,"m.heroes":["@user-2-bob:hs1"]}}}}}*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>
|
Thanks for the review @erikjohnston 🐿️ |
Add MSC4222
/syncstate_aftertest for initial sync lazy-loading room membersThis is by no means an exhaustive test suite. This test was simply used to try to reproduce element-hq/synapse#19455 (comment)
Running the test in Synapse (requires element-hq/synapse#19460):
Pull Request Checklist