Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(media): replace MediaThumbnailSettings default ctor with reasonable defaults #4225

Merged
merged 2 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -742,7 +741,6 @@ impl Client {
&MediaRequestParameters {
source,
format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
Method::Scale,
UInt::new(width).unwrap(),
UInt::new(height).unwrap(),
)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
};
let request_thumbnail = MediaRequestParameters {
source: MediaSource::Plain(uri.to_owned()),
format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
format: MediaFormat::Thumbnail(MediaThumbnailSettings::with_method(
Method::Crop,
uint!(100),
uint!(100),
Expand Down
12 changes: 11 additions & 1 deletion crates/matrix-sdk-base/src/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,19 @@ pub struct MediaThumbnailSettings {
impl MediaThumbnailSettings {
/// Constructs a new `MediaThumbnailSettings` with the given method, width
/// and height.
pub fn new(method: Method, width: UInt, height: UInt) -> Self {
///
/// 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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-sqlite/src/event_cache_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ mod tests {
};
let thumbnail_request = MediaRequestParameters {
source: MediaSource::Plain(uri.to_owned()),
format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
format: MediaFormat::Thumbnail(MediaThumbnailSettings::with_method(
Method::Crop,
uint!(100),
uint!(100),
Expand Down
9 changes: 1 addition & 8 deletions crates/matrix-sdk/src/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 6 additions & 10 deletions crates/matrix-sdk/src/send_queue/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk/tests/integration/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ async fn test_get_media_file_no_auth() {
.media()
.get_thumbnail(
&event_content,
MediaThumbnailSettings::new(Method::Scale, uint!(100), uint!(100)),
MediaThumbnailSettings::with_method(Method::Scale, uint!(100), uint!(100)),
true,
)
.await
Expand Down Expand Up @@ -272,7 +272,7 @@ async fn test_get_media_file_with_auth_matrix_1_11() {
.media()
.get_thumbnail(
&event_content,
MediaThumbnailSettings::new(Method::Scale, uint!(100), uint!(100)),
MediaThumbnailSettings::with_method(Method::Scale, uint!(100), uint!(100)),
true,
)
.await
Expand Down Expand Up @@ -387,7 +387,7 @@ async fn test_get_media_file_with_auth_matrix_stable_feature() {
.media()
.get_thumbnail(
&event_content,
MediaThumbnailSettings::new(Method::Scale, uint!(100), uint!(100)),
MediaThumbnailSettings::with_method(Method::Scale, uint!(100), uint!(100)),
true,
)
.await
Expand Down
14 changes: 2 additions & 12 deletions crates/matrix-sdk/tests/integration/room/attachment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down
25 changes: 8 additions & 17 deletions crates/matrix-sdk/tests/integration/send_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
Loading