fix: markseen_msgs(): Mark reactions to specified messages as seen too (#7884)

This allows to remove notifications for reactions from other devices. NB: UIs should pass all
messages to markseen_msgs(), incl. outgoing ones. markseen_msgs() should be called when a message
comes into view or when a reaction for a message being in view arrives.

Also don't emit `MsgsNoticed` from receive_imf_inner() if the chat still contains fresh hidden
messages, i.e. include reactions into this logic, to avoid removing notifications for reactions
until they are seen on another device.

deltachat-ffi/deltachat.h

@@ -2127,9 +2127,14 @@ int             dc_resend_msgs               (dc_context_t* context, const uint3
 
 
 /**
- * Mark messages as presented to the user.
+ * Mark messages and reactions to them as presented to the user.
  * Typically, UIs call this function on scrolling through the message list,
  * when the messages are presented at least for a little moment.
+ * UIs should pass all messages to this function, incl. outgoing and info ones, as this is used also
+ * for synchronization and to track last position.
+ * This should also be called when a reaction for a message being in view arrives.
+ * If this is called for already presented messages, unless they have new reactions, nothing
+ * happens.
  * The concrete action depends on the type of the chat and on the users settings
  * (dc_msgs_presented() may be a better name therefore, but well. :)
  *

deltachat-jsonrpc/src/api.rs

@@ -1327,6 +1327,11 @@ impl CommandApi {
     /// Mark messages as presented to the user.
     /// Typically, UIs call this function on scrolling through the message list,
     /// when the messages are presented at least for a little moment.
+    /// UIs should pass all messages to this function, incl. outgoing and info ones, as this is used
+    /// also for synchronization and to track last position.
+    /// This should also be called when a reaction for a message being in view arrives.
+    /// If this is called for already presented messages, unless they have new reactions, nothing
+    /// happens.
     /// The concrete action depends on the type of the chat and on the users settings
     /// (dc_msgs_presented() may be a better name therefore, but well. :)
     ///

deltachat-rpc-client/tests/test_something.py

@@ -416,27 +416,32 @@ def test_dont_move_sync_msgs(acfactory, direct_imap):
         time.sleep(1)
 
 
-def test_reaction_seen_on_another_dev(acfactory) -> None:
+@pytest.mark.parametrize("chat_noticed", [False, True])
+def test_reaction_seen_on_another_dev(acfactory, chat_noticed) -> None:
     alice, bob = acfactory.get_online_accounts(2)
     alice2 = alice.clone()
     alice2.start_io()
 
     alice_contact_bob = alice.create_contact(bob, "Bob")
     alice_chat_bob = alice_contact_bob.create_chat()
-    alice_chat_bob.send_text("Hello!")
+    alice_msg = alice_chat_bob.send_text("Hello!")
 
     event = bob.wait_for_incoming_msg_event()
     msg_id = event.msg_id
 
-    message = bob.get_message_by_id(msg_id)
-    snapshot = message.get_snapshot()
+    bob_msg = bob.get_message_by_id(msg_id)
+    snapshot = bob_msg.get_snapshot()
     snapshot.chat.accept()
-    message.send_reaction("😎")
+    bob_msg.send_reaction("😎")
     for a in [alice, alice2]:
         a.wait_for_event(EventType.INCOMING_REACTION)
 
     alice2.clear_all_events()
-    alice_chat_bob.mark_noticed()
+    if chat_noticed:
+        alice_chat_bob.mark_noticed()
+    else:
+        # UIs are supposed to mark messages being in view as seen, not reactions themselves.
+        alice_msg.mark_seen()
     chat_id = alice2.wait_for_event(EventType.MSGS_NOTICED).chat_id
     alice2_chat_bob = alice2.create_chat(bob)
     assert chat_id == alice2_chat_bob.id

src/imap.rs

@@ -323,8 +323,7 @@ impl Imap {
         if !ratelimit_duration.is_zero() {
             warn!(
                 context,
-                "Transport {}: IMAP got rate limited, waiting for {} until can connect.",
-                self.transport_id,
+                "IMAP got rate limited, waiting for {} until can connect.",
                 duration_to_str(ratelimit_duration),
             );
             let interrupted = async {
@@ -336,16 +335,12 @@ impl Imap {
             if interrupted {
                 info!(
                     context,
-                    "Transport {}: Connecting to IMAP without waiting for ratelimit due to interrupt.",
-                    self.transport_id
+                    "Connecting to IMAP without waiting for ratelimit due to interrupt."
                 );
             }
         }
 
-        info!(
-            context,
-            "Transport {}: Connecting to IMAP server.", self.transport_id
-        );
+        info!(context, "Connecting to IMAP server.");
         self.connectivity.set_connecting(context);
 
         self.conn_last_try = tools::Time::now();
@@ -360,10 +355,7 @@ impl Imap {
         let login_params = prioritize_server_login_params(&context.sql, &self.lp, "imap").await?;
         let mut first_error = None;
         for lp in login_params {
-            info!(
-                context,
-                "Transport {}: IMAP trying to connect to {}.", self.transport_id, &lp.connection
-            );
+            info!(context, "IMAP trying to connect to {}.", &lp.connection);
             let connection_candidate = lp.connection.clone();
             let client = match Client::connect(
                 context,
@@ -411,10 +403,7 @@ impl Imap {
                     let resync_request_sender = self.resync_request_sender.clone();
 
                     let session = if capabilities.can_compress {
-                        info!(
-                            context,
-                            "Transport {}: Enabling IMAP compression.", self.transport_id
-                        );
+                        info!(context, "Enabling IMAP compression.");
                         let compressed_session = session
                             .compress(|s| {
                                 let session_stream: Box<dyn SessionStream> = Box::new(s);
@@ -447,10 +436,7 @@ impl Imap {
                         lp.user
                     )));
                     self.connectivity.set_preparing(context);
-                    info!(
-                        context,
-                        "Transport {}: Successfully logged into IMAP server.", self.transport_id
-                    );
+                    info!(context, "Successfully logged into IMAP server.");
                     return Ok(session);
                 }
 
@@ -458,10 +444,7 @@ impl Imap {
                     let imap_user = lp.user.to_owned();
                     let message = stock_str::cannot_login(context, &imap_user);
 
-                    warn!(
-                        context,
-                        "Transport {}: IMAP failed to login: {err:#}.", self.transport_id
-                    );
+                    warn!(context, "IMAP failed to login: {err:#}.");
                     first_error.get_or_insert(format_err!("{message} ({err:#})"));
 
                     // If it looks like the password is wrong, send a notification:
@@ -480,11 +463,7 @@ impl Imap {
                             )
                             .await
                             {
-                                warn!(
-                                    context,
-                                    "Transport {}: Failed to add device message: {e:#}.",
-                                    self.transport_id
-                                );
+                                warn!(context, "Failed to add device message: {e:#}.");
                             } else {
                                 context
                                     .set_config_internal(Config::NotifyAboutWrongPw, None)
@@ -546,21 +525,10 @@ impl Imap {
             bail!("IMAP operation attempted while it is torn down");
         }
 
-        let transport_id = session.transport_id();
-        info!(
-            context,
-            "Transport {transport_id}: fetch_move_delete start."
-        );
-
         let msgs_fetched = self
             .fetch_new_messages(context, session, watch_folder, folder_meaning)
             .await
             .context("fetch_new_messages")?;
-
-        info!(
-            context,
-            "Transport {transport_id}: fetch_move_delete finished fetch_new_messages."
-        );
         if msgs_fetched && context.get_config_delete_device_after().await?.is_some() {
             // New messages were fetched and shall be deleted later, restart ephemeral loop.
             // Note that the `Config::DeleteDeviceAfter` timer starts as soon as the messages are
@@ -599,18 +567,10 @@ impl Imap {
             return Ok(false);
         }
 
-        info!(
-            context,
-            "Transport {transport_id}: fetch_new_messages selects folder {folder:?}."
-        );
         let folder_exists = session
             .select_with_uidvalidity(context, folder)
             .await
             .with_context(|| format!("Failed to select folder {folder:?}"))?;
-        info!(
-            context,
-            "Transport {transport_id}: fetch_new_messages selected folder {folder:?}."
-        );
 
         if !session.new_mail {
             info!(
@@ -1144,7 +1104,6 @@ impl Session {
         }
 
         let transport_id = self.transport_id();
-        info!(context, "Transport {transport_id}: Storing seen flags.");
         let rows = context
             .sql
             .query_map_vec(
@@ -1179,15 +1138,13 @@ impl Session {
             } else if let Err(err) = self.add_flag_finalized_with_set(&uid_set, "\\Seen").await {
                 warn!(
                     context,
-                    "Transport {transport_id}: Cannot mark messages {uid_set} in {folder} as seen, will retry later: {err:#}."
+                    "Cannot mark messages {uid_set} in {folder} as seen, will retry later: {err:#}."
                 );
                 continue;
             } else {
                 info!(
                     context,
-                    "Transport {transport_id}: Marked messages {} in folder {} as seen.",
-                    uid_set,
-                    folder
+                    "Marked messages {} in folder {} as seen.", uid_set, folder
                 );
             }
             context
@@ -1202,10 +1159,6 @@ impl Session {
                 .await
                 .context("Cannot remove messages marked as seen from imap_markseen table")?;
         }
-        info!(
-            context,
-            "Transport {transport_id}: Finished storing seen flags."
-        );
 
         Ok(())
     }
@@ -1546,10 +1499,9 @@ impl Session {
             return Ok(());
         }
 
-        let transport_id = self.transport_id();
         info!(
             context,
-            "Transport {transport_id}: Server supports metadata, retrieving server comment and admin contact."
+            "Server supports metadata, retrieving server comment and admin contact."
         );
 
         let mut comment = None;
@@ -1582,8 +1534,7 @@ impl Session {
                         } else {
                             warn!(
                                 context,
-                                "Transport {transport_id}: Got invalid URL from iroh relay metadata: {:?}.",
-                                value
+                                "Got invalid URL from iroh relay metadata: {:?}.", value
                             );
                         }
                     }
@@ -1612,7 +1563,6 @@ impl Session {
             create_fallback_ice_servers()
         };
 
-        info!(context, "Transport {transport_id}: Got IMAP metadata.");
         *lock = Some(ServerMetadata {
             comment,
             admin,

src/message.rs

@@ -1800,7 +1800,9 @@ pub async fn delete_msgs_ex(
     Ok(())
 }
 
-/// Marks requested messages as seen.
+/// Marks requested messages and reactions to them as seen.
+/// This should be called when a message comes into view or when a reaction for a message being in
+/// view arrives.
 pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()> {
     if msg_ids.is_empty() {
         return Ok(());
@@ -1814,10 +1816,18 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
 
     let mut msgs = Vec::with_capacity(msg_ids.len());
     for &id in &msg_ids {
-        if let Some(msg) = context
+        let Some(rfc724_mid): Option<String> = context
             .sql
-            .query_row_optional(
+            .query_get_value("SELECT rfc724_mid FROM msgs WHERE id=?", (id,))
+            .await?
+        else {
+            continue;
+        };
+        context
+            .sql
+            .query_map(
                 "SELECT
+                    m.id AS id,
                     m.chat_id AS chat_id,
                     m.state AS state,
                     m.ephemeral_timer AS ephemeral_timer,
@@ -1828,9 +1838,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
                     c.archived AS archived,
                     c.blocked AS blocked
                  FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id
-                 WHERE m.id=? AND m.chat_id>9",
-                (id,),
+                WHERE (m.id=? OR m.mime_in_reply_to=? AND m.hidden=1)
+                    AND m.chat_id>9 AND ?<=m.state AND m.state<?",
+                (id, rfc724_mid, MessageState::InFresh, MessageState::InSeen),
                 |row| {
+                    let id: MsgId = row.get("id")?;
                     let chat_id: ChatId = row.get("chat_id")?;
                     let state: MessageState = row.get("state")?;
                     let param: Params = row.get::<_, String>("param")?.parse().unwrap_or_default();
@@ -1855,11 +1867,14 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
                         ephemeral_timer,
                     ))
                 },
+                |rows| {
+                    for row in rows {
+                        msgs.push(row?);
+                    }
+                    Ok(())
+                },
             )
-            .await?
-        {
-            msgs.push(msg);
-        }
+            .await?;
     }
 
     if msgs
@@ -1888,60 +1903,57 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
         _curr_ephemeral_timer,
     ) in msgs
     {
-        if curr_state == MessageState::InFresh || curr_state == MessageState::InNoticed {
-            update_msg_state(context, id, MessageState::InSeen).await?;
-            info!(context, "Seen message {}.", id);
+        update_msg_state(context, id, MessageState::InSeen).await?;
+        info!(context, "Seen message {}.", id);
 
-            markseen_on_imap_table(context, &curr_rfc724_mid).await?;
+        markseen_on_imap_table(context, &curr_rfc724_mid).await?;
 
-            // Read receipts for system messages are never sent to contacts.
-            // These messages have no place to display received read receipt
-            // anyway. And since their text is locally generated,
-            // quoting them is dangerous as it may contain contact names. E.g., for original message
-            // "Group left by me", a read receipt will quote "Group left by <name>", and the name can
-            // be a display name stored in address book rather than the name sent in the From field by
-            // the user.
-            //
-            // We also don't send read receipts for contact requests.
-            // Read receipts will not be sent even after accepting the chat.
-            let to_id = if curr_blocked == Blocked::Not
-                && !curr_hidden
-                && curr_param.get_bool(Param::WantsMdn).unwrap_or_default()
-                && curr_param.get_cmd() == SystemMessage::Unknown
-                && context.should_send_mdns().await?
-            {
-                // Clear WantsMdn to not handle a MDN twice
-                // if the state later is InFresh again as markfresh_chat() was called.
-                // BccSelf MDN messages in the next branch may be sent twice for syncing.
-                context
-                    .sql
-                    .execute(
-                        "UPDATE msgs SET param=? WHERE id=?",
-                        (curr_param.clone().remove(Param::WantsMdn).to_string(), id),
-                    )
-                    .await
-                    .context("failed to clear WantsMdn")?;
-                Some(curr_from_id)
-            } else if context.get_config_bool(Config::BccSelf).await? {
-                Some(ContactId::SELF)
-            } else {
-                None
-            };
-            if let Some(to_id) = to_id {
-                context
-                    .sql
-                    .execute(
-                        "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)",
-                        (id, to_id, curr_rfc724_mid),
-                    )
-                    .await
-                    .context("failed to insert into smtp_mdns")?;
-                context.scheduler.interrupt_smtp().await;
-            }
+        // Read receipts for system messages are never sent to contacts. These messages have no
+        // place to display received read receipt anyway. And since their text is locally generated,
+        // quoting them is dangerous as it may contain contact names. E.g., for original message
+        // "Group left by me", a read receipt will quote "Group left by <name>", and the name can be
+        // a display name stored in address book rather than the name sent in the From field by the
+        // user.
+        //
+        // We also don't send read receipts for contact requests. Read receipts will not be sent
+        // even after accepting the chat.
+        let to_id = if curr_blocked == Blocked::Not
+            && !curr_hidden
+            && curr_param.get_bool(Param::WantsMdn).unwrap_or_default()
+            && curr_param.get_cmd() == SystemMessage::Unknown
+            && context.should_send_mdns().await?
+        {
+            // Clear WantsMdn to not handle a MDN twice
+            // if the state later is InFresh again as markfresh_chat() was called.
+            // BccSelf MDN messages in the next branch may be sent twice for syncing.
+            context
+                .sql
+                .execute(
+                    "UPDATE msgs SET param=? WHERE id=?",
+                    (curr_param.clone().remove(Param::WantsMdn).to_string(), id),
+                )
+                .await
+                .context("failed to clear WantsMdn")?;
+            Some(curr_from_id)
+        } else if context.get_config_bool(Config::BccSelf).await? {
+            Some(ContactId::SELF)
+        } else {
+            None
+        };
+        if let Some(to_id) = to_id {
+            context
+                .sql
+                .execute(
+                    "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)",
+                    (id, to_id, curr_rfc724_mid),
+                )
+                .await
+                .context("failed to insert into smtp_mdns")?;
+            context.scheduler.interrupt_smtp().await;
+        }
 
-            if !curr_hidden {
-                updated_chat_ids.insert(curr_chat_id);
-            }
+        if !curr_hidden {
+            updated_chat_ids.insert(curr_chat_id);
         }
         archived_chats_maybe_noticed |= curr_state == MessageState::InFresh
             && !curr_hidden

src/mimefactory.rs

@@ -852,7 +852,13 @@ impl MimeFactory {
 
         let rfc724_mid = match &self.loaded {
             Loaded::Message { msg, .. } => match &self.pre_message_mode {
-                PreMessageMode::Pre { .. } => create_outgoing_rfc724_mid(),
+                PreMessageMode::Pre { .. } => {
+                    if msg.pre_rfc724_mid.is_empty() {
+                        create_outgoing_rfc724_mid()
+                    } else {
+                        msg.pre_rfc724_mid.clone()
+                    }
+                }
                 _ => msg.rfc724_mid.clone(),
             },
             Loaded::Mdn { .. } => create_outgoing_rfc724_mid(),

src/reaction.rs

@@ -1105,4 +1105,37 @@ Content-Transfer-Encoding: 7bit\r
 
         Ok(())
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_markseen_referenced_msg() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = &tcm.alice().await;
+        let bob = &tcm.bob().await;
+        let chat_id = alice.create_chat(bob).await.id;
+
+        let alice_msg_id = send_text_msg(alice, chat_id, "foo".to_string()).await?;
+        let sent_msg = alice.pop_sent_msg().await;
+        let bob_msg = bob.recv_msg(&sent_msg).await;
+        bob_msg.chat_id.accept(bob).await?;
+
+        send_reaction(bob, bob_msg.id, "👀").await?;
+        let sent_msg = bob.pop_sent_msg().await;
+        let alice_reaction = alice.recv_msg_hidden(&sent_msg).await;
+        assert_eq!(alice_reaction.state, MessageState::InFresh);
+
+        markseen_msgs(alice, vec![alice_msg_id]).await?;
+        let alice_reaction = Message::load_from_db(alice, alice_reaction.id).await?;
+        assert_eq!(alice_reaction.state, MessageState::InSeen);
+        assert_eq!(
+            alice
+                .sql
+                .count(
+                    "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?",
+                    (ContactId::SELF,)
+                )
+                .await?,
+            1
+        );
+        Ok(())
+    }
 }

src/receive_imf.rs

@@ -35,7 +35,8 @@ use crate::message::{
     rfc724_mid_exists,
 };
 use crate::mimeparser::{
-    AvatarAction, GossipedKey, MimeMessage, PreMessageMode, SystemMessage, parse_message_ids,
+    AvatarAction, GossipedKey, MimeMessage, PreMessageMode, SystemMessage, parse_message_id,
+    parse_message_ids,
 };
 use crate::param::{Param, Params};
 use crate::peer_channels::{add_gossip_peer_from_header, insert_topic_stub, iroh_topic_from_str};
@@ -1018,10 +1019,14 @@ UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1
                 context
                     .sql
                     .execute(
+                        // Employ `msgs_index7 ON msgs (state, hidden, chat_id)` by explicit
+                        // enumeration of `hidden` values, otherwise SQLite would iterate over all
+                        // messages having `state` specified before, i.e. it would use the index
+                        // only partly.
                         "
 UPDATE msgs SET state=? WHERE
     state=? AND
-    hidden=0 AND
+    hidden IN (0,1) AND
     chat_id=? AND
     timestamp<?",
                         (
@@ -1033,7 +1038,18 @@ UPDATE msgs SET state=? WHERE
                     )
                     .await
                     .context("UPDATE msgs.state")?;
-                if chat_id.get_fresh_msg_cnt(context).await? == 0 {
+                let n_fresh_msgs = context
+                    .sql
+                    .count(
+                        "
+SELECT COUNT(*) FROM msgs WHERE
+    state=? AND
+    (hidden=0 OR hidden=1) AND
+    chat_id=?",
+                        (MessageState::InFresh, chat_id),
+                    )
+                    .await?;
+                if n_fresh_msgs == 0 {
                     // Removes all notifications for the chat in UIs.
                     context.emit_event(EventType::MsgsNoticed(chat_id));
                 } else {
@@ -2011,9 +2027,13 @@ async fn add_parts(
     )
     .await?;
 
-    let mime_in_reply_to = mime_parser
-        .get_header(HeaderDef::InReplyTo)
-        .unwrap_or_default();
+    let mime_in_reply_to = match mime_parser.get_header(HeaderDef::InReplyTo) {
+        Some(in_reply_to) => parse_message_id(in_reply_to)
+            .log_err(context)
+            .ok()
+            .unwrap_or_default(),
+        None => "".to_string(),
+    };
     let mime_references = mime_parser
         .get_header(HeaderDef::References)
         .unwrap_or_default();
@@ -2140,7 +2160,7 @@ async fn add_parts(
             let is_incoming_fresh = mime_parser.incoming && !seen;
             set_msg_reaction(
                 context,
-                mime_in_reply_to,
+                &mime_in_reply_to,
                 chat_id,
                 from_id,
                 sort_timestamp,
@@ -2282,7 +2302,7 @@ RETURNING id
                     } else {
                         Vec::new()
                     },
-                    if trash { "" } else { mime_in_reply_to },
+                    if trash { "" } else { &mime_in_reply_to },
                     if trash { "" } else { mime_references },
                     !trash && save_mime_modified,
                     if trash { "" } else { part.error.as_deref().unwrap_or_default() },

src/sql/migrations.rs

@@ -2363,6 +2363,15 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT;
         .await?;
     }
 
+    inc_and_check(&mut migration_version, 150)?;
+    if dbversion < migration_version {
+        sql.execute_migration(
+            "CREATE INDEX msgs_index10 ON msgs (mime_in_reply_to)",
+            migration_version,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?

src/stats.rs

@@ -554,7 +554,6 @@ async fn get_message_stats(context: &Context) -> Result<BTreeMap<Chattype, Messa
 }
 
 pub(crate) async fn update_message_stats(context: &Context) -> Result<()> {
-    info!(context, "Updating message statistics.");
     for chattype in [Chattype::Single, Chattype::Group, Chattype::OutBroadcast] {
         update_message_stats_inner(context, chattype).await?;
     }

src/test_utils.rs

@@ -715,7 +715,8 @@ impl TestContext {
     }
 
     pub async fn get_smtp_rows_for_msg<'a>(&'a self, msg_id: MsgId) -> Vec<SentMessage<'a>> {
-        self.ctx
+        let sent_msgs = self
+            .ctx
             .sql
             .query_map_vec(
                 "SELECT id, msg_id, mime, recipients FROM smtp WHERE msg_id=?",
@@ -737,7 +738,23 @@ impl TestContext {
                 sender_context: &self.ctx,
                 recipients,
             })
-            .collect()
+            .collect();
+        self.ctx
+            .sql
+            .execute("DELETE FROM smtp WHERE msg_id=?", (msg_id,))
+            .await
+            .expect("Delete smtp jobs");
+        update_msg_state(&self.ctx, msg_id, MessageState::OutDelivered)
+            .await
+            .expect("Update message state");
+        self.sql
+            .execute(
+                "UPDATE msgs SET timestamp_sent=? WHERE id=?",
+                (time(), msg_id),
+            )
+            .await
+            .expect("Update timestamp_sent");
+        sent_msgs
     }
 
     /// Parses a message.

src/tests/pre_messages/sending.rs

@@ -56,23 +56,27 @@ async fn test_sending_pre_message() -> Result<()> {
             .is_some()
     );
 
+    let post_rfc724_mid = post_message_parsed
+        .headers
+        .get_header_value(HeaderDef::MessageId);
     assert_eq!(
-        post_message_parsed
-            .headers
-            .get_header_value(HeaderDef::MessageId),
+        post_rfc724_mid,
         Some(format!("<{}>", msg.rfc724_mid)),
         "Post-Message should have the rfc message id of the database message"
     );
 
+    let pre_rfc724_mid = pre_message_parsed
+        .headers
+        .get_header_value(HeaderDef::MessageId);
     assert_ne!(
-        pre_message_parsed
-            .headers
-            .get_header_value(HeaderDef::MessageId),
-        post_message_parsed
-            .headers
-            .get_header_value(HeaderDef::MessageId),
+        pre_rfc724_mid, post_rfc724_mid,
         "message ids of Pre-Message and Post-Message should be different"
     );
+    assert_eq!(
+        pre_rfc724_mid,
+        Some(format!("<{}>", msg.pre_rfc724_mid)),
+        "Unexpected pre-message RFC 724 ID"
+    );
 
     let decrypted_post_message = bob.parse_msg(post_message).await;
     assert_eq!(decrypted_post_message.decrypting_failed, false);
@@ -86,9 +90,7 @@ async fn test_sending_pre_message() -> Result<()> {
         decrypted_pre_message
             .get_header(HeaderDef::ChatPostMessageId)
             .map(String::from),
-        post_message_parsed
-            .headers
-            .get_header_value(HeaderDef::MessageId)
+        post_rfc724_mid,
     );
     assert!(
         pre_message_parsed
@@ -98,6 +100,25 @@ async fn test_sending_pre_message() -> Result<()> {
         "no Chat-Post-Message-ID header in unprotected headers of Pre-Message"
     );
 
+    chat::resend_msgs(alice, &[msg_id]).await?;
+    let smtp_rows = alice.get_smtp_rows_for_msg(msg_id).await;
+    assert_eq!(smtp_rows.len(), 2);
+
+    let pre_message_parsed = mailparse::parse_mail(smtp_rows[0].payload.as_bytes())?;
+    assert_eq!(
+        pre_message_parsed
+            .headers
+            .get_header_value(HeaderDef::MessageId),
+        pre_rfc724_mid
+    );
+    let post_message_parsed = mailparse::parse_mail(smtp_rows[1].payload.as_bytes())?;
+    assert_eq!(
+        post_message_parsed
+            .headers
+            .get_header_value(HeaderDef::MessageId),
+        post_rfc724_mid
+    );
+
     Ok(())
 }
 

src/tools.rs

@@ -21,7 +21,6 @@ pub use std::time::SystemTime as Time;
 #[cfg(not(test))]
 pub use std::time::SystemTime;
 
-use crate::log::LogExt as _;
 use anyhow::{Context as _, Result, bail, ensure};
 use base64::Engine as _;
 use chrono::{Local, NaiveDateTime, NaiveTime, TimeZone};
@@ -249,8 +248,6 @@ async fn maybe_warn_on_bad_time(context: &Context, now: i64, known_past_timestam
                 true,
             )
             .await
-            .context("Failed to add bad time warning")
-            .log_err(context)
             .ok();
         } else {
             warn!(context, "Can't convert current timestamp");