refactor(ui): Move AllRemoteEvents inside observable_items.

This patch moves `AllRemoteEvents` inside `observable_items` so that
more methods can be made public, which reduce the risk of misuse of this
API. In particular,  the following methods are now strictly private:

- `clear`
- `push_front`
- `push_back`
- `remove`
- `timeline_item_has_been_inserted_at`
- `timeline_item_has_been_removed_at`

In fact, now, all `&mut self` method (except `get_by_event_id_mut`) are
now strictly private!

crates/matrix-sdk-ui/src/timeline/controller/mod.rs

@@ -54,10 +54,10 @@ use tracing::{
 #[cfg(test)]
 pub(super) use self::observable_items::ObservableItems;
 pub(super) use self::{
-    observable_items::ObservableItemsTransaction,
+    observable_items::{AllRemoteEvents, ObservableItemsTransaction},
     state::{
-        AllRemoteEvents, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata,
-        TimelineNewItemPosition, TimelineState, TimelineStateTransaction,
+        FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, TimelineNewItemPosition,
+        TimelineState, TimelineStateTransaction,
     },
 };
 use super::{
@@ -1487,13 +1487,22 @@ impl TimelineController {
 
         match receipt_type {
             SendReceiptType::Read => {
-                if let Some((old_pub_read, _)) =
-                    state.meta.user_receipt(own_user_id, ReceiptType::Read, room).await
+                if let Some((old_pub_read, _)) = state
+                    .meta
+                    .user_receipt(
+                        own_user_id,
+                        ReceiptType::Read,
+                        room,
+                        state.items.all_remote_events(),
+                    )
+                    .await
                 {
                     trace!(%old_pub_read, "found a previous public receipt");
-                    if let Some(relative_pos) =
-                        state.meta.compare_events_positions(&old_pub_read, event_id)
-                    {
+                    if let Some(relative_pos) = state.meta.compare_events_positions(
+                        &old_pub_read,
+                        event_id,
+                        state.items.all_remote_events(),
+                    ) {
                         trace!("event referred to new receipt is {relative_pos:?} the previous receipt");
                         return relative_pos == RelativePosition::After;
                     }
@@ -1506,9 +1515,11 @@ impl TimelineController {
                     state.latest_user_read_receipt(own_user_id, room).await
                 {
                     trace!(%old_priv_read, "found a previous private receipt");
-                    if let Some(relative_pos) =
-                        state.meta.compare_events_positions(&old_priv_read, event_id)
-                    {
+                    if let Some(relative_pos) = state.meta.compare_events_positions(
+                        &old_priv_read,
+                        event_id,
+                        state.items.all_remote_events(),
+                    ) {
                         trace!("event referred to new receipt is {relative_pos:?} the previous receipt");
                         return relative_pos == RelativePosition::After;
                     }
@@ -1517,9 +1528,11 @@ impl TimelineController {
             SendReceiptType::FullyRead => {
                 if let Some(prev_event_id) = self.room_data_provider.load_fully_read_marker().await
                 {
-                    if let Some(relative_pos) =
-                        state.meta.compare_events_positions(&prev_event_id, event_id)
-                    {
+                    if let Some(relative_pos) = state.meta.compare_events_positions(
+                        &prev_event_id,
+                        event_id,
+                        state.items.all_remote_events(),
+                    ) {
                         return relative_pos == RelativePosition::After;
                     }
                 }
@@ -1535,7 +1548,7 @@ impl TimelineController {
     /// it's folded into another timeline item.
     pub(crate) async fn latest_event_id(&self) -> Option<OwnedEventId> {
         let state = self.state.read().await;
-        state.meta.all_remote_events.last().map(|event_meta| &event_meta.event_id).cloned()
+        state.items.all_remote_events().last().map(|event_meta| &event_meta.event_id).cloned()
     }
 }
 

crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs

@@ -12,19 +12,36 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{ops::Deref, sync::Arc};
+use std::{
+    cmp::Ordering,
+    collections::{vec_deque::Iter, VecDeque},
+    ops::Deref,
+    sync::Arc,
+};
 
 use eyeball_im::{
     ObservableVector, ObservableVectorEntries, ObservableVectorEntry, ObservableVectorTransaction,
     ObservableVectorTransactionEntry, VectorSubscriber,
 };
 use imbl::Vector;
+use ruma::EventId;
 
-use super::TimelineItem;
+use super::{state::EventMeta, TimelineItem};
 
 #[derive(Debug)]
 pub struct ObservableItems {
+    /// All timeline items.
+    ///
+    /// Yeah, there are here!
     items: ObservableVector<Arc<TimelineItem>>,
+
+    /// List of all the remote events as received in the timeline, even the ones
+    /// that are discarded in the timeline items.
+    ///
+    /// This is useful to get this for the moment as it helps the `Timeline` to
+    /// compute read receipts and read markers. It also helps to map event to
+    /// timeline item, see [`EventMeta::timeline_item_index`] to learn more.
+    all_remote_events: AllRemoteEvents,
 }
 
 impl ObservableItems {
@@ -34,9 +51,14 @@ impl ObservableItems {
             // sliding-sync tests with 20 events lag. This should still be
             // small enough.
             items: ObservableVector::with_capacity(32),
+            all_remote_events: AllRemoteEvents::default(),
         }
     }
 
+    pub fn all_remote_events(&self) -> &AllRemoteEvents {
+        &self.all_remote_events
+    }
+
     pub fn is_empty(&self) -> bool {
         self.items.is_empty()
     }
@@ -50,7 +72,10 @@ impl ObservableItems {
     }
 
     pub fn transaction(&mut self) -> ObservableItemsTransaction<'_> {
-        ObservableItemsTransaction { items: self.items.transaction() }
+        ObservableItemsTransaction {
+            items: self.items.transaction(),
+            all_remote_events: &mut self.all_remote_events,
+        }
     }
 
     pub fn set(
@@ -85,13 +110,37 @@ impl Deref for ObservableItems {
 #[derive(Debug)]
 pub struct ObservableItemsTransaction<'observable_items> {
     items: ObservableVectorTransaction<'observable_items, Arc<TimelineItem>>,
+    all_remote_events: &'observable_items mut AllRemoteEvents,
 }
 
 impl<'observable_items> ObservableItemsTransaction<'observable_items> {
     pub fn get(&self, timeline_item_index: usize) -> Option<&Arc<TimelineItem>> {
         self.items.get(timeline_item_index)
     }
 
+    pub fn all_remote_events(&self) -> &AllRemoteEvents {
+        &self.all_remote_events
+    }
+
+    pub fn remove_remote_event(&mut self, event_index: usize) -> Option<EventMeta> {
+        self.all_remote_events.remove(event_index)
+    }
+
+    pub fn push_front_remote_event(&mut self, event_meta: EventMeta) {
+        self.all_remote_events.push_front(event_meta);
+    }
+
+    pub fn push_back_remote_event(&mut self, event_meta: EventMeta) {
+        self.all_remote_events.push_back(event_meta);
+    }
+
+    pub fn get_remote_event_by_event_id_mut(
+        &mut self,
+        event_id: &EventId,
+    ) -> Option<&mut EventMeta> {
+        self.all_remote_events.get_by_event_id_mut(event_id)
+    }
+
     pub fn set(
         &mut self,
         timeline_item_index: usize,
@@ -101,23 +150,36 @@ impl<'observable_items> ObservableItemsTransaction<'observable_items> {
     }
 
     pub fn remove(&mut self, timeline_item_index: usize) -> Arc<TimelineItem> {
-        self.items.remove(timeline_item_index)
+        let removed_timeline_item = self.items.remove(timeline_item_index);
+        self.all_remote_events.timeline_item_has_been_removed_at(timeline_item_index);
+
+        removed_timeline_item
     }
 
-    pub fn insert(&mut self, timeline_item_index: usize, timeline_item: Arc<TimelineItem>) {
+    pub fn insert(
+        &mut self,
+        timeline_item_index: usize,
+        timeline_item: Arc<TimelineItem>,
+        event_index: Option<usize>,
+    ) {
         self.items.insert(timeline_item_index, timeline_item);
+        self.all_remote_events.timeline_item_has_been_inserted_at(0, event_index);
     }
 
-    pub fn push_front(&mut self, timeline_item: Arc<TimelineItem>) {
+    pub fn push_front(&mut self, timeline_item: Arc<TimelineItem>, event_index: Option<usize>) {
         self.items.push_front(timeline_item);
+        self.all_remote_events.timeline_item_has_been_inserted_at(0, event_index);
     }
 
-    pub fn push_back(&mut self, timeline_item: Arc<TimelineItem>) {
+    pub fn push_back(&mut self, timeline_item: Arc<TimelineItem>, event_index: Option<usize>) {
         self.items.push_back(timeline_item);
+        self.all_remote_events
+            .timeline_item_has_been_inserted_at(self.items.len().saturating_sub(1), event_index);
     }
 
     pub fn clear(&mut self) {
         self.items.clear();
+        self.all_remote_events.clear();
     }
 
     pub fn for_each<F>(&mut self, f: F)
@@ -140,3 +202,142 @@ impl<'observable_items> Deref for ObservableItemsTransaction<'observable_items>
         &self.items
     }
 }
+
+/// 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 struct AllRemoteEvents(VecDeque<EventMeta>);
+
+impl AllRemoteEvents {
+    /// Return a front-to-back iterator over all remote events.
+    pub fn iter(&self) -> Iter<'_, EventMeta> {
+        self.0.iter()
+    }
+
+    /// Remove all remote events.
+    fn clear(&mut self) {
+        self.0.clear();
+    }
+
+    /// Insert a new remote event at the front of all the others.
+    fn push_front(&mut self, event_meta: EventMeta) {
+        // If there is an associated `timeline_item_index`, shift all the
+        // `timeline_item_index` that come after this one.
+        if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
+            self.increment_all_timeline_item_index_after(new_timeline_item_index);
+        }
+
+        // Push the event.
+        self.0.push_front(event_meta)
+    }
+
+    /// Insert a new remote event at the back of all the others.
+    fn push_back(&mut self, event_meta: EventMeta) {
+        // If there is an associated `timeline_item_index`, shift all the
+        // `timeline_item_index` that come after this one.
+        if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
+            self.increment_all_timeline_item_index_after(new_timeline_item_index);
+        }
+
+        // Push the event.
+        self.0.push_back(event_meta)
+    }
+
+    /// Remove one remote event at a specific index, and return it if it exists.
+    fn remove(&mut self, event_index: usize) -> Option<EventMeta> {
+        // Remove the event.
+        let event_meta = self.0.remove(event_index)?;
+
+        // If there is an associated `timeline_item_index`, shift all the
+        // `timeline_item_index` that come after this one.
+        if let Some(removed_timeline_item_index) = event_meta.timeline_item_index {
+            self.decrement_all_timeline_item_index_after(removed_timeline_item_index);
+        };
+
+        Some(event_meta)
+    }
+
+    /// Return a reference to the last remote event if it exists.
+    pub fn last(&self) -> Option<&EventMeta> {
+        self.0.back()
+    }
+
+    /// Return the index of the last remote event if it exists.
+    pub fn last_index(&self) -> Option<usize> {
+        self.0.len().checked_sub(1)
+    }
+
+    /// Get a mutable reference to a specific remote event by its ID.
+    pub fn get_by_event_id_mut(&mut self, event_id: &EventId) -> Option<&mut EventMeta> {
+        self.0.iter_mut().rev().find(|event_meta| event_meta.event_id == event_id)
+    }
+
+    /// Shift to the right all timeline item indexes that are equal to or
+    /// greater than `new_timeline_item_index`.
+    fn increment_all_timeline_item_index_after(&mut self, new_timeline_item_index: usize) {
+        for event_meta in self.0.iter_mut() {
+            if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
+                if *timeline_item_index >= new_timeline_item_index {
+                    *timeline_item_index += 1;
+                }
+            }
+        }
+    }
+
+    /// Shift to the left all timeline item indexes that are greater than
+    /// `removed_wtimeline_item_index`.
+    fn decrement_all_timeline_item_index_after(&mut self, removed_timeline_item_index: usize) {
+        for event_meta in self.0.iter_mut() {
+            if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
+                if *timeline_item_index > removed_timeline_item_index {
+                    *timeline_item_index -= 1;
+                }
+            }
+        }
+    }
+
+    fn timeline_item_has_been_inserted_at(
+        &mut self,
+        new_timeline_item_index: usize,
+        event_index: Option<usize>,
+    ) {
+        self.increment_all_timeline_item_index_after(new_timeline_item_index);
+
+        if let Some(event_index) = event_index {
+            if let Some(event_meta) = self.0.get_mut(event_index) {
+                event_meta.timeline_item_index = Some(new_timeline_item_index);
+            }
+        }
+    }
+
+    fn timeline_item_has_been_removed_at(&mut self, timeline_item_index_to_remove: usize) {
+        for event_meta in self.0.iter_mut() {
+            let mut remove_timeline_item_index = false;
+
+            // A `timeline_item_index` is removed. Let's shift all indexes that come
+            // after the removed one.
+            if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
+                match (*timeline_item_index).cmp(&timeline_item_index_to_remove) {
+                    Ordering::Equal => {
+                        remove_timeline_item_index = true;
+                    }
+
+                    Ordering::Greater => {
+                        *timeline_item_index -= 1;
+                    }
+
+                    Ordering::Less => {}
+                }
+            }
+
+            // This is the `event_meta` that holds the `timeline_item_index` that is being
+            // removed. So let's clean it.
+            if remove_timeline_item_index {
+                event_meta.timeline_item_index = None;
+            }
+        }
+    }
+}