refactor(timeline): in thread permalinks, avoid back-paginating if the root event is part of the /context response

For thread permalinks, we start with a /context query that will load the
focused event, and maybe a few other in-thread events. In fact, it can
also include the thread root event, which was excluded before. Instead,
we would get a previous-token for back-paginations, which would be used
in /relations. When the request to /relations returns an empty previous
token, that means we've reached the start of the thread, and in this
case we would manually load the root with /event.

We can do better, if the root event is part of the initial /context
response: skip the back-paginations altogether, and make sure to include
the root event in `init_focus()`.

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

@@ -25,7 +25,7 @@ use matrix_sdk::Result;
 use matrix_sdk::{
     deserialized_responses::TimelineEvent,
     event_cache::{RoomEventCache, RoomPaginationStatus},
-    paginators::{PaginationResult, Paginator},
+    paginators::{PaginationResult, PaginationToken, Paginator},
     send_queue::{
         LocalEcho, LocalEchoContent, RoomSendQueueUpdate, SendHandle, SendReactionHandle,
     },
@@ -474,12 +474,14 @@ impl<P: RoomDataProvider> TimelineController<P> {
 
                 let event_paginator = Paginator::new(self.room_data_provider.clone());
 
-                // Start a /context request so we can know if the event is in a thread or not
+                // Start a /context request so we can know if the event is in a thread or not,
+                // and know which kind of pagination we'll be using then.
                 let start_from_result = event_paginator
                     .start_from(event_id, (*num_context_events).into())
                     .await
                     .map_err(PaginationError::Paginator)?;
 
+                // Find the target event, and see if it's part of a thread.
                 let thread_root_event_id = start_from_result
                     .events
                     .iter()
@@ -492,13 +494,28 @@ impl<P: RoomDataProvider> TimelineController<P> {
 
                 let _ = paginator.set(match thread_root_event_id {
                     Some(root_id) => {
-                        let tokens = event_paginator.tokens();
+                        let mut tokens = event_paginator.tokens();
+
+                        // Look if the thread root event is part of the /context response. This
+                        // allows us to spare some backwards pagination with
+                        // /relations.
+                        let includes_root_event = start_from_result.events.iter().any(|event| {
+                            if let Some(id) = event.event_id() { id == root_id } else { false }
+                        });
+
+                        if includes_root_event {
+                            // If we have the root event, there's no need to do back-paginations
+                            // with /relations, since we are at the start of the thread.
+                            tokens.previous = PaginationToken::HitEnd;
+                        }
+
                         AnyPaginator::Threaded(ThreadedEventsLoader::new(
                             self.room_data_provider.clone(),
                             root_id,
                             tokens,
                         ))
                     }
+
                     None => AnyPaginator::Unthreaded {
                         paginator: event_paginator,
                         hide_threaded_events: *hide_threaded_events,
@@ -518,12 +535,13 @@ impl<P: RoomDataProvider> TimelineController<P> {
                     }
 
                     AnyPaginator::Threaded(threaded_events_loader) => {
-                        // We filter only events that are part of the thread, since /context will
-                        // return adjacent events without filters.
+                        // We filter only events that are part of the thread (including the root),
+                        // since /context will return adjacent events without filters.
+                        let thread_root = threaded_events_loader.thread_root_event_id();
                         let events_in_thread = events.into_iter().filter(|event| {
-                            extract_thread_root(event.raw()).is_some_and(|thread_root| {
-                                thread_root == threaded_events_loader.thread_root_event_id()
-                            })
+                            extract_thread_root(event.raw())
+                                .is_some_and(|event_thread_root| event_thread_root == thread_root)
+                                || event.event_id().as_deref() == Some(thread_root)
                         });
 
                         self.replace_with_initial_remote_events(

testing/matrix-sdk-integration-testing/src/tests/timeline.rs

@@ -26,22 +26,35 @@ use matrix_sdk::{
     config::SyncSettings,
     deserialized_responses::{VerificationLevel, VerificationState},
     encryption::{EncryptionSettings, backups::BackupState},
-    room::edit::EditedContent,
+    room::{
+        edit::EditedContent,
+        reply::{EnforceThread, Reply},
+    },
     ruma::{
         MilliSecondsSinceUnixEpoch, OwnedEventId, RoomId, UserId,
         api::client::room::create_room::v3::{Request as CreateRoomRequest, RoomPreset},
         events::{
             InitialStateEvent,
-            room::{encryption::RoomEncryptionEventContent, message::RoomMessageEventContent},
+            room::{
+                encryption::RoomEncryptionEventContent,
+                message::{
+                    ReplyWithinThread, RoomMessageEventContent,
+                    RoomMessageEventContentWithoutRelation,
+                },
+            },
         },
     },
 };
+use matrix_sdk_test::TestResult;
 use matrix_sdk_ui::{
     Timeline,
     notification_client::NotificationClient,
     room_list_service::RoomListLoadingState,
     sync_service::SyncService,
-    timeline::{EventSendState, EventTimelineItem, ReactionStatus, RoomExt, TimelineItem},
+    timeline::{
+        EventSendState, EventTimelineItem, ReactionStatus, RoomExt, TimelineBuilder, TimelineFocus,
+        TimelineItem,
+    },
 };
 use similar_asserts::assert_eq;
 use stream_assert::assert_pending;
@@ -910,3 +923,96 @@ async fn test_new_users_first_messages_dont_warn_about_insecure_device_if_it_is_
         assert_eq!(*index, 10);
     }
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn test_thread_focused_timeline() -> TestResult {
+    // Set up sync for user Alice, and create a room.
+    let alice = TestClientBuilder::new("alice").use_sqlite().build().await?;
+
+    let alice_clone = alice.clone();
+    let alice_sync = spawn(async move {
+        alice_clone.sync(Default::default()).await.expect("sync failed for alice!");
+    });
+
+    // Set up sync for another user Bob.
+    let bob = TestClientBuilder::new("bob").use_sqlite().build().await?;
+
+    // Enable Bob's event cache, to speed up creating the in-thread reply event.
+    bob.event_cache().subscribe().unwrap();
+
+    let bob_clone = bob.clone();
+    let bob_sync = spawn(async move {
+        bob_clone.sync(Default::default()).await.expect("sync failed for bob!");
+    });
+
+    debug!("Creating room…");
+    let alice_room = alice
+        .create_room(assign!(CreateRoomRequest::new(), {
+            is_direct: true,
+        }))
+        .await?;
+
+    alice_room.invite_user_by_id(bob.user_id().unwrap()).await?;
+
+    // Bob joins the room.
+
+    let bob_room = loop {
+        if let Some(room) = bob.get_room(alice_room.room_id()) {
+            room.join().await?;
+            break room;
+        }
+        sleep(Duration::from_millis(10)).await;
+    };
+
+    // Bob sends messages in a thread.
+    let resp = bob_room.send(RoomMessageEventContent::text_plain("Root message")).await?;
+    let thread_root = resp.event_id;
+
+    let thread_reply_event_content = bob_room
+        .make_reply_event(
+            RoomMessageEventContentWithoutRelation::text_plain("First reply"),
+            Reply {
+                event_id: thread_root.clone(),
+                enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No),
+            },
+        )
+        .await?;
+
+    let thread_reply_event_id = bob_room.send(thread_reply_event_content).await?.event_id;
+
+    // Alice creates a timeline focused on the in-thread event, so this will use
+    // /context, and the thread root will be part of the response.
+    let timeline = TimelineBuilder::new(&alice_room)
+        .with_focus(TimelineFocus::Event {
+            target: thread_reply_event_id.clone(),
+            num_context_events: 42,
+            hide_threaded_events: false,
+        })
+        .build()
+        .await?;
+
+    // The timeline should be focused on the thread reply, so it should be
+    // considered threaded.
+    assert!(timeline.is_threaded());
+
+    // Observe that the timeline contains both the root and the reply.
+    let (items, mut stream) = timeline.subscribe().await;
+
+    assert_eq!(items.len(), 3);
+    assert!(items[0].is_date_divider());
+    assert_eq!(items[1].as_event().unwrap().event_id().unwrap(), thread_root);
+    assert_eq!(items[2].as_event().unwrap().event_id().unwrap(), thread_reply_event_id);
+
+    // If Alice paginates backwards, nothing happens on the timeline, as we've hit
+    // the start in the /context response.
+    let hit_start = timeline.paginate_backwards(10).await?;
+    assert!(hit_start);
+
+    sleep(Duration::from_millis(100)).await;
+    assert_pending!(stream);
+
+    alice_sync.abort();
+    bob_sync.abort();
+
+    Ok(())
+}