element-hq
Sliding sync: remove the list ops from the response
This isn't in MSC4186, sync ops don't make any sense in the API, nothing currently uses it, and it takes up a lot of space in the sync result.
We probably do want to indicate which rooms are in which lists, but that can be added later.
We probably do want to indicate which rooms are in which lists, but that can be added later.
Whilst I appreciate wanting to break things up, if you don't indicate which room is in which list it cripples SSS. It makes the use of filters entirely useless because if I make two lists I don't know which room is in which list (which room matches which filter). We can't just strip out this information and not replace it in the same PR imho.
We probably do want to indicate which rooms are in which lists, but that can be added later.
Whilst I appreciate wanting to break things up, if you don't indicate which room is in which list it cripples SSS. It makes the use of
filtersentirely useless because if I make two lists I don't know which room is in which list (which room matches which filter). We can't just strip out this information and not replace it in the same PR imho.
That's fair, though I'd say that rust sdk doesn't use the list ops and I'd be surprised if anyone could really depend on the current implementation (note list ops are not in the msc). I don't think there is much harm in removing list ops (that no one is using, or really can use) and punting how to encode what room matches which lists till later.
Particularly, there may be no reason to have multiple lists with different filters (but there are use cases for multiple lists with different configs, eg different timeline limit for the top N rooms). And if not, no reason to need to encode which rooms match which lists.
By the nature of them having multiple lists with different filters, they need a list of room_ids that pertain to each list.
It looks like they do process ops but it could just be a list of room_ids: [] too.
Worth noting those filters seem to pertain to information the client will see, so they can infer? Would be very interested in knowing if EW actually used the list ops for those purposes, or if that code is just legacy from the older implementation.
@dbkr maybe you know?
I'm not sure if EW is something we need to worry about not breaking right now though? It's all behind labs flags and things still I believe
Currently EW does still read the ops to know which room goes into which list. I'm currently trying to get SSS working in EW hence why I'm chiming in here.
Worth noting those filters seem to pertain to information the client will see, so they can infer?
Inferring is only possible if the client also fetches the data used to make the filtering decision, which in EW's case is:
- room tags (room account data)
- DM map (global account data)
- room type (create event)
- self membership (m.room.member event for
$ME)
This also then relies on the client and server having precisely the same algorithm to determine inclusion, which can be more annoying and subtle than at first glance (e.g if you have a DM entry with 2 rooms what do you do?).
That being said, EW does fetch this information so inferring should be possible.
Trying to rethink around filters: what we're actually doing here (in the absence of knowing which room went into which list) is controlling the priority order of which rooms to return initially. This is exactly what EW needs because it needs to populate the invite list / favourites / DMs (all of which are above "Rooms") immediately on load: if we just had 1 list then you might have a favourite with a very low bump_stamp because it is ancient which would pop into existence potentially minutes after login, depending how long the full room list pagination takes. By having >1 list, we can guarantee we see the first [0,N] entries with the highest bump_stamps per list.
This distinction (priority ordering vs filtering) is important. If you keep the concept of filters then you need to A) inform the client which rooms falls into which lists (the aforementioned room_ids), B) inform the client when rooms come out of that list (which implies some kind of DELETE op), C) manage the book keeping of all this on the server. If, however, you just have 1 list and you're controlling the priority of which room comes first, you don't need any of this, simplifying the overall API.
Yep, I don't really know - Kegan is the expert here, but this all sounds plausible. It's not completely ideal that the server & clients might have subtle differences / bugs that mean one think a room is in a list & one doesn't, but I think basically as long as the rooms that filter selects appear in the list, EW can just work out their categorisation as it always did pre-SS.
So tl;dr - you can remove the list ops from the response. We don't read them in EW anymore.
@erikjohnston @MadLittleMods It looks to me that this PR has fallen through the cracks a bit. Is it still waiting for team review, or does it need changes (other than conflict resolution) first?
@anoadragon453 Removed the review request. It needs some work before we can consider it again.