Skip to content

Commit

Permalink
chore(ui): Rename TimelineEnd to TimelineNewItemPosition.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Hywan committed Nov 26, 2024
1 parent 2c45316 commit 3e7d7e8
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 66 deletions.
8 changes: 4 additions & 4 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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;
}
Expand Down
34 changes: 15 additions & 19 deletions crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -404,8 +404,11 @@ impl<P: RoomDataProvider> TimelineController<P> {
.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)
}
Expand All @@ -428,8 +431,11 @@ impl<P: RoomDataProvider> TimelineController<P> {
.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)
}
Expand Down Expand Up @@ -629,23 +635,14 @@ impl<P: RoomDataProvider> TimelineController<P> {
pub(super) async fn add_events_at(
&self,
events: Vec<impl Into<SyncTimelineEvent>>,
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) {
Expand Down Expand Up @@ -683,8 +680,7 @@ impl<P: RoomDataProvider> TimelineController<P> {
state
.replace_with_remote_events(
events,
TimelineEnd::Back,
origin,
TimelineNewItemPosition::End { origin },
&self.room_data_provider,
&self.settings,
)
Expand Down
48 changes: 26 additions & 22 deletions crates/matrix-sdk-ui/src/timeline/controller/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TimelineNewItemPosition> for TimelineItemPosition {
fn from(value: TimelineNewItemPosition) -> Self {
match value {
TimelineNewItemPosition::Start { origin } => Self::Start { origin },
TimelineNewItemPosition::End { origin } => Self::End { origin },
}
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -119,8 +130,7 @@ impl TimelineState {
pub(super) async fn add_remote_events_at<P: RoomDataProvider>(
&mut self,
events: Vec<impl Into<SyncTimelineEvent>>,
position: TimelineEnd,
origin: RemoteEventOrigin,
position: TimelineNewItemPosition,
room_data_provider: &P,
settings: &TimelineSettings,
) -> HandleManyEventsResult {
Expand All @@ -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
Expand Down Expand Up @@ -285,15 +295,13 @@ impl TimelineState {
pub(super) async fn replace_with_remote_events<P: RoomDataProvider>(
&mut self,
events: Vec<SyncTimelineEvent>,
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
}
Expand Down Expand Up @@ -347,17 +355,13 @@ impl TimelineStateTransaction<'_> {
pub(super) async fn add_remote_events_at<P: RoomDataProvider>(
&mut self,
events: Vec<impl Into<SyncTimelineEvent>>,
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();

Expand Down
4 changes: 2 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/pagination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 15 additions & 8 deletions crates/matrix-sdk-ui/src/timeline/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

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

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 9 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -237,7 +237,10 @@ impl TestTimeline {
async fn handle_live_event(&self, event: impl Into<SyncTimelineEvent>) {
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;
}

Expand All @@ -256,7 +259,10 @@ impl TestTimeline {
async fn handle_back_paginated_event(&self, event: Raw<AnyTimelineEvent>) {
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;
}

Expand Down
7 changes: 3 additions & 4 deletions crates/matrix-sdk-ui/src/timeline/tests/reactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "👍";
Expand Down Expand Up @@ -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;

Expand Down
7 changes: 3 additions & 4 deletions crates/matrix-sdk-ui/src/timeline/tests/redaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 3e7d7e8

Please sign in to comment.