New timeline API

[jplatte]

Status:

• [x] Basic message receiving
• [x] Basic back-pagination
• [x] Live edits
• [x] Live reactions
• [x] Server-side reaction aggregations
• [x] Server-side edit aggregations
• [x] JSON source of timeline items
• [x] Reaction redactions
• [x] Testing
• [x] Local echo
• [x] Encryption info

Not planned for this PR, but planned for later:

• Catch-all "raw" timeline item
• Decrypting existing previously-undecryptable events in the timeline when the key is received
• Storage

[jplatte]

Things I would like to discuss at the current stage:

• What do we want to do with the existing experimental-timeline flag?
  • We have an example plus the FFI crate activating it, for IDE experience to not be horrible when working on the new one, those need to allow both features to be active at the same time or make those dependencies optional (or possibly just the FFI one)
• How do we deal with thing like replies
  • I'm pretty sure we want users to be able to render the reply, even if they can't render the replied-to message yet
  • One approach: Don't make the replied-to message part of the timeline item, but offer a separate async function to obtain the model bits required to render it
  • Another approach: Make the replied-to message part of the timeline item, but have three states: [no reply, reply loading, reply] and automatically obtain the required model bits when encountering a reply, then update the timeline item again
  • The same question applies to media, although the answer could be different (the first approach seems more reasonable there because you might want to do the media loading at different times depending on network conditions)

[poljar]

Things I would like to discuss at the current stage:

* What do we want to do with the existing `experimental-timeline` flag?
  
  * We have an example plus the FFI crate activating it, for IDE experience to not be horrible when working on the new one, those need to allow both features to be active at the same time or make those dependencies optional (or possibly just the FFI one)

I think we should just remove the feature from the codebase. The example should be updated to use the new feature. For the FFI I would expect the FFI crate to be updated and hopefully tested in Element X as part of this PR. This does mean some coordination with the Element X team and live testing but should overall assure us that the PR works and is what Element X wants.

  * One approach: Don't make the replied-to message part of the timeline item, but offer a separate async function to obtain the model bits required to render it
  * Another approach: Make the replied-to message part of the timeline item, but have three states: [no reply, reply loading, reply] and automatically obtain the required model bits when encountering a reply, then update the timeline item again
  * The same question applies to media, although the answer could be different (the first approach seems more reasonable there because you might want to do the media loading at different times depending on network conditions)

In both cases you can't be too sure if the client actually wants to actually render the individual item, i.e. is the message in view or not. Seems like any decision on whether we should fetch additional data from the server or not should be delegated to the client itself.

In both cases the client can and should display a placeholder until it has all the data to render things fully.

[jplatte]

Re. experimental-timeline, I was kind of hoping for this PR to not get huge, i.e. merge it before we add any FFI things. But there's definitely merit to getting it close to feature parity before merging, and replacing the old API immediately.

In both cases you can't be too sure if the client actually wants to actually render the individual item, i.e. is the message in view or not. Seems like any decision on whether we should fetch additional data from the server or not should be delegated to the client itself.

This is a good point. I guess we can still have the three-states thing though, have the client call timeline.load_reply(event_id) which doesn't return anything, but causes another update to that timeline item to be sent once the data is available.

[zecakeh]

We need the TimelineKey, TimelineItemContent, ReactionDetails, Message and PaginationOutcome types to be exported by the matrix-sdk crate for this to be usable :slightly_smiling_face: .

[jplatte]

Right. I've now removed the re-exports from the room module and made the room::timeline module public instead, since I don't think we should be cluttering the room module with all those symbols.

[zecakeh]

I'm guessing all timeline event types will be supported later? What is the plan with events that are not successfully deserialized?

[jplatte]

Yeah, the plan is to support all specced event types. Events that aren't successfully deserialized haven't been discussed so far, but I'm pretty sure we'll want to cover that somehow. Element Web has a dev mode thing where reactions (and I think edits too) show up as greyed-out extra entries in the timeline, I think we'll want to support something like that, maybe in the form of a "raw" timeline item that is added whenever an event doesn't otherwise produce a new timeline item.

[zecakeh]

We plan to do the same thing in Fractal in debug builds, that's why I was asking. This seems like a good solution.

One basic thing currently missing for TimelineItem is the access to origin_server_ts.

[jplatte]

I want to see whether bindings are generating correctly, but this isn't actually ready for review.

[codecov[bot]]

Codecov Report

Base: 72.77% // Head: 73.25% // Increases project coverage by +0.48% :tada:

Coverage data is based on head (ff8f732) compared to base (67d968d). Patch coverage: 76.78% of modified lines in pull request are covered.

:exclamation: Current head ff8f732 differs from pull request most recent head 533484f. Consider uploading reports for the commit 533484f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #940      +/-   ##
==========================================
+ Coverage   72.77%   73.25%   +0.48%     
==========================================
  Files         106      109       +3     
  Lines       11867    12145     +278     
==========================================
+ Hits         8636     8897     +261     
- Misses       3231     3248      +17     

Impacted Files	Coverage Δ
crates/matrix-sdk/src/room/mod.rs	35.71% <ø> (ø)
...ates/matrix-sdk/src/room/timeline/event_handler.rs	71.00% <71.00%> (ø)
crates/matrix-sdk/src/room/timeline/event_item.rs	84.61% <84.61%> (ø)
crates/matrix-sdk/src/room/timeline/mod.rs	85.96% <85.96%> (ø)
crates/matrix-sdk/src/room/common.rs	76.12% <100.00%> (+8.72%)	:arrow_up:
crates/matrix-sdk/src/error.rs	25.92% <0.00%> (-16.94%)	:arrow_down:
crates/matrix-sdk/src/client/login_builder.rs	22.42% <0.00%> (-4.68%)	:arrow_down:
...ix-sdk-crypto/src/types/events/room_key_request.rs	72.50% <0.00%> (-1.31%)	:arrow_down:
crates/matrix-sdk-crypto/src/requests.rs	77.46% <0.00%> (-0.92%)	:arrow_down:
crates/matrix-sdk-crypto/src/machine.rs	66.66% <0.00%> (-0.35%)	:arrow_down:
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[zecakeh]

This is currently used in this MR on Fractal: fractal!1136.

I'll try to update it frequently with new changes from this PR.

[jplatte]

Rebased on top of #1000 and cleaned up the implementation a bit (made possible by that PR and others). Also added support for accessing the raw JSON of the initial event. How to deal with the JSON of aggregated events is left as a FIXME, potentially one that won't be solved in this PR.

[jplatte]

I want to see how much coverage the single test amounts to, even if it's not yet fully working. So this isn't actually ready for review, I just want CI to run..

[jplatte]

This is now ready for review! I have removed the FFI from this branch, and will post it as a separate PR.

[zecakeh]

Like I said on matrix, currently we don't get the aggregated reactions. However if we forward the Relations from the event to the NewEventTimelineItem, it works, since the rest of the code is in place.

[zecakeh]

I can confirm that I get all the reactions now.