From f6dd4973e2688b4400cf2354a09f616e53f0b37f Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 15:19:01 +0000 Subject: [PATCH 01/10] Initial API design --- internal/client/client.go | 197 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/internal/client/client.go b/internal/client/client.go index 5dc5170a..7627329b 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -3,6 +3,7 @@ package client import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" "net/http/httputil" @@ -22,6 +23,44 @@ import ( // See functions starting with `With...` in this package for more info. type RequestOpt func(req *http.Request) +// SyncCheckOpt is a functional option for use with SyncUntil which should return if +// the response satisfies the check, else return a human friendly error. +// The result object is the entire /sync response from this request. +type SyncCheckOpt func(clientUserID string, topLevelSyncJSON gjson.Result) error + +// SyncReq contains all the /sync request configuration options. The empty struct `SyncReq{}` is valid +// which will do a full /sync due to lack of a since token. +type SyncReq struct { + // A point in time to continue a sync from. This should be the next_batch token returned by an + // earlier call to this endpoint. + Since string + // The ID of a filter created using the filter API or a filter JSON object encoded as a string. + // The server will detect whether it is an ID or a JSON object by whether the first character is + // a "{" open brace. Passing the JSON inline is best suited to one off requests. Creating a + // filter using the filter API is recommended for clients that reuse the same filter multiple + // times, for example in long poll requests. + Filter string + // Controls whether to include the full state for all rooms the user is a member of. + // If this is set to true, then all state events will be returned, even if since is non-empty. + // The timeline will still be limited by the since parameter. In this case, the timeout parameter + // will be ignored and the query will return immediately, possibly with an empty timeline. + // If false, and since is non-empty, only state which has changed since the point indicated by + // since will be returned. + // By default, this is false. + FullState bool + // Controls whether the client is automatically marked as online by polling this API. If this + // parameter is omitted then the client is automatically marked as online when it uses this API. + // Otherwise if the parameter is set to “offline” then the client is not marked as being online + // when it uses this API. When set to “unavailable”, the client is marked as being idle. + // One of: [offline online unavailable]. + SetPresence string + // The maximum time to wait, in milliseconds, before returning this request. If no events + // (or other data) become available before this time elapses, the server will return a response + // with empty fields. + // By default, this is 1000 for Complement testing. + TimeoutMillis string // string for easier conversion to query params +} + type CSAPI struct { UserID string AccessToken string @@ -128,6 +167,101 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { return eventID } +// Perform a single /sync request with the given request options. To sync until something happens, +// see `SyncUntil`. +// +// Fails the test if the /sync request does not return 200 OK. +// Returns the top-level parsed /sync response JSON as well as the next_batch token from the response. +func (c *CSAPI) MustSync(t *testing.T, syncReq SyncReq) (gjson.Result, string) { + t.Helper() + query := url.Values{ + "timeout": []string{"1000"}, + } + // configure the HTTP request based on SyncReq + if syncReq.TimeoutMillis != "" { + query["timeout"] = []string{syncReq.TimeoutMillis} + } + if syncReq.Since != "" { + query["since"] = []string{syncReq.Since} + } + if syncReq.Filter != "" { + query["filter"] = []string{syncReq.Filter} + } + if syncReq.FullState { + query["full_state"] = []string{"true"} + } + if syncReq.SetPresence != "" { + query["set_presence"] = []string{syncReq.SetPresence} + } + res := c.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "sync"}, WithQueries(query)) + body := ParseJSON(t, res) + result := gjson.ParseBytes(body) + nextBatch := GetJSONFieldStr(t, body, "next_batch") + return result, nextBatch +} + +// MustSyncUntil blocks and continually calls /sync (advancing the since token) until all the +// checks options return true. Check options don't all need to return true on a single +// /sync request, they are cumulative. For example, this will repeatedly call /sync until the client +// sees the join event and the message in the same room: +// client.MustSyncUntil( +// t, SyncReq{}, client.SyncJoinedTo("!foo:bar"), client.SyncTimelineHas("!foo:bar", ...), +// ) +// Once a check option returns true it is removed from the list of checks and won't be called again. +// Will time out after CSAPI.SyncUntilTimeout. +func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheckOpt) { + t.Helper() + start := time.Now() + numResponsesReturned := 0 + // Print failing events in a defer() so we handle t.Fatalf in the same way as t.Errorf + var wasFailed = t.Failed() + var lastEvent *gjson.Result + timedOut := false + defer func() { + if !wasFailed && t.Failed() { + raw := "" + if lastEvent != nil { + raw = lastEvent.Raw + } + if !timedOut { + t.Logf("MustSyncUntil: failing event %s", raw) + } + } + }() + checkers := make([]struct { + check SyncCheckOpt + lastErr error + }, len(checks)) + for i := range checks { + c := checkers[i] + c.check = checks[i] + c.lastErr = fmt.Errorf("not run") + checkers[i] = c + } + for { + if time.Since(start) > c.SyncUntilTimeout { + timedOut = true + t.Fatalf("MustSyncUntil: timed out. Seen %d /sync responses", numResponsesReturned) + } + response, nextBatch := c.MustSync(t, syncReq) + syncReq.Since = nextBatch + numResponsesReturned += 1 + + for i := 0; i < len(checkers); i++ { + err := checkers[i].check(c.UserID, response) + if err == nil { + // check passed, removed from checkers + checkers = append(checkers[:i], checkers[i+1:]...) + i-- + } else { + c := checkers[i] + c.lastErr = err + checkers[i] = c + } + } + } +} + // SyncUntilTimelineHas is a wrapper around `SyncUntil`. // It blocks and continually calls `/sync` until // - we have joined the given room @@ -487,3 +621,66 @@ func GjsonEscape(in string) string { in = strings.ReplaceAll(in, "*", `\*`) return in } + +func SyncTimelineHas(roomID string, check func(gjson.Result) bool) SyncCheckOpt { + return func(clientUserID string, topLevelSyncJSON gjson.Result) error { + err := loopArray( + topLevelSyncJSON, "rooms.join."+GjsonEscape(roomID)+".timeline.events", check, + ) + if err == nil { + return nil + } + return fmt.Errorf("SyncTimelineHas(%s): %s", roomID, err) + } +} +func SyncInvitedTo(userID, roomID string) SyncCheckOpt { + return func(clientUserID string, topLevelSyncJSON gjson.Result) error { + // two forms which depend on what the client user is: + // - passively viewing an invite for a room you're joined to (timeline events) + // - actively being invited to a room. + if clientUserID == userID { + // active + err := loopArray( + topLevelSyncJSON, "rooms.invite."+GjsonEscape(roomID)+".invite_state.events", + func(ev gjson.Result) bool { + return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "invite" + }, + ) + if err != nil { + return fmt.Errorf("SyncInvitedTo(%s): %s", roomID, err) + } + return nil + } + // passive + return SyncTimelineHas(roomID, func(ev gjson.Result) bool { + return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "invite" + })(clientUserID, topLevelSyncJSON) + } +} +func SyncJoinedTo(userID, roomID string) SyncCheckOpt { + return SyncTimelineHas(roomID, func(ev gjson.Result) bool { + return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "join" + }) +} +func SyncGlobalAccountDataHas(check func(gjson.Result) bool) SyncCheckOpt { + return func(clientUserID string, topLevelSyncJSON gjson.Result) error { + return loopArray(topLevelSyncJSON, "account_data.events", check) + } +} + +func loopArray(object gjson.Result, key string, check func(gjson.Result) bool) error { + array := object.Get(key) + if !array.Exists() { + return fmt.Errorf("Key %s does not exist", key) + } + if !array.IsArray() { + return fmt.Errorf("Key %s exists but it isn't an array", key) + } + goArray := array.Array() + for _, ev := range goArray { + if check(ev) { + return nil + } + } + return fmt.Errorf("check function did not pass for %d elements", len(goArray)) +} From 4f380a3489dfee959ffdd319dd44349cd760551c Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 15:46:38 +0000 Subject: [PATCH 02/10] Remove SyncUntilInvited and SyncUntilJoined; add more docs --- internal/client/client.go | 88 ++++++++----------- tests/csapi/apidoc_room_members_test.go | 9 +- .../user_directory_display_names_test.go | 4 +- 3 files changed, 44 insertions(+), 57 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 7627329b..c9ad4bef 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -201,47 +201,42 @@ func (c *CSAPI) MustSync(t *testing.T, syncReq SyncReq) (gjson.Result, string) { } // MustSyncUntil blocks and continually calls /sync (advancing the since token) until all the -// checks options return true. Check options don't all need to return true on a single +// checks options return no error. Check options don't *all* need to return no error on a single // /sync request, they are cumulative. For example, this will repeatedly call /sync until the client // sees the join event and the message in the same room: -// client.MustSyncUntil( -// t, SyncReq{}, client.SyncJoinedTo("!foo:bar"), client.SyncTimelineHas("!foo:bar", ...), +// alice.MustSyncUntil( +// t, SyncReq{}, +// client.SyncJoinedTo(alice.UserID, "!foo:bar"), +// client.SyncTimelineHas("!foo:bar", (ev gjson.Result) bool { return ev.Type.Str == "m.room.message" }), // ) // Once a check option returns true it is removed from the list of checks and won't be called again. -// Will time out after CSAPI.SyncUntilTimeout. -func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheckOpt) { +// There is no ordering on the checkers. If you need ordering, call MustSyncUntil multiple times with +// a single checker, and reuse the returned since token. +// Will time out after CSAPI.SyncUntilTimeout. Returns the latest since token used. +func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheckOpt) string { t.Helper() start := time.Now() numResponsesReturned := 0 - // Print failing events in a defer() so we handle t.Fatalf in the same way as t.Errorf - var wasFailed = t.Failed() - var lastEvent *gjson.Result - timedOut := false - defer func() { - if !wasFailed && t.Failed() { - raw := "" - if lastEvent != nil { - raw = lastEvent.Raw - } - if !timedOut { - t.Logf("MustSyncUntil: failing event %s", raw) - } - } - }() checkers := make([]struct { - check SyncCheckOpt - lastErr error + check SyncCheckOpt + errs []string }, len(checks)) for i := range checks { c := checkers[i] c.check = checks[i] - c.lastErr = fmt.Errorf("not run") checkers[i] = c } + printErrors := func() string { + err := "Checkers: " + for _, c := range checkers { + err += strings.Join(c.errs, "\n") + err += ", " + } + return err + } for { if time.Since(start) > c.SyncUntilTimeout { - timedOut = true - t.Fatalf("MustSyncUntil: timed out. Seen %d /sync responses", numResponsesReturned) + t.Fatalf("MustSyncUntil: timed out. Seen %d /sync responses. %s", numResponsesReturned, printErrors()) } response, nextBatch := c.MustSync(t, syncReq) syncReq.Since = nextBatch @@ -255,10 +250,14 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck i-- } else { c := checkers[i] - c.lastErr = err + c.errs = append(c.errs, fmt.Sprintf("Response #%d: %s", numResponsesReturned, err)) checkers[i] = c } } + if len(checkers) == 0 { + // every checker has passed! + return syncReq.Since + } } } @@ -273,31 +272,6 @@ func (c *CSAPI) SyncUntilTimelineHas(t *testing.T, roomID string, check func(gjs c.SyncUntil(t, "", "", "rooms.join."+GjsonEscape(roomID)+".timeline.events", check) } -// SyncUntilInvitedTo is a wrapper around SyncUntil. -// It blocks and continually calls `/sync` until we've been invited to the given room. -// Will time out after CSAPI.SyncUntilTimeout. -func (c *CSAPI) SyncUntilInvitedTo(t *testing.T, roomID string) { - t.Helper() - check := func(event gjson.Result) bool { - return event.Get("type").Str == "m.room.member" && - event.Get("content.membership").Str == "invite" && - event.Get("state_key").Str == c.UserID - } - c.SyncUntil(t, "", "", "rooms.invite."+GjsonEscape(roomID)+".invite_state.events", check) -} - -// SyncUntilJoined is a wrapper around SyncUntil. -// It blocks and continually calls `/sync` until we've joined the given room. -// Will time out after CSAPI.SyncUntilTimeout. -func (c *CSAPI) SyncUntilJoined(t *testing.T, roomID string) { - t.Helper() - c.SyncUntilTimelineHas(t, roomID, func(event gjson.Result) bool { - return event.Get("type").Str == "m.room.member" && - event.Get("content.membership").Str == "join" && - event.Get("state_key").Str == c.UserID - }) -} - // SyncUntil blocks and continually calls /sync until // - the response contains a particular `key`, and // - its corresponding value is an array @@ -622,6 +596,7 @@ func GjsonEscape(in string) string { return in } +// Check that the timeline for `roomID` has an event which passes the check function. func SyncTimelineHas(roomID string, check func(gjson.Result) bool) SyncCheckOpt { return func(clientUserID string, topLevelSyncJSON gjson.Result) error { err := loopArray( @@ -633,6 +608,12 @@ func SyncTimelineHas(roomID string, check func(gjson.Result) bool) SyncCheckOpt return fmt.Errorf("SyncTimelineHas(%s): %s", roomID, err) } } + +// Checks that `userID` gets invited to `roomID`. +// +// This checks different parts of the /sync response depending on the client making the request. +// If the client is also the person being invited to the room then the 'invite' block will be inspected. +// If the client is different to the person being invited then the 'join' block will be inspected. func SyncInvitedTo(userID, roomID string) SyncCheckOpt { return func(clientUserID string, topLevelSyncJSON gjson.Result) error { // two forms which depend on what the client user is: @@ -657,11 +638,16 @@ func SyncInvitedTo(userID, roomID string) SyncCheckOpt { })(clientUserID, topLevelSyncJSON) } } + +// Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. func SyncJoinedTo(userID, roomID string) SyncCheckOpt { return SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "join" }) } + +// Calls the `check` function for each global account data event, and returns with success if the +// check function returns true. func SyncGlobalAccountDataHas(check func(gjson.Result) bool) SyncCheckOpt { return func(clientUserID string, topLevelSyncJSON gjson.Result) error { return loopArray(topLevelSyncJSON, "account_data.events", check) diff --git a/tests/csapi/apidoc_room_members_test.go b/tests/csapi/apidoc_room_members_test.go index e9502182..8017d05e 100644 --- a/tests/csapi/apidoc_room_members_test.go +++ b/tests/csapi/apidoc_room_members_test.go @@ -6,6 +6,7 @@ import ( "github.com/tidwall/gjson" "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/match" "github.com/matrix-org/complement/internal/must" ) @@ -116,12 +117,12 @@ func TestRoomMembers(t *testing.T) { alice.InviteRoom(t, roomID, bob.UserID) - bob.SyncUntilInvitedTo(t, roomID) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(bob.UserID, roomID)) bob.JoinRoom(t, roomID, nil) // Sync to make sure bob has joined - bob.SyncUntilJoined(t, roomID) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) stateKey := "" alice.SendEventSynced(t, roomID, b.Event{ @@ -151,11 +152,11 @@ func TestRoomMembers(t *testing.T) { bob.InviteRoom(t, roomID, alice.UserID) - alice.SyncUntilInvitedTo(t, roomID) + since := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, roomID)) alice.JoinRoom(t, roomID, nil) - alice.SyncUntilJoined(t, roomID) + alice.MustSyncUntil(t, client.SyncReq{Since: since}, client.SyncJoinedTo(alice.UserID, roomID)) }) }) } diff --git a/tests/csapi/user_directory_display_names_test.go b/tests/csapi/user_directory_display_names_test.go index fee4fdc6..649d631f 100644 --- a/tests/csapi/user_directory_display_names_test.go +++ b/tests/csapi/user_directory_display_names_test.go @@ -140,7 +140,7 @@ func TestRoomSpecificUsernameChange(t *testing.T) { }) // Alice waits until she sees the invite, then accepts. - alice.SyncUntilInvitedTo(t, privateRoom) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, privateRoom)) alice.JoinRoom(t, privateRoom, nil) // Alice reveals her private name to Bob @@ -169,7 +169,7 @@ func TestRoomSpecificUsernameAtJoin(t *testing.T) { // Alice waits until she sees the invite, then accepts. // When she accepts, she does so with a specific displayname. - alice.SyncUntilInvitedTo(t, privateRoom) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, privateRoom)) alice.JoinRoom(t, privateRoom, nil) // Alice reveals her private name to Bob From ed2de287ef9295a6b0b9823a535de279d9cdff77 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 16:29:06 +0000 Subject: [PATCH 03/10] Better docs and error messages --- internal/client/client.go | 62 ++++++++++++++++++------- tests/csapi/apidoc_room_members_test.go | 3 -- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index c9ad4bef..c9d8e1c1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -201,17 +201,38 @@ func (c *CSAPI) MustSync(t *testing.T, syncReq SyncReq) (gjson.Result, string) { } // MustSyncUntil blocks and continually calls /sync (advancing the since token) until all the -// checks options return no error. Check options don't *all* need to return no error on a single -// /sync request, they are cumulative. For example, this will repeatedly call /sync until the client -// sees the join event and the message in the same room: -// alice.MustSyncUntil( -// t, SyncReq{}, -// client.SyncJoinedTo(alice.UserID, "!foo:bar"), -// client.SyncTimelineHas("!foo:bar", (ev gjson.Result) bool { return ev.Type.Str == "m.room.message" }), -// ) -// Once a check option returns true it is removed from the list of checks and won't be called again. -// There is no ordering on the checkers. If you need ordering, call MustSyncUntil multiple times with -// a single checker, and reuse the returned since token. +// check functions return no error. Returns the final/latest since token. +// +// Initial /sync example: (no since token) +// bob.InviteRoom(t, roomID, alice.UserID) +// alice.JoinRoom(t, roomID, nil) +// alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, roomID)) +// +// Incremental /sync example: (test controls since token) +// since := alice.MustSyncUntil(t, client.SyncReq{TimeoutMillis: "0"}) // get a since token +// bob.InviteRoom(t, roomID, alice.UserID) +// since = alice.MustSyncUntil(t, client.SyncReq{Since: since}, client.SyncInvitedTo(alice.UserID, roomID)) +// alice.JoinRoom(t, roomID, nil) +// alice.MustSyncUntil(t, client.SyncReq{Since: since}, client.SyncJoinedTo(alice.UserID, roomID)) +// +// Checking multiple parts of /sync: +// alice.MustSyncUntil( +// t, client.SyncReq{}, +// client.SyncJoinedTo(alice.UserID, roomID), +// client.SyncJoinedTo(alice.UserID, roomID2), +// client.SyncJoinedTo(alice.UserID, roomID3), +// ) +// +// Check functions are unordered and independent. Once a check function returns true it is removed +// from the list of checks and won't be called again. +// +// In the unlikely event that you want all the checkers to pass *explicitly* in a single /sync +// response (e.g to assert some form of atomic update which updates multiple parts of the /sync +// response at once) then make your own checker function which does this. +// +// In the unlikely event that you need ordering on your checks, call MustSyncUntil multiple times +// with a single checker, and reuse the returned since token, as in the "Incremental sync" example. +// // Will time out after CSAPI.SyncUntilTimeout. Returns the latest since token used. func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheckOpt) string { t.Helper() @@ -227,7 +248,7 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck checkers[i] = c } printErrors := func() string { - err := "Checkers: " + err := "Checkers:\n" for _, c := range checkers { err += strings.Join(c.errs, "\n") err += ", " @@ -236,7 +257,7 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck } for { if time.Since(start) > c.SyncUntilTimeout { - t.Fatalf("MustSyncUntil: timed out. Seen %d /sync responses. %s", numResponsesReturned, printErrors()) + t.Fatalf("%s MustSyncUntil: timed out after %v. Seen %d /sync responses. %s", c.UserID, time.Since(start), numResponsesReturned, printErrors()) } response, nextBatch := c.MustSync(t, syncReq) syncReq.Since = nextBatch @@ -250,7 +271,7 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck i-- } else { c := checkers[i] - c.errs = append(c.errs, fmt.Sprintf("Response #%d: %s", numResponsesReturned, err)) + c.errs = append(c.errs, fmt.Sprintf("[t=%v] Response #%d: %s", time.Since(start), numResponsesReturned, err)) checkers[i] = c } } @@ -641,9 +662,16 @@ func SyncInvitedTo(userID, roomID string) SyncCheckOpt { // Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. func SyncJoinedTo(userID, roomID string) SyncCheckOpt { - return SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "join" - }) + return func(clientUserID string, topLevelSyncJSON gjson.Result) error { + // awkward wrapping to get the error message correct at the start :/ + err := SyncTimelineHas(roomID, func(ev gjson.Result) bool { + return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "join" + })(clientUserID, topLevelSyncJSON) + if err == nil { + return nil + } + return fmt.Errorf("SyncJoinedTo(%s,%s): %s", userID, roomID, err) + } } // Calls the `check` function for each global account data event, and returns with success if the diff --git a/tests/csapi/apidoc_room_members_test.go b/tests/csapi/apidoc_room_members_test.go index 8017d05e..644b3837 100644 --- a/tests/csapi/apidoc_room_members_test.go +++ b/tests/csapi/apidoc_room_members_test.go @@ -151,11 +151,8 @@ func TestRoomMembers(t *testing.T) { ) bob.InviteRoom(t, roomID, alice.UserID) - since := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, roomID)) - alice.JoinRoom(t, roomID, nil) - alice.MustSyncUntil(t, client.SyncReq{Since: since}, client.SyncJoinedTo(alice.UserID, roomID)) }) }) From b8fc53e19a33572669a7cbd561463b0877725918 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 17:44:49 +0000 Subject: [PATCH 04/10] Remove SyncUntilTimelineHas --- internal/client/client.go | 15 ++-------- tests/csapi/apidoc_room_members_test.go | 34 ++++----------------- tests/csapi/room_ban_test.go | 9 +----- tests/csapi/rooms_state_test.go | 9 +++--- tests/federation_room_event_auth_test.go | 6 ++-- tests/restricted_rooms_test.go | 38 +++++++++++------------- 6 files changed, 34 insertions(+), 77 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index c9d8e1c1..684d2fab 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -161,9 +161,9 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { body := ParseJSON(t, res) eventID := GetJSONFieldStr(t, body, "event_id") t.Logf("SendEventSynced waiting for event ID %s", eventID) - c.SyncUntilTimelineHas(t, roomID, func(r gjson.Result) bool { + c.MustSyncUntil(t, SyncReq{}, SyncTimelineHas(roomID, func(r gjson.Result) bool { return r.Get("event_id").Str == eventID - }) + })) return eventID } @@ -282,17 +282,6 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck } } -// SyncUntilTimelineHas is a wrapper around `SyncUntil`. -// It blocks and continually calls `/sync` until -// - we have joined the given room -// - we see an event in the room for which the `check` function returns True -// If the `check` function fails the test, the failing event will be automatically logged. -// Will time out after CSAPI.SyncUntilTimeout. -func (c *CSAPI) SyncUntilTimelineHas(t *testing.T, roomID string, check func(gjson.Result) bool) { - t.Helper() - c.SyncUntil(t, "", "", "rooms.join."+GjsonEscape(roomID)+".timeline.events", check) -} - // SyncUntil blocks and continually calls /sync until // - the response contains a particular `key`, and // - its corresponding value is an array diff --git a/tests/csapi/apidoc_room_members_test.go b/tests/csapi/apidoc_room_members_test.go index 644b3837..e9ff96d7 100644 --- a/tests/csapi/apidoc_room_members_test.go +++ b/tests/csapi/apidoc_room_members_test.go @@ -35,17 +35,7 @@ func TestRoomMembers(t *testing.T) { }, }) - bob.SyncUntilTimelineHas( - t, - roomID, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { - return false - } - must.EqualStr(t, ev.Get("content").Get("membership").Str, "join", "Bob failed to join the room") - return true - }, - ) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) }) // sytest: POST /join/:room_alias can join a room t.Run("POST /join/:room_alias can join a room", func(t *testing.T) { @@ -66,17 +56,7 @@ func TestRoomMembers(t *testing.T) { }, }) - bob.SyncUntilTimelineHas( - t, - roomID, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { - return false - } - must.EqualStr(t, ev.Get("content").Get("membership").Str, "join", "Bob failed to join the room") - return true - }, - ) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) }) // sytest: POST /join/:room_id can join a room t.Run("POST /join/:room_id can join a room", func(t *testing.T) { @@ -96,8 +76,7 @@ func TestRoomMembers(t *testing.T) { }, }) - bob.SyncUntilTimelineHas( - t, + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( roomID, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { @@ -106,7 +85,7 @@ func TestRoomMembers(t *testing.T) { must.EqualStr(t, ev.Get("content").Get("membership").Str, "join", "Bob failed to join the room") return true }, - ) + )) }) // sytest: Test that we can be reinvited to a room we created t.Run("Test that we can be reinvited to a room we created", func(t *testing.T) { @@ -140,15 +119,14 @@ func TestRoomMembers(t *testing.T) { alice.LeaveRoom(t, roomID) // Wait until alice has left the room - bob.SyncUntilTimelineHas( - t, + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( roomID, func(ev gjson.Result) bool { return ev.Get("type").Str == "m.room.member" && ev.Get("content.membership").Str == "leave" && ev.Get("state_key").Str == alice.UserID }, - ) + )) bob.InviteRoom(t, roomID, alice.UserID) since := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, roomID)) diff --git a/tests/csapi/room_ban_test.go b/tests/csapi/room_ban_test.go index 7823d52b..cbb27b9e 100644 --- a/tests/csapi/room_ban_test.go +++ b/tests/csapi/room_ban_test.go @@ -3,8 +3,6 @@ package csapi_tests import ( "testing" - "github.com/tidwall/gjson" - "github.com/matrix-org/complement/internal/b" "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/match" @@ -59,12 +57,7 @@ func TestNotPresentUserCannotBanOthers(t *testing.T) { }, }) - // todo: replace with `SyncUntilJoined` - bob.SyncUntilTimelineHas(t, roomID, func(event gjson.Result) bool { - return event.Get("type").Str == "m.room.member" && - event.Get("content.membership").Str == "join" && - event.Get("state_key").Str == bob.UserID - }) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) res := charlie.DoFunc(t, "POST", []string{"_matrix", "client", "r0", "rooms", roomID, "ban"}, client.WithJSONBody(t, map[string]interface{}{ "user_id": bob.UserID, diff --git a/tests/csapi/rooms_state_test.go b/tests/csapi/rooms_state_test.go index 4579f5c3..06a0bf4a 100644 --- a/tests/csapi/rooms_state_test.go +++ b/tests/csapi/rooms_state_test.go @@ -6,6 +6,7 @@ import ( "github.com/tidwall/gjson" "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/must" ) @@ -32,20 +33,20 @@ func TestRoomCreationReportsEventsToMyself(t *testing.T) { t.Run("Room creation reports m.room.create to myself", func(t *testing.T) { t.Parallel() alice := deployment.Client(t, "hs1", userID) - alice.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.create" { return false } must.EqualStr(t, ev.Get("sender").Str, userID, "wrong sender") must.EqualStr(t, ev.Get("content").Get("creator").Str, userID, "wrong content.creator") return true - }) + })) }) // sytest: Room creation reports m.room.member to myself t.Run("Room creation reports m.room.member to myself", func(t *testing.T) { t.Parallel() alice := deployment.Client(t, "hs1", userID) - alice.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" { return false } @@ -53,7 +54,7 @@ func TestRoomCreationReportsEventsToMyself(t *testing.T) { must.EqualStr(t, ev.Get("state_key").Str, userID, "wrong state_key") must.EqualStr(t, ev.Get("content").Get("membership").Str, "join", "wrong content.membership") return true - }) + })) }) }) } diff --git a/tests/federation_room_event_auth_test.go b/tests/federation_room_event_auth_test.go index 84f3906d..2777c5a1 100644 --- a/tests/federation_room_event_auth_test.go +++ b/tests/federation_room_event_auth_test.go @@ -15,6 +15,7 @@ import ( "github.com/tidwall/gjson" "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/federation" "github.com/matrix-org/complement/internal/must" ) @@ -208,13 +209,12 @@ func TestInboundFederationRejectsEventsWithRejectedAuthEvents(t *testing.T) { t.Logf("Sent transaction; awaiting arrival") // wait for alice to receive sentinelEvent - alice.SyncUntilTimelineHas( - t, + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room.RoomID, func(ev gjson.Result) bool { return ev.Get("event_id").Str == sentinelEvent.EventID() }, - ) + )) // now inspect the results. Each of the rejected events should give a 404 for /event t.Run("Outlier should be rejected", func(t *testing.T) { diff --git a/tests/restricted_rooms_test.go b/tests/restricted_rooms_test.go index 4daa8fe8..fb7ec556 100644 --- a/tests/restricted_rooms_test.go +++ b/tests/restricted_rooms_test.go @@ -108,13 +108,14 @@ func checkRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, a // Wait until Alice sees Bob leave the allowed room. This ensures that Alice's HS // has processed the leave before Bob tries rejoining, so that it rejects his // attempt to join the room. - alice.SyncUntilTimelineHas(t, allowed_room, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != bob.UserID { - return false - } + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( + allowed_room, func(ev gjson.Result) bool { + if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != bob.UserID { + return false + } - return ev.Get("content").Get("membership").Str == "leave" - }) + return ev.Get("content").Get("membership").Str == "leave" + })) failJoinRoom(t, bob, room, "hs1") }) @@ -266,8 +267,7 @@ func TestRestrictedRoomsRemoteJoinLocalUser(t *testing.T) { // Ensure that the join comes down sync on hs2. Note that we want to ensure hs2 // accepted the event. - charlie.SyncUntilTimelineHas( - t, + charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { @@ -278,7 +278,7 @@ func TestRestrictedRoomsRemoteJoinLocalUser(t *testing.T) { return true }, - ) + )) // Raise the power level so that users on hs1 can invite people and then leave // the room. @@ -296,8 +296,7 @@ func TestRestrictedRoomsRemoteJoinLocalUser(t *testing.T) { charlie.LeaveRoom(t, room) // Ensure the events have synced to hs1. - alice.SyncUntilTimelineHas( - t, + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { @@ -307,7 +306,7 @@ func TestRestrictedRoomsRemoteJoinLocalUser(t *testing.T) { return true }, - ) + )) // Have bob leave and rejoin. This should still work even though hs2 isn't in // the room anymore! @@ -391,8 +390,7 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { charlie.JoinRoom(t, room, []string{"hs2", "hs1"}) // Double check that the join was authorised via hs1. - bob.SyncUntilTimelineHas( - t, + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { @@ -403,7 +401,7 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { return true }, - ) + )) // Bump the power-level of bob. alice.SendEventSynced(t, room, b.Event{ @@ -422,8 +420,7 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { charlie.LeaveRoom(t, room) // Ensure the events have synced to hs2. - bob.SyncUntilTimelineHas( - t, + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { @@ -431,7 +428,7 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { } return ev.Get("content").Get("membership").Str == "leave" }, - ) + )) // Bob leaves the allowed room so that hs2 doesn't know if Charlie is in the // allowed room or not. @@ -445,8 +442,7 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { charlie.JoinRoom(t, room, []string{"hs2", "hs1"}) // Double check that the join was authorised via hs1. - bob.SyncUntilTimelineHas( - t, + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { @@ -457,5 +453,5 @@ func TestRestrictedRoomsRemoteJoinFailOver(t *testing.T) { return true }, - ) + )) } From fba3e4eb53a9caaf5bd57be3217e1624f9c7b094 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 18:02:46 +0000 Subject: [PATCH 05/10] Log the failing array --- internal/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/client/client.go b/internal/client/client.go index 684d2fab..a8b069f2 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -685,5 +685,5 @@ func loopArray(object gjson.Result, key string, check func(gjson.Result) bool) e return nil } } - return fmt.Errorf("check function did not pass for %d elements", len(goArray)) + return fmt.Errorf("check function did not pass for %d elements: %v", len(goArray), array.Raw) } From bde4494a72a9b414baff5f114a40776e046f05c0 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 18:11:40 +0000 Subject: [PATCH 06/10] Convert ignored users tests --- tests/csapi/ignored_users_test.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/csapi/ignored_users_test.go b/tests/csapi/ignored_users_test.go index f3b42d37..821cc3bf 100644 --- a/tests/csapi/ignored_users_test.go +++ b/tests/csapi/ignored_users_test.go @@ -36,13 +36,7 @@ func TestInviteFromIgnoredUsersDoesNotAppearInSync(t *testing.T) { }) // Alice waits to see the join event. - alice.SyncUntilTimelineHas( - t, publicRoom, func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && - ev.Get("state_key").Str == alice.UserID && - ev.Get("content.membership").Str == "join" - }, - ) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, publicRoom)) // Alice ignores Bob. alice.MustDoFunc( @@ -57,14 +51,13 @@ func TestInviteFromIgnoredUsersDoesNotAppearInSync(t *testing.T) { ) // Alice waits to see that the ignore was successful. - sinceJoinedAndIgnored := alice.SyncUntilGlobalAccountDataHas( - t, + sinceJoinedAndIgnored := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncGlobalAccountDataHas( func(ev gjson.Result) bool { t.Logf(ev.Raw + "\n") return ev.Get("type").Str == "m.ignored_user_list" && ev.Get("content.ignored_users."+client.GjsonEscape(bob.UserID)).Exists() }, - ) + )) // Bob invites Alice to a private room. bobRoom := bob.CreateRoom(t, map[string]interface{}{ @@ -79,7 +72,7 @@ func TestInviteFromIgnoredUsersDoesNotAppearInSync(t *testing.T) { }) // Alice waits until she's seen Chris's invite. - alice.SyncUntilInvitedTo(t, chrisRoom) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, chrisRoom)) // We re-request the sync with a `since` token. We should see Chris's invite, but not Bob's. queryParams := url.Values{ From ad43d899f017a57458a92cefbafe71265b2d4223 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 9 Dec 2021 18:27:26 +0000 Subject: [PATCH 07/10] Remove SyncUntil and fixup MSCs --- internal/client/client.go | 66 ++------------------------ tests/csapi/apidoc_room_create_test.go | 7 +-- tests/msc2403_test.go | 66 +++++++++++--------------- tests/msc2716_test.go | 14 +++--- 4 files changed, 38 insertions(+), 115 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index c9f8efe5..2c1b0589 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -23,7 +23,7 @@ import ( // See functions starting with `With...` in this package for more info. type RequestOpt func(req *http.Request) -// SyncCheckOpt is a functional option for use with SyncUntil which should return if +// SyncCheckOpt is a functional option for use with MustSyncUntil which should return if // the response satisfies the check, else return a human friendly error. // The result object is the entire /sync response from this request. type SyncCheckOpt func(clientUserID string, topLevelSyncJSON gjson.Result) error @@ -66,7 +66,7 @@ type CSAPI struct { AccessToken string BaseURL string Client *http.Client - // how long are we willing to wait for SyncUntil.... calls + // how long are we willing to wait for MustSyncUntil.... calls SyncUntilTimeout time.Duration // True to enable verbose logging Debug bool @@ -168,7 +168,7 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { } // Perform a single /sync request with the given request options. To sync until something happens, -// see `SyncUntil`. +// see `MustSyncUntil`. // // Fails the test if the /sync request does not return 200 OK. // Returns the top-level parsed /sync response JSON as well as the next_batch token from the response. @@ -282,66 +282,6 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck } } -// SyncUntil blocks and continually calls /sync until -// - the response contains a particular `key`, and -// - its corresponding value is an array -// - some element in that array makes the `check` function return true. -// If the `check` function fails the test, the failing event will be automatically logged. -// Will time out after CSAPI.SyncUntilTimeout. -// -// Returns the `next_batch` token from the last /sync response. This can be passed as -// `since` to sync from this point forward only. -func (c *CSAPI) SyncUntil(t *testing.T, since, filter, key string, check func(gjson.Result) bool) string { - t.Helper() - start := time.Now() - checkCounter := 0 - // Print failing events in a defer() so we handle t.Fatalf in the same way as t.Errorf - var wasFailed = t.Failed() - var lastEvent *gjson.Result - timedOut := false - defer func() { - if !wasFailed && t.Failed() { - raw := "" - if lastEvent != nil { - raw = lastEvent.Raw - } - if !timedOut { - t.Logf("SyncUntil: failing event %s", raw) - } - } - }() - for { - if time.Since(start) > c.SyncUntilTimeout { - timedOut = true - t.Fatalf("SyncUntil: timed out. Called check function %d times", checkCounter) - } - query := url.Values{ - "timeout": []string{"1000"}, - } - if since != "" { - query["since"] = []string{since} - } - if filter != "" { - query["filter"] = []string{filter} - } - res := c.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "sync"}, WithQueries(query)) - body := ParseJSON(t, res) - since = GetJSONFieldStr(t, body, "next_batch") - keyRes := gjson.GetBytes(body, key) - if keyRes.IsArray() { - events := keyRes.Array() - for i, ev := range events { - lastEvent = &events[i] - if check(ev) { - return since - } - wasFailed = t.Failed() - checkCounter++ - } - } - } -} - //RegisterUser will register the user with given parameters and // return user ID & access token, and fail the test on network error func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, accessToken string) { diff --git a/tests/csapi/apidoc_room_create_test.go b/tests/csapi/apidoc_room_create_test.go index 933b4fcf..167ca983 100644 --- a/tests/csapi/apidoc_room_create_test.go +++ b/tests/csapi/apidoc_room_create_test.go @@ -159,10 +159,5 @@ func TestRoomCreateWithInvites(t *testing.T) { "invite": []string{bob.UserID}, }) - bob.SyncUntil(t, "", "", "rooms.invite."+client.GjsonEscape(roomID)+".invite_state.events", func(event gjson.Result) bool { - return event.Get("type").Str == "m.room.member" && - event.Get("content.membership").Str == "invite" && - event.Get("state_key").Str == bob.UserID && - event.Get("sender").Str == alice.UserID - }) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(bob.UserID, roomID)) } diff --git a/tests/msc2403_test.go b/tests/msc2403_test.go index 01c83bd9..62077376 100644 --- a/tests/msc2403_test.go +++ b/tests/msc2403_test.go @@ -148,14 +148,14 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki } } - inRoomUser.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != knockingUser.UserID { return false } must.EqualStr(t, ev.Get("content").Get("reason").Str, testKnockReason, "incorrect reason for knock") must.EqualStr(t, ev.Get("content").Get("membership").Str, "knock", "incorrect membership for knocking user") return true - }) + })) }) if !testFederation { @@ -163,7 +163,7 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki t.Run("A user that has knocked on a local room can rescind their knock and then knock again", func(t *testing.T) { // We need to carry out an incremental sync after knocking in order to get leave information // Carry out an initial sync here and save the since token - since := doInitialSync(t, knockingUser) + _, since := knockingUser.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) // Rescind knock knockingUser.MustDo( @@ -179,19 +179,20 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki // Use our sync token from earlier to carry out an incremental sync. Initial syncs may not contain room // leave information for obvious reasons - knockingUser.SyncUntil( - t, - since, - "", - "rooms.leave."+client.GjsonEscape(roomID)+".timeline.events", - func(ev gjson.Result) bool { + knockingUser.MustSyncUntil(t, client.SyncReq{Since: since}, func(clientUserID string, topLevelSyncJSON gjson.Result) error { + events := topLevelSyncJSON.Get("rooms.leave." + client.GjsonEscape(roomID) + ".timeline.events") + if !events.Exists() { + return fmt.Errorf("no leave section for room %s", roomID) + } + for _, ev := range events.Array() { if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != knockingUser.UserID { - return false + continue } must.EqualStr(t, ev.Get("content").Get("membership").Str, "leave", "expected leave membership after rescinding a knock") - return true - }, - ) + return nil + } + return fmt.Errorf("leave timeline for %s doesn't have leave event for %s", roomID, knockingUser.UserID) + }) // Knock again to return us to the knocked state knockOnRoomSynced(t, knockingUser, roomID, "Let me in... again?", []string{"hs1"}) @@ -218,11 +219,11 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki ) // Wait until the leave membership event has come down sync - inRoomUser.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != knockingUser.UserID || ev.Get("content").Get("membership").Str != "leave" - }) + })) // Knock again knockOnRoomSynced(t, knockingUser, roomID, "Pleeease let me in?", []string{"hs1"}) @@ -262,11 +263,11 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki ) // Wait until the invite membership event has come down sync - inRoomUser.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != knockingUser.UserID || ev.Get("content").Get("membership").Str != "invite" - }) + })) }) t.Run("A user cannot knock on a room they are already in", func(t *testing.T) { @@ -294,11 +295,11 @@ func knockingBetweenTwoUsersTest(t *testing.T, roomID string, inRoomUser, knocki ) // Wait until the ban membership event has come down sync - inRoomUser.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != knockingUser.UserID || ev.Get("content").Get("membership").Str != "ban" - }) + })) knockOnRoomWithStatus(t, knockingUser, roomID, "I didn't mean it!", []string{"hs1"}, 403) }) @@ -311,17 +312,15 @@ func knockOnRoomSynced(t *testing.T, c *client.CSAPI, roomID, reason string, ser knockOnRoomWithStatus(t, c, roomID, reason, serverNames, 200) // The knock should have succeeded. Block until we see the knock appear down sync - c.SyncUntil( - t, - "", - "", - "rooms.knock."+client.GjsonEscape(roomID)+".knock_state.events", - func(ev gjson.Result) bool { + c.MustSyncUntil(t, client.SyncReq{}, func(clientUserID string, topLevelSyncJSON gjson.Result) error { + events := topLevelSyncJSON.Get("rooms.knock." + client.GjsonEscape(roomID) + ".knock_state.events") + if events.Exists() && events.IsArray() { // We don't currently define any required state event types to be sent. // If we've reached this point, then an entry for this room was found - return true - }, - ) + return nil + } + return fmt.Errorf("no knock section for room %s", roomID) + }) } // knockOnRoomWithStatus will knock on a given room on the behalf of a user. @@ -361,17 +360,6 @@ func knockOnRoomWithStatus(t *testing.T, c *client.CSAPI, roomID, reason string, }) } -// doInitialSync will carry out an initial sync and return the next_batch token -func doInitialSync(t *testing.T, c *client.CSAPI) string { - query := url.Values{ - "timeout": []string{"1000"}, - } - res := c.DoFunc(t, "GET", []string{"_matrix", "client", "r0", "sync"}, client.WithQueries(query)) - body := client.ParseJSON(t, res) - since := client.GetJSONFieldStr(t, body, "next_batch") - return since -} - // TestKnockRoomsInPublicRoomsDirectory will create a knock room, attempt to publish it to the public rooms directory, // and then check that the room appears in the directory. The room's entry should also have a 'join_rule' field // representing a knock room. For sanity-checking, this test will also create a public room and ensure it has a diff --git a/tests/msc2716_test.go b/tests/msc2716_test.go index db5317f6..0582c500 100644 --- a/tests/msc2716_test.go +++ b/tests/msc2716_test.go @@ -236,7 +236,7 @@ func TestImportHistoricalMessages(t *testing.T) { // Get a /sync `since` pagination token we can try paginating from later // on - since := doInitialSync(t, alice) + _, since := alice.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) // Import a historical event batchSendRes := batchSendHistoricalMessages( @@ -263,13 +263,13 @@ func TestImportHistoricalMessages(t *testing.T) { // eventIDAfterHistoricalImport without any the // historicalEventIDs/historicalStateEventIDs in between, we're probably // safe to assume it won't sync. - alice.SyncUntil(t, since, "", "rooms.join."+client.GjsonEscape(roomID)+".timeline.events", func(r gjson.Result) bool { + alice.MustSyncUntil(t, client.SyncReq{Since: since}, client.SyncTimelineHas(roomID, func(r gjson.Result) bool { if includes(r.Get("event_id").Str, historicalEventIDs) || includes(r.Get("event_id").Str, historicalStateEventIDs) { t.Fatalf("We should not see the %s historical event in /sync response but it was present", r.Get("event_id").Str) } return r.Get("event_id").Str == eventIDAfterHistoricalImport - }) + })) }) t.Run("Batch send endpoint only returns state events that we passed in via state_events_at_start", func(t *testing.T) { @@ -633,9 +633,9 @@ func TestImportHistoricalMessages(t *testing.T) { insertionSendBody := client.ParseJSON(t, insertionSendRes) insertionEventID := client.GetJSONFieldStr(t, insertionSendBody, "event_id") // Make sure the insertion event has reached the homeserver - alice.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("event_id").Str == insertionEventID - }) + })) // eventIDsAfter createMessagesInRoom(t, alice, roomID, 3) @@ -1128,9 +1128,9 @@ func sendMarkerAndEnsureBackfilled(t *testing.T, as *client.CSAPI, c *client.CSA markerEventID = client.GetJSONFieldStr(t, markerSendBody, "event_id") // Make sure the marker event has reached the remote homeserver - c.SyncUntilTimelineHas(t, roomID, func(ev gjson.Result) bool { + c.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { return ev.Get("event_id").Str == markerEventID - }) + })) // Make sure all of the base insertion event has been backfilled // after the marker was received From b195089233f3fac5b35839c41245968fb57fc8a1 Mon Sep 17 00:00:00 2001 From: kegsay Date: Thu, 9 Dec 2021 18:28:28 +0000 Subject: [PATCH 08/10] Update internal/client/client.go Co-authored-by: David Robertson --- internal/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/client/client.go b/internal/client/client.go index 2c1b0589..b3dff15e 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -607,7 +607,7 @@ func SyncJoinedTo(userID, roomID string) SyncCheckOpt { } // Calls the `check` function for each global account data event, and returns with success if the -// check function returns true. +// `check` function returns true for at least one event. func SyncGlobalAccountDataHas(check func(gjson.Result) bool) SyncCheckOpt { return func(clientUserID string, topLevelSyncJSON gjson.Result) error { return loopArray(topLevelSyncJSON, "account_data.events", check) From 661a9cd2ef99bf80a3a74f9739971436e6355313 Mon Sep 17 00:00:00 2001 From: kegsay Date: Fri, 10 Dec 2021 10:07:45 +0000 Subject: [PATCH 09/10] Update internal/client/client.go Co-authored-by: Eric Eastwood --- internal/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/client/client.go b/internal/client/client.go index b3dff15e..e4d5bc50 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -628,5 +628,5 @@ func loopArray(object gjson.Result, key string, check func(gjson.Result) bool) e return nil } } - return fmt.Errorf("check function did not pass for %d elements: %v", len(goArray), array.Raw) + return fmt.Errorf("check function did not pass while iterating over %d elements: %v", len(goArray), array.Raw) } From e8fa2d3f324ab6b1adcc9d0771b54bd75fe5e925 Mon Sep 17 00:00:00 2001 From: kegsay Date: Fri, 10 Dec 2021 10:07:52 +0000 Subject: [PATCH 10/10] Update internal/client/client.go Co-authored-by: Eric Eastwood --- internal/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/client/client.go b/internal/client/client.go index e4d5bc50..181797c1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -251,7 +251,7 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck err := "Checkers:\n" for _, c := range checkers { err += strings.Join(c.errs, "\n") - err += ", " + err += ", \n" } return err }