refactor(timeline): Hold a Decryptor inside TimelineController instead of passing it in when needed

Author: andybalaam

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

@@ -171,6 +171,7 @@ impl TimelineBuilder {
             .unwrap_or_default();
 
         let controller = TimelineController::new(
+            room.clone(),
             room.clone(),
             focus.clone(),
             internal_id_prefix.clone(),

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

@@ -21,16 +21,17 @@ use eyeball_im_util::vector::{FilterMap, VectorObserverExt};
 use futures_core::Stream;
 use imbl::Vector;
 #[cfg(test)]
-use matrix_sdk::crypto::OlmMachine;
+use matrix_sdk::Result;
 use matrix_sdk::{
-    Result, Room,
     deserialized_responses::TimelineEvent,
     event_cache::{RoomEventCache, RoomPaginationStatus},
     paginators::{PaginationResult, Paginator},
     send_queue::{
         LocalEcho, LocalEchoContent, RoomSendQueueUpdate, SendHandle, SendReactionHandle,
     },
 };
+#[cfg(test)]
+use ruma::events::receipt::ReceiptEventContent;
 use ruma::{
     EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, TransactionId, UserId,
     api::client::receipt::create_receipt::v3::ReceiptType as SendReceiptType,
@@ -46,8 +47,6 @@ use ruma::{
     room_version_rules::RoomVersionRules,
     serde::Raw,
 };
-#[cfg(test)]
-use ruma::{OwnedRoomId, RoomId, events::receipt::ReceiptEventContent};
 use tokio::sync::{RwLock, RwLockWriteGuard};
 use tracing::{debug, error, field::debug, info, instrument, trace, warn};
 
@@ -72,7 +71,7 @@ use super::{
 };
 use crate::{
     timeline::{
-        MsgLikeContent, MsgLikeKind, TimelineEventFilterFn,
+        MsgLikeContent, MsgLikeKind, Room, TimelineEventFilterFn,
         algorithms::rfind_event_by_item_id,
         date_dividers::DateDividerAdjuster,
         event_item::TimelineItemHandle,
@@ -161,9 +160,15 @@ pub(super) struct TimelineController<P: RoomDataProvider = Room, D: Decryptor =
 
     /// A [`RoomDataProvider`] implementation, providing data.
     ///
-    /// Useful for testing only; in the real world, it's just a [`Room`].
+    /// In the real world, this is just a [`Room`].
     pub(crate) room_data_provider: P,
 
+    /// A [`Decryptor`] implementation, providing the ability to decrypt
+    /// messages.
+    ///
+    /// In the real world, this is just a [`Room`].
+    pub(crate) decryptor: D,
+
     /// Settings applied to this timeline.
     pub(super) settings: TimelineSettings,
 
@@ -293,6 +298,7 @@ pub fn default_event_filter(event: &AnySyncTimelineEvent, rules: &RoomVersionRul
 impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
     pub(super) fn new(
         room_data_provider: P,
+        decryptor: D,
         focus: TimelineFocus,
         internal_id_prefix: Option<String>,
         unable_to_decrypt_hook: Option<Arc<UtdHookManager>>,
@@ -337,7 +343,7 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
         let decryption_retry_task =
             DecryptionRetryTask::new(state.clone(), room_data_provider.clone());
 
-        Self { state, focus, room_data_provider, settings, decryption_retry_task }
+        Self { state, focus, room_data_provider, decryptor, settings, decryption_retry_task }
     }
 
     /// Initializes the configured focus with appropriate data.
@@ -1099,12 +1105,10 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
         true
     }
 
-    async fn retry_event_decryption_inner(
-        &self,
-        decryptor: D,
-        session_ids: Option<BTreeSet<String>>,
-    ) {
-        self.decryption_retry_task.decrypt(decryptor, session_ids, self.settings.clone()).await;
+    pub(crate) async fn retry_event_decryption_inner(&self, session_ids: Option<BTreeSet<String>>) {
+        self.decryption_retry_task
+            .decrypt(self.decryptor.clone(), session_ids, self.settings.clone())
+            .await;
     }
 
     pub(super) async fn set_sender_profiles_pending(&self) {
@@ -1591,7 +1595,7 @@ impl TimelineController {
 
     #[instrument(skip(self), fields(room_id = ?self.room().room_id()))]
     pub(super) async fn retry_event_decryption(&self, session_ids: Option<BTreeSet<String>>) {
-        self.retry_event_decryption_inner(self.room().clone(), session_ids).await
+        self.retry_event_decryption_inner(session_ids).await
     }
 
     /// Combine the global (event cache) pagination status with the local state
@@ -1625,18 +1629,6 @@ impl TimelineController {
     }
 }
 
-#[cfg(test)]
-impl<P: RoomDataProvider> TimelineController<P, (OlmMachine, OwnedRoomId)> {
-    pub(super) async fn retry_event_decryption_test(
-        &self,
-        room_id: &RoomId,
-        olm_machine: OlmMachine,
-        session_ids: Option<BTreeSet<String>>,
-    ) {
-        self.retry_event_decryption_inner((olm_machine, room_id.to_owned()), session_ids).await
-    }
-}
-
 #[allow(clippy::too_many_arguments)]
 async fn fetch_replied_to_event<P: RoomDataProvider>(
     mut state_guard: RwLockWriteGuard<'_, TimelineState<P>>,

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

@@ -53,7 +53,7 @@ use super::TestTimeline;
 use crate::{
     timeline::{
         EncryptedMessage, MsgLikeContent, MsgLikeKind, TimelineDetails, TimelineItemContent,
-        tests::{TestRoomDataProvider, TestTimelineBuilder},
+        tests::{TestDecryptor, TestRoomDataProvider, TestTimelineBuilder},
     },
     unable_to_decrypt_hook::{UnableToDecryptHook, UnableToDecryptInfo, UtdHookManager},
 };
@@ -90,7 +90,16 @@ async fn test_retry_message_decryption() {
     let client = test_client_builder(None).build().await.unwrap();
     let utd_hook = Arc::new(UtdHookManager::new(hook.clone(), client));
 
-    let timeline = TestTimelineBuilder::new().unable_to_decrypt_hook(utd_hook.clone()).build();
+    let own_user_id = user_id!("@example:morheus.localhost");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .unable_to_decrypt_hook(utd_hook.clone())
+        .decryptor(TestDecryptor::new(
+            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
+            &olm_machine,
+        ))
+        .build();
     let mut stream = timeline.subscribe().await;
 
     let f = &timeline.factory;
@@ -146,19 +155,13 @@ async fn test_retry_message_decryption() {
         assert!(utds[0].time_to_decrypt.is_none());
     }
 
-    let own_user_id = user_id!("@example:morheus.localhost");
     let exported_keys = decrypt_room_key_export(Cursor::new(SESSION_KEY), "1234").unwrap();
 
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
     olm_machine.store().import_exported_room_keys(exported_keys, |_, _| {}).await.unwrap();
 
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
 
     assert_eq!(timeline.controller.items().await.len(), 2);
@@ -198,7 +201,16 @@ async fn test_false_positive_late_decryption_regression() {
         UtdHookManager::new(hook.clone(), client).with_max_delay(Duration::from_millis(500)),
     );
 
-    let timeline = TestTimelineBuilder::new().unable_to_decrypt_hook(utd_hook.clone()).build();
+    let own_user_id = user_id!("@example:morheus.localhost");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .unable_to_decrypt_hook(utd_hook.clone())
+        .decryptor(TestDecryptor::new(
+            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
+            &olm_machine,
+        ))
+        .build();
 
     let f = &timeline.factory;
     timeline
@@ -227,21 +239,14 @@ async fn test_false_positive_late_decryption_regression() {
         )
         .await;
 
-    let own_user_id = user_id!("@example:morheus.localhost");
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
-
     sleep(Duration::from_millis(200)).await;
 
     // Simulate a retry decryption.
     // Due to the regression this was marking the event as successfully decrypted on
     // retry
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
     assert_eq!(timeline.controller.items().await.len(), 2);
 
@@ -287,7 +292,16 @@ async fn test_retry_edit_decryption() {
         rHCyB4ElRjU\n\
         -----END MEGOLM SESSION DATA-----";
 
-    let timeline = TestTimeline::new();
+    let own_user_id = user_id!("@example:morheus.localhost");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .decryptor(TestDecryptor::new(
+            room_id!("!bdsREiCPHyZAPkpXer:morpheus.localhost"),
+            &olm_machine,
+        ))
+        .build();
+
     let f = &timeline.factory;
     let mut stream = timeline.subscribe_events().await;
 
@@ -353,18 +367,9 @@ async fn test_retry_edit_decryption() {
     let mut keys = decrypt_room_key_export(Cursor::new(SESSION1_KEY), "1234").unwrap();
     keys.extend(decrypt_room_key_export(Cursor::new(SESSION2_KEY), "1234").unwrap());
 
-    let own_user_id = user_id!("@example:morheus.localhost");
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
     olm_machine.store().import_exported_room_keys(keys, |_, _| {}).await.unwrap();
 
-    timeline
-        .controller
-        .retry_event_decryption_test(
-            room_id!("!bdsREiCPHyZAPkpXer:morpheus.localhost"),
-            olm_machine,
-            None,
-        )
-        .await;
+    timeline.controller.retry_event_decryption_test(None).await;
 
     // Then first, the first item gets decrypted on its own
     assert_next_matches_with_timeout!(stream, VectorDiff::Set { index: 0, .. });
@@ -419,7 +424,12 @@ async fn test_retry_edit_and_more() {
         )
     }
 
-    let timeline = TestTimeline::new();
+    let olm_machine = OlmMachine::new(user_id!("@jptest:matrix.org"), DEVICE_ID.into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .decryptor(TestDecryptor::new(room_id!("!wFnAUSQbxMcfIMgvNX:flipdot.org"), &olm_machine))
+        .build();
+
     let f = &timeline.factory;
     let mut stream = timeline.subscribe().await;
 
@@ -480,17 +490,12 @@ async fn test_retry_edit_and_more() {
     assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { .. });
     assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { .. });
 
-    let olm_machine = OlmMachine::new(user_id!("@jptest:matrix.org"), DEVICE_ID.into()).await;
     let keys = decrypt_room_key_export(Cursor::new(SESSION_KEY), "testing").unwrap();
     olm_machine.store().import_exported_room_keys(keys, |_, _| {}).await.unwrap();
 
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id!("!wFnAUSQbxMcfIMgvNX:flipdot.org"),
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
 
     // Then first, the original item got decrypted
@@ -536,7 +541,13 @@ async fn test_retry_message_decryption_highlighted() {
         FM8H2PpVkKgrA+tx8LNQD+FWDfp6MyhmEJEvk9r5vU9LtTXtZl4toYvNY0UHUBbZj2xF9U9Z9A\n\
         -----END MEGOLM SESSION DATA-----";
 
-    let timeline = TestTimeline::new();
+    let own_user_id = user_id!("@example:matrix.org");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .decryptor(TestDecryptor::new(room_id!("!rYtFvMGENJleNQVJzb:matrix.org"), &olm_machine))
+        .build();
+
     let f = &timeline.factory;
     let mut stream = timeline.subscribe().await;
 
@@ -583,19 +594,13 @@ async fn test_retry_message_decryption_highlighted() {
     let date_divider = assert_next_matches!(stream, VectorDiff::PushFront { value } => value);
     assert!(date_divider.is_date_divider());
 
-    let own_user_id = user_id!("@example:matrix.org");
     let exported_keys = decrypt_room_key_export(Cursor::new(SESSION_KEY), "1234").unwrap();
 
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
     olm_machine.store().import_exported_room_keys(exported_keys, |_, _| {}).await.unwrap();
 
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id!("!rYtFvMGENJleNQVJzb:matrix.org"),
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
 
     assert_eq!(timeline.controller.items().await.len(), 2);
@@ -615,12 +620,20 @@ async fn test_retry_fetching_encryption_info() {
     let sender = user_id!("@sender:s.co");
     let room_id = room_id!("!room:s.co");
 
+    let own_user_id = user_id!("@me:s.co");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
     // Given when I ask the room for new encryption info for any session, it will
     // say "verified"
     let verified_encryption_info = make_encryption_info(SESSION_ID, VerificationState::Verified);
     let provider =
         TestRoomDataProvider::default().with_encryption_info(SESSION_ID, verified_encryption_info);
-    let timeline = TestTimelineBuilder::new().provider(provider).build();
+
+    let timeline = TestTimelineBuilder::new()
+        .provider(provider)
+        .decryptor(TestDecryptor::new(room_id, &olm_machine))
+        .build();
+
     let f = &timeline.factory;
     let mut stream = timeline.subscribe_events().await;
 
@@ -671,15 +684,9 @@ async fn test_retry_fetching_encryption_info() {
     }
 
     // When we retry the session with ID SESSION_ID
-    let own_user_id = user_id!("@me:s.co");
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id,
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
 
     // Then the event in that session has been updated to be verified
@@ -817,7 +824,16 @@ async fn test_retry_decryption_updates_response() {
         HztoSJUr/2Y\n\
         -----END MEGOLM SESSION DATA-----";
 
-    let timeline = TestTimeline::new();
+    let own_user_id = user_id!("@example:morheus.localhost");
+    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
+
+    let timeline = TestTimelineBuilder::new()
+        .decryptor(TestDecryptor::new(
+            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
+            &olm_machine,
+        ))
+        .build();
+
     let mut stream = timeline.subscribe_events().await;
 
     let original_event_id = event_id!("$original");
@@ -883,20 +899,14 @@ async fn test_retry_decryption_updates_response() {
     }
 
     // Import a room key backup.
-    let own_user_id = user_id!("@example:morheus.localhost");
     let exported_keys = decrypt_room_key_export(Cursor::new(SESSION_KEY), "1234").unwrap();
 
-    let olm_machine = OlmMachine::new(own_user_id, "SomeDeviceId".into()).await;
     olm_machine.store().import_exported_room_keys(exported_keys, |_, _| {}).await.unwrap();
 
     // Retry decrypting the UTD.
     timeline
         .controller
-        .retry_event_decryption_test(
-            room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"),
-            olm_machine,
-            Some(iter::once(SESSION_ID.to_owned()).collect()),
-        )
+        .retry_event_decryption_test(Some(iter::once(SESSION_ID.to_owned()).collect()))
         .await;
 
     // The response is updated.