-
Notifications
You must be signed in to change notification settings - Fork 69
Add tests for edge cases of multiple membership changes in sync period #787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -731,6 +731,163 @@ func TestRoomSummary(t *testing.T) { | |||||||||||
| alice.MustSyncUntil(t, client.SyncReq{Since: aliceSince}, client.SyncJoinedTo(bob.UserID, roomID), joinedCheck) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // TestLeaveReinviteSync tests that when a user is kicked and then re-invited, | ||||||||||||
| // they only see an invite in their sync response, not both an invite and leave event. | ||||||||||||
| func TestLeaveReinviteSync(t *testing.T) { | ||||||||||||
| deployment := complement.Deploy(t, 1) | ||||||||||||
| defer deployment.Destroy(t) | ||||||||||||
|
|
||||||||||||
| alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) | ||||||||||||
| bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) | ||||||||||||
|
|
||||||||||||
| // 1. Alice creates a room and Bob joins | ||||||||||||
| roomID := alice.MustCreateRoom(t, map[string]any{ | ||||||||||||
| "preset": "public_chat", | ||||||||||||
| }) | ||||||||||||
| bob.MustJoinRoom(t, roomID, nil) | ||||||||||||
|
|
||||||||||||
| // 2. Bob does a sync and verifies they see the join | ||||||||||||
| bobSince := bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // 3. Alice kicks Bob from the room and then re-invites them. | ||||||||||||
| // For kicking, we need to use the POST /rooms/{roomId}/kick endpoint | ||||||||||||
| alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, | ||||||||||||
| client.WithJSONBody(t, map[string]any{ | ||||||||||||
| "user_id": bob.UserID, | ||||||||||||
| }), | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| // Wait until Bob is kicked. | ||||||||||||
| alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // Alice re-invites Bob | ||||||||||||
| alice.MustInviteRoom(t, roomID, bob.UserID) | ||||||||||||
|
|
||||||||||||
| // 4. Bob does a sync | ||||||||||||
| jsonRes, _ := bob.MustSync(t, client.SyncReq{Since: bobSince}) | ||||||||||||
|
|
||||||||||||
| // Bob should only see an invite, not both an invite and a leave event | ||||||||||||
| if !jsonRes.Get("rooms.invite." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Expected to see the room in the invite section of the sync response") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Make sure there's no leave event for the room | ||||||||||||
| if jsonRes.Get("rooms.leave." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Room should not appear in the leave section of the sync response") | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // TestLeaveJoinLeaveSync tests that when a user leaves, rejoins, and leaves again, | ||||||||||||
| // they only see a leave event in their sync response, not both a join and a leave event. | ||||||||||||
| func TestLeaveJoinLeaveSync(t *testing.T) { | ||||||||||||
| deployment := complement.Deploy(t, 1) | ||||||||||||
| defer deployment.Destroy(t) | ||||||||||||
|
|
||||||||||||
| alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) | ||||||||||||
| bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) | ||||||||||||
|
|
||||||||||||
| // 1. Alice creates a room and Bob joins | ||||||||||||
| roomID := alice.MustCreateRoom(t, map[string]any{ | ||||||||||||
| "preset": "public_chat", | ||||||||||||
| }) | ||||||||||||
| bob.MustJoinRoom(t, roomID, nil) | ||||||||||||
|
|
||||||||||||
| // 2. Bob does a sync and verifies they see the join | ||||||||||||
| bobSince := bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // 3. Bob leaves the room | ||||||||||||
| bob.MustLeaveRoom(t, roomID) | ||||||||||||
|
|
||||||||||||
| // 4. Bob rejoins the room | ||||||||||||
| bob.MustJoinRoom(t, roomID, nil) | ||||||||||||
|
|
||||||||||||
| // 5. Bob leaves the room again | ||||||||||||
| bob.MustLeaveRoom(t, roomID) | ||||||||||||
|
|
||||||||||||
| // 6. Bob does a sync | ||||||||||||
| jsonRes, _ := bob.MustSync(t, client.SyncReq{Since: bobSince}) | ||||||||||||
|
|
||||||||||||
| // Bob should only see a leave event, not both a join and a leave event | ||||||||||||
| if !jsonRes.Get("rooms.leave." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Expected to see the room in the leave section of the sync response") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Make sure there's no join event for the room | ||||||||||||
| if jsonRes.Get("rooms.join." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Room should not appear in the join section of the sync response") | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // TestLeaveReinviteSyncFederated tests that when a user is kicked and then re-invited over federation, | ||||||||||||
| // they only see an invite in their sync response, not both an invite and leave event. | ||||||||||||
| func TestLeaveReinviteSyncFederated(t *testing.T) { | ||||||||||||
| deployment := complement.Deploy(t, 2) | ||||||||||||
| defer deployment.Destroy(t) | ||||||||||||
|
|
||||||||||||
| alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) | ||||||||||||
| bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) | ||||||||||||
|
|
||||||||||||
| // Charlie is simply here to flag when events arrive over federation to hs2. | ||||||||||||
| charlie := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) | ||||||||||||
|
|
||||||||||||
| // 1. Alice creates a room and both Bob and Charlie join | ||||||||||||
| roomID := alice.MustCreateRoom(t, map[string]any{ | ||||||||||||
| "preset": "public_chat", | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| // Users on hs2 need to join via federation, so we need to specify the server name | ||||||||||||
| alice.MustInviteRoom(t, roomID, bob.UserID) | ||||||||||||
| alice.MustInviteRoom(t, roomID, charlie.UserID) | ||||||||||||
| bob.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(bob.UserID, roomID)) | ||||||||||||
| charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(charlie.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| bob.MustJoinRoom(t, roomID, []spec.ServerName{ | ||||||||||||
| deployment.GetFullyQualifiedHomeserverName(t, "hs1"), | ||||||||||||
| }) | ||||||||||||
| charlie.MustJoinRoom(t, roomID, []spec.ServerName{ | ||||||||||||
| deployment.GetFullyQualifiedHomeserverName(t, "hs1"), | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| // 2. Both Bob and Charlie do a sync and verify that they see the join | ||||||||||||
| bobSince := bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) | ||||||||||||
| charlieSince := charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(charlie.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // 3. Alice kicks Bob from the room and then re-invites them. | ||||||||||||
| // For kicking, we need to use the POST /rooms/{roomId}/kick endpoint | ||||||||||||
| alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, | ||||||||||||
| client.WithJSONBody(t, map[string]any{ | ||||||||||||
| "user_id": bob.UserID, | ||||||||||||
| }), | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| // Wait until Bob is kicked. | ||||||||||||
| aliceSince := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, roomID)) | ||||||||||||
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to worry about getting a The other sync's are just asserting pre-conditions.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment about why we wait here as well (needs wrapping)
Suggested change
|
||||||||||||
|
|
||||||||||||
| // Alice re-invites Bob | ||||||||||||
| alice.MustInviteRoom(t, roomID, bob.UserID) | ||||||||||||
| alice.MustSyncUntil(t, client.SyncReq{Since: aliceSince}, client.SyncInvitedTo(bob.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // Wait until hs2 sees that Bob has been kicked and re-invited (this is all | ||||||||||||
| // Charlie is needed for). If we don't wait, then Bob might sync too early | ||||||||||||
| // (slow federation) and catch the kick in one sync iteration, and the | ||||||||||||
| // invite in another, invalidating the test conditions. | ||||||||||||
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncInvitedTo(bob.UserID, roomID)) | ||||||||||||
|
|
||||||||||||
| // 4. Bob does an incremental sync. Bob's last sync was after they joined the room. | ||||||||||||
| jsonRes, _ := bob.MustSync(t, client.SyncReq{Since: bobSince}) | ||||||||||||
|
Comment on lines
+856
to
+880
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't guarantee that the leave/invite will be federated to Bob's homeserver by the time Bob syncs. Perhaps we need another If we just use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very good point. Another user would work. I'll modify the test to include that. |
||||||||||||
|
|
||||||||||||
| // Bob should only see an invite, not both an invite and a leave event | ||||||||||||
| if !jsonRes.Get("rooms.invite." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Expected to see the room in the invite section of the sync response") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Make sure there's no leave event for the room | ||||||||||||
| if jsonRes.Get("rooms.leave." + client.GjsonEscape(roomID)).Exists() { | ||||||||||||
| t.Errorf("Room should not appear in the leave section of the sync response") | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func sendMessages(t *testing.T, client *client.CSAPI, roomID string, prefix string, count int) { | ||||||||||||
| t.Helper() | ||||||||||||
| for i := 0; i < count; i++ { | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.