Add SyncStateHas check function

[squahtx]

These will turn out to be useful when testing lazy-loading /syncs,
among other things.

[kegsay]

I don't feel this PR should merge in its current state. I'd like to see more use cases where this functionality would be useful, and then we can see how this can be re-designed to work in a clear and consise way.

[kegsay]

We need to be careful here, because this can be misused. The state section of the room contains the state at the start of the timeline, so it's entirely possible that the state to check is in the timeline and not the state. We should be very careful about checking one or the other. What is the use case here?

[squahtx]

Ultimately I would like to check that a lazy_load_members /sync response includes the membership of a given sender in the state section. The sender joined the room before the syncing user and their membership is not expected to appear in the timeline. The planned usage looks like this:

syncRes, _ := alice.MustSync(t,
	client.SyncReq{
		Since:  previousSyncToken,
		Filter: buildLazyLoadingSyncFilter(nil),
	},
)

err = client.SyncTimelineHasEventID(serverRoom.RoomID, event.EventID())(alice.UserID, syncRes)
if err != nil {
	t.Errorf("Did not find message in lazy-loading /sync response: %s", err)
}

err := client.SyncStateHasStateKey(serverRoom.RoomID, "m.room.member", event.Sender())(alice.UserID, syncRes)
if err != nil {
	t.Errorf("Did not find %s's m.room.member event in lazy-loading /sync response: %s", event.Sender(), err)
}

I'm avoiding MustSyncUntil because it wouldn't fail immediately if event appears in the timeline without the corresponding sender membership, and the message is expected to already be available via /sync.

[kegsay]

Please remove: this doesn't compose well at all. What if we wanted to check the timeline for an event ID? What about ephemeral data? Room account data? Then we'd end up with:

• SyncStateHasEventID
• SyncTimelineHasEventID
• SyncEphemeralHasEventID
• SyncRoomAccountDataHasEventID

I hope it's clear how unruly and unmanageable this ends up becoming, especially if you then want to check the type/state key like SyncStateHasStateKey does. It would be better if you added functions which match func(gjson.Result) bool and compose well e.g:

• func CheckEventIDIs(eventID) func(gjson.Result) bool
• func CheckEventTypeIs(eventType) func(gjson.Result) bool

This would end up looking something like:

SyncStateHas(roomID, CheckEventIDIs(eventID), CheckEventTypeIs(eventType))

But this is also not great because often you want to check a few fields, though which fields depends on the test. Also, what is the relationship between the check functions: are they ANDed or ORd? At this point, it is often easier just to write the check function yourself:

SyncStateHas(roomID, func(ev gjson.Result) bool {
    		return ev.Get("type").Str == eventType && ev.Get("state_key").Str == stateKey
})

which also allows the flexibility to check say the content field easily enough. This is why we, as of yet, do not have per-event check functions: the benefits are unclear and the assertion itself is unclear.

[squahtx]

Do note that:

• The events in the ephemeral and account_data sections do not have event_ids and we would not want to define SyncEphemeralHasEventID and SyncRoomAccountDataHasEventID as the spec stands.
• SyncTimelineHasEventID already exists in Complement and this PR follows the existing pattern.

Shall we remove SyncTimelineHasEventID then?

[kegsay]

Yes

[squahtx]

@kegsay The other SyncStateHas* functions have been removed, please re-review when you have time.
Alternatively, #442 includes everything from this PR, so the internal/client/client.go change in that PR can be reviewed instead.

[squahtx]

Thanks for re-reviewing!