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

refactor!(sdk): Set thumbnail in AttachmentConfig with builder method instead of constructor #4452

Merged
merged 2 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 16 additions & 9 deletions bindings/matrix-sdk-ffi/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ impl Timeline {
fn build_thumbnail_info(
thumbnail_url: Option<String>,
thumbnail_info: Option<ThumbnailInfo>,
) -> Result<AttachmentConfig, RoomError> {
) -> Result<Option<Thumbnail>, RoomError> {
match (thumbnail_url, thumbnail_info) {
(None, None) => Ok(AttachmentConfig::new()),
(None, None) => Ok(None),

(Some(thumbnail_url), Some(thumbnail_info)) => {
let thumbnail_data =
Expand All @@ -163,15 +163,18 @@ fn build_thumbnail_info(
let mime_type =
mime_str.parse::<Mime>().map_err(|_| RoomError::InvalidAttachmentMimeType)?;

let thumbnail =
Thumbnail { data: thumbnail_data, content_type: mime_type, height, width, size };

Ok(AttachmentConfig::with_thumbnail(thumbnail))
Ok(Some(Thumbnail {
data: thumbnail_data,
content_type: mime_type,
height,
width,
size,
}))
}

_ => {
warn!("Ignoring thumbnail because either the thumbnail URL or info isn't defined");
Ok(AttachmentConfig::new())
Ok(None)
}
}
}
Expand Down Expand Up @@ -304,8 +307,10 @@ impl Timeline {
let base_image_info = BaseImageInfo::try_from(&image_info)
.map_err(|_| RoomError::InvalidAttachmentData)?;
let attachment_info = AttachmentInfo::Image(base_image_info);
let thumbnail = build_thumbnail_info(thumbnail_url, image_info.thumbnail_info)?;

let attachment_config = build_thumbnail_info(thumbnail_url, image_info.thumbnail_info)?
let attachment_config = AttachmentConfig::new()
.thumbnail(thumbnail)
.info(attachment_info)
.caption(caption)
.formatted_caption(formatted_caption);
Expand Down Expand Up @@ -338,8 +343,10 @@ impl Timeline {
let base_video_info: BaseVideoInfo = BaseVideoInfo::try_from(&video_info)
.map_err(|_| RoomError::InvalidAttachmentData)?;
let attachment_info = AttachmentInfo::Video(base_video_info);
let thumbnail = build_thumbnail_info(thumbnail_url, video_info.thumbnail_info)?;

let attachment_config = build_thumbnail_info(thumbnail_url, video_info.thumbnail_info)?
let attachment_config = AttachmentConfig::new()
.thumbnail(thumbnail)
.info(attachment_info)
.caption(caption)
.formatted_caption(formatted_caption.map(Into::into));
Expand Down
14 changes: 7 additions & 7 deletions crates/matrix-sdk/src/attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,21 +188,21 @@ pub struct AttachmentConfig {
}

impl AttachmentConfig {
/// Create a new default `AttachmentConfig` without providing a thumbnail.
///
/// To provide a thumbnail use [`AttachmentConfig::with_thumbnail()`].
/// Create a new empty `AttachmentConfig`.
pub fn new() -> Self {
Self::default()
}

/// Create a new default `AttachmentConfig` with a `thumbnail`.
/// Set the thumbnail to send.
///
/// # Arguments
///
/// * `thumbnail` - The thumbnail of the media. If the `content_type` does
/// not support it (eg audio clips), it is ignored.
pub fn with_thumbnail(thumbnail: Thumbnail) -> Self {
Self { thumbnail: Some(thumbnail), ..Default::default() }
/// not support it (e.g. audio clips), it is ignored.
#[must_use]
pub fn thumbnail(mut self, thumbnail: Option<Thumbnail>) -> Self {
self.thumbnail = thumbnail;
self
}

/// Set the transaction ID to send.
Expand Down
27 changes: 14 additions & 13 deletions crates/matrix-sdk/tests/integration/room/attachment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,20 @@ async fn test_room_attachment_send_info_thumbnail() {
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,
height: uint!(360),
width: uint!(480),
size: uint!(3600),
})
.info(AttachmentInfo::Image(BaseImageInfo {
height: Some(uint!(600)),
width: Some(uint!(800)),
size: None,
blurhash: None,
}));
let config = AttachmentConfig::new()
.thumbnail(Some(Thumbnail {
data: b"Thumbnail".to_vec(),
content_type: mime::IMAGE_JPEG,
height: uint!(360),
width: uint!(480),
size: uint!(3600),
}))
.info(AttachmentInfo::Image(BaseImageInfo {
height: Some(uint!(600)),
width: Some(uint!(800)),
size: None,
blurhash: None,
}));

let response = room
.send_attachment("image", &mime::IMAGE_JPEG, b"Hello world".to_vec(), config)
Expand Down
10 changes: 6 additions & 4 deletions crates/matrix-sdk/tests/integration/send_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,14 @@ async fn queue_attachment_with_thumbnail(q: &RoomSendQueue) -> (SendHandle, &'st
size: uint!(42),
};

let config =
AttachmentConfig::with_thumbnail(thumbnail).info(AttachmentInfo::Image(BaseImageInfo {
let config = AttachmentConfig::new().thumbnail(Some(thumbnail)).info(AttachmentInfo::Image(
BaseImageInfo {
height: Some(uint!(13)),
width: Some(uint!(37)),
size: Some(uint!(42)),
blurhash: None,
}));
},
));

let handle = q
.send_attachment(filename, content_type, data, config)
Expand Down Expand Up @@ -1801,7 +1802,8 @@ async fn test_media_uploads() {

let transaction_id = TransactionId::new();
let mentions = Mentions::with_user_ids([owned_user_id!("@ivan:sdk.rs")]);
let config = AttachmentConfig::with_thumbnail(thumbnail)
let config = AttachmentConfig::new()
.thumbnail(Some(thumbnail))
.txn_id(&transaction_id)
.caption(Some("caption".to_owned()))
.mentions(Some(mentions.clone()))
Expand Down
Loading