refactor: Include file name when uploading unencrypted media

bindings/matrix-sdk-ffi/src/client.rs

@@ -39,8 +39,8 @@ use matrix_sdk::{
     },
     sliding_sync::Version as SdkSlidingSyncVersion,
     store::RoomLoadSettings as SdkRoomLoadSettings,
-    AuthApi, AuthSession, Client as MatrixClient, SessionChange, SessionTokens,
-    STATE_STORE_DATABASE_NAME,
+    AuthApi, AuthSession, Client as MatrixClient, SendMediaUploadRequest, SessionChange,
+    SessionTokens, STATE_STORE_DATABASE_NAME,
 };
 use matrix_sdk_common::{stream::StreamExt, SendOutsideWasm, SyncOutsideWasm};
 use matrix_sdk_ui::{
@@ -1026,21 +1026,24 @@ impl Client {
         &self,
         mime_type: String,
         data: Vec<u8>,
+        filename: Option<String>,
         progress_watcher: Option<Box<dyn ProgressWatcher>>,
     ) -> Result<String, ClientError> {
         let mime_type: mime::Mime = mime_type.parse().context("Parsing mime type")?;
-        let request = self.inner.media().upload(&mime_type, data, None);
+        let send_media_request = SendMediaUploadRequest::new((*self.inner).clone(), data)
+            .with_content_type(mime_type.essence_str().to_owned())
+            .with_filename(filename);
 
         if let Some(progress_watcher) = progress_watcher {
-            let mut subscriber = request.subscribe_to_send_progress();
+            let mut subscriber = send_media_request.subscribe_to_send_progress();
             get_runtime_handle().spawn(async move {
                 while let Some(progress) = subscriber.next().await {
                     progress_watcher.transmission_progress(progress.into());
                 }
             });
         }
 
-        let response = request.await?;
+        let response = self.inner.media().upload(send_media_request).await?;
 
         Ok(String::from(response.content_uri))
     }

bindings/matrix-sdk-ffi/src/timeline/mod.rs

@@ -219,7 +219,11 @@ pub enum UploadSource {
     /// Upload source is a file on disk
     File {
         /// Path to file
-        filename: String,
+        path: String,
+
+        /// An optional filename, if the one in the path is not the one we want
+        /// to use for the file.
+        filename: Option<String>,
     },
     /// Upload source is data in memory
     Data {
@@ -233,7 +237,7 @@ pub enum UploadSource {
 impl From<UploadSource> for AttachmentSource {
     fn from(value: UploadSource) -> Self {
         match value {
-            UploadSource::File { filename } => Self::File(filename.into()),
+            UploadSource::File { path, filename } => Self::File { path: path.into(), filename },
             UploadSource::Data { bytes, filename } => Self::Data { bytes, filename },
         }
     }

crates/matrix-sdk-base/src/media.rs

@@ -142,6 +142,9 @@ pub trait MediaEventContent {
     /// Returns `None` if `Self` has no file.
     fn source(&self) -> Option<MediaSource>;
 
+    /// Get the name of the uploaded file for `Self`.
+    fn filename_or_body(&self) -> Option<String>;
+
     /// Get the source of the thumbnail for `Self`.
     ///
     /// Returns `None` if `Self` has no thumbnail.
@@ -153,6 +156,10 @@ impl MediaEventContent for StickerEventContent {
         Some(MediaSource::from(self.source.clone()))
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        None
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         None
     }
@@ -163,6 +170,14 @@ impl MediaEventContent for AudioMessageEventContent {
         Some(self.source.clone())
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        if let Some(filename) = &self.filename {
+            Some(filename.clone())
+        } else {
+            Some(self.body.clone())
+        }
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         None
     }
@@ -173,6 +188,14 @@ impl MediaEventContent for FileMessageEventContent {
         Some(self.source.clone())
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        if let Some(filename) = &self.filename {
+            Some(filename.clone())
+        } else {
+            Some(self.body.clone())
+        }
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         self.info.as_ref()?.thumbnail_source.clone()
     }
@@ -183,6 +206,14 @@ impl MediaEventContent for ImageMessageEventContent {
         Some(self.source.clone())
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        if let Some(filename) = &self.filename {
+            Some(filename.clone())
+        } else {
+            Some(self.body.clone())
+        }
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         self.info
             .as_ref()
@@ -196,6 +227,14 @@ impl MediaEventContent for VideoMessageEventContent {
         Some(self.source.clone())
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        if let Some(filename) = &self.filename {
+            Some(filename.clone())
+        } else {
+            Some(self.body.clone())
+        }
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         self.info
             .as_ref()
@@ -209,6 +248,10 @@ impl MediaEventContent for LocationMessageEventContent {
         None
     }
 
+    fn filename_or_body(&self) -> Option<String> {
+        None
+    }
+
     fn thumbnail_source(&self) -> Option<MediaSource> {
         self.info.as_ref()?.thumbnail_source.clone()
     }

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

@@ -109,6 +109,9 @@ pub enum QueuedRequestKind {
         #[cfg(feature = "unstable-msc4274")]
         #[serde(default)]
         accumulated: Vec<AccumulatedSentMediaInfo>,
+
+        /// The name of the file to upload, if known or public.
+        filename: Option<String>,
     },
 }
 
@@ -241,6 +244,9 @@ pub enum DependentQueuedRequestKind {
         #[cfg(feature = "unstable-msc4274")]
         #[serde(default = "default_parent_is_thumbnail_upload")]
         parent_is_thumbnail_upload: bool,
+
+        /// The name of the file to upload, if known.
+        filename: Option<String>,
     },
 
     /// Finish an upload by updating references to the media cache and sending

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

@@ -821,21 +821,24 @@ pub enum AttachmentSource {
     /// An attachment loaded from a file.
     ///
     /// The bytes and the filename will be read from the file at the given path.
-    File(PathBuf),
+    File { filename: Option<String>, path: PathBuf },
 }
 
 impl AttachmentSource {
     /// Try to convert this attachment source into a `(bytes, filename)` tuple.
     pub(crate) fn try_into_bytes_and_filename(self) -> Result<(Vec<u8>, String), Error> {
         match self {
             Self::Data { bytes, filename } => Ok((bytes, filename)),
-            Self::File(path) => {
-                let filename = path
-                    .file_name()
-                    .ok_or(Error::InvalidAttachmentFileName)?
-                    .to_str()
-                    .ok_or(Error::InvalidAttachmentFileName)?
-                    .to_owned();
+            Self::File { path, filename } => {
+                let filename = if let Some(filename) = filename {
+                    filename
+                } else {
+                    path.file_name()
+                        .ok_or(Error::InvalidAttachmentFileName)?
+                        .to_str()
+                        .ok_or(Error::InvalidAttachmentFileName)?
+                        .to_owned()
+                };
                 let bytes = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?;
                 Ok((bytes, filename))
             }
@@ -848,7 +851,7 @@ where
     P: Into<PathBuf>,
 {
     fn from(value: P) -> Self {
-        Self::File(value.into())
+        Self::File { path: value.into(), filename: None }
     }
 }
 

crates/matrix-sdk/src/account.rs

@@ -55,7 +55,7 @@ use ruma::{
 use serde::Deserialize;
 use tracing::error;
 
-use crate::{config::RequestConfig, Client, Error, Result};
+use crate::{config::RequestConfig, Client, Error, Result, SendMediaUploadRequest};
 
 /// A high-level API to manage the client owner's account.
 ///
@@ -264,7 +264,10 @@ impl Account {
     ///
     /// [`Media::upload()`]: crate::Media::upload
     pub async fn upload_avatar(&self, content_type: &Mime, data: Vec<u8>) -> Result<OwnedMxcUri> {
-        let upload_response = self.client.media().upload(content_type, data, None).await?;
+        let send_media_request = SendMediaUploadRequest::new(self.client.clone(), data)
+            .with_content_type(content_type.essence_str().to_owned());
+
+        let upload_response = self.client.media().upload(send_media_request).await?;
         self.set_avatar_url(Some(&upload_response.content_uri)).await?;
         Ok(upload_response.content_uri)
     }

crates/matrix-sdk/src/client/futures.rs

@@ -156,17 +156,50 @@ where
     }
 }
 
-/// `IntoFuture` used to send media upload requests. It wraps another
-/// [`SendRequest`], checking its size will be accepted by the homeserver before
-/// uploading.
+/// `IntoFuture` used to send media upload requests. It works as a builder which
+/// can create a [`SendRequest`] given several configuration options, checking
+/// its size will be accepted by the homeserver before uploading.
 #[allow(missing_debug_implementations)]
 pub struct SendMediaUploadRequest {
-    send_request: SendRequest<media::create_content::v3::Request>,
+    client: Client,
+    request: media::create_content::v3::Request,
+    /// The [`RequestConfig`] to use for uploading the media file.
+    pub request_config: Option<RequestConfig>,
+    progress_observable: SharedObservable<TransmissionProgress>,
 }
 
 impl SendMediaUploadRequest {
-    pub fn new(request: SendRequest<media::create_content::v3::Request>) -> Self {
-        Self { send_request: request }
+    /// Creates a new instance.
+    pub fn new(client: Client, file: Vec<u8>) -> Self {
+        Self {
+            client,
+            request: media::create_content::v3::Request::new(file),
+            request_config: Default::default(),
+            progress_observable: SharedObservable::new(TransmissionProgress::default()),
+        }
+    }
+
+    /// Returns the data to upload.
+    pub fn data(&self) -> &Vec<u8> {
+        &self.request.file
+    }
+
+    /// Sets the content type of the media for the media upload request.
+    pub fn with_content_type(mut self, content_type: impl Into<String>) -> Self {
+        self.request.content_type = Some(content_type.into());
+        self
+    }
+
+    /// Sets the filename for the media upload request.
+    pub fn with_filename(mut self, filename: Option<impl Into<String>>) -> Self {
+        self.request.filename = filename.map(Into::into);
+        self
+    }
+
+    /// Applies the provided [`RequestConfig`] to the future [`SendRequest`].
+    pub fn with_request_config(mut self, request_config: Option<RequestConfig>) -> Self {
+        self.request_config = request_config;
+        self
     }
 
     /// Replace the default `SharedObservable` used for tracking upload
@@ -179,14 +212,24 @@ impl SendMediaUploadRequest {
         mut self,
         send_progress: SharedObservable<TransmissionProgress>,
     ) -> Self {
-        self.send_request = self.send_request.with_send_progress_observable(send_progress);
+        self.progress_observable = send_progress;
         self
     }
 
     /// Get a subscriber to observe the progress of sending the request
     /// body.
     pub fn subscribe_to_send_progress(&self) -> Subscriber<TransmissionProgress> {
-        self.send_request.send_progress.subscribe()
+        self.progress_observable.subscribe()
+    }
+
+    /// Creates the [`SendRequest`] using the provided parameters.
+    pub fn build_send_request(self) -> SendRequest<media::create_content::v3::Request> {
+        SendRequest {
+            client: self.client,
+            request: self.request,
+            config: self.request_config,
+            send_progress: self.progress_observable,
+        }
     }
 }
 
@@ -195,9 +238,9 @@ impl IntoFuture for SendMediaUploadRequest {
     boxed_into_future!();
 
     fn into_future(self) -> Self::IntoFuture {
-        let request_length = self.send_request.request.file.len();
-        let client = self.send_request.client.clone();
-        let send_request = self.send_request;
+        let send_request = self.build_send_request();
+        let request_length = send_request.request.file.len();
+        let client = send_request.client.clone();
 
         Box::pin(async move {
             let max_upload_size = client.load_or_fetch_max_upload_size().await?;

crates/matrix-sdk/src/client/mod.rs

@@ -111,7 +111,10 @@ mod builder;
 pub(crate) mod caches;
 pub(crate) mod futures;
 
-pub use self::builder::{sanitize_server_name, ClientBuildError, ClientBuilder};
+pub use self::{
+    builder::{sanitize_server_name, ClientBuildError, ClientBuilder},
+    futures::SendMediaUploadRequest,
+};
 
 #[cfg(not(target_family = "wasm"))]
 type NotificationHandlerFut = Pin<Box<dyn Future<Output = ()> + Send>>;
@@ -2909,7 +2912,6 @@ pub(crate) mod tests {
 
     use assert_matches::assert_matches;
     use assert_matches2::assert_let;
-    use eyeball::SharedObservable;
     use futures_util::{pin_mut, FutureExt};
     use js_int::{uint, UInt};
     use matrix_sdk_base::{
@@ -2950,10 +2952,9 @@ pub(crate) mod tests {
     use crate::{
         client::{futures::SendMediaUploadRequest, WeakClient},
         config::RequestConfig,
-        futures::SendRequest,
         media::MediaError,
         test_utils::{client::MockClientBuilder, mocks::MatrixMockServer},
-        Error, TransmissionProgress,
+        Error,
     };
 
     #[async_test]
@@ -3781,15 +3782,7 @@ pub(crate) mod tests {
         assert_eq!(*client.inner.server_max_upload_size.lock().await.get().unwrap(), uint!(1));
 
         let data = vec![1, 2];
-        let upload_request =
-            ruma::api::client::media::create_content::v3::Request::new(data.clone());
-        let request = SendRequest {
-            client: client.clone(),
-            request: upload_request,
-            config: None,
-            send_progress: SharedObservable::new(TransmissionProgress::default()),
-        };
-        let media_request = SendMediaUploadRequest::new(request);
+        let media_request = SendMediaUploadRequest::new(client, data.clone());
 
         let error = media_request.await.err();
         assert_let!(Some(Error::Media(MediaError::MediaTooLargeToUpload { max, current })) = error);