feat(media): introduce a stripped down MediaThumbnailSettings::new …

…only taking a width and height This one is used when caching a thumbnail everywhere, and when attempting to retrieve it; it gives us a single place where to coordinate the default `MediaThumbnailSettings` parameters.
matrix-org · Nov 7, 2024 · 237419c · 237419c
1 parent 1658397
commit 237419c
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 50 deletions.
diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs
@@ -26,7 +26,6 @@ use matrix_sdk::{
     reqwest::StatusCode,
     ruma::{
         api::client::{
-            media::get_content_thumbnail::v3::Method,
             push::{EmailPusherData, PusherIds, PusherInit, PusherKind as RumaPusherKind},
             room::{create_room, Visibility},
             session::get_login_types,
@@ -741,8 +740,7 @@ impl Client {
             .get_media_content(
                 &MediaRequestParameters {
                     source,
-                    format: MediaFormat::Thumbnail(MediaThumbnailSettings::with_method(
-                        Method::Scale,
+                    format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
                         UInt::new(width).unwrap(),
                         UInt::new(height).unwrap(),
                     )),

diff --git a/crates/matrix-sdk-base/src/media.rs b/crates/matrix-sdk-base/src/media.rs
@@ -70,9 +70,19 @@ pub struct MediaThumbnailSettings {
 impl MediaThumbnailSettings {
     /// Constructs a new `MediaThumbnailSettings` with the given method, width
     /// and height.
+    ///
+    /// Requests a non-animated thumbnail by default.
     pub fn with_method(method: Method, width: UInt, height: UInt) -> Self {
         Self { method, width, height, animated: false }
     }
+
+    /// Constructs a new `MediaThumbnailSettings` with the given width and
+    /// height.
+    ///
+    /// Requests scaling, and a non-animated thumbnail.
+    pub fn new(width: UInt, height: UInt) -> Self {
+        Self { method: Method::Scale, width, height, animated: false }
+    }
 }
 
 impl UniqueKey for MediaThumbnailSettings {

diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs
@@ -2006,16 +2006,9 @@ impl Room {
             {
                 debug!("caching the thumbnail");
 
-                // Do a best guess at figuring the media request: not animated, cropped
-                // thumbnail of the original size.
                 let request = MediaRequestParameters {
                     source: source.clone(),
-                    format: MediaFormat::Thumbnail(MediaThumbnailSettings {
-                        method: ruma::media::Method::Scale,
-                        width,
-                        height,
-                        animated: false,
-                    }),
+                    format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(width, height)),
                 };
 
                 if let Err(err) = cache_store_lock_guard.add_media_content(&request, data).await {

diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs
@@ -29,7 +29,6 @@ use ruma::{
         message::{MessageType, RoomMessageEventContent},
         MediaSource, ThumbnailInfo,
     },
-    media::Method,
     uint, OwnedMxcUri, OwnedTransactionId, TransactionId, UInt,
 };
 use tracing::{debug, error, instrument, trace, warn, Span};
@@ -73,15 +72,12 @@ fn make_local_thumbnail_media_request(
     width: UInt,
 ) -> MediaRequestParameters {
     // See comment in [`make_local_file_media_request`].
-    let source =
-        MediaSource::Plain(OwnedMxcUri::from(format!("mxc://send-queue.localhost/{}", txn_id)));
-    let format = MediaFormat::Thumbnail(MediaThumbnailSettings {
-        method: Method::Scale,
-        width,
-        height,
-        animated: false,
-    });
-    MediaRequestParameters { source, format }
+    MediaRequestParameters {
+        source: MediaSource::Plain(OwnedMxcUri::from(format!(
+            "mxc://send-queue.localhost/{txn_id}"
+        ))),
+        format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(width, height)),
+    }
 }
 
 /// Replace the source by the final ones in all the media types handled by

diff --git a/crates/matrix-sdk/tests/integration/room/attachment/mod.rs b/crates/matrix-sdk/tests/integration/room/attachment/mod.rs
@@ -248,12 +248,7 @@ async fn test_room_attachment_send_info_thumbnail() {
         MediaRequestParameters { source: MediaSource::Plain(media_mxc), format: MediaFormat::File };
     let thumbnail_request = MediaRequestParameters {
         source: MediaSource::Plain(thumbnail_mxc.clone()),
-        format: MediaFormat::Thumbnail(MediaThumbnailSettings {
-            method: ruma::media::Method::Scale,
-            width: uint!(480),
-            height: uint!(360),
-            animated: false,
-        }),
+        format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(uint!(480), uint!(360))),
     };
     let _ = client.media().get_media_content(&media_request, true).await.unwrap_err();
     let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err();
@@ -309,12 +304,7 @@ async fn test_room_attachment_send_info_thumbnail() {
     // But it is not found when requesting it as a thumbnail with a different size.
     let thumbnail_request = MediaRequestParameters {
         source: MediaSource::Plain(thumbnail_mxc),
-        format: MediaFormat::Thumbnail(MediaThumbnailSettings {
-            method: ruma::media::Method::Scale,
-            width: uint!(42),
-            height: uint!(1337),
-            animated: false,
-        }),
+        format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(uint!(42), uint!(1337))),
     };
     let _ = client.media().get_media_content(&thumbnail_request, true).await.unwrap_err();
 }

diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs
@@ -40,7 +40,6 @@ use ruma::{
         },
         AnyMessageLikeEventContent, EventContent as _, Mentions,
     },
-    media::Method,
     mxc_uri, owned_user_id, room_id,
     serde::Raw,
     uint, EventId, MxcUri, OwnedEventId, TransactionId,
@@ -2150,14 +2149,10 @@ async fn test_media_uploads() {
         .get_media_content(
             &MediaRequestParameters {
                 source: local_thumbnail_source,
-                // TODO: extract this reasonable query into a helper function shared across the
-                // codebase
-                format: MediaFormat::Thumbnail(MediaThumbnailSettings {
-                    height: tinfo.height.unwrap(),
-                    width: tinfo.width.unwrap(),
-                    method: Method::Scale,
-                    animated: false,
-                }),
+                format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
+                    tinfo.width.unwrap(),
+                    tinfo.height.unwrap(),
+                )),
             },
             true,
         )
@@ -2222,14 +2217,10 @@ async fn test_media_uploads() {
         .get_media_content(
             &MediaRequestParameters {
                 source: new_thumbnail_source,
-                // TODO: extract this reasonable query into a helper function shared across the
-                // codebase
-                format: MediaFormat::Thumbnail(MediaThumbnailSettings {
-                    height: tinfo.height.unwrap(),
-                    width: tinfo.width.unwrap(),
-                    method: Method::Scale,
-                    animated: false,
-                }),
+                format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
+                    tinfo.width.unwrap(),
+                    tinfo.height.unwrap(),
+                )),
             },
             true,
         )