mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-14 11:05:32 -04:00
Code Issues Packages Projects Releases Wiki Activity

timeline: group updates of the day dividers when multiple events are added at the same time

Browse Source
This commit is contained in:
Benjamin Bouvier
2024-03-19 18:27:56 +01:00
parent b8174c437f
commit f704066fbe
10 changed files with 91 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
crates/matrix-sdk-ui/src/timeline/event_handler.rs
View File

@@ -397,9 +397,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
trace!("Removing UTD that was successfully retried");
self.items.remove(idx);
let mut adjuster = DayDividerAdjuster { items: self.items, meta: self.meta };
adjuster.maybe_adjust_date_dividers(); // TODO add test?
self.result.item_removed = true;
}
@@ -923,14 +920,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
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));
let mut adjuster =
DayDividerAdjuster { items: self.items, meta: self.meta };
adjuster.maybe_adjust_date_dividers();
return;
}
@@ -938,12 +929,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
trace!("Removing local echo or duplicate timeline item");
removed_event_item_id = Some(self.items.remove(idx).internal_id);
assert_ne!(
idx, 0,
"there is never an event item at index 0 because \
the first event item is preceded by a day divider"
);
// no return here, below code for adding a new event
// will run to re-add the removed item
} else if txn_id.is_some() {
@@ -1003,11 +988,6 @@ 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`?
let mut adjuster = DayDividerAdjuster { items: self.items, meta: self.meta };
adjuster.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);
@@ -1068,14 +1048,14 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
/// Algorithm ensuring that day dividers are adjusted correctly, according to new items that have
/// been inserted.
pub(super) struct DayDividerAdjuster<'a, 'o> {
items: &'a mut ObservableVectorTransaction<'o, Arc<TimelineItem>>,
meta: &'a mut TimelineInnerMetadata,
pub items: &'a mut ObservableVectorTransaction<'o, Arc<TimelineItem>>,
pub meta: &'a mut TimelineInnerMetadata,
}
impl<'a, 'o> DayDividerAdjuster<'a, 'o> {
/// Ensures that date separators are properly inserted/removed when needs
/// be.
fn maybe_adjust_date_dividers(&mut self) {
pub fn maybe_adjust_day_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
@@ -1225,7 +1205,9 @@ impl<'a, 'o> DayDividerAdjuster<'a, 'o> {
fn assert_date_divider_invariants(&self) {
// Assert invariants.
// 1. The timeline starts with a date separator.
assert!(self.items[0].is_day_divider());
if let Some(item) = self.items.get(0) {
assert!(item.is_day_divider());
}
// 2. There are no two date dividers following each other.
{
@@ -1241,7 +1223,9 @@ impl<'a, 'o> DayDividerAdjuster<'a, 'o> {
};
// 3. There's no trailing day divider.
assert!(!self.items.last().unwrap().is_day_divider());
if let Some(last) = self.items.last() {
assert!(!last.is_day_divider());
}
// 4. Items are properly separated with day dividers.
{

21
crates/matrix-sdk-ui/src/timeline/inner/state.rs
View File

@@ -37,8 +37,8 @@ use crate::{
events::SyncTimelineEventWithoutContent,
timeline::{
event_handler::{
Flow, HandleEventResult, TimelineEventContext, TimelineEventHandler, TimelineEventKind,
TimelineItemPosition,
DayDividerAdjuster, Flow, HandleEventResult, TimelineEventContext,
TimelineEventHandler, TimelineEventKind, TimelineItemPosition,
},
event_item::EventItemIdentifier,
polls::PollPendingEvents,
@@ -185,6 +185,9 @@ impl TimelineInnerState {
let mut txn = self.transaction();
TimelineEventHandler::new(&mut txn, ctx)
.handle_event(TimelineEventKind::Message { content, relations: Default::default() });
txn.maybe_adjust_day_dividers();
txn.commit();
}
@@ -227,6 +230,8 @@ impl TimelineInnerState {
}
}
txn.maybe_adjust_day_dividers();
txn.commit();
}
@@ -273,6 +278,8 @@ impl TimelineInnerState {
}
}
txn.maybe_adjust_day_dividers();
txn.commit();
}
@@ -444,6 +451,8 @@ impl TimelineInnerStateTransaction<'_> {
total.items_updated += handle_one_res.items_updated as u64;
}
self.maybe_adjust_day_dividers();
total
}
@@ -675,6 +684,14 @@ impl TimelineInnerStateTransaction<'_> {
self.maybe_add_implicit_read_receipt(event_meta);
}
}
/// Inserts/removes any day dividers that might be missing or superfluous, according to the
/// events we just handled.
fn maybe_adjust_day_dividers(&mut self) {
let mut adjuster = DayDividerAdjuster { items: &mut self.items, meta: &mut self.meta };
adjuster.maybe_adjust_day_dividers();
}
}
#[derive(Clone, Debug)]

13
crates/matrix-sdk-ui/src/timeline/tests/basic.rs
View File

@@ -65,11 +65,11 @@ async fn test_initial_events() {
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::PushFront { 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);
let item = assert_next_matches!(stream, VectorDiff::PushFront { value } => value);
assert_matches!(&item.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_)));
}
#[async_test]
@@ -257,6 +257,7 @@ async fn test_dedup_initial() {
let timeline_items = timeline.inner.items().await;
assert_eq!(timeline_items.len(), 4);
assert!(timeline_items[0].is_day_divider());
let event1 = &timeline_items[1];
@@ -270,9 +271,9 @@ async fn test_dedup_initial() {
// Make sure we reused IDs when deduplicating events
assert_eq!(event1.unique_id(), 0);
// 1 is for the day divider.
assert_eq!(event2.unique_id(), 2);
assert_eq!(event3.unique_id(), 3);
assert_eq!(event2.unique_id(), 1);
assert_eq!(event3.unique_id(), 2);
assert_eq!(timeline_items[0].unique_id(), 3);
}
#[async_test]

2
crates/matrix-sdk-ui/src/timeline/tests/event_filter.rs
View File

@@ -292,7 +292,7 @@ async fn test_event_type_filter_exclude_messages() {
)
.await;
// The timeline should contain everything except for the message event
// The timeline should contain everything except for the message event.
let event_items: Vec<Arc<TimelineItem>> = timeline.get_event_items().await;
let text_message_items_count = event_items.iter().filter(is_text_message_item).count();
let room_name_items_count = event_items.iter().filter(is_room_name_item).count();

12
crates/matrix-sdk-ui/tests/integration/timeline/mod.rs
View File

@@ -93,14 +93,10 @@ async fn test_reaction() {
assert_matches!(event_item.content(), TimelineItemContent::Message(_));
assert_eq!(event_item.read_receipts().len(), 1);
// The day divider.
assert_let!(Some(VectorDiff::PushFront { 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!(
Some(VectorDiff::Set { index: 1, value: updated_message }) = timeline_stream.next().await
Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next().await
);
let event_item = updated_message.as_event().unwrap();
assert_let!(TimelineItemContent::Message(msg) = event_item.content());
@@ -110,7 +106,7 @@ async fn test_reaction() {
// Then the reaction is taken into account.
assert_let!(
Some(VectorDiff::Set { index: 1, value: updated_message }) = timeline_stream.next().await
Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next().await
);
let event_item = updated_message.as_event().unwrap();
assert_let!(TimelineItemContent::Message(msg) = event_item.content());
@@ -122,6 +118,10 @@ async fn test_reaction() {
let senders: Vec<_> = group.senders().map(|v| &v.sender_id).collect();
assert_eq!(senders.as_slice(), [user_id!("@bob:example.org")]);
// The day divider.
assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next().await);
assert!(day_divider.is_day_divider());
// TODO: After adding raw timeline items, check for one here
ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event(

68
crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs
View File

@@ -87,12 +87,6 @@ async fn test_back_pagination() {
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "hello world");
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
let message = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
@@ -101,14 +95,6 @@ async fn test_back_pagination() {
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::PushFront { 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::PushFront { value } => value
@@ -124,6 +110,12 @@ async fn test_back_pagination() {
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::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
Mock::given(method("GET"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/messages$"))
.and(header("authorization", "Bearer 1234"))
@@ -214,12 +206,6 @@ async fn test_back_pagination_highlighted() {
// Own events don't trigger push rules.
assert!(!remote_event.is_highlighted());
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
let second = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
@@ -233,8 +219,6 @@ async fn test_back_pagination_highlighted() {
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 });
}
#[async_test]
@@ -595,12 +579,6 @@ async fn test_empty_chunk() {
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "hello world");
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
let message = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
@@ -609,13 +587,6 @@ async fn test_empty_chunk() {
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "the world is big");
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { 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::PushFront { value } => value
@@ -630,6 +601,12 @@ async fn test_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::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
}
#[async_test]
@@ -701,12 +678,6 @@ async fn test_until_num_items_with_empty_chunk() {
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "hello world");
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
let message = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
@@ -715,13 +686,6 @@ async fn test_until_num_items_with_empty_chunk() {
assert_let!(MessageType::Text(text) = msg.msgtype());
assert_eq!(text.body, "the world is big");
let day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { 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::PushFront { value } => value
@@ -742,7 +706,6 @@ async fn test_until_num_items_with_empty_chunk() {
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 });
let message = assert_next_matches!(
timeline_stream,
@@ -751,4 +714,11 @@ async fn test_until_num_items_with_empty_chunk() {
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 day_divider = assert_next_matches!(
timeline_stream,
VectorDiff::PushFront { value } => value
);
assert!(day_divider.is_day_divider());
assert_next_matches!(timeline_stream, VectorDiff::Remove { index: 2 });
}

16
crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs
View File

@@ -132,9 +132,6 @@ async fn test_read_receipts_updates() {
let first_event = first_item.as_event().unwrap();
assert!(first_event.read_receipts().is_empty());
assert_let!(Some(VectorDiff::PushFront { 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());
@@ -145,7 +142,7 @@ async fn test_read_receipts_updates() {
// Read receipt of @alice:localhost is moved to third event.
assert_let!(
Some(VectorDiff::Set { index: 2, value: second_item }) = timeline_stream.next().await
Some(VectorDiff::Set { index: 1, value: second_item }) = timeline_stream.next().await
);
let second_event = second_item.as_event().unwrap();
assert!(second_event.read_receipts().is_empty());
@@ -157,6 +154,9 @@ async fn test_read_receipts_updates() {
let (alice_receipt_event_id, _) = timeline.latest_user_read_receipt(alice).await.unwrap();
assert_eq!(alice_receipt_event_id, third_event_id);
assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next().await);
assert!(day_divider.is_day_divider());
// Read receipt on unknown event is ignored.
ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_ephemeral_event(
EphemeralTestEvent::Custom(json!({
@@ -367,9 +367,6 @@ async fn test_read_receipts_updates_on_filtered_events() {
let event_a = item_a.as_event().unwrap();
assert!(event_a.read_receipts().is_empty());
assert_let!(Some(VectorDiff::PushFront { 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 =
@@ -377,7 +374,7 @@ async fn test_read_receipts_updates_on_filtered_events() {
assert_eq!(own_receipt_timeline_event, event_a_id);
// Implicit read receipt of @bob:localhost.
assert_let!(Some(VectorDiff::Set { index: 1, value: item_a }) = timeline_stream.next().await);
assert_let!(Some(VectorDiff::Set { index: 0, value: item_a }) = timeline_stream.next().await);
let event_a = item_a.as_event().unwrap();
assert_eq!(event_a.read_receipts().len(), 1);
@@ -400,6 +397,9 @@ async fn test_read_receipts_updates_on_filtered_events() {
timeline.latest_user_read_receipt_timeline_event_id(*ALICE).await.unwrap();
assert_eq!(alice_receipt_timeline_event, event_c_id);
assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next().await);
assert!(day_divider.is_day_divider());
// Read receipt on filtered event.
ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_ephemeral_event(
EphemeralTestEvent::Custom(json!({

6
crates/matrix-sdk-ui/tests/integration/timeline/replies.rs
View File

@@ -83,9 +83,6 @@ async fn test_in_reply_to_details() {
assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await);
assert_matches!(first.as_event().unwrap().content(), TimelineItemContent::Message(_));
assert_let!(Some(VectorDiff::PushFront { 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());
@@ -93,6 +90,9 @@ async fn test_in_reply_to_details() {
assert_eq!(in_reply_to.event_id, event_id!("$event1"));
assert_matches!(in_reply_to.event, TimelineDetails::Ready(_));
assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next().await);
assert!(day_divider.is_day_divider());
// Add an reply to an unknown event to the timeline
let event_id_2 = event_id!("$event2");
sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event(

14
crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs
View File

@@ -341,9 +341,9 @@ async fn test_timeline_basic() -> Result<()> {
assert_timeline_stream! {
[timeline_stream]
append "$x1:bar.org";
prepend --- day divider ---;
update[1] "$x1:bar.org";
update[0] "$x1:bar.org";
append "$x2:bar.org";
prepend --- day divider ---;
};
}
@@ -387,11 +387,11 @@ async fn test_timeline_duplicated_events() -> Result<()> {
assert_timeline_stream! {
[timeline_stream]
append "$x1:bar.org";
prepend --- day divider ---;
update[1] "$x1:bar.org";
update[0] "$x1:bar.org";
append "$x2:bar.org";
update[2] "$x2:bar.org";
update[1] "$x2:bar.org";
append "$x3:bar.org";
prepend --- day divider ---;
};
}
@@ -465,9 +465,9 @@ async fn test_timeline_read_receipts_are_updated_live() -> Result<()> {
assert_timeline_stream! {
[timeline_stream]
append "$x1:bar.org";
prepend --- day divider ---;
update[1] "$x1:bar.org";
update[0] "$x1:bar.org";
append "$x2:bar.org";
prepend --- day divider ---;
};
}

24
crates/matrix-sdk-ui/tests/integration/timeline/subscribe.rs
View File

@@ -258,18 +258,18 @@ async fn test_timeline_is_reset_when_a_user_is_ignored_or_unignored() {
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::PushFront { 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));
});
assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => {
assert_next_matches!(timeline_stream, VectorDiff::Set { index: 0, value } => {
assert_eq!(value.as_event().unwrap().event_id(), Some(first_event_id));
});
assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => {
assert_eq!(value.as_event().unwrap().event_id(), Some(third_event_id));
});
assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => {
assert!(value.is_day_divider());
});
assert_pending!(timeline_stream);
let fourth_event_id = event_id!("$YTQwYl2pl4");
@@ -324,15 +324,15 @@ async fn test_timeline_is_reset_when_a_user_is_ignored_or_unignored() {
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::PushFront { value } => {
assert!(value.is_day_divider());
});
assert_next_matches!(timeline_stream, VectorDiff::Set { index: 1, value } => {
assert_next_matches!(timeline_stream, VectorDiff::Set { index: 0, value } => {
assert_eq!(value.as_event().unwrap().event_id(), Some(fourth_event_id));
});
assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => {
assert_eq!(value.as_event().unwrap().event_id(), Some(fiveth_event_id));
});
assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => {
assert!(value.is_day_divider());
});
assert_pending!(timeline_stream);
}
@@ -394,15 +394,15 @@ async fn test_profile_updates() {
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::PushFront { 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));
assert_matches!(event_2_item.sender_profile(), TimelineDetails::Unavailable);
assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => {
assert!(value.is_day_divider());
});
assert_pending!(timeline_stream);
// Add profiles of users and other message.