From 08df153ed9753321ff25fb081d40b25859cf8457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 10 Sep 2024 08:56:03 +0200 Subject: [PATCH] sdk-ui: create `TimelineState::replace_all` which combines `clear` and `add_remote_events_at` in the same transaction --- .../src/timeline/controller/mod.rs | 11 ++++--- .../src/timeline/controller/state.rs | 25 +++++++++++++-- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 32 +++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 2ce7bebec..130885949 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -637,8 +637,6 @@ impl TimelineController

{ ) { let mut state = self.state.write().await; - state.clear(); - let track_read_markers = self.settings.track_read_receipts; if track_read_markers { state.populate_initial_user_receipt(&self.room_data_provider, ReceiptType::Read).await; @@ -647,9 +645,14 @@ impl TimelineController

{ .await; } - if !events.is_empty() { + // Replace the events if either the current event list or the new one aren't + // empty. + // Previously we just had to check the new one wasn't empty because + // we did a clear operation before so the current one would always be empty, but + // now we may want to replace a populated timeline with an empty one. + if !state.items.is_empty() || !events.is_empty() { state - .add_remote_events_at( + .replace_with_remove_events( events, TimelineEnd::Back, origin, diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index fb81dd4dc..7aebfb4f4 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -63,7 +63,7 @@ use crate::{ pub(crate) enum TimelineEnd { /// Event should be prepended to the front of the timeline. Front, - /// Event should appended to the back of the timeline. + /// Event should be appended to the back of the timeline. Back, } @@ -101,7 +101,7 @@ impl TimelineState { } } - /// Add the given remove events at the given end of the timeline. + /// Add the given remote events at the given end of the timeline. /// /// Note: when the `position` is [`TimelineEnd::Front`], prepended events /// should be ordered in *reverse* topological order, that is, `events[0]` @@ -274,6 +274,27 @@ impl TimelineState { txn.commit(); } + /// Replaces the existing events in the timeline with the given remote ones. + /// + /// Note: when the `position` is [`TimelineEnd::Front`], prepended events + /// should be ordered in *reverse* topological order, that is, `events[0]` + /// is the most recent. + pub(super) async fn replace_with_remove_events( + &mut self, + events: Vec, + position: TimelineEnd, + origin: RemoteEventOrigin, + room_data_provider: &P, + settings: &TimelineSettings, + ) -> HandleManyEventsResult { + let mut txn = self.transaction(); + txn.clear(); + let result = + txn.add_remote_events_at(events, position, origin, room_data_provider, settings).await; + txn.commit(); + result + } + pub(super) fn transaction(&mut self) -> TimelineStateTransaction<'_> { let items = self.items.transaction(); let meta = self.meta.clone(); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index aa212be40..d0149b7b3 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -15,6 +15,7 @@ use assert_matches::assert_matches; use assert_matches2::assert_let; use eyeball_im::VectorDiff; +use futures_util::StreamExt; use matrix_sdk_test::{async_test, sync_timeline_event, ALICE, BOB, CAROL}; use ruma::{ events::{ @@ -505,3 +506,34 @@ async fn test_thread() { assert_let!(TimelineDetails::Ready(replied_to_event) = &in_reply_to.event); assert_eq!(replied_to_event.sender(), *ALICE); } + +#[async_test] +async fn test_replace_with_initial_events_when_batched() { + let timeline = TestTimeline::with_room_data_provider(TestRoomDataProvider::default()) + .with_settings(TimelineSettings::default()); + + let f = &timeline.factory; + let ev = f.text_msg("hey").sender(*ALICE).into_sync(); + + timeline.controller.add_events_at(vec![ev], TimelineEnd::Back, RemoteEventOrigin::Sync).await; + + let (items, mut stream) = timeline.controller.subscribe_batched().await; + assert_eq!(items.len(), 2); + assert!(items[0].is_day_divider()); + assert_eq!(items[1].as_event().unwrap().content().as_message().unwrap().body(), "hey"); + + let ev = f.text_msg("yo").sender(*BOB).into_sync(); + timeline.controller.replace_with_initial_remote_events(vec![ev], RemoteEventOrigin::Sync).await; + + // Assert there are more than a single Clear diff in the next batch: + // Clear + PushBack (event) + PushFront (day divider) + let batched_diffs = stream.next().await.unwrap(); + assert_eq!(batched_diffs.len(), 3); + assert_matches!(batched_diffs[0], VectorDiff::Clear); + assert_matches!(&batched_diffs[1], VectorDiff::PushBack { value } => { + assert!(value.as_event().is_some()); + }); + assert_matches!(&batched_diffs[2], VectorDiff::PushFront { value } => { + assert_matches!(value.as_virtual(), Some(VirtualTimelineItem::DayDivider(_))); + }); +}