From 3e7d7e8a3178d6fedf5e97cf4110020111a36251 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 26 Nov 2024 16:49:17 +0100 Subject: [PATCH] chore(ui): Rename `TimelineEnd` to `TimelineNewItemPosition`. This patch renames `TimelineEnd` into `TimelineNewItemPosition` for 2 reasons: 1. In the following patches, we will introduce a new variant to insert at a specific index, so the suffix `End` would no longer make sense. 2. It's exactly like `TimelineItemPosition` except that it's used only and strictly only to add **new** items, which is why we can't use `TimelineItemPosition` because it contains the `UpdateDecrypted` variant. This renaming reflects it's only about **new** items. This patch takes the opportunity to move the `RemoteEventOrigin` inside `TimelineNewItemPosition` to simplify method signatures. They always go together. --- crates/matrix-sdk-ui/src/timeline/builder.rs | 8 ++-- .../src/timeline/controller/mod.rs | 34 ++++++------- .../src/timeline/controller/state.rs | 48 ++++++++++--------- .../matrix-sdk-ui/src/timeline/pagination.rs | 4 +- .../matrix-sdk-ui/src/timeline/tests/basic.rs | 23 +++++---- .../matrix-sdk-ui/src/timeline/tests/mod.rs | 12 +++-- .../src/timeline/tests/reactions.rs | 7 ++- .../src/timeline/tests/redaction.rs | 7 ++- 8 files changed, 77 insertions(+), 66 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 2314a1afd3a..4de77f57862 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -31,7 +31,7 @@ use super::{ Error, Timeline, TimelineDropHandle, TimelineFocus, }; use crate::{ - timeline::{controller::TimelineEnd, event_item::RemoteEventOrigin}, + timeline::{controller::TimelineNewItemPosition, event_item::RemoteEventOrigin}, unable_to_decrypt_hook::UtdHookManager, }; @@ -273,9 +273,9 @@ impl TimelineBuilder { inner.add_events_at( events, - TimelineEnd::Back, - match origin { - EventsOrigin::Sync => RemoteEventOrigin::Sync, + TimelineNewItemPosition::End { origin: match origin { + EventsOrigin::Sync => RemoteEventOrigin::Sync, + } } ).await; } diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 6b8605ba7e2..05de8cb3c89 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -52,8 +52,8 @@ use tracing::{ }; pub(super) use self::state::{ - EventMeta, FullEventMeta, PendingEdit, PendingEditKind, TimelineEnd, TimelineMetadata, - TimelineState, TimelineStateTransaction, + EventMeta, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, + TimelineNewItemPosition, TimelineState, TimelineStateTransaction, }; use super::{ event_handler::TimelineEventKind, @@ -404,8 +404,11 @@ impl TimelineController

{ .map_err(PaginationError::Paginator)?, }; - self.add_events_at(pagination.events, TimelineEnd::Front, RemoteEventOrigin::Pagination) - .await; + self.add_events_at( + pagination.events, + TimelineNewItemPosition::Start { origin: RemoteEventOrigin::Pagination }, + ) + .await; Ok(pagination.hit_end_of_timeline) } @@ -428,8 +431,11 @@ impl TimelineController

{ .map_err(PaginationError::Paginator)?, }; - self.add_events_at(pagination.events, TimelineEnd::Back, RemoteEventOrigin::Pagination) - .await; + self.add_events_at( + pagination.events, + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Pagination }, + ) + .await; Ok(pagination.hit_end_of_timeline) } @@ -629,23 +635,14 @@ impl TimelineController

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

{ state .replace_with_remote_events( events, - TimelineEnd::Back, - origin, + TimelineNewItemPosition::End { origin }, &self.room_data_provider, &self.settings, ) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 3668622dacd..4204481b9f4 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -64,16 +64,27 @@ use crate::{ unable_to_decrypt_hook::UtdHookManager, }; -/// Which end of the timeline should an event be added to? -/// -/// This is a simplification of `TimelineItemPosition` which doesn't contain the -/// `Update` variant, when adding a bunch of events at the same time. +/// This is a simplification of [`TimelineItemPosition`] which doesn't contain +/// the [`TimelineItemPosition::UpdateDecrypted`] variant, because it is used +/// only for **new** items. #[derive(Debug)] -pub(crate) enum TimelineEnd { - /// Event should be prepended to the front of the timeline. - Front, - /// Event should be appended to the back of the timeline. - Back, +pub(crate) enum TimelineNewItemPosition { + /// One or more items are prepended to the timeline (i.e. they're the + /// oldest). + Start { origin: RemoteEventOrigin }, + + /// One or more items are appended to the timeline (i.e. they're the most + /// recent). + End { origin: RemoteEventOrigin }, +} + +impl From for TimelineItemPosition { + fn from(value: TimelineNewItemPosition) -> Self { + match value { + TimelineNewItemPosition::Start { origin } => Self::Start { origin }, + TimelineNewItemPosition::End { origin } => Self::End { origin }, + } + } } #[derive(Debug)] @@ -119,8 +130,7 @@ impl TimelineState { pub(super) async fn add_remote_events_at( &mut self, events: Vec>, - position: TimelineEnd, - origin: RemoteEventOrigin, + position: TimelineNewItemPosition, room_data_provider: &P, settings: &TimelineSettings, ) -> HandleManyEventsResult { @@ -130,7 +140,7 @@ impl TimelineState { let mut txn = self.transaction(); let handle_many_res = - txn.add_remote_events_at(events, position, origin, room_data_provider, settings).await; + txn.add_remote_events_at(events, position, room_data_provider, settings).await; txn.commit(); handle_many_res @@ -285,15 +295,13 @@ impl TimelineState { pub(super) async fn replace_with_remote_events( &mut self, events: Vec, - position: TimelineEnd, - origin: RemoteEventOrigin, + position: TimelineNewItemPosition, room_data_provider: &P, settings: &TimelineSettings, ) -> HandleManyEventsResult { let mut txn = self.transaction(); txn.clear(); - let result = - txn.add_remote_events_at(events, position, origin, room_data_provider, settings).await; + let result = txn.add_remote_events_at(events, position, room_data_provider, settings).await; txn.commit(); result } @@ -347,17 +355,13 @@ impl TimelineStateTransaction<'_> { pub(super) async fn add_remote_events_at( &mut self, events: Vec>, - position: TimelineEnd, - origin: RemoteEventOrigin, + position: TimelineNewItemPosition, room_data_provider: &P, settings: &TimelineSettings, ) -> HandleManyEventsResult { let mut total = HandleManyEventsResult::default(); - let position = match position { - TimelineEnd::Front => TimelineItemPosition::Start { origin }, - TimelineEnd::Back => TimelineItemPosition::End { origin }, - }; + let position = position.into(); let mut day_divider_adjuster = DayDividerAdjuster::default(); diff --git a/crates/matrix-sdk-ui/src/timeline/pagination.rs b/crates/matrix-sdk-ui/src/timeline/pagination.rs index d46c90f99c1..afdbb48c1ca 100644 --- a/crates/matrix-sdk-ui/src/timeline/pagination.rs +++ b/crates/matrix-sdk-ui/src/timeline/pagination.rs @@ -26,7 +26,7 @@ use matrix_sdk::event_cache::{ use tracing::{instrument, trace, warn}; use super::Error; -use crate::timeline::{controller::TimelineEnd, event_item::RemoteEventOrigin}; +use crate::timeline::{controller::TimelineNewItemPosition, event_item::RemoteEventOrigin}; impl super::Timeline { /// Add more events to the start of the timeline. @@ -81,7 +81,7 @@ impl super::Timeline { // `matrix_sdk::event_cache::RoomEventCacheUpdate` from // `matrix_sdk::event_cache::RoomPagination::run_backwards`. self.controller - .add_events_at(events, TimelineEnd::Front, RemoteEventOrigin::Pagination) + .add_events_at(events, TimelineNewItemPosition::Start { origin: RemoteEventOrigin::Pagination }) .await; if num_events == 0 && !reached_start { diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index 3d86d2c4383..bcc0d7cd296 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -35,7 +35,7 @@ use stream_assert::assert_next_matches; use super::TestTimeline; use crate::timeline::{ - controller::{TimelineEnd, TimelineSettings}, + controller::{TimelineNewItemPosition, TimelineSettings}, event_item::{AnyOtherFullStateEventContent, RemoteEventOrigin}, tests::{ReadReceiptMap, TestRoomDataProvider}, MembershipChange, TimelineDetails, TimelineItemContent, TimelineItemKind, VirtualTimelineItem, @@ -51,8 +51,7 @@ async fn test_initial_events() { .controller .add_events_at( vec![f.text_msg("A").sender(*ALICE), f.text_msg("B").sender(*BOB)], - TimelineEnd::Back, - RemoteEventOrigin::Sync, + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, ) .await; @@ -91,7 +90,10 @@ async fn test_replace_with_initial_events_and_read_marker() { let f = &timeline.factory; let ev = f.text_msg("hey").sender(*ALICE).into_sync(); - timeline.controller.add_events_at(vec![ev], TimelineEnd::Back, RemoteEventOrigin::Sync).await; + timeline + .controller + .add_events_at(vec![ev], TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }) + .await; let items = timeline.controller.items().await; assert_eq!(items.len(), 2); @@ -317,8 +319,7 @@ async fn test_dedup_initial() { // … and a new event also came in event_c, ], - TimelineEnd::Back, - RemoteEventOrigin::Sync, + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, ) .await; @@ -354,7 +355,10 @@ async fn test_internal_id_prefix() { timeline .controller - .add_events_at(vec![ev_a, ev_b, ev_c], TimelineEnd::Back, RemoteEventOrigin::Sync) + .add_events_at( + vec![ev_a, ev_b, ev_c], + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, + ) .await; let timeline_items = timeline.controller.items().await; @@ -516,7 +520,10 @@ async fn test_replace_with_initial_events_when_batched() { let f = &timeline.factory; let ev = f.text_msg("hey").sender(*ALICE).into_sync(); - timeline.controller.add_events_at(vec![ev], TimelineEnd::Back, RemoteEventOrigin::Sync).await; + timeline + .controller + .add_events_at(vec![ev], TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }) + .await; let (items, mut stream) = timeline.controller.subscribe_batched().await; assert_eq!(items.len(), 2); diff --git a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs index e212a82d6be..943b641b099 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/mod.rs @@ -57,7 +57,7 @@ use ruma::{ use tokio::sync::RwLock; use super::{ - controller::{TimelineEnd, TimelineSettings}, + controller::{TimelineNewItemPosition, TimelineSettings}, event_handler::TimelineEventKind, event_item::RemoteEventOrigin, traits::RoomDataProvider, @@ -237,7 +237,10 @@ impl TestTimeline { async fn handle_live_event(&self, event: impl Into) { let event = event.into(); self.controller - .add_events_at(vec![event], TimelineEnd::Back, RemoteEventOrigin::Sync) + .add_events_at( + vec![event], + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, + ) .await; } @@ -256,7 +259,10 @@ impl TestTimeline { async fn handle_back_paginated_event(&self, event: Raw) { let timeline_event = TimelineEvent::new(event.cast()); self.controller - .add_events_at(vec![timeline_event], TimelineEnd::Front, RemoteEventOrigin::Pagination) + .add_events_at( + vec![timeline_event], + TimelineNewItemPosition::Start { origin: RemoteEventOrigin::Pagination }, + ) .await; } diff --git a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs index d1e92e4ca49..c146abee637 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/reactions.rs @@ -28,8 +28,8 @@ use stream_assert::assert_next_matches; use tokio::time::timeout; use crate::timeline::{ - controller::TimelineEnd, event_item::RemoteEventOrigin, tests::TestTimeline, ReactionStatus, - TimelineEventItemId, TimelineItem, + controller::TimelineNewItemPosition, event_item::RemoteEventOrigin, tests::TestTimeline, + ReactionStatus, TimelineEventItemId, TimelineItem, }; const REACTION_KEY: &str = "👍"; @@ -204,8 +204,7 @@ async fn test_initial_reaction_timestamp_is_stored() { // Event comes next. f.text_msg("A").event_id(&message_event_id).into_sync(), ], - TimelineEnd::Back, - RemoteEventOrigin::Sync, + TimelineNewItemPosition::End { origin: 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 e592660f074..73f3eff0eff 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/redaction.rs @@ -29,8 +29,8 @@ use stream_assert::assert_next_matches; use super::TestTimeline; use crate::timeline::{ - controller::TimelineEnd, event_item::RemoteEventOrigin, AnyOtherFullStateEventContent, - TimelineDetails, TimelineItemContent, + controller::TimelineNewItemPosition, event_item::RemoteEventOrigin, + AnyOtherFullStateEventContent, TimelineDetails, TimelineItemContent, }; #[async_test] @@ -146,8 +146,7 @@ async fn test_reaction_redaction_timeline_filter() { .event_builder .make_sync_redacted_message_event(*ALICE, RedactedReactionEventContent::new()), )], - TimelineEnd::Back, - RemoteEventOrigin::Sync, + TimelineNewItemPosition::End { origin: RemoteEventOrigin::Sync }, ) .await; // Timeline items are actually empty.