libQuotient icon indicating copy to clipboard operation
libQuotient copied to clipboard

Track historical state

Open KitsuneRal opened this issue 4 years ago • 1 comments

As of now, Quotient cannot calculate, e.g., a user's display name as-of a certain event in the past. Due to the lack of this feature Quaternion and all other Quotient-based clients did not track renames and avatar changes (see, e.g., https://github.com/quotient-im/Quaternion/issues/317) - a rename led to the user name getting updated throughout the whole past timeline and even the rename event itself doesn't show the previous display name. Technically it's still possible to backtrack on state events in the timeline without involving the library but it's somewhat cumbersome, potentially leads to performance degradation (unless the spec recommendation is applied - but who reads all corners of the spec these days...) so noone got to touching it. And apparently, the library needs the state events backtracking mechanism for its own purposes. The current 0.6.x code to resolve the user display name (necessary to maintain internal Room::Private::membersMap used for display name disambiguation) relies on the display name coming either from the state event content or, in the worst case, the previous content (unsigned and therefore, not trusted). However, if a user changes the avatar a couple of times, even the previous content of the latest m.room.member event is not going to contain the display name, and the current content will lose the display name at the first avatar change. Since 0.7 tries to not use prev_content as a fallback, the described case damages the current to-be-0.7 membersMap logic (introduced in dcef98f9), failing the well-intentioned assertion as a result. Instead of stopping on the assertion, the release build has a workaround that kind of repairs membersMap and goes on as if nothing happened. This potentially breaks disambiguation because membersMap still doesn't have the right member display name; but at least the client remains usable. To improve on this without workarounds, the library should track the past state events for a given member until the one with display name in its content is found (if it's not, it's a server's fault not to send one, and there's nothing the library can do). To optimise this procedure, Room::Private::currentState could be made a map not to one event pointer but to a vector of those, including all known state event for this event type + state key. In turn, clients could use this vector to quickly browse through states, assuming that those states have a defined location in the timeline (a separate comment on this will come), and show correct display names and avatars.

KitsuneRal avatar Dec 29 '20 09:12 KitsuneRal

The situation with relating a particular state event to the timeline position is somewhat complicated. For the background, Room has two "owning" event storages: baseState that stores events that came in the state sync block, and timeline, for the sync block of the same name. The intention is to correspond baseState to the timeline position just-before-0; but the actual scope where each state event from baseState spans over historical events (timeline position -1 and further negative), until a matching historical state event is found in the timeline - but this only works for events stored in baseState. The problem arises when we navigate back the history beyond that historical state event. If the rest of loaded history doesn't have another matching state event (and we haven't reached the root of the room history - RoomCreateEvent), the only way to determine the state in this segment is to look into prev_content of that oldest state event found in the history. Despite using prev_content it is probably the best approach, since getting the "authoritative" state content corresponding to that segment is non-trivial at best.

KitsuneRal avatar Dec 29 '20 09:12 KitsuneRal