Skip to content

Commit

Permalink
feat!(timeline): Allow to send attachment from bytes
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
zecakeh committed Dec 22, 2024
1 parent 5992616 commit 2a0973f
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 19 deletions.
8 changes: 8 additions & 0 deletions crates/matrix-sdk-ui/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 15 additions & 13 deletions crates/matrix-sdk-ui/src/timeline/futures.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(),
Expand Down Expand Up @@ -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();
Expand Down
57 changes: 53 additions & 4 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
///
Expand All @@ -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
Expand Down Expand Up @@ -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())
}
}
103 changes: 101 additions & 2 deletions crates/matrix-sdk-ui/tests/integration/timeline/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 2a0973f

Please sign in to comment.