feat!(timeline): Allow to send attachments from bytes (matrix-org#4451)

Sometimes we can get the bytes directly, e.g. in Fractal we can get an
image from the clipboard. It avoids to have to write the data to a
temporary file only to have the data loaded back in memory by the SDK
right after.

The first commit to accept any type that implements `Into<String>` for
the filename is grouped here because it simplifies slightly the second
commit.

Note that we could also use `AttachmentSource` in the other
`send_attachment` APIs, on `Room` and `RoomSendQueue`, for consistency.

---------

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>

Author: zecakeh

crates/matrix-sdk-ui/CHANGELOG.md

@@ -13,6 +13,14 @@ All notable changes to this project will be documented in this file.
   non-left room filter did not take the new room stat into account.
   ([#4448](https://github.com/matrix-org/matrix-rust-sdk/pull/4448))
 
+### Features
+
+- [**breaking**] `Timeline::send_attachment()` now takes a type that implements
+  `Into<AttachmentSource>` instead of a type that implements `Into<PathBuf>`.
+  `AttachmentSource` allows to send an attachment either from a file, or with
+  the bytes and the filename of the attachment. Note that all types that
+  implement `Into<PathBuf>` also implement `Into<AttachmentSource>`.
+
 ## [0.9.0] - 2024-12-18
 
 ### Bug Fixes

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

@@ -1,16 +1,16 @@
-use std::{fs, future::IntoFuture, path::PathBuf};
+use std::future::IntoFuture;
 
 use eyeball::{SharedObservable, Subscriber};
 use matrix_sdk::{attachment::AttachmentConfig, TransmissionProgress};
 use matrix_sdk_base::boxed_into_future;
 use mime::Mime;
 use tracing::{Instrument as _, Span};
 
-use super::{Error, Timeline};
+use super::{AttachmentSource, Error, Timeline};
 
 pub struct SendAttachment<'a> {
     timeline: &'a Timeline,
-    path: PathBuf,
+    source: AttachmentSource,
     mime_type: Mime,
     config: AttachmentConfig,
     tracing_span: Span,
@@ -21,13 +21,13 @@ pub struct SendAttachment<'a> {
 impl<'a> SendAttachment<'a> {
     pub(crate) fn new(
         timeline: &'a Timeline,
-        path: PathBuf,
+        source: AttachmentSource,
         mime_type: Mime,
         config: AttachmentConfig,
     ) -> Self {
         Self {
             timeline,
-            path,
+            source,
             mime_type,
             config,
             tracing_span: Span::current(),
@@ -61,16 +61,18 @@ impl<'a> IntoFuture for SendAttachment<'a> {
     boxed_into_future!(extra_bounds: 'a);
 
     fn into_future(self) -> Self::IntoFuture {
-        let Self { timeline, path, mime_type, config, tracing_span, use_send_queue, send_progress } =
-            self;
+        let Self {
+            timeline,
+            source,
+            mime_type,
+            config,
+            tracing_span,
+            use_send_queue,
+            send_progress,
+        } = self;
 
         let fut = async move {
-            let filename = path
-                .file_name()
-                .ok_or(Error::InvalidAttachmentFileName)?
-                .to_str()
-                .ok_or(Error::InvalidAttachmentFileName)?;
-            let data = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?;
+            let (data, filename) = source.try_into_bytes_and_filename()?;
 
             if use_send_queue {
                 let send_queue = timeline.room().send_queue();

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

@@ -16,7 +16,7 @@
 //!
 //! See [`Timeline`] for details.
 
-use std::{path::PathBuf, pin::Pin, sync::Arc, task::Poll};
+use std::{fs, path::PathBuf, pin::Pin, sync::Arc, task::Poll};
 
 use event_item::{extract_room_msg_edit_content, TimelineItemHandle};
 use eyeball_im::VectorDiff;
@@ -540,7 +540,7 @@ impl Timeline {
     ///
     /// # Arguments
     ///
-    /// * `path` - The path of the file to be sent.
+    /// * `source` - The source of the attachment to send.
     ///
     /// * `mime_type` - The attachment's mime type.
     ///
@@ -551,11 +551,11 @@ impl Timeline {
     #[instrument(skip_all)]
     pub fn send_attachment(
         &self,
-        path: impl Into<PathBuf>,
+        source: impl Into<AttachmentSource>,
         mime_type: Mime,
         config: AttachmentConfig,
     ) -> SendAttachment<'_> {
-        SendAttachment::new(self, path.into(), mime_type, config)
+        SendAttachment::new(self, source.into(), mime_type, config)
     }
 
     /// Redact an event given its [`TimelineEventItemId`] and an optional
@@ -885,3 +885,52 @@ impl<S: Stream> Stream for TimelineStream<S> {
 
 pub type TimelineEventFilterFn =
     dyn Fn(&AnySyncTimelineEvent, &RoomVersionId) -> bool + Send + Sync;
+
+/// A source for sending an attachment.
+///
+/// The [`AttachmentSource::File`] variant can be constructed from any type that
+/// implements `Into<PathBuf>`.
+#[derive(Debug, Clone)]
+pub enum AttachmentSource {
+    /// The data of the attachment.
+    Data {
+        /// The bytes of the attachment.
+        bytes: Vec<u8>,
+
+        /// The filename of the attachment.
+        filename: String,
+    },
+
+    /// An attachment loaded from a file.
+    ///
+    /// The bytes and the filename will be read from the file at the given path.
+    File(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();
+                let bytes = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?;
+                Ok((bytes, filename))
+            }
+        }
+    }
+}
+
+impl<P> From<P> for AttachmentSource
+where
+    P: Into<PathBuf>,
+{
+    fn from(value: P) -> Self {
+        Self::File(value.into())
+    }
+}

crates/matrix-sdk-ui/tests/integration/timeline/media.rs

@@ -22,7 +22,7 @@ use matrix_sdk::{
     assert_let_timeout, attachment::AttachmentConfig, test_utils::mocks::MatrixMockServer,
 };
 use matrix_sdk_test::{async_test, event_factory::EventFactory, JoinedRoomBuilder, ALICE};
-use matrix_sdk_ui::timeline::{EventSendState, RoomExt, TimelineItemContent};
+use matrix_sdk_ui::timeline::{AttachmentSource, EventSendState, RoomExt, TimelineItemContent};
 use ruma::{
     event_id,
     events::room::{message::MessageType, MediaSource},
@@ -52,7 +52,7 @@ fn get_filename_and_caption(msg: &MessageType) -> (&str, Option<&str>) {
 }
 
 #[async_test]
-async fn test_send_attachment() {
+async fn test_send_attachment_from_file() {
     let mock = MatrixMockServer::new().await;
     let client = mock.client_builder().build().await;
 
@@ -148,6 +148,105 @@ async fn test_send_attachment() {
     assert!(timeline_stream.next().now_or_never().is_none());
 }
 
+#[async_test]
+async fn test_send_attachment_from_bytes() {
+    let mock = MatrixMockServer::new().await;
+    let client = mock.client_builder().build().await;
+
+    mock.mock_room_state_encryption().plain().mount().await;
+
+    let room_id = room_id!("!a98sd12bjh:example.org");
+    let room = mock.sync_joined_room(&client, room_id).await;
+    let timeline = room.timeline().await.unwrap();
+
+    let (items, mut timeline_stream) =
+        timeline.subscribe_filter_map(|item| item.as_event().cloned()).await;
+
+    assert!(items.is_empty());
+
+    let f = EventFactory::new();
+    mock.sync_room(
+        &client,
+        JoinedRoomBuilder::new(room_id).add_timeline_event(f.text_msg("hello").sender(&ALICE)),
+    )
+    .await;
+
+    // Sanity check.
+    assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next());
+    assert_let!(TimelineItemContent::Message(msg) = item.content());
+    assert_eq!(msg.body(), "hello");
+
+    // No other updates.
+    assert!(timeline_stream.next().now_or_never().is_none());
+
+    // The data of the file.
+    let filename = "test.bin";
+    let source =
+        AttachmentSource::Data { bytes: b"hello world".to_vec(), filename: filename.to_owned() };
+
+    // Set up mocks for the file upload.
+    mock.mock_upload()
+        .respond_with(ResponseTemplate::new(200).set_delay(Duration::from_secs(2)).set_body_json(
+            json!({
+              "content_uri": "mxc://sdk.rs/media"
+            }),
+        ))
+        .mock_once()
+        .mount()
+        .await;
+
+    mock.mock_room_send().ok(event_id!("$media")).mock_once().mount().await;
+
+    // Queue sending of an attachment.
+    let config = AttachmentConfig::new().caption(Some("caption".to_owned()));
+    timeline.send_attachment(source, mime::TEXT_PLAIN, config).use_send_queue().await.unwrap();
+
+    {
+        assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next());
+        assert_matches!(item.send_state(), Some(EventSendState::NotSentYet));
+        assert_let!(TimelineItemContent::Message(msg) = item.content());
+
+        // Body is the caption, because there's both a caption and filename.
+        assert_eq!(msg.body(), "caption");
+        assert_eq!(get_filename_and_caption(msg.msgtype()), (filename, Some("caption")));
+
+        // The URI refers to the local cache.
+        assert_let!(MessageType::File(file) = msg.msgtype());
+        assert_let!(MediaSource::Plain(uri) = &file.source);
+        assert!(uri.to_string().contains("localhost"));
+    }
+
+    // Eventually, the media is updated with the final MXC IDs…
+    sleep(Duration::from_secs(2)).await;
+
+    {
+        assert_let_timeout!(
+            Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next()
+        );
+        assert_let!(TimelineItemContent::Message(msg) = item.content());
+        assert_matches!(item.send_state(), Some(EventSendState::NotSentYet));
+        assert_eq!(get_filename_and_caption(msg.msgtype()), (filename, Some("caption")));
+
+        // The URI now refers to the final MXC URI.
+        assert_let!(MessageType::File(file) = msg.msgtype());
+        assert_let!(MediaSource::Plain(uri) = &file.source);
+        assert_eq!(uri.to_string(), "mxc://sdk.rs/media");
+    }
+
+    // And eventually the event itself is sent.
+    {
+        assert_let_timeout!(
+            Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next()
+        );
+        assert_matches!(item.send_state(), Some(EventSendState::Sent{ event_id }) => {
+            assert_eq!(event_id, event_id!("$media"));
+        });
+    }
+
+    // That's all, folks!
+    assert!(timeline_stream.next().now_or_never().is_none());
+}
+
 #[async_test]
 async fn test_react_to_local_media() {
     let mock = MatrixMockServer::new().await;

crates/matrix-sdk/CHANGELOG.md

@@ -15,6 +15,8 @@ All notable changes to this project will be documented in this file.
 - [**breaking**] Remove the `AttachmentConfig::with_thumbnail()` constructor and
   replace it with the `AttachmentConfig::thumbnail()` builder method. You should
   call `AttachmentConfig::new().thumbnail(thumbnail)` now instead.
+- [**breaking**] `Room::send_attachment()` and `RoomSendQueue::send_attachment()`
+  now take any type that implements `Into<String>` for the filename.
 
 ## [0.9.0] - 2024-12-18
 

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

@@ -240,7 +240,7 @@ impl<'a> IntoFuture for SendRawMessageLikeEvent<'a> {
 #[allow(missing_debug_implementations)]
 pub struct SendAttachment<'a> {
     room: &'a Room,
-    filename: &'a str,
+    filename: String,
     content_type: &'a Mime,
     data: Vec<u8>,
     config: AttachmentConfig,
@@ -252,7 +252,7 @@ pub struct SendAttachment<'a> {
 impl<'a> SendAttachment<'a> {
     pub(crate) fn new(
         room: &'a Room,
-        filename: &'a str,
+        filename: String,
         content_type: &'a Mime,
         data: Vec<u8>,
         config: AttachmentConfig,

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

@@ -1933,12 +1933,12 @@ impl Room {
     #[instrument(skip_all)]
     pub fn send_attachment<'a>(
         &'a self,
-        filename: &'a str,
+        filename: impl Into<String>,
         content_type: &'a Mime,
         data: Vec<u8>,
         config: AttachmentConfig,
     ) -> SendAttachment<'a> {
-        SendAttachment::new(self, filename, content_type, data, config)
+        SendAttachment::new(self, filename.into(), content_type, data, config)
     }
 
     /// Prepare and send an attachment to this room.
@@ -1971,7 +1971,7 @@ impl Room {
     #[instrument(skip_all)]
     pub(super) async fn prepare_and_send_attachment<'a>(
         &'a self,
-        filename: &'a str,
+        filename: String,
         content_type: &'a Mime,
         data: Vec<u8>,
         mut config: AttachmentConfig,
@@ -2076,7 +2076,7 @@ impl Room {
     pub(crate) fn make_attachment_type(
         &self,
         content_type: &Mime,
-        filename: &str,
+        filename: String,
         source: MediaSource,
         caption: Option<String>,
         formatted_caption: Option<FormattedBody>,
@@ -2087,8 +2087,8 @@ impl Room {
         // 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.to_owned())),
-            None => (filename.to_owned(), None),
+            Some(caption) => (caption, Some(filename)),
+            None => (filename, None),
         };
 
         let (thumbnail_source, thumbnail_info) = thumbnail.unzip();

crates/matrix-sdk/src/send_queue/upload.rs

@@ -105,7 +105,7 @@ impl RoomSendQueue {
     #[instrument(skip_all, fields(event_txn))]
     pub async fn send_attachment(
         &self,
-        filename: &str,
+        filename: impl Into<String>,
         content_type: Mime,
         data: Vec<u8>,
         mut config: AttachmentConfig,
@@ -118,6 +118,7 @@ impl RoomSendQueue {
             return Err(RoomSendQueueError::RoomNotJoined);
         }
 
+        let filename = filename.into();
         let upload_file_txn = TransactionId::new();
         let send_event_txn = config.txn_id.map_or_else(ChildTransactionId::new, Into::into);