Skip to content

Commit 58755bb

Browse files
committed
refactor(ffi): remove duplicated fields in media event contents
The caption and filenames were weirdly duplicated in each media content, when the expected behavior is well defined: - if there's both a caption and a filename, body := caption, filename is its own field. - if there's only a filename, body := filename. We can remove all duplicated fields, knowing this, and reconstruct the body based on that information. This should make it clearer to FFI users which is what, and provide a clearer API when creating the caption and so on.
1 parent aabe394 commit 58755bb

File tree

2 files changed

+34
-61
lines changed
  • bindings/matrix-sdk-ffi/src
  • crates/matrix-sdk/src/room

2 files changed

+34
-61
lines changed

bindings/matrix-sdk-ffi/src/ruma.rs

Lines changed: 33 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,23 @@ pub enum MessageType {
262262
Other { msgtype: String, body: String },
263263
}
264264

265+
/// From MSC2530: https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md
266+
/// If the filename field is present in a media message, clients should treat
267+
/// body as a caption instead of a file name. Otherwise, the body is the
268+
/// file name.
269+
///
270+
/// So:
271+
/// - if a media has a filename and a caption, the body is the caption, filename
272+
/// is its own field.
273+
/// - if a media only has a filename, then body is the filename.
274+
fn get_body_and_filename(filename: String, caption: Option<String>) -> (String, Option<String>) {
275+
if let Some(caption) = caption {
276+
(caption, Some(filename))
277+
} else {
278+
(filename, None)
279+
}
280+
}
281+
265282
impl TryFrom<MessageType> for RumaMessageType {
266283
type Error = serde_json::Error;
267284

@@ -273,35 +290,39 @@ impl TryFrom<MessageType> for RumaMessageType {
273290
}))
274291
}
275292
MessageType::Image { content } => {
293+
let (body, filename) = get_body_and_filename(content.filename, content.caption);
276294
let mut event_content =
277-
RumaImageMessageEventContent::new(content.body, (*content.source).clone())
295+
RumaImageMessageEventContent::new(body, (*content.source).clone())
278296
.info(content.info.map(Into::into).map(Box::new));
279-
event_content.formatted = content.formatted.map(Into::into);
280-
event_content.filename = content.raw_filename;
297+
event_content.formatted = content.formatted_caption.map(Into::into);
298+
event_content.filename = filename;
281299
Self::Image(event_content)
282300
}
283301
MessageType::Audio { content } => {
302+
let (body, filename) = get_body_and_filename(content.filename, content.caption);
284303
let mut event_content =
285-
RumaAudioMessageEventContent::new(content.body, (*content.source).clone())
304+
RumaAudioMessageEventContent::new(body, (*content.source).clone())
286305
.info(content.info.map(Into::into).map(Box::new));
287-
event_content.formatted = content.formatted.map(Into::into);
288-
event_content.filename = content.raw_filename;
306+
event_content.formatted = content.formatted_caption.map(Into::into);
307+
event_content.filename = filename;
289308
Self::Audio(event_content)
290309
}
291310
MessageType::Video { content } => {
311+
let (body, filename) = get_body_and_filename(content.filename, content.caption);
292312
let mut event_content =
293-
RumaVideoMessageEventContent::new(content.body, (*content.source).clone())
313+
RumaVideoMessageEventContent::new(body, (*content.source).clone())
294314
.info(content.info.map(Into::into).map(Box::new));
295-
event_content.formatted = content.formatted.map(Into::into);
296-
event_content.filename = content.raw_filename;
315+
event_content.formatted = content.formatted_caption.map(Into::into);
316+
event_content.filename = filename;
297317
Self::Video(event_content)
298318
}
299319
MessageType::File { content } => {
320+
let (body, filename) = get_body_and_filename(content.filename, content.caption);
300321
let mut event_content =
301-
RumaFileMessageEventContent::new(content.body, (*content.source).clone())
322+
RumaFileMessageEventContent::new(body, (*content.source).clone())
302323
.info(content.info.map(Into::into).map(Box::new));
303-
event_content.formatted = content.formatted.map(Into::into);
304-
event_content.filename = content.raw_filename;
324+
event_content.formatted = content.formatted_caption.map(Into::into);
325+
event_content.filename = filename;
305326
Self::File(event_content)
306327
}
307328
MessageType::Notice { content } => {
@@ -335,9 +356,6 @@ impl From<RumaMessageType> for MessageType {
335356
},
336357
RumaMessageType::Image(c) => MessageType::Image {
337358
content: ImageMessageContent {
338-
body: c.body.clone(),
339-
formatted: c.formatted.as_ref().map(Into::into),
340-
raw_filename: c.filename.clone(),
341359
filename: c.filename().to_owned(),
342360
caption: c.caption().map(ToString::to_string),
343361
formatted_caption: c.formatted_caption().map(Into::into),
@@ -347,9 +365,6 @@ impl From<RumaMessageType> for MessageType {
347365
},
348366
RumaMessageType::Audio(c) => MessageType::Audio {
349367
content: AudioMessageContent {
350-
body: c.body.clone(),
351-
formatted: c.formatted.as_ref().map(Into::into),
352-
raw_filename: c.filename.clone(),
353368
filename: c.filename().to_owned(),
354369
caption: c.caption().map(ToString::to_string),
355370
formatted_caption: c.formatted_caption().map(Into::into),
@@ -361,9 +376,6 @@ impl From<RumaMessageType> for MessageType {
361376
},
362377
RumaMessageType::Video(c) => MessageType::Video {
363378
content: VideoMessageContent {
364-
body: c.body.clone(),
365-
formatted: c.formatted.as_ref().map(Into::into),
366-
raw_filename: c.filename.clone(),
367379
filename: c.filename().to_owned(),
368380
caption: c.caption().map(ToString::to_string),
369381
formatted_caption: c.formatted_caption().map(Into::into),
@@ -373,9 +385,6 @@ impl From<RumaMessageType> for MessageType {
373385
},
374386
RumaMessageType::File(c) => MessageType::File {
375387
content: FileMessageContent {
376-
body: c.body.clone(),
377-
formatted: c.formatted.as_ref().map(Into::into),
378-
raw_filename: c.filename.clone(),
379388
filename: c.filename().to_owned(),
380389
caption: c.caption().map(ToString::to_string),
381390
formatted_caption: c.formatted_caption().map(Into::into),
@@ -452,15 +461,6 @@ pub struct EmoteMessageContent {
452461

453462
#[derive(Clone, uniffi::Record)]
454463
pub struct ImageMessageContent {
455-
/// The original body field, deserialized from the event. Prefer the use of
456-
/// `filename` and `caption` over this.
457-
pub body: String,
458-
/// The original formatted body field, deserialized from the event. Prefer
459-
/// the use of `filename` and `formatted_caption` over this.
460-
pub formatted: Option<FormattedBody>,
461-
/// The original filename field, deserialized from the event. Prefer the use
462-
/// of `filename` over this.
463-
pub raw_filename: Option<String>,
464464
/// The computed filename, for use in a client.
465465
pub filename: String,
466466
pub caption: Option<String>,
@@ -471,15 +471,6 @@ pub struct ImageMessageContent {
471471

472472
#[derive(Clone, uniffi::Record)]
473473
pub struct AudioMessageContent {
474-
/// The original body field, deserialized from the event. Prefer the use of
475-
/// `filename` and `caption` over this.
476-
pub body: String,
477-
/// The original formatted body field, deserialized from the event. Prefer
478-
/// the use of `filename` and `formatted_caption` over this.
479-
pub formatted: Option<FormattedBody>,
480-
/// The original filename field, deserialized from the event. Prefer the use
481-
/// of `filename` over this.
482-
pub raw_filename: Option<String>,
483474
/// The computed filename, for use in a client.
484475
pub filename: String,
485476
pub caption: Option<String>,
@@ -492,15 +483,6 @@ pub struct AudioMessageContent {
492483

493484
#[derive(Clone, uniffi::Record)]
494485
pub struct VideoMessageContent {
495-
/// The original body field, deserialized from the event. Prefer the use of
496-
/// `filename` and `caption` over this.
497-
pub body: String,
498-
/// The original formatted body field, deserialized from the event. Prefer
499-
/// the use of `filename` and `formatted_caption` over this.
500-
pub formatted: Option<FormattedBody>,
501-
/// The original filename field, deserialized from the event. Prefer the use
502-
/// of `filename` over this.
503-
pub raw_filename: Option<String>,
504486
/// The computed filename, for use in a client.
505487
pub filename: String,
506488
pub caption: Option<String>,
@@ -511,15 +493,6 @@ pub struct VideoMessageContent {
511493

512494
#[derive(Clone, uniffi::Record)]
513495
pub struct FileMessageContent {
514-
/// The original body field, deserialized from the event. Prefer the use of
515-
/// `filename` and `caption` over this.
516-
pub body: String,
517-
/// The original formatted body field, deserialized from the event. Prefer
518-
/// the use of `filename` and `formatted_caption` over this.
519-
pub formatted: Option<FormattedBody>,
520-
/// The original filename field, deserialized from the event. Prefer the use
521-
/// of `filename` over this.
522-
pub raw_filename: Option<String>,
523496
/// The computed filename, for use in a client.
524497
pub filename: String,
525498
pub caption: Option<String>,

crates/matrix-sdk/src/room/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2052,7 +2052,7 @@ impl Room {
20522052
) -> MessageType {
20532053
// If caption is set, use it as body, and filename as the file name; otherwise,
20542054
// body is the filename, and the filename is not set.
2055-
// https://github.com/tulir/matrix-spec-proposals/blob/body-as-caption/proposals/2530-body-as-caption.md
2055+
// https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md
20562056
let (body, filename) = match caption {
20572057
Some(caption) => (caption, Some(filename.to_owned())),
20582058
None => (filename.to_owned(), None),

0 commit comments

Comments
 (0)