diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 622fef4ac2a..c5f02b028ed 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -31,10 +31,7 @@ use super::{ to_device::{handle_forwarded_room_key_event, handle_room_key_event}, DateDividerMode, Error, Timeline, TimelineDropHandle, TimelineFocus, }; -use crate::{ - timeline::{controller::TimelineNewItemPosition, event_item::RemoteEventOrigin}, - unable_to_decrypt_hook::UtdHookManager, -}; +use crate::{timeline::event_item::RemoteEventOrigin, unable_to_decrypt_hook::UtdHookManager}; /// Builder that allows creating and configuring various parts of a /// [`Timeline`]. @@ -145,14 +142,6 @@ 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), @@ -162,17 +151,11 @@ impl TimelineBuilder { ) )] pub async fn build(self) -> Result { - let Self { room, mut settings, unable_to_decrypt_hook, focus, internal_id_prefix } = self; + let Self { room, settings, unable_to_decrypt_hook, focus, internal_id_prefix } = self; let client = room.client(); let event_cache = client.event_cache(); - // Enable `TimelineSettings::vectordiffs_as_inputs` if and only if the event - // cache storage is enabled. - settings.vectordiffs_as_inputs = event_cache.has_storage(); - - let settings_vectordiffs_as_inputs = settings.vectordiffs_as_inputs; - // Subscribe the event cache to sync responses, in case we hadn't done it yet. event_cache.subscribe()?; @@ -290,35 +273,16 @@ impl TimelineBuilder { inner.clear().await; } - // TODO: remove once `UpdateTimelineEvents` is stabilized. - RoomEventCacheUpdate::AddTimelineEvents { events, origin } => { - 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, - EventsOrigin::Pagination => RemoteEventOrigin::Pagination, - } - } - ).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, - EventsOrigin::Pagination => RemoteEventOrigin::Pagination, - } - ).await; - } + trace!("Received new timeline events diffs"); + + inner.handle_remote_events_with_diffs( + diffs, + match origin { + EventsOrigin::Sync => RemoteEventOrigin::Sync, + EventsOrigin::Pagination => RemoteEventOrigin::Pagination, + } + ).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 36409f66222..faa4305ea7e 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -141,12 +141,6 @@ pub(super) struct TimelineSettings { /// 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))] @@ -155,7 +149,6 @@ 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() } } @@ -167,7 +160,6 @@ 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, } } } @@ -789,7 +781,6 @@ 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 4eb4f019f3e..dffda89f6ce 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -223,7 +223,6 @@ impl TimelineState { txn_id: OwnedTransactionId, send_handle: Option, content: TimelineEventKind, - settings: &TimelineSettings, ) { let ctx = TimelineEventContext { sender: own_user_id, @@ -241,7 +240,7 @@ impl TimelineState { let mut date_divider_adjuster = DateDividerAdjuster::new(date_divider_mode); - TimelineEventHandler::new(&mut txn, ctx, settings) + TimelineEventHandler::new(&mut txn, ctx) .handle_event(&mut date_divider_adjuster, content) .await; @@ -745,9 +744,7 @@ impl TimelineStateTransaction<'_> { }; // Handle the event to create or update a timeline item. - TimelineEventHandler::new(self, ctx, settings) - .handle_event(date_divider_adjuster, event_kind) - .await + TimelineEventHandler::new(self, ctx).handle_event(date_divider_adjuster, event_kind).await } /// Remove one timeline item by its `event_index`. @@ -846,39 +843,12 @@ impl TimelineStateTransaction<'_> { room_data_provider: &P, settings: &TimelineSettings, ) { - /// 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, - 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 { .. } => { - deduplicate(event_meta.event_id, &mut self.items, settings); - self.items.push_front_remote_event(event_meta.base_meta()) } TimelineItemPosition::End { .. } => { - deduplicate(event_meta.event_id, &mut self.items, settings); - self.items.push_back_remote_event(event_meta.base_meta()); } diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 482a8d3073e..a1262a78a2c 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -53,7 +53,7 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn}; use super::{ controller::{ ObservableItemsTransaction, ObservableItemsTransactionEntry, PendingEdit, PendingEditKind, - TimelineMetadata, TimelineSettings, TimelineStateTransaction, + TimelineMetadata, TimelineStateTransaction, }, date_dividers::DateDividerAdjuster, event_item::{ @@ -338,17 +338,15 @@ 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(), settings } + Self { items, meta, ctx, result: HandleEventResult::default() } } /// Handle an event. @@ -1115,8 +1113,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { &mut item, Some(event_id), txn_id.as_ref().map(AsRef::as_ref), - self.meta, - self.settings, ); let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item); @@ -1136,8 +1132,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { &mut item, Some(event_id), txn_id.as_ref().map(AsRef::as_ref), - self.meta, - self.settings, ); let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item); @@ -1191,8 +1185,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { &mut item, Some(event_id), txn_id.as_ref().map(AsRef::as_ref), - self.meta, - self.settings, ); let item = new_timeline_item(self.meta, item, removed_duplicated_timeline_item); @@ -1268,12 +1260,6 @@ 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 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. - // // Note: this method doesn't take `&mut self` to avoid a borrow checker // conflict with `TimelineEventHandler::add_item`. fn deduplicate_local_timeline_item( @@ -1281,8 +1267,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { new_event_timeline_item: &mut EventTimelineItem, event_id: Option<&EventId>, transaction_id: Option<&TransactionId>, - metadata: &TimelineMetadata, - settings: &TimelineSettings, ) -> Option> { // Start with the canonical case: detect a local timeline item that matches // `event_id` or `transaction_id`. @@ -1310,39 +1294,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { 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 } diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs b/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs index 992ba6e6808..9cd83213239 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs @@ -318,10 +318,6 @@ impl TimelineItemContent { as_variant!(self, Self::UnableToDecrypt) } - pub(crate) fn is_redacted(&self) -> bool { - matches!(self, Self::RedactedMessage) - } - // These constructors could also be `From` implementations, but that would // allow users to call them directly, which should not be supported pub(crate) fn message( diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index 175ffe46a83..e7322b215e9 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -26,7 +26,6 @@ use matrix_sdk::event_cache::{ use tracing::{instrument, trace, warn}; use super::Error; -use crate::timeline::{controller::TimelineNewItemPosition, event_item::RemoteEventOrigin}; impl super::Timeline { /// Add more events to the start of the timeline. @@ -77,17 +76,6 @@ impl super::Timeline { let num_events = events.len(); trace!("Back-pagination succeeded with {num_events} events"); - // If `TimelineSettings::vectordiffs_as_inputs` is enabled, - // we don't need to add events manually: everything we need - // is to let the `EventCache` receive the events from this - // pagination, and emit 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, // and we've not hit the start of the timeline, retry until we get diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index ba7abb9f7a3..bb0adbeba00 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -23,7 +23,7 @@ use ruma::{ receipt::{Receipt, ReceiptThread, ReceiptType}, room::{ member::{MembershipState, RedactedRoomMemberEventContent, RoomMemberEventContent}, - message::{MessageType, RoomMessageEventContent}, + message::MessageType, name::RoomNameEventContent, topic::RedactedRoomTopicEventContent, }, @@ -280,77 +280,6 @@ async fn test_other_state() { assert_matches!(full_content, FullStateEventContent::Redacted(_)); } -#[async_test] -async fn test_dedup_pagination() { - let timeline = TestTimeline::new(); - - let event = timeline - .event_builder - .make_sync_message_event(*ALICE, RoomMessageEventContent::text_plain("o/")); - timeline.handle_live_event(SyncTimelineEvent::new(event.clone())).await; - // This cast is not actually correct, sync events aren't valid - // back-paginated events, as they are missing `room_id`. However, the - // timeline doesn't care about that `room_id` and casts back to - // `Raw` before attempting to deserialize. - timeline.handle_back_paginated_event(event.cast()).await; - - let timeline_items = timeline.controller.items().await; - assert_eq!(timeline_items.len(), 2); - assert_matches!( - timeline_items[0].kind, - TimelineItemKind::Virtual(VirtualTimelineItem::DateDivider(_)) - ); - assert_matches!(timeline_items[1].kind, TimelineItemKind::Event(_)); -} - -#[async_test] -async fn test_dedup_initial() { - let timeline = TestTimeline::new(); - - let f = &timeline.factory; - let event_a = f.text_msg("A").sender(*ALICE).into_sync(); - let event_b = f.text_msg("B").sender(*BOB).into_sync(); - let event_c = f.text_msg("C").sender(*CAROL).into_sync(); - - timeline - .controller - .add_events_at( - [ - // two events - event_a.clone(), - event_b.clone(), - // same events got duplicated in next sync response - event_a, - event_b, - // … and a new event also came in - event_c, - ] - .into_iter(), - TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, - ) - .await; - - let timeline_items = timeline.controller.items().await; - assert_eq!(timeline_items.len(), 4); - - assert!(timeline_items[0].is_date_divider()); - - let event1 = &timeline_items[1]; - let event2 = &timeline_items[2]; - let event3 = &timeline_items[3]; - - // Make sure the order is right. - assert_eq!(event1.as_event().unwrap().sender(), *ALICE); - assert_eq!(event2.as_event().unwrap().sender(), *BOB); - assert_eq!(event3.as_event().unwrap().sender(), *CAROL); - - // Make sure we reused IDs when deduplicating events. - assert_eq!(event1.unique_id().0, "0"); - assert_eq!(event2.unique_id().0, "1"); - assert_eq!(event3.unique_id().0, "2"); - assert_eq!(timeline_items[0].unique_id().0, "3"); -} - #[async_test] async fn test_internal_id_prefix() { let timeline = TestTimeline::with_internal_id_prefix("le_prefix_".to_owned()); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/echo.rs b/crates/matrix-sdk-ui/src/timeline/tests/echo.rs index 683f643ec85..f5a85de4495 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/echo.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/echo.rs @@ -18,7 +18,7 @@ use assert_matches::assert_matches; use eyeball_im::VectorDiff; use matrix_sdk::{assert_next_matches_with_timeout, send_queue::RoomSendQueueUpdate}; use matrix_sdk_base::store::QueueWedgeError; -use matrix_sdk_test::{async_test, event_factory::EventFactory, ALICE, BOB}; +use matrix_sdk_test::{async_test, ALICE, BOB}; use ruma::{ event_id, events::{room::message::RoomMessageEventContent, AnyMessageLikeEventContent}, @@ -187,42 +187,6 @@ async fn test_remote_echo_new_position() { assert_pending!(stream); } -#[async_test] -async fn test_date_divider_duplication() { - let timeline = TestTimeline::new(); - - // Given two remote events from one day, and a local event from another day… - let f = EventFactory::new().sender(&BOB); - timeline.handle_live_event(f.text_msg("A")).await; - timeline.handle_live_event(f.text_msg("B")).await; - timeline - .handle_local_event(AnyMessageLikeEventContent::RoomMessage( - RoomMessageEventContent::text_plain("C"), - )) - .await; - - let items = timeline.controller.items().await; - assert_eq!(items.len(), 5); - assert!(items[0].is_date_divider()); - assert!(items[1].is_remote_event()); - assert!(items[2].is_remote_event()); - assert!(items[3].is_date_divider()); - assert!(items[4].is_local_echo()); - - // … when the second remote event is re-received (day still the same) - let event_id = items[2].as_event().unwrap().event_id().unwrap(); - timeline.handle_live_event(f.text_msg("B").event_id(event_id).server_ts(1)).await; - - // … it should not impact the date dividers. - let items = timeline.controller.items().await; - assert_eq!(items.len(), 5); - assert!(items[0].is_date_divider()); - assert!(items[1].is_remote_event()); - assert!(items[2].is_remote_event()); - assert!(items[3].is_date_divider()); - assert!(items[4].is_local_echo()); -} - #[async_test] async fn test_date_divider_removed_after_local_echo_disappeared() { let timeline = TestTimeline::new(); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs index 11fa3023721..196e61ed058 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs @@ -19,10 +19,7 @@ use matrix_sdk_base::deserialized_responses::SyncTimelineEvent; use matrix_sdk_test::{async_test, ALICE, BOB}; use ruma::events::{ reaction::RedactedReactionEventContent, - room::{ - message::{OriginalSyncRoomMessageEvent, RedactedRoomMessageEventContent}, - name::RoomNameEventContent, - }, + room::{message::OriginalSyncRoomMessageEvent, name::RoomNameEventContent}, FullStateEventContent, }; use stream_assert::assert_next_matches; @@ -177,56 +174,3 @@ async fn test_reaction_redaction_timeline_filter() { assert_eq!(item.reactions().len(), 0); assert_eq!(timeline.controller.items().await.len(), 2); } - -#[async_test] -async fn test_receive_unredacted() { - let timeline = TestTimeline::new(); - - let f = &timeline.factory; - - // send two events, second one redacted - timeline.handle_live_event(f.text_msg("about to be redacted").sender(&ALICE)).await; - timeline - .handle_live_redacted_message_event(&ALICE, RedactedRoomMessageEventContent::new()) - .await; - - // redact the first one as well - let items = timeline.controller.items().await; - assert!(items[0].is_date_divider()); - let fst = items[1].as_event().unwrap(); - timeline.handle_live_event(f.redaction(fst.event_id().unwrap()).sender(&ALICE)).await; - - let items = timeline.controller.items().await; - assert_eq!(items.len(), 3); - let fst = items[1].as_event().unwrap(); - let snd = items[2].as_event().unwrap(); - - // make sure we have two redacted events - assert!(fst.content.is_redacted()); - assert!(snd.content.is_redacted()); - - // send new events with the same event ID as the previous ones - timeline - .handle_live_event( - f.text_msg("unredacted #1") - .sender(*ALICE) - .event_id(fst.event_id().unwrap()) - .server_ts(fst.timestamp()), - ) - .await; - - timeline - .handle_live_event( - f.text_msg("unredacted #2") - .sender(*ALICE) - .event_id(snd.event_id().unwrap()) - .server_ts(snd.timestamp()), - ) - .await; - - // make sure we still have two redacted events - let items = timeline.controller.items().await; - assert_eq!(items.len(), 3); - assert!(items[1].as_event().unwrap().content.is_redacted()); - assert!(items[2].as_event().unwrap().content.is_redacted()); -} diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs index 3a2c0aeaa54..a796fc740da 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs @@ -37,7 +37,7 @@ use ruma::{ room_id, }; use serde_json::{json, Value as JsonValue}; -use stream_assert::{assert_next_eq, assert_next_matches}; +use stream_assert::{assert_next_eq, assert_pending}; use tokio::{ spawn, time::{sleep, timeout}, @@ -87,42 +87,52 @@ async fn test_back_pagination() { }; join(paginate, observe_paginating).await; - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "hello world"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "the world is big"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); - assert_eq!(state.state_key(), ""); - assert_let!( - AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { - content, - prev_content - }) = state.content() - ); - assert_eq!(content.name, "New room name"); - assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + // `m.room.name` + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); + assert_eq!(state.state_key(), ""); + assert_let!( + AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { + content, + prev_content + }) = state.content() + ); + assert_eq!(content.name, "New room name"); + assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + } + + // `m.room.name` receives an update + { + assert_let!(Some(VectorDiff::Set { index, .. }) = timeline_stream.next().await); + assert_eq!(index, 0); + } + + // `m.room.message`: “the world is big” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "the world is big"); + } + + // `m.room.message`: “hello world” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "hello world"); + } + + // Date divider is updated. + { + assert_let!( + Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next().await + ); + assert!(date_divider.is_date_divider()); + } - let date_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert!(date_divider.is_date_divider()); + assert_pending!(timeline_stream); Mock::given(method("GET")) .and(path_regex(r"^/_matrix/client/r0/rooms/.*/messages$")) @@ -144,6 +154,8 @@ async fn test_back_pagination() { back_pagination_status, LiveBackPaginationStatus::Idle { hit_start_of_timeline: true } ); + + assert_pending!(timeline_stream); } #[async_test] @@ -212,27 +224,31 @@ async fn test_back_pagination_highlighted() { timeline.live_paginate_backwards(10).await.unwrap(); server.reset().await; - let first = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - let remote_event = first.as_event().unwrap(); - // Own events don't trigger push rules. - assert!(!remote_event.is_highlighted()); - - let second = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - let remote_event = second.as_event().unwrap(); - // `m.room.tombstone` should be highlighted by default. - assert!(remote_event.is_highlighted()); + // `m.room.tombstone` + { + assert_let!(Some(VectorDiff::PushBack { value: second }) = timeline_stream.next().await); + let remote_event = second.as_event().unwrap(); + // `m.room.tombstone` should be highlighted by default. + assert!(remote_event.is_highlighted()); + } + + // `m.room.message` + { + assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await); + let remote_event = first.as_event().unwrap(); + // Own events don't trigger push rules. + assert!(!remote_event.is_highlighted()); + } + + // Date divider + { + assert_let!( + Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next().await + ); + assert!(date_divider.is_date_divider()); + } - let date_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert!(date_divider.is_date_divider()); + assert_pending!(timeline_stream); } #[async_test] @@ -615,42 +631,52 @@ async fn test_empty_chunk() { }; join(paginate, observe_paginating).await; - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "hello world"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "the world is big"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); - assert_eq!(state.state_key(), ""); - assert_let!( - AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { - content, - prev_content - }) = state.content() - ); - assert_eq!(content.name, "New room name"); - assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + // `m.room.name` + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); + assert_eq!(state.state_key(), ""); + assert_let!( + AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { + content, + prev_content + }) = state.content() + ); + assert_eq!(content.name, "New room name"); + assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + } + + // `m.room.name` is updated + { + assert_let!(Some(VectorDiff::Set { index, .. }) = timeline_stream.next().await); + assert_eq!(index, 0); + } + + // `m.room.message`: “the world is big” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "the world is big"); + } + + // `m.room.name`: “hello world” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "hello world"); + } + + // Date divider + { + assert_let!( + Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next().await + ); + assert!(date_divider.is_date_divider()); + } - let date_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert!(date_divider.is_date_divider()); + assert_pending!(timeline_stream); } #[async_test] @@ -715,59 +741,65 @@ async fn test_until_num_items_with_empty_chunk() { }; join(paginate, observe_paginating).await; - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "hello world"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "the world is big"); - - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); - assert_eq!(state.state_key(), ""); - assert_let!( - AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { - content, - prev_content - }) = state.content() - ); - assert_eq!(content.name, "New room name"); - assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); - - let date_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert!(date_divider.is_date_divider()); + // `m.room.name` + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); + assert_eq!(state.state_key(), ""); + assert_let!( + AnyOtherFullStateEventContent::RoomName(FullStateEventContent::Original { + content, + prev_content + }) = state.content() + ); + assert_eq!(content.name, "New room name"); + assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + } + + // `m.room.name` is updated + { + assert_let!(Some(VectorDiff::Set { index, .. }) = timeline_stream.next().await); + assert_eq!(index, 0); + } + + // `m.room.message`: “the world is big” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "the world is big"); + } + + // `m.room.name`: “hello world” + { + assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "hello world"); + } + + // Date divider + { + assert_let!( + Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next().await + ); + assert!(date_divider.is_date_divider()); + } timeline.live_paginate_backwards(10).await.unwrap(); - let message = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); - assert_let!(MessageType::Text(text) = msg.msgtype()); - assert_eq!(text.body, "hello room then"); + // `m.room.name`: “hello room then” + { + assert_let!( + Some(VectorDiff::Insert { index, value: message }) = timeline_stream.next().await + ); + assert_eq!(index, 1); + assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); + assert_let!(MessageType::Text(text) = msg.msgtype()); + assert_eq!(text.body, "hello room then"); + } - let date_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert!(date_divider.is_date_divider()); - assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + assert_pending!(timeline_stream); } #[async_test] diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs index ae194ceecaf..9a1301acd90 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -409,9 +409,8 @@ async fn test_timeline_duplicated_events() -> Result<()> { assert_timeline_stream! { [timeline_stream] - update[3] "$x3:bar.org"; - update[1] "$x1:bar.org"; remove[1]; + update[2] "$x3:bar.org"; append "$x1:bar.org"; update[3] "$x1:bar.org"; append "$x4:bar.org"; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs b/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs index 249037faa1b..61b0c3efe44 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs @@ -289,6 +289,8 @@ async fn test_timeline_is_reset_when_a_user_is_ignored_or_unignored() { server.reset().await; // Timeline receives events as before. + assert_next_matches!(timeline_stream, VectorDiff::Clear); // TODO: Remove `RoomEventCacheUpdate::Clear` as it creates double + // `VectorDiff::Clear`. assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { assert_eq!(value.as_event().unwrap().event_id(), Some(fourth_event_id)); }); diff --git a/crates/matrix-sdk/src/event_cache/mod.rs b/crates/matrix-sdk/src/event_cache/mod.rs index 0b5b2b0e346..6fbbbb7deaf 100644 --- a/crates/matrix-sdk/src/event_cache/mod.rs +++ b/crates/matrix-sdk/src/event_cache/mod.rs @@ -662,16 +662,6 @@ pub enum RoomEventCacheUpdate { ambiguity_changes: BTreeMap, }, - /// 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, - - /// Where the events are coming from. - origin: EventsOrigin, - }, - /// The room has received updates for the timeline as _diffs_. UpdateTimelineEvents { /// Diffs to apply to the timeline. diff --git a/crates/matrix-sdk/src/event_cache/pagination.rs b/crates/matrix-sdk/src/event_cache/pagination.rs index 93a87bc730a..4d135666649 100644 --- a/crates/matrix-sdk/src/event_cache/pagination.rs +++ b/crates/matrix-sdk/src/event_cache/pagination.rs @@ -165,9 +165,10 @@ impl RoomPagination { None }; + // The new prev token from this pagination. let new_gap = paginator.prev_batch_token().map(|prev_token| Gap { prev_token }); - let (backpagination_outcome, updates_as_vector_diffs) = state + let (backpagination_outcome, sync_timeline_events_diffs) = state .with_events_mut(move |room_events| { // Note: The chunk could be empty. // @@ -231,9 +232,9 @@ impl RoomPagination { }) .await?; - if !updates_as_vector_diffs.is_empty() { + if !sync_timeline_events_diffs.is_empty() { let _ = self.inner.sender.send(RoomEventCacheUpdate::UpdateTimelineEvents { - diffs: updates_as_vector_diffs, + diffs: sync_timeline_events_diffs, origin: EventsOrigin::Pagination, }); } diff --git a/crates/matrix-sdk/src/event_cache/room/events.rs b/crates/matrix-sdk/src/event_cache/room/events.rs index 12fdf70f96b..516432304a6 100644 --- a/crates/matrix-sdk/src/event_cache/room/events.rs +++ b/crates/matrix-sdk/src/event_cache/room/events.rs @@ -15,13 +15,11 @@ use std::cmp::Ordering; use eyeball_im::VectorDiff; +use matrix_sdk_base::event_cache::store::DEFAULT_CHUNK_CAPACITY; pub use matrix_sdk_base::event_cache::{Event, Gap}; -use matrix_sdk_base::{ - event_cache::store::DEFAULT_CHUNK_CAPACITY, - linked_chunk::{AsVector, IterBackward, ObservableUpdates}, -}; use matrix_sdk_common::linked_chunk::{ - Chunk, ChunkIdentifier, EmptyChunk, Error, LinkedChunk, Position, + AsVector, Chunk, ChunkIdentifier, EmptyChunk, Error, Iter, IterBackward, LinkedChunk, + ObservableUpdates, Position, }; use ruma::OwnedEventId; use tracing::{debug, error, warn}; @@ -211,9 +209,7 @@ impl RoomEvents { /// Iterate over the chunks, forward. /// /// The oldest chunk comes first. - pub fn chunks( - &self, - ) -> matrix_sdk_common::linked_chunk::Iter<'_, DEFAULT_CHUNK_CAPACITY, Event, Gap> { + pub fn chunks(&self) -> Iter<'_, DEFAULT_CHUNK_CAPACITY, Event, Gap> { self.chunks.chunks() } diff --git a/crates/matrix-sdk/src/event_cache/room/mod.rs b/crates/matrix-sdk/src/event_cache/room/mod.rs index 0ef69e36096..7ad4a6afe2b 100644 --- a/crates/matrix-sdk/src/event_cache/room/mod.rs +++ b/crates/matrix-sdk/src/event_cache/room/mod.rs @@ -450,13 +450,12 @@ impl RoomEventCacheInner { let mut all_events = self.all_events.write().await; - for sync_timeline_event in &sync_timeline_events { + for sync_timeline_event in sync_timeline_events { if let Some(event_id) = sync_timeline_event.event_id() { - all_events.append_related_event(sync_timeline_event); - all_events.events.insert( - event_id.to_owned(), - (self.room_id.clone(), sync_timeline_event.clone()), - ); + all_events.append_related_event(&sync_timeline_event); + all_events + .events + .insert(event_id.to_owned(), (self.room_id.clone(), sync_timeline_event)); } } @@ -471,14 +470,6 @@ 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, - origin: EventsOrigin::Sync, - }); - } - if !sync_timeline_events_diffs.is_empty() { let _ = self.sender.send(RoomEventCacheUpdate::UpdateTimelineEvents { diffs: sync_timeline_events_diffs, diff --git a/crates/matrix-sdk/tests/integration/event_cache.rs b/crates/matrix-sdk/tests/integration/event_cache.rs index 5f9f76782d9..08189f34404 100644 --- a/crates/matrix-sdk/tests/integration/event_cache.rs +++ b/crates/matrix-sdk/tests/integration/event_cache.rs @@ -1,8 +1,12 @@ -use std::{future::ready, ops::ControlFlow, time::Duration}; +use std::{ + future::ready, + ops::{ControlFlow, Not}, + time::Duration, +}; use assert_matches::assert_matches; use assert_matches2::assert_let; -use futures_util::FutureExt as _; +use eyeball_im::VectorDiff; use matrix_sdk::{ assert_let_timeout, assert_next_matches_with_timeout, deserialized_responses::SyncTimelineEvent, @@ -82,10 +86,10 @@ async fn test_event_cache_receives_events() { // It does receive one update, assert_let_timeout!( - Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() ); - // It does also receive the update as `VectorDiff`. - assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); + assert_eq!(diffs.len(), 1); + assert_let!(VectorDiff::Append { values: events } = &diffs[0]); // Which contains the event that was sent beforehand. assert_eq!(events.len(), 1); @@ -170,10 +174,13 @@ async fn test_ignored_unignored() { // We do receive one update, assert_let_timeout!( - Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() ); - // It does also receive the update as `VectorDiff`. - assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); + assert_eq!(diffs.len(), 2); + + // Similar to the `RoomEventCacheUpdate::Clear`. + assert_let!(VectorDiff::Clear = &diffs[0]); + assert_let!(VectorDiff::Append { values: events } = &diffs[1]); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "i don't like this dexter"); @@ -195,14 +202,14 @@ async fn wait_for_initial_events( room_stream: &mut broadcast::Receiver, ) { if events.is_empty() { - let mut update = room_stream.recv().await.expect("read error"); + assert_let_timeout!(Ok(update) = room_stream.recv()); + let mut update = update; + // Could be a clear because of the limited timeline. if matches!(update, RoomEventCacheUpdate::Clear) { - update = room_stream.recv().await.expect("read error"); + assert_let_timeout!(Ok(new_update) = room_stream.recv()); + update = new_update; } - assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { .. }); - - let update = room_stream.recv().await.expect("read error"); assert_matches!(update, RoomEventCacheUpdate::UpdateTimelineEvents { .. }); } else { @@ -277,15 +284,26 @@ async fn test_backpaginate_once() { let BackPaginationOutcome { events, reached_start } = outcome; assert!(reached_start); + assert_eq!(events.len(), 2); assert_event_matches_msg(&events[0], "world"); 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 { .. }))); + // And we get update as diffs. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + + assert_eq!(diffs.len(), 2); + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "hello"); + }); + assert_matches!(&diffs[1], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 1); + assert_event_matches_msg(event, "world"); + }); - let next = room_stream.recv().now_or_never(); - assert_matches!(next, None); + assert!(room_stream.is_empty()); } #[async_test] @@ -394,8 +412,36 @@ async fn test_backpaginate_many_times_with_many_iterations() { assert_event_matches_msg(&global_events[2], "oh well"); assert_eq!(global_events.len(), 3); + // First iteration. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + + assert_eq!(diffs.len(), 2); + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "hello"); + }); + assert_matches!(&diffs[1], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 1); + assert_event_matches_msg(event, "world"); + }); + + // Second iteration. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + + assert_eq!(diffs.len(), 1); + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "oh well"); + }); + + assert!(room_stream.is_empty()); + // And next time I'll open the room, I'll get the events in the right order. - let (events, _receiver) = room_event_cache.subscribe().await.unwrap(); + let (events, room_stream) = room_event_cache.subscribe().await.unwrap(); assert_event_matches_msg(&events[0], "oh well"); assert_event_matches_msg(&events[1], "hello"); @@ -403,14 +449,6 @@ 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()); } @@ -525,8 +563,34 @@ async fn test_backpaginate_many_times_with_one_iteration() { assert_event_matches_msg(&global_events[2], "oh well"); assert_eq!(global_events.len(), 3); + // First pagination. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + + assert_eq!(diffs.len(), 2); + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "hello"); + }); + assert_matches!(&diffs[1], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 1); + assert_event_matches_msg(event, "world"); + }); + + // Second pagination. + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + + assert_eq!(diffs.len(), 1); + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "oh well"); + }); + // And next time I'll open the room, I'll get the events in the right order. - let (events, _receiver) = room_event_cache.subscribe().await.unwrap(); + let (events, room_stream) = room_event_cache.subscribe().await.unwrap(); assert_event_matches_msg(&events[0], "oh well"); assert_event_matches_msg(&events[1], "hello"); @@ -534,14 +598,6 @@ 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()); } @@ -582,7 +638,10 @@ async fn test_reset_while_backpaginating() { // cache (and no room updates will happen in this case), or it hasn't, and // the stream will return the next message soon. if events.is_empty() { - let _ = room_stream.recv().await.expect("read error"); + assert_let_timeout!(Ok(RoomEventCacheUpdate::Clear) = room_stream.recv()); + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = room_stream.recv() + ); } else { assert_eq!(events.len(), 1); } @@ -679,6 +738,35 @@ async fn test_reset_while_backpaginating() { ); assert!(first_token != second_token); assert_eq!(second_token, "third_backpagination"); + + // Assert the updates as diffs. + + // Being cleared from the reset. + assert_let_timeout!(Ok(RoomEventCacheUpdate::Clear) = room_stream.recv()); + + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + assert_eq!(diffs.len(), 2); + // The clear, again. + assert_matches!(&diffs[0], VectorDiff::Clear); + // The event from the sync. + assert_matches!(&diffs[1], VectorDiff::Append { values: events } => { + assert_eq!(events.len(), 1); + assert_event_matches_msg(&events[0], "heyo"); + }); + + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + assert_eq!(diffs.len(), 1); + // The event from the pagination. + assert_matches!(&diffs[0], VectorDiff::Insert { index, value: event } => { + assert_eq!(*index, 0); + assert_event_matches_msg(event, "finally!"); + }); + + assert!(room_stream.is_empty()); } #[async_test] @@ -731,8 +819,14 @@ 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_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + assert_eq!(diffs.len(), 1); + assert_matches!(&diffs[0], VectorDiff::Append { values: events } => { + assert_eq!(events.len(), 1); + assert_event_matches_msg(&events[0], "hi"); + }); assert!(room_stream.is_empty()); } @@ -785,6 +879,15 @@ async fn test_limited_timeline_resets_pagination() { assert_eq!(events.len(), 1); assert!(reached_start); + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = room_stream.recv() + ); + assert_eq!(diffs.len(), 1); + assert_matches!(&diffs[0], VectorDiff::Append { values: events } => { + assert_eq!(events.len(), 1); + assert_event_matches_msg(&events[0], "hi"); + }); + // And the paginator state delives this as an update, and is internally // consistent with it: assert_next_matches_with_timeout!(pagination_status, PaginatorState::Idle); @@ -794,7 +897,6 @@ 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 @@ -839,12 +941,11 @@ async fn test_limited_timeline_with_storage() { // This is racy: either the sync has been handled, or it hasn't yet. if initial_events.is_empty() { 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() + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() ); + assert_eq!(diffs.len(), 1); + + assert_let!(VectorDiff::Append { values: events } = &diffs[0]); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "hey yo"); } else { @@ -865,10 +966,11 @@ async fn test_limited_timeline_with_storage() { .await; assert_let_timeout!( - Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv() + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() ); - // It does also receive the update as `VectorDiff`. - assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv()); + assert_eq!(diffs.len(), 1); + + assert_let!(VectorDiff::Append { values: events } = &diffs[0]); assert_eq!(events.len(), 1); assert_event_matches_msg(&events[0], "gappy!"); @@ -1092,13 +1194,11 @@ async fn test_no_gap_stored_after_deduplicated_sync() { let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap(); let (events, mut stream) = room_event_cache.subscribe().await.unwrap(); + if events.is_empty() { - let update = stream.recv().await.expect("read error"); - assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { .. }); - // It does also receive the update as `VectorDiff`. - let update = stream.recv().await.expect("read error"); - assert_matches!(update, RoomEventCacheUpdate::UpdateTimelineEvents { .. }); + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = stream.recv()); } + drop(events); // Backpagination will return nothing. @@ -1126,15 +1226,14 @@ async fn test_no_gap_stored_after_deduplicated_sync() { ) .await; - let update = stream.recv().await.expect("read error"); - assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { .. }); + assert!(stream.is_empty()); // If this back-pagination fails, that's because we've stored a gap that's // useless. It should be short-circuited because there's no previous gap. let outcome = pagination.run_backwards(20, once).await.unwrap(); assert!(outcome.reached_start); - let (events, _stream) = room_event_cache.subscribe().await.unwrap(); + let (events, stream) = room_event_cache.subscribe().await.unwrap(); assert_event_matches_msg(&events[0], "hello"); assert_event_matches_msg(&events[1], "world"); assert_event_matches_msg(&events[2], "sup"); @@ -1172,13 +1271,11 @@ async fn test_no_gap_stored_after_deduplicated_backpagination() { let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap(); let (events, mut stream) = room_event_cache.subscribe().await.unwrap(); + if events.is_empty() { - let update = stream.recv().await.expect("read error"); - assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { .. }); - // It does also receive the update as `VectorDiff`. - let update = stream.recv().await.expect("read error"); - assert_matches!(update, RoomEventCacheUpdate::UpdateTimelineEvents { .. }); + assert_let_timeout!(Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = stream.recv()); } + drop(events); // Now, simulate that we expanded the timeline window with sliding sync, by @@ -1197,6 +1294,25 @@ async fn test_no_gap_stored_after_deduplicated_backpagination() { ) .await; + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = stream.recv() + ); + assert_eq!(diffs.len(), 2); + + // `$ev3` is duplicated, the older `$ev3` event is removed + assert_matches!(&diffs[0], VectorDiff::Remove { index } => { + assert_eq!(*index, 0); + }); + // `$ev1`, `$ev2` and `$ev3` are added. + assert_matches!(&diffs[1], VectorDiff::Append { values: events } => { + assert_eq!(events.len(), 3); + assert_eq!(events[0].event_id().unwrap().as_str(), "$1"); + assert_eq!(events[1].event_id().unwrap().as_str(), "$2"); + assert_eq!(events[2].event_id().unwrap().as_str(), "$3"); + }); + + assert!(stream.is_empty()); + // For prev-batch2, the back-pagination returns nothing. server .mock_room_messages() @@ -1229,21 +1345,36 @@ async fn test_no_gap_stored_after_deduplicated_backpagination() { // Run pagination once: it will consume prev-batch2 first, which is the most // recent token. - pagination.run_backwards(20, once).await.unwrap(); + let outcome = pagination.run_backwards(20, once).await.unwrap(); + + // The pagination is empty: no new event. + assert!(outcome.reached_start); + assert!(outcome.events.is_empty()); + assert!(stream.is_empty()); // Run pagination a second time: it will consume prev-batch, which is the least // recent token. - pagination.run_backwards(20, once).await.unwrap(); + let outcome = pagination.run_backwards(20, once).await.unwrap(); + + // The pagination contains events, but they are all duplicated; the gap is + // replaced by zero event: nothing happens. + assert!(outcome.reached_start.not()); + assert_eq!(outcome.events.len(), 2); + assert!(stream.is_empty()); // If this back-pagination fails, that's because we've stored a gap that's // useless. It should be short-circuited because storing the previous gap was // useless. let outcome = pagination.run_backwards(20, once).await.unwrap(); assert!(outcome.reached_start); + assert!(outcome.events.is_empty()); + assert!(stream.is_empty()); - let (events, _stream) = room_event_cache.subscribe().await.unwrap(); + let (events, stream) = room_event_cache.subscribe().await.unwrap(); assert_event_matches_msg(&events[0], "hello"); assert_event_matches_msg(&events[1], "world"); assert_event_matches_msg(&events[2], "sup"); assert_eq!(events.len(), 3); + + assert!(stream.is_empty()); } diff --git a/crates/matrix-sdk/tests/integration/room_preview.rs b/crates/matrix-sdk/tests/integration/room_preview.rs index 85e887e2e26..71d2dcb8d7a 100644 --- a/crates/matrix-sdk/tests/integration/room_preview.rs +++ b/crates/matrix-sdk/tests/integration/room_preview.rs @@ -8,10 +8,8 @@ use matrix_sdk_test::{ async_test, InvitedRoomBuilder, JoinedRoomBuilder, KnockedRoomBuilder, SyncResponseBuilder, }; #[cfg(feature = "experimental-sliding-sync")] -use ruma::{api::client::sync::sync_events::v5::response::Hero, assign}; -use ruma::{ - events::room::member::MembershipState, owned_user_id, room_id, space::SpaceRoomJoinRule, RoomId, -}; +use ruma::{api::client::sync::sync_events::v5::response::Hero, assign, owned_user_id}; +use ruma::{events::room::member::MembershipState, room_id, space::SpaceRoomJoinRule, RoomId}; use serde_json::json; use wiremock::{ matchers::{header, method, path_regex}, diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index 04d4a9774e1..f845287f6b9 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -58,7 +58,7 @@ use tokio::{ sync::Mutex, time::{sleep, timeout}, }; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, info, trace, warn}; use wiremock::{matchers::AnyMatcher, Mock, MockServer}; use crate::helpers::{wait_for_room, TestClientBuilder}; @@ -846,7 +846,7 @@ async fn test_delayed_invite_response_and_sent_message_decryption() -> Result<() let bob_sync_service = SyncService::builder(bob.clone()).build().await.unwrap(); bob_sync_service.start().await; - // alice creates a room and invites bob. + // Alice creates a room and will invite Bob. let alice_room = alice .create_room(assign!(CreateRoomRequest::new(), { invite: vec![], @@ -887,30 +887,38 @@ async fn test_delayed_invite_response_and_sent_message_decryption() -> Result<() assert_eq!(bob_room.state(), RoomState::Joined); assert!(bob_room.is_encrypted().await.unwrap()); - let bob_timeline = bob_room.timeline_builder().build().await?; + let bob_timeline = bob_room.timeline().await?; let (_, timeline_stream) = bob_timeline.subscribe().await; pin_mut!(timeline_stream); - // Get previous events, including the sent message + // Get previous events, including the sent messages bob_timeline.paginate_backwards(3).await?; // Look for the sent message, which should not be an UTD event loop { - let diff = timeout(Duration::from_millis(100), timeline_stream.next()) + let diff = timeout(Duration::from_millis(300), timeline_stream.next()) .await - .expect("Timed out. Neither an UTD nor the sent message were found") + .expect("Failed to receive the decrypted sent message") .unwrap(); - if let VectorDiff::PushFront { value } = diff { - if let Some(content) = value.as_event().map(|e| e.content()) { - if let Some(message) = content.as_message() { - if message.body() == "hello world" { - return Ok(()); + + trace!(?diff, "Received diff from Bob's room"); + + match diff { + VectorDiff::PushBack { value: event } + | VectorDiff::Insert { value: event, .. } + | VectorDiff::Set { value: event, .. } => { + if let Some(content) = event.as_event().map(|e| e.content()) { + if let Some(message) = content.as_message() { + if message.body() == "hello world" { + return Ok(()); + } + + panic!("Unexpected message event found"); } - panic!("Unexpected message event found"); - } else if content.as_unable_to_decrypt().is_some() { - panic!("UTD found!") } } + + _ => {} } } } diff --git a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs index 70d11ddc8af..40dbe1b8ed5 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs @@ -442,6 +442,11 @@ async fn test_enabling_backups_retries_decryption() { .await .expect("We should be able to paginate the timeline to fetch the history"); + // Wait for the event cache and the timeline to do their job. + // Timeline triggers a pagination, that inserts events in the event cache, that + // then broadcasts new events into the timeline. All this is async. + sleep(Duration::from_millis(300)).await; + let item = timeline.item_by_event_id(&event_id).await.expect("The event should be in the timeline"); @@ -622,6 +627,11 @@ async fn test_room_keys_received_on_notification_client_trigger_redecryption() { .await .expect("We should be able to paginate the timeline to fetch the history"); + // Wait for the event cache and the timeline to do their job. + // Timeline triggers a pagination, that inserts events in the event cache, that + // then broadcasts new events into the timeline. All this is async. + sleep(Duration::from_millis(300)).await; + if let Some(timeline_item) = timeline.item_by_event_id(&event_id).await { item = Some(timeline_item); break;