Proposal to improve /members and /joined_rooms

[richvdh]

Rendered
Author: @turt2live
this was converted from an issue to a PR using github's tooling, which apparently changes the author

Related: https://github.com/matrix-org/matrix-doc/issues/1123

[KitsuneRal]

As a client author, I very much welcome this. However, I need /members to get one more parameter, a sync batch token so that the list of members could somehow be related to what is received over /sync. Without that I have to stop the sync loop while I'm requesting /members to avoid a race condition.

[turt2live]

Including sync tokens is out of scope for this proposal. I'm pretty sure you could use the context API to do what you're looking for anyways.

[t3chguy]

or the /messages API with a Filter for m.room.member

[richvdh]

A +1 from me for Option 1 here. Options 2 and 3 feel somewhat overblown.

[richvdh]

Am I right in understanding that this proposal has not yet been implemented in a matrix server? (Indeed, no agreement has yet been reached on what to do.) I am surprised at @KitsuneRal 's assertion that he has implemented client-side features which rely on it...

[turt2live]

This proposal hasn't been implemented anywhere, although part of lazy loading overlaps with this proposal. I have to figure out what exactly needs to change here to accommodate the lazy laoding changes still.

[KitsuneRal]

Uhm, well, then that probably means me sending at= to the server completely in vain.

[erikjohnston]

Synapse does implement at= as part of lazy loading: https://github.com/matrix-org/synapse/blob/master/synapse/rest/client/v1/room.py#L396

[erikjohnston]

This proposal doesn't address the primary reason for /joined_members: to simply return the list of joined users without the member event; which significantly reduces load on the server. Given that, there are two options: 1) keep /joined_members or 2) add a query param to /members to be able to change the response format.

Personally, I'm in favour of keeping /joined_members for two reasons. Firstly, it feels like /members is starting to become a bit of a kitchen sink in terms of the number of options. Adding yet another option to change the response format feels sad, especially if we don't support changing response format anywhere else.

Secondly, and more importantly, the /joined_members API is designed to be "fast", and client/bridge implementers are relying on them being at least sufficiently fast that calling them won't get rate limited. This then means that that code path has to be sufficiently fast across all server implementations that they don't get rate limited, otherwise we'll end up in a world where some clients/bridges only really work with some server implementations.

In practice, its unlikely that a naive implementation of /members will be particularly fast, as its easy enough to implement all the options as post processing. This broadly means that we'll probably have to spec (or add a note to the spec) that that code path should be "fast", and I really think its easier to spec and feels more natural when its a separate API in the spec.

Having said all that I don't have an objection to adding enough knobs to /members to make it functionally basically equivalent to /joined_members (though perhaps without changing the event format). We can always add a quick note to the spec to indicate that /joined_members is basically just an optimised fast path.

TL;DR: I think APIs that are designed to be called a lot and need to be "fast" should be separate APIs (with a note in the spec to indicate that fact)

---

Separately, if we're doing filtering I'd prefer we be consistent and use the filter param, at which point I don't really see why we'd not just use /state with an appropriate filter. So broadly, I'm sort of happy with adding membership query param onto /member if they then basically become helper APIs for a doing a full /state with an appropriate filter.

Similarly, I like having /rooms if it basically has the same format as /sync so it becomes effectively a helper API for an initial /sync. Though for consistency then /rooms should probably then not include rooms you've left unless an include_leave query param has been specified.

[turt2live]

gah, merging branches is hard - reverted in 6ccc548

[turt2live]

Github won't let you reopen PRs, so #2895 exists now