Skip to content

Commit

Permalink
feat(media)!: introduce a custom media upload request to avoid clonin…
Browse files Browse the repository at this point in the history
…g data

Changelog: all upload related queries now take an `&[u8]` instead of a
`Vec<u8>`, avoiding the need for a copy.
  • Loading branch information
bnjbvr committed Oct 21, 2024
1 parent c03228d commit 722dd22
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 21 deletions.
6 changes: 3 additions & 3 deletions bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,9 @@ impl Client {
Ok(())
}

pub async fn upload_avatar(&self, mime_type: String, data: Vec<u8>) -> Result<(), ClientError> {
pub async fn upload_avatar(&self, mime_type: String, data: &[u8]) -> Result<(), ClientError> {
let mime: Mime = mime_type.parse()?;
self.inner.account().upload_avatar(&mime, data).await?;
self.inner.account().upload_avatar(&mime, &data).await?;
Ok(())
}

Expand Down Expand Up @@ -675,7 +675,7 @@ impl Client {
pub async fn upload_media(
&self,
mime_type: String,
data: Vec<u8>,
data: &[u8],
progress_watcher: Option<Box<dyn ProgressWatcher>>,
) -> Result<String, ClientError> {
let mime_type: mime::Mime = mime_type.parse().context("Parsing mime type")?;
Expand Down
2 changes: 1 addition & 1 deletion bindings/matrix-sdk-ffi/src/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ impl Room {
pub async fn upload_avatar(
&self,
mime_type: String,
data: Vec<u8>,
data: &[u8],
media_info: Option<ImageInfo>,
) -> Result<(), ClientError> {
let mime: Mime = mime_type.parse()?;
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl Account {
/// ```
///
/// [`Media::upload()`]: crate::Media::upload
pub async fn upload_avatar(&self, content_type: &Mime, data: Vec<u8>) -> Result<OwnedMxcUri> {
pub async fn upload_avatar(&self, content_type: &Mime, data: &[u8]) -> Result<OwnedMxcUri> {
let upload_response = self.client.media().upload(content_type, data).await?;
self.set_avatar_url(Some(&upload_response.content_uri)).await?;
Ok(upload_response.content_uri)
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/encryption/futures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ where

let response = client
.media()
.upload(content_type, buf)
.upload(content_type, &buf)
.with_send_progress_observable(send_progress)
.await?;

Expand Down
123 changes: 116 additions & 7 deletions crates/matrix-sdk/src/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl fmt::Display for PersistError {
}

/// `IntoFuture` returned by [`Media::upload`].
pub type SendUploadRequest<'a> = SendRequest<'a, media::create_content::v3::Request>;
pub type SendUploadRequest<'a> = SendRequest<'a, custom_upload::Request<'a>>;

impl Media {
pub(crate) fn new(client: Client) -> Self {
Expand Down Expand Up @@ -140,15 +140,13 @@ impl Media {
/// println!("Cat URI: {}", response.content_uri);
/// # anyhow::Ok(()) };
/// ```
pub fn upload(&self, content_type: &Mime, data: Vec<u8>) -> SendUploadRequest<'_> {
pub fn upload<'a>(&'a self, content_type: &Mime, data: &'a [u8]) -> SendUploadRequest<'a> {
let timeout = std::cmp::max(
Duration::from_secs(data.len() as u64 / DEFAULT_UPLOAD_SPEED),
MIN_UPLOAD_REQUEST_TIMEOUT,
);

let request = assign!(media::create_content::v3::Request::new(data), {
content_type: Some(content_type.essence_str().to_owned()),
});
let request = custom_upload::Request::new(data, content_type.essence_str().to_owned());

let request_config = self.client.request_config().timeout(timeout);
self.client.send(request, Some(request_config))
Expand Down Expand Up @@ -496,7 +494,7 @@ impl Media {
pub(crate) async fn upload_plain_media_and_thumbnail(
&self,
content_type: &Mime,
data: Vec<u8>,
data: &[u8],
thumbnail: Option<Thumbnail>,
send_progress: SharedObservable<TransmissionProgress>,
) -> Result<(MediaSource, Option<MediaSource>, Option<Box<ThumbnailInfo>>)> {
Expand Down Expand Up @@ -527,7 +525,7 @@ impl Media {
};

let response = self
.upload(&thumbnail.content_type, thumbnail.data)
.upload(&thumbnail.content_type, &thumbnail.data)
.with_send_progress_observable(send_progress)
.await?;
let url = response.content_uri;
Expand All @@ -543,3 +541,114 @@ impl Media {
Ok((Some(MediaSource::Plain(url)), Some(Box::new(thumbnail_info))))
}
}

/// Custom upload request that doesn't take bytes by ownership but by reference.
mod custom_upload {
use ruma::{
api::{client::error::Error, error::FromHttpResponseError, Metadata, OutgoingRequest},
metadata, OwnedMxcUri,
};

const METADATA: Metadata = metadata! {
method: POST,
rate_limited: true,
authentication: AccessToken,
history: {
1.0 => "/_matrix/media/r0/upload",
1.1 => "/_matrix/media/v3/upload",
}
};

/// Request type for the `create_media_content` endpoint.
#[derive(Clone)]
pub struct Request<'a> {
/// The content type of the file being uploaded.
content_type: String,

/// The file contents to upload.
data: &'a [u8],
}

impl<'a> Request<'a> {
pub fn new(data: &'a [u8], content_type: String) -> Self {
Self { content_type, data }
}
}

#[cfg(not(tarpaulin_include))]
impl<'a> std::fmt::Debug for Request<'a> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Request")
.field("content_type", &self.content_type)
.finish_non_exhaustive()
}
}

impl<'a> OutgoingRequest for Request<'a> {
type EndpointError = Error;
type IncomingResponse = Response;
const METADATA: Metadata = METADATA;

fn try_into_http_request<T: Default + bytes::BufMut>(
self,
base_url: &str,
access_token: ruma::api::SendAccessToken<'_>,
considering_versions: &'_ [ruma::api::MatrixVersion],
) -> Result<http::Request<T>, ruma::api::error::IntoHttpError> {
let url = METADATA.make_endpoint_url(considering_versions, base_url, &[], "")?;
let body = self.data;

let mut builder = http::Request::builder().method(METADATA.method).uri(url);

if let Some((header_name, header_value)) =
METADATA.authorization_header(access_token)?
{
builder = builder.header(header_name, header_value);
}

let builder = builder
.header(http::header::CONTENT_TYPE, self.content_type)
.header(http::header::CONTENT_LENGTH, body.len())
.body(ruma::serde::slice_to_buf(body))?;

Ok(builder)
}
}

/// Response type for the `create_media_content` endpoint.
#[derive(serde::Deserialize, Debug)]
pub struct Response {
/// The MXC URI for the uploaded content.
pub content_uri: OwnedMxcUri,

/// The [BlurHash](https://blurha.sh) for the uploaded content.
///
/// This uses the unstable prefix in
/// [MSC2448](https://github.com/matrix-org/matrix-spec-proposals/pull/2448).
#[serde(
rename = "xyz.amorgan.blurhash",
alias = "blurhash",
skip_serializing_if = "Option::is_none"
)]
pub blurhash: Option<String>,
}

impl ruma::api::IncomingResponse for Response {
type EndpointError = Error;

fn try_from_http_response<T: AsRef<[u8]>>(
response: http::Response<T>,
) -> Result<Self, FromHttpResponseError<Self::EndpointError>> {
use ruma::api::EndpointError;

if response.status().as_u16() >= 400 {
return Err(FromHttpResponseError::Server(
Self::EndpointError::from_http_response(response),
));
}

let body = response.into_body();
Ok(serde_json::from_slice(body.as_ref())?)
}
}
}
11 changes: 3 additions & 8 deletions crates/matrix-sdk/src/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1941,20 +1941,15 @@ impl Room {
} else {
self.client
.media()
.upload_plain_media_and_thumbnail(
content_type,
data.clone(),
thumbnail,
send_progress,
)
.upload_plain_media_and_thumbnail(content_type, &data, thumbnail, send_progress)
.await?
};

#[cfg(not(feature = "e2e-encryption"))]
let (media_source, thumbnail_source, thumbnail_info) = self
.client
.media()
.upload_plain_media_and_thumbnail(content_type, data.clone(), thumbnail, send_progress)
.upload_plain_media_and_thumbnail(content_type, &data, thumbnail, send_progress)
.await?;

if store_in_cache {
Expand Down Expand Up @@ -2203,7 +2198,7 @@ impl Room {
pub async fn upload_avatar(
&self,
mime: &Mime,
data: Vec<u8>,
data: &[u8],
info: Option<avatar::ImageInfo>,
) -> Result<send_state_event::v3::Response> {
self.ensure_room_joined()?;
Expand Down

0 comments on commit 722dd22

Please sign in to comment.