From d2f33f57e29a06b67ca4ef2041c14787e838ed7f Mon Sep 17 00:00:00 2001 From: iequidoo Date: Wed, 18 Mar 2026 10:46:46 -0300 Subject: [PATCH] test: MDN on pre-message has effect if received before sending full message (#8004) Actually, the problem in #8004 is that a pre-message doesn't "want MDN" if it has no text. Anyway, the added test reproduces the bug. --- src/config.rs | 4 ++ src/context.rs | 7 ++++ src/message.rs | 8 +++- src/mimeparser.rs | 2 + src/test_utils.rs | 17 ++++++-- src/tests/pre_messages/receiving.rs | 64 ++++++++++++++++++++++++++++- 6 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/config.rs b/src/config.rs index 16fac5de7..0bdc75547 100644 --- a/src/config.rs +++ b/src/config.rs @@ -427,6 +427,10 @@ pub enum Config { /// storing the same token multiple times on the server. EncryptedDeviceToken, + /// Make `TestContext::pop_sent_msg_opt()` and related functions pop messages from the `smtp` + /// head, i.e. make it a queue. For historical reasons the default is a stack. + PopSentMsgFromHead, + /// Enables running test hooks, e.g. see `InnerContext::pre_encrypt_mime_hook`. /// This way is better than conditional compilation, i.e. `#[cfg(test)]`, because tests not /// using this still run unmodified code. diff --git a/src/context.rs b/src/context.rs index adcb3b2a7..d78fc6f61 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1084,6 +1084,13 @@ impl Context { .await? .to_string(), ); + res.insert( + "pop_sent_msg_from_head", + self.sql + .get_raw_config("pop_sent_msg_from_head") + .await? + .unwrap_or_default(), + ); res.insert( "test_hooks", self.sql diff --git a/src/message.rs b/src/message.rs index 7769bd924..cf440bfdb 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1904,9 +1904,10 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> // // We also don't send read receipts for contact requests. // Read receipts will not be sent even after accepting the chat. + let wants_mdn = curr_param.get_bool(Param::WantsMdn).unwrap_or_default(); let to_id = if curr_blocked == Blocked::Not && !curr_hidden - && curr_param.get_bool(Param::WantsMdn).unwrap_or_default() + && wants_mdn && curr_param.get_cmd() == SystemMessage::Unknown && context.should_send_mdns().await? { @@ -1928,6 +1929,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> None }; if let Some(to_id) = to_id { + info!( + context, + "Queuing MDN to {to_id} for {id} from {curr_from_id}, wants_mdn={wants_mdn}, cmd={}.", + curr_param.get_cmd() + ); context .sql .execute( diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 942facaa4..58e459eb3 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -2501,6 +2501,8 @@ async fn handle_mdn( let Some((msg_id, chat_id, has_mdns, is_dup)) = context .sql .query_row_optional( + // MDN on a pre-message references the post-message, see `receive_imf`. So we can't tell + // which one was seen, but this is on purpose. "SELECT m.id AS msg_id, c.id AS chat_id, diff --git a/src/test_utils.rs b/src/test_utils.rs index 0435f330e..ed5cf8219 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -624,16 +624,25 @@ impl TestContext { } pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option> { + let from_head = self + .get_config_bool(Config::PopSentMsgFromHead) + .await + .unwrap(); + let order_subst = match from_head { + true => "", + false => " DESC", + }; let start = Instant::now(); let (rowid, msg_id, payload, recipients) = loop { let row = self .ctx .sql .query_row_optional( - r#" - SELECT id, msg_id, mime, recipients - FROM smtp - ORDER BY id DESC"#, + &format!( + "SELECT id, msg_id, mime, recipients +FROM smtp +ORDER BY id{order_subst}" + ), (), |row| { let rowid: i64 = row.get(0)?; diff --git a/src/tests/pre_messages/receiving.rs b/src/tests/pre_messages/receiving.rs index ec0aa9d64..b47f19346 100644 --- a/src/tests/pre_messages/receiving.rs +++ b/src/tests/pre_messages/receiving.rs @@ -4,6 +4,7 @@ use pretty_assertions::assert_eq; use crate::EventType; use crate::chat; +use crate::config::Config; use crate::contact; use crate::download::{DownloadState, PRE_MSG_ATTACHMENT_SIZE_THRESHOLD, PostMsgMetadata}; use crate::message::{Message, MessageState, Viewtype, delete_msgs, markseen_msgs}; @@ -253,6 +254,64 @@ async fn test_lost_pre_msg() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_pre_msg_mdn_before_sending_full() -> Result<()> { + pre_msg_mdn_before_sending_full("").await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_pre_msg_mdn_before_sending_full_with_text() -> Result<()> { + pre_msg_mdn_before_sending_full("text").await +} + +async fn pre_msg_mdn_before_sending_full(text: &str) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + alice + .set_config_bool(Config::PopSentMsgFromHead, true) + .await?; + let bob = &tcm.bob().await; + let alice_chat_id = alice.create_group_with_members("", &[bob]).await; + + let file_bytes = include_bytes!("../../../test-data/image/screenshot.gif"); + let mut msg = Message::new(Viewtype::Image); + msg.set_file_from_bytes(alice, "a.jpg", file_bytes, None)?; + msg.set_text(text.to_string()); + let pre_msg = alice.send_msg(alice_chat_id, &mut msg).await; + let alice_msg_id = msg.id; + + let msg = bob.recv_msg(&pre_msg).await; + assert_eq!(msg.download_state, DownloadState::Available); + assert_eq!(msg.id.get_state(bob).await?, MessageState::InFresh); + assert_eq!(msg.text, text); + assert!(msg.param.get_bool(Param::WantsMdn).unwrap_or_default()); + msg.chat_id.accept(bob).await?; + markseen_msgs(bob, vec![msg.id]).await?; + assert_eq!(msg.id.get_state(bob).await?, MessageState::InSeen); + assert_eq!( + bob.sql + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (msg.from_id,) + ) + .await?, + 1 + ); + smtp::queue_mdn(bob).await?; + alice.recv_msg_trash(&bob.pop_sent_msg().await).await; + assert_eq!( + alice_msg_id.get_state(alice).await?, + MessageState::OutPending + ); + + let _full_msg = alice.pop_sent_msg().await; + assert_eq!( + alice_msg_id.get_state(alice).await?, + MessageState::OutMdnRcvd + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_post_msg_bad_sender() -> Result<()> { let mut tcm = TestContextManager::new(); @@ -539,7 +598,10 @@ async fn test_markseen_pre_msg() -> Result<()> { assert_eq!( alice .sql - .count("SELECT COUNT(*) FROM smtp_mdns", ()) + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (msg.from_id,) + ) .await?, 1 );