From 02c7c2cdfc10a18d32c498f15d8ff40268108205 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 13 Nov 2024 14:53:44 +0100 Subject: [PATCH] test(send queue): caching a thumbnail of unknown dimensions removes it from cache after upload --- crates/matrix-sdk/src/attachment.rs | 2 +- crates/matrix-sdk/src/send_queue/upload.rs | 33 ++-- .../tests/integration/send_queue.rs | 154 ++++++++++++++++++ 3 files changed, 176 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index dbefd0d32a1..0fde6997466 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -148,7 +148,7 @@ impl From for FileInfo { } } -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] /// Base metadata about a thumbnail. pub struct BaseThumbnailInfo { /// The height of the thumbnail in pixels. diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index 9373f9798b2..9e885d3cc57 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -323,18 +323,27 @@ impl QueueStorage { let from_req = make_local_thumbnail_media_request(&info.txn, info.height, info.width); - trace!( from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); - - // Reuse the same format for the cached thumbnail with the final MXC ID. - let new_format = from_req.format.clone(); - - cache_store - .replace_media_key( - &from_req, - &MediaRequestParameters { source: new_source, format: new_format }, - ) - .await - .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + if info.height == uint!(0) || info.width == uint!(0) { + trace!(from = ?from_req.source, "removing thumbnail with unknown dimension from cache store"); + + cache_store + .remove_media_content(&from_req) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + } else { + trace!(from = ?from_req.source, to = ?new_source, "renaming thumbnail file key in cache store"); + + // Reuse the same format for the cached thumbnail with the final MXC ID. + let new_format = from_req.format.clone(); + + cache_store + .replace_media_key( + &from_req, + &MediaRequestParameters { source: new_source, format: new_format }, + ) + .await + .map_err(RoomSendQueueStorageError::EventCacheStoreError)?; + } } } diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 4198a123d80..49fc6cd670f 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -1980,6 +1980,160 @@ async fn test_media_uploads() { assert!(watch.is_empty()); } +#[async_test] +async fn test_media_uploads_no_caching_of_thumbnails_of_unknown_sizes() { + let mock = MatrixMockServer::new().await; + + // Mark the room as joined. + let room_id = room_id!("!a:b.c"); + let client = mock.client_builder().build().await; + let room = mock.sync_joined_room(&client, room_id).await; + + let q = room.send_queue(); + + let (local_echoes, mut watch) = q.subscribe().await.unwrap(); + assert!(local_echoes.is_empty()); + + // ---------------------- + // Create the media to send, with a thumbnail that has unknown dimensions. + let filename = "surprise.jpeg.exe"; + let content_type = mime::IMAGE_JPEG; + let data = b"hello world".to_vec(); + + let thumbnail = Thumbnail { + data: b"thumbnail".to_vec(), + content_type: content_type.clone(), + info: Some(BaseThumbnailInfo::default()), + }; + + let attachment_info = AttachmentInfo::Image(BaseImageInfo { + height: Some(uint!(14)), + width: Some(uint!(38)), + size: Some(uint!(43)), + blurhash: None, + }); + + let config = AttachmentConfig::with_thumbnail(thumbnail).info(attachment_info); + + // ---------------------- + // Prepare endpoints. + mock.mock_room_state_encryption().plain().mount().await; + mock.mock_room_send().ok(event_id!("$1")).mock_once().mount().await; + + mock.mock_upload().ok(mxc_uri!("mxc://sdk.rs/thumbnail")).mock_once().mount().await; + mock.mock_upload().ok(mxc_uri!("mxc://sdk.rs/media")).mock_once().mount().await; + + // ---------------------- + // Send the media. + assert!(watch.is_empty()); + q.send_attachment(filename, content_type, data, config) + .await + .expect("queuing the attachment works"); + + // ---------------------- + // Observe the local echo + let (txn, _send_handle, content) = assert_update!(watch => local echo event); + + // Sanity-check metadata. + assert_let!(MessageType::Image(img_content) = content.msgtype); + assert_eq!(img_content.filename(), filename); + + let info = img_content.info.unwrap(); + assert_eq!(info.height, Some(uint!(14))); + assert_eq!(info.width, Some(uint!(38))); + assert_eq!(info.size, Some(uint!(43))); + assert_eq!(info.mimetype.as_deref(), Some("image/jpeg")); + + // Check the data source: it should reference the send queue local storage. + let local_source = img_content.source; + assert_let!(MediaSource::Plain(mxc) = &local_source); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + // The media is immediately available from the cache. + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: local_source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(file_media, b"hello world"); + + // ---------------------- + // Thumbnail. + + // Check metadata. + let tinfo = info.thumbnail_info.unwrap(); + assert_eq!(tinfo.height, None); + assert_eq!(tinfo.width, None); + assert_eq!(tinfo.size, None); + assert_eq!(tinfo.mimetype.as_deref(), Some("image/jpeg")); + + // Check the thumbnail source: it should reference the send queue local storage. + let local_thumbnail_source = info.thumbnail_source.unwrap(); + assert_let!(MediaSource::Plain(mxc) = &local_thumbnail_source); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); + + let thumbnail_media = client + .media() + .get_media_content( + &MediaRequestParameters { + source: local_thumbnail_source, + format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(uint!(0), uint!(0))), + }, + true, + ) + .await + .expect("media should be found"); + assert_eq!(thumbnail_media, b"thumbnail"); + + // ---------------------- + // Let the upload progress. + + assert_update!(watch => uploaded { related_to = txn, mxc = mxc_uri!("mxc://sdk.rs/thumbnail") }); + assert_update!(watch => uploaded { related_to = txn, mxc = mxc_uri!("mxc://sdk.rs/media") }); + + let edit_msg = assert_update!(watch => edit local echo { txn = txn }); + assert_let!(MessageType::Image(new_content) = edit_msg.msgtype); + assert_let!(MediaSource::Plain(new_uri) = &new_content.source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/media")); + + let file_media = client + .media() + .get_media_content( + &MediaRequestParameters { source: new_content.source, format: MediaFormat::File }, + true, + ) + .await + .expect("media should be found with its final MXC uri in the cache"); + assert_eq!(file_media, b"hello world"); + + let new_thumbnail_source = new_content.info.unwrap().thumbnail_source.unwrap(); + assert_let!(MediaSource::Plain(new_uri) = &new_thumbnail_source); + assert_eq!(new_uri, mxc_uri!("mxc://sdk.rs/thumbnail")); + + // Retrieving the thumbnail should NOT work, since it doesn't make sense to + // cache it with a size of 0. + client + .media() + .get_media_content( + &MediaRequestParameters { + source: new_thumbnail_source, + format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(uint!(0), uint!(0))), + }, + true, + ) + .await + .unwrap_err(); + + // The event is sent, at some point. + assert_update!(watch => sent { event_id = event_id!("$1") }); + + // That's all, folks! + assert!(watch.is_empty()); +} + #[async_test] async fn test_media_upload_retry() { let mock = MatrixMockServer::new().await;