diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index e7ce10c17c0..fad7dd2fd1a 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -104,12 +104,18 @@ impl Timeline { mime_type: Option, attachment_config: AttachmentConfig, progress_watcher: Option>, + store_in_cache: bool, ) -> Result<(), RoomError> { let mime_str = mime_type.as_ref().ok_or(RoomError::InvalidAttachmentMimeType)?; let mime_type = mime_str.parse::().map_err(|_| RoomError::InvalidAttachmentMimeType)?; - let request = self.inner.send_attachment(filename, mime_type, attachment_config); + let mut request = self.inner.send_attachment(filename, mime_type, attachment_config); + + if store_in_cache { + request.store_in_cache(); + } + if let Some(progress_watcher) = progress_watcher { let mut subscriber = request.subscribe_to_send_progress(); RUNTIME.spawn(async move { @@ -270,6 +276,7 @@ impl Timeline { } } + #[allow(clippy::too_many_arguments)] pub fn send_image( self: Arc, url: String, @@ -278,6 +285,7 @@ impl Timeline { caption: Option, formatted_caption: Option, progress_watcher: Option>, + store_in_cache: bool, ) -> Arc { SendAttachmentJoinHandle::new(RUNTIME.spawn(async move { let base_image_info = BaseImageInfo::try_from(&image_info) @@ -289,11 +297,18 @@ impl Timeline { .caption(caption) .formatted_caption(formatted_caption.map(Into::into)); - self.send_attachment(url, image_info.mimetype, attachment_config, progress_watcher) - .await + self.send_attachment( + url, + image_info.mimetype, + attachment_config, + progress_watcher, + store_in_cache, + ) + .await })) } + #[allow(clippy::too_many_arguments)] pub fn send_video( self: Arc, url: String, @@ -302,6 +317,7 @@ impl Timeline { caption: Option, formatted_caption: Option, progress_watcher: Option>, + store_in_cache: bool, ) -> Arc { SendAttachmentJoinHandle::new(RUNTIME.spawn(async move { let base_video_info: BaseVideoInfo = BaseVideoInfo::try_from(&video_info) @@ -313,8 +329,14 @@ impl Timeline { .caption(caption) .formatted_caption(formatted_caption.map(Into::into)); - self.send_attachment(url, video_info.mimetype, attachment_config, progress_watcher) - .await + self.send_attachment( + url, + video_info.mimetype, + attachment_config, + progress_watcher, + store_in_cache, + ) + .await })) } @@ -325,6 +347,7 @@ impl Timeline { caption: Option, formatted_caption: Option, progress_watcher: Option>, + store_in_cache: bool, ) -> Arc { SendAttachmentJoinHandle::new(RUNTIME.spawn(async move { let base_audio_info: BaseAudioInfo = BaseAudioInfo::try_from(&audio_info) @@ -336,11 +359,18 @@ impl Timeline { .caption(caption) .formatted_caption(formatted_caption.map(Into::into)); - self.send_attachment(url, audio_info.mimetype, attachment_config, progress_watcher) - .await + self.send_attachment( + url, + audio_info.mimetype, + attachment_config, + progress_watcher, + store_in_cache, + ) + .await })) } + #[allow(clippy::too_many_arguments)] pub fn send_voice_message( self: Arc, url: String, @@ -349,6 +379,7 @@ impl Timeline { caption: Option, formatted_caption: Option, progress_watcher: Option>, + store_in_cache: bool, ) -> Arc { SendAttachmentJoinHandle::new(RUNTIME.spawn(async move { let base_audio_info: BaseAudioInfo = BaseAudioInfo::try_from(&audio_info) @@ -361,8 +392,14 @@ impl Timeline { .caption(caption) .formatted_caption(formatted_caption.map(Into::into)); - self.send_attachment(url, audio_info.mimetype, attachment_config, progress_watcher) - .await + self.send_attachment( + url, + audio_info.mimetype, + attachment_config, + progress_watcher, + store_in_cache, + ) + .await })) } @@ -371,6 +408,7 @@ impl Timeline { url: String, file_info: FileInfo, progress_watcher: Option>, + store_in_cache: bool, ) -> Arc { SendAttachmentJoinHandle::new(RUNTIME.spawn(async move { let base_file_info: BaseFileInfo = @@ -379,7 +417,14 @@ impl Timeline { let attachment_config = AttachmentConfig::new().info(attachment_info); - self.send_attachment(url, file_info.mimetype, attachment_config, progress_watcher).await + self.send_attachment( + url, + file_info.mimetype, + attachment_config, + progress_watcher, + store_in_cache, + ) + .await })) } diff --git a/crates/matrix-sdk-ui/src/timeline/futures.rs b/crates/matrix-sdk-ui/src/timeline/futures.rs index 694a2203fb4..91d20e708d9 100644 --- a/crates/matrix-sdk-ui/src/timeline/futures.rs +++ b/crates/matrix-sdk-ui/src/timeline/futures.rs @@ -15,6 +15,7 @@ pub struct SendAttachment<'a> { config: AttachmentConfig, tracing_span: Span, pub(crate) send_progress: SharedObservable, + store_in_cache: bool, } impl<'a> SendAttachment<'a> { @@ -31,6 +32,7 @@ impl<'a> SendAttachment<'a> { config, tracing_span: Span::current(), send_progress: Default::default(), + store_in_cache: false, } } @@ -40,6 +42,14 @@ impl<'a> SendAttachment<'a> { pub fn subscribe_to_send_progress(&self) -> Subscriber { self.send_progress.subscribe() } + + /// Whether the sent attachment should be stored in the cache or not. + /// + /// If set to true, then retrieving the data for the attachment will result + /// in a cache hit immediately after upload. + pub fn store_in_cache(&mut self) { + self.store_in_cache = true; + } } impl<'a> IntoFuture for SendAttachment<'a> { @@ -47,7 +57,9 @@ impl<'a> IntoFuture for SendAttachment<'a> { boxed_into_future!(extra_bounds: 'a); fn into_future(self) -> Self::IntoFuture { - let Self { timeline, path, mime_type, config, tracing_span, send_progress } = self; + let Self { timeline, path, mime_type, config, tracing_span, send_progress, store_in_cache } = + self; + let fut = async move { let filename = path .file_name() @@ -56,12 +68,16 @@ impl<'a> IntoFuture for SendAttachment<'a> { .ok_or(Error::InvalidAttachmentFileName)?; let data = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?; - timeline + let mut fut = timeline .room() .send_attachment(filename, &mime_type, data, config) - .with_send_progress_observable(send_progress) - .await - .map_err(|_| Error::FailedSendingAttachment)?; + .with_send_progress_observable(send_progress); + + if store_in_cache { + fut = fut.store_in_cache(); + } + + fut.await.map_err(|_| Error::FailedSendingAttachment)?; Ok(()) }; diff --git a/crates/matrix-sdk/src/encryption/mod.rs b/crates/matrix-sdk/src/encryption/mod.rs index 0466716b3e6..b59c3e96d0a 100644 --- a/crates/matrix-sdk/src/encryption/mod.rs +++ b/crates/matrix-sdk/src/encryption/mod.rs @@ -456,7 +456,7 @@ impl Client { pub(crate) async fn upload_encrypted_media_and_thumbnail( &self, content_type: &mime::Mime, - data: Vec, + data: &[u8], thumbnail: Option, send_progress: SharedObservable, ) -> Result<(MediaSource, Option, Option>)> { diff --git a/crates/matrix-sdk/src/room/futures.rs b/crates/matrix-sdk/src/room/futures.rs index 396d15cd2e4..f1b91463fe7 100644 --- a/crates/matrix-sdk/src/room/futures.rs +++ b/crates/matrix-sdk/src/room/futures.rs @@ -246,6 +246,7 @@ pub struct SendAttachment<'a> { config: AttachmentConfig, tracing_span: Span, send_progress: SharedObservable, + store_in_cache: bool, } impl<'a> SendAttachment<'a> { @@ -264,6 +265,7 @@ impl<'a> SendAttachment<'a> { config, tracing_span: Span::current(), send_progress: Default::default(), + store_in_cache: false, } } @@ -277,6 +279,15 @@ impl<'a> SendAttachment<'a> { self.send_progress = send_progress; self } + + /// Whether the sent attachment should be stored in the cache or not. + /// + /// If set to true, then retrieving the data for the attachment will result + /// in a cache hit immediately after upload. + pub fn store_in_cache(mut self) -> Self { + self.store_in_cache = true; + self + } } impl<'a> IntoFuture for SendAttachment<'a> { @@ -284,10 +295,26 @@ impl<'a> IntoFuture for SendAttachment<'a> { boxed_into_future!(extra_bounds: 'a); fn into_future(self) -> Self::IntoFuture { - let Self { room, filename, content_type, data, config, tracing_span, send_progress } = self; + let Self { + room, + filename, + content_type, + data, + config, + tracing_span, + send_progress, + store_in_cache, + } = self; let fut = async move { - room.prepare_and_send_attachment(filename, content_type, data, config, send_progress) - .await + room.prepare_and_send_attachment( + filename, + content_type, + data, + config, + send_progress, + store_in_cache, + ) + .await }; Box::pin(fut.instrument(tracing_span)) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index a0bf0ced59b..349b2cb7d64 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -26,6 +26,7 @@ use matrix_sdk_base::{ deserialized_responses::{ RawAnySyncOrStrippedState, RawSyncOrStrippedState, SyncOrStrippedState, TimelineEvent, }, + media::{MediaThumbnailSettings, MediaThumbnailSize}, store::StateStoreExt, ComposerDraft, RoomInfoNotableUpdateReasons, RoomMemberships, StateChanges, StateStoreDataKey, StateStoreDataValue, @@ -1911,6 +1912,13 @@ impl Room { /// media. /// /// * `config` - Metadata and configuration for the attachment. + /// + /// * `send_progress` - An observable to transmit forward progress about the + /// upload. + /// + /// * `store_in_cache` - A boolean defining whether the uploaded media will + /// be stored in the cache immediately after a successful upload. + #[instrument(skip_all)] pub(super) async fn prepare_and_send_attachment<'a>( &'a self, filename: &'a str, @@ -1918,29 +1926,43 @@ impl Room { data: Vec, mut config: AttachmentConfig, send_progress: SharedObservable, + store_in_cache: bool, ) -> Result { self.ensure_room_joined()?; let txn_id = config.txn_id.take(); let mentions = config.mentions.take(); + let thumbnail = config.thumbnail.take(); + + // If necessary, store caching data for the thumbnail ahead of time. + let thumbnail_cache_info = if store_in_cache { + // Use a small closure returning Option to avoid an unnecessary complicated + // chain of map/and_then. + let get_info = || { + let thumbnail = thumbnail.as_ref()?; + let info = thumbnail.info.as_ref()?; + Some((thumbnail.data.clone(), info.height?, info.width?)) + }; + get_info() + } else { + None + }; + #[cfg(feature = "e2e-encryption")] let (media_source, thumbnail_source, thumbnail_info) = if self.is_encrypted().await? { self.client - .upload_encrypted_media_and_thumbnail( - content_type, - data, - config.thumbnail.take(), - send_progress, - ) + .upload_encrypted_media_and_thumbnail(content_type, &data, thumbnail, send_progress) .await? } else { self.client .media() .upload_plain_media_and_thumbnail( content_type, - data, - config.thumbnail.take(), + // TODO: get rid of this clone; wait for Ruma to use `Bytes` or something + // similar. + data.clone(), + thumbnail, send_progress, ) .await? @@ -1950,14 +1972,46 @@ impl Room { let (media_source, thumbnail_source, thumbnail_info) = self .client .media() - .upload_plain_media_and_thumbnail( - content_type, - data, - config.thumbnail.take(), - send_progress, - ) + .upload_plain_media_and_thumbnail(content_type, data.clone(), thumbnail, send_progress) .await?; + if store_in_cache { + let cache_store = self.client.event_cache_store(); + + // A failure to cache shouldn't prevent the whole upload from finishing + // properly, so only log errors during caching. + + debug!("caching the media"); + let request = MediaRequest { source: media_source.clone(), format: MediaFormat::File }; + if let Err(err) = cache_store.add_media_content(&request, data).await { + warn!("unable to cache the media after uploading it: {err}"); + } + + if let Some(((data, height, width), source)) = + thumbnail_cache_info.zip(thumbnail_source.as_ref()) + { + debug!("caching the thumbnail"); + + // Do a best guess at figuring the media request: not animated, cropped + // thumbnail of the original size. + let request = MediaRequest { + source: source.clone(), + format: MediaFormat::Thumbnail(MediaThumbnailSettings { + size: MediaThumbnailSize { + method: ruma::media::Method::Crop, + width, + height, + }, + animated: false, + }), + }; + + if let Err(err) = cache_store.add_media_content(&request, data).await { + warn!("unable to cache the media after uploading it: {err}"); + } + } + } + let msg_type = self.make_attachment_message( content_type, media_source, diff --git a/crates/matrix-sdk/tests/integration/room/attachment/mod.rs b/crates/matrix-sdk/tests/integration/room/attachment/mod.rs index 1bb8aa5d83c..31e2d395958 100644 --- a/crates/matrix-sdk/tests/integration/room/attachment/mod.rs +++ b/crates/matrix-sdk/tests/integration/room/attachment/mod.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::{sync::Mutex, time::Duration}; use matrix_sdk::{ attachment::{ @@ -6,10 +6,15 @@ use matrix_sdk::{ Thumbnail, }, config::SyncSettings, + media::{MediaFormat, MediaRequest, MediaThumbnailSettings, MediaThumbnailSize}, test_utils::logged_in_client_with_server, }; use matrix_sdk_test::{async_test, mocks::mock_encryption_state, test_json, DEFAULT_TEST_ROOM_ID}; -use ruma::{event_id, events::Mentions, owned_user_id, uint}; +use ruma::{ + event_id, + events::{room::MediaSource, Mentions}, + owned_mxc_uri, owned_user_id, uint, +}; use serde_json::json; use wiremock::{ matchers::{body_partial_json, header, method, path, path_regex}, @@ -63,7 +68,7 @@ async fn test_room_attachment_send() { .await .unwrap(); - assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id) + assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id); } #[async_test] @@ -177,6 +182,9 @@ async fn test_room_attachment_send_wrong_info() { async fn test_room_attachment_send_info_thumbnail() { let (client, server) = logged_in_client_with_server().await; + let media_mxc = owned_mxc_uri!("mxc://example.com/media"); + let thumbnail_mxc = owned_mxc_uri!("mxc://example.com/thumbnail"); + Mock::given(method("PUT")) .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) .and(header("authorization", "Bearer 1234")) @@ -191,20 +199,37 @@ async fn test_room_attachment_send_info_thumbnail() { "mimetype":"image/jpeg", "size": 3600, }, - "thumbnail_url": "mxc://example.com/AQwafuaFswefuhsfAFAgsw", + "thumbnail_url": thumbnail_mxc, } }))) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::EVENT_ID)) .mount(&server) .await; + let counter = Mutex::new(0); Mock::given(method("POST")) .and(path("/_matrix/media/r0/upload")) .and(header("authorization", "Bearer 1234")) .and(header("content-type", "image/jpeg")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "content_uri": "mxc://example.com/AQwafuaFswefuhsfAFAgsw" - }))) + .respond_with({ + // First request: return the thumbnail MXC; + // Second request: return the media MXC. + let media_mxc = media_mxc.clone(); + let thumbnail_mxc = thumbnail_mxc.clone(); + move |_: &wiremock::Request| { + let mut counter = counter.lock().unwrap(); + if *counter == 0 { + *counter += 1; + ResponseTemplate::new(200).set_body_json(json!({ + "content_uri": &thumbnail_mxc + })) + } else { + ResponseTemplate::new(200).set_body_json(json!({ + "content_uri": &media_mxc + })) + } + } + }) .expect(2) .mount(&server) .await; @@ -218,6 +243,24 @@ async fn test_room_attachment_send_info_thumbnail() { let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); + // Preconditions: nothing is found in the cache. + let media_request = + MediaRequest { source: MediaSource::Plain(media_mxc), format: MediaFormat::File }; + let thumbnail_request = MediaRequest { + source: MediaSource::Plain(thumbnail_mxc.clone()), + format: MediaFormat::Thumbnail(MediaThumbnailSettings { + size: MediaThumbnailSize { + method: ruma::media::Method::Crop, + width: uint!(480), + height: uint!(360), + }, + animated: false, + }), + }; + let _ = client.media().get_media_content(&media_request, true).await.unwrap_err(); + let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err(); + + // Send the attachment with a thumbnail. let config = AttachmentConfig::with_thumbnail(Thumbnail { data: b"Thumbnail".to_vec(), content_type: mime::IMAGE_JPEG, @@ -236,10 +279,48 @@ async fn test_room_attachment_send_info_thumbnail() { let response = room .send_attachment("image", &mime::IMAGE_JPEG, b"Hello world".to_vec(), config) + .store_in_cache() .await .unwrap(); - assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id) + // The event was sent. + assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id); + + // The media is immediately cached in the cache store, so we don't need to set + // up another mock endpoint for getting the media. + let reloaded = client.media().get_media_content(&media_request, true).await.unwrap(); + assert_eq!(reloaded, b"Hello world"); + + // The thumbnail is cached with sensible defaults. + let reloaded = client.media().get_media_content(&thumbnail_request, true).await.unwrap(); + assert_eq!(reloaded, b"Thumbnail"); + + // The thumbnail can't be retrieved as a file. + let _ = client + .media() + .get_media_content( + &MediaRequest { + source: MediaSource::Plain(thumbnail_mxc.clone()), + format: MediaFormat::File, + }, + true, + ) + .await + .unwrap_err(); + + // But it is not found when requesting it as a thumbnail with a different size. + let thumbnail_request = MediaRequest { + source: MediaSource::Plain(thumbnail_mxc), + format: MediaFormat::Thumbnail(MediaThumbnailSettings { + size: MediaThumbnailSize { + method: ruma::media::Method::Crop, + width: uint!(42), + height: uint!(1337), + }, + animated: false, + }), + }; + let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err(); } #[async_test]