diff --git a/Cargo.lock b/Cargo.lock index 04130da61d5..5f4e0db3000 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2871,6 +2871,7 @@ dependencies = [ "backoff", "bytes", "bytesize", + "cfg-if", "dirs 6.0.0", "event-listener", "eyeball", @@ -3203,6 +3204,7 @@ dependencies = [ name = "matrix-sdk-sqlite" version = "0.10.0" dependencies = [ + "as_variant", "assert_matches", "async-trait", "deadpool-sqlite", diff --git a/crates/matrix-sdk-base/CHANGELOG.md b/crates/matrix-sdk-base/CHANGELOG.md index 9bf938aba88..313779ac64c 100644 --- a/crates/matrix-sdk-base/CHANGELOG.md +++ b/crates/matrix-sdk-base/CHANGELOG.md @@ -37,6 +37,12 @@ All notable changes to this project will be documented in this file. - [**breaking**] `BaseClient::set_session_metadata` is renamed `activate`, and `BaseClient::logged_in` is renamed `is_activated` ([#4850](https://github.com/matrix-org/matrix-rust-sdk/pull/4850)) +- [**breaking] `DependentQueuedRequestKind::UploadFileWithThumbnail` + was renamed to `DependentQueuedRequestKind::UploadFileOrThumbnail`. + Under the `unstable-msc4274` feature, `DependentQueuedRequestKind::UploadFileOrThumbnail` + and `SentMediaInfo` were generalized to allow chaining multiple dependent + file / thumbnail uploads. + ([#4897](https://github.com/matrix-org/matrix-rust-sdk/pull/4897)) ## [0.10.0] - 2025-02-04 diff --git a/crates/matrix-sdk-base/Cargo.toml b/crates/matrix-sdk-base/Cargo.toml index 330744efe90..3cd2326ca69 100644 --- a/crates/matrix-sdk-base/Cargo.toml +++ b/crates/matrix-sdk-base/Cargo.toml @@ -43,6 +43,9 @@ testing = [ "matrix-sdk-crypto?/testing", ] +# Add support for inline media galleries via msgtypes +unstable-msc4274 = [] + [dependencies] as_variant = { workspace = true } assert_matches = { workspace = true, optional = true } diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index 461fc89b6c3..6cf2b821a95 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -71,6 +71,8 @@ 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::{ 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 099476d255e..1d8f3e8c972 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -103,6 +103,12 @@ pub enum QueuedRequestKind { /// To which media event transaction does this upload relate? related_to: OwnedTransactionId, + + /// Accumulated list of infos for previously uploaded files and + /// thumbnails if used during a gallery transaction. Otherwise empty. + #[cfg(feature = "unstable-msc4274")] + #[serde(default)] + accumulated: Vec, }, } @@ -219,17 +225,23 @@ pub enum DependentQueuedRequestKind { key: String, }, - /// Upload a file that had a thumbnail. - UploadFileWithThumbnail { - /// Content type for the file itself (not the thumbnail). + /// Upload a file or thumbnail depending on another file or thumbnail + /// upload. + #[serde(alias = "UploadFileWithThumbnail")] + UploadFileOrThumbnail { + /// Content type for the file or thumbnail. content_type: String, - /// Media request necessary to retrieve the file itself (not the - /// thumbnail). + /// Media request necessary to retrieve the file or thumbnail itself. cache_key: MediaRequestParameters, /// To which media transaction id does this upload relate to? related_to: OwnedTransactionId, + + /// Whether the depended upon request was a thumbnail or a file upload. + #[cfg(feature = "unstable-msc4274")] + #[serde(default = "default_parent_is_thumbnail_upload")] + parent_is_thumbnail_upload: bool, }, /// Finish an upload by updating references to the media cache and sending @@ -248,6 +260,14 @@ pub enum DependentQueuedRequestKind { }, } +/// If parent_is_thumbnail_upload is missing, we assume the request is for a +/// file upload following a thumbnail upload. This was the only possible case +/// before parent_is_thumbnail_upload was introduced. +#[cfg(feature = "unstable-msc4274")] +fn default_parent_is_thumbnail_upload() -> bool { + true +} + /// Detailed record about a thumbnail used when finishing a media upload. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct FinishUploadThumbnailInfo { @@ -310,7 +330,7 @@ impl From for ChildTransactionId { } } -/// Information about a media (and its thumbnail) that have been sent to an +/// Information about a media (and its thumbnail) that have been sent to a /// homeserver. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct SentMediaInfo { @@ -324,6 +344,29 @@ pub struct SentMediaInfo { /// /// When uploading a thumbnail, this is set to `None`. pub thumbnail: Option, + + /// Accumulated list of infos for previously uploaded files and thumbnails + /// if used during a gallery transaction. Otherwise empty. + #[cfg(feature = "unstable-msc4274")] + #[serde(default)] + pub accumulated: Vec, +} + +/// Accumulated information about a media (and its thumbnail) that have been +/// sent to a homeserver. +#[cfg(feature = "unstable-msc4274")] +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct AccumulatedSentMediaInfo { + /// File that was uploaded by this request. + /// + /// If the request related to a thumbnail upload, this contains the + /// thumbnail media source. + pub file: MediaSource, + + /// Optional thumbnail previously uploaded, when uploading a file. + /// + /// When uploading a thumbnail, this is set to `None`. + pub thumbnail: Option, } /// A unique key (identifier) indicating that a transaction has been @@ -390,7 +433,7 @@ impl DependentQueuedRequest { DependentQueuedRequestKind::EditEvent { .. } | DependentQueuedRequestKind::RedactEvent | DependentQueuedRequestKind::ReactEvent { .. } - | DependentQueuedRequestKind::UploadFileWithThumbnail { .. } => { + | DependentQueuedRequestKind::UploadFileOrThumbnail { .. } => { // These are all aggregated events, or non-visible items (file upload producing // a new MXC ID). false diff --git a/crates/matrix-sdk-sqlite/Cargo.toml b/crates/matrix-sdk-sqlite/Cargo.toml index ac32218dded..fc567c959f3 100644 --- a/crates/matrix-sdk-sqlite/Cargo.toml +++ b/crates/matrix-sdk-sqlite/Cargo.toml @@ -17,6 +17,7 @@ event-cache = ["dep:matrix-sdk-base"] state-store = ["dep:matrix-sdk-base"] [dependencies] +as_variant = { workspace = true } async-trait = { workspace = true } deadpool-sqlite = "0.9.0" itertools = { workspace = true } diff --git a/crates/matrix-sdk-sqlite/src/state_store.rs b/crates/matrix-sdk-sqlite/src/state_store.rs index 8c496595843..3fa52b89e39 100644 --- a/crates/matrix-sdk-sqlite/src/state_store.rs +++ b/crates/matrix-sdk-sqlite/src/state_store.rs @@ -2237,8 +2237,10 @@ mod migration_tests { }, }; + use as_variant::as_variant; use deadpool_sqlite::Runtime; use matrix_sdk_base::{ + media::{MediaFormat, MediaRequestParameters}, store::{ ChildTransactionId, DependentQueuedRequestKind, RoomLoadSettings, SerializableEventContent, @@ -2250,13 +2252,14 @@ mod migration_tests { use once_cell::sync::Lazy; use ruma::{ events::{ - room::{create::RoomCreateEventContent, message::RoomMessageEventContent}, + room::{create::RoomCreateEventContent, message::RoomMessageEventContent, MediaSource}, StateEventType, }, - room_id, server_name, user_id, EventId, MilliSecondsSinceUnixEpoch, RoomId, TransactionId, - UserId, + room_id, server_name, user_id, EventId, MilliSecondsSinceUnixEpoch, OwnedTransactionId, + RoomId, TransactionId, UserId, }; use rusqlite::Transaction; + use serde::{Deserialize, Serialize}; use serde_json::json; use tempfile::{tempdir, TempDir}; use tokio::fs; @@ -2597,4 +2600,41 @@ mod migration_tests { Ok(()) } + + #[derive(Clone, Debug, Serialize, Deserialize)] + pub enum LegacyDependentQueuedRequestKind { + UploadFileWithThumbnail { + content_type: String, + cache_key: MediaRequestParameters, + related_to: OwnedTransactionId, + }, + } + + #[async_test] + pub async fn test_dependent_queued_request_variant_renaming() { + let path = new_path(); + let db = create_fake_db(&path, 7).await.unwrap(); + + let cache_key = MediaRequestParameters { + format: MediaFormat::File, + source: MediaSource::Plain("https://server.local/foobar".into()), + }; + let related_to = TransactionId::new(); + let request = LegacyDependentQueuedRequestKind::UploadFileWithThumbnail { + content_type: "image/png".to_owned(), + cache_key, + related_to: related_to.clone(), + }; + + let data = db + .serialize_json(&request) + .expect("should be able to serialize legacy dependent request"); + let deserialized: DependentQueuedRequestKind = db.deserialize_json(&data).expect( + "should be able to deserialize dependent request from legacy dependent request", + ); + + as_variant!(deserialized, DependentQueuedRequestKind::UploadFileOrThumbnail { related_to: de_related_to, .. } => { + assert_eq!(de_related_to, related_to); + }); + } } diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml index 11ec5ea6523..6c688c2d946 100644 --- a/crates/matrix-sdk/Cargo.toml +++ b/crates/matrix-sdk/Cargo.toml @@ -49,6 +49,9 @@ experimental-widgets = ["dep:uuid"] docsrs = ["e2e-encryption", "sqlite", "indexeddb", "sso-login", "qrcode"] +# Add support for inline media galleries via msgtypes +unstable-msc4274 = ["matrix-sdk-base/unstable-msc4274"] + [dependencies] anyhow = { workspace = true, optional = true } anymap2 = "0.13.0" @@ -112,6 +115,7 @@ urlencoding = "2.1.3" uuid = { workspace = true, features = ["serde", "v4"], optional = true } vodozemac = { workspace = true } zeroize = { workspace = true } +cfg-if = "1.0.0" [target.'cfg(target_arch = "wasm32")'.dependencies] gloo-timers = { workspace = true, features = ["futures"] } diff --git a/crates/matrix-sdk/src/send_queue/mod.rs b/crates/matrix-sdk/src/send_queue/mod.rs index 7ad431afe57..a5b223a455a 100644 --- a/crates/matrix-sdk/src/send_queue/mod.rs +++ b/crates/matrix-sdk/src/send_queue/mod.rs @@ -109,8 +109,8 @@ //! - the thumbnail is sent first as an [`QueuedRequestKind::MediaUpload`] //! request, //! - the file upload is pushed as a dependent request of kind -//! [`DependentQueuedRequestKind::UploadFileWithThumbnail`] (this variant -//! keeps the file's key used to look it up in the cache store). +//! [`DependentQueuedRequestKind::UploadFileOrThumbnail`] (this variant keeps +//! the file's key used to look it up in the cache store). //! - the media event is then sent as a dependent request as described in the //! previous section. //! @@ -699,6 +699,8 @@ impl RoomSendQueue { cache_key, thumbnail_source, related_to: relates_to, + #[cfg(feature = "unstable-msc4274")] + accumulated, } => { trace!(%relates_to, "uploading media related to event"); @@ -757,6 +759,8 @@ impl RoomSendQueue { Ok(SentRequestKey::Media(SentMediaInfo { file: media_source, thumbnail: thumbnail_source, + #[cfg(feature = "unstable-msc4274")] + accumulated, })) }; @@ -1215,6 +1219,8 @@ impl QueueStorage { 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, ) @@ -1227,10 +1233,12 @@ impl QueueStorage { &upload_thumbnail_txn, upload_file_txn.clone().into(), created_at, - DependentQueuedRequestKind::UploadFileWithThumbnail { + 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?; @@ -1248,6 +1256,8 @@ impl QueueStorage { cache_key: file_media_request, thumbnail_source: None, related_to: send_event_txn.clone(), + #[cfg(feature = "unstable-msc4274")] + accumulated: vec![], }, Self::LOW_PRIORITY, ) @@ -1376,7 +1386,7 @@ impl QueueStorage { }, }), - DependentQueuedRequestKind::UploadFileWithThumbnail { .. } => { + DependentQueuedRequestKind::UploadFileOrThumbnail { .. } => { // Don't reflect these: only the associated event is interesting to observers. None } @@ -1589,22 +1599,37 @@ impl QueueStorage { } } - DependentQueuedRequestKind::UploadFileWithThumbnail { + DependentQueuedRequestKind::UploadFileOrThumbnail { content_type, cache_key, related_to, + #[cfg(feature = "unstable-msc4274")] + parent_is_thumbnail_upload, } => { let Some(parent_key) = parent_key else { // Not finished yet, we should retry later => false. return Ok(false); }; - self.handle_dependent_file_upload_with_thumbnail( + let parent_is_thumbnail_upload = { + cfg_if::cfg_if! { + if #[cfg(feature = "unstable-msc4274")] { + parent_is_thumbnail_upload + } else { + // Before parent_is_thumbnail_upload was introduced, the only + // possible usage for this request was a file upload following + // a thumbnail upload. + true + } + } + }; + self.handle_dependent_file_or_thumbnail_upload( client, dependent_request.own_transaction_id.into(), parent_key, content_type, cache_key, related_to, + parent_is_thumbnail_upload, ) .await?; } @@ -2209,7 +2234,7 @@ fn canonicalize_dependent_requests( } } - DependentQueuedRequestKind::UploadFileWithThumbnail { .. } + DependentQueuedRequestKind::UploadFileOrThumbnail { .. } | DependentQueuedRequestKind::FinishUpload { .. } | DependentQueuedRequestKind::ReactEvent { .. } => { // These requests can't be canonicalized, push them as is. diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index c41c3a314c0..fdc432808b7 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -14,6 +14,8 @@ //! Private implementations of the media upload mechanism. +#[cfg(feature = "unstable-msc4274")] +use matrix_sdk_base::store::AccumulatedSentMediaInfo; use matrix_sdk_base::{ event_cache::store::media::IgnoreMediaRetentionPolicy, media::{MediaFormat, MediaRequestParameters}, @@ -347,8 +349,10 @@ impl QueueStorage { Ok(()) } - /// Consumes a finished upload of a thumbnail and queues the file upload. - pub(super) async fn handle_dependent_file_upload_with_thumbnail( + /// Consumes a finished file or thumbnail upload and queues the dependent + /// file or thumbnail upload. + #[allow(clippy::too_many_arguments)] + pub(super) async fn handle_dependent_file_or_thumbnail_upload( &self, client: &Client, next_upload_txn: OwnedTransactionId, @@ -356,28 +360,51 @@ impl QueueStorage { content_type: String, cache_key: MediaRequestParameters, event_txn: OwnedTransactionId, + parent_is_thumbnail_upload: bool, ) -> Result<(), RoomSendQueueError> { - // The thumbnail has been sent, now transform the dependent file upload request - // into a ready one. + // The previous file or thumbnail has been sent, now transform the dependent + // file or thumbnail upload request into a ready one. let sent_media = parent_key .into_media() .ok_or(RoomSendQueueError::StorageError(RoomSendQueueStorageError::InvalidParentKey))?; - // The media we just uploaded was a thumbnail, so the thumbnail shouldn't have + // If the previous upload was a thumbnail, it shouldn't have // a thumbnail itself. - debug_assert!(sent_media.thumbnail.is_none()); - if sent_media.thumbnail.is_some() { - warn!("unexpected thumbnail for a thumbnail!"); + if parent_is_thumbnail_upload { + debug_assert!(sent_media.thumbnail.is_none()); + if sent_media.thumbnail.is_some() { + warn!("unexpected thumbnail for a thumbnail!"); + } } - trace!(related_to = %event_txn, "done uploading thumbnail, now queuing a request to send the media file itself"); + trace!(related_to = %event_txn, "done uploading file or thumbnail, now queuing the dependent file or thumbnail upload request"); + + // If the parent request was a thumbnail upload, don't add it to the list of + // accumulated medias yet because its dependent file upload is still + // pending. If the parent request was a file upload, we know that both + // the file and its thumbnail (if any) have finished uploading and we + // can add them to the accumulated sent media. + #[cfg(feature = "unstable-msc4274")] + let accumulated = if parent_is_thumbnail_upload { + sent_media.accumulated + } else { + let mut accumulated = sent_media.accumulated; + accumulated.push(AccumulatedSentMediaInfo { + file: sent_media.file.clone(), + thumbnail: sent_media.thumbnail, + }); + accumulated + }; let request = QueuedRequestKind::MediaUpload { content_type, cache_key, - // The thumbnail for the next upload is the file we just uploaded here. - thumbnail_source: Some(sent_media.file), + // If the previous upload was a thumbnail, it becomes the thumbnail source for the next + // upload. + thumbnail_source: parent_is_thumbnail_upload.then_some(sent_media.file), related_to: event_txn, + #[cfg(feature = "unstable-msc4274")] + accumulated, }; client