From 0c3161b4a23bc00532c4ce407142ee7c785046cc Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 25 Apr 2025 08:37:52 +0200 Subject: [PATCH 01/23] feat(send_queue): Implement sending of MSC4274 galleries Signed-off-by: Johannes Marbach --- crates/matrix-sdk-base/src/store/mod.rs | 2 +- .../matrix-sdk-base/src/store/send_queue.rs | 29 ++ crates/matrix-sdk/CHANGELOG.md | 4 + crates/matrix-sdk/Cargo.toml | 2 +- crates/matrix-sdk/src/attachment.rs | 92 ++++ crates/matrix-sdk/src/room/edit.rs | 6 + crates/matrix-sdk/src/room/mod.rs | 99 +++- crates/matrix-sdk/src/send_queue/mod.rs | 275 +++++++++++ crates/matrix-sdk/src/send_queue/upload.rs | 404 +++++++++++++++- .../tests/integration/send_queue.rs | 456 ++++++++++++++++++ 10 files changed, 1363 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index df1c867bd3c..e3a03431d3c 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -72,7 +72,7 @@ mod send_queue; #[cfg(any(test, feature = "testing"))] pub use self::integration_tests::StateStoreIntegrationTests; #[cfg(feature = "unstable-msc4274")] -pub use self::send_queue::AccumulatedSentMediaInfo; +pub use self::send_queue::{AccumulatedSentMediaInfo, FinishGalleryItemInfo}; pub use self::{ memory_store::MemoryStore, send_queue::{ diff --git a/crates/matrix-sdk-base/src/store/send_queue.rs b/crates/matrix-sdk-base/src/store/send_queue.rs index 1d8f3e8c972..e08cf941d6d 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -258,6 +258,19 @@ pub enum DependentQueuedRequestKind { /// Information about the thumbnail, if present. thumbnail_info: Option, }, + + /// Finish a gallery upload by updating references to the media cache and + /// sending the final gallery event with the remote MXC URIs. + #[cfg(feature = "unstable-msc4274")] + FinishGallery { + /// Local echo for the event (containing the local MXC URIs). + /// + /// `Box` the local echo so that it reduces the size of the whole enum. + local_echo: Box, + + /// Metadata about the gallery items + item_infos: Vec, + }, } /// If parent_is_thumbnail_upload is missing, we assume the request is for a @@ -285,6 +298,17 @@ pub struct FinishUploadThumbnailInfo { pub height: Option, } +#[cfg(feature = "unstable-msc4274")] +#[derive(Clone, Debug, Serialize, Deserialize)] +/// Detailed record about a file and thumbnail used when finishing a gallery +/// upload. +pub struct FinishGalleryItemInfo { + /// Transaction id for the file upload. + pub file_upload: OwnedTransactionId, + /// Information about the thumbnail, if present. + pub thumbnail_info: Option, +} + /// A transaction id identifying a [`DependentQueuedRequest`] rather than its /// parent [`QueuedRequest`]. /// @@ -442,6 +466,11 @@ impl DependentQueuedRequest { // This one graduates into a new media event. true } + #[cfg(feature = "unstable-msc4274")] + DependentQueuedRequestKind::FinishGallery { .. } => { + // This one graduates into a new gallery event. + true + } } } } diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index fa17b56c94e..c0e08c999f8 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. ### Features +- `RoomSendQueue::send_gallery` has been added to allow sending MSC4274-style media galleries + via the send queue under the `unstable-msc4274` feature. + ([#4977](https://github.com/matrix-org/matrix-rust-sdk/pull/4977)) + ### Bug fixes ### Refactor diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml index 979b4ff08e7..981b34c0eda 100644 --- a/crates/matrix-sdk/Cargo.toml +++ b/crates/matrix-sdk/Cargo.toml @@ -51,7 +51,7 @@ docsrs = ["e2e-encryption", "sqlite", "indexeddb", "sso-login", "qrcode"] experimental-share-history-on-invite = [] # Add support for inline media galleries via msgtypes -unstable-msc4274 = ["matrix-sdk-base/unstable-msc4274"] +unstable-msc4274 = ["ruma/unstable-msc4274", "matrix-sdk-base/unstable-msc4274"] [dependencies] anyhow = { workspace = true, optional = true } diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index 7b3f0473e5b..81367da3061 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -276,3 +276,95 @@ impl AttachmentConfig { self } } + +/// Configuration for sending a gallery. +#[cfg(feature = "unstable-msc4274")] +#[derive(Debug, Default)] +pub struct GalleryConfig { + pub(crate) txn_id: Option, + pub(crate) caption: Option, + pub(crate) formatted_caption: Option, + pub(crate) mentions: Option, + pub(crate) reply: Option, +} + +#[cfg(feature = "unstable-msc4274")] +impl GalleryConfig { + /// Create a new empty `GalleryConfig`. + pub fn new() -> Self { + Self::default() + } + + /// Set the transaction ID to send. + /// + /// # Arguments + /// + /// * `txn_id` - A unique ID that can be attached to a `MessageEvent` held + /// in its unsigned field as `transaction_id`. If not given, one is + /// created for the message. + #[must_use] + pub fn txn_id(mut self, txn_id: &TransactionId) -> Self { + self.txn_id = Some(txn_id.to_owned()); + self + } + + /// Set the optional caption + /// + /// # Arguments + /// + /// * `caption` - The optional caption + pub fn caption(mut self, caption: Option) -> Self { + self.caption = caption; + self + } + + /// Set the optional formatted caption + /// + /// # Arguments + /// + /// * `formatted_caption` - The optional formatted caption + pub fn formatted_caption(mut self, formatted_caption: Option) -> Self { + self.formatted_caption = formatted_caption; + self + } + + /// Set the mentions of the message. + /// + /// # Arguments + /// + /// * `mentions` - The mentions of the message + pub fn mentions(mut self, mentions: Option) -> Self { + self.mentions = mentions; + self + } + + /// Set the reply information of the message. + /// + /// # Arguments + /// + /// * `reply` - The reply information of the message + pub fn reply(mut self, reply: Option) -> Self { + self.reply = reply; + self + } +} + +#[cfg(feature = "unstable-msc4274")] +#[derive(Debug)] +/// Metadata for a gallery item +pub struct GalleryItemInfo { + /// The filename + pub filename: String, + /// The mime type + pub content_type: mime::Mime, + /// The binary data + pub data: Vec, + /// The attachment info + pub attachment_info: AttachmentInfo, + /// The caption + pub caption: Option, + /// The formatted caption + pub formatted_caption: Option, + /// The thumbnail + pub thumbnail: Option, +} diff --git a/crates/matrix-sdk/src/room/edit.rs b/crates/matrix-sdk/src/room/edit.rs index 53e58aba685..ba5a38535fc 100644 --- a/crates/matrix-sdk/src/room/edit.rs +++ b/crates/matrix-sdk/src/room/edit.rs @@ -269,6 +269,12 @@ pub(crate) fn update_media_caption( event.formatted = formatted_caption; true } + #[cfg(feature = "unstable-msc4274")] + MessageType::Gallery(event) => { + event.body = caption.unwrap_or_default(); + event.formatted = formatted_caption; + true + } MessageType::Image(event) => { set_caption!(event, caption); event.formatted = formatted_caption; diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index b86ef107606..5238dcb5f06 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -56,6 +56,8 @@ use matrix_sdk_common::{ }; use mime::Mime; use reply::Reply; +#[cfg(feature = "unstable-msc4274")] +use ruma::events::room::message::GalleryItemType; #[cfg(feature = "e2e-encryption")] use ruma::events::{ room::encrypted::OriginalSyncRoomEncryptedEvent, AnySyncMessageLikeEvent, AnySyncTimelineEvent, @@ -2160,7 +2162,7 @@ impl Room { } let content = self - .make_attachment_event( + .make_message_event( self.make_attachment_type( content_type, filename, @@ -2276,7 +2278,7 @@ impl Room { /// Creates the [`RoomMessageEventContent`] based on the message type, /// mentions and reply information. - pub(crate) async fn make_attachment_event( + pub(crate) async fn make_message_event( &self, msg_type: MessageType, mentions: Option, @@ -2294,6 +2296,99 @@ impl Room { Ok(content) } + /// Creates the inner [`GalleryItemType`] for an already-uploaded media file + /// provided by its source. + #[cfg(feature = "unstable-msc4274")] + #[allow(clippy::too_many_arguments)] + pub(crate) fn make_gallery_item_type( + &self, + content_type: &Mime, + filename: String, + source: MediaSource, + caption: Option, + formatted_caption: Option, + info: Option, + thumbnail: Option<(MediaSource, Box)>, + ) -> GalleryItemType { + // If caption is set, use it as body, and filename as the file name; otherwise, + // body is the filename, and the filename is not set. + // https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md + let (body, filename) = match caption { + Some(caption) => (caption, Some(filename)), + None => (filename, None), + }; + + let (thumbnail_source, thumbnail_info) = thumbnail.unzip(); + + match content_type.type_() { + mime::IMAGE => { + let info = assign!(info.map(ImageInfo::from).unwrap_or_default(), { + mimetype: Some(content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(ImageMessageEventContent::new(body, source), { + info: Some(Box::new(info)), + formatted: formatted_caption, + filename + }); + GalleryItemType::Image(content) + } + + mime::AUDIO => { + let mut content = assign!(AudioMessageEventContent::new(body, source), { + formatted: formatted_caption, + filename + }); + + if let Some(AttachmentInfo::Voice { audio_info, waveform: Some(waveform_vec) }) = + &info + { + if let Some(duration) = audio_info.duration { + let waveform = waveform_vec.iter().map(|v| (*v).into()).collect(); + content.audio = + Some(UnstableAudioDetailsContentBlock::new(duration, waveform)); + } + content.voice = Some(UnstableVoiceContentBlock::new()); + } + + let mut audio_info = info.map(AudioInfo::from).unwrap_or_default(); + audio_info.mimetype = Some(content_type.as_ref().to_owned()); + let content = content.info(Box::new(audio_info)); + + GalleryItemType::Audio(content) + } + + mime::VIDEO => { + let info = assign!(info.map(VideoInfo::from).unwrap_or_default(), { + mimetype: Some(content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(VideoMessageEventContent::new(body, source), { + info: Some(Box::new(info)), + formatted: formatted_caption, + filename + }); + GalleryItemType::Video(content) + } + + _ => { + let info = assign!(info.map(FileInfo::from).unwrap_or_default(), { + mimetype: Some(content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(FileMessageEventContent::new(body, source), { + info: Some(Box::new(info)), + formatted: formatted_caption, + filename, + }); + GalleryItemType::File(content) + } + } + } + /// Update the power levels of a select set of users of this room. /// /// Issue a `power_levels` state event request to the server, changing the diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index a5b223a455a..b23b8ac9456 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -138,6 +138,8 @@ use std::{ }; use as_variant::as_variant; +#[cfg(feature = "unstable-msc4274")] +use matrix_sdk_base::store::FinishGalleryItemInfo; use matrix_sdk_base::{ event_cache::store::EventCacheStoreError, media::MediaRequestParameters, @@ -1284,6 +1286,199 @@ impl QueueStorage { Ok(()) } + /// Push requests (and dependents) to upload a gallery. + /// + /// See the module-level description for details of the whole processus. + #[cfg(feature = "unstable-msc4274")] + #[allow(clippy::too_many_arguments)] + async fn push_gallery( + &self, + event: RoomMessageEventContent, + send_event_txn: OwnedTransactionId, + created_at: MilliSecondsSinceUnixEpoch, + item_queue_infos: Vec, + ) -> Result<(), RoomSendQueueStorageError> { + let guard = self.store.lock().await; + let client = guard.client()?; + let store = client.state_store(); + + let mut last_upload_file_txn: Option = None; + + let mut finish_item_infos = Vec::::new(); + + if let Some((first, rest)) = item_queue_infos.split_first() { + let GalleryItemQueueInfo { + content_type, + upload_file_txn, + file_media_request, + thumbnail, + } = first; + + let thumbnail_info = + if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = + thumbnail + { + let upload_thumbnail_txn = thumbnail_info.txn.clone(); + + // Save the thumbnail upload request. + store + .save_send_queue_request( + &self.room_id, + upload_thumbnail_txn.clone(), + created_at, + QueuedRequestKind::MediaUpload { + content_type: thumbnail_content_type.to_string(), + cache_key: thumbnail_media_request.clone(), + thumbnail_source: None, // the thumbnail has no thumbnails :) + related_to: send_event_txn.clone(), + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; + + // Save the file upload request as a dependent request of the thumbnail upload. + store + .save_dependent_queued_request( + &self.room_id, + &upload_thumbnail_txn, + upload_file_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: true, + }, + ) + .await?; + + Some(thumbnail_info) + } else { + // Save the file upload as its own request, not a dependent one. + store + .save_send_queue_request( + &self.room_id, + upload_file_txn.clone(), + created_at, + QueuedRequestKind::MediaUpload { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + thumbnail_source: None, + related_to: send_event_txn.clone(), + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; + + None + }; + + finish_item_infos.push(FinishGalleryItemInfo { + file_upload: upload_file_txn.clone(), + thumbnail_info: thumbnail_info.cloned(), + }); + + last_upload_file_txn = Some(upload_file_txn.clone()); + + for item_queue_info in rest { + let GalleryItemQueueInfo { + content_type, + upload_file_txn, + file_media_request, + thumbnail, + } = item_queue_info; + + let thumbnail_info = if let Some(( + thumbnail_info, + thumbnail_media_request, + thumbnail_content_type, + )) = thumbnail + { + let upload_thumbnail_txn = thumbnail_info.txn.clone(); + + // Save the thumbnail upload request as a dependent request of the last file + // upload. + store + .save_dependent_queued_request( + &self.room_id, + &last_upload_file_txn.unwrap(), + upload_thumbnail_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: thumbnail_content_type.to_string(), + cache_key: thumbnail_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: false, + }, + ) + .await?; + + // Save the file upload request as a dependent request of the thumbnail upload. + store + .save_dependent_queued_request( + &self.room_id, + &upload_thumbnail_txn, + upload_file_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: true, + }, + ) + .await?; + + Some(thumbnail_info) + } else { + // Save the file upload as a dependent request of the last file upload. + store + .save_dependent_queued_request( + &self.room_id, + &last_upload_file_txn.unwrap(), + upload_file_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: false, + }, + ) + .await?; + + None + }; + + finish_item_infos.push(FinishGalleryItemInfo { + file_upload: upload_file_txn.clone(), + thumbnail_info: thumbnail_info.cloned(), + }); + + last_upload_file_txn = Some(upload_file_txn.clone()); + } + } + + // Push the request for the event itself as a dependent request of the last file + // upload. + store + .save_dependent_queued_request( + &self.room_id, + &last_upload_file_txn.unwrap(), + send_event_txn.into(), + created_at, + DependentQueuedRequestKind::FinishGallery { + local_echo: Box::new(event), + item_infos: finish_item_infos, + }, + ) + .await?; + + Ok(()) + } + /// Reacts to the given local echo of an event. #[instrument(skip(self))] async fn react( @@ -1415,11 +1610,54 @@ impl QueueStorage { }, }) } + + #[cfg(feature = "unstable-msc4274")] + DependentQueuedRequestKind::FinishGallery { local_echo, item_infos } => { + // Materialize as an event local echo. + self.create_gallery_local_echo( + dep.own_transaction_id, + room, + dep.created_at, + local_echo, + item_infos, + ) + } }); Ok(local_requests.chain(reactions_and_medias).collect()) } + /// Create a local echo for a gallery event. + #[cfg(feature = "unstable-msc4274")] + fn create_gallery_local_echo( + &self, + transaction_id: ChildTransactionId, + room: &RoomSendQueue, + created_at: MilliSecondsSinceUnixEpoch, + local_echo: Box, + item_infos: Vec, + ) -> Option { + Some(LocalEcho { + transaction_id: transaction_id.clone().into(), + content: LocalEchoContent::Event { + serialized_event: SerializableEventContent::new(&(*local_echo).into()).ok()?, + send_handle: SendHandle { + room: room.clone(), + transaction_id: transaction_id.into(), + media_handles: item_infos + .into_iter() + .map(|i| MediaHandles { + upload_thumbnail_txn: i.thumbnail_info.map(|info| info.txn), + upload_file_txn: i.file_upload, + }) + .collect(), + created_at, + }, + send_error: None, + }, + }) + } + /// Try to apply a single dependent request, whether it's local or remote. /// /// This swallows errors that would retrigger every time if we retried @@ -1654,6 +1892,23 @@ impl QueueStorage { ) .await?; } + + #[cfg(feature = "unstable-msc4274")] + DependentQueuedRequestKind::FinishGallery { local_echo, item_infos } => { + let Some(parent_key) = parent_key else { + // Not finished yet, we should retry later => false. + return Ok(false); + }; + self.handle_dependent_finish_gallery_upload( + client, + dependent_request.own_transaction_id.into(), + parent_key, + *local_echo, + item_infos, + new_updates, + ) + .await?; + } } Ok(true) @@ -1748,6 +2003,15 @@ impl QueueStorage { } } +#[cfg(feature = "unstable-msc4274")] +/// Metadata needed for pushing gallery item uploads onto the send queue. +struct GalleryItemQueueInfo { + content_type: Mime, + upload_file_txn: OwnedTransactionId, + file_media_request: MediaRequestParameters, + thumbnail: Option<(FinishUploadThumbnailInfo, MediaRequestParameters, Mime)>, +} + /// The content of a local echo. #[derive(Clone, Debug)] pub enum LocalEchoContent { @@ -1872,6 +2136,11 @@ pub enum RoomSendQueueError { /// The attachment event failed to be created. #[error("the attachment event could not be created")] FailedToCreateAttachment, + + /// The gallery event failed to be created. + #[cfg(feature = "unstable-msc4274")] + #[error("the gallery event could not be created")] + FailedToCreateGallery, } /// An error triggered by the send queue storage. @@ -2241,6 +2510,12 @@ fn canonicalize_dependent_requests( prevs.push(d); } + #[cfg(feature = "unstable-msc4274")] + DependentQueuedRequestKind::FinishGallery { .. } => { + // This request can't be canonicalized, push it as is. + prevs.push(d); + } + DependentQueuedRequestKind::RedactEvent => { // Remove every other dependent action. prevs.clear(); diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index fdc432808b7..00deacf93b4 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -15,7 +15,8 @@ //! Private implementations of the media upload mechanism. #[cfg(feature = "unstable-msc4274")] -use matrix_sdk_base::store::AccumulatedSentMediaInfo; +use std::{collections::HashMap, iter::zip}; + use matrix_sdk_base::{ event_cache::store::media::IgnoreMediaRetentionPolicy, media::{MediaFormat, MediaRequestParameters}, @@ -25,7 +26,14 @@ use matrix_sdk_base::{ }, RoomState, }; +#[cfg(feature = "unstable-msc4274")] +use matrix_sdk_base::{ + media::UniqueKey, + store::{AccumulatedSentMediaInfo, FinishGalleryItemInfo}, +}; use mime::Mime; +#[cfg(feature = "unstable-msc4274")] +use ruma::events::room::message::{GalleryItemType, GalleryMessageEventContent}; use ruma::{ events::{ room::message::{FormattedBody, MessageType, RoomMessageEventContent}, @@ -45,6 +53,11 @@ use crate::{ }, Client, Media, }; +#[cfg(feature = "unstable-msc4274")] +use crate::{ + attachment::{GalleryConfig, GalleryItemInfo}, + send_queue::GalleryItemQueueInfo, +}; /// Replace the source by the final ones in all the media types handled by /// [`Room::make_attachment_type()`]. @@ -85,6 +98,89 @@ fn update_media_event_after_upload(echo: &mut RoomMessageEventContent, sent: Sen } } +/// Replace the sources by the final ones in all the media types handled by +/// [`Room::make_gallery_item_type()`]. +#[cfg(feature = "unstable-msc4274")] +fn update_gallery_event_after_upload( + echo: &mut RoomMessageEventContent, + sent: HashMap, +) { + match &mut echo.msgtype { + MessageType::Gallery(gallery) => { + // Some variants look really similar below, but the `event` and `info` are all + // different types… + for itemtype in gallery.itemtypes.iter_mut() { + match itemtype { + GalleryItemType::Audio(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => event.source = sent.file.clone(), + None => error!( + "key for item {:?} does not exist on gallery event", + &event.source + ), + }, + GalleryItemType::File(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); + } + } + None => error!( + "key for item {:?} does not exist on gallery event", + &event.source + ), + }, + GalleryItemType::Image(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); + } + } + None => error!( + "key for item {:?} does not exist on gallery event", + &event.source + ), + }, + GalleryItemType::Video(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); + } + } + None => error!( + "key for item {:?} does not exist on gallery event", + &event.source + ), + }, + + _ => { + // All `GalleryItemType` created by `Room::make_gallery_item_type` should be + // handled here. The only way to end up here is that a item type has + // been tampered with in the database. + error!("Invalid gallery item types in database"); + // Only crash debug builds. + debug_assert!( + false, + "invalid gallery item type in database {:?}", + itemtype + ); + } + } + } + } + _ => { + // All `GalleryItemType` created by `Room::make_gallery_item_type` should be + // handled here. The only way to end up here is that a item type has + // been tampered with in the database. + error!("Invalid gallery item types in database"); + // Only crash debug builds. + debug_assert!(false, "invalid item type in database {:?}", echo.msgtype()); + } + } +} + impl RoomSendQueue { /// Queues an attachment to be sent to the room, using the send queue. /// @@ -186,7 +282,7 @@ impl RoomSendQueue { // Create the content for the media event. let event_content = room - .make_attachment_event( + .make_message_event( room.make_attachment_type( &content_type, filename, @@ -241,6 +337,182 @@ impl RoomSendQueue { Ok(send_handle) } + + /// Queues a gallery to be sent to the room, using the send queue. + /// + /// This returns quickly (without sending or uploading anything), and will + /// push the event to be sent into a queue, handled in the background. + /// + /// Callers are expected to consume [`RoomSendQueueUpdate`] via calling + /// the [`Self::subscribe()`] method to get updates about the sending of + /// that event. + /// + /// By default, if sending failed on the first attempt, it will be retried a + /// few times. If sending failed after those retries, the entire + /// client's sending queue will be disabled, and it will need to be + /// manually re-enabled by the caller (e.g. after network is back, or when + /// something has been done about the faulty requests). + /// + /// The attachments and their optional thumbnails are stored in the media + /// cache and can be retrieved at any time, by calling + /// [`Media::get_media_content()`] with the `MediaSource` that can be found + /// in the local or remote echo, and using a `MediaFormat::File`. + #[cfg(feature = "unstable-msc4274")] + #[instrument(skip_all, fields(event_txn))] + pub async fn send_gallery( + &self, + item_infos: Vec, + config: GalleryConfig, + ) -> Result { + let Some(room) = self.inner.room.get() else { + return Err(RoomSendQueueError::RoomDisappeared); + }; + + if room.state() != RoomState::Joined { + return Err(RoomSendQueueError::RoomNotJoined); + } + + let send_event_txn = config.txn_id.map_or_else(ChildTransactionId::new, Into::into); + + Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); + + let mut item_types = Vec::::new(); + let mut item_queue_infos = Vec::::new(); + let mut media_handles = Vec::::new(); + + for item_info in item_infos { + let GalleryItemInfo { filename, content_type, data, .. } = item_info; + + let upload_file_txn = TransactionId::new(); + + debug!(filename, %content_type, %upload_file_txn, "uploading a gallery attachment"); + + let file_media_request = Media::make_local_file_media_request(&upload_file_txn); + + let (upload_thumbnail_txn, event_thumbnail_info, queue_thumbnail_info) = { + let client = room.client(); + let cache_store = client + .event_cache_store() + .lock() + .await + .map_err(RoomSendQueueStorageError::LockError)?; + + // Cache the file itself in the cache store. + cache_store + .add_media_content( + &file_media_request, + data.clone(), + // Make sure that the file is stored until it has been uploaded. + IgnoreMediaRetentionPolicy::Yes, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + // Process the thumbnail, if it's been provided. + if let Some(thumbnail) = item_info.thumbnail { + let txn = TransactionId::new(); + trace!(upload_thumbnail_txn = %txn, "gallery item has a thumbnail"); + + // Create the information required for filling the thumbnail section of the + // gallery event. + let (data, content_type, thumbnail_info) = thumbnail.into_parts(); + + // Cache thumbnail in the cache store. + let thumbnail_media_request = Media::make_local_file_media_request(&txn); + cache_store + .add_media_content( + &thumbnail_media_request, + data, + // Make sure that the thumbnail is stored until it has been uploaded. + IgnoreMediaRetentionPolicy::Yes, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + ( + Some(txn.clone()), + Some((thumbnail_media_request.source.clone(), thumbnail_info)), + Some(( + FinishUploadThumbnailInfo { txn, width: None, height: None }, + thumbnail_media_request, + content_type, + )), + ) + } else { + Default::default() + } + }; + + item_types.push(room.make_gallery_item_type( + &content_type, + filename, + file_media_request.source.clone(), + item_info.caption, + item_info.formatted_caption, + Some(item_info.attachment_info), + event_thumbnail_info, + )); + + item_queue_infos.push(GalleryItemQueueInfo { + content_type, + upload_file_txn: upload_file_txn.clone(), + file_media_request, + thumbnail: queue_thumbnail_info, + }); + + media_handles.push(MediaHandles { upload_file_txn, upload_thumbnail_txn }); + } + + // Create the content for the gallery event. + let event_content = room + .make_message_event( + MessageType::Gallery(GalleryMessageEventContent::new( + config.caption.unwrap_or_default(), + config.formatted_caption, + item_types, + )), + config.mentions, + config.reply, + ) + .await + .map_err(|_| RoomSendQueueError::FailedToCreateGallery)?; + + let created_at = MilliSecondsSinceUnixEpoch::now(); + + // Save requests in the queue storage. + self.inner + .queue + .push_gallery( + event_content.clone(), + send_event_txn.clone().into(), + created_at, + item_queue_infos, + ) + .await?; + + trace!("manager sends a gallery to the background task"); + + self.inner.notifier.notify_one(); + + let send_handle = SendHandle { + room: self.clone(), + transaction_id: send_event_txn.clone().into(), + media_handles, + created_at, + }; + + let _ = self.inner.updates.send(RoomSendQueueUpdate::NewLocalEvent(LocalEcho { + transaction_id: send_event_txn.clone().into(), + content: LocalEchoContent::Event { + serialized_event: SerializableEventContent::new(&event_content.into()) + .map_err(RoomSendQueueStorageError::JsonSerialization)?, + send_handle: send_handle.clone(), + send_error: None, + }, + })); + + Ok(send_handle) + } } impl QueueStorage { @@ -349,6 +621,134 @@ impl QueueStorage { Ok(()) } + /// Consumes a finished gallery upload and queues sending of the final + /// gallery event. + #[cfg(feature = "unstable-msc4274")] + #[allow(clippy::too_many_arguments)] + pub(super) async fn handle_dependent_finish_gallery_upload( + &self, + client: &Client, + event_txn: OwnedTransactionId, + parent_key: SentRequestKey, + mut local_echo: RoomMessageEventContent, + item_infos: Vec, + new_updates: &mut Vec, + ) -> Result<(), RoomSendQueueError> { + // All uploads are ready: enqueue the event with its final data. + let sent_gallery = parent_key + .into_media() + .ok_or(RoomSendQueueError::StorageError(RoomSendQueueStorageError::InvalidParentKey))?; + + let mut sent_media_vec = sent_gallery.accumulated; + sent_media_vec.push(AccumulatedSentMediaInfo { + file: sent_gallery.file, + thumbnail: sent_gallery.thumbnail, + }); + + let mut sent_infos = HashMap::::new(); + + for (item_info, sent_media) in zip(item_infos, sent_media_vec) { + let FinishGalleryItemInfo { file_upload: file_upload_txn, thumbnail_info } = item_info; + + // Update cache keys in the cache store. + { + // Do it for the file itself. + let from_req = Media::make_local_file_media_request(&file_upload_txn); + + // Store the sent media under the original cache key for later insertion into + // the local echo + sent_infos.insert(from_req.source.unique_key(), sent_media.clone()); + + trace!(from = ?from_req.source, to = ?sent_media.file, "renaming media file key in cache store"); + let cache_store = client + .event_cache_store() + .lock() + .await + .map_err(RoomSendQueueStorageError::LockError)?; + + // The media can now be removed during cleanups. + cache_store + .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + cache_store + .replace_media_key( + &from_req, + &MediaRequestParameters { + source: sent_media.file.clone(), + format: MediaFormat::File, + }, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + // Rename the thumbnail too, if needs be. + if let Some((info, new_source)) = + thumbnail_info.as_ref().zip(sent_media.thumbnail.clone()) + { + // Previously the media request used `MediaFormat::Thumbnail`. Handle this case + // for send queue requests that were in the state store before the change. + let from_req = if let Some((height, width)) = info.height.zip(info.width) { + Media::make_local_thumbnail_media_request(&info.txn, height, width) + } else { + Media::make_local_file_media_request(&info.txn) + }; + + trace!(from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); + + // The media can now be removed during cleanups. + cache_store + .set_ignore_media_retention_policy( + &from_req, + IgnoreMediaRetentionPolicy::No, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + cache_store + .replace_media_key( + &from_req, + &MediaRequestParameters { + source: new_source, + format: MediaFormat::File, + }, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + } + } + } + + update_gallery_event_after_upload(&mut local_echo, sent_infos); + + let new_content = SerializableEventContent::new(&local_echo.into()) + .map_err(RoomSendQueueStorageError::JsonSerialization)?; + + // Indicates observers that the upload finished, by editing the local echo for + // the event into its final form before sending. + new_updates.push(RoomSendQueueUpdate::ReplacedLocalEvent { + transaction_id: event_txn.clone(), + new_content: new_content.clone(), + }); + + trace!(%event_txn, "queueing media event after successfully uploading media(s)"); + + client + .state_store() + .save_send_queue_request( + &self.room_id, + event_txn, + MilliSecondsSinceUnixEpoch::now(), + new_content.into(), + Self::HIGH_PRIORITY, + ) + .await + .map_err(RoomSendQueueStorageError::StateStoreError)?; + + Ok(()) + } + /// Consumes a finished file or thumbnail upload and queues the dependent /// file or thumbnail upload. #[allow(clippy::too_many_arguments)] diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index d98b32a555e..4693ed8d792 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -2,6 +2,8 @@ use std::{ops::Not as _, sync::Arc, time::Duration}; use as_variant::as_variant; use assert_matches2::{assert_let, assert_matches}; +#[cfg(feature = "unstable-msc4274")] +use matrix_sdk::attachment::{GalleryConfig, GalleryItemInfo}; use matrix_sdk::{ attachment::{AttachmentConfig, AttachmentInfo, BaseImageInfo, Thumbnail}, config::StoreConfig, @@ -18,6 +20,8 @@ use matrix_sdk_test::{ async_test, event_factory::EventFactory, InvitedRoomBuilder, KnockedRoomBuilder, LeftRoomBuilder, ALICE, }; +#[cfg(feature = "unstable-msc4274")] +use ruma::events::room::message::GalleryItemType; use ruma::{ event_id, events::{ @@ -2039,6 +2043,458 @@ async fn test_media_uploads() { assert!(watch.is_empty()); } +#[cfg(feature = "unstable-msc4274")] +#[async_test] +async fn test_gallery_uploads() { + let mock = MatrixMockServer::new().await; + + // Mark the room as joined. + let room_id = room_id!("!a:b.c"); + let client = mock.client_builder().build().await; + let room = mock.sync_joined_room(&client, room_id).await; + + let q = room.send_queue(); + + let (local_echoes, mut watch) = q.subscribe().await.unwrap(); + assert!(local_echoes.is_empty()); + + // ---------------------- + // Create the medias to send, with thumbnails. + let filename1 = "surprise.jpeg.exe"; + let content_type1 = mime::IMAGE_JPEG; + let data1 = b"hello world".to_vec(); + + let filename2 = "onemore.jpeg.exe"; + let content_type2 = mime::IMAGE_JPEG; + let data2 = b"hello again".to_vec(); + + let replied_to_event_id = event_id!("$foo:bar.com"); + + let thumbnail1 = Thumbnail { + data: b"thumbnail".to_vec(), + content_type: content_type1.clone(), + height: uint!(13), + width: uint!(37), + size: uint!(42), + }; + + let thumbnail2 = Thumbnail { + data: b"another thumbnail".to_vec(), + content_type: content_type2.clone(), + height: uint!(15), + width: uint!(39), + size: uint!(44), + }; + + let attachment_info1 = AttachmentInfo::Image(BaseImageInfo { + height: Some(uint!(14)), + width: Some(uint!(38)), + size: Some(uint!(43)), + is_animated: Some(false), + ..Default::default() + }); + + let attachment_info2 = AttachmentInfo::Image(BaseImageInfo { + height: Some(uint!(16)), + width: Some(uint!(40)), + size: Some(uint!(45)), + is_animated: Some(false), + ..Default::default() + }); + + let item_infos = vec![ + GalleryItemInfo { + attachment_info: attachment_info1, + content_type: content_type1, + filename: filename1.into(), + data: data1, + thumbnail: Some(thumbnail1), + caption: Some("caption1".to_owned()), + formatted_caption: None, + }, + GalleryItemInfo { + attachment_info: attachment_info2, + content_type: content_type2, + filename: filename2.into(), + data: data2, + thumbnail: Some(thumbnail2), + caption: Some("caption2".to_owned()), + formatted_caption: None, + }, + ]; + + let transaction_id = TransactionId::new(); + let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); + let config = GalleryConfig::new() + .txn_id(&transaction_id) + .caption(Some("caption".to_owned())) + .mentions(Some(mentions.clone())) + .reply(Some(Reply { + event_id: replied_to_event_id.into(), + enforce_thread: matrix_sdk::room::reply::EnforceThread::Threaded(ReplyWithinThread::No), + })); + + // ---------------------- + // Prepare endpoints. + mock.mock_room_state_encryption().plain().mount().await; + mock.mock_room_send().ok(event_id!("$1")).mock_once().mount().await; + + let f = EventFactory::new(); + mock.mock_room_event() + .match_event_id() + .ok(f + .text_msg("Send me your galleries") + .sender(*ALICE) + .event_id(replied_to_event_id) + .into()) + .mock_once() + .mount() + .await; + + let allow_upload_lock = Arc::new(Mutex::new(())); + let block_upload = allow_upload_lock.lock().await; + + mock_jpeg_upload(&mock, mxc_uri!("mxc://sdk.rs/thumbnail1"), allow_upload_lock.clone()) + .mock_once() + .mount() + .await; + mock_jpeg_upload(&mock, mxc_uri!("mxc://sdk.rs/media1"), allow_upload_lock.clone()) + .mock_once() + .mount() + .await; + mock_jpeg_upload(&mock, mxc_uri!("mxc://sdk.rs/thumbnail2"), allow_upload_lock.clone()) + .mock_once() + .mount() + .await; + mock_jpeg_upload(&mock, mxc_uri!("mxc://sdk.rs/media2"), allow_upload_lock.clone()) + .mock_once() + .mount() + .await; + + // ---------------------- + // Send the media. + assert!(watch.is_empty()); + q.send_gallery(item_infos, config).await.expect("queuing the gallery works"); + + // ---------------------- + // Observe the local echo + let (txn, send_handle, content) = assert_update!(watch => local echo event); + assert_eq!(txn, transaction_id); + + // Check mentions. + let mentions = content.mentions.unwrap(); + assert!(!mentions.room); + assert_eq!( + mentions.user_ids.into_iter().collect::>(), + vec![owned_user_id!("@ivan:sdk.rs")] + ); + + // Check relations. + assert_let!(Some(Relation::Thread(thread)) = content.relates_to); + assert_eq!(thread.event_id, replied_to_event_id); + assert_eq!(thread.in_reply_to.unwrap().event_id, replied_to_event_id); + assert!(thread.is_falling_back); + + // Check metadata. + assert_let!(MessageType::Gallery(gallery_content) = content.msgtype); + assert_eq!(gallery_content.body, "caption"); + assert!(gallery_content.formatted.is_none()); + assert_eq!(gallery_content.itemtypes.len(), 2); + + // ---------------------- + // Media 1. + assert_let!(GalleryItemType::Image(img_content) = gallery_content.itemtypes.first().unwrap()); + assert_eq!(img_content.filename.as_deref().unwrap(), filename1); + assert_eq!(img_content.body, "caption1"); + + let info = img_content.info.as_ref().unwrap(); + assert_eq!(info.height, Some(uint!(14))); + assert_eq!(info.width, Some(uint!(38))); + assert_eq!(info.size, Some(uint!(43))); + assert_eq!(info.mimetype.as_deref(), Some("image/jpeg")); + assert!(info.blurhash.is_none()); + assert_eq!(info.is_animated, Some(false)); + + // Check the data source: it should reference the send queue local storage. + let local_source = img_content.source.clone(); + assert_let!(MediaSource::Plain(mxc) = &local_source); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + // The media is immediately available from the cache. + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: local_source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(file_media, b"hello world"); + + // ---------------------- + // Thumbnail 1. + + // Check metadata. + let tinfo = info.thumbnail_info.as_ref().unwrap(); + assert_eq!(tinfo.height, Some(uint!(13))); + assert_eq!(tinfo.width, Some(uint!(37))); + assert_eq!(tinfo.size, Some(uint!(42))); + assert_eq!(tinfo.mimetype.as_deref(), Some("image/jpeg")); + + // Check the thumbnail source: it should reference the send queue local storage. + let local_thumbnail_source1 = info.thumbnail_source.as_ref().unwrap(); + assert_let!(MediaSource::Plain(mxc) = &local_thumbnail_source1); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source1.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"thumbnail"); + + // The format should be ignored when requesting a local media. + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source1.clone(), + format: MediaFormat::Thumbnail(MediaThumbnailSettings::new( + tinfo.width.unwrap(), + tinfo.height.unwrap(), + )), + }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"thumbnail"); + + // Media 2 + assert_let!(GalleryItemType::Image(img_content) = gallery_content.itemtypes.get(1).unwrap()); + assert_eq!(img_content.filename.as_deref().unwrap(), filename2); + assert_eq!(img_content.body, "caption2"); + + let info = img_content.info.as_ref().unwrap(); + assert_eq!(info.height, Some(uint!(16))); + assert_eq!(info.width, Some(uint!(40))); + assert_eq!(info.size, Some(uint!(45))); + assert_eq!(info.mimetype.as_deref(), Some("image/jpeg")); + assert!(info.blurhash.is_none()); + assert_eq!(info.is_animated, Some(false)); + + // Check the data source: it should reference the send queue local storage. + let local_source = img_content.source.clone(); + assert_let!(MediaSource::Plain(mxc) = &local_source); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + // The media is immediately available from the cache. + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: local_source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(file_media, b"hello again"); + + // ---------------------- + // Thumbnail 2. + + // Check metadata. + let tinfo = info.thumbnail_info.as_ref().unwrap(); + assert_eq!(tinfo.height, Some(uint!(15))); + assert_eq!(tinfo.width, Some(uint!(39))); + assert_eq!(tinfo.size, Some(uint!(44))); + assert_eq!(tinfo.mimetype.as_deref(), Some("image/jpeg")); + + // Check the thumbnail source: it should reference the send queue local storage. + let local_thumbnail_source2 = info.thumbnail_source.as_ref().unwrap(); + assert_let!(MediaSource::Plain(mxc) = &local_thumbnail_source2); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source2.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"another thumbnail"); + + // The format should be ignored when requesting a local media. + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source2.clone(), + format: MediaFormat::Thumbnail(MediaThumbnailSettings::new( + tinfo.width.unwrap(), + tinfo.height.unwrap(), + )), + }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"another thumbnail"); + + // ---------------------- + // Send handle operations. + + // This operation should be invalid, we shouldn't turn a gallery into a + // message. + assert_matches!( + send_handle.edit(RoomMessageEventContent::text_plain("hi").into()).await, + Err(RoomSendQueueStorageError::OperationNotImplementedYet) + ); + + // ---------------------- + // Let the upload progress. + assert!(watch.is_empty()); + drop(block_upload); + + assert_update!(watch => uploaded { + related_to = transaction_id, + mxc = mxc_uri!("mxc://sdk.rs/thumbnail1") + }); + + assert_update!(watch => uploaded { + related_to = transaction_id, + mxc = mxc_uri!("mxc://sdk.rs/media1") + }); + + assert_update!(watch => uploaded { + related_to = transaction_id, + mxc = mxc_uri!("mxc://sdk.rs/thumbnail2") + }); + + assert_update!(watch => uploaded { + related_to = transaction_id, + mxc = mxc_uri!("mxc://sdk.rs/media2") + }); + + let edit_msg = assert_update!(watch => edit local echo { + txn = transaction_id + }); + assert_let!(MessageType::Gallery(gallery_content) = edit_msg.msgtype); + assert_eq!(gallery_content.itemtypes.len(), 2); + + // ---------------------- + // Media & thumbnail 1. + assert_let!(GalleryItemType::Image(new_content) = gallery_content.itemtypes.first().unwrap()); + + assert_let!(MediaSource::Plain(new_uri) = &new_content.source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/media1")); + + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: new_content.source.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect("media should be found with its final MXC uri in the cache"); + assert_eq!(file_media, b"hello world"); + + let new_thumbnail_source = new_content.info.clone().unwrap().thumbnail_source.unwrap(); + assert_let!(MediaSource::Plain(new_uri) = &new_thumbnail_source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/thumbnail1")); + + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: new_thumbnail_source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"thumbnail"); + + // The local URI does not work anymore. + client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source1.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect_err("media with local URI should not be found"); + + // ---------------------- + // Media & thumbnail 2. + assert_let!(GalleryItemType::Image(new_content) = gallery_content.itemtypes.get(1).unwrap()); + + assert_let!(MediaSource::Plain(new_uri) = &new_content.source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/media2")); + + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: new_content.source.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect("media should be found with its final MXC uri in the cache"); + assert_eq!(file_media, b"hello again"); + + let new_thumbnail_source = new_content.info.clone().unwrap().thumbnail_source.unwrap(); + assert_let!(MediaSource::Plain(new_uri) = &new_thumbnail_source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/thumbnail2")); + + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: new_thumbnail_source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"another thumbnail"); + + // The local URI does not work anymore. + client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source2.clone(), + format: MediaFormat::File, + }, + true, + ) + .await + .expect_err("media with local URI should not be found"); + + // The event is sent, at some point. + assert_update!(watch => sent { + txn = transaction_id, + event_id = event_id!("$1") + }); + + // That's all, folks! + assert!(watch.is_empty()); +} + #[async_test] async fn test_media_upload_retry() { let mock = MatrixMockServer::new().await; From 9a627670a8d27a25b2bfcc78b0125f8ce2b9ab06 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:09:45 +0200 Subject: [PATCH 02/23] Use an OwnedTransactionId to avoid cloning --- crates/matrix-sdk/src/attachment.rs | 10 +++++----- crates/matrix-sdk/tests/integration/send_queue.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index 81367da3061..300259a32d5 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -25,7 +25,7 @@ use ruma::{ }, Mentions, }, - OwnedTransactionId, TransactionId, UInt, + OwnedTransactionId, UInt, }; use crate::room::reply::Reply; @@ -219,8 +219,8 @@ impl AttachmentConfig { /// in its unsigned field as `transaction_id`. If not given, one is /// created for the message. #[must_use] - pub fn txn_id(mut self, txn_id: &TransactionId) -> Self { - self.txn_id = Some(txn_id.to_owned()); + pub fn txn_id(mut self, txn_id: OwnedTransactionId) -> Self { + self.txn_id = Some(txn_id); self } @@ -303,8 +303,8 @@ impl GalleryConfig { /// in its unsigned field as `transaction_id`. If not given, one is /// created for the message. #[must_use] - pub fn txn_id(mut self, txn_id: &TransactionId) -> Self { - self.txn_id = Some(txn_id.to_owned()); + pub fn txn_id(mut self, txn_id: OwnedTransactionId) -> Self { + self.txn_id = Some(txn_id); self } diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 4693ed8d792..2211c0c6a07 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -1827,7 +1827,7 @@ async fn test_media_uploads() { let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); let config = AttachmentConfig::new() .thumbnail(Some(thumbnail)) - .txn_id(&transaction_id) + .txn_id(transaction_id.clone()) .caption(Some("caption".to_owned())) .mentions(Some(mentions.clone())) .reply(Some(Reply { @@ -2126,7 +2126,7 @@ async fn test_gallery_uploads() { let transaction_id = TransactionId::new(); let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); let config = GalleryConfig::new() - .txn_id(&transaction_id) + .txn_id(transaction_id.clone()) .caption(Some("caption".to_owned())) .mentions(Some(mentions.clone())) .reply(Some(Reply { From 58626d2ab057998aded2ceda53a5feaa89340305 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:09:54 +0200 Subject: [PATCH 03/23] Add missing trailing periods in comments --- .../matrix-sdk-base/src/store/send_queue.rs | 2 +- crates/matrix-sdk/src/attachment.rs | 38 +++++++++---------- crates/matrix-sdk/src/send_queue/upload.rs | 2 +- .../tests/integration/send_queue.rs | 7 ++-- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/send_queue.rs b/crates/matrix-sdk-base/src/store/send_queue.rs index e08cf941d6d..4600bf7a7ee 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -268,7 +268,7 @@ pub enum DependentQueuedRequestKind { /// `Box` the local echo so that it reduces the size of the whole enum. local_echo: Box, - /// Metadata about the gallery items + /// Metadata about the gallery items. item_infos: Vec, }, } diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index 300259a32d5..e99fa39d36a 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -236,21 +236,21 @@ impl AttachmentConfig { self } - /// Set the optional caption + /// Set the optional caption. /// /// # Arguments /// - /// * `caption` - The optional caption + /// * `caption` - The optional caption. pub fn caption(mut self, caption: Option) -> Self { self.caption = caption; self } - /// Set the optional formatted caption + /// Set the optional formatted caption. /// /// # Arguments /// - /// * `formatted_caption` - The optional formatted caption + /// * `formatted_caption` - The optional formatted caption. pub fn formatted_caption(mut self, formatted_caption: Option) -> Self { self.formatted_caption = formatted_caption; self @@ -260,7 +260,7 @@ impl AttachmentConfig { /// /// # Arguments /// - /// * `mentions` - The mentions of the message + /// * `mentions` - The mentions of the message. pub fn mentions(mut self, mentions: Option) -> Self { self.mentions = mentions; self @@ -270,7 +270,7 @@ impl AttachmentConfig { /// /// # Arguments /// - /// * `reply` - The reply information of the message + /// * `reply` - The reply information of the message. pub fn reply(mut self, reply: Option) -> Self { self.reply = reply; self @@ -308,21 +308,21 @@ impl GalleryConfig { self } - /// Set the optional caption + /// Set the optional caption. /// /// # Arguments /// - /// * `caption` - The optional caption + /// * `caption` - The optional caption. pub fn caption(mut self, caption: Option) -> Self { self.caption = caption; self } - /// Set the optional formatted caption + /// Set the optional formatted caption. /// /// # Arguments /// - /// * `formatted_caption` - The optional formatted caption + /// * `formatted_caption` - The optional formatted caption. pub fn formatted_caption(mut self, formatted_caption: Option) -> Self { self.formatted_caption = formatted_caption; self @@ -332,7 +332,7 @@ impl GalleryConfig { /// /// # Arguments /// - /// * `mentions` - The mentions of the message + /// * `mentions` - The mentions of the message. pub fn mentions(mut self, mentions: Option) -> Self { self.mentions = mentions; self @@ -342,7 +342,7 @@ impl GalleryConfig { /// /// # Arguments /// - /// * `reply` - The reply information of the message + /// * `reply` - The reply information of the message. pub fn reply(mut self, reply: Option) -> Self { self.reply = reply; self @@ -353,18 +353,18 @@ impl GalleryConfig { #[derive(Debug)] /// Metadata for a gallery item pub struct GalleryItemInfo { - /// The filename + /// The filename. pub filename: String, - /// The mime type + /// The mime type. pub content_type: mime::Mime, - /// The binary data + /// The binary data. pub data: Vec, - /// The attachment info + /// The attachment info. pub attachment_info: AttachmentInfo, - /// The caption + /// The caption. pub caption: Option, - /// The formatted caption + /// The formatted caption. pub formatted_caption: Option, - /// The thumbnail + /// The thumbnail. pub thumbnail: Option, } diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 00deacf93b4..54d40756686 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -656,7 +656,7 @@ impl QueueStorage { let from_req = Media::make_local_file_media_request(&file_upload_txn); // Store the sent media under the original cache key for later insertion into - // the local echo + // the local echo. sent_infos.insert(from_req.source.unique_key(), sent_media.clone()); trace!(from = ?from_req.source, to = ?sent_media.file, "renaming media file key in cache store"); diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 2211c0c6a07..dea60cb505c 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -1873,7 +1873,7 @@ async fn test_media_uploads() { .expect("queuing the attachment works"); // ---------------------- - // Observe the local echo + // Observe the local echo. let (txn, send_handle, content) = assert_update!(watch => local echo event); assert_eq!(txn, transaction_id); @@ -2177,7 +2177,7 @@ async fn test_gallery_uploads() { q.send_gallery(item_infos, config).await.expect("queuing the gallery works"); // ---------------------- - // Observe the local echo + // Observe the local echo. let (txn, send_handle, content) = assert_update!(watch => local echo event); assert_eq!(txn, transaction_id); @@ -2276,7 +2276,8 @@ async fn test_gallery_uploads() { .expect("media should be found"); assert_eq!(thumbnail_media, b"thumbnail"); - // Media 2 + // ---------------------- + // Media 2. assert_let!(GalleryItemType::Image(img_content) = gallery_content.itemtypes.get(1).unwrap()); assert_eq!(img_content.filename.as_deref().unwrap(), filename2); assert_eq!(img_content.body, "caption2"); From 932dbf94c4777503d1f975ea6817a2ab5f96779e Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:09:57 +0200 Subject: [PATCH 04/23] Move client and cache store outside of loop --- crates/matrix-sdk/src/send_queue/upload.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 54d40756686..1c09dd34c83 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -380,6 +380,13 @@ impl RoomSendQueue { let mut item_queue_infos = Vec::::new(); let mut media_handles = Vec::::new(); + let client = room.client(); + let cache_store = client + .event_cache_store() + .lock() + .await + .map_err(RoomSendQueueStorageError::LockError)?; + for item_info in item_infos { let GalleryItemInfo { filename, content_type, data, .. } = item_info; @@ -390,13 +397,6 @@ impl RoomSendQueue { let file_media_request = Media::make_local_file_media_request(&upload_file_txn); let (upload_thumbnail_txn, event_thumbnail_info, queue_thumbnail_info) = { - let client = room.client(); - let cache_store = client - .event_cache_store() - .lock() - .await - .map_err(RoomSendQueueStorageError::LockError)?; - // Cache the file itself in the cache store. cache_store .add_media_content( From c6731ca74ad85c0a99f09ed96fc2e7f4891181a7 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:09:59 +0200 Subject: [PATCH 05/23] Prevent sending empty galleries --- crates/matrix-sdk/src/send_queue/mod.rs | 5 +++++ crates/matrix-sdk/src/send_queue/upload.rs | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index f070962c534..6ca47f01f90 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -2137,6 +2137,11 @@ pub enum RoomSendQueueError { #[error("the attachment event could not be created")] FailedToCreateAttachment, + /// The gallery contains no items. + #[cfg(feature = "unstable-msc4274")] + #[error("the gallery contains no items")] + EmptyGallery, + /// The gallery event failed to be created. #[cfg(feature = "unstable-msc4274")] #[error("the gallery event could not be created")] diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 1c09dd34c83..c25f4ad9e9e 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -372,6 +372,10 @@ impl RoomSendQueue { return Err(RoomSendQueueError::RoomNotJoined); } + if item_infos.is_empty() { + return Err(RoomSendQueueError::EmptyGallery); + } + let send_event_txn = config.txn_id.map_or_else(ChildTransactionId::new, Into::into); Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); From 47fe934ab01c3df848831d059f08a05ae1b8a859 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:01 +0200 Subject: [PATCH 06/23] Remove superfluous type annotations --- crates/matrix-sdk/src/send_queue/upload.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index c25f4ad9e9e..baf05f1a285 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -380,9 +380,9 @@ impl RoomSendQueue { Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); - let mut item_types = Vec::::new(); - let mut item_queue_infos = Vec::::new(); - let mut media_handles = Vec::::new(); + let mut item_types = Vec::new(); + let mut item_queue_infos = Vec::new(); + let mut media_handles = Vec::new(); let client = room.client(); let cache_store = client From 49c9576953118e5b23d3fe85326da7b9e6dab4f9 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:03 +0200 Subject: [PATCH 07/23] Pre-allocate capacity for item vectors --- crates/matrix-sdk/src/send_queue/upload.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index baf05f1a285..2fba6bd260f 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -380,9 +380,9 @@ impl RoomSendQueue { Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); - let mut item_types = Vec::new(); - let mut item_queue_infos = Vec::new(); - let mut media_handles = Vec::new(); + let mut item_types = Vec::with_capacity(item_infos.len()); + let mut item_queue_infos = Vec::with_capacity(item_infos.len()); + let mut media_handles = Vec::with_capacity(item_infos.len()); let client = room.client(); let cache_store = client From 421f5bd92d4175e06cda93ea3f837b03cb77055d Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:05 +0200 Subject: [PATCH 08/23] Extract macro for MessageType / GalleryItemType --- crates/matrix-sdk/src/room/mod.rs | 256 ++++++++++++------------------ 1 file changed, 102 insertions(+), 154 deletions(-) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index cfde378288d..705a0fbca07 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -220,6 +220,88 @@ impl PushContext { } } +macro_rules! make_media_type { + ($t:ty, $content_type: ident, $filename: ident, $source: ident, $caption: ident, $formatted_caption: ident, $info: ident, $thumbnail: ident) => {{ + // If caption is set, use it as body, and filename as the file name; otherwise, + // body is the filename, and the filename is not set. + // https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md + let (body, filename) = match $caption { + Some(caption) => (caption, Some($filename)), + None => ($filename, None), + }; + + let (thumbnail_source, thumbnail_info) = $thumbnail.unzip(); + + match $content_type.type_() { + mime::IMAGE => { + let info = assign!($info.map(ImageInfo::from).unwrap_or_default(), { + mimetype: Some($content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(ImageMessageEventContent::new(body, $source), { + info: Some(Box::new(info)), + formatted: $formatted_caption, + filename + }); + <$t>::Image(content) + } + + mime::AUDIO => { + let mut content = assign!(AudioMessageEventContent::new(body, $source), { + formatted: $formatted_caption, + filename + }); + + if let Some(AttachmentInfo::Voice { audio_info, waveform: Some(waveform_vec) }) = + &$info + { + if let Some(duration) = audio_info.duration { + let waveform = waveform_vec.iter().map(|v| (*v).into()).collect(); + content.audio = + Some(UnstableAudioDetailsContentBlock::new(duration, waveform)); + } + content.voice = Some(UnstableVoiceContentBlock::new()); + } + + let mut audio_info = $info.map(AudioInfo::from).unwrap_or_default(); + audio_info.mimetype = Some($content_type.as_ref().to_owned()); + let content = content.info(Box::new(audio_info)); + + <$t>::Audio(content) + } + + mime::VIDEO => { + let info = assign!($info.map(VideoInfo::from).unwrap_or_default(), { + mimetype: Some($content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(VideoMessageEventContent::new(body, $source), { + info: Some(Box::new(info)), + formatted: $formatted_caption, + filename + }); + <$t>::Video(content) + } + + _ => { + let info = assign!($info.map(FileInfo::from).unwrap_or_default(), { + mimetype: Some($content_type.as_ref().to_owned()), + thumbnail_source, + thumbnail_info + }); + let content = assign!(FileMessageEventContent::new(body, $source), { + info: Some(Box::new(info)), + formatted: $formatted_caption, + filename, + }); + <$t>::File(content) + } + } + }}; +} + impl Room { /// Create a new `Room` /// @@ -2215,83 +2297,16 @@ impl Room { info: Option, thumbnail: Option<(MediaSource, Box)>, ) -> MessageType { - // If caption is set, use it as body, and filename as the file name; otherwise, - // body is the filename, and the filename is not set. - // https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md - let (body, filename) = match caption { - Some(caption) => (caption, Some(filename)), - None => (filename, None), - }; - - let (thumbnail_source, thumbnail_info) = thumbnail.unzip(); - - match content_type.type_() { - mime::IMAGE => { - let info = assign!(info.map(ImageInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(ImageMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename - }); - MessageType::Image(content) - } - - mime::AUDIO => { - let mut content = assign!(AudioMessageEventContent::new(body, source), { - formatted: formatted_caption, - filename - }); - - if let Some(AttachmentInfo::Voice { audio_info, waveform: Some(waveform_vec) }) = - &info - { - if let Some(duration) = audio_info.duration { - let waveform = waveform_vec.iter().map(|v| (*v).into()).collect(); - content.audio = - Some(UnstableAudioDetailsContentBlock::new(duration, waveform)); - } - content.voice = Some(UnstableVoiceContentBlock::new()); - } - - let mut audio_info = info.map(AudioInfo::from).unwrap_or_default(); - audio_info.mimetype = Some(content_type.as_ref().to_owned()); - let content = content.info(Box::new(audio_info)); - - MessageType::Audio(content) - } - - mime::VIDEO => { - let info = assign!(info.map(VideoInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(VideoMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename - }); - MessageType::Video(content) - } - - _ => { - let info = assign!(info.map(FileInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(FileMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename, - }); - MessageType::File(content) - } - } + make_media_type!( + MessageType, + content_type, + filename, + source, + caption, + formatted_caption, + info, + thumbnail + ) } /// Creates the [`RoomMessageEventContent`] based on the message type, @@ -2328,83 +2343,16 @@ impl Room { info: Option, thumbnail: Option<(MediaSource, Box)>, ) -> GalleryItemType { - // If caption is set, use it as body, and filename as the file name; otherwise, - // body is the filename, and the filename is not set. - // https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md - let (body, filename) = match caption { - Some(caption) => (caption, Some(filename)), - None => (filename, None), - }; - - let (thumbnail_source, thumbnail_info) = thumbnail.unzip(); - - match content_type.type_() { - mime::IMAGE => { - let info = assign!(info.map(ImageInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(ImageMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename - }); - GalleryItemType::Image(content) - } - - mime::AUDIO => { - let mut content = assign!(AudioMessageEventContent::new(body, source), { - formatted: formatted_caption, - filename - }); - - if let Some(AttachmentInfo::Voice { audio_info, waveform: Some(waveform_vec) }) = - &info - { - if let Some(duration) = audio_info.duration { - let waveform = waveform_vec.iter().map(|v| (*v).into()).collect(); - content.audio = - Some(UnstableAudioDetailsContentBlock::new(duration, waveform)); - } - content.voice = Some(UnstableVoiceContentBlock::new()); - } - - let mut audio_info = info.map(AudioInfo::from).unwrap_or_default(); - audio_info.mimetype = Some(content_type.as_ref().to_owned()); - let content = content.info(Box::new(audio_info)); - - GalleryItemType::Audio(content) - } - - mime::VIDEO => { - let info = assign!(info.map(VideoInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(VideoMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename - }); - GalleryItemType::Video(content) - } - - _ => { - let info = assign!(info.map(FileInfo::from).unwrap_or_default(), { - mimetype: Some(content_type.as_ref().to_owned()), - thumbnail_source, - thumbnail_info - }); - let content = assign!(FileMessageEventContent::new(body, source), { - info: Some(Box::new(info)), - formatted: formatted_caption, - filename, - }); - GalleryItemType::File(content) - } - } + make_media_type!( + GalleryItemType, + content_type, + filename, + source, + caption, + formatted_caption, + info, + thumbnail + ) } /// Update the power levels of a select set of users of this room. From b5f4828c3e9cb26a07d0c47c53b168097cc9e610 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:07 +0200 Subject: [PATCH 09/23] Remove superfluous self --- crates/matrix-sdk/src/room/mod.rs | 4 +--- crates/matrix-sdk/src/send_queue/upload.rs | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 705a0fbca07..7bb5081457e 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -2263,7 +2263,7 @@ impl Room { let content = self .make_message_event( - self.make_attachment_type( + Room::make_attachment_type( content_type, filename, media_source, @@ -2288,7 +2288,6 @@ impl Room { /// provided by its source. #[allow(clippy::too_many_arguments)] pub(crate) fn make_attachment_type( - &self, content_type: &Mime, filename: String, source: MediaSource, @@ -2334,7 +2333,6 @@ impl Room { #[cfg(feature = "unstable-msc4274")] #[allow(clippy::too_many_arguments)] pub(crate) fn make_gallery_item_type( - &self, content_type: &Mime, filename: String, source: MediaSource, diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 2fba6bd260f..67033ecc227 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -51,7 +51,7 @@ use crate::{ LocalEcho, LocalEchoContent, MediaHandles, RoomSendQueueStorageError, RoomSendQueueUpdate, SendHandle, }, - Client, Media, + Client, Media, Room, }; #[cfg(feature = "unstable-msc4274")] use crate::{ @@ -283,7 +283,7 @@ impl RoomSendQueue { // Create the content for the media event. let event_content = room .make_message_event( - room.make_attachment_type( + Room::make_attachment_type( &content_type, filename, file_media_request.source.clone(), @@ -447,7 +447,7 @@ impl RoomSendQueue { } }; - item_types.push(room.make_gallery_item_type( + item_types.push(Room::make_gallery_item_type( &content_type, filename, file_media_request.source.clone(), From 7149b7350c996884344679eda680f50e81b153e0 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:09 +0200 Subject: [PATCH 10/23] Rename make_message_event to make_media_event --- crates/matrix-sdk/src/room/mod.rs | 4 ++-- crates/matrix-sdk/src/send_queue/upload.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 7bb5081457e..f19df8c2634 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -2262,7 +2262,7 @@ impl Room { } let content = self - .make_message_event( + .make_media_event( Room::make_attachment_type( content_type, filename, @@ -2310,7 +2310,7 @@ impl Room { /// Creates the [`RoomMessageEventContent`] based on the message type, /// mentions and reply information. - pub(crate) async fn make_message_event( + pub(crate) async fn make_media_event( &self, msg_type: MessageType, mentions: Option, diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 67033ecc227..5481732346a 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -282,7 +282,7 @@ impl RoomSendQueue { // Create the content for the media event. let event_content = room - .make_message_event( + .make_media_event( Room::make_attachment_type( &content_type, filename, @@ -469,7 +469,7 @@ impl RoomSendQueue { // Create the content for the gallery event. let event_content = room - .make_message_event( + .make_media_event( MessageType::Gallery(GalleryMessageEventContent::new( config.caption.unwrap_or_default(), config.formatted_caption, From 0e58e500508969ab285d22352b949e83f60ed648 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:11 +0200 Subject: [PATCH 11/23] Bundle items with GalleryConfig --- crates/matrix-sdk/src/attachment.rs | 22 +++++++++++++++++++ crates/matrix-sdk/src/send_queue/upload.rs | 13 +++++------ .../tests/integration/send_queue.rs | 21 ++++++++---------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index e99fa39d36a..c8c405c642f 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -282,6 +282,7 @@ impl AttachmentConfig { #[derive(Debug, Default)] pub struct GalleryConfig { pub(crate) txn_id: Option, + pub(crate) items: Vec, pub(crate) caption: Option, pub(crate) formatted_caption: Option, pub(crate) mentions: Option, @@ -308,6 +309,17 @@ impl GalleryConfig { self } + /// Adds a media item to the gallery. + /// + /// # Arguments + /// + /// * `item` - Information about the item to be added. + #[must_use] + pub fn add_item(mut self, item: GalleryItemInfo) -> Self { + self.items.push(item); + self + } + /// Set the optional caption. /// /// # Arguments @@ -347,6 +359,16 @@ impl GalleryConfig { self.reply = reply; self } + + /// Returns the number of media items in the gallery. + pub fn len(&self) -> usize { + self.items.len() + } + + /// Checks whether the gallery contains any media items or not. + pub fn is_empty(&self) -> bool { + self.items.is_empty() + } } #[cfg(feature = "unstable-msc4274")] diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 5481732346a..781858043d7 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -361,7 +361,6 @@ impl RoomSendQueue { #[instrument(skip_all, fields(event_txn))] pub async fn send_gallery( &self, - item_infos: Vec, config: GalleryConfig, ) -> Result { let Some(room) = self.inner.room.get() else { @@ -372,17 +371,17 @@ impl RoomSendQueue { return Err(RoomSendQueueError::RoomNotJoined); } - if item_infos.is_empty() { + if config.is_empty() { return Err(RoomSendQueueError::EmptyGallery); } - let send_event_txn = config.txn_id.map_or_else(ChildTransactionId::new, Into::into); + let send_event_txn = config.txn_id.clone().map_or_else(ChildTransactionId::new, Into::into); Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); - let mut item_types = Vec::with_capacity(item_infos.len()); - let mut item_queue_infos = Vec::with_capacity(item_infos.len()); - let mut media_handles = Vec::with_capacity(item_infos.len()); + let mut item_types = Vec::with_capacity(config.len()); + let mut item_queue_infos = Vec::with_capacity(config.len()); + let mut media_handles = Vec::with_capacity(config.len()); let client = room.client(); let cache_store = client @@ -391,7 +390,7 @@ impl RoomSendQueue { .await .map_err(RoomSendQueueStorageError::LockError)?; - for item_info in item_infos { + for item_info in config.items { let GalleryItemInfo { filename, content_type, data, .. } = item_info; let upload_file_txn = TransactionId::new(); diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index dea60cb505c..836762693d2 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -2102,8 +2102,11 @@ async fn test_gallery_uploads() { ..Default::default() }); - let item_infos = vec![ - GalleryItemInfo { + let transaction_id = TransactionId::new(); + let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); + let config = GalleryConfig::new() + .txn_id(transaction_id.clone()) + .add_item(GalleryItemInfo { attachment_info: attachment_info1, content_type: content_type1, filename: filename1.into(), @@ -2111,8 +2114,8 @@ async fn test_gallery_uploads() { thumbnail: Some(thumbnail1), caption: Some("caption1".to_owned()), formatted_caption: None, - }, - GalleryItemInfo { + }) + .add_item(GalleryItemInfo { attachment_info: attachment_info2, content_type: content_type2, filename: filename2.into(), @@ -2120,13 +2123,7 @@ async fn test_gallery_uploads() { thumbnail: Some(thumbnail2), caption: Some("caption2".to_owned()), formatted_caption: None, - }, - ]; - - let transaction_id = TransactionId::new(); - let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); - let config = GalleryConfig::new() - .txn_id(transaction_id.clone()) + }) .caption(Some("caption".to_owned())) .mentions(Some(mentions.clone())) .reply(Some(Reply { @@ -2174,7 +2171,7 @@ async fn test_gallery_uploads() { // ---------------------- // Send the media. assert!(watch.is_empty()); - q.send_gallery(item_infos, config).await.expect("queuing the gallery works"); + q.send_gallery(config).await.expect("queuing the gallery works"); // ---------------------- // Observe the local echo. From fc4aa7efb9426b7eebb12dec4f37b4a06aa6bde4 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:12 +0200 Subject: [PATCH 12/23] Avoid type annotation and pre-allocate vector --- crates/matrix-sdk/src/send_queue/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 6ca47f01f90..16248351990 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -1304,7 +1304,7 @@ impl QueueStorage { let mut last_upload_file_txn: Option = None; - let mut finish_item_infos = Vec::::new(); + let mut finish_item_infos = Vec::with_capacity(item_queue_infos.len()); if let Some((first, rest)) = item_queue_infos.split_first() { let GalleryItemQueueInfo { From e47706eb3fbb2da4006a2005fa457607e830de6b Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:15 +0200 Subject: [PATCH 13/23] Avoid unnecessary type annotation --- crates/matrix-sdk/src/send_queue/upload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 781858043d7..5e93c560a5e 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -648,7 +648,7 @@ impl QueueStorage { thumbnail: sent_gallery.thumbnail, }); - let mut sent_infos = HashMap::::new(); + let mut sent_infos = HashMap::new(); for (item_info, sent_media) in zip(item_infos, sent_media_vec) { let FinishGalleryItemInfo { file_upload: file_upload_txn, thumbnail_info } = item_info; From 4028bb4f96055ab52c3cbff19f5090b00a9a586d Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:17 +0200 Subject: [PATCH 14/23] Use let / else to reduce indentation --- crates/matrix-sdk/src/send_queue/upload.rs | 118 +++++++++------------ 1 file changed, 50 insertions(+), 68 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 5e93c560a5e..b6df9da7696 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -105,79 +105,61 @@ fn update_gallery_event_after_upload( echo: &mut RoomMessageEventContent, sent: HashMap, ) { - match &mut echo.msgtype { - MessageType::Gallery(gallery) => { - // Some variants look really similar below, but the `event` and `info` are all - // different types… - for itemtype in gallery.itemtypes.iter_mut() { - match itemtype { - GalleryItemType::Audio(event) => match sent.get(&event.source.unique_key()) { - Some(sent) => event.source = sent.file.clone(), - None => error!( - "key for item {:?} does not exist on gallery event", - &event.source - ), - }, - GalleryItemType::File(event) => match sent.get(&event.source.unique_key()) { - Some(sent) => { - event.source = sent.file.clone(); - if let Some(info) = event.info.as_mut() { - info.thumbnail_source = sent.thumbnail.clone(); - } - } - None => error!( - "key for item {:?} does not exist on gallery event", - &event.source - ), - }, - GalleryItemType::Image(event) => match sent.get(&event.source.unique_key()) { - Some(sent) => { - event.source = sent.file.clone(); - if let Some(info) = event.info.as_mut() { - info.thumbnail_source = sent.thumbnail.clone(); - } - } - None => error!( - "key for item {:?} does not exist on gallery event", - &event.source - ), - }, - GalleryItemType::Video(event) => match sent.get(&event.source.unique_key()) { - Some(sent) => { - event.source = sent.file.clone(); - if let Some(info) = event.info.as_mut() { - info.thumbnail_source = sent.thumbnail.clone(); - } - } - None => error!( - "key for item {:?} does not exist on gallery event", - &event.source - ), - }, + let MessageType::Gallery(gallery) = &mut echo.msgtype else { + // All `GalleryItemType` created by `Room::make_gallery_item_type` should be + // handled here. The only way to end up here is that a item type has + // been tampered with in the database. + error!("Invalid gallery item types in database"); + // Only crash debug builds. + debug_assert!(false, "invalid item type in database {:?}", echo.msgtype()); + return; + }; - _ => { - // All `GalleryItemType` created by `Room::make_gallery_item_type` should be - // handled here. The only way to end up here is that a item type has - // been tampered with in the database. - error!("Invalid gallery item types in database"); - // Only crash debug builds. - debug_assert!( - false, - "invalid gallery item type in database {:?}", - itemtype - ); + // Some variants look really similar below, but the `event` and `info` are all + // different types… + for itemtype in gallery.itemtypes.iter_mut() { + match itemtype { + GalleryItemType::Audio(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => event.source = sent.file.clone(), + None => error!("key for item {:?} does not exist on gallery event", &event.source), + }, + GalleryItemType::File(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); } } + None => error!("key for item {:?} does not exist on gallery event", &event.source), + }, + GalleryItemType::Image(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); + } + } + None => error!("key for item {:?} does not exist on gallery event", &event.source), + }, + GalleryItemType::Video(event) => match sent.get(&event.source.unique_key()) { + Some(sent) => { + event.source = sent.file.clone(); + if let Some(info) = event.info.as_mut() { + info.thumbnail_source = sent.thumbnail.clone(); + } + } + None => error!("key for item {:?} does not exist on gallery event", &event.source), + }, + + _ => { + // All `GalleryItemType` created by `Room::make_gallery_item_type` should be + // handled here. The only way to end up here is that a item type has + // been tampered with in the database. + error!("Invalid gallery item types in database"); + // Only crash debug builds. + debug_assert!(false, "invalid gallery item type in database {:?}", itemtype); } } - _ => { - // All `GalleryItemType` created by `Room::make_gallery_item_type` should be - // handled here. The only way to end up here is that a item type has - // been tampered with in the database. - error!("Invalid gallery item types in database"); - // Only crash debug builds. - debug_assert!(false, "invalid item type in database {:?}", echo.msgtype()); - } } } From 3acc0e8a3bf2788202c7252ab723b51af19ca144 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:19 +0200 Subject: [PATCH 15/23] Relocate and rephrase comment for FinishGalleryItemInfo --- crates/matrix-sdk-base/src/store/send_queue.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/send_queue.rs b/crates/matrix-sdk-base/src/store/send_queue.rs index 4600bf7a7ee..9f134997ad0 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -298,10 +298,11 @@ pub struct FinishUploadThumbnailInfo { pub height: Option, } +/// Detailed record about a file and thumbnail. When finishing a gallery +/// upload, one [`FinishGalleryItemInfo`] will be used for each media in the +/// gallery. #[cfg(feature = "unstable-msc4274")] #[derive(Clone, Debug, Serialize, Deserialize)] -/// Detailed record about a file and thumbnail used when finishing a gallery -/// upload. pub struct FinishGalleryItemInfo { /// Transaction id for the file upload. pub file_upload: OwnedTransactionId, From 7e7a38c47af9e28bb7d4d4ecea8290b946fab556 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 23 May 2025 21:10:21 +0200 Subject: [PATCH 16/23] Extract shared function to update cache keys --- .../matrix-sdk-base/src/store/send_queue.rs | 7 + crates/matrix-sdk/src/send_queue/upload.rs | 198 +++++++----------- 2 files changed, 79 insertions(+), 126 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/send_queue.rs b/crates/matrix-sdk-base/src/store/send_queue.rs index 9f134997ad0..5739546674a 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -394,6 +394,13 @@ pub struct AccumulatedSentMediaInfo { pub thumbnail: Option, } +#[cfg(feature = "unstable-msc4274")] +impl From for SentMediaInfo { + fn from(value: AccumulatedSentMediaInfo) -> Self { + Self { file: value.file, thumbnail: value.thumbnail, accumulated: vec![] } + } +} + /// A unique key (identifier) indicating that a transaction has been /// successfully sent to the server. /// diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index b6df9da7696..d0b39f4d850 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -518,65 +518,8 @@ impl QueueStorage { .into_media() .ok_or(RoomSendQueueError::StorageError(RoomSendQueueStorageError::InvalidParentKey))?; - // Update cache keys in the cache store. - { - // Do it for the file itself. - let from_req = Media::make_local_file_media_request(&file_upload_txn); - - trace!(from = ?from_req.source, to = ?sent_media.file, "renaming media file key in cache store"); - let cache_store = client - .event_cache_store() - .lock() - .await - .map_err(RoomSendQueueStorageError::LockError)?; - - // The media can now be removed during cleanups. - cache_store - .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - cache_store - .replace_media_key( - &from_req, - &MediaRequestParameters { - source: sent_media.file.clone(), - format: MediaFormat::File, - }, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - // Rename the thumbnail too, if needs be. - if let Some((info, new_source)) = - thumbnail_info.as_ref().zip(sent_media.thumbnail.clone()) - { - // Previously the media request used `MediaFormat::Thumbnail`. Handle this case - // for send queue requests that were in the state store before the change. - let from_req = if let Some((height, width)) = info.height.zip(info.width) { - Media::make_local_thumbnail_media_request(&info.txn, height, width) - } else { - Media::make_local_file_media_request(&info.txn) - }; - - trace!(from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); - - // The media can now be removed during cleanups. - cache_store - .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - cache_store - .replace_media_key( - &from_req, - &MediaRequestParameters { source: new_source, format: MediaFormat::File }, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - } - } - + update_media_cache_keys_after_upload(client, &file_upload_txn, thumbnail_info, &sent_media) + .await?; update_media_event_after_upload(&mut local_echo, sent_media); let new_content = SerializableEventContent::new(&local_echo.into()) @@ -635,74 +578,18 @@ impl QueueStorage { for (item_info, sent_media) in zip(item_infos, sent_media_vec) { let FinishGalleryItemInfo { file_upload: file_upload_txn, thumbnail_info } = item_info; - // Update cache keys in the cache store. - { - // Do it for the file itself. - let from_req = Media::make_local_file_media_request(&file_upload_txn); - - // Store the sent media under the original cache key for later insertion into - // the local echo. - sent_infos.insert(from_req.source.unique_key(), sent_media.clone()); - - trace!(from = ?from_req.source, to = ?sent_media.file, "renaming media file key in cache store"); - let cache_store = client - .event_cache_store() - .lock() - .await - .map_err(RoomSendQueueStorageError::LockError)?; - - // The media can now be removed during cleanups. - cache_store - .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - cache_store - .replace_media_key( - &from_req, - &MediaRequestParameters { - source: sent_media.file.clone(), - format: MediaFormat::File, - }, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - // Rename the thumbnail too, if needs be. - if let Some((info, new_source)) = - thumbnail_info.as_ref().zip(sent_media.thumbnail.clone()) - { - // Previously the media request used `MediaFormat::Thumbnail`. Handle this case - // for send queue requests that were in the state store before the change. - let from_req = if let Some((height, width)) = info.height.zip(info.width) { - Media::make_local_thumbnail_media_request(&info.txn, height, width) - } else { - Media::make_local_file_media_request(&info.txn) - }; - - trace!(from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); - - // The media can now be removed during cleanups. - cache_store - .set_ignore_media_retention_policy( - &from_req, - IgnoreMediaRetentionPolicy::No, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + // Store the sent media under the original cache key for later insertion into + // the local echo. + let from_req = Media::make_local_file_media_request(&file_upload_txn); + sent_infos.insert(from_req.source.unique_key(), sent_media.clone()); - cache_store - .replace_media_key( - &from_req, - &MediaRequestParameters { - source: new_source, - format: MediaFormat::File, - }, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - } - } + update_media_cache_keys_after_upload( + client, + &file_upload_txn, + thumbnail_info, + &sent_media.into(), + ) + .await?; } update_gallery_event_after_upload(&mut local_echo, sent_infos); @@ -1052,3 +939,62 @@ impl QueueStorage { Ok(Some(any_content)) } } + +/// Update cache keys in the cache store after uploading a media file / +/// thumbnail. +async fn update_media_cache_keys_after_upload( + client: &Client, + file_upload_txn: &OwnedTransactionId, + thumbnail_info: Option, + sent_media: &SentMediaInfo, +) -> Result<(), RoomSendQueueError> { + // Do it for the file itself. + let from_req = Media::make_local_file_media_request(file_upload_txn); + + trace!(from = ?from_req.source, to = ?sent_media.file, "renaming media file key in cache store"); + let cache_store = + client.event_cache_store().lock().await.map_err(RoomSendQueueStorageError::LockError)?; + + // The media can now be removed during cleanups. + cache_store + .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + cache_store + .replace_media_key( + &from_req, + &MediaRequestParameters { source: sent_media.file.clone(), format: MediaFormat::File }, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + // Rename the thumbnail too, if needs be. + if let Some((info, new_source)) = thumbnail_info.as_ref().zip(sent_media.thumbnail.clone()) { + // Previously the media request used `MediaFormat::Thumbnail`. Handle this case + // for send queue requests that were in the state store before the change. + let from_req = if let Some((height, width)) = info.height.zip(info.width) { + Media::make_local_thumbnail_media_request(&info.txn, height, width) + } else { + Media::make_local_file_media_request(&info.txn) + }; + + trace!(from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); + + // The media can now be removed during cleanups. + cache_store + .set_ignore_media_retention_policy(&from_req, IgnoreMediaRetentionPolicy::No) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + cache_store + .replace_media_key( + &from_req, + &MediaRequestParameters { source: new_source, format: MediaFormat::File }, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + } + + Ok(()) +} From aaa392c9901d133fbdbbc4786176454118384e9d Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 26 May 2025 20:10:33 +0200 Subject: [PATCH 17/23] Extract method to push thumbnail & media requests --- crates/matrix-sdk/src/send_queue/mod.rs | 231 +++++++++++------------- 1 file changed, 107 insertions(+), 124 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 16248351990..89b55c8feef 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -144,7 +144,7 @@ use matrix_sdk_base::{ event_cache::store::EventCacheStoreError, media::MediaRequestParameters, store::{ - ChildTransactionId, DependentQueuedRequest, DependentQueuedRequestKind, + ChildTransactionId, DependentQueuedRequest, DependentQueuedRequestKind, DynStateStore, FinishUploadThumbnailInfo, QueueWedgeError, QueuedRequest, QueuedRequestKind, SentMediaInfo, SentRequestKey, SerializableEventContent, }, @@ -1204,69 +1204,18 @@ impl QueueStorage { let guard = self.store.lock().await; let client = guard.client()?; let store = client.state_store(); - let thumbnail_info = - if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = - thumbnail - { - let upload_thumbnail_txn = thumbnail_info.txn.clone(); - - // Save the thumbnail upload request. - store - .save_send_queue_request( - &self.room_id, - upload_thumbnail_txn.clone(), - created_at, - QueuedRequestKind::MediaUpload { - content_type: thumbnail_content_type.to_string(), - cache_key: thumbnail_media_request, - thumbnail_source: None, // the thumbnail has no thumbnails :) - related_to: send_event_txn.clone(), - #[cfg(feature = "unstable-msc4274")] - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; - // Save the file upload request as a dependent request of the thumbnail upload. - store - .save_dependent_queued_request( - &self.room_id, - &upload_thumbnail_txn, - upload_file_txn.clone().into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request, - related_to: send_event_txn.clone(), - #[cfg(feature = "unstable-msc4274")] - parent_is_thumbnail_upload: true, - }, - ) - .await?; - - Some(thumbnail_info) - } else { - // Save the file upload as its own request, not a dependent one. - store - .save_send_queue_request( - &self.room_id, - upload_file_txn.clone(), - created_at, - QueuedRequestKind::MediaUpload { - content_type: content_type.to_string(), - cache_key: file_media_request, - thumbnail_source: None, - related_to: send_event_txn.clone(), - #[cfg(feature = "unstable-msc4274")] - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; - - None - }; + let thumbnail_info = self + .push_thumbnail_and_media_uploads( + store, + &content_type, + send_event_txn.clone(), + created_at, + upload_file_txn.clone(), + file_media_request, + thumbnail, + ) + .await?; // Push the dependent request for the event itself. store @@ -1314,70 +1263,21 @@ impl QueueStorage { thumbnail, } = first; - let thumbnail_info = - if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = - thumbnail - { - let upload_thumbnail_txn = thumbnail_info.txn.clone(); - - // Save the thumbnail upload request. - store - .save_send_queue_request( - &self.room_id, - upload_thumbnail_txn.clone(), - created_at, - QueuedRequestKind::MediaUpload { - content_type: thumbnail_content_type.to_string(), - cache_key: thumbnail_media_request.clone(), - thumbnail_source: None, // the thumbnail has no thumbnails :) - related_to: send_event_txn.clone(), - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; - - // Save the file upload request as a dependent request of the thumbnail upload. - store - .save_dependent_queued_request( - &self.room_id, - &upload_thumbnail_txn, - upload_file_txn.clone().into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request.clone(), - related_to: send_event_txn.clone(), - parent_is_thumbnail_upload: true, - }, - ) - .await?; - - Some(thumbnail_info) - } else { - // Save the file upload as its own request, not a dependent one. - store - .save_send_queue_request( - &self.room_id, - upload_file_txn.clone(), - created_at, - QueuedRequestKind::MediaUpload { - content_type: content_type.to_string(), - cache_key: file_media_request.clone(), - thumbnail_source: None, - related_to: send_event_txn.clone(), - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; - - None - }; + let thumbnail_info = self + .push_thumbnail_and_media_uploads( + store, + content_type, + send_event_txn.clone(), + created_at, + upload_file_txn.clone(), + file_media_request.clone(), + thumbnail.clone(), + ) + .await?; finish_item_infos.push(FinishGalleryItemInfo { file_upload: upload_file_txn.clone(), - thumbnail_info: thumbnail_info.cloned(), + thumbnail_info, }); last_upload_file_txn = Some(upload_file_txn.clone()); @@ -1479,6 +1379,89 @@ impl QueueStorage { Ok(()) } + /// If a thumbnail exists, pushes a [`QueuedRequestKind::MediaUpload`] to + /// upload it + /// and a [`DependentQueuedRequestKind::UploadFileOrThumbnail`] to upload + /// the media itself. Otherwise, pushes a + /// [`QueuedRequestKind::MediaUpload`] to upload the media directly. + #[allow(clippy::too_many_arguments)] + async fn push_thumbnail_and_media_uploads( + &self, + store: &DynStateStore, + content_type: &Mime, + send_event_txn: OwnedTransactionId, + created_at: MilliSecondsSinceUnixEpoch, + upload_file_txn: OwnedTransactionId, + file_media_request: MediaRequestParameters, + thumbnail: Option<(FinishUploadThumbnailInfo, MediaRequestParameters, Mime)>, + ) -> Result, RoomSendQueueStorageError> { + let thumbnail_info = + if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = + thumbnail + { + let upload_thumbnail_txn = thumbnail_info.txn.clone(); + + // Save the thumbnail upload request. + store + .save_send_queue_request( + &self.room_id, + upload_thumbnail_txn.clone(), + created_at, + QueuedRequestKind::MediaUpload { + content_type: thumbnail_content_type.to_string(), + cache_key: thumbnail_media_request, + thumbnail_source: None, // the thumbnail has no thumbnails :) + related_to: send_event_txn.clone(), + #[cfg(feature = "unstable-msc4274")] + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; + + // Save the file upload request as a dependent request of the thumbnail upload. + store + .save_dependent_queued_request( + &self.room_id, + &upload_thumbnail_txn, + upload_file_txn.into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request, + related_to: send_event_txn, + #[cfg(feature = "unstable-msc4274")] + parent_is_thumbnail_upload: true, + }, + ) + .await?; + + Some(thumbnail_info) + } else { + // Save the file upload as its own request, not a dependent one. + store + .save_send_queue_request( + &self.room_id, + upload_file_txn, + created_at, + QueuedRequestKind::MediaUpload { + content_type: content_type.to_string(), + cache_key: file_media_request, + thumbnail_source: None, + related_to: send_event_txn, + #[cfg(feature = "unstable-msc4274")] + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; + + None + }; + + Ok(thumbnail_info) + } + /// Reacts to the given local echo of an event. #[instrument(skip(self))] async fn react( From 26601a40baf9e1d1b438050e113de3586595fa7b Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 26 May 2025 20:10:40 +0200 Subject: [PATCH 18/23] Move pushing the dependent media request out of the if / else --- crates/matrix-sdk/src/send_queue/mod.rs | 48 +++++++++---------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 89b55c8feef..34533bb2923 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -1315,43 +1315,29 @@ impl QueueStorage { ) .await?; - // Save the file upload request as a dependent request of the thumbnail upload. - store - .save_dependent_queued_request( - &self.room_id, - &upload_thumbnail_txn, - upload_file_txn.clone().into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request.clone(), - related_to: send_event_txn.clone(), - parent_is_thumbnail_upload: true, - }, - ) - .await?; + last_upload_file_txn = Some(upload_thumbnail_txn); Some(thumbnail_info) } else { - // Save the file upload as a dependent request of the last file upload. - store - .save_dependent_queued_request( - &self.room_id, - &last_upload_file_txn.unwrap(), - upload_file_txn.clone().into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request.clone(), - related_to: send_event_txn.clone(), - parent_is_thumbnail_upload: false, - }, - ) - .await?; - None }; + // Save the file upload as a dependent request of the previous upload. + store + .save_dependent_queued_request( + &self.room_id, + &last_upload_file_txn.unwrap(), + upload_file_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: thumbnail.is_some(), + }, + ) + .await?; + finish_item_infos.push(FinishGalleryItemInfo { file_upload: upload_file_txn.clone(), thumbnail_info: thumbnail_info.cloned(), From b5b67233b298d02395da7f8addddfd8bf2c488eb Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 26 May 2025 20:10:41 +0200 Subject: [PATCH 19/23] Avoid unwrapping last_upload_file_txn --- crates/matrix-sdk/src/send_queue/mod.rs | 112 +++++++++++------------- 1 file changed, 52 insertions(+), 60 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 34533bb2923..66b64d23323 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -1251,50 +1251,43 @@ impl QueueStorage { let client = guard.client()?; let store = client.state_store(); - let mut last_upload_file_txn: Option = None; - let mut finish_item_infos = Vec::with_capacity(item_queue_infos.len()); - if let Some((first, rest)) = item_queue_infos.split_first() { + let Some((first, rest)) = item_queue_infos.split_first() else { + return Ok(()); + }; + + let GalleryItemQueueInfo { content_type, upload_file_txn, file_media_request, thumbnail } = + first; + + let thumbnail_info = self + .push_thumbnail_and_media_uploads( + store, + content_type, + send_event_txn.clone(), + created_at, + upload_file_txn.clone(), + file_media_request.clone(), + thumbnail.clone(), + ) + .await?; + + finish_item_infos + .push(FinishGalleryItemInfo { file_upload: upload_file_txn.clone(), thumbnail_info }); + + let mut last_upload_file_txn = upload_file_txn.clone(); + + for item_queue_info in rest { let GalleryItemQueueInfo { content_type, upload_file_txn, file_media_request, thumbnail, - } = first; - - let thumbnail_info = self - .push_thumbnail_and_media_uploads( - store, - content_type, - send_event_txn.clone(), - created_at, - upload_file_txn.clone(), - file_media_request.clone(), - thumbnail.clone(), - ) - .await?; - - finish_item_infos.push(FinishGalleryItemInfo { - file_upload: upload_file_txn.clone(), - thumbnail_info, - }); + } = item_queue_info; - last_upload_file_txn = Some(upload_file_txn.clone()); - - for item_queue_info in rest { - let GalleryItemQueueInfo { - content_type, - upload_file_txn, - file_media_request, - thumbnail, - } = item_queue_info; - - let thumbnail_info = if let Some(( - thumbnail_info, - thumbnail_media_request, - thumbnail_content_type, - )) = thumbnail + let thumbnail_info = + if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = + thumbnail { let upload_thumbnail_txn = thumbnail_info.txn.clone(); @@ -1303,7 +1296,7 @@ impl QueueStorage { store .save_dependent_queued_request( &self.room_id, - &last_upload_file_txn.unwrap(), + &last_upload_file_txn, upload_thumbnail_txn.clone().into(), created_at, DependentQueuedRequestKind::UploadFileOrThumbnail { @@ -1315,36 +1308,35 @@ impl QueueStorage { ) .await?; - last_upload_file_txn = Some(upload_thumbnail_txn); + last_upload_file_txn = upload_thumbnail_txn; Some(thumbnail_info) } else { None }; - // Save the file upload as a dependent request of the previous upload. - store - .save_dependent_queued_request( - &self.room_id, - &last_upload_file_txn.unwrap(), - upload_file_txn.clone().into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request.clone(), - related_to: send_event_txn.clone(), - parent_is_thumbnail_upload: thumbnail.is_some(), - }, - ) - .await?; + // Save the file upload as a dependent request of the previous upload. + store + .save_dependent_queued_request( + &self.room_id, + &last_upload_file_txn, + upload_file_txn.clone().into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request.clone(), + related_to: send_event_txn.clone(), + parent_is_thumbnail_upload: thumbnail.is_some(), + }, + ) + .await?; - finish_item_infos.push(FinishGalleryItemInfo { - file_upload: upload_file_txn.clone(), - thumbnail_info: thumbnail_info.cloned(), - }); + finish_item_infos.push(FinishGalleryItemInfo { + file_upload: upload_file_txn.clone(), + thumbnail_info: thumbnail_info.cloned(), + }); - last_upload_file_txn = Some(upload_file_txn.clone()); - } + last_upload_file_txn = upload_file_txn.clone(); } // Push the request for the event itself as a dependent request of the last file @@ -1352,7 +1344,7 @@ impl QueueStorage { store .save_dependent_queued_request( &self.room_id, - &last_upload_file_txn.unwrap(), + &last_upload_file_txn, send_event_txn.into(), created_at, DependentQueuedRequestKind::FinishGallery { From 9a491b699fa3fcc5ad90957c7d095126d005fdb6 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 26 May 2025 20:10:43 +0200 Subject: [PATCH 20/23] Extract method for caching --- crates/matrix-sdk/src/send_queue/upload.rs | 192 +++++++++------------ 1 file changed, 84 insertions(+), 108 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index d0b39f4d850..b1845f45064 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -36,7 +36,10 @@ use mime::Mime; use ruma::events::room::message::{GalleryItemType, GalleryMessageEventContent}; use ruma::{ events::{ - room::message::{FormattedBody, MessageType, RoomMessageEventContent}, + room::{ + message::{FormattedBody, MessageType, RoomMessageEventContent}, + MediaSource, ThumbnailInfo, + }, AnyMessageLikeEventContent, Mentions, }, MilliSecondsSinceUnixEpoch, OwnedTransactionId, TransactionId, @@ -45,7 +48,7 @@ use tracing::{debug, error, instrument, trace, warn, Span}; use super::{QueueStorage, RoomSendQueue, RoomSendQueueError}; use crate::{ - attachment::AttachmentConfig, + attachment::{AttachmentConfig, Thumbnail}, room::edit::update_media_caption, send_queue::{ LocalEcho, LocalEchoContent, MediaHandles, RoomSendQueueStorageError, RoomSendQueueUpdate, @@ -163,6 +166,14 @@ fn update_gallery_event_after_upload( } } +#[derive(Default)] + +struct MediaCacheResult { + upload_thumbnail_txn: Option, + event_thumbnail_info: Option<(MediaSource, Box)>, + queue_thumbnail_info: Option<(FinishUploadThumbnailInfo, MediaRequestParameters, Mime)>, +} + impl RoomSendQueue { /// Queues an attachment to be sent to the room, using the send queue. /// @@ -208,59 +219,9 @@ impl RoomSendQueue { let file_media_request = Media::make_local_file_media_request(&upload_file_txn); - let (upload_thumbnail_txn, event_thumbnail_info, queue_thumbnail_info) = { - let client = room.client(); - let cache_store = client - .event_cache_store() - .lock() - .await - .map_err(RoomSendQueueStorageError::LockError)?; - - // Cache the file itself in the cache store. - cache_store - .add_media_content( - &file_media_request, - data.clone(), - // Make sure that the file is stored until it has been uploaded. - IgnoreMediaRetentionPolicy::Yes, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - // Process the thumbnail, if it's been provided. - if let Some(thumbnail) = config.thumbnail.take() { - let txn = TransactionId::new(); - trace!(upload_thumbnail_txn = %txn, "attachment has a thumbnail"); - - // Create the information required for filling the thumbnail section of the - // media event. - let (data, content_type, thumbnail_info) = thumbnail.into_parts(); - - // Cache thumbnail in the cache store. - let thumbnail_media_request = Media::make_local_file_media_request(&txn); - cache_store - .add_media_content( - &thumbnail_media_request, - data, - // Make sure that the thumbnail is stored until it has been uploaded. - IgnoreMediaRetentionPolicy::Yes, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - ( - Some(txn.clone()), - Some((thumbnail_media_request.source.clone(), thumbnail_info)), - Some(( - FinishUploadThumbnailInfo { txn, width: None, height: None }, - thumbnail_media_request, - content_type, - )), - ) - } else { - Default::default() - } - }; + let MediaCacheResult { upload_thumbnail_txn, event_thumbnail_info, queue_thumbnail_info } = + RoomSendQueue::cache_media(&room, data, config.thumbnail.take(), &file_media_request) + .await?; // Create the content for the media event. let event_content = room @@ -365,13 +326,6 @@ impl RoomSendQueue { let mut item_queue_infos = Vec::with_capacity(config.len()); let mut media_handles = Vec::with_capacity(config.len()); - let client = room.client(); - let cache_store = client - .event_cache_store() - .lock() - .await - .map_err(RoomSendQueueStorageError::LockError)?; - for item_info in config.items { let GalleryItemInfo { filename, content_type, data, .. } = item_info; @@ -381,52 +335,12 @@ impl RoomSendQueue { let file_media_request = Media::make_local_file_media_request(&upload_file_txn); - let (upload_thumbnail_txn, event_thumbnail_info, queue_thumbnail_info) = { - // Cache the file itself in the cache store. - cache_store - .add_media_content( - &file_media_request, - data.clone(), - // Make sure that the file is stored until it has been uploaded. - IgnoreMediaRetentionPolicy::Yes, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - // Process the thumbnail, if it's been provided. - if let Some(thumbnail) = item_info.thumbnail { - let txn = TransactionId::new(); - trace!(upload_thumbnail_txn = %txn, "gallery item has a thumbnail"); - - // Create the information required for filling the thumbnail section of the - // gallery event. - let (data, content_type, thumbnail_info) = thumbnail.into_parts(); - - // Cache thumbnail in the cache store. - let thumbnail_media_request = Media::make_local_file_media_request(&txn); - cache_store - .add_media_content( - &thumbnail_media_request, - data, - // Make sure that the thumbnail is stored until it has been uploaded. - IgnoreMediaRetentionPolicy::Yes, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; - - ( - Some(txn.clone()), - Some((thumbnail_media_request.source.clone(), thumbnail_info)), - Some(( - FinishUploadThumbnailInfo { txn, width: None, height: None }, - thumbnail_media_request, - content_type, - )), - ) - } else { - Default::default() - } - }; + let MediaCacheResult { + upload_thumbnail_txn, + event_thumbnail_info, + queue_thumbnail_info, + } = RoomSendQueue::cache_media(&room, data, item_info.thumbnail, &file_media_request) + .await?; item_types.push(Room::make_gallery_item_type( &content_type, @@ -498,6 +412,68 @@ impl RoomSendQueue { Ok(send_handle) } + + async fn cache_media( + room: &Room, + data: Vec, + thumbnail: Option, + file_media_request: &MediaRequestParameters, + ) -> Result { + let client = room.client(); + let cache_store = client + .event_cache_store() + .lock() + .await + .map_err(RoomSendQueueStorageError::LockError)?; + + // Cache the file itself in the cache store. + cache_store + .add_media_content( + file_media_request, + data, + // Make sure that the file is stored until it has been uploaded. + IgnoreMediaRetentionPolicy::Yes, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + // Process the thumbnail, if it's been provided. + if let Some(thumbnail) = thumbnail { + let txn = TransactionId::new(); + trace!(upload_thumbnail_txn = %txn, "media has a thumbnail"); + + // Create the information required for filling the thumbnail section of the + // event. + let (data, content_type, thumbnail_info) = thumbnail.into_parts(); + + // Cache thumbnail in the cache store. + let thumbnail_media_request = Media::make_local_file_media_request(&txn); + cache_store + .add_media_content( + &thumbnail_media_request, + data, + // Make sure that the thumbnail is stored until it has been uploaded. + IgnoreMediaRetentionPolicy::Yes, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + + Ok(MediaCacheResult { + upload_thumbnail_txn: Some(txn.clone()), + event_thumbnail_info: Some(( + thumbnail_media_request.source.clone(), + thumbnail_info, + )), + queue_thumbnail_info: Some(( + FinishUploadThumbnailInfo { txn, width: None, height: None }, + thumbnail_media_request, + content_type, + )), + }) + } else { + Ok(Default::default()) + } + } } impl QueueStorage { From 946e05a7196fb3701b5e0663ca7876345242d3f1 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 27 May 2025 10:46:10 +0200 Subject: [PATCH 21/23] Rename config parameter to gallery --- crates/matrix-sdk/src/send_queue/upload.rs | 23 ++++++++++--------- .../tests/integration/send_queue.rs | 4 ++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index b1845f45064..bb13c953e8c 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -304,7 +304,7 @@ impl RoomSendQueue { #[instrument(skip_all, fields(event_txn))] pub async fn send_gallery( &self, - config: GalleryConfig, + gallery: GalleryConfig, ) -> Result { let Some(room) = self.inner.room.get() else { return Err(RoomSendQueueError::RoomDisappeared); @@ -314,19 +314,20 @@ impl RoomSendQueue { return Err(RoomSendQueueError::RoomNotJoined); } - if config.is_empty() { + if gallery.is_empty() { return Err(RoomSendQueueError::EmptyGallery); } - let send_event_txn = config.txn_id.clone().map_or_else(ChildTransactionId::new, Into::into); + let send_event_txn = + gallery.txn_id.clone().map_or_else(ChildTransactionId::new, Into::into); Span::current().record("event_txn", tracing::field::display(&*send_event_txn)); - let mut item_types = Vec::with_capacity(config.len()); - let mut item_queue_infos = Vec::with_capacity(config.len()); - let mut media_handles = Vec::with_capacity(config.len()); + let mut item_types = Vec::with_capacity(gallery.len()); + let mut item_queue_infos = Vec::with_capacity(gallery.len()); + let mut media_handles = Vec::with_capacity(gallery.len()); - for item_info in config.items { + for item_info in gallery.items { let GalleryItemInfo { filename, content_type, data, .. } = item_info; let upload_file_txn = TransactionId::new(); @@ -366,12 +367,12 @@ impl RoomSendQueue { let event_content = room .make_media_event( MessageType::Gallery(GalleryMessageEventContent::new( - config.caption.unwrap_or_default(), - config.formatted_caption, + gallery.caption.unwrap_or_default(), + gallery.formatted_caption, item_types, )), - config.mentions, - config.reply, + gallery.mentions, + gallery.reply, ) .await .map_err(|_| RoomSendQueueError::FailedToCreateGallery)?; diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 836762693d2..28b17dd28ba 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -2104,7 +2104,7 @@ async fn test_gallery_uploads() { let transaction_id = TransactionId::new(); let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]); - let config = GalleryConfig::new() + let gallery = GalleryConfig::new() .txn_id(transaction_id.clone()) .add_item(GalleryItemInfo { attachment_info: attachment_info1, @@ -2171,7 +2171,7 @@ async fn test_gallery_uploads() { // ---------------------- // Send the media. assert!(watch.is_empty()); - q.send_gallery(config).await.expect("queuing the gallery works"); + q.send_gallery(gallery).await.expect("queuing the gallery works"); // ---------------------- // Observe the local echo. From 662d6c79530aca0a7b5db5ba3d3140670264af5e Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 27 May 2025 10:52:50 +0200 Subject: [PATCH 22/23] Remove superfluous blank line --- crates/matrix-sdk/src/send_queue/upload.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index bb13c953e8c..ae1fca8fa1f 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -167,7 +167,6 @@ fn update_gallery_event_after_upload( } #[derive(Default)] - struct MediaCacheResult { upload_thumbnail_txn: Option, event_thumbnail_info: Option<(MediaSource, Box)>, From f235e56a562b032b112030e66cce6752d9589d6c Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 27 May 2025 10:57:24 +0200 Subject: [PATCH 23/23] Reduce indentation by returning early --- crates/matrix-sdk/src/send_queue/mod.rs | 117 ++++++++++++------------ 1 file changed, 56 insertions(+), 61 deletions(-) diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 66b64d23323..17025a18279 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -1373,71 +1373,66 @@ impl QueueStorage { file_media_request: MediaRequestParameters, thumbnail: Option<(FinishUploadThumbnailInfo, MediaRequestParameters, Mime)>, ) -> Result, RoomSendQueueStorageError> { - let thumbnail_info = - if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = - thumbnail - { - let upload_thumbnail_txn = thumbnail_info.txn.clone(); - - // Save the thumbnail upload request. - store - .save_send_queue_request( - &self.room_id, - upload_thumbnail_txn.clone(), - created_at, - QueuedRequestKind::MediaUpload { - content_type: thumbnail_content_type.to_string(), - cache_key: thumbnail_media_request, - thumbnail_source: None, // the thumbnail has no thumbnails :) - related_to: send_event_txn.clone(), - #[cfg(feature = "unstable-msc4274")] - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; + if let Some((thumbnail_info, thumbnail_media_request, thumbnail_content_type)) = thumbnail { + let upload_thumbnail_txn = thumbnail_info.txn.clone(); - // Save the file upload request as a dependent request of the thumbnail upload. - store - .save_dependent_queued_request( - &self.room_id, - &upload_thumbnail_txn, - upload_file_txn.into(), - created_at, - DependentQueuedRequestKind::UploadFileOrThumbnail { - content_type: content_type.to_string(), - cache_key: file_media_request, - related_to: send_event_txn, - #[cfg(feature = "unstable-msc4274")] - parent_is_thumbnail_upload: true, - }, - ) - .await?; + // Save the thumbnail upload request. + store + .save_send_queue_request( + &self.room_id, + upload_thumbnail_txn.clone(), + created_at, + QueuedRequestKind::MediaUpload { + content_type: thumbnail_content_type.to_string(), + cache_key: thumbnail_media_request, + thumbnail_source: None, // the thumbnail has no thumbnails :) + related_to: send_event_txn.clone(), + #[cfg(feature = "unstable-msc4274")] + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; - Some(thumbnail_info) - } else { - // Save the file upload as its own request, not a dependent one. - store - .save_send_queue_request( - &self.room_id, - upload_file_txn, - created_at, - QueuedRequestKind::MediaUpload { - content_type: content_type.to_string(), - cache_key: file_media_request, - thumbnail_source: None, - related_to: send_event_txn, - #[cfg(feature = "unstable-msc4274")] - accumulated: vec![], - }, - Self::LOW_PRIORITY, - ) - .await?; + // Save the file upload request as a dependent request of the thumbnail upload. + store + .save_dependent_queued_request( + &self.room_id, + &upload_thumbnail_txn, + upload_file_txn.into(), + created_at, + DependentQueuedRequestKind::UploadFileOrThumbnail { + content_type: content_type.to_string(), + cache_key: file_media_request, + related_to: send_event_txn, + #[cfg(feature = "unstable-msc4274")] + parent_is_thumbnail_upload: true, + }, + ) + .await?; - None - }; + Ok(Some(thumbnail_info)) + } else { + // Save the file upload as its own request, not a dependent one. + store + .save_send_queue_request( + &self.room_id, + upload_file_txn, + created_at, + QueuedRequestKind::MediaUpload { + content_type: content_type.to_string(), + cache_key: file_media_request, + thumbnail_source: None, + related_to: send_event_txn, + #[cfg(feature = "unstable-msc4274")] + accumulated: vec![], + }, + Self::LOW_PRIORITY, + ) + .await?; - Ok(thumbnail_info) + Ok(None) + } } /// Reacts to the given local echo of an event.