Skip to content

Commit

Permalink
event cache/timeline: reuse the Paginator when running back-paginat…
Browse files Browse the repository at this point in the history
…ions (#3373)

* event cache: reuse the paginator internally

Fixes #3355.

* event cache: move the `pagination_token_notifier` into the `RoomPaginationData` as well

* event cache: introduce a `RoomPagination` API object and move code around

Only code motion. No changes in functionality.

* event cache: remove "paginate" (et al.) in `RoomPagination` method names

No changes in functionality, just renamings.

* event_cache/timeline: have the event cache handle restarting a back-pagination that failed under our feet

When a timeline reset happens while we're back-paginating, the event
cache method to run back pagination would return an success result
indicating that the pagination token disappeared. After thinking about
it, it's not the best API in the world; ideally, the backpagination
mechanism would restart automatically.

Now, this was handled in the timeline before, and the reason it was
handled there was because it was possible to back-paginate and ask for a
certain number of events. I've removed that feature, so that
back-pagination on a live timeline matches the capabilities of a
focused-timeline back-pagination: one can only ask for a given number of
*events*, not timeline items.

As a matter of fact, this simplifies the code a lot by removing many
data structures, that were also exposed (and unused, since recent
changes) in the FFI layer.

* Address review comments
  • Loading branch information
bnjbvr authored May 16, 2024
1 parent 21804ab commit c8f6fe4
Show file tree
Hide file tree
Showing 11 changed files with 747 additions and 969 deletions.
45 changes: 11 additions & 34 deletions bindings/matrix-sdk-ffi/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ use std::{collections::HashMap, fmt::Write as _, fs, sync::Arc};
use anyhow::{Context, Result};
use as_variant::as_variant;
use eyeball_im::VectorDiff;
use futures_util::{pin_mut, StreamExt};
use matrix_sdk::attachment::{
AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo,
BaseThumbnailInfo, BaseVideoInfo, Thumbnail,
use futures_util::{pin_mut, StreamExt as _};
use matrix_sdk::{
attachment::{
AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo,
BaseThumbnailInfo, BaseVideoInfo, Thumbnail,
},
event_cache::paginator::PaginatorState,
};
use matrix_sdk_ui::timeline::{EventItemOrigin, PaginationStatus, Profile, TimelineDetails};
use matrix_sdk_ui::timeline::{EventItemOrigin, Profile, TimelineDetails};
use mime::Mime;
use ruma::{
events::{
Expand Down Expand Up @@ -164,11 +167,11 @@ impl Timeline {
&self,
listener: Box<dyn PaginationStatusListener>,
) -> Result<Arc<TaskHandle>, ClientError> {
let mut subscriber = self.inner.back_pagination_status();
let (initial, mut subscriber) = self.inner.back_pagination_status();

Ok(Arc::new(TaskHandle::new(RUNTIME.spawn(async move {
// Send the current state even if it hasn't changed right away.
listener.on_update(subscriber.next_now());
listener.on_update(initial);

while let Some(status) = subscriber.next().await {
listener.on_update(status);
Expand Down Expand Up @@ -588,7 +591,7 @@ pub trait TimelineListener: Sync + Send {

#[uniffi::export(callback_interface)]
pub trait PaginationStatusListener: Sync + Send {
fn on_update(&self, status: PaginationStatus);
fn on_update(&self, status: PaginatorState);
}

#[derive(Clone, uniffi::Object)]
Expand Down Expand Up @@ -978,32 +981,6 @@ impl SendAttachmentJoinHandle {
}
}

#[derive(uniffi::Enum)]
pub enum PaginationOptions {
SimpleRequest { event_limit: u16, wait_for_token: bool },
UntilNumItems { event_limit: u16, items: u16, wait_for_token: bool },
}

impl From<PaginationOptions> for matrix_sdk_ui::timeline::PaginationOptions<'static> {
fn from(value: PaginationOptions) -> Self {
use matrix_sdk_ui::timeline::PaginationOptions as Opts;
let (wait_for_token, mut opts) = match value {
PaginationOptions::SimpleRequest { event_limit, wait_for_token } => {
(wait_for_token, Opts::simple_request(event_limit))
}
PaginationOptions::UntilNumItems { event_limit, items, wait_for_token } => {
(wait_for_token, Opts::until_num_items(event_limit, items))
}
};

if wait_for_token {
opts = opts.wait_for_token();
}

opts
}
}

/// A [`TimelineItem`](super::TimelineItem) that doesn't correspond to an event.
#[derive(uniffi::Enum)]
pub enum VirtualTimelineItem {
Expand Down
7 changes: 1 addition & 6 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

use std::{collections::BTreeSet, sync::Arc};

use eyeball::SharedObservable;
use futures_util::{pin_mut, StreamExt};
use matrix_sdk::{event_cache::RoomEventCacheUpdate, executor::spawn, Room};
use ruma::{events::AnySyncTimelineEvent, RoomVersionId};
Expand All @@ -28,10 +27,7 @@ use super::{
queue::send_queued_messages,
Error, Timeline, TimelineDropHandle, TimelineFocus,
};
use crate::{
timeline::{event_item::RemoteEventOrigin, PaginationStatus},
unable_to_decrypt_hook::UtdHookManager,
};
use crate::{timeline::event_item::RemoteEventOrigin, unable_to_decrypt_hook::UtdHookManager};

/// Builder that allows creating and configuring various parts of a
/// [`Timeline`].
Expand Down Expand Up @@ -299,7 +295,6 @@ impl TimelineBuilder {

let timeline = Timeline {
inner,
back_pagination_status: SharedObservable::new(PaginationStatus::Idle),
msg_sender,
event_cache: room_event_cache,
drop_handle: Arc::new(TimelineDropHandle {
Expand Down
5 changes: 0 additions & 5 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

use std::{path::PathBuf, pin::Pin, sync::Arc, task::Poll};

use eyeball::SharedObservable;
use eyeball_im::VectorDiff;
use futures_core::Stream;
use imbl::Vector;
Expand Down Expand Up @@ -96,7 +95,6 @@ pub use self::{
event_type_filter::TimelineEventTypeFilter,
inner::default_event_filter,
item::{TimelineItem, TimelineItemKind},
pagination::{PaginationOptions, PaginationOutcome, PaginationStatus},
polls::PollResult,
reactions::ReactionSenderData,
sliding_sync_ext::SlidingSyncRoomExt,
Expand Down Expand Up @@ -130,9 +128,6 @@ pub struct Timeline {

/// References to long-running tasks held by the timeline.
drop_handle: Arc<TimelineDropHandle>,

/// Observable for whether a backward pagination is currently running.
pub(crate) back_pagination_status: SharedObservable<PaginationStatus>,
}

// Implements hash etc
Expand Down
Loading

0 comments on commit c8f6fe4

Please sign in to comment.