From 8f5e1f3dfc367c0aefd6160a5d5b875a3ffbf843 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 19 Apr 2024 17:54:07 +0200 Subject: [PATCH] timeline: include the remote event's origin in the timeline position/end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit and don't assume inserting to the end means it's coming from sync — as it won't be true for forward pagination anymore. --- .../src/timeline/event_handler.rs | 24 ++++++----------- .../matrix-sdk-ui/src/timeline/inner/mod.rs | 12 ++++++--- .../matrix-sdk-ui/src/timeline/inner/state.rs | 27 +++++++++++-------- .../matrix-sdk-ui/src/timeline/pagination.rs | 12 ++++++--- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 12 +++++---- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 5 +++- .../src/timeline/tests/reactions.rs | 6 +++-- .../src/timeline/tests/redaction.rs | 8 ++++-- 8 files changed, 63 insertions(+), 43 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index ce23b8509..6ec2b05ed 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -213,16 +213,11 @@ impl TimelineEventKind { pub(super) enum TimelineItemPosition { /// One or more items are prepended to the timeline (i.e. they're the /// oldest). - /// - /// Usually this means items coming from back-pagination. - Start, + Start { origin: RemoteEventOrigin }, /// One or more items are appended to the timeline (i.e. they're the most /// recent). - End { - /// Whether this event is coming from a local cache. - from_cache: bool, - }, + End { origin: RemoteEventOrigin }, /// A single item is updated. /// @@ -832,16 +827,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } - let origin = match position { - TimelineItemPosition::Start => RemoteEventOrigin::Pagination, - - // We only paginate backwards for now, so End only happens for syncs - TimelineItemPosition::End { from_cache: true } => RemoteEventOrigin::Cache, - - TimelineItemPosition::End { from_cache: false } => RemoteEventOrigin::Sync, + let origin = match *position { + TimelineItemPosition::Start { origin } + | TimelineItemPosition::End { origin } => origin, + // For updates, reuse the origin of the encrypted event. #[cfg(feature = "e2e-encryption")] - TimelineItemPosition::Update(idx) => self.items[*idx] + TimelineItemPosition::Update(idx) => self.items[idx] .as_event() .and_then(|ev| Some(ev.as_remote()?.origin)) .unwrap_or_else(|| { @@ -875,7 +867,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { self.items.push_back(item); } - Flow::Remote { position: TimelineItemPosition::Start, event_id, .. } => { + Flow::Remote { position: TimelineItemPosition::Start { .. }, event_id, .. } => { if self .items .iter() diff --git a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs index 2c47457d9..fd4c00f65 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/mod.rs @@ -52,6 +52,7 @@ use tracing::{field, info_span, Instrument as _}; use super::traits::Decryptor; use super::{ event_handler::TimelineEventKind, + event_item::RemoteEventOrigin, reactions::ReactionToggleResult, traits::RoomDataProvider, util::{rfind_event_by_id, rfind_event_item, RelativePosition}, @@ -396,13 +397,16 @@ impl TimelineInner

{ &self, events: Vec>, position: TimelineEnd, + origin: RemoteEventOrigin, ) -> HandleManyEventsResult { if events.is_empty() { return Default::default(); } let mut state = self.state.write().await; - state.add_events_at(events, position, &self.room_data_provider, &self.settings).await + state + .add_events_at(events, position, origin, &self.room_data_provider, &self.settings) + .await } pub(super) async fn clear(&self) { @@ -433,7 +437,8 @@ impl TimelineInner

{ state .add_events_at( events, - TimelineEnd::Back { from_cache: true }, + TimelineEnd::Back, + RemoteEventOrigin::Cache, &self.room_data_provider, &self.settings, ) @@ -468,7 +473,8 @@ impl TimelineInner

{ state .add_events_at( vec![event], - TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, &self.room_data_provider, &self.settings, ) diff --git a/crates/matrix-sdk-ui/src/timeline/inner/state.rs b/crates/matrix-sdk-ui/src/timeline/inner/state.rs index c9a734571..5ad2bb896 100644 --- a/crates/matrix-sdk-ui/src/timeline/inner/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/inner/state.rs @@ -38,7 +38,7 @@ use crate::{ Flow, HandleEventResult, TimelineEventContext, TimelineEventHandler, TimelineEventKind, TimelineItemPosition, }, - event_item::EventItemIdentifier, + event_item::{EventItemIdentifier, RemoteEventOrigin}, polls::PollPendingEvents, reactions::{ReactionToggleResult, Reactions}, read_receipts::ReadReceipts, @@ -59,10 +59,7 @@ pub(crate) enum TimelineEnd { /// Event should be prepended to the front of the timeline. Front, /// Event should appended to the back of the timeline. - Back { - /// Did the event come from the cache? - from_cache: bool, - }, + Back, } #[derive(Debug)] @@ -100,6 +97,7 @@ impl TimelineInnerState { &mut self, events: Vec>, position: TimelineEnd, + origin: RemoteEventOrigin, room_data_provider: &P, settings: &TimelineInnerSettings, ) -> HandleManyEventsResult { @@ -109,7 +107,7 @@ impl TimelineInnerState { let mut txn = self.transaction(); let handle_many_res = - txn.add_events_at(events, position, room_data_provider, settings).await; + txn.add_events_at(events, position, origin, room_data_provider, settings).await; txn.commit(); handle_many_res @@ -135,7 +133,8 @@ impl TimelineInnerState { txn.add_events_at( events, - TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, room_data_provider, settings, ) @@ -398,14 +397,15 @@ impl TimelineInnerStateTransaction<'_> { &mut self, events: Vec>, position: TimelineEnd, + origin: RemoteEventOrigin, room_data_provider: &P, settings: &TimelineInnerSettings, ) -> HandleManyEventsResult { let mut total = HandleManyEventsResult::default(); let position = match position { - TimelineEnd::Front => TimelineItemPosition::Start, - TimelineEnd::Back { from_cache } => TimelineItemPosition::End { from_cache }, + TimelineEnd::Front => TimelineItemPosition::Start { origin }, + TimelineEnd::Back => TimelineItemPosition::End { origin }, }; let mut day_divider_adjuster = DayDividerAdjuster::default(); @@ -630,7 +630,9 @@ impl TimelineInnerStateTransaction<'_> { settings: &TimelineInnerSettings, ) { match position { - TimelineItemPosition::Start => self.meta.all_events.push_front(event_meta.base_meta()), + TimelineItemPosition::Start { .. } => { + self.meta.all_events.push_front(event_meta.base_meta()) + } TimelineItemPosition::End { .. } => { // Handle duplicated event. if let Some(pos) = @@ -660,7 +662,10 @@ impl TimelineInnerStateTransaction<'_> { } if settings.track_read_receipts - && matches!(position, TimelineItemPosition::Start | TimelineItemPosition::End { .. }) + && matches!( + position, + TimelineItemPosition::Start { .. } | TimelineItemPosition::End { .. } + ) { self.load_read_receipts_for_event(event_meta.event_id, room_data_provider).await; diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index 70546753d..155b0105b 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -17,7 +17,7 @@ use std::{fmt, ops::ControlFlow, sync::Arc, time::Duration}; use matrix_sdk::event_cache::{self, BackPaginationOutcome}; use tracing::{instrument, trace, warn}; -use crate::timeline::inner::TimelineEnd; +use crate::timeline::{event_item::RemoteEventOrigin, inner::TimelineEnd}; impl super::Timeline { /// Add more events to the start of the timeline. @@ -53,8 +53,14 @@ impl super::Timeline { let num_events = events.len(); trace!("Back-pagination succeeded with {num_events} events"); - let handle_many_res = - self.inner.add_events_at(events, TimelineEnd::Front).await; + let handle_many_res = self + .inner + .add_events_at( + events, + TimelineEnd::Front, + RemoteEventOrigin::Pagination, + ) + .await; if reached_start { self.back_pagination_status diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index ff101ffab..3c152a2ef 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -37,7 +37,7 @@ use stream_assert::assert_next_matches; use super::TestTimeline; use crate::timeline::{ - event_item::AnyOtherFullStateEventContent, + event_item::{AnyOtherFullStateEventContent, RemoteEventOrigin}, inner::{TimelineEnd, TimelineInnerSettings}, tests::{ReadReceiptMap, TestRoomDataProvider}, MembershipChange, TimelineDetails, TimelineItemContent, TimelineItemKind, VirtualTimelineItem, @@ -63,7 +63,8 @@ async fn test_initial_events() { .make_sync_message_event(*BOB, RoomMessageEventContent::text_plain("B")), ), ], - TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, ) .await; @@ -102,7 +103,7 @@ async fn test_replace_with_initial_events_and_read_marker() { let factory = EventFactory::new(); let ev = factory.text_msg("hey").sender(*ALICE).into_sync(); - timeline.inner.add_events_at(vec![ev], TimelineEnd::Back { from_cache: false }).await; + timeline.inner.add_events_at(vec![ev], TimelineEnd::Back, RemoteEventOrigin::Sync).await; let items = timeline.inner.items().await; assert_eq!(items.len(), 2); @@ -286,7 +287,8 @@ async fn test_dedup_initial() { // … and a new event also came in event_c, ], - TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, ) .await; @@ -322,7 +324,7 @@ async fn test_internal_id_prefix() { timeline .inner - .add_events_at(vec![ev_a, ev_b, ev_c], TimelineEnd::Back { from_cache: false }) + .add_events_at(vec![ev_a, ev_b, ev_c], TimelineEnd::Back, RemoteEventOrigin::Sync) .await; let timeline_items = timeline.inner.items().await; diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index 1fc79592a..3a736b3a4 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -48,6 +48,7 @@ use ruma::{ use super::{ event_handler::TimelineEventKind, + event_item::RemoteEventOrigin, inner::{ReactionAction, TimelineEnd, TimelineInnerSettings}, reactions::ReactionToggleResult, traits::RoomDataProvider, @@ -255,7 +256,9 @@ impl TestTimeline { async fn handle_back_paginated_custom_event(&self, event: Raw) { let timeline_event = TimelineEvent::new(event.cast()); - self.inner.add_events_at(vec![timeline_event], TimelineEnd::Front).await; + self.inner + .add_events_at(vec![timeline_event], TimelineEnd::Front, RemoteEventOrigin::Pagination) + .await; } async fn handle_read_receipts( diff --git a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs index 228b12ab0..1dcdd9979 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs @@ -27,7 +27,8 @@ use ruma::{ use stream_assert::assert_next_matches; use crate::timeline::{ - inner::ReactionAction, + event_item::RemoteEventOrigin, + inner::{ReactionAction, TimelineEnd}, reactions::ReactionToggleResult, tests::{assert_event_is_updated, assert_no_more_updates, TestTimeline}, TimelineItem, @@ -256,7 +257,8 @@ async fn test_initial_reaction_timestamp_is_stored() { RoomMessageEventContent::text_plain("A"), )), ], - crate::timeline::inner::TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, ) .await; diff --git a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs index b876f05e0..6c125a0c1 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs @@ -35,7 +35,10 @@ use ruma::{ use stream_assert::assert_next_matches; use super::TestTimeline; -use crate::timeline::{AnyOtherFullStateEventContent, TimelineDetails, TimelineItemContent}; +use crate::timeline::{ + event_item::RemoteEventOrigin, inner::TimelineEnd, AnyOtherFullStateEventContent, + TimelineDetails, TimelineItemContent, +}; #[async_test] async fn test_redact_state_event() { @@ -152,7 +155,8 @@ async fn test_reaction_redaction_timeline_filter() { .event_builder .make_sync_redacted_message_event(*ALICE, RedactedReactionEventContent::new()), )], - crate::timeline::inner::TimelineEnd::Back { from_cache: false }, + TimelineEnd::Back, + RemoteEventOrigin::Sync, ) .await; // Timeline items are actually empty.