From b02fd92ad0f018e79cd628501b438ad41dfc2b53 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 3 Dec 2024 10:31:20 +0100 Subject: [PATCH] refactor(ui): Introduce the `AllRemoteEvents` type. This patch replaces `VecDeque` by `AllRemoteEvents` which is a wrapper type around `VecDeque`, but this new type aims at adding semantics API rather than a generic API. It also helps to isolate the use of these values and to know precisely when and how they are used. As a first step, `AllRemoteEvents` implements a generic API to not break the existing code. Next patches will revisit that a little bit step by step. --- .../src/timeline/controller/mod.rs | 2 +- .../src/timeline/controller/state.rs | 55 ++++++++++++++++++- .../src/timeline/read_receipts.rs | 12 ++-- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 29cd9fdd3..e6ca9b54b 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -52,7 +52,7 @@ use tracing::{ }; pub(super) use self::state::{ - EventMeta, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, + AllRemoteEvents, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, TimelineNewItemPosition, TimelineState, TimelineStateTransaction, }; use super::{ diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index feff57277..b040f88d6 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -13,7 +13,10 @@ // limitations under the License. use std::{ - collections::{HashMap, VecDeque}, + collections::{ + vec_deque::{Iter, IterMut}, + HashMap, VecDeque, + }, future::Future, num::NonZeroUsize, sync::{Arc, RwLock}, @@ -715,7 +718,7 @@ impl TimelineStateTransaction<'_> { // Returns its position, in this case. fn event_already_exists( new_event_id: &EventId, - all_remote_events: &VecDeque, + all_remote_events: &AllRemoteEvents, ) -> Option { all_remote_events.iter().position(|EventMeta { event_id, .. }| event_id == new_event_id) } @@ -924,7 +927,7 @@ pub(in crate::timeline) struct TimelineMetadata { /// /// This is useful to get this for the moment as it helps the `Timeline` to /// compute read receipts and read markers. - pub all_remote_events: VecDeque, + pub all_remote_events: AllRemoteEvents, /// State helping matching reactions to their associated events, and /// stashing pending reactions. @@ -1139,6 +1142,52 @@ impl TimelineMetadata { } } +/// A type for all remote events. +/// +/// Having this type helps to know exactly which parts of the code and how they +/// use all remote events. It also helps to give a bit of semantics on top of +/// them. +#[derive(Clone, Debug, Default)] +pub(crate) struct AllRemoteEvents(VecDeque); + +impl AllRemoteEvents { + /// Return a front-to-back iterator over all remote events. + pub fn iter(&self) -> Iter<'_, EventMeta> { + self.0.iter() + } + + /// Return a front-to-back iterator over all remote events as mutable + /// references. + pub fn iter_mut(&mut self) -> IterMut<'_, EventMeta> { + self.0.iter_mut() + } + + /// Remove all remote events. + pub fn clear(&mut self) { + self.0.clear(); + } + + /// Insert a new remote event at the front of all the others. + pub fn push_front(&mut self, event_meta: EventMeta) { + self.0.push_front(event_meta) + } + + /// Insert a new remote event at the back of all the others. + pub fn push_back(&mut self, event_meta: EventMeta) { + self.0.push_back(event_meta) + } + + /// Remove one remote event at a specific index, and return it if it exists. + pub fn remove(&mut self, event_index: usize) -> Option { + self.0.remove(event_index) + } + + /// Return a reference to the last remote event if it exists. + pub fn back(&self) -> Option<&EventMeta> { + self.0.back() + } +} + /// Full metadata about an event. /// /// Only used to group function parameters. diff --git a/crates/matrix-sdk-ui/src/timeline/read_receipts.rs b/crates/matrix-sdk-ui/src/timeline/read_receipts.rs index a97cbdc93..a12ebbb09 100644 --- a/crates/matrix-sdk-ui/src/timeline/read_receipts.rs +++ b/crates/matrix-sdk-ui/src/timeline/read_receipts.rs @@ -12,11 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{ - cmp::Ordering, - collections::{HashMap, VecDeque}, - sync::Arc, -}; +use std::{cmp::Ordering, collections::HashMap, sync::Arc}; use eyeball_im::ObservableVectorTransaction; use futures_core::Stream; @@ -31,7 +27,7 @@ use tracing::{debug, error, warn}; use super::{ controller::{ - EventMeta, FullEventMeta, TimelineMetadata, TimelineState, TimelineStateTransaction, + AllRemoteEvents, FullEventMeta, TimelineMetadata, TimelineState, TimelineStateTransaction, }, traits::RoomDataProvider, util::{rfind_event_by_id, RelativePosition}, @@ -103,7 +99,7 @@ impl ReadReceipts { &mut self, new_receipt: FullReceipt<'_>, is_own_user_id: bool, - all_events: &VecDeque, + all_events: &AllRemoteEvents, timeline_items: &mut ObservableVectorTransaction<'_, Arc>, ) { // Get old receipt. @@ -243,7 +239,7 @@ impl ReadReceipts { pub(super) fn compute_event_receipts( &self, event_id: &EventId, - all_events: &VecDeque, + all_events: &AllRemoteEvents, at_end: bool, ) -> IndexMap { let mut all_receipts = self.get_event_receipts(event_id).cloned().unwrap_or_default();