From 7856dab56d59ecb385730caf869a0efa297bdd4d Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 23 Apr 2024 19:02:17 +0200 Subject: [PATCH 1/3] timeline: add support for a focus mode in the timeline This introduces the `TimelineFocus`, a new enum to declare if the timeline is "live" aka looking at events from sync and displaying them as they come in, or focused on an event (e.g. after clicking a permalink). When in the second mode, the timeline can paginate forwards and backwards, without interacting with the event cache (as this would require some complicated reconciliation of known events with events received from pagination, with no guarantee that those events are event connected in whatever way). An event-focused timeline will also show edits/reactions/redactions in real-time (as the events are received from the sync), but will not show new timeline items, be they for local echoes or events received from the sync. --- .../src/room_list_service/mod.rs | 4 +- crates/matrix-sdk-ui/src/timeline/builder.rs | 62 ++++--- crates/matrix-sdk-ui/src/timeline/error.rs | 32 +++- .../src/timeline/event_handler.rs | 43 ++++- .../matrix-sdk-ui/src/timeline/inner/mod.rs | 173 ++++++++++++++++-- .../matrix-sdk-ui/src/timeline/inner/state.rs | 17 +- crates/matrix-sdk-ui/src/timeline/mod.rs | 34 ++-- .../matrix-sdk-ui/src/timeline/pagination.rs | 77 ++++++-- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 2 +- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 43 ++++- crates/matrix-sdk-ui/src/timeline/traits.rs | 10 +- .../tests/integration/timeline/pagination.rs | 39 ++-- labs/multiverse/src/main.rs | 7 +- 13 files changed, 430 insertions(+), 113 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index c8cfc818601..cea6d33d0db 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -95,6 +95,8 @@ use tokio::{ time::timeout, }; +use crate::timeline; + /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { @@ -553,7 +555,7 @@ pub enum Error { TimelineAlreadyExists(OwnedRoomId), #[error("An error occurred while initializing the timeline")] - InitializingTimeline(#[source] EventCacheError), + InitializingTimeline(#[source] timeline::Error), #[error("The attached event cache ran into an error")] EventCache(#[from] EventCacheError), diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index acd1ebe9052..25475a3fbe2 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -16,11 +16,7 @@ use std::{collections::BTreeSet, sync::Arc}; use eyeball::SharedObservable; use futures_util::{pin_mut, StreamExt}; -use matrix_sdk::{ - event_cache::{self, RoomEventCacheUpdate}, - executor::spawn, - Room, -}; +use matrix_sdk::{event_cache::RoomEventCacheUpdate, executor::spawn, Room}; use ruma::{events::AnySyncTimelineEvent, RoomVersionId}; use tokio::sync::{broadcast, mpsc}; use tracing::{info, info_span, trace, warn, Instrument, Span}; @@ -30,9 +26,12 @@ use super::to_device::{handle_forwarded_room_key_event, handle_room_key_event}; use super::{ inner::{TimelineInner, TimelineInnerSettings}, queue::send_queued_messages, - BackPaginationStatus, Timeline, TimelineDropHandle, + Error, Timeline, TimelineDropHandle, TimelineFocus, +}; +use crate::{ + timeline::{event_item::RemoteEventOrigin, PaginationStatus}, + unable_to_decrypt_hook::UtdHookManager, }; -use crate::unable_to_decrypt_hook::UtdHookManager; /// Builder that allows creating and configuring various parts of a /// [`Timeline`]. @@ -41,6 +40,7 @@ use crate::unable_to_decrypt_hook::UtdHookManager; pub struct TimelineBuilder { room: Room, settings: TimelineInnerSettings, + focus: TimelineFocus, /// An optional hook to call whenever we run into an unable-to-decrypt or a /// late-decryption event. @@ -56,10 +56,19 @@ impl TimelineBuilder { room: room.clone(), settings: TimelineInnerSettings::default(), unable_to_decrypt_hook: None, + focus: TimelineFocus::Live, internal_id_prefix: None, } } + /// Sets up the initial focus for this timeline. + /// + /// This can be changed later on while the timeline is alive. + pub fn with_focus(mut self, focus: TimelineFocus) -> Self { + self.focus = focus; + self + } + /// Sets up a hook to catch unable-to-decrypt (UTD) events for the timeline /// we're building. /// @@ -134,8 +143,8 @@ impl TimelineBuilder { track_read_receipts = self.settings.track_read_receipts, ) )] - pub async fn build(self) -> event_cache::Result { - let Self { room, settings, unable_to_decrypt_hook, internal_id_prefix } = self; + pub async fn build(self) -> Result { + let Self { room, settings, unable_to_decrypt_hook, focus, internal_id_prefix } = self; let client = room.client(); let event_cache = client.event_cache(); @@ -144,14 +153,12 @@ impl TimelineBuilder { event_cache.subscribe()?; let (room_event_cache, event_cache_drop) = room.event_cache().await?; - let (events, mut event_subscriber) = room_event_cache.subscribe().await?; - - let has_events = !events.is_empty(); + let (_, mut event_subscriber) = room_event_cache.subscribe().await?; - let inner = TimelineInner::new(room, internal_id_prefix, unable_to_decrypt_hook) + let inner = TimelineInner::new(room, focus, internal_id_prefix, unable_to_decrypt_hook) .with_settings(settings); - inner.replace_with_initial_events(events).await; + let has_events = inner.init_focus(&room_event_cache).await?; let room = inner.room(); let client = room.client(); @@ -165,10 +172,10 @@ impl TimelineBuilder { span.follows_from(Span::current()); async move { - trace!("Spawned the event subscriber task"); + trace!("Spawned the event subscriber task."); loop { - trace!("Waiting for an event"); + trace!("Waiting for an event."); let update = match event_subscriber.recv().await { Ok(up) => up, @@ -187,7 +194,7 @@ impl TimelineBuilder { // current timeline. match room_event_cache.subscribe().await { Ok((events, _)) => { - inner.replace_with_initial_events(events).await; + inner.replace_with_initial_events(events, RemoteEventOrigin::Sync).await; } Err(err) => { warn!("Error when re-inserting initial events into the timeline: {err}"); @@ -200,18 +207,25 @@ impl TimelineBuilder { }; match update { - RoomEventCacheUpdate::Clear => { - trace!("Clearing the timeline."); - inner.clear().await; - } - RoomEventCacheUpdate::UpdateReadMarker { event_id } => { trace!(target = %event_id, "Handling fully read marker."); inner.handle_fully_read_marker(event_id).await; } + RoomEventCacheUpdate::Clear => { + if !inner.is_live().await { + // Ignore a clear for a timeline not in the live mode; the + // focused-on-event mode doesn't add any new items to the timeline + // anyways. + continue; + } + + trace!("Clearing the timeline."); + inner.clear().await; + } + RoomEventCacheUpdate::Append { events, ephemeral, ambiguity_changes } => { - trace!("Received new events"); + trace!("Received new events from sync."); // TODO: (bnjbvr) ephemeral should be handled by the event cache, and // we should replace this with a simple `add_events_at`. @@ -300,7 +314,7 @@ impl TimelineBuilder { let timeline = Timeline { inner, - back_pagination_status: SharedObservable::new(BackPaginationStatus::Idle), + back_pagination_status: SharedObservable::new(PaginationStatus::Idle), msg_sender, event_cache: room_event_cache, drop_handle: Arc::new(TimelineDropHandle { diff --git a/crates/matrix-sdk-ui/src/timeline/error.rs b/crates/matrix-sdk-ui/src/timeline/error.rs index 7be0b5f80a5..8ed3e0b4ac4 100644 --- a/crates/matrix-sdk-ui/src/timeline/error.rs +++ b/crates/matrix-sdk-ui/src/timeline/error.rs @@ -14,6 +14,7 @@ use std::fmt; +use matrix_sdk::event_cache::{paginator::PaginatorError, EventCacheError}; use thiserror::Error; /// Errors specific to the timeline. @@ -28,23 +29,23 @@ pub enum Error { #[error("Event not found, can't retry sending")] RetryEventNotInTimeline, - /// The event is currently unsupported for this use case. + /// The event is currently unsupported for this use case.. #[error("Unsupported event")] UnsupportedEvent, - /// Couldn't read the attachment data from the given URL + /// Couldn't read the attachment data from the given URL. #[error("Invalid attachment data")] InvalidAttachmentData, - /// The attachment file name used as a body is invalid + /// The attachment file name used as a body is invalid. #[error("Invalid attachment file name")] InvalidAttachmentFileName, - /// The attachment could not be sent + /// The attachment could not be sent. #[error("Failed sending attachment")] FailedSendingAttachment, - /// The reaction could not be toggled + /// The reaction could not be toggled. #[error("Failed toggling reaction")] FailedToToggleReaction, @@ -52,9 +53,28 @@ pub enum Error { #[error("Room is not joined")] RoomNotJoined, - /// Could not get user + /// Could not get user. #[error("User ID is not available")] UserIdNotAvailable, + + /// Something went wrong with the room event cache. + #[error("Something went wrong with the room event cache.")] + EventCacheError(#[from] EventCacheError), + + /// An error happened during pagination. + #[error("An error happened during pagination.")] + PaginationError(#[from] PaginationError), +} + +#[derive(Error, Debug)] +pub enum PaginationError { + /// The timeline isn't in the event focus mode. + #[error("The timeline isn't in the event focus mode")] + NotEventFocusMode, + + /// An error occurred while paginating. + #[error("Error when paginating.")] + Paginator(#[source] PaginatorError), } #[derive(Error)] diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 6ec2b05edb9..4f06b437edc 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -255,6 +255,7 @@ pub(super) struct TimelineEventHandler<'a, 'o> { meta: &'a mut TimelineInnerMetadata, ctx: TimelineEventContext, result: HandleEventResult, + is_live_timeline: bool, } impl<'a, 'o> TimelineEventHandler<'a, 'o> { @@ -262,8 +263,14 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { state: &'a mut TimelineInnerStateTransaction<'o>, ctx: TimelineEventContext, ) -> Self { - let TimelineInnerStateTransaction { items, meta, .. } = state; - Self { items, meta, ctx, result: HandleEventResult::default() } + let TimelineInnerStateTransaction { items, meta, is_live_timeline, .. } = state; + Self { + items, + meta, + ctx, + is_live_timeline: *is_live_timeline, + result: HandleEventResult::default(), + } } /// Handle an event. @@ -284,7 +291,9 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { span.record("txn_id", debug(txn_id)); debug!("Handling local event"); - true + // Only add new timeline items if we're in the live mode, i.e. not in the + // event-focused mode. + self.is_live_timeline } Flow::Remote { event_id, txn_id, position, should_add, .. } => { @@ -295,7 +304,31 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } trace!("Handling remote event"); - *should_add + // Retrieve the origin of the event. + let origin = match position { + TimelineItemPosition::End { origin } + | TimelineItemPosition::Start { origin } => *origin, + + TimelineItemPosition::Update(idx) => self + .items + .get(*idx) + .and_then(|item| item.as_event()) + .and_then(|item| item.as_remote()) + .map_or(RemoteEventOrigin::Unknown, |item| item.origin), + }; + + match origin { + RemoteEventOrigin::Sync | RemoteEventOrigin::Unknown => { + // If the event comes the sync (or is unknown), consider adding it only if + // the timeline is in live mode; we don't want to display arbitrary sync + // events in an event-focused timeline. + self.is_live_timeline && *should_add + } + RemoteEventOrigin::Pagination | RemoteEventOrigin::Cache => { + // Otherwise, forward the previous decision to add it. + *should_add + } + } } }; @@ -315,7 +348,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } AnyMessageLikeEventContent::RoomEncrypted(c) => { // TODO: Handle replacements if the replaced event is also UTD - self.add(true, TimelineItemContent::unable_to_decrypt(c)); + self.add(should_add, TimelineItemContent::unable_to_decrypt(c)); // Let the hook know that we ran into an unable-to-decrypt that is added to the // timeline. diff --git a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs index fd4c00f65e6..4f4be2b8175 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs @@ -24,7 +24,11 @@ use imbl::Vector; use itertools::Itertools; #[cfg(all(test, feature = "e2e-encryption"))] use matrix_sdk::crypto::OlmMachine; -use matrix_sdk::{deserialized_responses::SyncTimelineEvent, Error, Result, Room}; +use matrix_sdk::{ + deserialized_responses::SyncTimelineEvent, + event_cache::{paginator::Paginator, RoomEventCache}, + Result, Room, +}; #[cfg(test)] use ruma::events::receipt::ReceiptEventContent; #[cfg(all(test, feature = "e2e-encryption"))] @@ -56,8 +60,9 @@ use super::{ reactions::ReactionToggleResult, traits::RoomDataProvider, util::{rfind_event_by_id, rfind_event_item, RelativePosition}, - AnnotationKey, EventSendState, EventTimelineItem, InReplyToDetails, Message, Profile, - RepliedToEvent, TimelineDetails, TimelineItem, TimelineItemContent, TimelineItemKind, + AnnotationKey, Error, EventSendState, EventTimelineItem, InReplyToDetails, Message, + PaginationError, Profile, RepliedToEvent, TimelineDetails, TimelineFocus, TimelineItem, + TimelineItemContent, TimelineItemKind, }; use crate::{ timeline::{day_dividers::DayDividerAdjuster, TimelineEventFilterFn}, @@ -71,10 +76,38 @@ pub(super) use self::state::{ TimelineInnerStateTransaction, }; +/// Data associated to the current timeline focus. +#[derive(Debug)] +enum TimelineFocusData { + /// The timeline receives live events from the sync. + Live, + + /// The timeline is focused on a single event, and it can expand in one + /// direction or another. + Event { + /// The event id we've started to focus on. + event_id: OwnedEventId, + /// The paginator instance. + paginator: Paginator, + /// Number of context events to request for the first request. + num_context_events: u16, + }, +} + #[derive(Clone, Debug)] pub(super) struct TimelineInner { + /// Inner mutable state. state: Arc>, + + /// Inner mutable focus state. + focus: Arc>, + + /// A [`RoomDataProvider`] implementation, providing data. + /// + /// Useful for testing only; in the real world, it's just a [`Room`]. room_data_provider: P, + + /// Settings applied to this timeline. settings: TimelineInnerSettings, } @@ -214,21 +247,132 @@ pub fn default_event_filter(event: &AnySyncTimelineEvent, room_version: &RoomVer impl TimelineInner

{ pub(super) fn new( room_data_provider: P, + focus: TimelineFocus, internal_id_prefix: Option, unable_to_decrypt_hook: Option>, ) -> Self { + let (focus_data, is_live) = match focus { + TimelineFocus::Live => (TimelineFocusData::Live, true), + TimelineFocus::Event { target, num_context_events } => { + let paginator = Paginator::new(Box::new(room_data_provider.clone())); + ( + TimelineFocusData::Event { paginator, event_id: target, num_context_events }, + false, + ) + } + }; + let state = TimelineInnerState::new( room_data_provider.room_version(), + is_live, internal_id_prefix, unable_to_decrypt_hook, ); + Self { state: Arc::new(RwLock::new(state)), + focus: Arc::new(RwLock::new(focus_data)), room_data_provider, - settings: TimelineInnerSettings::default(), + settings: Default::default(), + } + } + + /// Initializes the configured focus with appropriate data. + /// + /// Should be called only once after creation of the [`TimelineInner`], with + /// all its fields set. + /// + /// Returns whether there were any events added to the timeline. + pub(super) async fn init_focus( + &self, + room_event_cache: &RoomEventCache, + ) -> Result { + let focus_guard = self.focus.read().await; + + match &*focus_guard { + TimelineFocusData::Live => { + // Retrieve the cached events, and add them to the timeline. + let (events, _) = + room_event_cache.subscribe().await.map_err(Error::EventCacheError)?; + + let has_events = !events.is_empty(); + + self.replace_with_initial_events(events, RemoteEventOrigin::Cache).await; + + Ok(has_events) + } + + TimelineFocusData::Event { event_id, paginator, num_context_events } => { + // Start a /context request, and append the results (in order) to the timeline. + let start_from_result = paginator + .start_from(event_id, (*num_context_events).into()) + .await + .map_err(PaginationError::Paginator)?; + + drop(focus_guard); + + let has_events = !start_from_result.events.is_empty(); + + self.replace_with_initial_events( + start_from_result.events.into_iter().map(Into::into).collect(), + RemoteEventOrigin::Pagination, + ) + .await; + + Ok(has_events) + } } } + /// Run a backward pagination (in focused mode) and append the results to + /// the timeline. + /// + /// Returns whether we hit the start of the timeline. + pub(super) async fn focused_paginate_backwards( + &self, + num_events: u16, + ) -> Result { + let pagination = match &*self.focus.read().await { + TimelineFocusData::Live => return Err(PaginationError::NotEventFocusMode), + TimelineFocusData::Event { paginator, .. } => paginator + .paginate_backward(num_events.into()) + .await + .map_err(PaginationError::Paginator)?, + }; + + self.add_events_at(pagination.events, TimelineEnd::Front, RemoteEventOrigin::Pagination) + .await; + + Ok(pagination.hit_end_of_timeline) + } + + /// Run a forward pagination (in focused mode) and append the results to + /// the timeline. + /// + /// Returns whether we hit the end of the timeline. + pub(super) async fn focused_paginate_forwards( + &self, + num_events: u16, + ) -> Result { + let pagination = match &*self.focus.read().await { + TimelineFocusData::Live => return Err(PaginationError::NotEventFocusMode), + TimelineFocusData::Event { paginator, .. } => paginator + .paginate_forward(num_events.into()) + .await + .map_err(PaginationError::Paginator)?, + }; + + self.add_events_at(pagination.events, TimelineEnd::Back, RemoteEventOrigin::Pagination) + .await; + + Ok(pagination.hit_end_of_timeline) + } + + /// Is this timeline receiving events from sync (aka has a live focus)? + pub(super) async fn is_live(&self) -> bool { + matches!(&*self.focus.read().await, TimelineFocusData::Live) + } + pub(super) fn with_settings(mut self, settings: TimelineInnerSettings) -> Self { self.settings = settings; self @@ -273,7 +417,7 @@ impl TimelineInner

{ pub(super) async fn toggle_reaction_local( &self, annotation: &Annotation, - ) -> Result { + ) -> Result { let mut state = self.state.write().await; let user_id = self.room_data_provider.own_user_id(); @@ -420,7 +564,11 @@ impl TimelineInner

{ /// /// This is all done with a single lock guard, since we don't want the state /// to be modified between the clear and re-insertion of new events. - pub(super) async fn replace_with_initial_events(&self, events: Vec) { + pub(super) async fn replace_with_initial_events( + &self, + events: Vec, + origin: RemoteEventOrigin, + ) { let mut state = self.state.write().await; state.clear(); @@ -438,7 +586,7 @@ impl TimelineInner

{ .add_events_at( events, TimelineEnd::Back, - RemoteEventOrigin::Cache, + origin, &self.room_data_provider, &self.settings, ) @@ -600,7 +748,7 @@ impl TimelineInner

{ &self, annotation: &Annotation, result: &ReactionToggleResult, - ) -> Result { + ) -> Result { let mut state = self.state.write().await; let user_id = self.room_data_provider.own_user_id(); let annotation_key: AnnotationKey = annotation.into(); @@ -841,7 +989,7 @@ impl TimelineInner

{ self.set_non_ready_sender_profiles(TimelineDetails::Pending).await; } - pub(super) async fn set_sender_profiles_error(&self, error: Arc) { + pub(super) async fn set_sender_profiles_error(&self, error: Arc) { self.set_non_ready_sender_profiles(TimelineDetails::Error(error)).await; } @@ -974,10 +1122,7 @@ impl TimelineInner { } #[instrument(skip(self))] - pub(super) async fn fetch_in_reply_to_details( - &self, - event_id: &EventId, - ) -> Result<(), super::Error> { + pub(super) async fn fetch_in_reply_to_details(&self, event_id: &EventId) -> Result<(), Error> { let state = self.state.write().await; let (index, item) = rfind_event_by_id(&state.items, event_id) .ok_or(super::Error::RemoteEventNotInTimeline)?; @@ -1133,7 +1278,7 @@ async fn fetch_replied_to_event( message: &Message, in_reply_to: &EventId, room: &Room, -) -> Result>, super::Error> { +) -> Result>, Error> { if let Some((_, item)) = rfind_event_by_id(&state.items, in_reply_to) { let details = TimelineDetails::Ready(Box::new(RepliedToEvent { content: item.content.clone(), diff --git a/crates/matrix-sdk-ui/src/timeline/inner/state.rs b/crates/matrix-sdk-ui/src/timeline/inner/state.rs index 5ad2bb896fe..fb95d045241 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/state.rs @@ -66,11 +66,15 @@ pub(crate) enum TimelineEnd { pub(in crate::timeline) struct TimelineInnerState { pub items: ObservableVector>, pub meta: TimelineInnerMetadata, + + /// Is the timeline focused on a live view? + pub is_live_timeline: bool, } impl TimelineInnerState { pub(super) fn new( room_version: RoomVersionId, + is_live_timeline: bool, internal_id_prefix: Option, unable_to_decrypt_hook: Option>, ) -> Self { @@ -84,6 +88,7 @@ impl TimelineInnerState { internal_id_prefix, unable_to_decrypt_hook, ), + is_live_timeline, } } @@ -368,7 +373,12 @@ impl TimelineInnerState { pub(super) fn transaction(&mut self) -> TimelineInnerStateTransaction<'_> { let items = self.items.transaction(); let meta = self.meta.clone(); - TimelineInnerStateTransaction { items, previous_meta: &mut self.meta, meta } + TimelineInnerStateTransaction { + items, + previous_meta: &mut self.meta, + meta, + is_live_timeline: self.is_live_timeline, + } } } @@ -382,6 +392,9 @@ pub(in crate::timeline) struct TimelineInnerStateTransaction<'a> { /// [`Self::commit`]. pub meta: TimelineInnerMetadata, + /// Is the timeline focused on a live view? + pub is_live_timeline: bool, + /// Pointer to the previous meta, only used during [`Self::commit`]. previous_meta: &'a mut TimelineInnerMetadata, } @@ -614,7 +627,7 @@ impl TimelineInnerStateTransaction<'_> { } pub(super) fn commit(self) { - let Self { items, previous_meta, meta } = self; + let Self { items, previous_meta, meta, .. } = self; // Replace the pointer to the previous meta with the new one. *previous_meta = meta; diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 525ce18a4cc..8f7133b8217 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -18,7 +18,7 @@ use std::{pin::Pin, sync::Arc, task::Poll}; -use eyeball::{SharedObservable, Subscriber}; +use eyeball::SharedObservable; use eyeball_im::VectorDiff; use futures_core::Stream; use imbl::Vector; @@ -86,7 +86,7 @@ mod virtual_item; pub use self::{ builder::TimelineBuilder, - error::{Error, UnsupportedEditItem, UnsupportedReplyItem}, + error::{Error, PaginationError, UnsupportedEditItem, UnsupportedReplyItem}, event_item::{ AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventItemOrigin, EventSendState, EventTimelineItem, InReplyToDetails, MemberProfileChange, MembershipChange, @@ -96,7 +96,7 @@ pub use self::{ event_type_filter::TimelineEventTypeFilter, inner::default_event_filter, item::{TimelineItem, TimelineItemKind}, - pagination::{BackPaginationStatus, PaginationOptions, PaginationOutcome}, + pagination::{PaginationOptions, PaginationOutcome, PaginationStatus}, polls::PollResult, reactions::ReactionSenderData, sliding_sync_ext::SlidingSyncRoomExt, @@ -124,15 +124,15 @@ pub struct Timeline { /// The event cache specialized for this room's view. event_cache: RoomEventCache, - /// Observable for whether a pagination is currently running - back_pagination_status: SharedObservable, - /// A sender to the task which responsibility is to send messages to the /// current room. msg_sender: Sender, /// References to long-running tasks held by the timeline. drop_handle: Arc, + + /// Observable for whether a backward pagination is currently running. + pub(crate) back_pagination_status: SharedObservable, } // Implements hash etc @@ -148,6 +148,17 @@ impl From<&Annotation> for AnnotationKey { } } +/// What should the timeline focus on? +#[derive(Clone, Debug, PartialEq)] +pub enum TimelineFocus { + /// Focus on live events, i.e. receive events from sync and append them in + /// real-time. + Live, + + /// Focus on a specific event, e.g. after clicking a permalink. + Event { target: OwnedEventId, num_context_events: u16 }, +} + impl Timeline { /// Create a new [`TimelineBuilder`] for the given room. pub fn builder(room: &Room) -> TimelineBuilder { @@ -164,11 +175,6 @@ impl Timeline { self.inner.clear().await; } - /// Subscribe to the back-pagination status of the timeline. - pub fn back_pagination_status(&self) -> Subscriber { - self.back_pagination_status.subscribe() - } - /// Retry decryption of previously un-decryptable events given a list of /// session IDs whose keys have been imported. /// @@ -225,7 +231,11 @@ impl Timeline { /// Get the latest of the timeline's event items. pub async fn latest_event(&self) -> Option { - self.inner.items().await.last()?.as_event().cloned() + if self.inner.is_live().await { + self.inner.items().await.last()?.as_event().cloned() + } else { + None + } } /// Get the current timeline items, and a stream of changes. diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index 155b0105be0..3c7f28f27b0 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -14,26 +14,67 @@ use std::{fmt, ops::ControlFlow, sync::Arc, time::Duration}; +use eyeball::Subscriber; use matrix_sdk::event_cache::{self, BackPaginationOutcome}; use tracing::{instrument, trace, warn}; +use super::Error; use crate::timeline::{event_item::RemoteEventOrigin, inner::TimelineEnd}; impl super::Timeline { /// Add more events to the start of the timeline. + /// + /// Returns whether we hit the start of the timeline. + #[instrument(skip_all, fields(room_id = ?self.room().room_id()))] + pub async fn paginate_backwards(&self, num_events: u16) -> Result { + if self.inner.is_live().await { + Ok(self + .live_paginate_backwards(PaginationOptions::until_num_items(num_events, 20)) + .await?) + } else { + Ok(self.focused_paginate_backwards(num_events).await?) + } + } + + /// Assuming the timeline is focused on an event, starts a forwards + /// pagination. + /// + /// Returns whether we hit the end of the timeline. + #[instrument(skip_all)] + pub async fn focused_paginate_forwards(&self, num_events: u16) -> Result { + Ok(self.inner.focused_paginate_forwards(num_events).await?) + } + + /// Assuming the timeline is focused on an event, starts a backwards + /// pagination. + /// + /// Returns whether we hit the start of the timeline. + #[instrument(skip(self), fields(room_id = ?self.room().room_id()))] + pub async fn focused_paginate_backwards(&self, num_events: u16) -> Result { + Ok(self.inner.focused_paginate_backwards(num_events).await?) + } + + /// Paginate backwards in live mode. + /// + /// This can only be called when the timeline is in live mode, not focused + /// on a specific event. + /// + /// Returns whether we hit the start of the timeline. #[instrument(skip_all, fields(room_id = ?self.room().room_id(), ?options))] - pub async fn paginate_backwards( + pub async fn live_paginate_backwards( &self, mut options: PaginationOptions<'_>, - ) -> event_cache::Result<()> { - if self.back_pagination_status.get() == BackPaginationStatus::TimelineStartReached { + ) -> event_cache::Result { + let back_pagination_status = &self.back_pagination_status; + + if back_pagination_status.get() == PaginationStatus::TimelineEndReached { warn!("Start of timeline reached, ignoring backwards-pagination request"); - return Ok(()); + return Ok(true); } - if self.back_pagination_status.set_if_not_eq(BackPaginationStatus::Paginating).is_none() { + if back_pagination_status.set_if_not_eq(PaginationStatus::Paginating).is_none() { warn!("Another back-pagination is already running in the background"); - return Ok(()); + return Ok(false); } // The first time, we allow to wait a bit for *a* back-pagination token to come @@ -63,9 +104,9 @@ impl super::Timeline { .await; if reached_start { - self.back_pagination_status - .set_if_not_eq(BackPaginationStatus::TimelineStartReached); - return Ok(()); + back_pagination_status + .set_if_not_eq(PaginationStatus::TimelineEndReached); + return Ok(true); } outcome.events_received = num_events as u64; @@ -101,8 +142,13 @@ impl super::Timeline { } } - self.back_pagination_status.set_if_not_eq(BackPaginationStatus::Idle); - Ok(()) + back_pagination_status.set_if_not_eq(PaginationStatus::Idle); + Ok(false) + } + + /// Subscribe to the back-pagination status of the timeline. + pub fn back_pagination_status(&self) -> Subscriber { + self.back_pagination_status.subscribe() } } @@ -254,12 +300,17 @@ pub struct PaginationOutcome { pub total_items_updated: u64, } +/// The status of a pagination. #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] -pub enum BackPaginationStatus { +pub enum PaginationStatus { + /// No pagination happening. Idle, + /// Timeline is paginating for this end. Paginating, - TimelineStartReached, + /// An end of the timeline (front or back) has been reached by this + /// pagination. + TimelineEndReached, } #[cfg(test)] diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index 3c152a2ef08..4e1d695e8bb 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -111,7 +111,7 @@ async fn test_replace_with_initial_events_and_read_marker() { assert_eq!(items[1].as_event().unwrap().content().as_message().unwrap().body(), "hey"); let ev = factory.text_msg("yo").sender(*BOB).into_sync(); - timeline.inner.replace_with_initial_events(vec![ev]).await; + timeline.inner.replace_with_initial_events(vec![ev], RemoteEventOrigin::Sync).await; let items = timeline.inner.items().await; assert_eq!(items.len(), 2); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index 3a736b3a449..59b93a96f48 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -25,7 +25,11 @@ use eyeball_im::VectorDiff; use futures_core::Stream; use futures_util::{FutureExt, StreamExt}; use indexmap::IndexMap; -use matrix_sdk::deserialized_responses::{SyncTimelineEvent, TimelineEvent}; +use matrix_sdk::{ + deserialized_responses::{SyncTimelineEvent, TimelineEvent}, + event_cache::paginator::{PaginableRoom, PaginatorError}, + room::{EventWithContextResponse, Messages, MessagesOptions}, +}; use matrix_sdk_base::latest_event::LatestEvent; use matrix_sdk_test::{EventBuilder, ALICE, BOB}; use ruma::{ @@ -43,7 +47,7 @@ use ruma::{ room_id, serde::Raw, server_name, uint, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, - OwnedUserId, RoomId, RoomVersionId, TransactionId, UserId, + OwnedUserId, RoomId, RoomVersionId, TransactionId, UInt, UserId, }; use super::{ @@ -52,7 +56,7 @@ use super::{ inner::{ReactionAction, TimelineEnd, TimelineInnerSettings}, reactions::ReactionToggleResult, traits::RoomDataProvider, - EventTimelineItem, Profile, TimelineInner, TimelineItem, + EventTimelineItem, Profile, TimelineFocus, TimelineInner, TimelineItem, }; use crate::unable_to_decrypt_hook::UtdHookManager; @@ -82,21 +86,31 @@ impl TestTimeline { fn with_internal_id_prefix(prefix: String) -> Self { Self { - inner: TimelineInner::new(TestRoomDataProvider::default(), Some(prefix), None), + inner: TimelineInner::new( + TestRoomDataProvider::default(), + TimelineFocus::Live, + Some(prefix), + None, + ), event_builder: EventBuilder::new(), } } fn with_room_data_provider(room_data_provider: TestRoomDataProvider) -> Self { Self { - inner: TimelineInner::new(room_data_provider, None, None), + inner: TimelineInner::new(room_data_provider, TimelineFocus::Live, None, None), event_builder: EventBuilder::new(), } } fn with_unable_to_decrypt_hook(hook: Arc) -> Self { Self { - inner: TimelineInner::new(TestRoomDataProvider::default(), None, Some(hook)), + inner: TimelineInner::new( + TestRoomDataProvider::default(), + TimelineFocus::Live, + None, + Some(hook), + ), event_builder: EventBuilder::new(), } } @@ -305,6 +319,23 @@ impl TestRoomDataProvider { } } +#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))] +#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)] +impl PaginableRoom for TestRoomDataProvider { + async fn event_with_context( + &self, + _event_id: &EventId, + _lazy_load_members: bool, + _num_events: UInt, + ) -> Result { + unimplemented!(); + } + + async fn messages(&self, _opts: MessagesOptions) -> Result { + unimplemented!(); + } +} + #[async_trait] impl RoomDataProvider for TestRoomDataProvider { fn own_user_id(&self) -> &UserId { diff --git a/crates/matrix-sdk-ui/src/timeline/traits.rs b/crates/matrix-sdk-ui/src/timeline/traits.rs index bdcb695391c..af8711cfe4b 100644 --- a/crates/matrix-sdk-ui/src/timeline/traits.rs +++ b/crates/matrix-sdk-ui/src/timeline/traits.rs @@ -16,7 +16,7 @@ use async_trait::async_trait; use indexmap::IndexMap; #[cfg(feature = "e2e-encryption")] use matrix_sdk::{deserialized_responses::TimelineEvent, Result}; -use matrix_sdk::{event_cache, Room}; +use matrix_sdk::{event_cache::paginator::PaginableRoom, Room}; use matrix_sdk_base::latest_event::LatestEvent; #[cfg(feature = "e2e-encryption")] use ruma::{events::AnySyncTimelineEvent, serde::Raw}; @@ -31,7 +31,7 @@ use ruma::{ use tracing::{debug, error}; use super::{Profile, TimelineBuilder}; -use crate::timeline::Timeline; +use crate::timeline::{self, Timeline}; #[async_trait] pub trait RoomExt { @@ -42,7 +42,7 @@ pub trait RoomExt { /// independent events. /// /// This is the same as using `room.timeline_builder().build()`. - async fn timeline(&self) -> event_cache::Result; + async fn timeline(&self) -> Result; /// Get a [`TimelineBuilder`] for this room. /// @@ -57,7 +57,7 @@ pub trait RoomExt { #[async_trait] impl RoomExt for Room { - async fn timeline(&self) -> event_cache::Result { + async fn timeline(&self) -> Result { self.timeline_builder().build().await } @@ -67,7 +67,7 @@ impl RoomExt for Room { } #[async_trait] -pub(super) trait RoomDataProvider: Clone + Send + Sync + 'static { +pub(super) trait RoomDataProvider: Clone + Send + Sync + 'static + PaginableRoom { fn own_user_id(&self) -> &UserId; fn room_version(&self) -> RoomVersionId; async fn profile_from_user_id(&self, user_id: &UserId) -> Option; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs index 81653ba5a0a..02a1dda9bf7 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs @@ -22,7 +22,7 @@ use matrix_sdk_test::{ async_test, EventBuilder, JoinedRoomBuilder, StateTestEvent, SyncResponseBuilder, ALICE, BOB, }; use matrix_sdk_ui::timeline::{ - AnyOtherFullStateEventContent, BackPaginationStatus, PaginationOptions, RoomExt, + AnyOtherFullStateEventContent, PaginationOptions, PaginationStatus, RoomExt, TimelineItemContent, }; use once_cell::sync::Lazy; @@ -71,11 +71,11 @@ async fn test_back_pagination() { .await; let paginate = async { - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); server.reset().await; }; let observe_paginating = async { - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Paginating)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Paginating)); }; join(paginate, observe_paginating).await; @@ -130,8 +130,8 @@ async fn test_back_pagination() { .mount(&server) .await; - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); - assert_next_eq!(back_pagination_status, BackPaginationStatus::TimelineStartReached); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + assert_next_eq!(back_pagination_status, PaginationStatus::TimelineEndReached); } #[async_test] @@ -195,7 +195,7 @@ async fn test_back_pagination_highlighted() { .mount(&server) .await; - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); server.reset().await; let first = assert_next_matches!( @@ -263,13 +263,13 @@ async fn test_wait_for_token() { let paginate = async { timeline - .paginate_backwards(PaginationOptions::simple_request(10).wait_for_token()) + .live_paginate_backwards(PaginationOptions::simple_request(10).wait_for_token()) .await .unwrap(); }; let observe_paginating = async { - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Paginating)); - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Idle)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Paginating)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Idle)); }; let sync = async { // Make sure syncing starts a little bit later than pagination @@ -328,10 +328,10 @@ async fn test_dedup() { // If I try to paginate twice at the same time, let paginate_1 = async { - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); }; let paginate_2 = async { - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); }; timeout(Duration::from_secs(5), join(paginate_1, paginate_2)).await.unwrap(); @@ -422,18 +422,15 @@ async fn test_timeline_reset_while_paginating() { let paginate = async { timeline - .paginate_backwards(PaginationOptions::simple_request(10).wait_for_token()) + .live_paginate_backwards(PaginationOptions::simple_request(10).wait_for_token()) .await .unwrap(); }; let observe_paginating = async { - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Paginating)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Paginating)); // timeline start reached because second pagination response contains // no end field - assert_eq!( - back_pagination_status.next().await, - Some(BackPaginationStatus::TimelineStartReached) - ); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::TimelineEndReached)); }; let sync = async { client.sync_once(sync_settings.clone()).await.unwrap(); @@ -563,11 +560,11 @@ async fn test_empty_chunk() { .await; let paginate = async { - timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); server.reset().await; }; let observe_paginating = async { - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Paginating)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Paginating)); }; join(paginate, observe_paginating).await; @@ -662,11 +659,11 @@ async fn test_until_num_items_with_empty_chunk() { .await; let paginate = async { - timeline.paginate_backwards(PaginationOptions::until_num_items(4, 4)).await.unwrap(); + timeline.live_paginate_backwards(PaginationOptions::until_num_items(4, 4)).await.unwrap(); server.reset().await; }; let observe_paginating = async { - assert_eq!(back_pagination_status.next().await, Some(BackPaginationStatus::Paginating)); + assert_eq!(back_pagination_status.next().await, Some(PaginationStatus::Paginating)); }; join(paginate, observe_paginating).await; diff --git a/labs/multiverse/src/main.rs b/labs/multiverse/src/main.rs index 305fc8a08b9..f2539eb3d25 100644 --- a/labs/multiverse/src/main.rs +++ b/labs/multiverse/src/main.rs @@ -110,8 +110,8 @@ struct StatefulList { #[derive(Default, PartialEq)] enum DetailsMode { - #[default] ReadReceipts, + #[default] TimelineItems, // Events // TODO: Soon™ } @@ -343,8 +343,9 @@ impl App { // Start a new one, request batches of 20 events, stop after 10 timeline items // have been added. *pagination = Some(spawn(async move { - if let Err(err) = - sdk_timeline.paginate_backwards(PaginationOptions::until_num_items(20, 10)).await + if let Err(err) = sdk_timeline + .live_paginate_backwards(PaginationOptions::until_num_items(20, 10)) + .await { // TODO: would be nice to be able to set the status // message remotely? From a6c42404a63d70b7606ef74c77c191cb624014a7 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 23 Apr 2024 19:02:45 +0200 Subject: [PATCH 2/3] tests: add integration tests for the new timeline focus mode --- .../matrix-sdk-ui/tests/integration/main.rs | 69 ++++- .../tests/integration/timeline/focus_event.rs | 267 ++++++++++++++++++ .../tests/integration/timeline/mod.rs | 1 + 3 files changed, 333 insertions(+), 4 deletions(-) create mode 100644 crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs diff --git a/crates/matrix-sdk-ui/tests/integration/main.rs b/crates/matrix-sdk-ui/tests/integration/main.rs index 476570138c8..1e8c2f319d9 100644 --- a/crates/matrix-sdk-ui/tests/integration/main.rs +++ b/crates/matrix-sdk-ui/tests/integration/main.rs @@ -12,8 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; +use matrix_sdk::deserialized_responses::TimelineEvent; use matrix_sdk_test::test_json; +use ruma::{events::AnyStateEvent, serde::Raw, EventId, RoomId}; use serde::Serialize; +use serde_json::json; use wiremock::{ matchers::{header, method, path, path_regex, query_param, query_param_is_missing}, Mock, MockServer, ResponseTemplate, @@ -32,22 +36,79 @@ matrix_sdk_test::init_tracing_for_tests!(); /// an optional `since` param that returns a 200 status code with the given /// response body. async fn mock_sync(server: &MockServer, response_body: impl Serialize, since: Option) { - let mut builder = Mock::given(method("GET")) + let mut mock_builder = Mock::given(method("GET")) .and(path("/_matrix/client/r0/sync")) .and(header("authorization", "Bearer 1234")); if let Some(since) = since { - builder = builder.and(query_param("since", since)); + mock_builder = mock_builder.and(query_param("since", since)); } else { - builder = builder.and(query_param_is_missing("since")); + mock_builder = mock_builder.and(query_param_is_missing("since")); } - builder + mock_builder .respond_with(ResponseTemplate::new(200).set_body_json(response_body)) .mount(server) .await; } +/// Mocks the /context endpoint +/// +/// Note: pass `events_before` in the normal order, I'll revert the order for +/// you. +#[allow(clippy::too_many_arguments)] // clippy you've got such a fixed mindset +async fn mock_context( + server: &MockServer, + room_id: &RoomId, + event_id: &EventId, + prev_batch_token: Option, + events_before: Vec, + event: TimelineEvent, + events_after: Vec, + next_batch_token: Option, + state: Vec>, +) { + Mock::given(method("GET")) + .and(path(format!("/_matrix/client/r0/rooms/{room_id}/context/{event_id}"))) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "events_before": events_before.into_iter().rev().map(|ev| ev.event).collect_vec(), + "event": event.event, + "events_after": events_after.into_iter().map(|ev| ev.event).collect_vec(), + "state": state, + "start": prev_batch_token, + "end": next_batch_token + }))) + .mount(server) + .await; +} + +/// Mocks the /messages endpoint. +/// +/// Note: pass `chunk` in the correct order: topological for forward pagination, +/// reverse topological for backwards pagination. +async fn mock_messages( + server: &MockServer, + start: String, + end: Option, + chunk: Vec, + state: Vec>, +) { + Mock::given(method("GET")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/messages$")) + .and(header("authorization", "Bearer 1234")) + .and(query_param("from", start.clone())) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "start": start, + "end": end, + "chunk": chunk.into_iter().map(|ev| ev.event).collect_vec(), + "state": state, + }))) + .expect(1) + .mount(server) + .await; +} + /// Mount a Mock on the given server to handle the `GET /// /rooms/.../state/m.room.encryption` endpoint with an option whether it /// should return an encryption event or not. diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs new file mode 100644 index 00000000000..3a29e2166da --- /dev/null +++ b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs @@ -0,0 +1,267 @@ +// Copyright 2024 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests specific to a timeline focused on an event. + +use std::time::Duration; + +use assert_matches2::assert_let; +use eyeball_im::VectorDiff; +use futures_util::StreamExt; +use matrix_sdk::{ + config::SyncSettings, + test_utils::{events::EventFactory, logged_in_client_with_server}, +}; +use matrix_sdk_test::{ + async_test, sync_timeline_event, JoinedRoomBuilder, SyncResponseBuilder, ALICE, BOB, +}; +use matrix_sdk_ui::{timeline::TimelineFocus, Timeline}; +use ruma::{event_id, room_id}; +use stream_assert::assert_pending; + +use crate::{mock_context, mock_messages, mock_sync}; + +#[async_test] +async fn test_new_focused() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client_with_server().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut sync_response_builder = SyncResponseBuilder::new(); + sync_response_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + // Mark the room as joined. + mock_sync(&server, sync_response_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let f = EventFactory::new().room(room_id); + let target_event = event_id!("$1"); + + mock_context( + &server, + room_id, + target_event, + Some("prev1".to_owned()), + vec![ + f.text_msg("i tried so hard").sender(*ALICE).into_timeline(), + f.text_msg("and got so far").sender(*ALICE).into_timeline(), + ], + f.text_msg("in the end").event_id(target_event).sender(*BOB).into_timeline(), + vec![ + f.text_msg("it doesn't even").sender(*ALICE).into_timeline(), + f.text_msg("matter").sender(*ALICE).into_timeline(), + ], + Some("next1".to_owned()), + vec![], + ) + .await; + + let room = client.get_room(room_id).unwrap(); + let timeline = Timeline::builder(&room) + .with_focus(TimelineFocus::Event { + target: target_event.to_owned(), + num_context_events: 20, + }) + .build() + .await + .unwrap(); + + server.reset().await; + + let (items, mut timeline_stream) = timeline.subscribe().await; + + assert_eq!(items.len(), 5 + 1); // event items + a day divider + assert!(items[0].is_day_divider()); + assert_eq!( + items[1].as_event().unwrap().content().as_message().unwrap().body(), + "i tried so hard" + ); + assert_eq!( + items[2].as_event().unwrap().content().as_message().unwrap().body(), + "and got so far" + ); + assert_eq!(items[3].as_event().unwrap().content().as_message().unwrap().body(), "in the end"); + assert_eq!( + items[4].as_event().unwrap().content().as_message().unwrap().body(), + "it doesn't even" + ); + assert_eq!(items[5].as_event().unwrap().content().as_message().unwrap().body(), "matter"); + + assert_pending!(timeline_stream); + + // Now trigger a backward pagination. + mock_messages( + &server, + "prev1".to_owned(), + None, + vec![ + // reversed manually here + f.text_msg("And even though I tried, it all fell apart").sender(*BOB).into_timeline(), + f.text_msg("I kept everything inside").sender(*BOB).into_timeline(), + ], + vec![], + ) + .await; + + let hit_start = timeline.focused_paginate_backwards(20).await.unwrap(); + assert!(hit_start); + + server.reset().await; + + assert_let!(Some(VectorDiff::PushFront { value: message }) = timeline_stream.next().await); + assert_eq!( + message.as_event().unwrap().content().as_message().unwrap().body(), + "And even though I tried, it all fell apart" + ); + + assert_let!(Some(VectorDiff::PushFront { value: message }) = timeline_stream.next().await); + assert_eq!( + message.as_event().unwrap().content().as_message().unwrap().body(), + "I kept everything inside" + ); + + // Day divider post processing. + assert_let!(Some(VectorDiff::PushFront { value: item }) = timeline_stream.next().await); + assert!(item.is_day_divider()); + assert_let!(Some(VectorDiff::Remove { index }) = timeline_stream.next().await); + assert_eq!(index, 3); + + // Now trigger a forward pagination. + mock_messages( + &server, + "next1".to_owned(), + Some("next2".to_owned()), + vec![ + f.text_msg("I had to fall, to lose it all").sender(*BOB).into_timeline(), + f.text_msg("But in the end, it doesn't event matter").sender(*BOB).into_timeline(), + ], + vec![], + ) + .await; + + let hit_start = timeline.focused_paginate_forwards(20).await.unwrap(); + assert!(!hit_start); // because we gave it another next2 token. + + server.reset().await; + + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_eq!( + message.as_event().unwrap().content().as_message().unwrap().body(), + "I had to fall, to lose it all" + ); + + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_eq!( + message.as_event().unwrap().content().as_message().unwrap().body(), + "But in the end, it doesn't event matter" + ); + + assert_pending!(timeline_stream); +} + +#[async_test] +async fn test_focused_timeline_reacts() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client_with_server().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut sync_response_builder = SyncResponseBuilder::new(); + sync_response_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + // Mark the room as joined. + mock_sync(&server, sync_response_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + // Start a focused timeline. + let f = EventFactory::new().room(room_id); + let target_event = event_id!("$1"); + + mock_context( + &server, + room_id, + target_event, + None, + vec![], + f.text_msg("yolo").event_id(target_event).sender(*BOB).into_timeline(), + vec![], + None, + vec![], + ) + .await; + + let room = client.get_room(room_id).unwrap(); + let timeline = Timeline::builder(&room) + .with_focus(TimelineFocus::Event { + target: target_event.to_owned(), + num_context_events: 20, + }) + .build() + .await + .unwrap(); + + server.reset().await; + + let (items, mut timeline_stream) = timeline.subscribe().await; + + assert_eq!(items.len(), 1 + 1); // event items + a day divider + assert!(items[0].is_day_divider()); + + let event_item = items[1].as_event().unwrap(); + assert_eq!(event_item.content().as_message().unwrap().body(), "yolo"); + assert_eq!(event_item.reactions().len(), 0); + + assert_pending!(timeline_stream); + + // Now simulate a sync that returns a new message-like event, and a reaction + // to the $1 event. + sync_response_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_bulk([ + // This event must be ignored. + f.text_msg("this is a sync event").sender(*ALICE).into_raw_sync(), + // This event must not be ignored. + sync_timeline_event!({ + "content": { + "m.relates_to": { + "event_id": "$1", + "key": "👍", + "rel_type": "m.annotation" + } + }, + "event_id": "$15275047031IXQRi:localhost", + "origin_server_ts": 159027581000000_u64, + "sender": *BOB, + "type": "m.reaction", + "unsigned": { + "age": 85 + } + }), + ])); + + // Sync the room. + mock_sync(&server, sync_response_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next().await); + + let event_item = item.as_event().unwrap(); + // Text hasn't changed. + assert_eq!(event_item.content().as_message().unwrap().body(), "yolo"); + // But now there's one reaction to the event. + assert_eq!(event_item.reactions().len(), 1); + + // And nothing more. + assert_pending!(timeline_stream); +} diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index b3b83c112cc..4277af25285 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -30,6 +30,7 @@ use crate::mock_sync; mod echo; mod edit; +mod focus_event; mod pagination; mod profiles; mod queue; From 397a26e00bdc005c540c6995d81c44e16a14c16d Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 23 Apr 2024 19:02:59 +0200 Subject: [PATCH 3/3] ffi: add bindings for the timeline focus mode and associated functions --- bindings/matrix-sdk-ffi/src/room.rs | 51 +++++++++++++++++++-- bindings/matrix-sdk-ffi/src/timeline/mod.rs | 35 ++++++++++---- 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 5a24fd4f5f2..f0bcff74804 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -2,10 +2,11 @@ use std::sync::Arc; use anyhow::{Context, Result}; use matrix_sdk::{ + event_cache::paginator::PaginatorError, room::{power_levels::RoomPowerLevelChanges, Room as SdkRoom, RoomMemberRole}, RoomMemberships, RoomState, }; -use matrix_sdk_ui::timeline::RoomExt; +use matrix_sdk_ui::timeline::{PaginationError, RoomExt, TimelineFocus}; use mime::Mime; use ruma::{ api::client::room::report_content, @@ -30,7 +31,7 @@ use crate::{ room_info::RoomInfo, room_member::RoomMember, ruma::ImageInfo, - timeline::{EventTimelineItem, ReceiptType, Timeline}, + timeline::{EventTimelineItem, FocusEventError, ReceiptType, Timeline}, utils::u64_to_uint, TaskHandle, }; @@ -167,6 +168,48 @@ impl Room { } } + /// Returns a timeline focused on the given event. + /// + /// Note: this timeline is independent from that returned with + /// [`Self::timeline`], and as such it is not cached. + pub async fn timeline_focused_on_event( + &self, + event_id: String, + num_context_events: u16, + internal_id_prefix: Option, + ) -> Result, FocusEventError> { + let parsed_event_id = EventId::parse(&event_id).map_err(|err| { + FocusEventError::InvalidEventId { event_id: event_id.clone(), err: err.to_string() } + })?; + + let room = &self.inner; + + let mut builder = matrix_sdk_ui::timeline::Timeline::builder(room); + + if let Some(internal_id_prefix) = internal_id_prefix { + builder = builder.with_internal_id_prefix(internal_id_prefix); + } + + let timeline = match builder + .with_focus(TimelineFocus::Event { target: parsed_event_id, num_context_events }) + .build() + .await + { + Ok(t) => t, + Err(err) => { + if let matrix_sdk_ui::timeline::Error::PaginationError( + PaginationError::Paginator(PaginatorError::EventNotFound(..)), + ) = err + { + return Err(FocusEventError::EventNotFound { event_id: event_id.to_string() }); + } + return Err(FocusEventError::Other { msg: err.to_string() }); + } + }; + + Ok(Timeline::new(timeline)) + } + pub fn display_name(&self) -> Result { let r = self.inner.clone(); RUNTIME.block_on(async move { Ok(r.display_name().await?.to_string()) }) @@ -237,7 +280,8 @@ impl Room { } } - // Otherwise, fallback to the classical path. + // Otherwise, create a synthetic [`EventTimelineItem`] using the classical + // [`Room`] path. let latest_event = match self.inner.latest_event() { Some(latest_event) => matrix_sdk_ui::timeline::EventTimelineItem::from_latest_event( self.inner.client(), @@ -249,6 +293,7 @@ impl Room { .map(Arc::new), None => None, }; + Ok(RoomInfo::new(&self.inner, avatar_url, latest_event).await?) } diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 94c83289716..62372acc110 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -22,7 +22,7 @@ use matrix_sdk::attachment::{ AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo, BaseThumbnailInfo, BaseVideoInfo, Thumbnail, }; -use matrix_sdk_ui::timeline::{BackPaginationStatus, EventItemOrigin, Profile, TimelineDetails}; +use matrix_sdk_ui::timeline::{EventItemOrigin, PaginationStatus, Profile, TimelineDetails}; use mime::Mime; use ruma::{ events::{ @@ -162,7 +162,7 @@ impl Timeline { pub fn subscribe_to_back_pagination_status( &self, - listener: Box, + listener: Box, ) -> Result, ClientError> { let mut subscriber = self.inner.back_pagination_status(); @@ -176,11 +176,18 @@ impl Timeline { })))) } - /// Loads older messages into the timeline. + /// Paginate backwards, whether we are in focused mode or in live mode. /// - /// Raises an exception if there are no timeline listeners. - pub fn paginate_backwards(&self, opts: PaginationOptions) -> Result<(), ClientError> { - RUNTIME.block_on(async { Ok(self.inner.paginate_backwards(opts.into()).await?) }) + /// Returns whether we hit the end of the timeline or not. + pub async fn paginate_backwards(&self, num_events: u16) -> Result { + Ok(self.inner.paginate_backwards(num_events).await?) + } + + /// Paginate forwards, when in focused mode. + /// + /// Returns whether we hit the end of the timeline or not. + pub async fn paginate_forwards(&self, num_events: u16) -> Result { + Ok(self.inner.focused_paginate_forwards(num_events).await?) } pub fn send_read_receipt( @@ -573,6 +580,18 @@ impl Timeline { } } +#[derive(Debug, thiserror::Error, uniffi::Error)] +pub enum FocusEventError { + #[error("the event id parameter {event_id} is incorrect: {err}")] + InvalidEventId { event_id: String, err: String }, + + #[error("the event {event_id} could not be found")] + EventNotFound { event_id: String }, + + #[error("error when trying to focus on an event: {msg}")] + Other { msg: String }, +} + #[derive(uniffi::Record)] pub struct RoomTimelineListenerResult { pub items: Vec>, @@ -585,8 +604,8 @@ pub trait TimelineListener: Sync + Send { } #[uniffi::export(callback_interface)] -pub trait BackPaginationStatusListener: Sync + Send { - fn on_update(&self, status: BackPaginationStatus); +pub trait PaginationStatusListener: Sync + Send { + fn on_update(&self, status: PaginationStatus); } #[derive(Clone, uniffi::Object)]