From 88cd2557f36610935b76bdcf52c42dd965d3f335 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 18 Mar 2024 18:27:40 +0100 Subject: [PATCH] timeline: rework the day divider separation as a post-processing algorithm --- .../src/timeline/event_handler.rs | 312 ++++++++++++------ .../matrix-sdk-ui/src/timeline/inner/state.rs | 15 +- crates/matrix-sdk-ui/src/timeline/item.rs | 4 +- .../matrix-sdk-ui/src/timeline/pagination.rs | 6 +- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 29 +- .../matrix-sdk-ui/src/timeline/tests/echo.rs | 43 ++- .../matrix-sdk-ui/src/timeline/tests/edit.rs | 13 +- .../src/timeline/tests/encryption.rs | 8 +- .../src/timeline/tests/event_filter.rs | 18 +- .../src/timeline/tests/reactions.rs | 6 +- .../src/timeline/tests/read_receipts.rs | 36 +- .../matrix-sdk-ui/src/timeline/tests/virt.rs | 33 +- crates/matrix-sdk-ui/src/timeline/util.rs | 2 +- .../tests/integration/timeline/echo.rs | 42 +-- .../tests/integration/timeline/edit.rs | 8 +- .../tests/integration/timeline/mod.rs | 31 +- .../tests/integration/timeline/pagination.rs | 112 ++++--- .../integration/timeline/read_receipts.rs | 16 +- .../tests/integration/timeline/replies.rs | 8 +- .../integration/timeline/sliding_sync.rs | 30 +- .../tests/integration/timeline/subscribe.rs | 30 +- 21 files changed, 517 insertions(+), 285 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index d721faf80..585c51a95 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -57,7 +57,8 @@ use super::{ polls::PollState, util::{rfind_event_by_id, rfind_event_item, timestamp_to_date}, EventTimelineItem, InReplyToDetails, Message, OtherState, ReactionGroup, ReactionSenderData, - Sticker, TimelineDetails, TimelineItem, TimelineItemContent, VirtualTimelineItem, + Sticker, TimelineDetails, TimelineItem, TimelineItemContent, TimelineItemKind, + VirtualTimelineItem, }; use crate::{events::SyncTimelineEventWithoutContent, DEFAULT_SANITIZER_MODE}; @@ -395,6 +396,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // wouldn't normally be visible. Remove it. trace!("Removing UTD that was successfully retried"); self.items.remove(idx); + self.maybe_adjust_date_dividers(); // TODO add test? + self.result.item_removed = true; } @@ -856,25 +859,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { Flow::Local { .. } => { trace!("Adding new local timeline item"); - // Check if the latest event has the same date as this event. - if let Some(latest_event) = self.items.iter().rev().find_map(|item| item.as_event()) - { - let old_ts = latest_event.timestamp(); - - if let Some(day_divider_item) = - self.meta.maybe_create_day_divider_from_timestamps(old_ts, timestamp) - { - trace!("Adding day divider (local)"); - self.items.push_back(day_divider_item); - } - } else { - // If there is no event item, there is no day divider yet. - trace!("Adding first day divider (local)"); - let day_divider = - self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)); - self.items.push_back(day_divider); - } - let item = self.meta.new_timeline_item(item); self.items.push_back(item); } @@ -892,27 +876,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { trace!("Adding new remote timeline item at the start"); - // Check if the earliest day divider has the same date as this event. - if let Some(VirtualTimelineItem::DayDivider(divider_ts)) = - self.items.front().and_then(|item| item.as_virtual()) - { - let divider_ts = *divider_ts; - if let Some(day_divider_item) = - self.meta.maybe_create_day_divider_from_timestamps(divider_ts, timestamp) - { - trace!("Adding day divider (remote - start)"); - self.items.push_front(day_divider_item); - } - } else { - // The list must always start with a day divider. - trace!("Adding first day divider (remote - start)"); - let day_divider = - self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)); - self.items.push_front(day_divider); - } - let item = self.meta.new_timeline_item(item); - self.items.insert(1, item); + self.items.insert(0, item); } Flow::Remote { @@ -927,7 +892,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { }); let mut removed_event_item_id = None; - let mut removed_day_divider_id = None; if let Some((idx, old_item)) = result { if old_item.as_remote().is_some() { @@ -954,19 +918,17 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { let old_item_id = old_item.internal_id; - if idx == self.items.len() - 1 - && timestamp_to_date(old_item.timestamp()) == timestamp_to_date(timestamp) - { + if idx == self.items.len() - 1 { // If the old item is the last one and no day divider // changes need to happen, replace and return early. trace!(idx, "Replacing existing event"); self.items.set(idx, TimelineItem::new(item, old_item_id)); + self.maybe_adjust_date_dividers(); return; } - // In more complex cases, remove the item and day - // divider (if necessary) before re-adding the item. + // 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); @@ -976,20 +938,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { the first event item is preceded by a day divider" ); - // Pre-requisites for removing the day divider: - // 1. there is one preceding the old item at all - if self.items[idx - 1].is_day_divider() - // 2. the item after the old one that was removed is virtual (it should be - // impossible for this to be a read marker) - && self - .items - .get(idx) - .map_or(true, |item| item.is_virtual()) - { - trace!("Removing day divider"); - removed_day_divider_id = Some(self.items.remove(idx - 1).internal_id); - } - // no return here, below code for adding a new event // will run to re-add the removed item } else if txn_id.is_some() { @@ -1001,7 +949,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // Local echoes that are pending should stick to the bottom, // find the latest event that isn't that. - let (latest_event_idx, latest_event) = self + let latest_event_idx = self .items .iter() .enumerate() @@ -1013,52 +961,16 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { Some(EventSendState::NotSentYet | EventSendState::Sent { .. }) ) }) - .unzip(); + .unzip() + .0; // Insert the next item after the latest event item that's not a // pending local echo, or at the start if there is no such item. - let mut insert_idx = latest_event_idx.map_or(0, |idx| idx + 1); + let insert_idx = latest_event_idx.map_or(0, |idx| idx + 1); // Keep push semantics, if we're inserting at the end. let should_push = insert_idx == self.items.len(); - if let Some(latest_event) = latest_event { - // Check if that event has the same date as the new one. - let old_ts = latest_event.timestamp(); - - if timestamp_to_date(old_ts) != timestamp_to_date(timestamp) { - trace!("Adding day divider (remote - end)"); - - let id = match removed_day_divider_id { - // If a day divider was removed for an item about to be moved and we - // now need to add a new one, reuse the previous one's ID. - Some(day_divider_id) => day_divider_id, - None => self.meta.next_internal_id(), - }; - - let day_divider_item = - TimelineItem::new(VirtualTimelineItem::DayDivider(timestamp), id); - - if should_push { - self.items.push_back(day_divider_item); - } else { - self.items.insert(insert_idx, day_divider_item); - insert_idx += 1; - } - } - } else { - // If there is no event item, there is no day divider yet. - trace!("Adding first day divider (remote - end)"); - let new_day_divider = - self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)); - if should_push { - self.items.push_back(new_day_divider); - } else { - self.items.insert(insert_idx, new_day_divider); - insert_idx += 1; - } - } - let id = 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 @@ -1084,12 +996,212 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } + // TODO: it may be a bit expensive to run it on each add, try to group. + // TODO: move it to `handle_event`? + self.maybe_adjust_date_dividers(); + // If we don't have a read marker item, look if we need to add one now. if !self.meta.has_up_to_date_read_marker_item { self.meta.update_read_marker(self.items); } } + /// Ensures that date separators are properly inserted/removed when needs + /// be. + fn maybe_adjust_date_dividers(&mut self) { + // We're going to record operations like inserting, replacing and removing day + // dividers. Since we may remove or insert new items, recorded offsets + // will need to be updated, and the list of items must be iterated in + // non-decreasing order. + + #[derive(Debug)] + enum Operation { + Insert(usize, MilliSecondsSinceUnixEpoch), + Replace(usize, MilliSecondsSinceUnixEpoch), + Remove(usize), + } + + let mut ops = vec![]; + + let mut prev_item: Option<(&Arc, MilliSecondsSinceUnixEpoch)> = None; + let mut last_event_ts = None; + + for (i, item) in self.items.iter().enumerate() { + if let TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(ts)) = item.kind() { + // It's a day divider. + + if let Some((prev_item, prev_ts)) = prev_item { + if prev_item.is_day_divider() { + trace!("removing duplicate day divider @ {i}"); + // This day divider is preceded by another one: remove the current one. + ops.push(Operation::Remove(i)); + } else if prev_item.is_event() { + // This day divider is preceded by an event. + if timestamp_to_date(prev_ts) == timestamp_to_date(*ts) { + // The event has the same date as the day divider: remove the day + // divider. + trace!( + "removing day divider following event with same timestamp @ {i}" + ); + ops.push(Operation::Remove(i)); + } + } + } else { + // No interesting item prior to the day divider: it must be + // the first one, nothing to do. + } + + prev_item = Some((item, *ts)); + } + + if let Some(event) = item.as_event() { + // It's an event. + let ts = event.timestamp(); + + if let Some((prev_item, prev_ts)) = prev_item { + if prev_item.is_day_divider() { + // The event is preceded by a day divider. + if timestamp_to_date(prev_ts) != timestamp_to_date(ts) { + // The day divider is wrong. Should we replace it with the correct + // value, or remove it entirely? + // + // Look at the previous event timestamp: if they became the same value, + // remove the divider entirely. Otherwise, they show different values, + // so keep the day divider but adjust its value. + if last_event_ts.map_or( + false, + |last_event_ts: MilliSecondsSinceUnixEpoch| { + timestamp_to_date(last_event_ts) == timestamp_to_date(ts) + }, + ) { + // Remove + trace!("removed day divider @ {i} between two events that have the same date"); + ops.push(Operation::Remove(i - 1)); + } else { + trace!("replacing day divider @ {i} with new timestamp from event"); + ops.push(Operation::Replace(i - 1, ts)); + } + } + } else if let Some(prev_event) = prev_item.as_event() { + // The event is preceded by another event. If they're not the same date, + // insert a date divider. + let prev_ts = prev_event.timestamp(); + if timestamp_to_date(prev_ts) != timestamp_to_date(ts) { + trace!("inserting day divider @ {} between two events with different dates", i); + ops.push(Operation::Insert(i, ts)); + } + } + } else { + // The event was the first item, so there wasn't any day divider before it: + // insert one. + trace!("inserting the first day divider @ {}", i); + ops.push(Operation::Insert(i, ts)); + } + + prev_item = Some((item, ts)); + last_event_ts = Some(ts); + } + } + + // Record the deletion offset. + let mut offset = 0; + // Remember what the maximum index was, so we can assert that it's non-decreasing. + let mut max_i = 0; + + for op in ops { + match op { + Operation::Insert(i, ts) => { + assert!(i >= max_i); + self.items.insert( + i + offset, + self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(ts)), + ); + offset = (offset + 1).min(self.items.len()); + max_i = i; + } + + Operation::Replace(i, ts) => { + assert!(i >= max_i); + let prev = &self.items[i + offset]; + assert!(prev.is_day_divider(), "we replaced a non day-divider"); + + let unique_id = prev.unique_id(); + let item = TimelineItem::new(VirtualTimelineItem::DayDivider(ts), unique_id); + + self.items.set(i + offset, item); + max_i = i; + } + + Operation::Remove(i) => { + assert!(i >= max_i); + let prev = self.items.remove(i + offset); + assert!(prev.is_day_divider(), "we removed a non day-divider"); + offset = offset.saturating_sub(1); + max_i = i; + } + } + } + + // Then check invariants. + self.assert_date_divider_invariants(); + } + + fn assert_date_divider_invariants(&self) { + // Assert invariants. + // 1. The timeline starts with a date separator. + assert!(self.items[0].is_day_divider()); + + // 2. There are no two date dividers following each other. + { + let mut prev_was_day_divider = false; + for (i, item) in self.items.iter().enumerate() { + if item.is_day_divider() { + assert!(!prev_was_day_divider, "double day divider at {i}"); + prev_was_day_divider = true; + } else { + prev_was_day_divider = false; + } + } + }; + + // 3. There's no trailing day divider. + assert!(!self.items.last().unwrap().is_day_divider()); + + // 4. Items are properly separated with day dividers. + { + let mut prev_event_ts = None; + let mut prev_day_divider_ts = None; + + for item in self.items.iter() { + if let Some(ev) = item.as_event() { + let ts = ev.timestamp(); + + // We have the same date as the previous event we've seen. + if let Some(prev_ts) = prev_event_ts { + assert_eq!(timestamp_to_date(prev_ts), timestamp_to_date(ts)); + } + + // There is a day divider before us, and it's the same date as our timestamp. + let prev_ts = + prev_day_divider_ts.expect("there must be a day divider before an event"); + assert_eq!(timestamp_to_date(prev_ts), timestamp_to_date(ts)); + + prev_event_ts = Some(ts); + } else if let TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(ts)) = + item.kind() + { + // The previous day divider is for a different date. + if let Some(prev_ts) = prev_day_divider_ts { + assert_ne!(timestamp_to_date(prev_ts), timestamp_to_date(*ts)); + } + + prev_event_ts = None; + prev_day_divider_ts = Some(*ts); + } + } + } + } + fn pending_reactions(&mut self) -> Option { match &self.ctx.flow { Flow::Local { .. } => None, diff --git a/crates/matrix-sdk-ui/src/timeline/inner/state.rs b/crates/matrix-sdk-ui/src/timeline/inner/state.rs index 3cc389811..242b67239 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/state.rs @@ -45,9 +45,9 @@ use crate::{ reactions::{ReactionToggleResult, Reactions}, read_receipts::ReadReceipts, traits::RoomDataProvider, - util::{rfind_event_by_id, rfind_event_item, timestamp_to_date, RelativePosition}, + util::{rfind_event_by_id, rfind_event_item, RelativePosition}, AnnotationKey, Error as TimelineError, Profile, ReactionSenderData, TimelineItem, - TimelineItemKind, VirtualTimelineItem, + TimelineItemKind, }, unable_to_decrypt_hook::UtdHookManager, }; @@ -777,17 +777,6 @@ impl TimelineInnerMetadata { TimelineItem::new(kind, self.next_internal_id()) } - /// Returns a new day divider item for the new timestamp if it is on a - /// different day than the old timestamp - pub fn maybe_create_day_divider_from_timestamps( - &mut self, - old_ts: MilliSecondsSinceUnixEpoch, - new_ts: MilliSecondsSinceUnixEpoch, - ) -> Option> { - (timestamp_to_date(old_ts) != timestamp_to_date(new_ts)) - .then(|| self.new_timeline_item(VirtualTimelineItem::DayDivider(new_ts))) - } - /// Try to update the read marker item in the timeline. pub(crate) fn update_read_marker( &mut self, diff --git a/crates/matrix-sdk-ui/src/timeline/item.rs b/crates/matrix-sdk-ui/src/timeline/item.rs index 4f693fa06..1598b7b58 100644 --- a/crates/matrix-sdk-ui/src/timeline/item.rs +++ b/crates/matrix-sdk-ui/src/timeline/item.rs @@ -89,8 +89,8 @@ impl TimelineItem { matches!(&self.kind, TimelineItemKind::Event(ev) if ev.is_remote_event()) } - pub(crate) fn is_virtual(&self) -> bool { - matches!(self.kind, TimelineItemKind::Virtual(_)) + pub(crate) fn is_event(&self) -> bool { + matches!(self.kind, TimelineItemKind::Event(_)) } /// Check whether this item is a day divider. diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index 7fa1072f0..70546753d 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -275,7 +275,7 @@ mod tests { } #[test] - fn simple_request_limits() { + fn test_simple_request_limits() { let mut opts = PaginationOptions::simple_request(10); let mut outcome = PaginationOutcome::default(); assert_eq!(opts.next_event_limit(outcome), Some(10)); @@ -285,7 +285,7 @@ mod tests { } #[test] - fn until_num_items_limits() { + fn test_until_num_items_limits() { let mut opts = PaginationOptions::until_num_items(10, 10); let mut outcome = PaginationOutcome::default(); assert_eq!(opts.next_event_limit(outcome), Some(10)); @@ -298,7 +298,7 @@ mod tests { } #[test] - fn custom_limits() { + fn test_custom_limits() { let num_calls = AtomicU8::new(0); let mut opts = PaginationOptions::custom(8, |outcome| { num_calls.fetch_add(1, Ordering::AcqRel); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index 6519849a0..d74ece34c 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -62,10 +62,12 @@ async fn test_initial_events() { ) .await; - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - assert_matches!(&item.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_))); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); assert_eq!(item.as_event().unwrap().sender(), *ALICE); + + let item = assert_next_matches!(stream, VectorDiff::Insert { index: 0 , value } => value); + assert_matches!(&item.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_))); + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); assert_eq!(item.as_event().unwrap().sender(), *BOB); } @@ -175,8 +177,6 @@ async fn test_other_state() { .handle_live_state_event(&ALICE, RoomNameEventContent::new("Alice's room".to_owned()), None) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); assert_let!(TimelineItemContent::OtherState(ev) = item.as_event().unwrap().content()); assert_let!(AnyOtherFullStateEventContent::RoomName(full_content) = ev.content()); @@ -184,6 +184,9 @@ async fn test_other_state() { assert_eq!(content.name, "Alice's room"); assert_matches!(prev_content, None); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + timeline.handle_live_redacted_state_event(&ALICE, RedactedRoomTopicEventContent::new()).await; let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -266,7 +269,8 @@ async fn test_dedup_initial() { assert_eq!(event3.as_event().unwrap().sender(), *CAROL); // Make sure we reused IDs when deduplicating events - assert_eq!(event1.unique_id(), 1); + assert_eq!(event1.unique_id(), 0); + // 1 is for the day divider. assert_eq!(event2.unique_id(), 2); assert_eq!(event3.unique_id(), 3); } @@ -296,8 +300,6 @@ async fn test_sanitized() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event = item.as_event().unwrap(); assert_let!(TimelineItemContent::Message(message) = event.content()); @@ -310,6 +312,9 @@ async fn test_sanitized() { Some code\ " ); + + let day_divider = assert_next_matches!(stream, VectorDiff::Insert {index: 0, value } => value); + assert!(day_divider.is_day_divider()); } #[async_test] @@ -324,14 +329,15 @@ async fn test_reply() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let first_event = item.as_event().unwrap(); assert!(first_event.can_be_replied_to()); let first_event_id = first_event.event_id().unwrap(); let first_event_sender = *ALICE; + let day_divider = assert_next_matches!(stream, VectorDiff::Insert {index: 0, value } => value); + assert!(day_divider.is_day_divider()); + let reply_formatted_body = format!("\ \
\ @@ -380,12 +386,13 @@ async fn test_thread() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let first_event = item.as_event().unwrap(); let first_event_id = first_event.event_id().unwrap(); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + let reply = assign!(RoomMessageEventContent::text_plain("I'm replying in a thread"), { relates_to: Some(Relation::Thread( Thread::plain(first_event_id.to_owned(), first_event_id.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 c229f5290..e54e536fb 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/echo.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/echo.rs @@ -39,8 +39,6 @@ async fn test_remote_echo_full_trip() { )) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - // Scenario 1: The local event has not been sent yet to the server. let id = { let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); @@ -51,6 +49,14 @@ async fn test_remote_echo_full_trip() { item.unique_id() }; + { + // The day divider comes in late. + let (index, day_divider) = + assert_next_matches!(stream, VectorDiff::Insert {index, value } => (index, value)); + assert_eq!(index, 0); + assert!(day_divider.is_day_divider()); + } + // Scenario 2: The local event has not been sent to the server successfully, it // has failed. In this case, there is no event ID. { @@ -106,7 +112,7 @@ async fn test_remote_echo_full_trip() { })) .await; - // The local echo is replaced with the remote echo + // The local echo is replaced with the remote echo. let item = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); assert!(!item.as_event().unwrap().is_local_echo()); assert_eq!(item.unique_id(), id); @@ -124,19 +130,24 @@ async fn test_remote_echo_new_position() { )) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let txn_id_from_event = item.as_event().unwrap(); assert_eq!(txn_id, txn_id_from_event.transaction_id().unwrap()); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + // … and another event that comes back before the remote echo timeline.handle_live_message_event(&BOB, RoomMessageEventContent::text_plain("test")).await; + // … and is inserted before the local echo item - let _day_divider = - assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); - let _bob_message = - assert_next_matches!(stream, VectorDiff::Insert { index: 1, value } => value); + let bob_message = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(bob_message.is_remote_event()); + + let (day_divider_index, day_divider) = + assert_next_matches!(stream, VectorDiff::Insert {index, value } => (index, value)); + assert_eq!(day_divider_index, 0); + assert!(day_divider.is_day_divider()); // When the remote echo comes in… timeline @@ -155,15 +166,13 @@ async fn test_remote_echo_new_position() { })) .await; - // … the local echo should be removed - assert_next_matches!(stream, VectorDiff::Remove { index: 3 }); - // … along with its day divider - assert_next_matches!(stream, VectorDiff::Remove { index: 2 }); - - // … and the remote echo added (no new day divider because both bob's and - // alice's message are from the same day according to server timestamps) - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + // … the remote echo replaces the previous event. + let item = assert_next_matches!(stream, VectorDiff::Set { index: 3, value } => value); assert!(!item.as_event().unwrap().is_local_echo()); + + // … the day 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 }); } #[async_test] diff --git a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs index 0b4fa4e37..ad8f1433b 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/edit.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/edit.rs @@ -38,7 +38,6 @@ async fn test_live_redacted() { timeline .handle_live_redacted_message_event(*ALICE, RedactedRoomMessageEventContent::new()) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let redacted_event_id = item.as_event().unwrap().event_id().unwrap(); @@ -52,6 +51,9 @@ async fn test_live_redacted() { timeline.handle_live_message_event(&ALICE, edit).await; assert_eq!(timeline.inner.items().await.len(), 2); + + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); } #[async_test] @@ -69,8 +71,6 @@ async fn test_live_sanitized() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let first_event = item.as_event().unwrap(); assert_let!(TimelineItemContent::Message(message) = first_event.content()); @@ -78,6 +78,9 @@ async fn test_live_sanitized() { assert_eq!(text.body, "**original** message"); assert_eq!(text.formatted.as_ref().unwrap().body, "original message"); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + let first_event_id = first_event.event_id().unwrap(); let new_plain_content = "!!edited!! **better** message"; @@ -150,11 +153,13 @@ async fn test_aggregated_sanitized() { }); timeline.handle_live_event(ev).await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let first_event = item.as_event().unwrap(); assert_let!(TimelineItemContent::Message(message) = first_event.content()); assert_let!(MessageType::Text(text) = message.msgtype()); assert_eq!(text.body, "!!edited!! **better** message"); assert_eq!(text.formatted.as_ref().unwrap().body, " better message"); + + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs index dd24b9c1e..12fdda843 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/encryption.rs @@ -102,7 +102,6 @@ async fn test_retry_message_decryption() { assert_eq!(timeline.inner.items().await.len(), 2); - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event = item.as_event().unwrap(); assert_let!( @@ -113,6 +112,9 @@ async fn test_retry_message_decryption() { ); assert_eq!(session_id, SESSION_ID); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + { let utds = hook.utds.lock().unwrap(); assert_eq!(utds.len(), 1); @@ -415,7 +417,6 @@ async fn test_retry_message_decryption_highlighted() { assert_eq!(timeline.inner.items().await.len(), 2); - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event = item.as_event().unwrap(); assert_let!( @@ -426,6 +427,9 @@ async fn test_retry_message_decryption_highlighted() { ); assert_eq!(session_id, SESSION_ID); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + let own_user_id = user_id!("@example:matrix.org"); let exported_keys = decrypt_room_key_export(Cursor::new(SESSION_KEY), "1234").unwrap(); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/event_filter.rs b/crates/matrix-sdk-ui/src/timeline/tests/event_filter.rs index 790fb7773..cd9a0b092 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/event_filter.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/event_filter.rs @@ -43,7 +43,7 @@ use crate::timeline::{ }; #[async_test] -async fn default_filter() { +async fn test_default_filter() { let timeline = TestTimeline::new(); let mut stream = timeline.subscribe().await; @@ -51,8 +51,9 @@ async fn default_filter() { timeline .handle_live_message_event(&ALICE, RoomMessageEventContent::text_plain("The first message")) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); let first_event_id = item.as_event().unwrap().event_id().unwrap(); let edit = assign!(RoomMessageEventContent::text_plain(" * The _edited_ first message"), { @@ -106,7 +107,7 @@ async fn default_filter() { } #[async_test] -async fn filter_always_false() { +async fn test_filter_always_false() { let timeline = TestTimeline::new().with_settings(TimelineInnerSettings { event_filter: Arc::new(|_, _| false), ..Default::default() @@ -137,7 +138,7 @@ async fn filter_always_false() { } #[async_test] -async fn custom_filter() { +async fn test_custom_filter() { // Filter out all state events. let timeline = TestTimeline::new().with_settings(TimelineInnerSettings { event_filter: Arc::new(|ev, _| matches!(ev, AnySyncTimelineEvent::MessageLike(_))), @@ -148,8 +149,9 @@ async fn custom_filter() { timeline .handle_live_message_event(&ALICE, RoomMessageEventContent::text_plain("The first message")) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let _item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); timeline .handle_live_redacted_message_event(&ALICE, RedactedRoomMessageEventContent::new()) @@ -173,7 +175,7 @@ async fn custom_filter() { } #[async_test] -async fn hide_failed_to_parse() { +async fn test_hide_failed_to_parse() { let timeline = TestTimeline::new() .with_settings(TimelineInnerSettings { add_failed_to_parse: false, ..Default::default() }); @@ -206,7 +208,7 @@ async fn hide_failed_to_parse() { } #[async_test] -async fn event_type_filter_include_only_room_names() { +async fn test_event_type_filter_include_only_room_names() { // Only return room name events let event_filter = TimelineEventTypeFilter::Include(vec![TimelineEventType::RoomName]); @@ -255,7 +257,7 @@ async fn event_type_filter_include_only_room_names() { } #[async_test] -async fn event_type_filter_exclude_messages() { +async fn test_event_type_filter_exclude_messages() { // Don't return any messages let event_filter = TimelineEventTypeFilter::Exclude(vec![TimelineEventType::RoomMessage]); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs index bd9a5207c..ece104756 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs @@ -285,11 +285,13 @@ async fn send_first_message( .handle_live_message_event(&BOB, RoomMessageEventContent::text_plain("I want you to react")) .await; - let _day_divider = assert_next_matches!(*stream, VectorDiff::PushBack { value } => value); - let item = assert_next_matches!(*stream, VectorDiff::PushBack { value } => value); let event_id = item.as_event().unwrap().clone().event_id().unwrap().to_owned(); let position = timeline.len().await - 1; + + let _day_divider = + assert_next_matches!(*stream, VectorDiff::Insert { index: 0, value } => value); + (event_id, position) } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs b/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs index 776b8e61f..a2b53569f 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/read_receipts.rs @@ -48,13 +48,14 @@ async fn test_read_receipts_updates_on_live_events() { timeline.handle_live_message_event(*ALICE, RoomMessageEventContent::text_plain("A")).await; timeline.handle_live_message_event(*BOB, RoomMessageEventContent::text_plain("B")).await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - // No read receipt for our own user. let item_a = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event_a = item_a.as_event().unwrap(); assert!(event_a.read_receipts().is_empty()); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + // Implicit read receipt of Bob. let item_b = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event_b = item_b.as_event().unwrap(); @@ -149,13 +150,14 @@ async fn test_read_receipts_updates_on_filtered_events() { timeline.handle_live_message_event(*ALICE, RoomMessageEventContent::text_plain("A")).await; timeline.handle_live_message_event(*BOB, RoomMessageEventContent::notice_plain("B")).await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - // No read receipt for our own user. let item_a = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event_a = item_a.as_event().unwrap(); assert!(event_a.read_receipts().is_empty()); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + // Implicit read receipt of Bob. let item_a = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); let event_a = item_a.as_event().unwrap(); @@ -240,13 +242,14 @@ async fn test_read_receipts_updates_on_filtered_events_with_stored() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - // No read receipt for our own user. let item_a = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let event_a = item_a.as_event().unwrap(); assert!(event_a.read_receipts().is_empty()); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + // Stored read receipt of Bob. let item_a = assert_next_matches!(stream, VectorDiff::Set { index: 1, value } => value); let event_a = item_a.as_event().unwrap(); @@ -302,13 +305,14 @@ async fn test_read_receipts_updates_on_back_paginated_filtered_events() { ) .await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value); - // No read receipt for our own user. - let item_a = assert_next_matches!(stream, VectorDiff::Insert { index: 1, value } => value); + let item_a = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); let event_a = item_a.as_event().unwrap(); assert!(event_a.read_receipts().is_empty()); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + // Add non-filtered event to show read receipts. timeline .handle_back_paginated_message_event_with_id( @@ -320,12 +324,19 @@ async fn test_read_receipts_updates_on_back_paginated_filtered_events() { .await; // Implicit read receipt of Carol. - let item_c = assert_next_matches!(stream, VectorDiff::Insert { index: 1, value } => value); + let item_c = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); let event_c = item_c.as_event().unwrap(); assert_eq!(event_c.read_receipts().len(), 2); assert!(event_c.read_receipts().get(*BOB).is_some()); assert!(event_c.read_receipts().get(*CAROL).is_some()); + // Reinsert a new day divider before the first back-paginated event. + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + + // Remove the last day divider. + assert_next_matches!(stream, VectorDiff::Remove { index: 2 }); + assert_pending!(stream); } @@ -411,8 +422,6 @@ async fn test_read_receipts_updates_on_message_decryption() { assert_eq!(timeline.inner.items().await.len(), 3); - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - // The first event only has Carol's receipt. let clear_item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let clear_event = clear_item.as_event().unwrap(); @@ -420,6 +429,9 @@ async fn test_read_receipts_updates_on_message_decryption() { assert_eq!(clear_event.read_receipts().len(), 1); assert!(clear_event.read_receipts().get(*CAROL).is_some()); + let _day_divider = + assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + // The second event is encrypted and only has Bob's receipt. let encrypted_item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let encrypted_event = encrypted_item.as_event().unwrap(); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/virt.rs b/crates/matrix-sdk-ui/src/timeline/tests/virt.rs index 49ba1442c..3101823f0 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/virt.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/virt.rs @@ -27,7 +27,7 @@ use super::TestTimeline; use crate::timeline::{TimelineItemKind, VirtualTimelineItem}; #[async_test] -async fn day_divider() { +async fn test_day_divider() { let timeline = TestTimeline::new(); let mut stream = timeline.subscribe().await; @@ -38,16 +38,16 @@ async fn day_divider() { ) .await; - let day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + item.as_event().unwrap(); + + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); assert_let!(VirtualTimelineItem::DayDivider(ts) = day_divider.as_virtual().unwrap()); let date = Local.timestamp_millis_opt(ts.0.into()).single().unwrap(); assert_eq!(date.year(), 1970); assert_eq!(date.month(), 1); assert_eq!(date.day(), 1); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - item.as_event().unwrap(); - timeline .handle_live_message_event( *ALICE, @@ -68,29 +68,29 @@ async fn day_divider() { ) .await; - let day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + item.as_event().unwrap(); + + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 3, value } => value); assert_let!(VirtualTimelineItem::DayDivider(ts) = day_divider.as_virtual().unwrap()); let date = Local.timestamp_millis_opt(ts.0.into()).single().unwrap(); assert_eq!(date.year(), 1970); assert_eq!(date.month(), 1); assert_eq!(date.day(), 2); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - item.as_event().unwrap(); - let _ = timeline .handle_local_event(AnyMessageLikeEventContent::RoomMessage( RoomMessageEventContent::text_plain("A message I'm sending just now"), )) .await; - // The other events are in the past so a local event always creates a new day - // divider. - let day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); - assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider { .. }); - let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); item.as_event().unwrap(); + + // The other events are in the past so a local event always creates a new day + // divider. + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 5, value } => value); + assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider { .. }); } #[async_test] @@ -99,10 +99,13 @@ async fn test_update_read_marker() { let mut stream = timeline.subscribe().await; timeline.handle_live_message_event(&ALICE, RoomMessageEventContent::text_plain("A")).await; - let _day_divider = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); let first_event_id = item.as_event().unwrap().event_id().unwrap().to_owned(); + let day_divider = assert_next_matches!(stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + timeline.inner.set_fully_read_event(first_event_id.clone()).await; // Nothing should happen, the marker cannot be added at the end. diff --git a/crates/matrix-sdk-ui/src/timeline/util.rs b/crates/matrix-sdk-ui/src/timeline/util.rs index 63ac140c6..4776446f8 100644 --- a/crates/matrix-sdk-ui/src/timeline/util.rs +++ b/crates/matrix-sdk-ui/src/timeline/util.rs @@ -84,7 +84,7 @@ pub(super) enum RelativePosition { Before, } -#[derive(PartialEq)] +#[derive(Debug, PartialEq)] pub(super) struct Date { year: i32, month: u32, diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs b/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs index ca1f8ee21..f6e8a7b1f 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/echo.rs @@ -23,9 +23,7 @@ use matrix_sdk::{ test_utils::logged_in_client_with_server, }; use matrix_sdk_test::{async_test, sync_timeline_event, JoinedRoomBuilder, SyncResponseBuilder}; -use matrix_sdk_ui::timeline::{ - EventSendState, RoomExt, TimelineItemContent, TimelineItemKind, VirtualTimelineItem, -}; +use matrix_sdk_ui::timeline::{EventSendState, RoomExt, TimelineItemContent}; use ruma::{ event_id, events::room::message::{MessageType, RoomMessageEventContent}, @@ -76,13 +74,16 @@ async fn test_echo() { timeline.send(RoomMessageEventContent::text_plain("Hello, World!").into()).await }); - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); assert_let!(Some(VectorDiff::PushBack { value: local_echo }) = timeline_stream.next().await); let item = local_echo.as_event().unwrap(); assert_matches!(item.send_state(), Some(EventSendState::NotSentYet)); let txn_id = item.transaction_id().unwrap(); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + assert_let!(TimelineItemContent::Message(msg) = item.content()); assert_let!(MessageType::Text(text) = msg.msgtype()); assert_eq!(text.body, "Hello, World!"); @@ -114,20 +115,17 @@ async fn test_echo() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - // Local echo is removed - assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 1 })); - // Local echo day divider is removed - assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 0 })); - - // New day divider is added - assert_let!(Some(VectorDiff::PushBack { value: new_item }) = timeline_stream.next().await); - assert_matches!(**new_item, TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_))); - - // Remote echo is added - assert_let!(Some(VectorDiff::PushBack { value: remote_echo }) = timeline_stream.next().await); + // Local echo is replaced with the remote echo. + let remote_echo = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => value); let item = remote_echo.as_event().unwrap(); assert!(item.is_own()); assert_eq!(item.timestamp(), MilliSecondsSinceUnixEpoch(uint!(152038280))); + + // The day divider is also replaced. + let day_divider = + assert_next_matches!(timeline_stream, VectorDiff::Set { index: 0, value } => value); + assert!(day_divider.is_day_divider()); } #[async_test] @@ -222,11 +220,14 @@ async fn test_dedup_by_event_id_late() { timeline.send(RoomMessageEventContent::text_plain("Hello, World!").into()).await; - assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { .. })); // day divider assert_let!(Some(VectorDiff::PushBack { value: local_echo }) = timeline_stream.next().await); let item = local_echo.as_event().unwrap(); assert_matches!(item.send_state(), Some(EventSendState::NotSentYet)); + let day_divider = + assert_next_matches!( timeline_stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( sync_timeline_event!({ "content": { @@ -244,12 +245,15 @@ async fn test_dedup_by_event_id_late() { mock_sync(&server, ev_builder.build_json_sync_response(), None).await; let _response = client.sync_once(sync_settings.clone()).await.unwrap(); - assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, .. }); // day divider let remote_echo = - assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 1, value } => value); + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, value } => value); let item = remote_echo.as_event().unwrap(); assert_eq!(item.event_id(), Some(event_id)); + let day_divider = + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, value } => value); + assert!(day_divider.is_day_divider()); + // Local echo and its day divider are removed. assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 3 })); assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 2 })); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs index 8c8e581ea..c80bd160e 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs @@ -79,9 +79,6 @@ async fn test_edit() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); - assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await); let item = first.as_event().unwrap(); assert_eq!(item.read_receipts().len(), 1, "implicit read receipt"); @@ -90,6 +87,11 @@ async fn test_edit() { assert_matches!(msg.in_reply_to(), None); assert!(!msg.is_edited()); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + ev_builder.add_joined_room( JoinedRoomBuilder::new(room_id) .add_timeline_event(event_builder.make_sync_message_event( diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index 886f4ee54..26177ccc8 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -87,16 +87,18 @@ async fn test_reaction() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - // The day divider. - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); - // The new message starts with their author's read receipt. assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); let event_item = message.as_event().unwrap(); assert_matches!(event_item.content(), TimelineItemContent::Message(_)); assert_eq!(event_item.read_receipts().len(), 1); + // The day divider. + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + // The new message is getting the reaction, which implies an implicit read // receipt that's obtained first. assert_let!( @@ -198,11 +200,14 @@ async fn test_redacted_message() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await); assert_matches!(first.as_event().unwrap().content(), TimelineItemContent::RedactedMessage); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + // TODO: After adding raw timeline items, check for one here } @@ -240,11 +245,14 @@ async fn test_read_marker() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); assert_matches!(message.as_event().unwrap().content(), TimelineItemContent::Message(_)); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + ev_builder.add_joined_room( JoinedRoomBuilder::new(room_id).add_account_data(RoomAccountDataTestEvent::FullyRead), ); @@ -321,13 +329,16 @@ async fn test_sync_highlighted() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); 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()); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( sync_timeline_event!({ "content": { diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs index 6fd8778e1..65e1f01e1 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs @@ -14,7 +14,6 @@ use std::{sync::Arc, time::Duration}; -use assert_matches::assert_matches; use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::future::{join, join3}; @@ -24,7 +23,7 @@ use matrix_sdk_test::{ }; use matrix_sdk_ui::timeline::{ AnyOtherFullStateEventContent, BackPaginationStatus, PaginationOptions, RoomExt, - TimelineItemContent, VirtualTimelineItem, + TimelineItemContent, }; use once_cell::sync::Lazy; use ruma::{ @@ -80,31 +79,39 @@ async fn test_back_pagination() { }; join(paginate, observe_paginating).await; - let day_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider(_)); - let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, 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 day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, 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"); + // The day divider is replaced. + let day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); assert_eq!(state.state_key(), ""); @@ -199,27 +206,35 @@ async fn test_back_pagination_highlighted() { timeline.paginate_backwards(PaginationOptions::simple_request(10)).await.unwrap(); server.reset().await; - let day_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider(_)); - let first = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); let remote_event = first.as_event().unwrap(); // Own events don't trigger push rules. assert!(!remote_event.is_highlighted()); + let day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + let second = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); let remote_event = second.as_event().unwrap(); // `m.room.tombstone` should be highlighted by default. assert!(remote_event.is_highlighted()); + + let day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); } #[async_test] @@ -572,31 +587,38 @@ async fn test_empty_chunk() { }; join(paginate, observe_paginating).await; - let day_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider(_)); - let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, 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 day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, 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 day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); assert_eq!(state.state_key(), ""); @@ -671,31 +693,38 @@ async fn test_until_num_items_with_empty_chunk() { }; join(paginate, observe_paginating).await; - let day_divider = assert_next_matches!( - timeline_stream, - VectorDiff::PushFront { value } => value - ); - assert_matches!(day_divider.as_virtual().unwrap(), VirtualTimelineItem::DayDivider(_)); - let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index:0, 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 day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, 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 day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); assert_let!(TimelineItemContent::OtherState(state) = message.as_event().unwrap().content()); assert_eq!(state.state_key(), ""); @@ -708,9 +737,16 @@ async fn test_until_num_items_with_empty_chunk() { assert_eq!(content.name, "New room name"); assert_eq!(prev_content.as_ref().unwrap().name.as_ref().unwrap(), "Old room name"); + let day_divider = assert_next_matches!( + timeline_stream, + VectorDiff::Insert { index: 0, value } => value + ); + assert!(day_divider.is_day_divider()); + assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 }); + let message = assert_next_matches!( timeline_stream, - VectorDiff::Insert { index: 1, value } => value + VectorDiff::Insert { index: 0, value } => value ); assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content()); assert_let!(MessageType::Text(text) = msg.msgtype()); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs b/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs index 1f908bcba..9b5f370f5 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs @@ -127,14 +127,16 @@ async fn test_read_receipts_updates() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); - // We don't list the read receipt of our own user on events. assert_let!(Some(VectorDiff::PushBack { value: first_item }) = timeline_stream.next().await); let first_event = first_item.as_event().unwrap(); assert!(first_event.read_receipts().is_empty()); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + let (own_receipt_event_id, _) = timeline.latest_user_read_receipt(own_user_id).await.unwrap(); assert_eq!(own_receipt_event_id, first_event.event_id().unwrap()); @@ -362,14 +364,16 @@ async fn test_read_receipts_updates_on_filtered_events() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); - // We don't list the read receipt of our own user on events. assert_let!(Some(VectorDiff::PushBack { value: item_a }) = timeline_stream.next().await); let event_a = item_a.as_event().unwrap(); assert!(event_a.read_receipts().is_empty()); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + let (own_receipt_event_id, _) = timeline.latest_user_read_receipt(own_user_id).await.unwrap(); assert_eq!(own_receipt_event_id, event_a_id); let own_receipt_timeline_event = diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs index 6ccd9a6a1..7f24bca71 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs @@ -80,10 +80,14 @@ async fn test_in_reply_to_details() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await); assert_matches!(first.as_event().unwrap().content(), TimelineItemContent::Message(_)); + + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + assert_let!(Some(VectorDiff::PushBack { value: second }) = timeline_stream.next().await); let second_event = second.as_event().unwrap(); assert_let!(TimelineItemContent::Message(message) = second_event.content()); 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 12e2c405d..6d5c19d81 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -120,6 +120,30 @@ macro_rules! assert_timeline_stream { ) }; + // `insert [$nth] --- day divider ---` + ( @_ [ $stream:ident ] [ insert [$index:literal] --- day divider --- ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_timeline_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + { + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Insert { index: $index, value })) => { + assert_matches!( + &**value, + TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_)) => {} + ); + } + ); + } + ] + ) + }; + + // `insert [$nth] "$event_id"` ( @_ [ $stream:ident ] [ insert [$index:literal] $event_id:literal ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { assert_timeline_stream!( @@ -316,8 +340,8 @@ async fn test_timeline_basic() -> Result<()> { assert_timeline_stream! { [timeline_stream] - --- day divider ---; append "$x1:bar.org"; + insert[0] --- day divider ---; update[1] "$x1:bar.org"; append "$x2:bar.org"; }; @@ -362,8 +386,8 @@ async fn test_timeline_duplicated_events() -> Result<()> { assert_timeline_stream! { [timeline_stream] - --- day divider ---; append "$x1:bar.org"; + insert[0] --- day divider ---; update[1] "$x1:bar.org"; append "$x2:bar.org"; update[2] "$x2:bar.org"; @@ -440,8 +464,8 @@ async fn test_timeline_read_receipts_are_updated_live() -> Result<()> { assert_timeline_stream! { [timeline_stream] - --- day divider ---; append "$x1:bar.org"; + insert[0] --- day divider ---; update[1] "$x1:bar.org"; append "$x2: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 0ded3b888..18f996992 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs @@ -23,7 +23,7 @@ use matrix_sdk_test::{ async_test, sync_timeline_event, EventBuilder, GlobalAccountDataTestEvent, JoinedRoomBuilder, SyncResponseBuilder, ALICE, BOB, }; -use matrix_sdk_ui::timeline::{RoomExt, TimelineDetails, TimelineItemContent, VirtualTimelineItem}; +use matrix_sdk_ui::timeline::{RoomExt, TimelineDetails, TimelineItemContent}; use ruma::{ event_id, events::room::{ @@ -121,9 +121,6 @@ async fn test_event_filter() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!(Some(VectorDiff::PushBack { value: day_divider }) = timeline_stream.next().await); - assert!(day_divider.is_day_divider()); - assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await); let first_event = first.as_event().unwrap(); assert_eq!(first_event.event_id(), Some(first_event_id)); @@ -132,6 +129,11 @@ async fn test_event_filter() { assert_matches!(msg.msgtype(), MessageType::Text(_)); assert!(!msg.is_edited()); + assert_let!( + Some(VectorDiff::Insert { index: 0, value: day_divider }) = timeline_stream.next().await + ); + assert!(day_divider.is_day_divider()); + let second_event_id = event_id!("$Ga6Y2l0gKY"); let edit_event_id = event_id!("$7i9In0gEmB"); ev_builder.add_joined_room( @@ -255,12 +257,12 @@ async fn test_timeline_is_reset_when_a_user_is_ignored_or_unignored() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { - assert_matches!(value.as_virtual(), Some(VirtualTimelineItem::DayDivider(_))); - }); assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { assert_eq!(value.as_event().unwrap().event_id(), Some(first_event_id)); }); + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, value } => { + assert!(value.is_day_divider()); + }); assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { assert_eq!(value.as_event().unwrap().event_id(), Some(second_event_id)); }); @@ -321,12 +323,12 @@ 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::PushBack { value } => { - assert_matches!(value.as_virtual(), Some(VirtualTimelineItem::DayDivider(_))); - }); assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { assert_eq!(value.as_event().unwrap().event_id(), Some(fourth_event_id)); }); + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, value } => { + assert!(value.is_day_divider()); + }); assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => { assert_eq!(value.as_event().unwrap().event_id(), Some(fourth_event_id)); }); @@ -389,15 +391,15 @@ async fn test_profile_updates() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => { - assert_matches!(value.as_virtual(), Some(VirtualTimelineItem::DayDivider(_))); - }); - let item_1 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); let event_1_item = item_1.as_event().unwrap(); assert_eq!(event_1_item.event_id(), Some(event_1_id)); assert_matches!(event_1_item.sender_profile(), TimelineDetails::Unavailable); + assert_next_matches!(timeline_stream, VectorDiff::Insert { index: 0, value } => { + assert!(value.is_day_divider()); + }); + let item_2 = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); let event_2_item = item_2.as_event().unwrap(); assert_eq!(event_2_item.event_id(), Some(event_2_id));