Skip to content

Commit

Permalink
feat(media): cache thumbnails too with a sensible media request key
Browse files Browse the repository at this point in the history
We can't know which key is going to be used precisely for the thumbnail,
so assume non-animated cropped same-size thumbnail media request.

Changelog: when `SendAttachment::store_in_cache()` is set, the thumbnail
is also cached with a sensible default media request (not animated,
cropped, same dimensions as the uploaded thumbnail).
  • Loading branch information
bnjbvr committed Oct 22, 2024
1 parent b46ebbf commit 9c03c5d
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 27 deletions.
47 changes: 46 additions & 1 deletion crates/matrix-sdk/src/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use matrix_sdk_base::{
deserialized_responses::{
RawAnySyncOrStrippedState, RawSyncOrStrippedState, SyncOrStrippedState, TimelineEvent,
},
media::{MediaThumbnailSettings, MediaThumbnailSize},
store::StateStoreExt,
ComposerDraft, RoomInfoNotableUpdateReasons, RoomMemberships, StateChanges, StateStoreDataKey,
StateStoreDataValue,
Expand Down Expand Up @@ -1915,6 +1916,7 @@ impl Room {
///
/// * `store_in_cache` - A boolean defining whether the uploaded media will
/// be stored in the cache immediately after a successful upload.
#[instrument(skip_all)]
pub(super) async fn prepare_and_send_attachment<'a>(
&'a self,
filename: &'a str,
Expand All @@ -1931,6 +1933,20 @@ impl Room {

let thumbnail = config.thumbnail.take();

// If necessary, store caching data for the thumbnail ahead of time.
let thumbnail_cache_info = if store_in_cache {
// Use a small closure returning Option to avoid an unnecessary complicated
// chain of map/and_then.
let get_info = || {
let thumbnail = thumbnail.as_ref()?;
let info = thumbnail.info.as_ref()?;
Some((thumbnail.data.clone(), info.height?, info.width?))
};
get_info()
} else {
None
};

#[cfg(feature = "e2e-encryption")]
let (media_source, thumbnail_source, thumbnail_info) = if self.is_encrypted().await? {
self.client
Expand All @@ -1941,6 +1957,8 @@ impl Room {
.media()
.upload_plain_media_and_thumbnail(
content_type,
// TODO: get rid of this clone; wait for Ruma to use `Bytes` or something
// similar.
data.clone(),
thumbnail,
send_progress,
Expand All @@ -1958,11 +1976,38 @@ impl Room {
if store_in_cache {
let cache_store = self.client.event_cache_store();

// A failure to cache shouldn't prevent the whole upload from finishing
// properly, so only log errors during caching.

debug!("caching the media");
let request = MediaRequest { source: media_source.clone(), format: MediaFormat::File };
// This shouldn't prevent the whole process from finishing properly.
if let Err(err) = cache_store.add_media_content(&request, data).await {
warn!("unable to cache the media after uploading it: {err}");
}

if let Some(((data, height, width), source)) =
thumbnail_cache_info.zip(thumbnail_source.as_ref())
{
debug!("caching the thumbnail");

// Do a best guess at figuring the media request: not animated, cropped
// thumbnail of the original size.
let request = MediaRequest {
source: source.clone(),
format: MediaFormat::Thumbnail(MediaThumbnailSettings {
size: MediaThumbnailSize {
method: ruma::media::Method::Crop,
width,
height,
},
animated: false,
}),
};

if let Err(err) = cache_store.add_media_content(&request, data).await {
warn!("unable to cache the media after uploading it: {err}");
}
}
}

let msg_type = self.make_attachment_message(
Expand Down
109 changes: 83 additions & 26 deletions crates/matrix-sdk/tests/integration/room/attachment/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::time::Duration;
use std::{sync::Mutex, time::Duration};

use matrix_sdk::{
attachment::{
AttachmentConfig, AttachmentInfo, BaseImageInfo, BaseThumbnailInfo, BaseVideoInfo,
Thumbnail,
},
config::SyncSettings,
media::{MediaFormat, MediaRequest},
media::{MediaFormat, MediaRequest, MediaThumbnailSettings, MediaThumbnailSize},
test_utils::logged_in_client_with_server,
};
use matrix_sdk_test::{async_test, mocks::mock_encryption_state, test_json, DEFAULT_TEST_ROOM_ID};
Expand Down Expand Up @@ -65,29 +65,10 @@ async fn test_room_attachment_send() {
b"Hello world".to_vec(),
AttachmentConfig::new(),
)
.store_in_cache()
.await
.unwrap();

assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id);

// The media is immediately cached in the cache store, so we don't need to set
// up another mock endpoint for getting the media.
let reloaded = client
.media()
.get_media_content(
&MediaRequest {
source: MediaSource::Plain(owned_mxc_uri!(
"mxc://example.com/AQwafuaFswefuhsfAFAgsw"
)),
format: MediaFormat::File,
},
true,
)
.await
.unwrap();

assert_eq!(reloaded, b"Hello world");
}

#[async_test]
Expand Down Expand Up @@ -201,6 +182,9 @@ async fn test_room_attachment_send_wrong_info() {
async fn test_room_attachment_send_info_thumbnail() {
let (client, server) = logged_in_client_with_server().await;

let media_mxc = owned_mxc_uri!("mxc://example.com/media");
let thumbnail_mxc = owned_mxc_uri!("mxc://example.com/thumbnail");

Mock::given(method("PUT"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*"))
.and(header("authorization", "Bearer 1234"))
Expand All @@ -215,20 +199,37 @@ async fn test_room_attachment_send_info_thumbnail() {
"mimetype":"image/jpeg",
"size": 3600,
},
"thumbnail_url": "mxc://example.com/AQwafuaFswefuhsfAFAgsw",
"thumbnail_url": thumbnail_mxc,
}
})))
.respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::EVENT_ID))
.mount(&server)
.await;

let counter = Mutex::new(0);
Mock::given(method("POST"))
.and(path("/_matrix/media/r0/upload"))
.and(header("authorization", "Bearer 1234"))
.and(header("content-type", "image/jpeg"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"content_uri": "mxc://example.com/AQwafuaFswefuhsfAFAgsw"
})))
.respond_with({
// First request: return the thumbnail MXC;
// Second request: return the media MXC.
let media_mxc = media_mxc.clone();
let thumbnail_mxc = thumbnail_mxc.clone();
move |_: &wiremock::Request| {
let mut counter = counter.lock().unwrap();
if *counter == 0 {
*counter += 1;
ResponseTemplate::new(200).set_body_json(json!({
"content_uri": &thumbnail_mxc
}))
} else {
ResponseTemplate::new(200).set_body_json(json!({
"content_uri": &media_mxc
}))
}
}
})
.expect(2)
.mount(&server)
.await;
Expand All @@ -242,6 +243,24 @@ async fn test_room_attachment_send_info_thumbnail() {

let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();

// Preconditions: nothing is found in the cache.
let media_request =
MediaRequest { source: MediaSource::Plain(media_mxc), format: MediaFormat::File };
let thumbnail_request = MediaRequest {
source: MediaSource::Plain(thumbnail_mxc.clone()),
format: MediaFormat::Thumbnail(MediaThumbnailSettings {
size: MediaThumbnailSize {
method: ruma::media::Method::Crop,
width: uint!(480),
height: uint!(360),
},
animated: false,
}),
};
let _ = client.media().get_media_content(&media_request, true).await.unwrap_err();
let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err();

// Send the attachment with a thumbnail.
let config = AttachmentConfig::with_thumbnail(Thumbnail {
data: b"Thumbnail".to_vec(),
content_type: mime::IMAGE_JPEG,
Expand All @@ -260,10 +279,48 @@ async fn test_room_attachment_send_info_thumbnail() {

let response = room
.send_attachment("image", &mime::IMAGE_JPEG, b"Hello world".to_vec(), config)
.store_in_cache()
.await
.unwrap();

assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id)
// The event was sent.
assert_eq!(event_id!("$h29iv0s8:example.com"), response.event_id);

// The media is immediately cached in the cache store, so we don't need to set
// up another mock endpoint for getting the media.
let reloaded = client.media().get_media_content(&media_request, true).await.unwrap();
assert_eq!(reloaded, b"Hello world");

// The thumbnail is cached with sensible defaults.
let reloaded = client.media().get_media_content(&thumbnail_request, true).await.unwrap();
assert_eq!(reloaded, b"Thumbnail");

// The thumbnail can't be retrieved as a file.
let _ = client
.media()
.get_media_content(
&MediaRequest {
source: MediaSource::Plain(thumbnail_mxc.clone()),
format: MediaFormat::File,
},
true,
)
.await
.unwrap_err();

// But it is not found when requesting it as a thumbnail with a different size.
let thumbnail_request = MediaRequest {
source: MediaSource::Plain(thumbnail_mxc),
format: MediaFormat::Thumbnail(MediaThumbnailSettings {
size: MediaThumbnailSize {
method: ruma::media::Method::Crop,
width: uint!(42),
height: uint!(1337),
},
animated: false,
}),
};
let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err();
}

#[async_test]
Expand Down

0 comments on commit 9c03c5d

Please sign in to comment.