api: Add dc_msg_get_filedata_path(), deprecate dc_msg_get_file() (#4309)

`dc_msg_get_file()` should create a temporary directory inside the blobdir, save a copy with
unmangled filename, and return its path. In the housekeeping we regularly clear these
directories. This API is kept for compatibility, but deprecated.

Instead, add `dc_msg_get_filedata_path()` that avoids unnecessary copying if an app needs just to
open a file and its name doesn't matter.

Author: iequidoo

deltachat-ffi/deltachat.h

@@ -4081,14 +4081,32 @@ char*           dc_msg_get_subject            (const dc_msg_t* msg);
  *
  * Typically files are associated with images, videos, audios, documents.
  * Plain text messages do not have a file.
- * File name may be mangled. To obtain the original attachment filename use dc_msg_get_filename().
+ * The filename isn't meaningful, only the extension is preserved. To obtain the original attachment
+ * filename use dc_msg_get_filename().
  *
  * @memberof dc_msg_t
  * @param msg The message object.
  * @return The full path (with file name and extension) of the file associated with the message.
  *     If there is no file associated with the message, an empty string is returned.
  *     NULL is never returned and the returned value must be released using dc_str_unref().
  */
+char*           dc_msg_get_filedata_path      (const dc_msg_t* msg);
+
+
+/**
+ * Get full path to the copy of the file, associated with a message, with the original filename.
+ * Deprecated, use dc_msg_get_filedata_path() and dc_msg_get_filename() instead.
+ *
+ * Typically files are associated with images, videos, audios, documents.
+ * Plain text messages do not have a file.
+ *
+ * @memberof dc_msg_t
+ * @param msg The message object.
+ * @return The full path (with file name and extension) of the file associated with the message.
+ *     If there is no file associated with the message, an empty string is returned.
+ *     In case of an error an empty string is returned.
+ *     NULL is never returned and the returned value must be released using dc_str_unref().
+ */
 char*           dc_msg_get_file               (const dc_msg_t* msg);
 
 

deltachat-ffi/src/lib.rs

@@ -3359,20 +3359,39 @@ pub unsafe extern "C" fn dc_msg_get_subject(msg: *mut dc_msg_t) -> *mut libc::c_
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_msg_get_file(msg: *mut dc_msg_t) -> *mut libc::c_char {
+pub unsafe extern "C" fn dc_msg_get_filedata_path(msg: *mut dc_msg_t) -> *mut libc::c_char {
     if msg.is_null() {
-        eprintln!("ignoring careless call to dc_msg_get_file()");
+        eprintln!("ignoring careless call to dc_msg_get_filedata_path()");
         return "".strdup();
     }
     let ffi_msg = &*msg;
     let ctx = &*ffi_msg.context;
     ffi_msg
         .message
-        .get_file(ctx)
+        .get_filedata_path(ctx)
         .map(|p| p.to_string_lossy().strdup())
         .unwrap_or_else(|| "".strdup())
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn dc_msg_get_file(msg: *mut dc_msg_t) -> *mut libc::c_char {
+    if msg.is_null() {
+        eprintln!("ignoring careless call to dc_msg_get_file()");
+        return "".strdup();
+    }
+    let ffi_msg = &*msg;
+    let ctx = &*ffi_msg.context;
+    let r = block_on(ffi_msg.message.get_file(ctx));
+    let Ok(r) = r else {
+        r.context("Failed to get file from message")
+            .log_err(ctx)
+            .unwrap_or_default();
+        return "".strdup();
+    };
+    r.map(|p| p.to_string_lossy().strdup())
+        .unwrap_or_else(|| "".strdup())
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn dc_msg_save_file(
     msg: *mut dc_msg_t,

deltachat-jsonrpc/src/api/types/message.rs

@@ -148,7 +148,7 @@ impl MessageObject {
                             || quote.get_viewtype() == Viewtype::Gif
                             || quote.get_viewtype() == Viewtype::Sticker
                         {
-                            match quote.get_file(context) {
+                            match quote.get_filedata_path(context) {
                                 Some(path_buf) => path_buf.to_str().map(|s| s.to_owned()),
                                 None => None,
                             }
@@ -221,7 +221,7 @@ impl MessageObject {
 
             setup_code_begin: message.get_setupcodebegin(context).await,
 
-            file: match message.get_file(context) {
+            file: match message.get_filedata_path(context) {
                 Some(path_buf) => path_buf.to_str().map(|s| s.to_owned()),
                 None => None,
             }, //BLOBS
@@ -420,7 +420,7 @@ impl MessageNotificationInfo {
             Viewtype::Image | Viewtype::Gif | Viewtype::Sticker
         ) {
             message
-                .get_file(context)
+                .get_filedata_path(context)
                 .map(|path_buf| path_buf.to_str().map(|s| s.to_owned()))
                 .unwrap_or_default()
         } else {

python/src/deltachat/message.py

@@ -109,7 +109,7 @@ def set_html(self, html_text):
     @props.with_doc
     def file_path(self):
         """file path if there was an attachment, otherwise empty string."""
-        return from_dc_charpointer(lib.dc_msg_get_file(self._dc_msg))
+        return from_dc_charpointer(lib.dc_msg_get_filedata_path(self._dc_msg))
 
     def set_file(self, path, mime_type=None):
         """set file for this message from path and mime_type."""

src/imex.rs

@@ -280,7 +280,7 @@ pub async fn continue_key_transfer(
         "Message is no Autocrypt Setup Message."
     );
 
-    if let Some(filename) = msg.get_file(context) {
+    if let Some(filename) = msg.get_filedata_path(context) {
         let file = open_file_std(context, filename)?;
         let sc = normalize_setup_code(setup_code);
         let armored_key = decrypt_setup_file(&sc, file).await?;

src/imex/transfer.rs

@@ -651,8 +651,9 @@ mod tests {
         };
         let msg = Message::load_from_db(&ctx1, *msgid).await.unwrap();
 
-        let path = msg.get_file(&ctx1).unwrap();
-        assert_eq!(path.with_file_name("hello.txt"), path);
+        assert_eq!(&msg.get_filename().unwrap(), "hello.txt");
+        let path = msg.get_filedata_path(&ctx1).unwrap();
+        assert_eq!(path.with_extension("txt"), path);
         let text = fs::read_to_string(&path).await.unwrap();
         assert_eq!(text, "i am attachment");
 

src/message.rs

@@ -1,6 +1,7 @@
 //! # Messages and their identifiers.
 
-use std::collections::BTreeSet;
+use std::collections::{hash_map::DefaultHasher, BTreeSet};
+use std::hash::{Hash, Hasher};
 use std::path::{Path, PathBuf};
 
 use anyhow::{ensure, format_err, Context as _, Result};
@@ -288,9 +289,14 @@ WHERE id=?;
             ret += &format!("Error: {error}");
         }
 
-        if let Some(path) = msg.get_file(context) {
+        if let Some(path) = msg.get_filedata_path(context) {
             let bytes = get_filebytes(context, &path).await?;
-            ret += &format!("\nFile: {}, {} bytes\n", path.display(), bytes);
+            ret += &format!(
+                "\nFile: {}, name: {}, {} bytes\n",
+                path.display(),
+                msg.get_filename().unwrap_or_default(),
+                bytes
+            );
         }
 
         if msg.viewtype != Viewtype::Text {
@@ -575,14 +581,49 @@ impl Message {
         None
     }
 
-    /// Returns the full path to the file associated with a message.
-    pub fn get_file(&self, context: &Context) -> Option<PathBuf> {
+    /// Returns the full path to the file associated with a message. The filename isn't meaningful,
+    /// only the extension is preserved.
+    pub fn get_filedata_path(&self, context: &Context) -> Option<PathBuf> {
         self.param.get_path(Param::File, context).unwrap_or(None)
     }
 
+    /// Returns the full path to the copy of the file, associated with a message, with the original
+    /// filename.
+    ///
+    /// Deprecated, use `get_filedata_path()` and `get_filename()` instead.
+    pub async fn get_file(&self, context: &Context) -> Result<Option<PathBuf>> {
+        let Some(data_path) = self.get_filedata_path(context) else {
+            return Ok(None);
+        };
+        let Some(filename) = self.get_filename() else {
+            return Ok(Some(data_path));
+        };
+        if data_path.with_file_name(&filename) == data_path {
+            return Ok(Some(data_path));
+        }
+        let mut hasher = DefaultHasher::new();
+        data_path
+            .file_name()
+            .context("no filename??")?
+            .hash(&mut hasher);
+        let dirname = format!("tmp.{:016x}", hasher.finish());
+        let dir_path = context.get_blobdir().join(&dirname);
+        fs::create_dir_all(&dir_path).await?;
+        let file_path = dir_path.join(&filename);
+        let mut src = fs::OpenOptions::new().read(true).open(data_path).await?;
+        let mut dst = fs::OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(true)
+            .open(&file_path)
+            .await?;
+        io::copy(&mut src, &mut dst).await?;
+        Ok(Some(file_path))
+    }
+
     /// Save file copy at the user-provided path.
     pub async fn save_file(&self, context: &Context, path: &Path) -> Result<()> {
-        let path_src = self.get_file(context).context("No file")?;
+        let path_src = self.get_filedata_path(context).context("No file")?;
         let mut src = fs::OpenOptions::new().read(true).open(path_src).await?;
         let mut dst = fs::OpenOptions::new()
             .write(true)
@@ -728,8 +769,6 @@ impl Message {
     }
 
     /// Returns original filename (as shown in chat).
-    ///
-    /// To get the full path, use [`Self::get_file()`].
     pub fn get_filename(&self) -> Option<String> {
         if let Some(name) = self.param.get(Param::Filename) {
             return Some(name.to_string());
@@ -904,7 +943,7 @@ impl Message {
             return None;
         }
 
-        if let Some(filename) = self.get_file(context) {
+        if let Some(filename) = self.get_filedata_path(context) {
             if let Ok(ref buf) = read_file(context, filename).await {
                 if let Ok((typ, headers, _)) = split_armored_data(buf) {
                     if typ == pgp::armor::BlockType::Message {
@@ -1925,7 +1964,7 @@ pub enum Viewtype {
 
     /// Animated GIF message.
     /// File, width and height are set via dc_msg_set_file(), dc_msg_set_dimension()
-    /// and retrieved via dc_msg_get_file(), dc_msg_get_width(), dc_msg_get_height().
+    /// and retrieved via dc_msg_get_filedata_path(), dc_msg_get_width(), dc_msg_get_height().
     Gif = 21,
 
     /// Message containing a sticker, similar to image.
@@ -1935,26 +1974,24 @@ pub enum Viewtype {
 
     /// Message containing an Audio file.
     /// File and duration are set via dc_msg_set_file(), dc_msg_set_duration()
-    /// and retrieved via dc_msg_get_file(), dc_msg_get_duration().
+    /// and retrieved via dc_msg_get_filedata_path(), dc_msg_get_duration().
     Audio = 40,
 
     /// A voice message that was directly recorded by the user.
     /// For all other audio messages, the type #DC_MSG_AUDIO should be used.
     /// File and duration are set via dc_msg_set_file(), dc_msg_set_duration()
-    /// and retrieved via dc_msg_get_file(), dc_msg_get_duration()
+    /// and retrieved via dc_msg_get_filedata_path(), dc_msg_get_duration().
     Voice = 41,
 
     /// Video messages.
     /// File, width, height and durarion
     /// are set via dc_msg_set_file(), dc_msg_set_dimension(), dc_msg_set_duration()
-    /// and retrieved via
-    /// dc_msg_get_file(), dc_msg_get_width(),
+    /// and retrieved via dc_msg_get_filedata_path(), dc_msg_get_width(),
     /// dc_msg_get_height(), dc_msg_get_duration().
     Video = 50,
 
     /// Message containing any file, eg. a PDF.
-    /// The file is set via dc_msg_set_file()
-    /// and retrieved via dc_msg_get_file().
+    /// The file is set via dc_msg_set_file() and retrieved via dc_msg_get_filedata_path().
     File = 60,
 
     /// Message is an invitation to a videochat.

src/mimeparser.rs

@@ -3458,7 +3458,10 @@ On 2020-10-25, Bob wrote:
         assert_eq!(msg.chat_blocked, Blocked::Request);
         assert_eq!(msg.state, MessageState::InFresh);
         assert_eq!(msg.get_filebytes(&t).await.unwrap().unwrap(), 2115);
-        assert!(msg.get_file(&t).is_some());
+        assert_eq!(
+            msg.get_filedata_path(&t).unwrap().extension().unwrap(),
+            "png"
+        );
         assert_eq!(msg.get_filename().unwrap(), "avatar64x64.png");
         assert_eq!(msg.get_width(), 64);
         assert_eq!(msg.get_height(), 64);

src/receive_imf/tests.rs

@@ -2964,7 +2964,7 @@ async fn test_long_and_duplicated_filenames() -> Result<()> {
             assert_eq!(msg.get_viewtype(), Viewtype::File);
             let resulting_filename = msg.get_filename().unwrap();
             assert_eq!(resulting_filename, filename);
-            let path = msg.get_file(t).unwrap();
+            let path = msg.get_filedata_path(t).unwrap();
             let path2 = path.with_file_name("saved.txt");
             msg.save_file(t, &path2).await.unwrap();
             assert!(
@@ -2974,6 +2974,13 @@ async fn test_long_and_duplicated_filenames() -> Result<()> {
             assert_eq!(fs::read_to_string(&path).await.unwrap(), content);
             assert_eq!(fs::read_to_string(&path2).await.unwrap(), content);
             fs::remove_file(path2).await.unwrap();
+
+            let path = msg.get_file(t).await.unwrap().unwrap();
+            assert_eq!(path.with_file_name(resulting_filename), path);
+            assert_eq!(fs::read_to_string(&path).await.unwrap(), content);
+            let path2 = msg.get_file(t).await.unwrap().unwrap();
+            assert_eq!(path2, path);
+            assert_eq!(fs::read_to_string(&path).await.unwrap(), content);
         }
         check_message(&msg_alice, &alice, filename_sent, &content).await;
         check_message(&msg_bob, &bob, filename_sent, &content).await;

src/summary.rs

@@ -96,7 +96,7 @@ impl Summary {
             || msg.viewtype == Viewtype::Gif
             || msg.viewtype == Viewtype::Sticker
         {
-            msg.get_file(context)
+            msg.get_filedata_path(context)
                 .and_then(|path| path.to_str().map(|p| p.to_owned()))
         } else {
             None