Skip to content

Commit

Permalink
timeline: include the remote event's origin in the timeline position/end
Browse files Browse the repository at this point in the history
and don't assume inserting to the end means it's coming from sync — as
it won't be true for forward pagination anymore.
  • Loading branch information
bnjbvr committed Apr 23, 2024
1 parent d986437 commit fa4018b
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 43 deletions.
24 changes: 8 additions & 16 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 9 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/inner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -396,13 +397,16 @@ impl<P: RoomDataProvider> TimelineInner<P> {
&self,
events: Vec<impl Into<SyncTimelineEvent>>,
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) {
Expand Down Expand Up @@ -433,7 +437,8 @@ impl<P: RoomDataProvider> TimelineInner<P> {
state
.add_events_at(
events,
TimelineEnd::Back { from_cache: true },
TimelineEnd::Back,
RemoteEventOrigin::Cache,
&self.room_data_provider,
&self.settings,
)
Expand Down Expand Up @@ -468,7 +473,8 @@ impl<P: RoomDataProvider> TimelineInner<P> {
state
.add_events_at(
vec![event],
TimelineEnd::Back { from_cache: false },
TimelineEnd::Back,
RemoteEventOrigin::Sync,
&self.room_data_provider,
&self.settings,
)
Expand Down
27 changes: 16 additions & 11 deletions crates/matrix-sdk-ui/src/timeline/inner/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -100,6 +97,7 @@ impl TimelineInnerState {
&mut self,
events: Vec<impl Into<SyncTimelineEvent>>,
position: TimelineEnd,
origin: RemoteEventOrigin,
room_data_provider: &P,
settings: &TimelineInnerSettings,
) -> HandleManyEventsResult {
Expand All @@ -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
Expand All @@ -135,7 +133,8 @@ impl TimelineInnerState {

txn.add_events_at(
events,
TimelineEnd::Back { from_cache: false },
TimelineEnd::Back,
RemoteEventOrigin::Sync,
room_data_provider,
settings,
)
Expand Down Expand Up @@ -398,14 +397,15 @@ impl TimelineInnerStateTransaction<'_> {
&mut self,
events: Vec<impl Into<SyncTimelineEvent>>,
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();
Expand Down Expand Up @@ -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) =
Expand Down Expand Up @@ -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;

Expand Down
12 changes: 9 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/pagination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions crates/matrix-sdk-ui/src/timeline/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use ruma::{

use super::{
event_handler::TimelineEventKind,
event_item::RemoteEventOrigin,
inner::{ReactionAction, TimelineEnd, TimelineInnerSettings},
reactions::ReactionToggleResult,
traits::RoomDataProvider,
Expand Down Expand Up @@ -255,7 +256,9 @@ impl TestTimeline {

async fn handle_back_paginated_custom_event(&self, event: Raw<AnyTimelineEvent>) {
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(
Expand Down
6 changes: 4 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/tests/reactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down
8 changes: 6 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/tests/redaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit fa4018b

Please sign in to comment.