From 0a14007feecae1d0797fecb8b64b8fc2fd64a7df Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 13 Dec 2024 08:56:13 +0100 Subject: [PATCH 01/18] feat(ui): Add `TimelineBuilder::with_vectordiffs_as_inputs`. This patch adds `with_vectordiffs_as_inputs` on `TimelineBuilder` and `vectordiffs_as_inputs` on `TimelineSettings`. This new flag allows to transition from one system to another for the `Timeline`, when enabled, the `Timeline` will accept `VectorDiff` for the inputs instead of `Vec`. --- crates/matrix-sdk-ui/src/timeline/builder.rs | 8 ++++++++ crates/matrix-sdk-ui/src/timeline/controller/mod.rs | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 44bba18431f..97afe7800f4 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -145,6 +145,14 @@ impl TimelineBuilder { self } + /// Use `VectorDiff`s as the new “input mechanism” for the `Timeline`. + /// + /// Read `TimelineSettings::vectordiffs_as_inputs` to learn more. + pub fn with_vectordiffs_as_inputs(mut self) -> Self { + self.settings.vectordiffs_as_inputs = true; + self + } + /// Create a [`Timeline`] with the options set on this builder. #[tracing::instrument( skip(self), diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index a90aafd9d91..d317f38a481 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -131,13 +131,22 @@ pub(super) struct TimelineController { pub(super) struct TimelineSettings { /// Should the read receipts and read markers be handled? pub(super) track_read_receipts: bool, + /// Event filter that controls what's rendered as a timeline item (and thus /// what can carry read receipts). pub(super) event_filter: Arc, + /// Are unparsable events added as timeline items of their own kind? pub(super) add_failed_to_parse: bool, + /// Should the timeline items be grouped by day or month? pub(super) date_divider_mode: DateDividerMode, + + /// Whether `VectorDiff` is the “input mechanism” to use. + /// + /// This mechanism will replace the existing one, but this runtime feature + /// flag is necessary for the transition and the testing phase. + pub(super) vectordiffs_as_inputs: bool, } #[cfg(not(tarpaulin_include))] @@ -146,6 +155,7 @@ impl fmt::Debug for TimelineSettings { f.debug_struct("TimelineSettings") .field("track_read_receipts", &self.track_read_receipts) .field("add_failed_to_parse", &self.add_failed_to_parse) + .field("vectordiffs_as_inputs", &self.vectordiffs_as_inputs) .finish_non_exhaustive() } } @@ -157,6 +167,7 @@ impl Default for TimelineSettings { event_filter: Arc::new(default_event_filter), add_failed_to_parse: true, date_divider_mode: DateDividerMode::Daily, + vectordiffs_as_inputs: false, } } } From 8b2635fa000a1747f34460ae3ea4bca792fd9605 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 11:51:37 +0100 Subject: [PATCH 02/18] refactor(sdk): Add `RoomEventCacheUpdate::UpdateTimelineEvents`. This patch adds a new variant to `RoomEventCacheUpdate`, namely `UpdateTimelineEvents. It's going to replace `AddTimelineEvents` soon once it's stable enough. This is a transition. They are read by the `Timeline` if and only if `TimelineSettings::vectordiffs_as_inputs` is turned on. --- crates/matrix-sdk/src/event_cache/mod.rs | 11 +++++++++++ crates/matrix-sdk/src/event_cache/room/mod.rs | 18 +++++++++++++++--- .../tests/integration/event_cache.rs | 14 ++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/mod.rs b/crates/matrix-sdk/src/event_cache/mod.rs index 762e2a64950..7c4764785df 100644 --- a/crates/matrix-sdk/src/event_cache/mod.rs +++ b/crates/matrix-sdk/src/event_cache/mod.rs @@ -34,6 +34,7 @@ use std::{ }; use eyeball::Subscriber; +use eyeball_im::VectorDiff; use matrix_sdk_base::{ deserialized_responses::{AmbiguityChange, SyncTimelineEvent, TimelineEvent}, event_cache::store::{EventCacheStoreError, EventCacheStoreLock}, @@ -543,6 +544,7 @@ pub enum RoomEventCacheUpdate { }, /// The room has received new timeline events. + // TODO: remove once `UpdateTimelineEvents` is stabilized AddTimelineEvents { /// All the new events that have been added to the room's timeline. events: Vec, @@ -551,6 +553,15 @@ pub enum RoomEventCacheUpdate { origin: EventsOrigin, }, + /// The room has received updates for the timeline as _diffs_. + UpdateTimelineEvents { + /// Diffs to apply to the timeline. + diffs: Vec>, + + /// Where the diffs are coming from. + origin: EventsOrigin, + }, + /// The room has received new ephemeral events. AddEphemeralEvents { /// XXX: this is temporary, until read receipts are handled in the event diff --git a/crates/matrix-sdk/src/event_cache/room/mod.rs b/crates/matrix-sdk/src/event_cache/room/mod.rs index ddbb5ceab56..e2c37f186f7 100644 --- a/crates/matrix-sdk/src/event_cache/room/mod.rs +++ b/crates/matrix-sdk/src/event_cache/room/mod.rs @@ -529,14 +529,16 @@ impl RoomEventCacheInner { // Add the previous back-pagination token (if present), followed by the timeline // events themselves. - { - state + let sync_timeline_events_diffs = { + let sync_timeline_events_diffs = state .with_events_mut(|room_events| { if let Some(prev_token) = &prev_batch { room_events.push_gap(Gap { prev_token: prev_token.clone() }); } room_events.push_events(sync_timeline_events.clone()); + + room_events.updates_as_vector_diffs() }) .await?; @@ -547,7 +549,9 @@ impl RoomEventCacheInner { cache.events.insert(event_id.to_owned(), (self.room_id.clone(), ev.clone())); } } - } + + sync_timeline_events_diffs + }; // Now that all events have been added, we can trigger the // `pagination_token_notifier`. @@ -557,6 +561,7 @@ impl RoomEventCacheInner { // The order of `RoomEventCacheUpdate`s is **really** important here. { + // TODO: remove once `UpdateTimelineEvents` is stabilized. if !sync_timeline_events.is_empty() { let _ = self.sender.send(RoomEventCacheUpdate::AddTimelineEvents { events: sync_timeline_events, @@ -564,6 +569,13 @@ impl RoomEventCacheInner { }); } + if !sync_timeline_events_diffs.is_empty() { + let _ = self.sender.send(RoomEventCacheUpdate::UpdateTimelineEvents { + diffs: sync_timeline_events_diffs, + origin: EventsOrigin::Sync, + }); + } + if !ephemeral_events.is_empty() { let _ = self .sender diff --git a/crates/matrix-sdk/tests/integration/event_cache.rs b/crates/matrix-sdk/tests/integration/event_cache.rs index 0832fea4b5d..6540117fd89 100644 --- a/crates/matrix-sdk/tests/integration/event_cache.rs +++ b/crates/matrix-sdk/tests/integration/event_cache.rs @@ -83,6 +83,8 @@ async fn test_event_cache_receives_events() { assert_let_timeout!( Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() ); + // It does also receive the update as `VectorDiff`. + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); // Which contains the event that was sent beforehand. assert_eq!(events.len(), 1); @@ -169,6 +171,8 @@ async fn test_ignored_unignored() { assert_let_timeout!( Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() ); + // It does also receive the update as `VectorDiff`. + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "i don't like this dexter"); @@ -196,6 +200,10 @@ async fn wait_for_initial_events( update = room_stream.recv().await.expect("read error"); } assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { .. }); + + let update = room_stream.recv().await.expect("read error"); + + assert_matches!(update, RoomEventCacheUpdate::UpdateTimelineEvents { .. }); } else { assert_eq!(events.len(), 1); } @@ -806,6 +814,10 @@ async fn test_limited_timeline_with_storage() { assert_let_timeout!( Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() ); + // It does also receive the update as `VectorDiff`. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv() + ); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "hey yo"); } else { @@ -828,6 +840,8 @@ async fn test_limited_timeline_with_storage() { assert_let_timeout!( Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() ); + // It does also receive the update as `VectorDiff`. + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "gappy!"); From 2cbdeb22002a25d0d03ea006bd76220a624669a1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 13:54:23 +0100 Subject: [PATCH 03/18] feat(ui): Add blank `handle_remote_events_with_diffs`. --- crates/matrix-sdk-ui/src/timeline/builder.rs | 29 ++++++++++++---- .../src/timeline/controller/mod.rs | 21 ++++++++++++ .../src/timeline/controller/state.rs | 33 +++++++++++++++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 97afe7800f4..ab850666ef2 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -163,6 +163,7 @@ impl TimelineBuilder { )] pub async fn build(self) -> Result { let Self { room, settings, unable_to_decrypt_hook, focus, internal_id_prefix } = self; + let settings_vectordiffs_as_inputs = settings.vectordiffs_as_inputs; let client = room.client(); let event_cache = client.event_cache(); @@ -284,16 +285,32 @@ impl TimelineBuilder { inner.clear().await; } + // TODO: remove once `UpdateTimelineEvents` is stabilized. RoomEventCacheUpdate::AddTimelineEvents { events, origin } => { - trace!("Received new timeline events."); + if !settings_vectordiffs_as_inputs { + trace!("Received new timeline events."); - inner.add_events_at( - events.into_iter(), - TimelineNewItemPosition::End { origin: match origin { + inner.add_events_at( + events.into_iter(), + TimelineNewItemPosition::End { origin: match origin { + EventsOrigin::Sync => RemoteEventOrigin::Sync, + } + } + ).await; + } + } + + RoomEventCacheUpdate::UpdateTimelineEvents { diffs, origin } => { + if settings_vectordiffs_as_inputs { + trace!("Received new timeline events diffs"); + + inner.handle_remote_events_with_diffs( + diffs, + match origin { EventsOrigin::Sync => RemoteEventOrigin::Sync, } - } - ).await; + ).await; + } } RoomEventCacheUpdate::AddEphemeralEvents { events } => { diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index d317f38a481..3253ee66bcb 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -676,6 +676,27 @@ impl TimelineController

{ state.add_remote_events_at(events, position, &self.room_data_provider, &self.settings).await } + /// Handle updates on events as [`VectorDiff`]s. + pub(super) async fn handle_remote_events_with_diffs( + &self, + diffs: Vec>, + origin: RemoteEventOrigin, + ) { + if diffs.is_empty() { + return Default::default(); + } + + let mut state = self.state.write().await; + state + .handle_remote_events_with_diffs( + diffs, + origin, + &self.room_data_provider, + &self.settings, + ) + .await + } + pub(super) async fn clear(&self) { self.state.write().await.clear(); } diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index bfff486a6fe..c1b71560208 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -19,6 +19,7 @@ use std::{ sync::{Arc, RwLock}, }; +use eyeball_im::VectorDiff; use itertools::Itertools as _; use matrix_sdk::{ deserialized_responses::SyncTimelineEvent, ring_buffer::RingBuffer, send_queue::SendHandle, @@ -153,6 +154,25 @@ impl TimelineState { handle_many_res } + /// Handle updates on events as [`VectorDiff`]s. + pub(super) async fn handle_remote_events_with_diffs( + &mut self, + diffs: Vec>, + origin: RemoteEventOrigin, + room_data: &RoomData, + settings: &TimelineSettings, + ) where + RoomData: RoomDataProvider, + { + if diffs.is_empty() { + return Default::default(); + } + + let mut transaction = self.transaction(); + transaction.handle_remote_events_with_diffs(diffs, origin, room_data, settings).await; + transaction.commit(); + } + /// Marks the given event as fully read, using the read marker received from /// sync. pub(super) fn handle_fully_read_marker(&mut self, fully_read_event_id: OwnedEventId) { @@ -416,6 +436,19 @@ impl TimelineStateTransaction<'_> { total } + /// Handle updates on events as [`VectorDiff`]s. + pub(super) async fn handle_remote_events_with_diffs( + &mut self, + diffs: Vec>, + origin: RemoteEventOrigin, + room_data_provider: &RoomData, + settings: &TimelineSettings, + ) where + RoomData: RoomDataProvider, + { + todo!() + } + fn check_no_unused_unique_ids(&self) { let duplicates = self .items From ea0f1ab57ea76de5c03763bbf5402cd71a0fc118 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 15:31:34 +0100 Subject: [PATCH 04/18] task(ui): Support `VectorDiff::Append` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::Append`. --- .../src/timeline/controller/state.rs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index c1b71560208..2b34f9526e6 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -446,7 +446,26 @@ impl TimelineStateTransaction<'_> { ) where RoomData: RoomDataProvider, { - todo!() + let mut day_divider_adjuster = DayDividerAdjuster::default(); + + for diff in diffs { + match diff { + VectorDiff::Append { values: events } => { + for event in events { + self.handle_remote_event( + event, + TimelineItemPosition::End { origin }, + room_data_provider, + settings, + &mut day_divider_adjuster, + ) + .await; + } + } + + v => unimplemented!("{v:?}"), + } + } } fn check_no_unused_unique_ids(&self) { From e8eff1b951e7387fb3ba15870c4fc50085ed0141 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 15:37:38 +0100 Subject: [PATCH 05/18] task(ui): Support `VectorDiff::PushFront` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::PushFront`. --- crates/matrix-sdk-ui/src/timeline/controller/state.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 2b34f9526e6..783dc8279a5 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -463,6 +463,17 @@ impl TimelineStateTransaction<'_> { } } + VectorDiff::PushFront { value: event } => { + self.handle_remote_event( + event, + TimelineItemPosition::Start { origin }, + room_data_provider, + settings, + &mut day_divider_adjuster, + ) + .await; + } + v => unimplemented!("{v:?}"), } } From c9248f7f74248129dca7243724ce0036be4892a3 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 15:38:04 +0100 Subject: [PATCH 06/18] task(ui): Support `VectorDiff::PushBack` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::PushBack`. --- crates/matrix-sdk-ui/src/timeline/controller/state.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 783dc8279a5..9b400fbe98e 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -474,6 +474,17 @@ impl TimelineStateTransaction<'_> { .await; } + VectorDiff::PushBack { value: event } => { + self.handle_remote_event( + event, + TimelineItemPosition::End { origin }, + room_data_provider, + settings, + &mut day_divider_adjuster, + ) + .await; + } + v => unimplemented!("{v:?}"), } } From 7d25b0168786385806192094a266d90763c94225 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 15:40:01 +0100 Subject: [PATCH 07/18] task(ui): Support `VectorDiff::Clear` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::Clear`. --- crates/matrix-sdk-ui/src/timeline/controller/state.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 9b400fbe98e..60682dc5276 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -485,6 +485,10 @@ impl TimelineStateTransaction<'_> { .await; } + VectorDiff::Clear => { + self.clear(); + } + v => unimplemented!("{v:?}"), } } From 2a70ee6980dcdef0b82bf7d13ec1d5937ffb1994 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 9 Dec 2024 15:56:52 +0100 Subject: [PATCH 08/18] task(ui): Add `ObservableItems::insert_remote_event`. This patch adds the `ObservavbleItems::insert_remote_event` method. This is going to be useful to implement `VectorDiff::Insert` inside `TimelineStateTransaction::handle_remote_events_with_diffs`. --- .../timeline/controller/observable_items.rs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs index e5f62d70716..a02614862de 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs @@ -220,6 +220,13 @@ impl<'observable_items> ObservableItemsTransaction<'observable_items> { self.all_remote_events.push_back(event_meta); } + /// Insert a new remote event at a specific index. + /// + /// Not to be confused with inserting a timeline item! + pub fn insert_remote_event(&mut self, event_index: usize, event_meta: EventMeta) { + self.all_remote_events.insert(event_index, event_meta); + } + /// Get a remote event by using an event ID. pub fn get_remote_event_by_event_id_mut( &mut self, @@ -1189,6 +1196,18 @@ impl AllRemoteEvents { self.0.push_back(event_meta) } + /// Insert a new remote event at a specific index. + fn insert(&mut self, event_index: usize, event_meta: EventMeta) { + // If there is an associated `timeline_item_index`, shift all the + // `timeline_item_index` that come after this one. + if let Some(new_timeline_item_index) = event_meta.timeline_item_index { + self.increment_all_timeline_item_index_after(new_timeline_item_index); + } + + // Insert the event. + self.0.insert(event_index, event_meta) + } + /// Remove one remote event at a specific index, and return it if it exists. fn remove(&mut self, event_index: usize) -> Option { // Remove the event. @@ -1399,6 +1418,37 @@ mod all_remote_events_tests { ); } + #[test] + fn test_insert() { + let mut events = AllRemoteEvents::default(); + + // Insert on an empty set, nothing particular. + events.insert(0, event_meta("$ev0", Some(0))); + + // Insert at the end with no `timeline_item_index`. + events.insert(1, event_meta("$ev1", None)); + + // Insert at the end with a `timeline_item_index`. + events.insert(2, event_meta("$ev2", Some(1))); + + // Insert at the start, with a `timeline_item_index`. + events.insert(0, event_meta("$ev3", Some(0))); + + assert_events!( + events, + [ + // `timeline_item_index` is untouched + ("$ev3", Some(0)), + // `timeline_item_index` has been shifted once + ("$ev0", Some(1)), + // no `timeline_item_index` + ("$ev1", None), + // `timeline_item_index` has been shifted once + ("$ev2", Some(2)), + ] + ); + } + #[test] fn test_remove() { let mut events = AllRemoteEvents::default(); From bb01c8a5454b7e5146ef90245c95899e3005c5f0 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 10 Dec 2024 09:23:14 +0100 Subject: [PATCH 09/18] task(ui): Add `AllRemoteEvents::range`. This patch adds the `AllRemoteEvents::range` method. This is going to be useful to support `VectorDiff::Insert` inside `TimelineStateTransaction::handle_remote_events_with_diffs`. --- .../timeline/controller/observable_items.rs | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs index a02614862de..743f9926bec 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs @@ -15,7 +15,7 @@ use std::{ cmp::Ordering, collections::{vec_deque::Iter, VecDeque}, - ops::Deref, + ops::{Deref, RangeBounds}, sync::Arc, }; @@ -1167,6 +1167,15 @@ impl AllRemoteEvents { self.0.iter() } + /// Return a front-to-back iterator covering ranges of all remote events + /// describes by `range`. + pub fn range(&self, range: R) -> Iter<'_, EventMeta> + where + R: RangeBounds, + { + self.0.range(range) + } + /// Remove all remote events. fn clear(&mut self) { self.0.clear(); @@ -1337,6 +1346,29 @@ mod all_remote_events_tests { } } + #[test] + fn test_range() { + let mut events = AllRemoteEvents::default(); + + // Push some events. + events.push_back(event_meta("$ev0", None)); + events.push_back(event_meta("$ev1", None)); + events.push_back(event_meta("$ev2", None)); + + assert_eq!(events.iter().count(), 3); + + // Iterate on some of them. + let mut some_events = events.range(1..); + + assert_matches!(some_events.next(), Some(EventMeta { event_id, .. }) => { + assert_eq!(event_id.as_str(), "$ev1"); + }); + assert_matches!(some_events.next(), Some(EventMeta { event_id, .. }) => { + assert_eq!(event_id.as_str(), "$ev2"); + }); + assert!(some_events.next().is_none()); + } + #[test] fn test_clear() { let mut events = AllRemoteEvents::default(); From 291336eef27739631ce95f40b70d0a20c05d5a3b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 10 Dec 2024 09:38:03 +0100 Subject: [PATCH 10/18] task(ui): Support `VectorDiff::Insert` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::Insert`. --- .../src/timeline/controller/state.rs | 22 ++++++- .../src/timeline/event_handler.rs | 63 ++++++++++++++++++- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 60682dc5276..c69a016a017 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -485,6 +485,17 @@ impl TimelineStateTransaction<'_> { .await; } + VectorDiff::Insert { index: event_index, value: event } => { + self.handle_remote_event( + event, + TimelineItemPosition::At { event_index, origin }, + room_data_provider, + settings, + &mut day_divider_adjuster, + ) + .await; + } + VectorDiff::Clear => { self.clear(); } @@ -550,7 +561,8 @@ impl TimelineStateTransaction<'_> { // Retrieve the origin of the event. let origin = match position { TimelineItemPosition::End { origin } - | TimelineItemPosition::Start { origin } => origin, + | TimelineItemPosition::Start { origin } + | TimelineItemPosition::At { origin, .. } => origin, TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx } => self .items @@ -826,6 +838,10 @@ impl TimelineStateTransaction<'_> { self.items.push_back_remote_event(event_meta.base_meta()); } + TimelineItemPosition::At { event_index, .. } => { + self.items.insert_remote_event(event_index, event_meta.base_meta()); + } + TimelineItemPosition::UpdateDecrypted { .. } => { if let Some(event) = self.items.get_remote_event_by_event_id_mut(event_meta.event_id) @@ -846,7 +862,9 @@ impl TimelineStateTransaction<'_> { if settings.track_read_receipts && matches!( position, - TimelineItemPosition::Start { .. } | TimelineItemPosition::End { .. } + TimelineItemPosition::Start { .. } + | TimelineItemPosition::End { .. } + | TimelineItemPosition::At { .. } ) { self.load_read_receipts_for_event(event_meta.event_id, room_data_provider).await; diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 475c1f30a14..c180f4aee6e 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -290,6 +290,15 @@ pub(super) enum TimelineItemPosition { origin: RemoteEventOrigin, }, + /// One item is inserted to the timeline. + At { + /// Where to insert the remote event. + event_index: usize, + + /// The origin of the new item. + origin: RemoteEventOrigin, + }, + /// A single item is updated, after it's been successfully decrypted. /// /// This happens when an item that was a UTD must be replaced with the @@ -619,7 +628,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } - TimelineItemPosition::End { .. } => { + TimelineItemPosition::At { .. } | TimelineItemPosition::End { .. } => { // This is a more recent edit, coming either live from sync or from a // forward-pagination: it's fine to overwrite the previous one, if // available. @@ -1039,7 +1048,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { Flow::Remote { event_id, raw_event, position, txn_id, encryption_info, .. } => { let origin = match *position { TimelineItemPosition::Start { origin } - | TimelineItemPosition::End { origin } => origin, + | TimelineItemPosition::End { origin } + | TimelineItemPosition::At { origin, .. } => origin, // For updates, reuse the origin of the encrypted event. TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx } => self @@ -1108,6 +1118,55 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { self.items.push_front(item, Some(0)); } + Flow::Remote { position: TimelineItemPosition::At { event_index, .. }, .. } => { + let event_index = *event_index; + let timeline_item_index = self + .items + .all_remote_events() + // The remote events at the left. + // + // Note: the case where `event_index = 0` is matched with + // `TimelineItemPosition::Start`. + .range(0..=event_index) + .rev() + .find_map(|event_meta| event_meta.timeline_item_index) + // The new `timeline_item_index` is the previous + 1. + .map(|timeline_item_index| timeline_item_index + 1) + // + // No index? Look for the closest `timeline_item_index` at the right of the + // current events. + .or_else(|| { + self.items + .all_remote_events() + // The events at the right. + .range(event_index + 1..) + .find_map(|event_meta| event_meta.timeline_item_index) + }) + // + // No index? Well, it means there is no existing `timeline_item_index` + // so we are inserting at the last non-local item position as a fallback. + .unwrap_or_else(|| { + self.items + .iter() + .enumerate() + .rev() + .find_map(|(timeline_item_index, timeline_item)| { + (!timeline_item.as_event()?.is_local_echo()) + .then_some(timeline_item_index + 1) + }) + .unwrap_or(0) + }); + + trace!( + ?event_index, + ?timeline_item_index, + "Adding new remote timeline at specific event index" + ); + + let item = self.meta.new_timeline_item(item); + self.items.insert(timeline_item_index, item, Some(event_index)); + } + Flow::Remote { position: TimelineItemPosition::End { .. }, txn_id, event_id, .. } => { From 262b9b5ea0e2e09042ca3f03cddfe8771f81fc1c Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 10 Dec 2024 10:54:23 +0100 Subject: [PATCH 11/18] task(ui): Support `VectorDiff::Remove,` in `TimelineStateTransaction::handle_remote_events_with_diffs`. This patch updates `TimelineStateTransaction::handle_remote_events_with_diffs` to support `VectorDiff::Remove,`. --- .../timeline/controller/observable_items.rs | 8 +++-- .../src/timeline/controller/state.rs | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs index 743f9926bec..cbe7efc02f3 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs @@ -1162,6 +1162,11 @@ mod observable_items_tests { pub struct AllRemoteEvents(VecDeque); impl AllRemoteEvents { + /// Return a reference to a remote event. + pub fn get(&self, event_index: usize) -> Option<&EventMeta> { + self.0.get(event_index) + } + /// Return a front-to-back iterator over all remote events. pub fn iter(&self) -> Iter<'_, EventMeta> { self.0.iter() @@ -1289,8 +1294,7 @@ impl AllRemoteEvents { } /// Notify that a timeline item has been removed at - /// `new_timeline_item_index`. If `event_index` is `Some(_)`, it means the - /// remote event at `event_index` must be unmapped. + /// `new_timeline_item_index`. fn timeline_item_has_been_removed_at(&mut self, timeline_item_index_to_remove: usize) { for event_meta in self.0.iter_mut() { let mut remove_timeline_item_index = false; diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index c69a016a017..ef62fec5f05 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -496,6 +496,10 @@ impl TimelineStateTransaction<'_> { .await; } + VectorDiff::Remove { index: event_index } => { + self.remove_timeline_item(event_index, &mut day_divider_adjuster); + } + VectorDiff::Clear => { self.clear(); } @@ -503,6 +507,9 @@ impl TimelineStateTransaction<'_> { v => unimplemented!("{v:?}"), } } + + self.adjust_day_dividers(day_divider_adjuster); + self.check_no_unused_unique_ids(); } fn check_no_unused_unique_ids(&self) { @@ -739,6 +746,34 @@ impl TimelineStateTransaction<'_> { TimelineEventHandler::new(self, ctx).handle_event(date_divider_adjuster, event_kind).await } + /// Remove one timeline item by its `event_index`. + fn remove_timeline_item( + &mut self, + event_index: usize, + day_divider_adjuster: &mut DayDividerAdjuster, + ) { + day_divider_adjuster.mark_used(); + + // We need to be careful here. + // + // We must first remove the timeline item, which will update the mapping between + // remote events and timeline items. Removing the timeline item will “unlink” + // this mapping as the remote event will be updated to map to nothing. Only + // after that, we can remove the remote event. Doing this in the other order + // will update the mapping twice, and will result in a corrupted state. + + // Remove the timeline item first. + if let Some(event_meta) = self.items.all_remote_events().get(event_index) { + // Fetch the `timeline_item_index` associated to the remote event. + if let Some(timeline_item_index) = event_meta.timeline_item_index { + let _removed_timeine_item = self.items.remove(timeline_item_index); + } + + // Now we can remove the remote event. + self.items.remove_remote_event(event_index); + } + } + fn clear(&mut self) { let has_local_echoes = self.items.iter().any(|item| item.is_local_echo()); From 0ccb524cef60caa9dadf6ef8edc38f36457a5150 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 13 Dec 2024 09:10:08 +0100 Subject: [PATCH 12/18] refactor(ui): Deduplicate remote events conditionnally. The `Timeline` has its own remote event deduplication mechanism. But we are transitioning to receive updates from the `EventCache` via `VectorDiff`, which are emitted via `RoomEvents`, which already runs its own deduplication mechanism (`matrix_sdk::event_cache::Deduplicator`). Deduplication from the `EventCache` will generate `VectorDiff::Remove` for example. It can create a conflict with the `Timeline` deduplication mechanism. This patch updates the deduplication mechanism from the `Timeline` when adding or updating remote events to be conditionnal: when `TimelineSettings::vectordiffs_as_inputs` is enabled, the deduplication mechanism of the `Timeline` is silent, it does nothing, otherwise it runs. --- .../src/timeline/controller/state.rs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index ef62fec5f05..ad3b192d85d 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -842,33 +842,38 @@ impl TimelineStateTransaction<'_> { room_data_provider: &P, settings: &TimelineSettings, ) { - // Detect if an event already exists in [`ObservableItems::all_remote_events`]. - // - // Returns its position, in this case. - fn event_already_exists( + /// Remove duplicated events. + /// + /// If `VectorDiff`s are the inputs of the `Timeline`, this is not + /// necessary, as they are generated by the `EventCache`, which supports + /// its own deduplication algorithm. + fn deduplicate( new_event_id: &EventId, - all_remote_events: &AllRemoteEvents, - ) -> Option { - all_remote_events.iter().position(|EventMeta { event_id, .. }| event_id == new_event_id) + items: &mut ObservableItemsTransaction<'_>, + settings: &TimelineSettings, + ) { + if settings.vectordiffs_as_inputs { + return; + } + + if let Some(pos) = items + .all_remote_events() + .iter() + .position(|EventMeta { event_id, .. }| event_id == new_event_id) + { + items.remove_remote_event(pos); + } } match position { TimelineItemPosition::Start { .. } => { - if let Some(pos) = - event_already_exists(event_meta.event_id, self.items.all_remote_events()) - { - self.items.remove_remote_event(pos); - } + deduplicate(event_meta.event_id, &mut self.items, settings); self.items.push_front_remote_event(event_meta.base_meta()) } TimelineItemPosition::End { .. } => { - if let Some(pos) = - event_already_exists(event_meta.event_id, self.items.all_remote_events()) - { - self.items.remove_remote_event(pos); - } + deduplicate(event_meta.event_id, &mut self.items, settings); self.items.push_back_remote_event(event_meta.base_meta()); } From c6ada9e3b2c96619732614d67034011617212682 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 13 Dec 2024 09:47:21 +0100 Subject: [PATCH 13/18] task(ui): `DayDivider` has been renamed `DateDivider`. This patch updates this branch to `main` where `DayDivider` has been renamed `DateDivider`. --- .../src/timeline/controller/state.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index ad3b192d85d..2bce3da0ad4 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -446,7 +446,8 @@ impl TimelineStateTransaction<'_> { ) where RoomData: RoomDataProvider, { - let mut day_divider_adjuster = DayDividerAdjuster::default(); + let mut date_divider_adjuster = + DateDividerAdjuster::new(settings.date_divider_mode.clone()); for diff in diffs { match diff { @@ -457,7 +458,7 @@ impl TimelineStateTransaction<'_> { TimelineItemPosition::End { origin }, room_data_provider, settings, - &mut day_divider_adjuster, + &mut date_divider_adjuster, ) .await; } @@ -469,7 +470,7 @@ impl TimelineStateTransaction<'_> { TimelineItemPosition::Start { origin }, room_data_provider, settings, - &mut day_divider_adjuster, + &mut date_divider_adjuster, ) .await; } @@ -480,7 +481,7 @@ impl TimelineStateTransaction<'_> { TimelineItemPosition::End { origin }, room_data_provider, settings, - &mut day_divider_adjuster, + &mut date_divider_adjuster, ) .await; } @@ -491,13 +492,13 @@ impl TimelineStateTransaction<'_> { TimelineItemPosition::At { event_index, origin }, room_data_provider, settings, - &mut day_divider_adjuster, + &mut date_divider_adjuster, ) .await; } VectorDiff::Remove { index: event_index } => { - self.remove_timeline_item(event_index, &mut day_divider_adjuster); + self.remove_timeline_item(event_index, &mut date_divider_adjuster); } VectorDiff::Clear => { @@ -508,7 +509,7 @@ impl TimelineStateTransaction<'_> { } } - self.adjust_day_dividers(day_divider_adjuster); + self.adjust_date_dividers(date_divider_adjuster); self.check_no_unused_unique_ids(); } From bded9087022e10b585f41efded78041b7cde68df Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 13 Dec 2024 09:56:08 +0100 Subject: [PATCH 14/18] test(ui): Increase the `recursion_limit`. Since we have added a new variant to `RoomEventCacheUpdate`, a macro hits the recursion limit. It needs to be updated in order for tests to run again. --- crates/matrix-sdk-ui/tests/integration/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/matrix-sdk-ui/tests/integration/main.rs b/crates/matrix-sdk-ui/tests/integration/main.rs index da8bc38c3ee..59158ececd2 100644 --- a/crates/matrix-sdk-ui/tests/integration/main.rs +++ b/crates/matrix-sdk-ui/tests/integration/main.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#![recursion_limit = "256"] + use itertools::Itertools as _; use matrix_sdk::deserialized_responses::TimelineEvent; use ruma::{events::AnyStateEvent, serde::Raw, EventId, RoomId}; From 1b77716c93784d648c83d0b1edc3c41452a19cc4 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 18 Dec 2024 12:24:07 +0100 Subject: [PATCH 15/18] fix(common): Use a trick to avoid hitting the `recursion_limit` too quickly. This patch adds a trick around `SyncTimelineEvent` to avoid reaching the `recursion_limit` too quickly. Read the documentation in this patch to learn more. --- .../src/deserialized_responses.rs | 34 +++++++++++++++++++ .../matrix-sdk-ui/tests/integration/main.rs | 2 -- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index a1b07c0267e..20025db8c5e 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -306,6 +306,25 @@ pub struct EncryptionInfo { /// Previously, this differed from [`TimelineEvent`] by wrapping an /// [`AnySyncTimelineEvent`] instead of an [`AnyTimelineEvent`], but nowadays /// they are essentially identical, and one of them should probably be removed. +// +// 🚨 Note about this type, please read! 🚨 +// +// `SyncTimelineEvent` is heavily used across the SDK crates. In some cases, we +// are reaching a [`recursion_limit`] when the compiler is trying to figure out +// if `SyncTimelineEvent` implements `Sync` when it's embedded in other types. +// +// We want to help the compiler so that one doesn't need to increase the +// `recursion_limit`. We stop the recursive check by (un)safely implement `Sync` +// and `Send` on `SyncTimelineEvent` directly. However, before doing that, we +// need to ensure that every field of `SyncTimelineEvent` implements `Sync` and +// `Send` too. That's why we have `assert_sync_and_send` in a const expression +// so that it is checked at compile-time. If these checks pass, we can (un)safely +// implement `Sync` and `Send` for `SyncTimelineEvent`. +// +// If a field must be added to this type, please update the calls to +// `assert_sync_and_send`. +// +// [`recursion_limit`]: https://doc.rust-lang.org/reference/attributes/limits.html#the-recursion_limit-attribute #[derive(Clone, Debug, Serialize)] pub struct SyncTimelineEvent { /// The event itself, together with any information on decryption. @@ -316,6 +335,21 @@ pub struct SyncTimelineEvent { pub push_actions: Vec, } +const _: fn() = || { + fn assert_sync_and_send() {} + + // `SyncTimelineEvent::kind` is of type `TimelineEventKind`. + assert_sync_and_send::(); + + // `SyncTimelineEvent::push_actions` is of type `Vec`. + assert_sync_and_send::>(); +}; + +// All fields of `SyncTimelineEvent` implements `Sync` and `Send`: we can safely +// implement `Sync` and `Send` for `SyncTimelineEvent` directly. +unsafe impl Sync for SyncTimelineEvent {} +unsafe impl Send for SyncTimelineEvent {} + impl SyncTimelineEvent { /// Create a new `SyncTimelineEvent` from the given raw event. /// diff --git a/crates/matrix-sdk-ui/tests/integration/main.rs b/crates/matrix-sdk-ui/tests/integration/main.rs index 59158ececd2..da8bc38c3ee 100644 --- a/crates/matrix-sdk-ui/tests/integration/main.rs +++ b/crates/matrix-sdk-ui/tests/integration/main.rs @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![recursion_limit = "256"] - use itertools::Itertools as _; use matrix_sdk::deserialized_responses::TimelineEvent; use ruma::{events::AnyStateEvent, serde::Raw, EventId, RoomId}; From 66c37a3b573ec79ae9494ede58c0d515e47d5153 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 16 Dec 2024 16:38:15 +0100 Subject: [PATCH 16/18] refactor(ui): Deduplicate timeline items conditionnally. A previous patch deduplicates the remote events conditionnally. This patch does the same but for timeline items. The `Timeline` has its own deduplication algorithm (for remote events, and for timeline items). The `Timeline` is about to receive its updates via the `EventCache` which has its own deduplication mechanism (`matrix_sdk::event_cache::Deduplicator`). To avoid conflicts between the two, we conditionnally deduplicate timeline items based on `TimelineSettings::vectordiffs_as_inputs`. This patch takes the liberty to refactor the deduplication mechanism of the timeline items to make it explicit with its own methods, so that it can be re-used for `TimelineItemPosition::At`. A specific short-circuit was present before, which is no more possible with the rewrite to a generic mechanism. Consequently, when a local timeline item becomes a remote timeline item, it was previously updated (via `ObservableItems::replace`), but now the local timeline item is removed (via `ObservableItems::remove`), and then the remote timeline item is inserted (via `ObservableItems::insert`). Depending of whether a virtual timeline item like a date divider is around, the position of the removal and the insertion might not be the same (!), which is perfectly fine as the date divider will be re-computed anyway. The result is exactly the same, but the `VectorDiff` updates emitted by the `Timeline` are a bit different (different paths, same result). This is why this patch needs to update a couple of tests. --- .../src/timeline/controller/mod.rs | 1 + .../src/timeline/controller/state.rs | 9 +- .../src/timeline/event_handler.rs | 267 ++++++++++++------ .../matrix-sdk-ui/src/timeline/tests/echo.rs | 23 +- .../src/timeline/tests/shields.rs | 12 +- .../tests/integration/timeline/echo.rs | 6 +- .../tests/integration/timeline/queue.rs | 9 +- .../src/tests/timeline.rs | 49 +++- 8 files changed, 252 insertions(+), 124 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 3253ee66bcb..902d69bbee0 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -789,6 +789,7 @@ impl TimelineController

{ txn_id, send_handle, content, + &self.settings, ) .await; } diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 2bce3da0ad4..3b30de0279f 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -223,6 +223,7 @@ impl TimelineState { txn_id: OwnedTransactionId, send_handle: Option, content: TimelineEventKind, + settings: &TimelineSettings, ) { let ctx = TimelineEventContext { sender: own_user_id, @@ -240,7 +241,7 @@ impl TimelineState { let mut date_divider_adjuster = DateDividerAdjuster::new(date_divider_mode); - TimelineEventHandler::new(&mut txn, ctx) + TimelineEventHandler::new(&mut txn, ctx, settings) .handle_event(&mut date_divider_adjuster, content) .await; @@ -744,14 +745,16 @@ impl TimelineStateTransaction<'_> { }; // Handle the event to create or update a timeline item. - TimelineEventHandler::new(self, ctx).handle_event(date_divider_adjuster, event_kind).await + TimelineEventHandler::new(self, ctx, settings) + .handle_event(date_divider_adjuster, event_kind) + .await } /// Remove one timeline item by its `event_index`. fn remove_timeline_item( &mut self, event_index: usize, - day_divider_adjuster: &mut DayDividerAdjuster, + day_divider_adjuster: &mut DateDividerAdjuster, ) { day_divider_adjuster.mark_used(); diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index c180f4aee6e..67d58c5379d 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -46,13 +46,14 @@ use ruma::{ }, serde::Raw, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, + TransactionId, }; use tracing::{debug, error, field::debug, info, instrument, trace, warn}; use super::{ controller::{ ObservableItemsTransaction, ObservableItemsTransactionEntry, PendingEdit, PendingEditKind, - TimelineMetadata, TimelineStateTransaction, + TimelineMetadata, TimelineSettings, TimelineStateTransaction, }, date_dividers::DateDividerAdjuster, event_item::{ @@ -337,15 +338,17 @@ pub(super) struct TimelineEventHandler<'a, 'o> { meta: &'a mut TimelineMetadata, ctx: TimelineEventContext, result: HandleEventResult, + settings: &'a TimelineSettings, } impl<'a, 'o> TimelineEventHandler<'a, 'o> { pub(super) fn new( state: &'a mut TimelineStateTransaction<'o>, ctx: TimelineEventContext, + settings: &'a TimelineSettings, ) -> Self { let TimelineStateTransaction { items, meta, .. } = state; - Self { items, meta, ctx, result: HandleEventResult::default() } + Self { items, meta, ctx, result: HandleEventResult::default(), settings } } /// Handle an event. @@ -1097,28 +1100,47 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { Flow::Local { .. } => { trace!("Adding new local timeline item"); - let item = self.meta.new_timeline_item(item); + let item = Self::new_timeline_item(self.meta, item, None); + self.items.push_back(item, None); } - Flow::Remote { position: TimelineItemPosition::Start { .. }, event_id, .. } => { - if self - .items - .iter() - .filter_map(|ev| ev.as_event()?.event_id()) - .any(|id| id == event_id) - { - trace!("Skipping back-paginated event that has already been seen"); - return; - } + Flow::Remote { + position: TimelineItemPosition::Start { .. }, event_id, txn_id, .. + } => { + let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item( + self.items, + &mut item, + Some(event_id), + txn_id.as_ref().map(AsRef::as_ref), + self.meta, + self.settings, + ); + let item = + Self::new_timeline_item(self.meta, item, removed_duplicated_timeline_item); trace!("Adding new remote timeline item at the start"); - let item = self.meta.new_timeline_item(item); self.items.push_front(item, Some(0)); } - Flow::Remote { position: TimelineItemPosition::At { event_index, .. }, .. } => { + Flow::Remote { + position: TimelineItemPosition::At { event_index, .. }, + event_id, + txn_id, + .. + } => { + let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item( + self.items, + &mut item, + Some(event_id), + txn_id.as_ref().map(AsRef::as_ref), + self.meta, + self.settings, + ); + let item = + Self::new_timeline_item(self.meta, item, removed_duplicated_timeline_item); + let event_index = *event_index; let timeline_item_index = self .items @@ -1163,69 +1185,22 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { "Adding new remote timeline at specific event index" ); - let item = self.meta.new_timeline_item(item); self.items.insert(timeline_item_index, item, Some(event_index)); } Flow::Remote { - position: TimelineItemPosition::End { .. }, txn_id, event_id, .. + position: TimelineItemPosition::End { .. }, event_id, txn_id, .. } => { - // Look if we already have a corresponding item somewhere, based on the - // transaction id (if a local echo) or the event id (if a - // duplicate remote event). - let result = rfind_event_item(self.items, |it| { - txn_id.is_some() && it.transaction_id() == txn_id.as_deref() - || it.event_id() == Some(event_id) - }); - - let mut removed_event_item_id = None; - - if let Some((idx, old_item)) = result { - if old_item.as_remote().is_some() { - // Item was previously received from the server. This should be very rare - // normally, but with the sliding- sync proxy, it is actually very - // common. - // NOTE: SS proxy workaround. - trace!(?item, old_item = ?*old_item, "Received duplicate event"); - - if old_item.content.is_redacted() && !item.content.is_redacted() { - warn!("Got original form of an event that was previously redacted"); - item.content = item.content.redact(&self.meta.room_version); - item.reactions.clear(); - } - } - - // TODO: Check whether anything is different about the - // old and new item? - - transfer_details(&mut item, &old_item); - - let old_item_id = old_item.internal_id; - - if idx == self.items.len() - 1 { - // If the old item is the last one and no date divider - // changes need to happen, replace and return early. - trace!(idx, "Replacing existing event"); - self.items.replace(idx, TimelineItem::new(item, old_item_id.to_owned())); - return; - } - - // In more complex cases, remove the item before re-adding the item. - trace!("Removing local echo or duplicate timeline item"); - removed_event_item_id = Some(self.items.remove(idx).internal_id.clone()); - - // no return here, below code for adding a new event - // will run to re-add the removed item - } - - trace!("Adding new remote timeline item after all non-pending events"); - let new_item = match removed_event_item_id { - // If a previous version of the same item (usually a local - // echo) was removed and we now need to add it again, reuse - // the previous item's ID. - Some(id) => TimelineItem::new(item, id), - None => self.meta.new_timeline_item(item), - }; + let removed_duplicated_timeline_item = Self::deduplicate_local_timeline_item( + self.items, + &mut item, + Some(event_id), + txn_id.as_ref().map(AsRef::as_ref), + self.meta, + self.settings, + ); + let item = + Self::new_timeline_item(self.meta, item, removed_duplicated_timeline_item); // Local events are always at the bottom. Let's find the latest remote event // and insert after it, otherwise, if there is no remote event, insert at 0. @@ -1263,16 +1238,16 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { if timeline_item_index == self.items.len() { trace!("Adding new remote timeline item at the back"); - self.items.push_back(new_item, event_index); + self.items.push_back(item, event_index); } else if timeline_item_index == 0 { trace!("Adding new remote timeline item at the front"); - self.items.push_front(new_item, event_index); + self.items.push_front(item, event_index); } else { trace!( timeline_item_index, "Adding new remote timeline item at specific index" ); - self.items.insert(timeline_item_index, new_item, event_index); + self.items.insert(timeline_item_index, item, event_index); } } @@ -1297,6 +1272,128 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } + /// Remove the local timeline item matching the `event_id` or the + /// `transaction_id` of `new_event_timeline_item` if it exists. + /// + /// Let's also try to deduplicate remote evevents. If `VectorDiff`s are the + /// inputs of the `Timeline`, this is not necessary, as they are + /// generated by the `EventCache`, which supports its own deduplication + /// algorithm. + // + // Note: this method doesn't take `&mut self` to avoid a borrow checker + // conflict with `TimelineEventHandler::add_item`. It's fine. + fn deduplicate_local_timeline_item( + items: &mut ObservableItemsTransaction<'_>, + new_event_timeline_item: &mut EventTimelineItem, + event_id: Option<&EventId>, + transaction_id: Option<&TransactionId>, + metadata: &TimelineMetadata, + settings: &TimelineSettings, + ) -> Option> { + /// Transfer `TimelineDetails` that weren't available on the original + /// item and have been fetched separately (only `reply_to` for + /// now) from `old_item` to `item`, given two items for an event + /// that was re-received. + /// + /// `old_item` *should* always be a local timeline item usually, but it + /// can be a remote timeline item. + fn transfer_details(new_item: &mut EventTimelineItem, old_item: &EventTimelineItem) { + let TimelineItemContent::Message(msg) = &mut new_item.content else { return }; + let TimelineItemContent::Message(old_msg) = &old_item.content else { return }; + + let Some(in_reply_to) = &mut msg.in_reply_to else { return }; + let Some(old_in_reply_to) = &old_msg.in_reply_to else { return }; + + if matches!(&in_reply_to.event, TimelineDetails::Unavailable) { + in_reply_to.event = old_in_reply_to.event.clone(); + } + } + + // Start with the canonical case: detect a local timeline item that matches + // `event_id` or `transaction_id`. + if let Some((local_timeline_item_index, local_timeline_item)) = + rfind_event_item(items, |event_timeline_item| { + if event_timeline_item.is_local_echo() { + event_id == event_timeline_item.event_id() + || (transaction_id.is_some() + && transaction_id == event_timeline_item.transaction_id()) + } else { + false + } + }) + { + trace!( + ?event_id, + ?transaction_id, + ?local_timeline_item_index, + "Removing local timeline item" + ); + + transfer_details(new_event_timeline_item, &local_timeline_item); + + // Remove the local timeline item. + return Some(items.remove(local_timeline_item_index)); + }; + + if !settings.vectordiffs_as_inputs { + if let Some((remote_timeline_item_index, remote_timeline_item)) = + rfind_event_item(items, |event_timeline_item| { + if event_timeline_item.is_remote_event() { + event_id == event_timeline_item.event_id() + } else { + false + } + }) + { + trace!( + ?event_id, + ?transaction_id, + ?remote_timeline_item_index, + "Removing remote timeline item (it is a duplicate)" + ); + + if remote_timeline_item.content.is_redacted() + && !new_event_timeline_item.content.is_redacted() + { + warn!("Got original form of an event that was previously redacted"); + new_event_timeline_item.content = + new_event_timeline_item.content.redact(&metadata.room_version); + new_event_timeline_item.reactions.clear(); + } + + transfer_details(new_event_timeline_item, &remote_timeline_item); + + // Remove the remote timeline item. + return Some(items.remove(remote_timeline_item_index)); + } + } + + None + } + + /// Create a new timeline item from an [`EventTimelineItem`]. + /// + /// It is possible that the new timeline item replaces a duplicated timeline + /// event (see [`TimelineEventHandler::deduplicate_local_timeline_item`]) in + /// case it replaces a local timeline item. + // + // Note: this method doesn't take `&mut self` to avoid a borrow checker + // conflict with `Self::add_item`. It's fine. + fn new_timeline_item( + metadata: &mut TimelineMetadata, + event_timeline_item: EventTimelineItem, + replaced_timeline_item: Option>, + ) -> Arc { + match replaced_timeline_item { + // Reuse the internal ID. + Some(to_replace_timeline_item) => { + TimelineItem::new(event_timeline_item, to_replace_timeline_item.internal_id.clone()) + } + + None => metadata.new_timeline_item(event_timeline_item), + } + } + /// After updating the timeline item `new_item` which id is /// `target_event_id`, update other items that are responses to this item. fn maybe_update_responses( @@ -1355,21 +1452,3 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { }) } } - -/// Transfer `TimelineDetails` that weren't available on the original item and -/// have been fetched separately (only `reply_to` for now) from `old_item` to -/// `item`, given two items for an event that was re-received. -/// -/// `old_item` *should* always be a local echo usually, but with the sliding -/// sync proxy, we often re-receive remote events that aren't remote echoes. -fn transfer_details(item: &mut EventTimelineItem, old_item: &EventTimelineItem) { - let TimelineItemContent::Message(msg) = &mut item.content else { return }; - let TimelineItemContent::Message(old_msg) = &old_item.content else { return }; - - let Some(in_reply_to) = &mut msg.in_reply_to else { return }; - let Some(old_in_reply_to) = &old_msg.in_reply_to else { return }; - - if matches!(&in_reply_to.event, TimelineDetails::Unavailable) { - in_reply_to.event = old_in_reply_to.event.clone(); - } -} diff --git a/crates/matrix-sdk-ui/src/timeline/tests/echo.rs b/crates/matrix-sdk-ui/src/timeline/tests/echo.rs index 533c1c2c56d..683f643ec85 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/echo.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/echo.rs @@ -24,7 +24,7 @@ use ruma::{ events::{room::message::RoomMessageEventContent, AnyMessageLikeEventContent}, user_id, MilliSecondsSinceUnixEpoch, }; -use stream_assert::assert_next_matches; +use stream_assert::{assert_next_matches, assert_pending}; use super::TestTimeline; use crate::timeline::{ @@ -121,9 +121,18 @@ async fn test_remote_echo_full_trip() { .await; // The local echo is replaced with the remote echo. - let item = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); + assert_next_matches!(stream, VectorDiff::Remove { index: 1 }); + let item = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); assert!(!item.as_event().unwrap().is_local_echo()); assert_eq!(*item.unique_id(), id); + + // The date divider is adjusted. + // A new date divider is inserted, and the older one is removed. + let date_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); + assert!(date_divider.is_date_divider()); + assert_next_matches!(stream, VectorDiff::Remove { index: 2 }); + + assert_pending!(stream); } #[async_test] @@ -168,12 +177,14 @@ async fn test_remote_echo_new_position() { .await; // … the remote echo replaces the previous event. - let item = assert_next_matches!(stream, VectorDiff::Set { index: 3, value } => value); + assert_next_matches!(stream, VectorDiff::Remove { index: 3 }); + let item = assert_next_matches!(stream, VectorDiff::Insert { index: 2, value} => value); assert!(!item.as_event().unwrap().is_local_echo()); - // … the date divider is removed (because both bob's and alice's message are - // from the same day according to server timestamps). - assert_next_matches!(stream, VectorDiff::Remove { index: 2 }); + // Date divider is updated. + assert_next_matches!(stream, VectorDiff::Remove { index: 3 }); + + assert_pending!(stream); } #[async_test] diff --git a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs index b87177e4ed0..c7f50c26731 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/shields.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/shields.rs @@ -14,7 +14,7 @@ use ruma::{ AnyMessageLikeEventContent, }, }; -use stream_assert::assert_next_matches; +use stream_assert::{assert_next_matches, assert_pending}; use crate::timeline::{tests::TestTimeline, EventSendState}; @@ -108,7 +108,8 @@ async fn test_local_sent_in_clear_shield() { "type": "m.room.message", }))) .await; - let item = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); + assert_next_matches!(stream, VectorDiff::Remove { index: 1 }); + let item = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); let event_item = item.as_event().unwrap(); // Then the remote echo should now be showing the shield. @@ -118,6 +119,13 @@ async fn test_local_sent_in_clear_shield() { shield, Some(ShieldState::Red { code: ShieldStateCode::SentInClear, message: "Not encrypted." }) ); + + // Date divider is adjusted. + let date_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); + assert!(date_divider.is_date_divider()); + assert_next_matches!(stream, VectorDiff::Remove { index: 2 }); + + assert_pending!(stream); } #[async_test] diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs b/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs index f00ad135d05..2d715963ab4 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs @@ -121,16 +121,18 @@ async fn test_echo() { server.reset().await; // Local echo is replaced with the remote echo. + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 1 }); let remote_echo = - assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => value); + assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => value); let item = remote_echo.as_event().unwrap(); assert!(item.is_own()); assert_eq!(item.timestamp(), MilliSecondsSinceUnixEpoch(uint!(152038280))); // The date divider is also replaced. let date_divider = - assert_next_matches!(timeline_stream, VectorDiff::Set { index: 0, value } => value); + assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => value); assert!(date_divider.is_date_divider()); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); } #[async_test] diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/queue.rs b/crates/matrix-sdk-ui/tests/integration/timeline/queue.rs index 32a11923dfc..d9d27689180 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/queue.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/queue.rs @@ -508,20 +508,21 @@ async fn test_no_duplicate_date_divider() { assert_eq!(value.event_id().unwrap(), "$PyHxV5mYzjetBUT3qZq7V95GOzxb02EP"); }); - // The second message is replaced -> [First DD Second] - assert_next_matches!(timeline_stream, VectorDiff::Set { index: 2, value } => { + // The second message is replaced -> [First Second DD] + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 1, value } => { let value = value.as_event().unwrap(); assert_eq!(value.content().as_message().unwrap().body(), "Second."); assert_eq!(value.event_id().unwrap(), "$5E2kLK/Sg342bgBU9ceEIEPYpbFaqJpZ"); }); - // A new date divider is inserted -> [DD First DD Second] + // A new date divider is inserted -> [DD First Second DD] assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => { assert!(value.is_date_divider()); }); // The useless date divider is removed. -> [DD First Second] - assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 3 }); assert_pending!(timeline_stream); } diff --git a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs index 0b63cfb1c26..cd7eb8fb6d5 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs @@ -41,6 +41,7 @@ use matrix_sdk_ui::{ timeline::{EventSendState, ReactionStatus, RoomExt, TimelineItem, TimelineItemContent}, }; use similar_asserts::assert_eq; +use stream_assert::assert_pending; use tokio::{ spawn, task::JoinHandle, @@ -274,22 +275,42 @@ async fn test_stale_local_echo_time_abort_edit() { // - or the remote echo comes up faster. // // Handle both orderings. - while let Ok(Some(vector_diff)) = timeout(Duration::from_secs(3), stream.next()).await { - let VectorDiff::Set { index: 0, value: echo } = vector_diff else { - panic!("unexpected diff: {vector_diff:#?}"); - }; + { + let mut diffs = Vec::with_capacity(3); + + while let Ok(Some(vector_diff)) = timeout(Duration::from_secs(5), stream.next()).await { + diffs.push(vector_diff); + } + + assert!(diffs.len() >= 3); + + for diff in diffs { + match diff { + VectorDiff::Set { index: 0, value: event } + | VectorDiff::PushBack { value: event } + | VectorDiff::Insert { index: 0, value: event } => { + if event.is_local_echo() { + // If the sender profile wasn't available, we may receive an update about + // it; ignore it. + if !has_sender_profile && event.sender_profile().is_ready() { + has_sender_profile = true; + continue; + } + + assert_matches!(event.send_state(), Some(EventSendState::Sent { .. })); + } + + assert!(event.is_editable()); + assert_eq!(event.content().as_message().unwrap().body(), "hi!"); + } + + VectorDiff::Remove { index } => assert_eq!(index, 0), - if echo.is_local_echo() { - // If the sender profile wasn't available, we may receive an update about it; - // ignore it. - if !has_sender_profile && echo.sender_profile().is_ready() { - has_sender_profile = true; - continue; + diff => { + panic!("unexpected diff: {diff:?}"); + } } - assert_matches!(echo.send_state(), Some(EventSendState::Sent { .. })); } - assert!(echo.is_editable()); - assert_eq!(echo.content().as_message().unwrap().body(), "hi!"); } // Now do a crime: try to edit the local echo. @@ -310,6 +331,8 @@ async fn test_stale_local_echo_time_abort_edit() { assert_eq!(remote_echo.content().as_message().unwrap().body(), "bonjour"); alice_sync.abort(); + + assert_pending!(stream); } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] From fc8302071d301e63d3c80f45e9f81b8a904c8f9e Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 17 Dec 2024 17:30:44 +0100 Subject: [PATCH 17/18] refactor(ui): `Timeline` receives pagination events as `VectorDiff`s! This patch allows the paginated events of a `Timeline` to be received via `RoomEventCacheUpdate::UpdateTimelineEvents` as `VectorDiff`s. --- .../src/timeline/controller/mod.rs | 2 +- .../matrix-sdk-ui/src/timeline/pagination.rs | 16 +++++++----- .../matrix-sdk/src/event_cache/pagination.rs | 11 +++++++- .../tests/integration/event_cache.rs | 25 ++++++++++++++++++- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 902d69bbee0..c7f6ad19b18 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -124,7 +124,7 @@ pub(super) struct TimelineController { pub(crate) room_data_provider: P, /// Settings applied to this timeline. - settings: TimelineSettings, + pub(super) settings: TimelineSettings, } #[derive(Clone)] diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index abe49acac0f..cc4e3e7a0ef 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -77,12 +77,16 @@ impl super::Timeline { let num_events = events.len(); trace!("Back-pagination succeeded with {num_events} events"); - // TODO(hywan): Remove, and let spread events via - // `matrix_sdk::event_cache::RoomEventCacheUpdate` from - // `matrix_sdk::event_cache::RoomPagination::run_backwards`. - self.controller - .add_events_at(events.into_iter(), TimelineNewItemPosition::Start { origin: RemoteEventOrigin::Pagination }) - .await; + // If `TimelineSettings::vectordiffs_as_inputs` is enabled, + // we don't need to add events manually: everything we need + // is to let the `EventCache` receives the events from this + // pagination, and emits its updates as `VectorDiff`s, which + // will be handled by the `Timeline` naturally. + if !self.controller.settings.vectordiffs_as_inputs { + self.controller + .add_events_at(events.into_iter(), TimelineNewItemPosition::Start { origin: RemoteEventOrigin::Pagination }) + .await; + } if num_events == 0 && !reached_start { // As an exceptional contract: if there were no events in the response, diff --git a/crates/matrix-sdk/src/event_cache/pagination.rs b/crates/matrix-sdk/src/event_cache/pagination.rs index 064acd5ae00..66cf1c042a5 100644 --- a/crates/matrix-sdk/src/event_cache/pagination.rs +++ b/crates/matrix-sdk/src/event_cache/pagination.rs @@ -28,7 +28,7 @@ use super::{ events::{Gap, RoomEvents}, RoomEventCacheInner, }, - BackPaginationOutcome, Result, + BackPaginationOutcome, EventsOrigin, Result, RoomEventCacheUpdate, }; /// An API object to run pagination queries on a [`super::RoomEventCache`]. @@ -211,6 +211,15 @@ impl RoomPagination { } } + let sync_timeline_events_diffs = room_events.updates_as_vector_diffs(); + + if !sync_timeline_events_diffs.is_empty() { + let _ = self.inner.sender.send(RoomEventCacheUpdate::UpdateTimelineEvents { + diffs: sync_timeline_events_diffs, + origin: EventsOrigin::Sync, + }); + } + BackPaginationOutcome { events, reached_start } }) .await?; diff --git a/crates/matrix-sdk/tests/integration/event_cache.rs b/crates/matrix-sdk/tests/integration/event_cache.rs index 6540117fd89..6341bf74313 100644 --- a/crates/matrix-sdk/tests/integration/event_cache.rs +++ b/crates/matrix-sdk/tests/integration/event_cache.rs @@ -280,6 +280,9 @@ async fn test_backpaginate_once() { assert_event_matches_msg(&events[1], "hello"); assert_eq!(events.len(), 2); + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + let next = room_stream.recv().now_or_never(); assert_matches!(next, None); } @@ -399,6 +402,14 @@ async fn test_backpaginate_many_times_with_many_iterations() { assert_event_matches_msg(&events[3], "heyo"); assert_eq!(events.len(), 4); + // First iteration. + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + + // Second iteration. + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + assert!(room_stream.is_empty()); } @@ -522,6 +533,14 @@ async fn test_backpaginate_many_times_with_one_iteration() { assert_event_matches_msg(&events[3], "heyo"); assert_eq!(events.len(), 4); + // First pagination. + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + + // Second pagination. + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + assert!(room_stream.is_empty()); } @@ -676,7 +695,7 @@ async fn test_backpaginating_without_token() { let room = server.sync_joined_room(&client, room_id).await; let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap(); - let (events, room_stream) = room_event_cache.subscribe().await.unwrap(); + let (events, mut room_stream) = room_event_cache.subscribe().await.unwrap(); assert!(events.is_empty()); assert!(room_stream.is_empty()); @@ -708,6 +727,9 @@ async fn test_backpaginating_without_token() { assert_event_matches_msg(&events[0], "hi"); assert_eq!(events.len(), 1); + let next = room_stream.recv().now_or_never(); + assert_matches!(next, Some(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }))); + assert!(room_stream.is_empty()); } @@ -768,6 +790,7 @@ async fn test_limited_timeline_resets_pagination() { server.sync_room(&client, JoinedRoomBuilder::new(room_id).set_timeline_limited()).await; // We receive an update about the limited timeline. + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = room_stream.recv()); assert_let_timeout!(Ok(RoomEventCacheUpdate::Clear) = room_stream.recv()); // The paginator state is reset: status set to Initial, hasn't hit the timeline From 382144001967595937775cc014b9e3e156813187 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 18 Dec 2024 11:05:53 +0100 Subject: [PATCH 18/18] chore(ui): Make Clippy happy. --- crates/matrix-sdk-ui/src/timeline/controller/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 3b30de0279f..bfe6b399551 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -798,7 +798,7 @@ impl TimelineStateTransaction<'_> { let mut idx = 0; while idx < self.items.len() { if self.items[idx].is_date_divider() - && self.items.get(idx + 1).map_or(true, |item| item.is_date_divider()) + && self.items.get(idx + 1).is_none_or(|item| item.is_date_divider()) { self.items.remove(idx); // don't increment idx because all elements have shifted