Skip to content

Re-design client.SyncUntil#272

Merged
kegsay merged 11 commits intomasterfrom
kegan/sync-api-revamp
Dec 10, 2021
Merged

Re-design client.SyncUntil#272
kegsay merged 11 commits intomasterfrom
kegan/sync-api-revamp

Conversation

@kegsay
Copy link
Member

@kegsay kegsay commented Dec 9, 2021

See #271 for the rationale why this is being changed. All affected tests have been updated.

Using functional options similar to client.MustDoFunc, API usage now looks like:

Initial sync:

bob.InviteRoom(t, roomID, alice.UserID)
alice.JoinRoom(t, roomID, nil)
alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, roomID))

Incremental sync:

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))

The full list of check functions currently supported mirrors the removed functions:

  • SyncTimelineHas(roomID, func(gjson.Result) bool)
  • SyncInvitedTo(userID, roomID)
  • SyncJoinedTo(userID, roomID)
  • SyncGlobalAccountDataHas(func(gjson.Result) bool)

The addition of a client.SyncReq struct also now allows test writers to either do initial or incremental syncs. Previously, all the helper Sync functions would be doing initial syncs. Furthermore, helper functions for "X must be joined|invited to room Y" can now be done via a passive watching user in the room, whereas previously the user being invited|joined had to be the one syncing.

In addition, when MustSyncUntil fails, it produces more structured output like this:

@alice:hs1 MustSyncUntil: timed out after 5.042472596s. Seen 7 /sync responses. Checkers:
                [t=17.011919ms] Response #1: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): check function did not pass for 1 elements: [{"content":{"displayname":"Alice","membership":"invite"},"event_id":"$_b969RP9k7W_igBzH7u3a5J-gRjrpBAEcBvoae3GBuY","origin_server_ts":1639072742359,"sender":"@bob:hs1","state_key":"@alice:hs1","type":"m.room.member","unsigned":{"prev_content":{"membership":"leave"},"prev_sender":"@alice:hs1","replaces_state":"$zIyl36Gdk4y-yFKfI7xiNBDASjOamA6XjQHnF_Ra1Mc"}}]
                [t=27.172348ms] Response #2: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): check function did not pass for 1 elements: [{"content":{"avatar_url":"","displayname":"Alice","membership":"join"},"event_id":"$lD1wxscnoiptouGcSlNaFcos-aTarQhReKlKna4Q2nQ","origin_server_ts":1639072742422,"sender":"@alice:hs1","state_key":"@alice:hs1","type":"m.room.member","unsigned":{"prev_content":{"displayname":"Alice","membership":"invite"},"prev_sender":"@bob:hs1","replaces_state":"$_b969RP9k7W_igBzH7u3a5J-gRjrpBAEcBvoae3GBuY"}}]
                [t=1.028929806s] Response #3: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): Key rooms.join.!R8eqLIUTShHAAOMs:hs1.timeline.events does not exist
                [t=2.032334539s] Response #4: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): Key rooms.join.!R8eqLIUTShHAAOMs:hs1.timeline.events does not exist
                [t=3.034670555s] Response #5: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): Key rooms.join.!R8eqLIUTShHAAOMs:hs1.timeline.events does not exist
                [t=4.037416588s] Response #6: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): Key rooms.join.!R8eqLIUTShHAAOMs:hs1.timeline.events does not exist
                [t=5.042470504s] Response #7: SyncJoinedTo(no-body-here,!R8eqLIUTShHAAOMs:hs1): SyncTimelineHas(!R8eqLIUTShHAAOMs:hs1): Key rooms.join.!R8eqLIUTShHAAOMs:hs1.timeline.events does not exist

cc @clokep @richvdh @ShadowJonathan @MadLittleMods @DMRobertson as you have all recently contributed code to this project.

DMRobertson
DMRobertson reviewed Dec 9, 2021
View reviewed changes
kegsay and others added 2 commits December 9, 2021 18:27
@kegsay
Remove SyncUntil and fixup MSCs
ad43d89
@kegsay
Update internal/client/client.go
b195089 
Co-authored-by: David Robertson <davidr@element.io>
ShadowJonathan
ShadowJonathan approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@ShadowJonathan ShadowJonathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While a bit more verbose, overall this is definitely a better API

MadLittleMods
MadLittleMods reviewed Dec 9, 2021
View reviewed changes
MadLittleMods
MadLittleMods requested changes Dec 10, 2021
View reviewed changes
kegsay and others added 2 commits December 10, 2021 10:07
@kegsay @MadLittleMods
Update internal/client/client.go
661a9cd 
Co-authored-by: Eric Eastwood <erice@element.io>
@kegsay @MadLittleMods
Update internal/client/client.go
e8fa2d3 
Co-authored-by: Eric Eastwood <erice@element.io>
@kegsay kegsay merged commit 501589c into master Dec 10, 2021
This was referenced Dec 10, 2021
Closed
Closed
Closed
Closed
@MadLittleMods MadLittleMods mentioned this pull request Oct 27, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MadLittleMods MadLittleMods MadLittleMods requested changes

+2 more reviewers

@DMRobertson DMRobertson DMRobertson left review comments

@ShadowJonathan ShadowJonathan ShadowJonathan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kegsay @MadLittleMods @DMRobertson @ShadowJonathan