timeline: add doc comments here and there

Author: bnjbvr

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -61,16 +61,28 @@ use super::{
 };
 use crate::{events::SyncTimelineEventWithoutContent, DEFAULT_SANITIZER_MODE};
 
+/// When adding an event, useful information related to the source of the event.
 #[derive(Clone)]
 pub(super) enum Flow {
+    /// The event was locally created.
     Local {
+        /// The transaction id we've used in requests associated to this event.
         txn_id: OwnedTransactionId,
     },
+
+    /// The event has been received from a remote source (sync, pagination,
+    /// etc.). This can be a "remote echo".
     Remote {
+        /// The event identifier as returned by the server.
         event_id: OwnedEventId,
+        /// The transaction id we might have used, if we're the sender of the
+        /// event.
         txn_id: Option<OwnedTransactionId>,
+        /// The raw serialized JSON event.
         raw_event: Raw<AnySyncTimelineEvent>,
+        /// Where should this be added in the timeline.
         position: TimelineItemPosition,
+        /// Should this event actually be added, based on the event filters.
         should_add: bool,
     },
 }
@@ -194,29 +206,53 @@ impl TimelineEventKind {
     }
 }
 
+/// The position at which to perform an update of the timeline with events.
 #[derive(Clone, Copy, Debug)]
 pub(super) enum TimelineItemPosition {
+    /// One or more items are prepended to the timeline (i.e. they're the
+    /// oldest).
+    ///
+    /// Usually this means items coming from back-pagination.
     Start,
+
+    /// One or more items are appended to the timeline (i.e. they're the most
+    /// recent).
     End {
         /// Whether this event is coming from a local cache.
         from_cache: bool,
     },
+
+    /// A single item is updated.
+    ///
+    /// This only happens when a UTD must be replaced with the decrypted event.
     #[cfg(feature = "e2e-encryption")]
     Update(usize),
 }
 
+/// The outcome of handling a single event with
+/// [`TimelineEventHandler::handle_event`].
 #[derive(Default)]
 pub(super) struct HandleEventResult {
+    /// Was some timeline item added?
     pub(super) item_added: bool,
+
+    /// Was some timeline item removed?
+    ///
+    /// This can happen only if there was a UTD item that has been decrypted
+    /// into an item that was filtered out with the event filter.
     #[cfg(feature = "e2e-encryption")]
     pub(super) item_removed: bool,
+
+    /// How many items were updated?
     pub(super) items_updated: u16,
 }
 
-// Bundles together a few things that are needed throughout the different stages
-// of handling an event (figuring out whether it should update an existing
-// timeline item, transforming that item or creating a new one, updating the
-// reactive Vec).
+/// Data necessary to update the timeline, given a single event to handle.
+///
+/// Bundles together a few things that are needed throughout the different
+/// stages of handling an event (figuring out whether it should update an
+/// existing timeline item, transforming that item or creating a new one,
+/// updating the reactive Vec).
 pub(super) struct TimelineEventHandler<'a, 'o> {
     items: &'a mut ObservableVectorTransaction<'o, Arc<TimelineItem>>,
     meta: &'a mut TimelineInnerMetadata,
@@ -769,11 +805,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
 
         let kind: EventTimelineItemKind = match &self.ctx.flow {
             Flow::Local { txn_id } => {
-                let send_state = EventSendState::NotSentYet;
-                let transaction_id = txn_id.to_owned();
-                LocalEventTimelineItem { send_state, transaction_id }
+                LocalEventTimelineItem {
+                    send_state: EventSendState::NotSentYet,
+                    transaction_id: txn_id.to_owned(),
+                }
             }
             .into(),
+
             Flow::Remote { event_id, raw_event, position, .. } => {
                 // Drop pending reactions if the message is redacted.
                 if let TimelineItemContent::RedactedMessage = content {
@@ -784,9 +822,12 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
 
                 let origin = match position {
                     TimelineItemPosition::Start => RemoteEventOrigin::Pagination,
+
                     // We only paginate backwards for now, so End only happens for syncs
                     TimelineItemPosition::End { from_cache: true } => RemoteEventOrigin::Cache,
+
                     TimelineItemPosition::End { from_cache: false } => RemoteEventOrigin::Sync,
+
                     #[cfg(feature = "e2e-encryption")]
                     TimelineItemPosition::Update(idx) => self.items[*idx]
                         .as_event()
@@ -862,10 +903,12 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                     if let Some(day_divider_item) =
                         self.meta.maybe_create_day_divider_from_timestamps(divider_ts, timestamp)
                     {
+                        trace!("Adding day divider (remote - start)");
                         self.items.push_front(day_divider_item);
                     }
                 } else {
                     // The list must always start with a day divider.
+                    trace!("Adding first day divider (remote - start)");
                     let day_divider =
                         self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp));
                     self.items.push_front(day_divider);
@@ -878,18 +921,22 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
             Flow::Remote {
                 position: TimelineItemPosition::End { .. }, txn_id, event_id, ..
             } => {
+                // Look if we already have a corresponding item somewhere, based on the
+                // transaction id (if a local echo) or the event id (if a
+                // duplicate remote event).
                 let result = rfind_event_item(self.items, |it| {
                     txn_id.is_some() && it.transaction_id() == txn_id.as_deref()
                         || it.event_id() == Some(event_id)
                 });
 
                 let mut removed_event_item_id = None;
                 let mut removed_day_divider_id = None;
+
                 if let Some((idx, old_item)) = result {
                     if old_item.as_remote().is_some() {
-                        // Item was previously received from the server. This
-                        // should be very rare normally, but with the sliding-
-                        // sync proxy, it is actually very common.
+                        // Item was previously received from the server. This should be very rare
+                        // normally, but with the sliding- sync proxy, it is actually very
+                        // common.
                         // NOTE: SS proxy workaround.
                         trace!(?item, old_item = ?*old_item, "Received duplicate event");
 
@@ -983,7 +1030,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                     let old_ts = latest_event.timestamp();
 
                     if timestamp_to_date(old_ts) != timestamp_to_date(timestamp) {
-                        trace!("Adding day divider (remote)");
+                        trace!("Adding day divider (remote - end)");
 
                         let id = match removed_day_divider_id {
                             // If a day divider was removed for an item about to be moved and we
@@ -1004,7 +1051,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                     }
                 } else {
                     // If there is no event item, there is no day divider yet.
-                    trace!("Adding first day divider (remote)");
+                    trace!("Adding first day divider (remote - end)");
                     let new_day_divider =
                         self.meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp));
                     if should_push {

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

@@ -75,7 +75,10 @@ pub(super) enum EventTimelineItemKind {
 /// A wrapper that can contain either a transaction id, or an event id.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub enum EventItemIdentifier {
+    /// The item is local, identified by its transaction id (to be used in
+    /// subsequent requests).
     TransactionId(OwnedTransactionId),
+    /// The item is remote, identified by its event id.
     EventId(OwnedEventId),
 }
 
@@ -90,9 +93,8 @@ impl EventTimelineItem {
         Self { sender, sender_profile, timestamp, content, kind }
     }
 
-    /// If the supplied low-level `SyncTimelineEventy` is suitable for use as
-    /// the `latest_event` in a message preview, wrap it as an
-    /// `EventTimelineItem`.
+    /// If the supplied low-level `SyncTimelineEvent` is suitable for use as the
+    /// `latest_event` in a message preview, wrap it as an `EventTimelineItem`.
     pub async fn from_latest_event(
         client: Client,
         room_id: &RoomId,
@@ -347,15 +349,15 @@ impl EventTimelineItem {
     /// yet.
     pub fn original_json(&self) -> Option<&Raw<AnySyncTimelineEvent>> {
         match &self.kind {
-            EventTimelineItemKind::Local(_local_event) => None,
+            EventTimelineItemKind::Local(_) => None,
             EventTimelineItemKind::Remote(remote_event) => remote_event.original_json.as_ref(),
         }
     }
 
     /// Get the raw JSON representation of the latest edit, if any.
     pub fn latest_edit_json(&self) -> Option<&Raw<AnySyncTimelineEvent>> {
         match &self.kind {
-            EventTimelineItemKind::Local(_local_event) => None,
+            EventTimelineItemKind::Local(_) => None,
             EventTimelineItemKind::Remote(remote_event) => remote_event.latest_edit_json.as_ref(),
         }
     }
@@ -445,12 +447,14 @@ impl From<RemoteEventTimelineItem> for EventTimelineItemKind {
 pub struct Profile {
     /// The display name, if set.
     pub display_name: Option<String>,
+
     /// Whether the display name is ambiguous.
     ///
     /// Note that in rooms with lazy-loading enabled, this could be `false` even
     /// though the display name is actually ambiguous if not all member events
     /// have been seen yet.
     pub display_name_ambiguous: bool,
+
     /// The avatar URL, if set.
     pub avatar_url: Option<OwnedMxcUri>,
 }
@@ -531,7 +535,7 @@ mod tests {
     use crate::timeline::TimelineDetails;
 
     #[async_test]
-    async fn latest_message_event_can_be_wrapped_as_a_timeline_item() {
+    async fn test_latest_message_event_can_be_wrapped_as_a_timeline_item() {
         // Given a sync event that is suitable to be used as a latest_event
 
         let room_id = room_id!("!q:x.uk");

crates/matrix-sdk-ui/src/timeline/event_item/reactions.rs

@@ -15,15 +15,15 @@
 use std::ops::Deref;
 
 use indexmap::IndexMap;
-use itertools::Itertools;
+use itertools::Itertools as _;
 use ruma::{OwnedEventId, OwnedTransactionId, UserId};
 
 use super::EventItemIdentifier;
 use crate::timeline::ReactionSenderData;
 
 /// The reactions grouped by key.
 ///
-/// Key: The reaction, usually an emoji.\
+/// Key: The reaction, usually an emoji.
 /// Value: The group of reactions.
 pub type BundledReactions = IndexMap<String, ReactionGroup>;
 

crates/matrix-sdk-ui/src/timeline/event_item/remote.rs

@@ -29,21 +29,27 @@ use super::BundledReactions;
 pub(in crate::timeline) struct RemoteEventTimelineItem {
     /// The event ID.
     pub event_id: OwnedEventId,
+
     /// All bundled reactions about the event.
     pub reactions: BundledReactions,
+
     /// All read receipts for the event.
     ///
     /// The key is the ID of a room member and the value are details about the
     /// read receipt.
     ///
     /// Note that currently this ignores threads.
     pub read_receipts: IndexMap<OwnedUserId, Receipt>,
+
     /// Whether the event has been sent by the the logged-in user themselves.
     pub is_own: bool,
+
     /// Whether the item should be highlighted in the timeline.
     pub is_highlighted: bool,
+
     /// Encryption information.
     pub encryption_info: Option<EncryptionInfo>,
+
     /// JSON of the original event.
     ///
     /// If the event is edited, this *won't* change, instead `latest_edit_json`
@@ -55,8 +61,10 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
     /// thus the whole event is available), but it's not clear whether there is
     /// a clear need for that.
     pub original_json: Option<Raw<AnySyncTimelineEvent>>,
+
     /// JSON of the latest edit to this item.
     pub latest_edit_json: Option<Raw<AnySyncTimelineEvent>>,
+
     /// Where we got this event from: A sync response or pagination.
     pub origin: RemoteEventOrigin,
 }

crates/matrix-sdk-ui/src/timeline/inner/state.rs

@@ -683,7 +683,10 @@ pub(in crate::timeline) struct TimelineInnerMetadata {
     /// are discarded in the timeline items.
     pub all_events: VecDeque<EventMeta>,
 
+    /// The next internal identifier for timeline items, used for both local and
+    /// remote echoes.
     next_internal_id: u64,
+
     pub reactions: Reactions,
     pub poll_pending_events: PollPendingEvents,
     pub fully_read_event: Option<OwnedEventId>,
@@ -706,6 +709,7 @@ pub(in crate::timeline) struct TimelineInnerMetadata {
     /// The hook to call whenever we run into a unable-to-decrypt event.
     pub(crate) unable_to_decrypt_hook: Option<Arc<UtdHookManager>>,
 
+    /// Matrix room version of the timeline's room, or a sensible default.
     pub room_version: RoomVersionId,
 }
 

crates/matrix-sdk-ui/src/timeline/tests/reaction_group.rs

@@ -20,7 +20,7 @@ use ruma::{server_name, uint, user_id, EventId, MilliSecondsSinceUnixEpoch, Owne
 use crate::timeline::{event_item::EventItemIdentifier, ReactionGroup, ReactionSenderData};
 
 #[test]
-fn by_sender() {
+fn test_by_sender() {
     let alice = ALICE.to_owned();
     let bob = BOB.to_owned();
 
@@ -40,7 +40,7 @@ fn by_sender() {
 }
 
 #[test]
-fn by_sender_with_empty_group() {
+fn test_by_sender_with_empty_group() {
     let reaction_group = ReactionGroup::default();
 
     let reactions = reaction_group.by_sender(&ALICE).collect::<Vec<_>>();
@@ -49,7 +49,7 @@ fn by_sender_with_empty_group() {
 }
 
 #[test]
-fn by_sender_with_multiple_users() {
+fn test_by_sender_with_multiple_users() {
     let alice = ALICE.to_owned();
     let bob = BOB.to_owned();
     let carol = user_id!("@carol:other.server");
@@ -76,7 +76,7 @@ fn by_sender_with_multiple_users() {
 /// is still possible for duplicates to be received over federation. And in
 /// that case, clients are expected to treat duplicates as a single annotation.
 #[test]
-fn senders_are_deduplicated() {
+fn test_senders_are_deduplicated() {
     let group = {
         let mut group = ReactionGroup::default();
         insert(&mut group, &ALICE, 3);
@@ -89,7 +89,7 @@ fn senders_are_deduplicated() {
 }
 
 #[test]
-fn timestamps_are_stored() {
+fn test_timestamps_are_stored() {
     let reaction = new_reaction();
     let reaction_2 = new_reaction();
     let timestamp = MilliSecondsSinceUnixEpoch(uint!(0));

crates/matrix-sdk-ui/src/timeline/util.rs

@@ -18,10 +18,13 @@ use chrono::{Datelike, Local, TimeZone};
 use imbl::Vector;
 use ruma::{EventId, MilliSecondsSinceUnixEpoch};
 
+#[cfg(doc)]
+use super::inner::TimelineInnerMetadata;
 use super::{event_item::EventTimelineItemKind, EventTimelineItem, TimelineItem};
 
 pub(super) struct EventTimelineItemWithId<'a> {
     pub inner: &'a EventTimelineItem,
+    /// Internal identifier generated by [`TimelineInnerMetadata`].
     pub internal_id: u64,
 }