From 2a0973fbb8695a28ef07cc8958559bb926e2af13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sat, 21 Dec 2024 16:33:24 +0100 Subject: [PATCH] feat!(timeline): Allow to send attachment from bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes we can get the bytes directly. It avoids to have to write the data to a temporary file only to have the data loaded back in memory right after. Signed-off-by: Kévin Commaille --- crates/matrix-sdk-ui/CHANGELOG.md | 8 ++ crates/matrix-sdk-ui/src/timeline/futures.rs | 28 ++--- crates/matrix-sdk-ui/src/timeline/mod.rs | 57 +++++++++- .../tests/integration/timeline/media.rs | 103 +++++++++++++++++- 4 files changed, 177 insertions(+), 19 deletions(-) diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 1be4ef45ec8..9aa4649cbf7 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/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` instead of a type that implements `Into`. + `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` also implement `Into`. + ## [0.9.0] - 2024-12-18 ### Bug Fixes diff --git a/crates/matrix-sdk-ui/src/timeline/futures.rs b/crates/matrix-sdk-ui/src/timeline/futures.rs index 546b8ef5407..8109e0cb8c9 100644 --- a/crates/matrix-sdk-ui/src/timeline/futures.rs +++ b/crates/matrix-sdk-ui/src/timeline/futures.rs @@ -1,4 +1,4 @@ -use std::{fs, future::IntoFuture, path::PathBuf}; +use std::future::IntoFuture; use eyeball::{SharedObservable, Subscriber}; use matrix_sdk::{attachment::AttachmentConfig, TransmissionProgress}; @@ -6,11 +6,11 @@ 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(); diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 927180093cc..e4ce251b5fc 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/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, + source: impl Into, 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 Stream for TimelineStream { 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`. +#[derive(Debug, Clone)] +pub enum AttachmentSource { + /// The data of the attachment. + Data { + /// The bytes of the attachment. + bytes: Vec, + + /// 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, 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

From

for AttachmentSource +where + P: Into, +{ + fn from(value: P) -> Self { + Self::File(value.into()) + } +} diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/media.rs b/crates/matrix-sdk-ui/tests/integration/timeline/media.rs index e11ea3a8283..d552b4d38ff 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/media.rs +++ b/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;