Skip to content

Commit

Permalink
refactor(ffi): remove duplicated fields in media event contents
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bnjbvr committed Nov 12, 2024
1 parent d446eb9 commit f341dc4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 61 deletions.
93 changes: 33 additions & 60 deletions bindings/matrix-sdk-ffi/src/ruma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,23 @@ pub enum MessageType {
Other { msgtype: String, body: String },
}

/// From MSC2530: https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md
/// If the filename field is present in a media message, clients should treat
/// body as a caption instead of a file name. Otherwise, the body is the
/// file name.
///
/// So:
/// - if a media has a filename and a caption, the body is the caption, filename
/// is its own field.
/// - if a media only has a filename, then body is the filename.
fn get_body_and_filename(filename: String, caption: Option<String>) -> (String, Option<String>) {
if let Some(caption) = caption {
(caption, Some(filename))
} else {
(filename, None)
}
}

impl TryFrom<MessageType> for RumaMessageType {
type Error = serde_json::Error;

Expand All @@ -273,35 +290,39 @@ impl TryFrom<MessageType> for RumaMessageType {
}))
}
MessageType::Image { content } => {
let (body, filename) = get_body_and_filename(content.filename, content.caption);
let mut event_content =
RumaImageMessageEventContent::new(content.body, (*content.source).clone())
RumaImageMessageEventContent::new(body, (*content.source).clone())
.info(content.info.map(Into::into).map(Box::new));
event_content.formatted = content.formatted.map(Into::into);
event_content.filename = content.raw_filename;
event_content.formatted = content.formatted_caption.map(Into::into);
event_content.filename = filename;
Self::Image(event_content)
}
MessageType::Audio { content } => {
let (body, filename) = get_body_and_filename(content.filename, content.caption);
let mut event_content =
RumaAudioMessageEventContent::new(content.body, (*content.source).clone())
RumaAudioMessageEventContent::new(body, (*content.source).clone())
.info(content.info.map(Into::into).map(Box::new));
event_content.formatted = content.formatted.map(Into::into);
event_content.filename = content.raw_filename;
event_content.formatted = content.formatted_caption.map(Into::into);
event_content.filename = filename;
Self::Audio(event_content)
}
MessageType::Video { content } => {
let (body, filename) = get_body_and_filename(content.filename, content.caption);
let mut event_content =
RumaVideoMessageEventContent::new(content.body, (*content.source).clone())
RumaVideoMessageEventContent::new(body, (*content.source).clone())
.info(content.info.map(Into::into).map(Box::new));
event_content.formatted = content.formatted.map(Into::into);
event_content.filename = content.raw_filename;
event_content.formatted = content.formatted_caption.map(Into::into);
event_content.filename = filename;
Self::Video(event_content)
}
MessageType::File { content } => {
let (body, filename) = get_body_and_filename(content.filename, content.caption);
let mut event_content =
RumaFileMessageEventContent::new(content.body, (*content.source).clone())
RumaFileMessageEventContent::new(body, (*content.source).clone())
.info(content.info.map(Into::into).map(Box::new));
event_content.formatted = content.formatted.map(Into::into);
event_content.filename = content.raw_filename;
event_content.formatted = content.formatted_caption.map(Into::into);
event_content.filename = filename;
Self::File(event_content)
}
MessageType::Notice { content } => {
Expand Down Expand Up @@ -335,9 +356,6 @@ impl From<RumaMessageType> for MessageType {
},
RumaMessageType::Image(c) => MessageType::Image {
content: ImageMessageContent {
body: c.body.clone(),
formatted: c.formatted.as_ref().map(Into::into),
raw_filename: c.filename.clone(),
filename: c.filename().to_owned(),
caption: c.caption().map(ToString::to_string),
formatted_caption: c.formatted_caption().map(Into::into),
Expand All @@ -347,9 +365,6 @@ impl From<RumaMessageType> for MessageType {
},
RumaMessageType::Audio(c) => MessageType::Audio {
content: AudioMessageContent {
body: c.body.clone(),
formatted: c.formatted.as_ref().map(Into::into),
raw_filename: c.filename.clone(),
filename: c.filename().to_owned(),
caption: c.caption().map(ToString::to_string),
formatted_caption: c.formatted_caption().map(Into::into),
Expand All @@ -361,9 +376,6 @@ impl From<RumaMessageType> for MessageType {
},
RumaMessageType::Video(c) => MessageType::Video {
content: VideoMessageContent {
body: c.body.clone(),
formatted: c.formatted.as_ref().map(Into::into),
raw_filename: c.filename.clone(),
filename: c.filename().to_owned(),
caption: c.caption().map(ToString::to_string),
formatted_caption: c.formatted_caption().map(Into::into),
Expand All @@ -373,9 +385,6 @@ impl From<RumaMessageType> for MessageType {
},
RumaMessageType::File(c) => MessageType::File {
content: FileMessageContent {
body: c.body.clone(),
formatted: c.formatted.as_ref().map(Into::into),
raw_filename: c.filename.clone(),
filename: c.filename().to_owned(),
caption: c.caption().map(ToString::to_string),
formatted_caption: c.formatted_caption().map(Into::into),
Expand Down Expand Up @@ -452,15 +461,6 @@ pub struct EmoteMessageContent {

#[derive(Clone, uniffi::Record)]
pub struct ImageMessageContent {
/// The original body field, deserialized from the event. Prefer the use of
/// `filename` and `caption` over this.
pub body: String,
/// The original formatted body field, deserialized from the event. Prefer
/// the use of `filename` and `formatted_caption` over this.
pub formatted: Option<FormattedBody>,
/// The original filename field, deserialized from the event. Prefer the use
/// of `filename` over this.
pub raw_filename: Option<String>,
/// The computed filename, for use in a client.
pub filename: String,
pub caption: Option<String>,
Expand All @@ -471,15 +471,6 @@ pub struct ImageMessageContent {

#[derive(Clone, uniffi::Record)]
pub struct AudioMessageContent {
/// The original body field, deserialized from the event. Prefer the use of
/// `filename` and `caption` over this.
pub body: String,
/// The original formatted body field, deserialized from the event. Prefer
/// the use of `filename` and `formatted_caption` over this.
pub formatted: Option<FormattedBody>,
/// The original filename field, deserialized from the event. Prefer the use
/// of `filename` over this.
pub raw_filename: Option<String>,
/// The computed filename, for use in a client.
pub filename: String,
pub caption: Option<String>,
Expand All @@ -492,15 +483,6 @@ pub struct AudioMessageContent {

#[derive(Clone, uniffi::Record)]
pub struct VideoMessageContent {
/// The original body field, deserialized from the event. Prefer the use of
/// `filename` and `caption` over this.
pub body: String,
/// The original formatted body field, deserialized from the event. Prefer
/// the use of `filename` and `formatted_caption` over this.
pub formatted: Option<FormattedBody>,
/// The original filename field, deserialized from the event. Prefer the use
/// of `filename` over this.
pub raw_filename: Option<String>,
/// The computed filename, for use in a client.
pub filename: String,
pub caption: Option<String>,
Expand All @@ -511,15 +493,6 @@ pub struct VideoMessageContent {

#[derive(Clone, uniffi::Record)]
pub struct FileMessageContent {
/// The original body field, deserialized from the event. Prefer the use of
/// `filename` and `caption` over this.
pub body: String,
/// The original formatted body field, deserialized from the event. Prefer
/// the use of `filename` and `formatted_caption` over this.
pub formatted: Option<FormattedBody>,
/// The original filename field, deserialized from the event. Prefer the use
/// of `filename` over this.
pub raw_filename: Option<String>,
/// The computed filename, for use in a client.
pub filename: String,
pub caption: Option<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2052,7 +2052,7 @@ impl Room {
) -> MessageType {
// If caption is set, use it as body, and filename as the file name; otherwise,
// body is the filename, and the filename is not set.
// https://github.com/tulir/matrix-spec-proposals/blob/body-as-caption/proposals/2530-body-as-caption.md
// https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2530-body-as-caption.md
let (body, filename) = match caption {
Some(caption) => (caption, Some(filename.to_owned())),
None => (filename.to_owned(), None),
Expand Down

0 comments on commit f341dc4

Please sign in to comment.