Skip to content

timeline: include the remote event's origin in the timeline position/end #3350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading