message-toolbar: Use send queue to send attachments

Allows to benefit from its built-in features: local echos, sending
messages in the right order, stopping the queue when an error occurs…

src/session/view/account_settings/general_page/mod.rs

@@ -21,7 +21,7 @@ use crate::{
     prelude::*,
     session::model::Session,
     spawn, spawn_tokio, toast,
-    utils::{media::load_file, template_callbacks::TemplateCallbacks, OngoingAsyncAction},
+    utils::{media::FileInfo, template_callbacks::TemplateCallbacks, OngoingAsyncAction},
 };
 
 mod imp {
@@ -224,8 +224,18 @@ impl GeneralPage {
         let avatar = &imp.avatar;
         avatar.edit_in_progress();
 
-        let (data, info) = match load_file(&file).await {
-            Ok(res) => res,
+        let info = match FileInfo::try_from_file(&file).await {
+            Ok(info) => info,
+            Err(error) => {
+                error!("Could not load user avatar file info: {error}");
+                toast!(self, gettext("Could not load file"));
+                avatar.reset();
+                return;
+            }
+        };
+
+        let data = match file.load_contents_future().await {
+            Ok((data, _)) => data,
             Err(error) => {
                 error!("Could not load user avatar file: {error}");
                 toast!(self, gettext("Could not load file"));
@@ -236,8 +246,12 @@ impl GeneralPage {
 
         let client = session.client();
         let client_clone = client.clone();
-        let handle =
-            spawn_tokio!(async move { client_clone.media().upload(&info.mime, data, None).await });
+        let handle = spawn_tokio!(async move {
+            client_clone
+                .media()
+                .upload(&info.mime, data.into(), None)
+                .await
+        });
 
         let uri = match handle.await.unwrap() {
             Ok(res) => res.content_uri,

src/session/view/content/room_details/edit_details_subpage.rs

@@ -17,7 +17,7 @@ use crate::{
     session::model::Room,
     spawn_tokio, toast,
     utils::{
-        media::{image::ImageInfoLoader, load_file},
+        media::{image::ImageInfoLoader, FileInfo},
         template_callbacks::TemplateCallbacks,
         BoundObjectWeakRef, OngoingAsyncAction,
     },
@@ -173,8 +173,18 @@ mod imp {
             let avatar = &self.avatar;
             avatar.edit_in_progress();
 
-            let (data, info) = match load_file(&file).await {
-                Ok(res) => res,
+            let info = match FileInfo::try_from_file(&file).await {
+                Ok(info) => info,
+                Err(error) => {
+                    error!("Could not load room avatar file info: {error}");
+                    toast!(obj, gettext("Could not load file"));
+                    avatar.reset();
+                    return;
+                }
+            };
+
+            let data = match file.load_contents_future().await {
+                Ok((data, _)) => data,
                 Err(error) => {
                     error!("Could not load room avatar file: {error}");
                     toast!(obj, gettext("Could not load file"));
@@ -196,7 +206,9 @@ mod imp {
             };
             let client = session.client();
             let handle =
-                spawn_tokio!(async move { client.media().upload(&info.mime, data, None).await });
+                spawn_tokio!(
+                    async move { client.media().upload(&info.mime, data.into(), None).await }
+                );
 
             let uri = match handle.await.unwrap() {
                 Ok(res) => res.content_uri,

src/session/view/content/room_history/message_row/audio.rs

@@ -6,7 +6,7 @@ use gtk::{
 };
 use tracing::warn;
 
-use super::ContentFormat;
+use super::{content::MessageCacheKey, ContentFormat};
 use crate::{
     components::AudioPlayer,
     gettext_f,
@@ -37,6 +37,12 @@ mod imp {
         /// The filename of the audio file.
         #[property(get)]
         filename: RefCell<Option<String>>,
+        /// The cache key for the current audio message.
+        ///
+        /// The audio is only reloaded if the cache key changes. This is to
+        /// avoid reloading the audio when the local echo is updated to a remote
+        /// echo.
+        cache_key: RefCell<MessageCacheKey>,
         /// The media file.
         file: RefCell<Option<File>>,
         /// The state of the audio file.
@@ -137,13 +143,29 @@ mod imp {
             self.state_error.set_tooltip_text(Some(error));
         }
 
-        /// Display the given audio message.
+        /// Set the cache key with the given value.
+        ///
+        /// Returns `true` if the audio should be reloaded.
+        fn set_cache_key(&self, key: MessageCacheKey) -> bool {
+            let should_reload = self.cache_key.borrow().should_reload(&key);
+            self.cache_key.replace(key);
+
+            should_reload
+        }
+
+        /// Display the given `audio` message.
         pub(super) fn audio(
             &self,
             message: MediaMessage,
             session: &Session,
             format: ContentFormat,
+            cache_key: MessageCacheKey,
         ) {
+            if !self.set_cache_key(cache_key) {
+                // We do not need to reload the audio.
+                return;
+            }
+
             self.file.take();
             self.set_filename(Some(message.filename()));
 
@@ -212,8 +234,14 @@ impl MessageAudio {
     }
 
     /// Display the given `audio` message.
-    pub(crate) fn audio(&self, message: MediaMessage, session: &Session, format: ContentFormat) {
-        self.imp().audio(message, session, format);
+    pub(crate) fn audio(
+        &self,
+        message: MediaMessage,
+        session: &Session,
+        format: ContentFormat,
+        cache_key: MessageCacheKey,
+    ) {
+        self.imp().audio(message, session, format, cache_key);
     }
 }
 

src/session/view/content/room_history/message_row/content.rs

@@ -4,7 +4,7 @@ use gtk::{gdk, glib, glib::clone};
 use matrix_sdk_ui::timeline::{
     Message, RepliedToInfo, ReplyContent, TimelineDetails, TimelineItemContent,
 };
-use ruma::events::room::message::MessageType;
+use ruma::{events::room::message::MessageType, OwnedEventId, OwnedTransactionId};
 use tracing::{error, warn};
 
 use super::{
@@ -158,6 +158,8 @@ impl MessageContent {
                             ContentFormat::Compact,
                             &replied_to_sender,
                             replied_to_detect_at_room,
+                            None,
+                            event.reply_to_id(),
                         );
                         build_content(
                             reply.content(),
@@ -165,6 +167,8 @@ impl MessageContent {
                             ContentFormat::Natural,
                             &event.sender(),
                             detect_at_room,
+                            event.transaction_id(),
+                            event.event_id(),
                         );
                         self.set_child(Some(&reply));
 
@@ -181,6 +185,8 @@ impl MessageContent {
             format,
             &event.sender(),
             detect_at_room,
+            event.transaction_id(),
+            event.event_id(),
         );
     }
 
@@ -192,7 +198,15 @@ impl MessageContent {
 
         let detect_at_room = message.can_contain_at_room() && sender.can_notify_room();
 
-        build_message_content(self, message, self.format(), sender, detect_at_room);
+        build_message_content(
+            self,
+            message,
+            self.format(),
+            sender,
+            detect_at_room,
+            None,
+            Some(info.event_id().to_owned()),
+        );
     }
 
     /// Get the texture displayed by this widget, if any.
@@ -208,12 +222,22 @@ fn build_content(
     format: ContentFormat,
     sender: &Member,
     detect_at_room: bool,
+    transaction_id: Option<OwnedTransactionId>,
+    event_id: Option<OwnedEventId>,
 ) {
     let room = sender.room();
 
     match content {
         TimelineItemContent::Message(message) => {
-            build_message_content(parent, &message, format, sender, detect_at_room);
+            build_message_content(
+                parent,
+                &message,
+                format,
+                sender,
+                detect_at_room,
+                transaction_id,
+                event_id,
+            );
         }
         TimelineItemContent::Sticker(sticker) => {
             build_media_message_content(
@@ -222,6 +246,11 @@ fn build_content(
                 format,
                 &room,
                 detect_at_room,
+                MessageCacheKey {
+                    transaction_id,
+                    event_id,
+                    is_edited: false,
+                },
             );
         }
         TimelineItemContent::UnableToDecrypt(_) => {
@@ -265,11 +294,24 @@ fn build_message_content(
     format: ContentFormat,
     sender: &Member,
     detect_at_room: bool,
+    transaction_id: Option<OwnedTransactionId>,
+    event_id: Option<OwnedEventId>,
 ) {
     let room = sender.room();
 
     if let Some(media_message) = MediaMessage::from_message(message.msgtype()) {
-        build_media_message_content(parent, media_message, format, &room, detect_at_room);
+        build_media_message_content(
+            parent,
+            media_message,
+            format,
+            &room,
+            detect_at_room,
+            MessageCacheKey {
+                transaction_id,
+                event_id,
+                is_edited: message.is_edited(),
+            },
+        );
         return;
     }
 
@@ -364,6 +406,7 @@ fn build_media_message_content(
     format: ContentFormat,
     room: &Room,
     detect_at_room: bool,
+    cache_key: MessageCacheKey,
 ) {
     let Some(session) = room.session() else {
         return;
@@ -387,11 +430,17 @@ fn build_media_message_content(
             detect_at_room,
         );
 
-        let new_widget =
-            build_media_content(caption_widget.child(), media_message, format, &session);
+        let new_widget = build_media_content(
+            caption_widget.child(),
+            media_message,
+            format,
+            &session,
+            cache_key,
+        );
         caption_widget.set_child(Some(new_widget));
     } else {
-        let new_widget = build_media_content(parent.child(), media_message, format, &session);
+        let new_widget =
+            build_media_content(parent.child(), media_message, format, &session, cache_key);
         parent.set_child(Some(&new_widget));
     }
 }
@@ -404,6 +453,7 @@ fn build_media_content(
     media_message: MediaMessage,
     format: ContentFormat,
     session: &Session,
+    cache_key: MessageCacheKey,
 ) -> gtk::Widget {
     match media_message {
         MediaMessage::Audio(audio) => {
@@ -411,7 +461,7 @@ fn build_media_content(
                 .and_downcast::<MessageAudio>()
                 .unwrap_or_default();
 
-            widget.audio(audio.into(), session, format);
+            widget.audio(audio.into(), session, format, cache_key);
 
             widget.upcast()
         }
@@ -429,7 +479,7 @@ fn build_media_content(
                 .and_downcast::<MessageVisualMedia>()
                 .unwrap_or_default();
 
-            widget.set_media_message(image.into(), session, format);
+            widget.set_media_message(image.into(), session, format, cache_key);
 
             widget.upcast()
         }
@@ -438,7 +488,7 @@ fn build_media_content(
                 .and_downcast::<MessageVisualMedia>()
                 .unwrap_or_default();
 
-            widget.set_media_message(video.into(), session, format);
+            widget.set_media_message(video.into(), session, format, cache_key);
 
             widget.upcast()
         }
@@ -447,9 +497,52 @@ fn build_media_content(
                 .and_downcast::<MessageVisualMedia>()
                 .unwrap_or_default();
 
-            widget.set_media_message(sticker.into(), session, format);
+            widget.set_media_message(sticker.into(), session, format, cache_key);
 
             widget.upcast()
         }
     }
 }
+
+/// The data used as a cache key for messages.
+///
+/// This is used when there is no reliable way to detect if the content of a
+/// message changed. For example, the URI of a media file might change between a
+/// local echo and a remote echo, but we do not need to reload the media in this
+/// case, and we have no other way to know that both URIs point to the same
+/// file.
+#[derive(Debug, Clone, Default)]
+pub(crate) struct MessageCacheKey {
+    /// The transaction ID of the event.
+    ///
+    /// Local echo should keep its transaction ID after the message is sent, so
+    /// we do not need to reload the message if it did not change.
+    transaction_id: Option<OwnedTransactionId>,
+    /// The global ID of the event.
+    ///
+    /// Local echo that was sent and remote echo should have the same event ID,
+    /// so we do not need to reload the message if it did not change.
+    event_id: Option<OwnedEventId>,
+    /// Whether the message is edited.
+    ///
+    /// The message must be reloaded when it was edited.
+    is_edited: bool,
+}
+
+impl MessageCacheKey {
+    /// Whether the given new `MessageCacheKey` should trigger a reload of the
+    /// mmessage compared to this one.
+    pub(super) fn should_reload(&self, new: &MessageCacheKey) -> bool {
+        if new.is_edited {
+            return true;
+        }
+
+        let transaction_id_invalidated = self.transaction_id.is_none()
+            || new.transaction_id.is_none()
+            || self.transaction_id != new.transaction_id;
+        let event_id_invalidated =
+            self.event_id.is_none() || new.event_id.is_none() || self.event_id != new.event_id;
+
+        transaction_id_invalidated && event_id_invalidated
+    }
+}

src/session/view/content/room_history/message_row/visual_media.rs

@@ -9,7 +9,7 @@ use matrix_sdk::Client;
 use ruma::api::client::media::get_content_thumbnail::v3::Method;
 use tracing::{error, warn};
 
-use super::ContentFormat;
+use super::{content::MessageCacheKey, ContentFormat};
 use crate::{
     components::{AnimatedImagePaintable, VideoPlayer},
     gettext_f,
@@ -74,6 +74,11 @@ mod imp {
         #[property(get)]
         activatable: Cell<bool>,
         gesture_click: glib::WeakRef<gtk::GestureClick>,
+        /// The cache key for the current media message.
+        ///
+        /// We only try to reload the media if the key changes. This is to avoid
+        /// reloading the media when a local echo changes to a remote echo.
+        cache_key: RefCell<MessageCacheKey>,
         /// The current video file, if any.
         file: RefCell<Option<File>>,
         paintable_animation_ref: RefCell<Option<CountedRef>>,
@@ -310,13 +315,30 @@ mod imp {
             }
         }
 
+        /// Set the cache key with the given value.
+        ///
+        /// Returns `true` if the media should be reloaded.
+        fn set_cache_key(&self, key: MessageCacheKey) -> bool {
+            let should_reload = self.cache_key.borrow().should_reload(&key);
+
+            self.cache_key.replace(key);
+
+            should_reload
+        }
+
         /// Build the content for the given media message.
         pub(super) fn build(
             &self,
             media_message: VisualMediaMessage,
             session: &Session,
             format: ContentFormat,
+            cache_key: MessageCacheKey,
         ) {
+            if !self.set_cache_key(cache_key) {
+                // We do not need to reload the media.
+                return;
+            }
+
             self.file.take();
             self.dimensions.set(media_message.dimensions());
 
@@ -525,8 +547,9 @@ impl MessageVisualMedia {
         media_message: VisualMediaMessage,
         session: &Session,
         format: ContentFormat,
+        cache_key: MessageCacheKey,
     ) {
-        self.imp().build(media_message, session, format);
+        self.imp().build(media_message, session, format, cache_key);
     }
 
     /// Get the texture displayed by this widget, if any.

src/session/view/content/room_history/message_toolbar/mod.rs

@@ -12,7 +12,7 @@ use matrix_sdk::{
     attachment::{AttachmentConfig, AttachmentInfo, BaseFileInfo, Thumbnail},
     room::edit::EditedContent,
 };
-use matrix_sdk_ui::timeline::{RepliedToInfo, TimelineItemContent};
+use matrix_sdk_ui::timeline::{AttachmentSource, RepliedToInfo, TimelineItemContent};
 use ruma::{
     events::{
         room::message::{
@@ -43,8 +43,8 @@ use crate::{
     spawn, spawn_tokio, toast,
     utils::{
         media::{
-            filename_for_mime, image::ImageInfoLoader, load_audio_info, load_file,
-            video::load_video_info,
+            filename_for_mime, image::ImageInfoLoader, load_audio_info, video::load_video_info,
+            FileInfo,
         },
         template_callbacks::TemplateCallbacks,
         Location, LocationError, TokioDrop,
@@ -665,9 +665,8 @@ mod imp {
         /// Send the attachment with the given data.
         async fn send_attachment(
             &self,
-            bytes: Vec<u8>,
+            source: AttachmentSource,
             mime: mime::Mime,
-            body: String,
             info: AttachmentInfo,
             thumbnail: Option<Thumbnail>,
         ) {
@@ -677,11 +676,12 @@ mod imp {
 
             let config = AttachmentConfig::new().thumbnail(thumbnail).info(info);
 
-            let matrix_room = room.matrix_room().clone();
+            let matrix_timeline = room.timeline().matrix_timeline();
 
             let handle = spawn_tokio!(async move {
-                matrix_room
-                    .send_attachment(&body, &mime, bytes, config)
+                matrix_timeline
+                    .send_attachment(source, mime, config)
+                    .use_send_queue()
                     .await
             });
 
@@ -722,7 +722,11 @@ mod imp {
             base_info.size = filesize.map(Into::into);
 
             let info = AttachmentInfo::Image(base_info);
-            self.send_attachment(bytes.to_vec(), mime::IMAGE_PNG, filename, info, thumbnail)
+            let source = AttachmentSource::Data {
+                bytes: bytes.to_vec(),
+                filename,
+            };
+            self.send_attachment(source, mime::IMAGE_PNG, info, thumbnail)
                 .await;
         }
 
@@ -779,10 +783,16 @@ mod imp {
         async fn send_file_inner(&self, file: gio::File) {
             let obj = self.obj();
 
-            let (bytes, file_info) = match load_file(&file).await {
-                Ok(data) => data,
+            let Some(path) = file.path() else {
+                warn!("Could not read file: file does not have a path");
+                toast!(obj, gettext("Error reading file"));
+                return;
+            };
+
+            let file_info = match FileInfo::try_from_file(&file).await {
+                Ok(file_info) => file_info,
                 Err(error) => {
-                    warn!("Could not read file: {error}");
+                    warn!("Could not read file info: {error}");
                     toast!(obj, gettext("Error reading file"));
                     return;
                 }
@@ -818,7 +828,7 @@ mod imp {
                 _ => (AttachmentInfo::File(BaseFileInfo { size }), None),
             };
 
-            self.send_attachment(bytes, file_info.mime, file_info.filename, info, thumbnail)
+            self.send_attachment(path.into(), file_info.mime, info, thumbnail)
                 .await;
         }
 

src/utils/media/mod.rs

@@ -45,57 +45,54 @@ pub fn filename_for_mime(mime_type: Option<&str>, fallback: Option<mime::Name>)
         .unwrap_or(name)
 }
 
-/// Information about a file
-pub struct FileInfo {
+/// Information about a file.
+pub(crate) struct FileInfo {
     /// The mime type of the file.
-    pub mime: Mime,
+    pub(crate) mime: Mime,
     /// The name of the file.
-    pub filename: String,
+    pub(crate) filename: String,
     /// The size of the file in bytes.
-    pub size: Option<u32>,
+    pub(crate) size: Option<u32>,
 }
 
-/// Load a file and return its content and some information
-pub async fn load_file(file: &gio::File) -> Result<(Vec<u8>, FileInfo), glib::Error> {
-    let attributes: &[&str] = &[
-        gio::FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
-        gio::FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME,
-        gio::FILE_ATTRIBUTE_STANDARD_SIZE,
-    ];
-
-    // Read mime type.
-    let info = file
-        .query_info_future(
-            &attributes.join(","),
-            gio::FileQueryInfoFlags::NONE,
-            glib::Priority::DEFAULT,
-        )
-        .await?;
-
-    let mime = info
-        .content_type()
-        .and_then(|content_type| Mime::from_str(&content_type).ok())
-        .unwrap_or(mime::APPLICATION_OCTET_STREAM);
-
-    let filename = info.display_name().to_string();
-
-    let raw_size = info.size();
-    let size = if raw_size >= 0 {
-        Some(raw_size.try_into().unwrap_or(u32::MAX))
-    } else {
-        None
-    };
-
-    let (data, _) = file.load_contents_future().await?;
+impl FileInfo {
+    /// Try to load information about the given file.
+    pub(crate) async fn try_from_file(file: &gio::File) -> Result<FileInfo, glib::Error> {
+        let attributes: &[&str] = &[
+            gio::FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
+            gio::FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME,
+            gio::FILE_ATTRIBUTE_STANDARD_SIZE,
+        ];
+
+        // Read mime type.
+        let info = file
+            .query_info_future(
+                &attributes.join(","),
+                gio::FileQueryInfoFlags::NONE,
+                glib::Priority::DEFAULT,
+            )
+            .await?;
+
+        let mime = info
+            .content_type()
+            .and_then(|content_type| Mime::from_str(&content_type).ok())
+            .unwrap_or(mime::APPLICATION_OCTET_STREAM);
+
+        let filename = info.display_name().to_string();
+
+        let raw_size = info.size();
+        let size = if raw_size >= 0 {
+            Some(raw_size.try_into().unwrap_or(u32::MAX))
+        } else {
+            None
+        };
 
-    Ok((
-        data.into(),
-        FileInfo {
+        Ok(FileInfo {
             mime,
             filename,
             size,
-        },
-    ))
+        })
+    }
 }
 
 /// Load information for the given media file.