Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Timeline consumes updates as VectorDiffs from the EventCache #4408

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0a14007
feat(ui): Add `TimelineBuilder::with_vectordiffs_as_inputs`.
Hywan Dec 13, 2024
8b2635f
refactor(sdk): Add `RoomEventCacheUpdate::UpdateTimelineEvents`.
Hywan Dec 9, 2024
2cbdeb2
feat(ui): Add blank `handle_remote_events_with_diffs`.
Hywan Dec 9, 2024
ea0f1ab
task(ui): Support `VectorDiff::Append` in `TimelineStateTransaction::…
Hywan Dec 9, 2024
e8eff1b
task(ui): Support `VectorDiff::PushFront` in `TimelineStateTransactio…
Hywan Dec 9, 2024
c9248f7
task(ui): Support `VectorDiff::PushBack` in `TimelineStateTransaction…
Hywan Dec 9, 2024
7d25b01
task(ui): Support `VectorDiff::Clear` in `TimelineStateTransaction::h…
Hywan Dec 9, 2024
2a70ee6
task(ui): Add `ObservableItems::insert_remote_event`.
Hywan Dec 9, 2024
bb01c8a
task(ui): Add `AllRemoteEvents::range`.
Hywan Dec 10, 2024
291336e
task(ui): Support `VectorDiff::Insert` in `TimelineStateTransaction::…
Hywan Dec 10, 2024
262b9b5
task(ui): Support `VectorDiff::Remove,` in `TimelineStateTransaction:…
Hywan Dec 10, 2024
0ccb524
refactor(ui): Deduplicate remote events conditionnally.
Hywan Dec 13, 2024
c6ada9e
task(ui): `DayDivider` has been renamed `DateDivider`.
Hywan Dec 13, 2024
bded908
test(ui): Increase the `recursion_limit`.
Hywan Dec 13, 2024
1b77716
fix(common): Use a trick to avoid hitting the `recursion_limit` too q…
Hywan Dec 18, 2024
66c37a3
refactor(ui): Deduplicate timeline items conditionnally.
Hywan Dec 16, 2024
65ced49
refactor(ui): `Timeline` receives pagination events as `VectorDiff`s!
Hywan Dec 17, 2024
0aa794f
chore(ui): Make Clippy happy.
Hywan Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions crates/matrix-sdk-common/src/deserialized_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -316,6 +335,21 @@ pub struct SyncTimelineEvent {
pub push_actions: Vec<Action>,
}

const _: fn() = || {
fn assert_sync_and_send<T: ?Sized + Sync + Send>() {}

// `SyncTimelineEvent::kind` is of type `TimelineEventKind`.
assert_sync_and_send::<TimelineEventKind>();

// `SyncTimelineEvent::push_actions` is of type `Vec<Action>`.
assert_sync_and_send::<Vec<Action>>();
};

// 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.
///
Expand Down
37 changes: 31 additions & 6 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -155,6 +163,7 @@ impl TimelineBuilder {
)]
pub async fn build(self) -> Result<Timeline, Error> {
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();
Expand Down Expand Up @@ -276,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 {
EventsOrigin::Sync => RemoteEventOrigin::Sync,
}
}
).await;
}
}

inner.add_events_at(
events.into_iter(),
TimelineNewItemPosition::End { origin: match origin {
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 } => {
Expand Down
35 changes: 34 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,20 +124,29 @@ pub(super) struct TimelineController<P: RoomDataProvider = Room> {
pub(crate) room_data_provider: P,

/// Settings applied to this timeline.
settings: TimelineSettings,
pub(super) settings: TimelineSettings,
}

#[derive(Clone)]
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<TimelineEventFilterFn>,

/// 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))]
Expand All @@ -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()
}
}
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -665,6 +676,27 @@ impl<P: RoomDataProvider> TimelineController<P> {
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<VectorDiff<SyncTimelineEvent>>,
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();
}
Expand Down Expand Up @@ -757,6 +789,7 @@ impl<P: RoomDataProvider> TimelineController<P> {
txn_id,
send_handle,
content,
&self.settings,
)
.await;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use std::{
cmp::Ordering,
collections::{vec_deque::Iter, VecDeque},
ops::Deref,
ops::{Deref, RangeBounds},
sync::Arc,
};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1155,11 +1162,25 @@ mod observable_items_tests {
pub struct AllRemoteEvents(VecDeque<EventMeta>);

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()
}

/// Return a front-to-back iterator covering ranges of all remote events
/// describes by `range`.
pub fn range<R>(&self, range: R) -> Iter<'_, EventMeta>
where
R: RangeBounds<usize>,
{
self.0.range(range)
}

/// Remove all remote events.
fn clear(&mut self) {
self.0.clear();
Expand Down Expand Up @@ -1189,6 +1210,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<EventMeta> {
// Remove the event.
Expand Down Expand Up @@ -1261,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;
Expand Down Expand Up @@ -1318,6 +1350,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();
Expand Down Expand Up @@ -1399,6 +1454,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();
Expand Down
Loading
Loading