refactor: Remove Message.set_file() / dc_msg_set_file() and related code (#6558)

Now that we are deduplicating everywhere, we can get rid of some code.

The old python bindings did not get an optional `name` parameter because
they are deprecated anyway, but it would be easy to add it.

Author: Hocuri

deltachat-ffi/deltachat.h

@@ -975,7 +975,7 @@ uint32_t        dc_get_chat_id_by_contact_id (dc_context_t* context, uint32_t co
  * ~~~
  * dc_msg_t* msg = dc_msg_new(context, DC_MSG_IMAGE);
  *
- * dc_msg_set_file(msg, "/file/to/send.jpg", NULL);
+ * dc_msg_set_file_and_deduplicate(msg, "/file/to/send.jpg", NULL, NULL);
  * dc_send_msg(context, chat_id, msg);
  *
  * dc_msg_unref(msg);
@@ -4772,22 +4772,6 @@ void            dc_msg_set_subject            (dc_msg_t* msg, const char* subjec
 void            dc_msg_set_override_sender_name(dc_msg_t* msg, const char* name);
 
 
-/**
- * Set the file associated with a message object.
- * This does not alter any information in the database
- * nor copy or move the file or checks if the file exist.
- * All this can be done with dc_send_msg() later.
- *
- * @memberof dc_msg_t
- * @param msg The message object.
- * @param file If the message object is used in dc_send_msg() later,
- *     this must be the full path of the image file to send.
- * @param filemime The MIME type of the file. NULL if you don't know or don't care.
- * @deprecated 2025-01-21 Use dc_msg_set_file_and_deduplicate instead
- */
-void            dc_msg_set_file               (dc_msg_t* msg, const char* file, const char* filemime);
-
-
 /**
  * Sets the file associated with a message.
  *
@@ -4815,7 +4799,7 @@ void            dc_msg_set_file_and_deduplicate(dc_msg_t* msg, const char* file,
 
 /**
  * Set the dimensions associated with message object.
- * Typically this is the width and the height of an image or video associated using dc_msg_set_file().
+ * Typically this is the width and the height of an image or video associated using dc_msg_set_file_and_deduplicate().
  * This does not alter any information in the database; this may be done by dc_send_msg() later.
  *
  * @memberof dc_msg_t
@@ -4828,7 +4812,7 @@ void            dc_msg_set_dimension          (dc_msg_t* msg, int width, int hei
 
 /**
  * Set the duration associated with message object.
- * Typically this is the duration of an audio or video associated using dc_msg_set_file().
+ * Typically this is the duration of an audio or video associated using dc_msg_set_file_and_deduplicate().
  * This does not alter any information in the database; this may be done by dc_send_msg() later.
  *
  * @memberof dc_msg_t
@@ -5467,7 +5451,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
  * If you want to define the type of a dc_msg_t object for sending,
  * use dc_msg_new().
  * Depending on the type, you will set more properties using e.g.
- * dc_msg_set_text() or dc_msg_set_file().
+ * dc_msg_set_text() or dc_msg_set_file_and_deduplicate().
  * To finally send the message, use dc_send_msg().
  *
  * To get the types of dc_msg_t objects received, use dc_msg_get_viewtype().
@@ -5488,7 +5472,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 /**
  * Image message.
  * If the image is an animated GIF, the type #DC_MSG_GIF should be used.
- * File, width, and height are set via dc_msg_set_file(), dc_msg_set_dimension()
+ * File, width, and height are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension()
  * and retrieved via dc_msg_get_file(), dc_msg_get_width(), and dc_msg_get_height().
  *
  * Before sending, the image is recoded to an reasonable size,
@@ -5501,7 +5485,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 
 /**
  * Animated GIF message.
- * File, width, and height are set via dc_msg_set_file(), dc_msg_set_dimension()
+ * File, width, and height are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension()
  * and retrieved via dc_msg_get_file(), dc_msg_get_width(), and dc_msg_get_height().
  */
 #define DC_MSG_GIF       21
@@ -5519,7 +5503,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 
 /**
  * Message containing an audio file.
- * File and duration are set via dc_msg_set_file(), dc_msg_set_duration()
+ * File and duration are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_duration()
  * and retrieved via dc_msg_get_file(), and dc_msg_get_duration().
  */
 #define DC_MSG_AUDIO     40
@@ -5528,7 +5512,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 /**
  * 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()
+ * File and duration are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_duration()
  * and retrieved via dc_msg_get_file(), and dc_msg_get_duration().
  */
 #define DC_MSG_VOICE     41
@@ -5537,7 +5521,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 /**
  * Video messages.
  * File, width, height, and duration
- * are set via dc_msg_set_file(), dc_msg_set_dimension(), dc_msg_set_duration()
+ * are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension(), dc_msg_set_duration()
  * and retrieved via
  * dc_msg_get_file(), dc_msg_get_width(),
  * dc_msg_get_height(), and dc_msg_get_duration().
@@ -5547,7 +5531,7 @@ int64_t         dc_lot_get_timestamp     (const dc_lot_t* lot);
 
 /**
  * Message containing any file, e.g. a PDF.
- * The file is set via dc_msg_set_file()
+ * The file is set via dc_msg_set_file_and_deduplicate()
  * and retrieved via dc_msg_get_file().
  */
 #define DC_MSG_FILE      60

deltachat-ffi/src/lib.rs

@@ -3845,23 +3845,6 @@ pub unsafe extern "C" fn dc_msg_set_override_sender_name(
         .set_override_sender_name(to_opt_string_lossy(name))
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn dc_msg_set_file(
-    msg: *mut dc_msg_t,
-    file: *const libc::c_char,
-    filemime: *const libc::c_char,
-) {
-    if msg.is_null() || file.is_null() {
-        eprintln!("ignoring careless call to dc_msg_set_file()");
-        return;
-    }
-    let ffi_msg = &mut *msg;
-    ffi_msg.message.set_file(
-        to_string_lossy(file),
-        to_opt_string_lossy(filemime).as_deref(),
-    )
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn dc_msg_set_file_and_deduplicate(
     msg: *mut dc_msg_t,

python/src/deltachat/message.py

@@ -118,7 +118,7 @@ def set_file(self, path, mime_type=None):
         mtype = ffi.NULL if mime_type is None else as_dc_charpointer(mime_type)
         if not os.path.exists(path):
             raise ValueError(f"path does not exist: {path!r}")
-        lib.dc_msg_set_file(self._dc_msg, as_dc_charpointer(path), mtype)
+        lib.dc_msg_set_file_and_deduplicate(self._dc_msg, as_dc_charpointer(path), ffi.NULL, mtype)
 
     @props.with_doc
     def basename(self) -> str:

python/tests/test_3_offline.py

@@ -436,11 +436,11 @@ def test_message_file(self, chat1, data, lp, fn, typein, typeout):
         assert msg.id > 0
         assert msg.is_file()
         assert os.path.exists(msg.filename)
-        assert msg.filename.endswith(msg.basename)
+        assert msg.filename.endswith(".txt") == fn.endswith(".txt")
         assert msg.filemime == typeout
         msg2 = chat1.send_file(fp, typein)
         assert msg2 != msg
-        assert msg2.filename != msg.filename
+        assert msg2.filename == msg.filename
 
     def test_create_contact(self, acfactory):
         ac1 = acfactory.get_pseudo_configured_account()

src/blob.rs

@@ -288,16 +288,6 @@ impl<'a> BlobObject<'a> {
         &self.name
     }
 
-    /// Returns the filename of the blob.
-    pub fn as_file_name(&self) -> &str {
-        self.name.rsplit('/').next().unwrap_or_default()
-    }
-
-    /// The path relative in the blob directory.
-    pub fn as_rel_path(&self) -> &Path {
-        Path::new(self.as_file_name())
-    }
-
     /// Returns the extension of the blob.
     ///
     /// If a blob's filename has an extension, it is always guaranteed

src/blob/blob_tests.rs

@@ -2,6 +2,7 @@ use std::time::Duration;
 
 use super::*;
 use crate::message::{Message, Viewtype};
+use crate::param::Param;
 use crate::sql;
 use crate::test_utils::{self, TestContext};
 use crate::tools::SystemTime;
@@ -44,20 +45,6 @@ async fn test_lowercase_ext() {
     );
 }
 
-#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_as_file_name() {
-    let t = TestContext::new().await;
-    let blob = BlobObject::create_and_deduplicate_from_bytes(&t, FILE_BYTES, "foo.txt").unwrap();
-    assert_eq!(blob.as_file_name(), FILE_DEDUPLICATED);
-}
-
-#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_as_rel_path() {
-    let t = TestContext::new().await;
-    let blob = BlobObject::create_and_deduplicate_from_bytes(&t, FILE_BYTES, "foo.txt").unwrap();
-    assert_eq!(blob.as_rel_path(), Path::new(FILE_DEDUPLICATED));
-}
-
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_suffix() {
     let t = TestContext::new().await;
@@ -655,6 +642,12 @@ impl SendImageCheckMediaquality<'_> {
         alice_msg.save_file(&alice, &file_saved).await?;
         check_image_size(file_saved, compressed_width, compressed_height);
 
+        if original_width == compressed_width {
+            assert_extension(&alice, alice_msg, extension).await;
+        } else {
+            assert_extension(&alice, alice_msg, "jpg").await;
+        }
+
         let bob_msg = bob.recv_msg(&sent).await;
         assert_eq!(bob_msg.get_viewtype(), Viewtype::Image);
         assert_eq!(bob_msg.get_width() as u32, compressed_width);
@@ -673,10 +666,53 @@ impl SendImageCheckMediaquality<'_> {
         assert!(exif.is_none());
 
         let img = check_image_size(file_saved, compressed_width, compressed_height);
+
+        if original_width == compressed_width {
+            assert_extension(&bob, bob_msg, extension).await;
+        } else {
+            assert_extension(&bob, bob_msg, "jpg").await;
+        }
+
         Ok(img)
     }
 }
 
+async fn assert_extension(context: &TestContext, msg: Message, extension: &str) {
+    assert!(msg
+        .param
+        .get(Param::File)
+        .unwrap()
+        .ends_with(&format!(".{extension}")));
+    assert!(msg
+        .param
+        .get(Param::Filename)
+        .unwrap()
+        .ends_with(&format!(".{extension}")));
+    assert!(msg
+        .get_filename()
+        .unwrap()
+        .ends_with(&format!(".{extension}")));
+    assert_eq!(
+        msg.get_file(context)
+            .unwrap()
+            .extension()
+            .unwrap()
+            .to_str()
+            .unwrap(),
+        extension
+    );
+    assert_eq!(
+        msg.param
+            .get_blob(Param::File, context)
+            .await
+            .unwrap()
+            .unwrap()
+            .suffix()
+            .unwrap(),
+        extension
+    );
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_send_big_gif_as_image() -> Result<()> {
     let bytes = include_bytes!("../../test-data/image/screenshot.gif");

src/chat.rs

@@ -890,12 +890,6 @@ impl ChatId {
                 }
             }
             _ => {
-                let blob = msg
-                    .param
-                    .get_blob(Param::File, context)
-                    .await?
-                    .context("no file stored in params")?;
-                msg.param.set(Param::File, blob.as_name());
                 if msg.viewtype == Viewtype::File {
                     if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg)
                         // We do not do an automatic conversion to other viewtypes here so that
@@ -908,6 +902,11 @@ impl ChatId {
                     }
                 }
                 if msg.viewtype == Viewtype::Vcard {
+                    let blob = msg
+                        .param
+                        .get_blob(Param::File, context)
+                        .await?
+                        .context("no file stored in params")?;
                     msg.try_set_vcard(context, &blob.to_abs_path()).await?;
                 }
             }
@@ -2801,20 +2800,12 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
                 .recode_to_image_size(context, msg.get_filename(), &mut maybe_sticker)
                 .await?;
             msg.param.set(Param::Filename, new_name);
+            msg.param.set(Param::File, blob.as_name());
 
             if !maybe_sticker {
                 msg.viewtype = Viewtype::Image;
             }
         }
-        msg.param.set(Param::File, blob.as_name());
-        if let (Some(filename), Some(blob_ext)) = (msg.param.get(Param::Filename), blob.suffix()) {
-            let stem = match filename.rsplit_once('.') {
-                Some((stem, _)) => stem,
-                None => filename,
-            };
-            msg.param
-                .set(Param::Filename, stem.to_string() + "." + blob_ext);
-        }
 
         if !msg.param.exists(Param::MimeType) {
             if let Some((_, mime)) = message::guess_msgtype_from_suffix(msg) {

src/message.rs

@@ -1075,21 +1075,6 @@ impl Message {
         self.subject = subject;
     }
 
-    /// Sets the file associated with a message.
-    ///
-    /// This function does not use the file or check if it exists,
-    /// the file will only be used when the message is prepared
-    /// for sending.
-    pub fn set_file(&mut self, file: impl ToString, filemime: Option<&str>) {
-        if let Some(name) = Path::new(&file.to_string()).file_name() {
-            if let Some(name) = name.to_str() {
-                self.param.set(Param::Filename, name);
-            }
-        }
-        self.param.set(Param::File, file);
-        self.param.set_optional(Param::MimeType, filemime);
-    }
-
     /// Sets the file associated with a message, deduplicating files with the same name.
     ///
     /// If `name` is Some, it is used as the file name
@@ -2167,12 +2152,12 @@ pub enum Viewtype {
     /// Image message.
     /// If the image is a GIF and has the appropriate extension, the viewtype is auto-changed to
     /// `Gif` when sending the message.
-    /// File, width and height are set via dc_msg_set_file(), dc_msg_set_dimension
-    /// and retrieved via dc_msg_set_file(), dc_msg_set_dimension().
+    /// File, width and height are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension()
+    /// and retrieved via dc_msg_get_file(), dc_msg_get_height(), dc_msg_get_width().
     Image = 20,
 
     /// Animated GIF message.
-    /// File, width and height are set via dc_msg_set_file(), dc_msg_set_dimension()
+    /// File, width and height are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension()
     /// and retrieved via dc_msg_get_file(), dc_msg_get_width(), dc_msg_get_height().
     Gif = 21,
 
@@ -2185,26 +2170,26 @@ pub enum Viewtype {
     Sticker = 23,
 
     /// Message containing an Audio file.
-    /// File and duration are set via dc_msg_set_file(), dc_msg_set_duration()
+    /// File and duration are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_duration()
     /// and retrieved via dc_msg_get_file(), 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()
+    /// File and duration are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_duration()
     /// and retrieved via dc_msg_get_file(), 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()
+    /// are set via dc_msg_set_file_and_deduplicate(), dc_msg_set_dimension(), dc_msg_set_duration()
     /// and retrieved via
     /// dc_msg_get_file(), 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()
+    /// The file is set via dc_msg_set_file_and_deduplicate()
     /// and retrieved via dc_msg_get_file().
     File = 60,
 

src/param.rs

@@ -547,7 +547,7 @@ mod tests {
 
         fs::write(fname, b"boo").await.unwrap();
         let blob = p.get_blob(Param::File, &t).await.unwrap().unwrap();
-        assert!(blob.as_file_name().starts_with("foo"));
+        assert!(blob.as_name().starts_with("$BLOBDIR/foo"));
 
         // Blob in blobdir, expect blob.
         let bar_path = t.get_blobdir().join("bar");