feat(timeline): insert the start of the timeline in places where it's required

Author: bnjbvr

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

@@ -41,7 +41,7 @@ pub(in crate::timeline) struct TimelineMetadata {
     /// This value is constant over the lifetime of the metadata.
     internal_id_prefix: Option<String>,
 
-    /// The `count` value for the `Skip higher-order stream used by the
+    /// The `count` value for the `Skip` higher-order stream used by the
     /// `TimelineSubscriber`. See its documentation to learn more.
     pub(super) subscriber_skip_count: SkipCount,
 

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

@@ -71,7 +71,7 @@ use super::{
     traits::{Decryptor, RoomDataProvider},
     DateDividerMode, Error, EventSendState, EventTimelineItem, InReplyToDetails, Message,
     PaginationError, Profile, RepliedToEvent, TimelineDetails, TimelineEventItemId, TimelineFocus,
-    TimelineItem, TimelineItemContent, TimelineItemKind,
+    TimelineItem, TimelineItemContent, TimelineItemKind, VirtualTimelineItem,
 };
 use crate::{
     timeline::{
@@ -1309,6 +1309,18 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
             }
         }
     }
+
+    /// Insert a timeline start item at the beginning of the room, if it's
+    /// missing.
+    pub async fn insert_timeline_start_if_missing(&self) {
+        let mut state = self.state.write().await;
+        let mut txn = state.transaction();
+        if txn.items.get(0).is_some_and(|item| item.is_timeline_start()) {
+            return;
+        }
+        txn.items.push_front(txn.meta.new_timeline_item(VirtualTimelineItem::TimelineStart), None);
+        txn.commit();
+    }
 }
 
 impl TimelineController {

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

@@ -34,7 +34,7 @@ use super::{
     ObservableItems, ObservableItemsTransaction, TimelineFocusKind, TimelineMetadata,
     TimelineSettings,
 };
-use crate::events::SyncTimelineEventWithoutContent;
+use crate::{events::SyncTimelineEventWithoutContent, timeline::VirtualTimelineItem};
 
 pub(in crate::timeline) struct TimelineStateTransaction<'a> {
     /// A vector transaction over the items themselves. Holds temporary state
@@ -484,9 +484,16 @@ impl<'a> TimelineStateTransaction<'a> {
         // `VectorDiff::Clear` should be much more efficient to process for
         // subscribers.
         if has_local_echoes {
-            // Remove all remote events and the read marker
+            // Remove all remote events and virtual items that aren't date dividers.
             self.items.for_each(|entry| {
-                if entry.is_remote_event() || entry.is_read_marker() {
+                if entry.is_remote_event()
+                    || entry.as_virtual().is_some_and(|vitem| match vitem {
+                        VirtualTimelineItem::DateDivider(_) => false,
+                        VirtualTimelineItem::ReadMarker | VirtualTimelineItem::TimelineStart => {
+                            true
+                        }
+                    })
+                {
                     ObservableItemsTransactionEntry::remove(entry);
                 }
             });

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

@@ -402,16 +402,21 @@ impl DateDividerAdjuster {
         };
 
         // Assert invariants.
-        // 1. The timeline starts with a date divider.
-        if let Some(item) = items.get(0) {
-            if item.is_read_marker() {
-                if let Some(next_item) = items.get(1) {
-                    if !next_item.is_date_divider() {
-                        report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
+        // 1. The timeline starts with a date divider, if it's not only virtual items.
+        {
+            let mut i = 0;
+            while let Some(item) = items.get(i) {
+                if let Some(virt) = item.as_virtual() {
+                    if matches!(virt, VirtualTimelineItem::DateDivider(_)) {
+                        // We found a date divider among the first virtual items: stop here.
+                        break;
                     }
+                } else {
+                    // We found an event, but we didn't have a date divider: report an error.
+                    report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
+                    break;
                 }
-            } else if !item.is_date_divider() {
-                report.errors.push(DateDividerInsertError::FirstItemNotDateDivider);
+                i += 1;
             }
         }
 

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

@@ -1128,7 +1128,17 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                             (!timeline_item.as_event()?.is_local_echo())
                                 .then_some(timeline_item_index + 1)
                         })
-                        .unwrap_or(0)
+                        .unwrap_or_else(|| {
+                            // We don't have any local echo, so we could insert at 0. However, in
+                            // the case of an insertion caused by a pagination, we
+                            // may have already pushed the start of the timeline item, so we need
+                            // to check if the first item is that, and insert after it otherwise.
+                            if self.items.get(0).is_some_and(|item| item.is_timeline_start()) {
+                                1
+                            } else {
+                                0
+                            }
+                        })
                 });
 
                 trace!(
@@ -1225,6 +1235,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
     /// `transaction_id` of `new_event_timeline_item` if it exists.
     // Note: this method doesn't take `&mut self` to avoid a borrow checker
     // conflict with `TimelineEventHandler::add_item`.
+    // TODO(bnjbvr): refactor
     fn deduplicate_local_timeline_item(
         items: &mut ObservableItemsTransaction<'_>,
         new_event_timeline_item: &mut EventTimelineItem,

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

@@ -106,15 +106,19 @@ impl TimelineItem {
         matches!(&self.kind, TimelineItemKind::Event(_))
     }
 
-    /// Check whether this item is a date divider.
-    #[must_use]
+    /// Check whether this item is a (virtual) date divider.
     pub fn is_date_divider(&self) -> bool {
         matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DateDivider(_)))
     }
 
     pub(crate) fn is_read_marker(&self) -> bool {
         matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker))
     }
+
+    /// Check whether this item is a (virtual) timeline start item.
+    pub fn is_timeline_start(&self) -> bool {
+        matches!(self.kind, TimelineItemKind::Virtual(VirtualTimelineItem::TimelineStart))
+    }
 }
 
 impl Deref for TimelineItem {

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

@@ -75,6 +75,9 @@ impl super::Timeline {
                     // As an exceptional contract, restart the back-pagination if we received an
                     // empty chunk.
                     if outcome.reached_start || !outcome.events.is_empty() {
+                        if outcome.reached_start {
+                            self.controller.insert_timeline_start_if_missing().await;
+                        }
                         return Ok(outcome.reached_start);
                     }
                 }
@@ -111,10 +114,22 @@ impl super::Timeline {
 
         let current_value = status.next_now();
 
+        let controller = self.controller.clone();
         let stream = Box::pin(stream! {
             let status_stream = status.dedup();
+
             pin_mut!(status_stream);
+
             while let Some(state) = status_stream.next().await {
+                match state {
+                    RoomPaginationStatus::Idle { hit_timeline_start } => {
+                        if hit_timeline_start {
+                            controller.insert_timeline_start_if_missing().await;
+                        }
+                    }
+                    RoomPaginationStatus::Paginating => {}
+                }
+
                 yield state;
             }
         });

crates/matrix-sdk-ui/tests/integration/timeline/decryption.rs

@@ -318,21 +318,29 @@ async fn test_an_utd_from_the_event_cache_as_a_paginated_item_is_decrypted() {
     // let's test everything :-).
     assert!(reached_start);
 
+    assert_next_matches_with_timeout!(updates_stream, 250, updates => {
+        assert_eq!(updates.len(), 1, "We get the start of timeline item");
+
+        assert_matches!(&updates[0], VectorDiff::PushFront { value } => {
+            assert!(value.is_timeline_start());
+        });
+    });
+
     // Now, let's look at the updates. We must observe an update reflecting the UTD
     // has entered the `Timeline`.
     assert_next_matches_with_timeout!(updates_stream, 250, updates => {
         assert_eq!(updates.len(), 2, "Expecting 2 updates from the `Timeline`");
 
         // UTD! UTD!
-        assert_matches!(&updates[0], VectorDiff::Insert { index: 1, value: event } => {
+        assert_matches!(&updates[0], VectorDiff::Insert { index: 2, value: event } => {
             assert_matches!(event.as_event(), Some(event) => {
                 assert_eq!(event.event_id().unwrap().as_str(), "$ev0");
                 assert!(event.content().is_unable_to_decrypt());
             });
         });
 
         // UTD is decrypted now!
-        assert_matches!(&updates[1], VectorDiff::Set { index: 1, value: event } => {
+        assert_matches!(&updates[1], VectorDiff::Set { index: 2, value: event } => {
             assert_matches!(event.as_event(), Some(event) => {
                 assert_eq!(event.event_id().unwrap().as_str(), "$ev0");
                 assert_matches!(event.content().as_message(), Some(message) => {

crates/matrix-sdk-ui/tests/integration/timeline/pagination.rs

@@ -158,6 +158,14 @@ async fn test_back_pagination() {
         RoomPaginationStatus::Idle { hit_timeline_start: true }
     );
 
+    // Timeline start is inserted.
+    {
+        assert_let!(Some(timeline_updates) = timeline_stream.next().await);
+        assert_eq!(timeline_updates.len(), 1);
+        assert_let!(VectorDiff::PushFront { value } = &timeline_updates[0]);
+        assert!(value.is_timeline_start());
+    }
+
     assert_pending!(timeline_stream);
 }
 
@@ -493,9 +501,9 @@ async fn test_timeline_reset_while_paginating() {
     // field.
     assert!(hit_start);
 
-    // No events in back-pagination responses, date divider + event from latest
-    // sync is present
-    assert_eq!(timeline.items().await.len(), 2);
+    // No events in back-pagination responses, start of timeline + date divider +
+    // event from latest sync is present
+    assert_eq!(timeline.items().await.len(), 3);
 
     // Make sure both pagination mocks were called
     server.verify().await;
@@ -781,14 +789,20 @@ async fn test_until_num_items_with_empty_chunk() {
         assert!(date_divider.is_date_divider());
     }
 
-    timeline.paginate_backwards(10).await.unwrap();
+    let reached_start = timeline.paginate_backwards(10).await.unwrap();
+    assert!(reached_start);
 
     assert_let!(Some(timeline_updates) = timeline_stream.next().await);
+    assert_eq!(timeline_updates.len(), 1);
+    assert_let!(VectorDiff::PushFront { value } = &timeline_updates[0]);
+    assert!(value.is_timeline_start());
 
     // `m.room.name`: “hello room then”
     {
-        assert_let!(VectorDiff::Insert { index, value: message } = &timeline_updates[0]);
-        assert_eq!(*index, 1);
+        assert_let!(Some(timeline_updates) = timeline_stream.next().await);
+        assert_eq!(timeline_updates.len(), 1);
+
+        assert_let!(VectorDiff::Insert { index: 2, value: message } = &timeline_updates[0]);
         assert_let!(TimelineItemContent::Message(msg) = message.as_event().unwrap().content());
         assert_let!(MessageType::Text(text) = msg.msgtype());
         assert_eq!(text.body, "hello room then");
@@ -1073,13 +1087,21 @@ async fn test_lazy_back_pagination() {
 
         assert!(hit_end_of_timeline);
 
+        // The start of the timeline is inserted as its own timeline update.
+        assert_timeline_stream! {
+            [timeline_stream]
+            prepend --- timeline start ---;
+        };
+
         // Receive 3 new items.
         //
-        // They are inserted after the date divider, hence the indices 1 and 2.
+        // They are inserted after the date divider and start of timeline, hence the
+        // indices 2 and 3.
+
         assert_timeline_stream! {
             [timeline_stream]
-            insert[1] "$ev201";
-            insert[2] "$ev200";
+            insert[2] "$ev201";
+            insert[3] "$ev200";
         };
         // So cool.
         assert_pending!(timeline_stream);

crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs

@@ -1198,18 +1198,20 @@ async fn test_no_duplicate_receipt_after_backpagination() {
         .mount()
         .await;
 
-    timeline.paginate_backwards(42).await.unwrap();
+    let reached_start = timeline.paginate_backwards(42).await.unwrap();
+    assert!(reached_start);
 
     yield_now().await;
 
     // Check that the receipts are at the correct place.
     let timeline_items = timeline.items().await;
-    assert_eq!(timeline_items.len(), 3);
+    assert_eq!(timeline_items.len(), 4);
 
-    assert!(timeline_items[0].is_date_divider());
+    assert!(timeline_items[0].is_timeline_start());
+    assert!(timeline_items[1].is_date_divider());
 
     {
-        let event1 = timeline_items[1].as_event().unwrap();
+        let event1 = timeline_items[2].as_event().unwrap();
         // Sanity check: this is the edited event from Alice.
         assert_eq!(event1.event_id().unwrap(), eid1);
 
@@ -1232,7 +1234,7 @@ async fn test_no_duplicate_receipt_after_backpagination() {
     }
 
     {
-        let event2 = timeline_items[2].as_event().unwrap();
+        let event2 = timeline_items[3].as_event().unwrap();
 
         // Sanity check: this is Bob's event.
         assert_eq!(event2.event_id().unwrap(), eid2);

crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs

@@ -145,6 +145,32 @@ macro_rules! assert_timeline_stream {
         )
     };
 
+
+    // `prepend --- timeline start ---`
+    ( @_ [ $iterator:ident ] [ prepend --- timeline start --- ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => {
+        assert_timeline_stream!(
+            @_
+            [ $iterator ]
+            [ $( $rest )* ]
+            [
+                $( $accumulator )*
+                {
+                    assert_matches!(
+                        $iterator .next(),
+                        Some(eyeball_im::VectorDiff::PushFront { value }) => {
+                            assert_matches!(
+                                &**value,
+                                matrix_sdk_ui::timeline::TimelineItemKind::Virtual(
+                                    matrix_sdk_ui::timeline::VirtualTimelineItem::TimelineStart
+                                ) => {}
+                            );
+                        }
+                    );
+                }
+            ]
+        )
+    };
+
     // `prepend "$event_id"`
     ( @_ [ $iterator:ident ] [ prepend $event_id:literal ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => {
         assert_timeline_stream!(