Allow focused timelines to replace UTDs after redecryption

crates/matrix-sdk-ui/CHANGELOG.md

@@ -19,6 +19,12 @@ All notable changes to this project will be documented in this file.
   to network mode after a cache miss.
   ([#5930](https://github.com/matrix-org/matrix-rust-sdk/pull/5930))
 
+### Bug Fixes
+
+- Fix the redecryption of events in timelines built using the
+  `TimelineFocus` of `PinnedEvents`, `Thread`, `Event`.
+  ([#5955](https://github.com/matrix-org/matrix-rust-sdk/pull/5955))
+
 ## [0.16.0] - 2025-12-04
 
 ### Features

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

@@ -210,7 +210,7 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
         let event_id = deserialized.event_id().to_owned();
         let txn_id = deserialized.transaction_id().map(ToOwned::to_owned);
 
-        if let Some(action @ TimelineAction::HandleAggregation { .. }) = TimelineAction::from_event(
+        let timeline_action = TimelineAction::from_event(
             deserialized,
             raw_event,
             room_data_provider,
@@ -219,31 +219,52 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
             None,
             None,
         )
-        .await
-        {
-            let encryption_info = event.kind.encryption_info().cloned();
+        .await;
 
-            let sender_profile = room_data_provider.profile_from_user_id(&sender).await;
+        match timeline_action {
+            Some(action @ TimelineAction::AddItem { .. })
+            | Some(action @ TimelineAction::HandleAggregation { .. }) => {
+                let encryption_info = event.kind.encryption_info().cloned();
+                let sender_profile = room_data_provider.profile_from_user_id(&sender).await;
+                let mut ctx = TimelineEventContext {
+                    sender,
+                    sender_profile,
+                    timestamp,
+                    // These are not used when handling an aggregation.
+                    read_receipts: Default::default(),
+                    is_highlighted: false,
+                    flow: Flow::Remote {
+                        event_id: event_id.clone(),
+                        raw_event: event.raw().clone(),
+                        encryption_info,
+                        txn_id,
+                        position,
+                    },
+                    // This field is not used when handling an aggregation.
+                    should_add_new_items: false,
+                };
 
-            let ctx = TimelineEventContext {
-                sender,
-                sender_profile,
-                timestamp,
-                // These are not used when handling an aggregation.
-                read_receipts: Default::default(),
-                is_highlighted: false,
-                flow: Flow::Remote {
-                    event_id: event_id.clone(),
-                    raw_event: event.raw().clone(),
-                    encryption_info,
-                    txn_id,
-                    position,
-                },
-                // This field is not used when handling an aggregation.
-                should_add_new_items: false,
-            };
+                // FIXME: Continuation of the hackjob to get UTDs for focused timelines
+                // working from `handle_remote_aggregations()`.
+                if let TimelineAction::AddItem { .. } = action
+                    && let TimelineItemPosition::UpdateAt { timeline_item_index } = position
+                    && let Some(event) = self.items.get(timeline_item_index)
+                    && event
+                        .as_event()
+                        .map(|e| {
+                            e.content().is_unable_to_decrypt() && e.event_id() == Some(&event_id)
+                        })
+                        .unwrap_or_default()
+                {
+                    // Except when this is an UTD transitioning into a decrypted event.
+                    ctx.should_add_new_items = true;
+                }
 
-            TimelineEventHandler::new(self, ctx).handle_event(date_divider_adjuster, action).await;
+                TimelineEventHandler::new(self, ctx)
+                    .handle_event(date_divider_adjuster, action)
+                    .await;
+            }
+            None => {}
         }
     }
 
@@ -321,6 +342,40 @@ impl<'a, P: RoomDataProvider> TimelineStateTransaction<'a, P> {
                             &mut date_divider_adjuster,
                         )
                         .await;
+                    } else if let Some(event_id) = event.event_id()
+                        && let Some(meta) =
+                            self.items.all_remote_events().get_by_event_id(&event_id)
+                        && let Some(timeline_item_index) = meta.timeline_item_index
+                    {
+                        // FIXME: This branch is a complete hackjob.
+                        //
+                        // The reason being is that this branch is here to handle UTD -> Decrypted
+                        // event remplacements for focused timelines. But this transition should
+                        // naturally happen the same way it happens for unfocused timelines.
+                        //
+                        // Why it doesn't work here? Because the event cache fires out a
+                        // VectorDiff::Set with an index that matches to the cache's view of the
+                        // timeline, which is unfiltered, while the focused timeline will only show
+                        // i.e. pinned events.
+                        //
+                        // The `test_pinned_events_are_decrypted_after_recovering` integration test
+                        // showcases this. The event cache fires out the `Set` with an index of 7,
+                        // but the timeline with the PinnedEvents focus has only 4 items.
+                        //
+                        // This hackjob continues in the `handle_remote_aggregation()` method as we
+                        // can't just handle any `TimelineAction::AddItem` due to:
+                        //  https://github.com/matrix-org/matrix-rust-sdk/pull/4645
+                        //
+                        // Doing so breaks the `test_new_pinned_events_are_not_added_on_sync` test.
+                        //
+                        // Relevant issue: https://github.com/matrix-org/matrix-rust-sdk/issues/5954.
+                        self.handle_remote_aggregation(
+                            event,
+                            TimelineItemPosition::UpdateAt { timeline_item_index },
+                            room_data_provider,
+                            &mut date_divider_adjuster,
+                        )
+                        .await;
                     } else {
                         warn!(
                             event_index,

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

@@ -25,13 +25,15 @@ use matrix_sdk::{
     Client, Room, RoomState, assert_next_with_timeout,
     config::SyncSettings,
     deserialized_responses::{VerificationLevel, VerificationState},
-    encryption::{EncryptionSettings, backups::BackupState},
+    encryption::{
+        BackupDownloadStrategy, EncryptionSettings, backups::BackupState, recovery::RecoveryState,
+    },
     room::{
         edit::EditedContent,
         reply::{EnforceThread, Reply},
     },
     ruma::{
-        MilliSecondsSinceUnixEpoch, OwnedEventId, RoomId, UserId,
+        MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, RoomId, UserId,
         api::client::room::create_room::v3::{Request as CreateRoomRequest, RoomPreset},
         events::{
             InitialStateEvent,
@@ -45,7 +47,7 @@ use matrix_sdk::{
         },
     },
 };
-use matrix_sdk_test::TestResult;
+use matrix_sdk_test::{TestError, TestResult};
 use matrix_sdk_ui::{
     Timeline,
     notification_client::NotificationClient,
@@ -65,7 +67,7 @@ use tokio::{
 };
 use tracing::{debug, warn};
 
-use crate::helpers::TestClientBuilder;
+use crate::helpers::{TestClientBuilder, wait_for_room};
 
 /// Checks that there a timeline update, and returns the EventTimelineItem.
 ///
@@ -345,8 +347,7 @@ async fn test_stale_local_echo_time_abort_edit() {
 async fn test_enabling_backups_retries_decryption() {
     let encryption_settings = EncryptionSettings {
         auto_enable_backups: true,
-        backup_download_strategy:
-            matrix_sdk::encryption::BackupDownloadStrategy::AfterDecryptionFailure,
+        backup_download_strategy: BackupDownloadStrategy::AfterDecryptionFailure,
         ..Default::default()
     };
     let alice = TestClientBuilder::new("alice")
@@ -1081,3 +1082,188 @@ async fn test_local_echo_to_send_event_has_encryption_info() -> TestResult {
 
     Ok(())
 }
+
+async fn prepare_room_with_pinned_events(
+    alice: &Client,
+    recovery_passphrase: &str,
+    number_of_normal_events: usize,
+) -> Result<(OwnedRoomId, OwnedEventId), TestError> {
+    let sync_service = SyncService::builder(alice.clone()).build().await?;
+    sync_service.start().await;
+
+    alice.encryption().wait_for_e2ee_initialization_tasks().await;
+    alice.encryption().recovery().enable().with_passphrase(recovery_passphrase).await?;
+
+    debug!("Creating room…");
+    let room = alice
+        .create_room(assign!(CreateRoomRequest::new(), {
+            is_direct: true,
+            initial_state: vec![],
+            preset: Some(RoomPreset::PrivateChat)
+        }))
+        .await?;
+
+    room.enable_encryption().await?;
+
+    // Send an event to the encrypted room and pin it.
+    let room_id = room.room_id().to_owned();
+    let result =
+        room.send(RoomMessageEventContent::text_plain("It's a secret to everybody")).await?;
+    let event_id = result.response.event_id;
+
+    let timeline = room.timeline().await?;
+    timeline.pin_event(&event_id).await?;
+
+    // Now send a bunch of normal events, this ensures that our pinned event isn't
+    // in the main timeline when we restore things.
+    for i in 0..number_of_normal_events {
+        room.send(RoomMessageEventContent::text_plain(format!("Normal event {i}"))).await?;
+    }
+
+    sync_service.stop().await;
+
+    Ok((room_id, event_id))
+}
+
+async fn test_pinned_events_are_decrypted_after_recovering_with_event_count(
+    event_count: usize,
+) -> TestResult {
+    const RECOVERY_PASSPHRASE: &str = "I am error";
+
+    let encryption_settings = EncryptionSettings {
+        auto_enable_cross_signing: true,
+        auto_enable_backups: true,
+        backup_download_strategy: BackupDownloadStrategy::AfterDecryptionFailure,
+    };
+
+    // Set up sync for user Alice, and create a room.
+    let alice = TestClientBuilder::new("alice")
+        .encryption_settings(encryption_settings)
+        .use_sqlite()
+        .build()
+        .await?;
+    let user_id = alice.user_id().expect("We should have a user ID by now");
+
+    let (room_id, event_id) =
+        prepare_room_with_pinned_events(&alice, RECOVERY_PASSPHRASE, event_count).await?;
+
+    // Now `another_alice` comes into play.
+    let another_alice = TestClientBuilder::with_exact_username(user_id.localpart().to_owned())
+        .encryption_settings(encryption_settings)
+        .use_sqlite()
+        .build()
+        .await?;
+
+    // Alright, we're done with the original Alice.
+    drop(alice);
+
+    // No rooms as of yet, we have not synced with the server as of yet.
+    assert!(another_alice.rooms().is_empty());
+    another_alice.event_cache().subscribe()?;
+
+    let sync_service = SyncService::builder(another_alice.clone()).build().await?;
+    // We need to subscribe to the room, otherwise we won't request the
+    // `m.room.pinned_events` stat event.
+    //
+    // Additionally if we subscribe to the room after we already synced, we'll won't
+    // receive the event, likely due to a Synapse bug.
+    sync_service.room_list_service().subscribe_to_rooms(&[&room_id]).await;
+    sync_service.start().await;
+    another_alice.encryption().wait_for_e2ee_initialization_tasks().await;
+
+    // Let's get the room.
+    let room = wait_for_room(&another_alice, &room_id).await;
+
+    assert!(room.latest_encryption_state().await?.is_encrypted(), "The room should be encrypted");
+
+    // Let's see if the pinned event is there.
+    let pinned_events = room.load_pinned_events().await?.unwrap_or_default();
+    assert!(
+        pinned_events.contains(&event_id),
+        "The pinned event should be found in the pinned events state event"
+    );
+
+    // Let's see if the event is there, it should be a UTD.
+    let event = room.event(&event_id, Default::default()).await?;
+    assert!(event.kind.is_utd());
+
+    // Alright, let's now get to the timeline with a PinnedEvents focus.
+    let pinned_timeline = room
+        .timeline_builder()
+        .with_focus(TimelineFocus::PinnedEvents {
+            max_events_to_load: 100,
+            max_concurrent_requests: 10,
+        })
+        .build()
+        .await?;
+
+    let (items, mut stream) = pinned_timeline.subscribe_filter_map(|i| i.as_event().cloned()).await;
+
+    // If we don't have any items as of yet, wait on the stream.
+    if items.is_empty() {
+        let _ = assert_next_with_timeout!(stream, 5000);
+    }
+
+    // Alright, let's get the event from the timeline.
+    let item = pinned_timeline
+        .item_by_event_id(&event_id)
+        .await
+        .expect("We should have access to the pinned event");
+
+    // Still a UTD.
+    assert!(
+        item.content().is_unable_to_decrypt(),
+        "The pinned event should be a UTD as we didn't recover yet"
+    );
+    assert_eq!(pinned_timeline.items().await.len(), 2);
+
+    // Let's now recover.
+    another_alice.encryption().recovery().recover(RECOVERY_PASSPHRASE).await?;
+    assert_eq!(another_alice.encryption().recovery().state(), RecoveryState::Enabled);
+
+    // The next update for the timeline should replace the UTD item with a decrypted
+    // value.
+    let next_item = assert_next_with_timeout!(stream, 5000);
+
+    assert_let!(VectorDiff::Set { index: 0, value } = next_item);
+    let content = value.content();
+
+    // And we're not a UTD anymore.
+    assert!(!content.is_unable_to_decrypt());
+    let message = content.as_message().expect("The pinned event should be a message");
+    assert_eq!(message.body(), "It's a secret to everybody");
+
+    // And we check that we don't have any more items in the timeline, the UTD item
+    // was indeed replaced.
+    let items = pinned_timeline.items().await;
+    assert_eq!(items.len(), 2);
+
+    Ok(())
+}
+
+/// Test that pinned UTD events, once decrypted by R2D2 (the redecryptor), get
+/// replaced in the timeline with the decrypted variant.
+///
+/// We do this by first creating the pinned events on one Client, called
+/// `alice`. Then another client object is created, called `another_alice`.
+/// `another_alice` initially doesn't have access to the room history.
+///
+/// Only once `another_alice` recovers things and gets access to the backup can
+/// she download the room key to decrypt the pinned event.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn test_pinned_events_are_decrypted_after_recovering_with_event_in_timeline() -> TestResult {
+    test_pinned_events_are_decrypted_after_recovering_with_event_count(0).await
+}
+
+/// Test that pinned UTD events, once decrypted by R2D2 (the redecryptor), get
+/// replaced in the timeline with the decrypted variant even if the pinened UTD
+/// event isn't part of the main timeline and thus wasn't put into the event
+/// cache by the main timeline backpaginating.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+// FIXME: This test is ignored because R2D2 can't decrypt this pinned event as it's never put into
+// the event cache.
+#[ignore]
+async fn test_pinned_events_are_decrypted_after_recovering_with_event_not_in_timeline() -> TestResult
+{
+    test_pinned_events_are_decrypted_after_recovering_with_event_count(30).await
+}