From e4f229978598a65c2a1dc0aaef8075c2b6bd4f9d Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 13 May 2025 18:14:14 +0200 Subject: [PATCH] refactor(timeline): get rid of `FullEventMeta` and replace it with `EventMeta` + function parameters --- .../src/timeline/controller/metadata.rs | 6 ++ .../src/timeline/controller/mod.rs | 2 +- .../src/timeline/controller/read_receipts.rs | 15 +++-- .../src/timeline/controller/state.rs | 29 +-------- .../timeline/controller/state_transaction.rs | 62 ++++++++++--------- 5 files changed, 51 insertions(+), 63 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs b/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs index 65a7e7b3e..238b89c03 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs @@ -372,3 +372,9 @@ pub(in crate::timeline) struct EventMeta { /// remote event, but if it moves, it has an impact on this mapping. pub timeline_item_index: Option, } + +impl EventMeta { + pub fn new(event_id: OwnedEventId, visible: bool) -> Self { + Self { event_id, visible, timeline_item_index: None } + } +} diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 6e16a9251..0d89aacf7 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -59,7 +59,7 @@ pub(super) use self::{ AllRemoteEvents, ObservableItems, ObservableItemsEntry, ObservableItemsTransaction, ObservableItemsTransactionEntry, }, - state::{FullEventMeta, PendingEdit, PendingEditKind, TimelineState}, + state::{PendingEdit, PendingEditKind, TimelineState}, state_transaction::TimelineStateTransaction, }; use super::{ diff --git a/crates/matrix-sdk-ui/src/timeline/controller/read_receipts.rs b/crates/matrix-sdk-ui/src/timeline/controller/read_receipts.rs index 0951ce4e0..974753841 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/read_receipts.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/read_receipts.rs @@ -18,15 +18,15 @@ use futures_core::Stream; use indexmap::IndexMap; use ruma::{ events::receipt::{Receipt, ReceiptEventContent, ReceiptThread, ReceiptType}, - EventId, OwnedEventId, OwnedUserId, UserId, + EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedUserId, UserId, }; use tokio::sync::watch; use tokio_stream::wrappers::WatchStream; use tracing::{debug, error, instrument, trace, warn}; use super::{ - rfind_event_by_id, AllRemoteEvents, FullEventMeta, ObservableItemsTransaction, - RelativePosition, RoomDataProvider, TimelineMetadata, TimelineState, + rfind_event_by_id, AllRemoteEvents, ObservableItemsTransaction, RelativePosition, + RoomDataProvider, TimelineMetadata, TimelineState, }; use crate::timeline::{controller::TimelineStateTransaction, TimelineItem}; @@ -555,9 +555,12 @@ impl TimelineStateTransaction<'_> { /// count, so we need to handle them locally too. For that we create an /// "implicit" read receipt, compared to the "explicit" ones sent by the /// client. - pub(super) fn maybe_add_implicit_read_receipt(&mut self, event_meta: FullEventMeta<'_>) { - let FullEventMeta { event_id, sender, timestamp, .. } = event_meta; - + pub(super) fn maybe_add_implicit_read_receipt( + &mut self, + event_id: &EventId, + sender: Option<&UserId>, + timestamp: Option, + ) { let (Some(user_id), Some(timestamp)) = (sender, timestamp) else { // We cannot add a read receipt if we do not know the user or the timestamp. return; diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index c7b58968c..302d7f323 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -26,7 +26,7 @@ use ruma::{ }, serde::Raw, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, - RoomVersionId, UserId, + RoomVersionId, }; use tracing::{instrument, trace, warn}; @@ -40,7 +40,6 @@ use super::{ traits::RoomDataProvider, Profile, TimelineItem, }, - metadata::EventMeta, observable_items::ObservableItems, DateDividerMode, TimelineFocusKind, TimelineMetadata, TimelineSettings, TimelineStateTransaction, @@ -230,7 +229,7 @@ impl TimelineState { pub(super) fn handle_read_receipts( &mut self, receipt_event_content: ReceiptEventContent, - own_user_id: &UserId, + own_user_id: &ruma::UserId, ) { let mut txn = self.transaction(); txn.handle_explicit_read_receipts(receipt_event_content, own_user_id); @@ -316,27 +315,3 @@ impl std::fmt::Debug for PendingEdit { } } } - -/// Full metadata about an event. -/// -/// Only used to group function parameters. -pub(crate) struct FullEventMeta<'a> { - /// The ID of the event. - pub event_id: &'a EventId, - /// Whether the event is among the timeline items. - pub visible: bool, - /// The sender of the event. - pub sender: Option<&'a UserId>, - /// The timestamp of the event. - pub timestamp: Option, -} - -impl FullEventMeta<'_> { - pub(super) fn base_meta(&self) -> EventMeta { - EventMeta { - event_id: self.event_id.to_owned(), - visible: self.visible, - timeline_item_index: None, - } - } -} diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs index 7c7864a4e..7fd20382b 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs @@ -19,18 +19,19 @@ use itertools::Itertools as _; use matrix_sdk::deserialized_responses::TimelineEvent; use ruma::{ events::AnySyncTimelineEvent, push::Action, serde::Raw, MilliSecondsSinceUnixEpoch, - OwnedEventId, OwnedTransactionId, OwnedUserId, + OwnedEventId, OwnedTransactionId, OwnedUserId, UserId, }; use tracing::{debug, instrument, warn}; use super::{ super::{ - controller::{FullEventMeta, ObservableItemsTransactionEntry}, + controller::ObservableItemsTransactionEntry, date_dividers::DateDividerAdjuster, event_handler::{Flow, TimelineEventContext, TimelineEventHandler, TimelineItemPosition}, event_item::RemoteEventOrigin, traits::RoomDataProvider, }, + metadata::EventMeta, ObservableItems, ObservableItemsTransaction, TimelineFocusKind, TimelineMetadata, TimelineSettings, }; @@ -365,17 +366,17 @@ impl<'a> TimelineStateTransaction<'a> { "Failed to deserialize timeline event: {deserialization_error}" ); - let event_meta = FullEventMeta { - event_id: &event_id, - sender: sender.as_deref(), - timestamp: origin_server_ts, - visible: false, - }; - // Remember the event before returning prematurely. // See [`ObservableItems::all_remote_events`]. - self.add_or_update_remote_event(event_meta, position, room_data_provider, settings) - .await; + self.add_or_update_remote_event( + EventMeta::new(event_id, false), + sender.as_deref(), + origin_server_ts, + position, + room_data_provider, + settings, + ) + .await; None } } @@ -440,16 +441,17 @@ impl<'a> TimelineStateTransaction<'a> { } }; - let event_meta = FullEventMeta { - event_id: &event_id, - sender: Some(&sender), - timestamp: Some(timestamp), - visible: should_add, - }; - // Remember the event. // See [`ObservableItems::all_remote_events`]. - self.add_or_update_remote_event(event_meta, position, room_data_provider, settings).await; + self.add_or_update_remote_event( + EventMeta::new(event_id.clone(), should_add), + Some(&sender), + Some(timestamp), + position, + room_data_provider, + settings, + ) + .await; // Handle the event to create or update a timeline item. if let Some(timeline_action) = timeline_action { @@ -596,27 +598,29 @@ impl<'a> TimelineStateTransaction<'a> { /// This method also adjusts read receipt if needed. async fn add_or_update_remote_event( &mut self, - event_meta: FullEventMeta<'_>, + event_meta: EventMeta, + sender: Option<&UserId>, + timestamp: Option, position: TimelineItemPosition, room_data_provider: &P, settings: &TimelineSettings, ) { + let event_id = event_meta.event_id.clone(); + match position { - TimelineItemPosition::Start { .. } => { - self.items.push_front_remote_event(event_meta.base_meta()) - } + TimelineItemPosition::Start { .. } => self.items.push_front_remote_event(event_meta), TimelineItemPosition::End { .. } => { - self.items.push_back_remote_event(event_meta.base_meta()); + self.items.push_back_remote_event(event_meta); } TimelineItemPosition::At { event_index, .. } => { - self.items.insert_remote_event(event_index, event_meta.base_meta()); + self.items.insert_remote_event(event_index, event_meta); } TimelineItemPosition::UpdateAt { .. } => { if let Some(event) = - self.items.get_remote_event_by_event_id_mut(event_meta.event_id) + self.items.get_remote_event_by_event_id_mut(&event_meta.event_id) { if event.visible != event_meta.visible { event.visible = event_meta.visible; @@ -624,7 +628,7 @@ impl<'a> TimelineStateTransaction<'a> { if settings.track_read_receipts { // Since the event's visibility changed, we need to update the read // receipts of the previous visible event. - self.maybe_update_read_receipts_of_prev_event(event_meta.event_id); + self.maybe_update_read_receipts_of_prev_event(&event_meta.event_id); } } } @@ -639,9 +643,9 @@ impl<'a> TimelineStateTransaction<'a> { | TimelineItemPosition::At { .. } ) { - self.load_read_receipts_for_event(event_meta.event_id, room_data_provider).await; + self.load_read_receipts_for_event(&event_id, room_data_provider).await; - self.maybe_add_implicit_read_receipt(event_meta); + self.maybe_add_implicit_read_receipt(&event_id, sender, timestamp); } }