diff --git a/src/imap.rs b/src/imap.rs index 29a97b12f..b76af2a75 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -2352,11 +2352,11 @@ pub(crate) async fn prefetch_should_download( ) .await? { - if is_trash { - message::prune_tombstone(context, message_id).await?; + let should = is_trash && !message::prune_tombstone(context, message_id).await?; + if !should { + markseen_on_imap_table(context, message_id).await?; } - markseen_on_imap_table(context, message_id).await?; - return Ok(false); + return Ok(should); } // We do not know the Message-ID or the Message-ID is missing (in this case, we create one in @@ -2429,8 +2429,11 @@ pub(crate) async fn prefetch_should_download( pub(crate) fn is_dup_msg(is_chat_msg: bool, ts_sent: i64, ts_sent_old: i64) -> bool { // If the existing message has timestamp_sent == 0, that means we don't know its actual sent // timestamp, so don't delete the new message. E.g. outgoing messages have zero timestamp_sent - // because they are stored to the db before sending. Also consider as duplicates only messages - // with greater timestamp to avoid deleting both messages in a multi-device setting. + // because they are stored to the db before sending. Trashed messages also have zero + // timestamp_sent and mustn't make new messages "duplicates", otherwise if a webxdc message is + // deleted because of DeleteDeviceAfter set, it won't be recovered from a re-sent message. Also + // consider as duplicates only messages with greater timestamp to avoid deleting both messages + // in a multi-device setting. is_chat_msg && ts_sent_old != 0 && ts_sent > ts_sent_old } diff --git a/src/message.rs b/src/message.rs index 3742a9ed4..5be9eccf3 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1745,8 +1745,9 @@ pub async fn delete_msgs_ex( } /// Removes from the database a locally deleted message that also doesn't have a server UID. -pub(crate) async fn prune_tombstone(context: &Context, rfc724_mid: &str) -> Result<()> { - context +/// Returns whether the removal happened. +pub(crate) async fn prune_tombstone(context: &Context, rfc724_mid: &str) -> Result { + Ok(context .sql .execute( "DELETE FROM msgs @@ -1757,8 +1758,8 @@ pub(crate) async fn prune_tombstone(context: &Context, rfc724_mid: &str) -> Resu )", (rfc724_mid, DC_CHAT_ID_TRASH), ) - .await?; - Ok(()) + .await? + > 0) } /// Marks requested messages as seen. diff --git a/src/receive_imf.rs b/src/receive_imf.rs index e8ab97b9a..65e070541 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -558,10 +558,13 @@ pub(crate) async fn receive_imf_inner( let (replace_msg_id, replace_chat_id); if let Some((old_msg_id, _)) = message::rfc724_mid_exists(context, rfc724_mid).await? { let msg = Message::load_from_db_optional(context, old_msg_id).await?; - if msg.is_none() { - message::prune_tombstone(context, rfc724_mid).await?; + // The tombstone being pruned means that we expected the message to appear on IMAP after + // deletion. NB: Not all such messages have `msgs.deleted=1`, see how external deletion + // requests deal with message reordering. + match msg.is_none() && !message::prune_tombstone(context, rfc724_mid).await? { + true => replace_msg_id = None, + false => replace_msg_id = Some(old_msg_id), } - replace_msg_id = Some(old_msg_id); if let Some(msg) = msg.filter(|msg| msg.download_state() != DownloadState::Done) { // the message was partially downloaded before and is fully downloaded now. info!(context, "Message already partly in DB, replacing."); @@ -571,32 +574,39 @@ pub(crate) async fn receive_imf_inner( // or cannot be loaded because it is deleted. replace_chat_id = None; } - } else { - replace_msg_id = if rfc724_mid_orig == rfc724_mid { - None - } else if let Some((old_msg_id, old_ts_sent)) = - message::rfc724_mid_exists(context, rfc724_mid_orig).await? - { - message::prune_tombstone(context, rfc724_mid_orig).await?; - if imap::is_dup_msg( - mime_parser.has_chat_version(), - mime_parser.timestamp_sent, - old_ts_sent, - ) { - info!(context, "Deleting duplicate message {rfc724_mid_orig}."); - let target = context.get_delete_msgs_target().await?; - context - .sql - .execute( - "UPDATE imap SET target=? WHERE folder=? AND uidvalidity=? AND uid=?", - (target, folder, uidvalidity, uid), - ) - .await?; - } - Some(old_msg_id) + } else if rfc724_mid_orig == rfc724_mid { + replace_msg_id = None; + replace_chat_id = None; + } else if let Some((old_msg_id, old_ts_sent, is_trash)) = message::rfc724_mid_exists_ex( + context, + rfc724_mid_orig, + "chat_id=3", // Trash + ) + .await? + { + if is_trash && !message::prune_tombstone(context, rfc724_mid_orig).await? { + replace_msg_id = None; + } else if imap::is_dup_msg( + mime_parser.has_chat_version(), + mime_parser.timestamp_sent, + old_ts_sent, + ) { + info!(context, "Deleting duplicate message {rfc724_mid_orig}."); + let target = context.get_delete_msgs_target().await?; + context + .sql + .execute( + "UPDATE imap SET target=? WHERE folder=? AND uidvalidity=? AND uid=?", + (target, folder, uidvalidity, uid), + ) + .await?; + replace_msg_id = Some(old_msg_id); } else { - None - }; + replace_msg_id = Some(old_msg_id); + } + replace_chat_id = None; + } else { + replace_msg_id = None; replace_chat_id = None; } diff --git a/src/webxdc/webxdc_tests.rs b/src/webxdc/webxdc_tests.rs index d08ad2b30..1e8cc7d5f 100644 --- a/src/webxdc/webxdc_tests.rs +++ b/src/webxdc/webxdc_tests.rs @@ -1853,6 +1853,14 @@ async fn test_status_update_vs_delete_device_after() -> Result<()> { .await?; let alice_chat = alice.create_chat(bob).await; let alice_instance = send_webxdc_instance(alice, alice_chat.id).await?; + // Needed to test receiving of re-sent locally deleted webxdc. + bob.sql + .execute( + "INSERT INTO imap (rfc724_mid, folder, uid, uidvalidity, target) + VALUES (?1, ?2, ?3, ?4, ?5)", + (&alice_instance.rfc724_mid, "INBOX", 1, 1, "INBOX"), + ) + .await?; let bob_instance = bob.recv_msg(&alice.pop_sent_msg().await).await; assert_eq!(bob.add_or_lookup_contact(alice).await.is_bot(), false); @@ -1882,8 +1890,15 @@ async fn test_status_update_vs_delete_device_after() -> Result<()> { SystemTime::shift(Duration::from_secs(2700)); ephemeral::delete_expired_messages(bob, tools::time()).await?; let bob_instance = Message::load_from_db(bob, bob_instance.id).await?; - assert_eq!(bob_instance.chat_id.is_trash(), false); + SystemTime::shift(Duration::from_secs(1800)); + ephemeral::delete_expired_messages(bob, tools::time()).await?; + let bob_instance = Message::load_from_db_optional(bob, bob_instance.id).await?; + assert!(bob_instance.is_none()); + + // Additionally test that a re-sent instance can be received after deletion. + resend_msgs(alice, &[alice_instance.id]).await?; + bob.recv_msg(&alice.pop_sent_msg().await).await; Ok(()) }