Skip to content

Commit 1c17ade

Browse files
committed
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.
1 parent e4ae501 commit 1c17ade

6 files changed

Lines changed: 88 additions & 6 deletions

File tree

src/config.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,10 @@ pub enum Config {
466466
/// storing the same token multiple times on the server.
467467
EncryptedDeviceToken,
468468

469+
/// Make `TestContext::pop_sent_msg_opt()` and related functions pop messages from the `smtp`
470+
/// head, i.e. make it a queue. For historical reasons the default is a stack.
471+
PopSentMsgFromHead,
472+
469473
/// Enables running test hooks, e.g. see `InnerContext::pre_encrypt_mime_hook`.
470474
/// This way is better than conditional compilation, i.e. `#[cfg(test)]`, because tests not
471475
/// using this still run unmodified code.

src/context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,13 @@ impl Context {
10441044
.await?
10451045
.to_string(),
10461046
);
1047+
res.insert(
1048+
"pop_sent_msg_from_head",
1049+
self.sql
1050+
.get_raw_config("pop_sent_msg_from_head")
1051+
.await?
1052+
.unwrap_or_default(),
1053+
);
10471054
res.insert(
10481055
"test_hooks",
10491056
self.sql

src/message.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1901,9 +1901,10 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
19011901
//
19021902
// We also don't send read receipts for contact requests.
19031903
// Read receipts will not be sent even after accepting the chat.
1904+
let wants_mdn = curr_param.get_bool(Param::WantsMdn).unwrap_or_default();
19041905
let to_id = if curr_blocked == Blocked::Not
19051906
&& !curr_hidden
1906-
&& curr_param.get_bool(Param::WantsMdn).unwrap_or_default()
1907+
&& wants_mdn
19071908
&& curr_param.get_cmd() == SystemMessage::Unknown
19081909
&& context.should_send_mdns().await?
19091910
{
@@ -1925,6 +1926,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
19251926
None
19261927
};
19271928
if let Some(to_id) = to_id {
1929+
info!(
1930+
context,
1931+
"Queuing MDN to {to_id} for {id} from {curr_from_id}, wants_mdn={wants_mdn}, cmd={}.",
1932+
curr_param.get_cmd()
1933+
);
19281934
context
19291935
.sql
19301936
.execute(

src/mimeparser.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2492,6 +2492,8 @@ async fn handle_mdn(
24922492
let Some((msg_id, chat_id, has_mdns, is_dup)) = context
24932493
.sql
24942494
.query_row_optional(
2495+
// MDN on a pre-message references the post-message, see `receive_imf`. So we can't tell
2496+
// which one was seen, but this is on purpose.
24952497
"SELECT
24962498
m.id AS msg_id,
24972499
c.id AS chat_id,

src/test_utils.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,10 @@ impl TestContext {
624624
}
625625

626626
pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option<SentMessage<'_>> {
627-
let rev_order = true;
627+
let rev_order = !self
628+
.get_config_bool(Config::PopSentMsgFromHead)
629+
.await
630+
.unwrap();
628631
self.pop_sent_msg_ex(rev_order, timeout).await
629632
}
630633

src/tests/pre_messages/receiving.rs

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use pretty_assertions::assert_eq;
66

77
use crate::EventType;
88
use crate::chat;
9-
use crate::chat::send_msg;
109
use crate::config::Config;
1110
use crate::contact;
1211
use crate::download::{DownloadState, PRE_MSG_ATTACHMENT_SIZE_THRESHOLD, PostMsgMetadata};
@@ -265,6 +264,64 @@ async fn test_lost_pre_msg() -> Result<()> {
265264
Ok(())
266265
}
267266

267+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
268+
async fn test_pre_msg_mdn_before_sending_full() -> Result<()> {
269+
pre_msg_mdn_before_sending_full("").await
270+
}
271+
272+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
273+
async fn test_pre_msg_mdn_before_sending_full_with_text() -> Result<()> {
274+
pre_msg_mdn_before_sending_full("text").await
275+
}
276+
277+
async fn pre_msg_mdn_before_sending_full(text: &str) -> Result<()> {
278+
let mut tcm = TestContextManager::new();
279+
let alice = &tcm.alice().await;
280+
alice
281+
.set_config_bool(Config::PopSentMsgFromHead, true)
282+
.await?;
283+
let bob = &tcm.bob().await;
284+
let alice_chat_id = alice.create_group_with_members("", &[bob]).await;
285+
286+
let file_bytes = include_bytes!("../../../test-data/image/screenshot.gif");
287+
let mut msg = Message::new(Viewtype::Image);
288+
msg.set_file_from_bytes(alice, "a.jpg", file_bytes, None)?;
289+
msg.set_text(text.to_string());
290+
let pre_msg = alice.send_msg(alice_chat_id, &mut msg).await;
291+
let alice_msg_id = msg.id;
292+
293+
let msg = bob.recv_msg(&pre_msg).await;
294+
assert_eq!(msg.download_state, DownloadState::Available);
295+
assert_eq!(msg.id.get_state(bob).await?, MessageState::InFresh);
296+
assert_eq!(msg.text, text);
297+
assert!(msg.param.get_bool(Param::WantsMdn).unwrap_or_default());
298+
msg.chat_id.accept(bob).await?;
299+
markseen_msgs(bob, vec![msg.id]).await?;
300+
assert_eq!(msg.id.get_state(bob).await?, MessageState::InSeen);
301+
assert_eq!(
302+
bob.sql
303+
.count(
304+
"SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?",
305+
(msg.from_id,)
306+
)
307+
.await?,
308+
1
309+
);
310+
smtp::queue_mdn(bob).await?;
311+
alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
312+
assert_eq!(
313+
alice_msg_id.get_state(alice).await?,
314+
MessageState::OutPending
315+
);
316+
317+
let _full_msg = alice.pop_sent_msg().await;
318+
assert_eq!(
319+
alice_msg_id.get_state(alice).await?,
320+
MessageState::OutMdnRcvd
321+
);
322+
Ok(())
323+
}
324+
268325
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
269326
async fn test_post_msg_bad_sender() -> Result<()> {
270327
let mut tcm = TestContextManager::new();
@@ -550,7 +607,7 @@ async fn test_webxdc_updates_in_post_message_after_pre_message() -> Result<()> {
550607
.send_webxdc_status_update(alice_instance.id, r#"{"payload":42, "info":"i"}"#)
551608
.await?;
552609

553-
send_msg(alice, alice_chat_id, &mut alice_instance).await?;
610+
chat::send_msg(alice, alice_chat_id, &mut alice_instance).await?;
554611
let post_message = alice.pop_sent_msg().await;
555612
let pre_message = alice.pop_sent_msg().await;
556613

@@ -591,7 +648,7 @@ async fn test_webxdc_updates_in_post_message_without_pre_message() -> Result<()>
591648
.send_webxdc_status_update(alice_instance.id, r#"{"payload":42, "info":"i"}"#)
592649
.await?;
593650

594-
send_msg(alice, alice_chat_id, &mut alice_instance).await?;
651+
chat::send_msg(alice, alice_chat_id, &mut alice_instance).await?;
595652
let post_message = alice.pop_sent_msg().await;
596653
let pre_message = alice.pop_sent_msg().await;
597654

@@ -644,7 +701,10 @@ async fn test_markseen_pre_msg() -> Result<()> {
644701
assert_eq!(
645702
alice
646703
.sql
647-
.count("SELECT COUNT(*) FROM smtp_mdns", ())
704+
.count(
705+
"SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?",
706+
(msg.from_id,)
707+
)
648708
.await?,
649709
1
650710
);

0 commit comments

Comments
 (0)