Generate rfc724_mid when creating Message (#6704)

Set `rfc724_mid` in `Message::new()`, `Message::new_text()`, and
`Message::default()` instead of when sending the message. This way the
rfc724 mid can be read in the draft stage which makes it more consistent
for bots. Tests had to be adjusted to create multiple messages to get
unique mid, otherwise core would not send the messages out.

Author: Septias

deltachat-ffi/src/lib.rs

@@ -37,6 +37,7 @@ use deltachat::*;
 use deltachat::{accounts::Accounts, log::LogExt};
 use deltachat_jsonrpc::api::CommandApi;
 use deltachat_jsonrpc::yerpc::{OutReceiver, RpcClient, RpcSession};
+use message::Viewtype;
 use num_traits::{FromPrimitive, ToPrimitive};
 use rand::Rng;
 use tokio::runtime::Runtime;
@@ -2076,7 +2077,7 @@ pub unsafe extern "C" fn dc_get_msg(context: *mut dc_context_t, msg_id: u32) ->
                         ctx,
                         "dc_get_msg called with special msg_id={msg_id}, returning empty msg"
                     );
-                    message::Message::default()
+                    message::Message::new(Viewtype::default())
                 } else {
                     warn!(ctx, "dc_get_msg could not retrieve msg_id {msg_id}: {e:#}");
                     return ptr::null_mut();

src/chat.rs

@@ -2005,7 +2005,9 @@ impl Chat {
         let mut to_id = 0;
         let mut location_id = 0;
 
-        let new_rfc724_mid = create_outgoing_rfc724_mid();
+        if msg.rfc724_mid.is_empty() {
+            msg.rfc724_mid = create_outgoing_rfc724_mid();
+        }
 
         if self.typ == Chattype::Single {
             if let Some(id) = context
@@ -2103,7 +2105,7 @@ impl Chat {
             if references_vec.is_empty() {
                 // As a fallback, use our Message-ID,
                 // same as in the case of top-level message.
-                new_references = new_rfc724_mid.clone();
+                new_references = msg.rfc724_mid.clone();
             } else {
                 new_references = references_vec.join(" ");
             }
@@ -2113,7 +2115,7 @@ impl Chat {
             // This allows us to identify replies to our message even if
             // email server such as Outlook changes `Message-ID:` header.
             // MUAs usually keep the first Message-ID in `References:` header unchanged.
-            new_references = new_rfc724_mid.clone();
+            new_references = msg.rfc724_mid.clone();
         }
 
         // add independent location to database
@@ -2181,7 +2183,6 @@ impl Chat {
 
         msg.chat_id = self.id;
         msg.from_id = ContactId::SELF;
-        msg.rfc724_mid = new_rfc724_mid;
         msg.timestamp_sort = timestamp;
 
         // add message to the database
@@ -3846,7 +3847,7 @@ pub(crate) async fn add_contact_to_chat_ex(
 ) -> Result<bool> {
     ensure!(!chat_id.is_special(), "can not add member to special chats");
     let contact = Contact::get_by_id(context, contact_id).await?;
-    let mut msg = Message::default();
+    let mut msg = Message::new(Viewtype::default());
 
     chat_id.reset_gossiped_timestamp(context).await?;
 
@@ -4077,7 +4078,7 @@ pub async fn remove_contact_from_chat(
         "Cannot remove special contact"
     );
 
-    let mut msg = Message::default();
+    let mut msg = Message::new(Viewtype::default());
 
     let chat = Chat::load_from_db(context, chat_id).await?;
     if chat.typ == Chattype::Group || chat.typ == Chattype::Broadcast {
@@ -4184,7 +4185,7 @@ async fn rename_ex(
     ensure!(!chat_id.is_special(), "Invalid chat ID");
 
     let chat = Chat::load_from_db(context, chat_id).await?;
-    let mut msg = Message::default();
+    let mut msg = Message::new(Viewtype::default());
 
     if chat.typ == Chattype::Group
         || chat.typ == Chattype::Mailinglist
@@ -4347,9 +4348,11 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId)
         msg.subject = "".to_string();
 
         msg.state = MessageState::OutPending;
+        msg.rfc724_mid = create_outgoing_rfc724_mid();
         let new_msg_id = chat
             .prepare_msg_raw(context, &mut msg, None, curr_timestamp)
             .await?;
+
         curr_timestamp += 1;
         if !create_send_msg_jobs(context, &mut msg).await?.is_empty() {
             context.scheduler.interrupt_smtp().await;

src/chat/chat_tests.rs

@@ -2040,20 +2040,28 @@ async fn test_sticker_forward() -> Result<()> {
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_forward() -> Result<()> {
+async fn test_forward_basic() -> Result<()> {
     let alice = TestContext::new_alice().await;
     let bob = TestContext::new_bob().await;
     let alice_chat = alice.create_chat(&bob).await;
     let bob_chat = bob.create_chat(&alice).await;
 
-    let mut msg = Message::new_text("Hi Bob".to_owned());
-    let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await;
+    let mut alice_msg = Message::new_text("Hi Bob".to_owned());
+    let sent_msg = alice.send_msg(alice_chat.get_id(), &mut alice_msg).await;
     let msg = bob.recv_msg(&sent_msg).await;
+    assert_eq!(alice_msg.rfc724_mid, msg.rfc724_mid);
 
     forward_msgs(&bob, &[msg.id], bob_chat.get_id()).await?;
 
     let forwarded_msg = bob.pop_sent_msg().await;
+    assert_eq!(bob_chat.id.get_msg_cnt(&bob).await?, 2);
+    assert_ne!(
+        forwarded_msg.load_from_db().await.rfc724_mid,
+        msg.rfc724_mid,
+    );
+    let msg_bob = Message::load_from_db(&bob, forwarded_msg.sender_msg_id).await?;
     let msg = alice.recv_msg(&forwarded_msg).await;
+    assert_eq!(msg.rfc724_mid(), msg_bob.rfc724_mid());
     assert_eq!(msg.get_text(), "Hi Bob");
     assert!(msg.is_forwarded());
     Ok(())

src/imex/key_transfer.rs

@@ -32,10 +32,7 @@ pub async fn initiate_key_transfer(context: &Context) -> Result<String> {
     )?;
 
     let chat_id = ChatId::create_for_contact(context, ContactId::SELF).await?;
-    let mut msg = Message {
-        viewtype: Viewtype::File,
-        ..Default::default()
-    };
+    let mut msg = Message::new(Viewtype::File);
     msg.param.set(Param::File, setup_file_blob.as_name());
     msg.param
         .set(Param::Filename, "autocrypt-setup-message.html");

src/message.rs

@@ -33,6 +33,7 @@ use crate::reaction::get_msg_reactions;
 use crate::sql;
 use crate::summary::Summary;
 use crate::sync::SyncData;
+use crate::tools::create_outgoing_rfc724_mid;
 use crate::tools::{
     buf_compress, buf_decompress, get_filebytes, get_filemeta, gm2local_offset, read_file,
     sanitize_filename, time, timestamp_to_str,
@@ -470,6 +471,7 @@ impl Message {
     pub fn new(viewtype: Viewtype) -> Self {
         Message {
             viewtype,
+            rfc724_mid: create_outgoing_rfc724_mid(),
             ..Default::default()
         }
     }
@@ -479,6 +481,7 @@ impl Message {
         Message {
             viewtype: Viewtype::Text,
             text,
+            rfc724_mid: create_outgoing_rfc724_mid(),
             ..Default::default()
         }
     }

src/message/message_tests.rs

@@ -135,7 +135,7 @@ async fn test_get_width_height() {
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_quote() {
+async fn test_quote_basic() {
     let d = TestContext::new().await;
     let ctx = &d.ctx;
 
@@ -144,13 +144,10 @@ async fn test_quote() {
         .unwrap();
 
     let chat = d.create_chat_with_contact("", "dest@example.com").await;
-
     let mut msg = Message::new_text("Quoted message".to_string());
 
-    // Send message, so it gets a Message-Id.
-    assert!(msg.rfc724_mid.is_empty());
-    let msg_id = chat::send_msg(ctx, chat.id, &mut msg).await.unwrap();
-    let msg = Message::load_from_db(ctx, msg_id).await.unwrap();
+    // Message has to be sent such that it gets saved to db.
+    chat::send_msg(ctx, chat.id, &mut msg).await.unwrap();
     assert!(!msg.rfc724_mid.is_empty());
 
     let mut msg2 = Message::new(Viewtype::Text);
@@ -358,6 +355,7 @@ async fn test_markseen_msgs() -> Result<()> {
     let sent1 = alice.send_msg(alice_chat.id, &mut msg).await;
     let msg1 = bob.recv_msg(&sent1).await;
     let bob_chat_id = msg1.chat_id;
+    let mut msg = Message::new_text("this is the text!".to_string());
     let sent2 = alice.send_msg(alice_chat.id, &mut msg).await;
     let msg2 = bob.recv_msg(&sent2).await;
     assert_eq!(msg1.chat_id, msg2.chat_id);
@@ -380,9 +378,11 @@ async fn test_markseen_msgs() -> Result<()> {
 
     // bob sends to alice,
     // alice knows bob and messages appear in normal chat
+    let mut msg = Message::new_text("this is the text!".to_string());
     let msg1 = alice
         .recv_msg(&bob.send_msg(bob_chat_id, &mut msg).await)
         .await;
+    let mut msg = Message::new_text("this is the text!".to_string());
     let msg2 = alice
         .recv_msg(&bob.send_msg(bob_chat_id, &mut msg).await)
         .await;

src/mimefactory/mimefactory_tests.rs

@@ -728,6 +728,7 @@ async fn test_selfavatar_unencrypted_signed() {
         .is_some());
 
     // if another message is sent, that one must not contain the avatar
+    let mut msg = Message::new_text("this is the text!".to_string());
     let sent_msg = t.send_msg(chat.id, &mut msg).await;
     let mut payload = sent_msg.payload().splitn(4, "\r\n\r\n");
 

src/receive_imf/receive_imf_tests.rs

@@ -5090,7 +5090,7 @@ async fn test_references() -> Result<()> {
     alice.set_config_bool(Config::BccSelf, true).await?;
 
     let alice_chat_id = create_group_chat(alice, ProtectionStatus::Unprotected, "Group").await?;
-    let _sent = alice
+    alice
         .send_text(alice_chat_id, "Hi! I created a group.")
         .await;
 

src/webxdc/webxdc_tests.rs

@@ -2195,3 +2195,25 @@ async fn test_webxdc_href() -> Result<()> {
 
     Ok(())
 }
+
+/// Test that the draft `selfAddr` is equal to the `selfAddr` of the sent message.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_self_addr_consistency() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let alice_chat = alice
+        .create_group_with_members(ProtectionStatus::Unprotected, "No friends :(", &[])
+        .await;
+    let mut instance = create_webxdc_instance(
+        alice,
+        "minimal.xdc",
+        include_bytes!("../../test-data/webxdc/minimal.xdc"),
+    )?;
+    alice_chat.set_draft(alice, Some(&mut instance)).await?;
+    let self_addr = instance.get_webxdc_self_addr(alice).await?;
+    let sent = alice.send_msg(alice_chat, &mut instance).await;
+    let db_msg = Message::load_from_db(alice, sent.sender_msg_id).await?;
+    assert_eq!(db_msg.get_webxdc_self_addr(alice).await?, self_addr);
+    assert_eq!(alice_chat.get_msg_cnt(alice).await?, 1);
+    Ok(())
+}