refactor(send_queue): generalize SentRequestKey::Media and DependentQueuedRequestKind::UploadFileWithThumbnail to prepare for MSC4274 gallery uploads (matrix-org#4897)

This was broken out of
matrix-org#4838 and is a
preliminary step towards implementing
[MSC4274](matrix-org/matrix-spec-proposals#4274).
`SentRequestKey::Media` and
`DependentQueuedRequestKind::UploadFileWithThumbnail` are generalized to
allow chaining dependent media uploads and accumulating sent media
sources.

- [x] Public API changes documented in changelogs (optional)

---------

Signed-off-by: Johannes Marbach <[email protected]>
Co-authored-by: Benjamin Bouvier <[email protected]>

Author: Johennes

Cargo.lock

@@ -40,6 +40,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
 

crates/matrix-sdk-base/CHANGELOG.md

@@ -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 }

crates/matrix-sdk-base/Cargo.toml

@@ -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::{

crates/matrix-sdk-base/src/store/mod.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<AccumulatedSentMediaInfo>,
     },
 }
 
@@ -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<OwnedTransactionId> 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<MediaSource>,
+
+    /// 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<AccumulatedSentMediaInfo>,
+}
+
+/// 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<MediaSource>,
 }
 
 /// 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

crates/matrix-sdk-base/src/store/send_queue.rs

@@ -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.10.0"
 itertools = { workspace = true }

crates/matrix-sdk-sqlite/Cargo.toml

@@ -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);
+        });
+    }
 }

crates/matrix-sdk-sqlite/src/state_store.rs

@@ -50,6 +50,9 @@ experimental-widgets = ["dep:uuid"]
 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"]
+
 [dependencies]
 anyhow = { workspace = true, optional = true }
 anymap2 = "0.13.0"
@@ -114,6 +117,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"] }

crates/matrix-sdk/Cargo.toml

@@ -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.