Skip to content

Commit d2f33f5

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 8c006ca commit d2f33f5

6 files changed

Lines changed: 96 additions & 6 deletions

File tree

src/config.rs

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

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

src/context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,6 +1084,13 @@ impl Context {
10841084
.await?
10851085
.to_string(),
10861086
);
1087+
res.insert(
1088+
"pop_sent_msg_from_head",
1089+
self.sql
1090+
.get_raw_config("pop_sent_msg_from_head")
1091+
.await?
1092+
.unwrap_or_default(),
1093+
);
10871094
res.insert(
10881095
"test_hooks",
10891096
self.sql

src/message.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1904,9 +1904,10 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
19041904
//
19051905
// We also don't send read receipts for contact requests.
19061906
// Read receipts will not be sent even after accepting the chat.
1907+
let wants_mdn = curr_param.get_bool(Param::WantsMdn).unwrap_or_default();
19071908
let to_id = if curr_blocked == Blocked::Not
19081909
&& !curr_hidden
1909-
&& curr_param.get_bool(Param::WantsMdn).unwrap_or_default()
1910+
&& wants_mdn
19101911
&& curr_param.get_cmd() == SystemMessage::Unknown
19111912
&& context.should_send_mdns().await?
19121913
{
@@ -1928,6 +1929,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
19281929
None
19291930
};
19301931
if let Some(to_id) = to_id {
1932+
info!(
1933+
context,
1934+
"Queuing MDN to {to_id} for {id} from {curr_from_id}, wants_mdn={wants_mdn}, cmd={}.",
1935+
curr_param.get_cmd()
1936+
);
19311937
context
19321938
.sql
19331939
.execute(

src/mimeparser.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,6 +2501,8 @@ async fn handle_mdn(
25012501
let Some((msg_id, chat_id, has_mdns, is_dup)) = context
25022502
.sql
25032503
.query_row_optional(
2504+
// MDN on a pre-message references the post-message, see `receive_imf`. So we can't tell
2505+
// which one was seen, but this is on purpose.
25042506
"SELECT
25052507
m.id AS msg_id,
25062508
c.id AS chat_id,

src/test_utils.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -624,16 +624,25 @@ impl TestContext {
624624
}
625625

626626
pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option<SentMessage<'_>> {
627+
let from_head = self
628+
.get_config_bool(Config::PopSentMsgFromHead)
629+
.await
630+
.unwrap();
631+
let order_subst = match from_head {
632+
true => "",
633+
false => " DESC",
634+
};
627635
let start = Instant::now();
628636
let (rowid, msg_id, payload, recipients) = loop {
629637
let row = self
630638
.ctx
631639
.sql
632640
.query_row_optional(
633-
r#"
634-
SELECT id, msg_id, mime, recipients
635-
FROM smtp
636-
ORDER BY id DESC"#,
641+
&format!(
642+
"SELECT id, msg_id, mime, recipients
643+
FROM smtp
644+
ORDER BY id{order_subst}"
645+
),
637646
(),
638647
|row| {
639648
let rowid: i64 = row.get(0)?;

src/tests/pre_messages/receiving.rs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use pretty_assertions::assert_eq;
44

55
use crate::EventType;
66
use crate::chat;
7+
use crate::config::Config;
78
use crate::contact;
89
use crate::download::{DownloadState, PRE_MSG_ATTACHMENT_SIZE_THRESHOLD, PostMsgMetadata};
910
use crate::message::{Message, MessageState, Viewtype, delete_msgs, markseen_msgs};
@@ -253,6 +254,64 @@ async fn test_lost_pre_msg() -> Result<()> {
253254
Ok(())
254255
}
255256

257+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
258+
async fn test_pre_msg_mdn_before_sending_full() -> Result<()> {
259+
pre_msg_mdn_before_sending_full("").await
260+
}
261+
262+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
263+
async fn test_pre_msg_mdn_before_sending_full_with_text() -> Result<()> {
264+
pre_msg_mdn_before_sending_full("text").await
265+
}
266+
267+
async fn pre_msg_mdn_before_sending_full(text: &str) -> Result<()> {
268+
let mut tcm = TestContextManager::new();
269+
let alice = &tcm.alice().await;
270+
alice
271+
.set_config_bool(Config::PopSentMsgFromHead, true)
272+
.await?;
273+
let bob = &tcm.bob().await;
274+
let alice_chat_id = alice.create_group_with_members("", &[bob]).await;
275+
276+
let file_bytes = include_bytes!("../../../test-data/image/screenshot.gif");
277+
let mut msg = Message::new(Viewtype::Image);
278+
msg.set_file_from_bytes(alice, "a.jpg", file_bytes, None)?;
279+
msg.set_text(text.to_string());
280+
let pre_msg = alice.send_msg(alice_chat_id, &mut msg).await;
281+
let alice_msg_id = msg.id;
282+
283+
let msg = bob.recv_msg(&pre_msg).await;
284+
assert_eq!(msg.download_state, DownloadState::Available);
285+
assert_eq!(msg.id.get_state(bob).await?, MessageState::InFresh);
286+
assert_eq!(msg.text, text);
287+
assert!(msg.param.get_bool(Param::WantsMdn).unwrap_or_default());
288+
msg.chat_id.accept(bob).await?;
289+
markseen_msgs(bob, vec![msg.id]).await?;
290+
assert_eq!(msg.id.get_state(bob).await?, MessageState::InSeen);
291+
assert_eq!(
292+
bob.sql
293+
.count(
294+
"SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?",
295+
(msg.from_id,)
296+
)
297+
.await?,
298+
1
299+
);
300+
smtp::queue_mdn(bob).await?;
301+
alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
302+
assert_eq!(
303+
alice_msg_id.get_state(alice).await?,
304+
MessageState::OutPending
305+
);
306+
307+
let _full_msg = alice.pop_sent_msg().await;
308+
assert_eq!(
309+
alice_msg_id.get_state(alice).await?,
310+
MessageState::OutMdnRcvd
311+
);
312+
Ok(())
313+
}
314+
256315
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
257316
async fn test_post_msg_bad_sender() -> Result<()> {
258317
let mut tcm = TestContextManager::new();
@@ -539,7 +598,10 @@ async fn test_markseen_pre_msg() -> Result<()> {
539598
assert_eq!(
540599
alice
541600
.sql
542-
.count("SELECT COUNT(*) FROM smtp_mdns", ())
601+
.count(
602+
"SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?",
603+
(msg.from_id,)
604+
)
543605
.await?,
544606
1
545607
);

0 commit comments

Comments
 (0)