Skip to content

Commit 2a0973f

Browse files
committed
feat!(timeline): Allow to send attachment from bytes
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 <zecakeh@tedomum.fr>
1 parent 5992616 commit 2a0973f

File tree

4 files changed

+177
-19
lines changed

4 files changed

+177
-19
lines changed

crates/matrix-sdk-ui/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ All notable changes to this project will be documented in this file.
1313
non-left room filter did not take the new room stat into account.
1414
([#4448](https://github.com/matrix-org/matrix-rust-sdk/pull/4448))
1515

16+
### Features
17+
18+
- [**breaking**] `Timeline::send_attachment()` now takes a type that implements
19+
`Into<AttachmentSource>` instead of a type that implements `Into<PathBuf>`.
20+
`AttachmentSource` allows to send an attachment either from a file, or with
21+
the bytes and the filename of the attachment. Note that all types that
22+
implement `Into<PathBuf>` also implement `Into<AttachmentSource>`.
23+
1624
## [0.9.0] - 2024-12-18
1725

1826
### Bug Fixes

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
use std::{fs, future::IntoFuture, path::PathBuf};
1+
use std::future::IntoFuture;
22

33
use eyeball::{SharedObservable, Subscriber};
44
use matrix_sdk::{attachment::AttachmentConfig, TransmissionProgress};
55
use matrix_sdk_base::boxed_into_future;
66
use mime::Mime;
77
use tracing::{Instrument as _, Span};
88

9-
use super::{Error, Timeline};
9+
use super::{AttachmentSource, Error, Timeline};
1010

1111
pub struct SendAttachment<'a> {
1212
timeline: &'a Timeline,
13-
path: PathBuf,
13+
source: AttachmentSource,
1414
mime_type: Mime,
1515
config: AttachmentConfig,
1616
tracing_span: Span,
@@ -21,13 +21,13 @@ pub struct SendAttachment<'a> {
2121
impl<'a> SendAttachment<'a> {
2222
pub(crate) fn new(
2323
timeline: &'a Timeline,
24-
path: PathBuf,
24+
source: AttachmentSource,
2525
mime_type: Mime,
2626
config: AttachmentConfig,
2727
) -> Self {
2828
Self {
2929
timeline,
30-
path,
30+
source,
3131
mime_type,
3232
config,
3333
tracing_span: Span::current(),
@@ -61,16 +61,18 @@ impl<'a> IntoFuture for SendAttachment<'a> {
6161
boxed_into_future!(extra_bounds: 'a);
6262

6363
fn into_future(self) -> Self::IntoFuture {
64-
let Self { timeline, path, mime_type, config, tracing_span, use_send_queue, send_progress } =
65-
self;
64+
let Self {
65+
timeline,
66+
source,
67+
mime_type,
68+
config,
69+
tracing_span,
70+
use_send_queue,
71+
send_progress,
72+
} = self;
6673

6774
let fut = async move {
68-
let filename = path
69-
.file_name()
70-
.ok_or(Error::InvalidAttachmentFileName)?
71-
.to_str()
72-
.ok_or(Error::InvalidAttachmentFileName)?;
73-
let data = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?;
75+
let (data, filename) = source.try_into_bytes_and_filename()?;
7476

7577
if use_send_queue {
7678
let send_queue = timeline.room().send_queue();

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

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//!
1717
//! See [`Timeline`] for details.
1818
19-
use std::{path::PathBuf, pin::Pin, sync::Arc, task::Poll};
19+
use std::{fs, path::PathBuf, pin::Pin, sync::Arc, task::Poll};
2020

2121
use event_item::{extract_room_msg_edit_content, TimelineItemHandle};
2222
use eyeball_im::VectorDiff;
@@ -540,7 +540,7 @@ impl Timeline {
540540
///
541541
/// # Arguments
542542
///
543-
/// * `path` - The path of the file to be sent.
543+
/// * `source` - The source of the attachment to send.
544544
///
545545
/// * `mime_type` - The attachment's mime type.
546546
///
@@ -551,11 +551,11 @@ impl Timeline {
551551
#[instrument(skip_all)]
552552
pub fn send_attachment(
553553
&self,
554-
path: impl Into<PathBuf>,
554+
source: impl Into<AttachmentSource>,
555555
mime_type: Mime,
556556
config: AttachmentConfig,
557557
) -> SendAttachment<'_> {
558-
SendAttachment::new(self, path.into(), mime_type, config)
558+
SendAttachment::new(self, source.into(), mime_type, config)
559559
}
560560

561561
/// Redact an event given its [`TimelineEventItemId`] and an optional
@@ -885,3 +885,52 @@ impl<S: Stream> Stream for TimelineStream<S> {
885885

886886
pub type TimelineEventFilterFn =
887887
dyn Fn(&AnySyncTimelineEvent, &RoomVersionId) -> bool + Send + Sync;
888+
889+
/// A source for sending an attachment.
890+
///
891+
/// The [`AttachmentSource::File`] variant can be constructed from any type that
892+
/// implements `Into<PathBuf>`.
893+
#[derive(Debug, Clone)]
894+
pub enum AttachmentSource {
895+
/// The data of the attachment.
896+
Data {
897+
/// The bytes of the attachment.
898+
bytes: Vec<u8>,
899+
900+
/// The filename of the attachment.
901+
filename: String,
902+
},
903+
904+
/// An attachment loaded from a file.
905+
///
906+
/// The bytes and the filename will be read from the file at the given path.
907+
File(PathBuf),
908+
}
909+
910+
impl AttachmentSource {
911+
/// Try to convert this attachment source into a `(bytes, filename)` tuple.
912+
pub(crate) fn try_into_bytes_and_filename(self) -> Result<(Vec<u8>, String), Error> {
913+
match self {
914+
Self::Data { bytes, filename } => Ok((bytes, filename)),
915+
Self::File(path) => {
916+
let filename = path
917+
.file_name()
918+
.ok_or(Error::InvalidAttachmentFileName)?
919+
.to_str()
920+
.ok_or(Error::InvalidAttachmentFileName)?
921+
.to_owned();
922+
let bytes = fs::read(&path).map_err(|_| Error::InvalidAttachmentData)?;
923+
Ok((bytes, filename))
924+
}
925+
}
926+
}
927+
}
928+
929+
impl<P> From<P> for AttachmentSource
930+
where
931+
P: Into<PathBuf>,
932+
{
933+
fn from(value: P) -> Self {
934+
Self::File(value.into())
935+
}
936+
}

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

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use matrix_sdk::{
2222
assert_let_timeout, attachment::AttachmentConfig, test_utils::mocks::MatrixMockServer,
2323
};
2424
use matrix_sdk_test::{async_test, event_factory::EventFactory, JoinedRoomBuilder, ALICE};
25-
use matrix_sdk_ui::timeline::{EventSendState, RoomExt, TimelineItemContent};
25+
use matrix_sdk_ui::timeline::{AttachmentSource, EventSendState, RoomExt, TimelineItemContent};
2626
use ruma::{
2727
event_id,
2828
events::room::{message::MessageType, MediaSource},
@@ -52,7 +52,7 @@ fn get_filename_and_caption(msg: &MessageType) -> (&str, Option<&str>) {
5252
}
5353

5454
#[async_test]
55-
async fn test_send_attachment() {
55+
async fn test_send_attachment_from_file() {
5656
let mock = MatrixMockServer::new().await;
5757
let client = mock.client_builder().build().await;
5858

@@ -148,6 +148,105 @@ async fn test_send_attachment() {
148148
assert!(timeline_stream.next().now_or_never().is_none());
149149
}
150150

151+
#[async_test]
152+
async fn test_send_attachment_from_bytes() {
153+
let mock = MatrixMockServer::new().await;
154+
let client = mock.client_builder().build().await;
155+
156+
mock.mock_room_state_encryption().plain().mount().await;
157+
158+
let room_id = room_id!("!a98sd12bjh:example.org");
159+
let room = mock.sync_joined_room(&client, room_id).await;
160+
let timeline = room.timeline().await.unwrap();
161+
162+
let (items, mut timeline_stream) =
163+
timeline.subscribe_filter_map(|item| item.as_event().cloned()).await;
164+
165+
assert!(items.is_empty());
166+
167+
let f = EventFactory::new();
168+
mock.sync_room(
169+
&client,
170+
JoinedRoomBuilder::new(room_id).add_timeline_event(f.text_msg("hello").sender(&ALICE)),
171+
)
172+
.await;
173+
174+
// Sanity check.
175+
assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next());
176+
assert_let!(TimelineItemContent::Message(msg) = item.content());
177+
assert_eq!(msg.body(), "hello");
178+
179+
// No other updates.
180+
assert!(timeline_stream.next().now_or_never().is_none());
181+
182+
// The data of the file.
183+
let filename = "test.bin";
184+
let source =
185+
AttachmentSource::Data { bytes: b"hello world".to_vec(), filename: filename.to_owned() };
186+
187+
// Set up mocks for the file upload.
188+
mock.mock_upload()
189+
.respond_with(ResponseTemplate::new(200).set_delay(Duration::from_secs(2)).set_body_json(
190+
json!({
191+
"content_uri": "mxc://sdk.rs/media"
192+
}),
193+
))
194+
.mock_once()
195+
.mount()
196+
.await;
197+
198+
mock.mock_room_send().ok(event_id!("$media")).mock_once().mount().await;
199+
200+
// Queue sending of an attachment.
201+
let config = AttachmentConfig::new().caption(Some("caption".to_owned()));
202+
timeline.send_attachment(source, mime::TEXT_PLAIN, config).use_send_queue().await.unwrap();
203+
204+
{
205+
assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next());
206+
assert_matches!(item.send_state(), Some(EventSendState::NotSentYet));
207+
assert_let!(TimelineItemContent::Message(msg) = item.content());
208+
209+
// Body is the caption, because there's both a caption and filename.
210+
assert_eq!(msg.body(), "caption");
211+
assert_eq!(get_filename_and_caption(msg.msgtype()), (filename, Some("caption")));
212+
213+
// The URI refers to the local cache.
214+
assert_let!(MessageType::File(file) = msg.msgtype());
215+
assert_let!(MediaSource::Plain(uri) = &file.source);
216+
assert!(uri.to_string().contains("localhost"));
217+
}
218+
219+
// Eventually, the media is updated with the final MXC IDs…
220+
sleep(Duration::from_secs(2)).await;
221+
222+
{
223+
assert_let_timeout!(
224+
Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next()
225+
);
226+
assert_let!(TimelineItemContent::Message(msg) = item.content());
227+
assert_matches!(item.send_state(), Some(EventSendState::NotSentYet));
228+
assert_eq!(get_filename_and_caption(msg.msgtype()), (filename, Some("caption")));
229+
230+
// The URI now refers to the final MXC URI.
231+
assert_let!(MessageType::File(file) = msg.msgtype());
232+
assert_let!(MediaSource::Plain(uri) = &file.source);
233+
assert_eq!(uri.to_string(), "mxc://sdk.rs/media");
234+
}
235+
236+
// And eventually the event itself is sent.
237+
{
238+
assert_let_timeout!(
239+
Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next()
240+
);
241+
assert_matches!(item.send_state(), Some(EventSendState::Sent{ event_id }) => {
242+
assert_eq!(event_id, event_id!("$media"));
243+
});
244+
}
245+
246+
// That's all, folks!
247+
assert!(timeline_stream.next().now_or_never().is_none());
248+
}
249+
151250
#[async_test]
152251
async fn test_react_to_local_media() {
153252
let mock = MatrixMockServer::new().await;

0 commit comments

Comments
 (0)