Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-27 18:36:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: Prune tombstone when a message isn't expected to appear on IMAP anymore (#7115)

Browse Source 
This allows to receive deleted messages again if they're re-sent:
- After a manual deletion,
- If both DeleteServerAfter and DeleteDeviceAfter are set,

w/o waiting for 2 days when stale tombstones are GC-ed. This is particularly useful for webxdc. If
only DeleteDeviceAfter is set though, this changes nothing and maybe a separate fix is needed.

This may also greately reduce the db size for some bots which receive and immediately delete many
messages.

Also insert a tombstone to the db if a deletion request (incl. sync messages) can't find
`rfc724_mid`. This may happen in case of message reordering and the deleted message mustn't appear
when it finally arrives.
This commit is contained in:
iequidoo
2025-08-23 16:35:36 -03:00
parent 2d7dc7a1be
commit f0144f8789
5 changed files with 120 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/imap.rs
View File

@@ -634,6 +634,9 @@ impl Imap {
let target = if let Some(message_id) = &message_id {
let msg_info =
message::rfc724_mid_exists_ex(context, message_id, "deleted=1").await?;
if msg_info.is_some_and(|(_, ts_sent, _)| ts_sent == 0) {
message::prune_tombstone(context, message_id).await?;
}
let delete = if let Some((_, _, true)) = msg_info {
info!(context, "Deleting locally deleted message {message_id}.");
true
@@ -2342,10 +2345,16 @@ pub(crate) async fn prefetch_should_download(
message_id: &str,
mut flags: impl Iterator<Item = Flag<'_>>,
) -> Result<bool> {
if message::rfc724_mid_exists(context, message_id)
.await?
.is_some()
if let Some((_, _, is_trash)) = message::rfc724_mid_exists_ex(
context,
message_id,
"chat_id=3", // Trash
)
.await?
{
if is_trash {
message::prune_tombstone(context, message_id).await?;
}
markseen_on_imap_table(context, message_id).await?;
return Ok(false);
}

74
src/message.rs
View File

@@ -112,18 +112,20 @@ impl MsgId {
.unwrap_or_default())
}
/// Put message into trash chat and delete message text.
/// Put message into trash chat or delete it.
///
/// It means the message is deleted locally, but not on the server.
/// We keep some infos to
/// 1. not download the same message again
/// 2. be able to delete the message on the server if we want to
///
/// * `on_server`: Delete the message on the server also if it is seen on IMAP later, but only
/// if all parts of the message are trashed with this flag. `true` if the user explicitly
/// deletes the message. As for trashing a partially downloaded message when replacing it with
/// a fully downloaded one, see `receive_imf::add_parts()`.
/// * `on_server`: Keep some info to delete the message on the server also if it is seen on IMAP
/// later.
pub(crate) async fn trash(self, context: &Context, on_server: bool) -> Result<()> {
if !on_server {
context
.sql
.execute("DELETE FROM msgs WHERE id=?1", (self,))
.await?;
return Ok(());
}
context
.sql
.execute(
@@ -132,7 +134,7 @@ impl MsgId {
// still adds to the db if chat_id is TRASH.
"INSERT OR REPLACE INTO msgs (id, rfc724_mid, timestamp, chat_id, deleted)
SELECT ?1, rfc724_mid, timestamp, ?, ? FROM msgs WHERE id=?1",
(self, DC_CHAT_ID_TRASH, on_server),
(self, DC_CHAT_ID_TRASH, true),
)
.await?;
@@ -1590,11 +1592,15 @@ pub(crate) async fn get_mime_headers(context: &Context, msg_id: MsgId) -> Result
/// Delete a single message from the database, including references in other tables.
/// This may be called in batches; the final events are emitted in delete_msgs_locally_done() then.
pub(crate) async fn delete_msg_locally(context: &Context, msg: &Message) -> Result<()> {
pub(crate) async fn delete_msg_locally(
context: &Context,
msg: &Message,
keep_tombstone: bool,
) -> Result<()> {
if msg.location_id > 0 {
delete_poi_location(context, msg.location_id).await?;
}
let on_server = true;
let on_server = keep_tombstone;
msg.id
.trash(context, on_server)
.await
@@ -1662,6 +1668,7 @@ pub async fn delete_msgs_ex(
let mut modified_chat_ids = HashSet::new();
let mut deleted_rfc724_mid = Vec::new();
let mut res = Ok(());
let mut msgs = Vec::with_capacity(msg_ids.len());
for &msg_id in msg_ids {
let msg = Message::load_from_db(context, msg_id).await?;
@@ -1679,18 +1686,24 @@ pub async fn delete_msgs_ex(
let target = context.get_delete_msgs_target().await?;
let update_db = |trans: &mut rusqlite::Transaction| {
trans.execute(
// NB: If the message isn't sent yet, keeping its tombstone is unnecessary, but safe.
let keep_tombstone = trans.execute(
"UPDATE imap SET target=? WHERE rfc724_mid=?",
(target, msg.rfc724_mid),
)?;
(&target, msg.rfc724_mid),
)? == 0
|| !target.is_empty();
trans.execute("DELETE FROM smtp WHERE msg_id=?", (msg_id,))?;
Ok(())
Ok(keep_tombstone)
};
if let Err(e) = context.sql.transaction(update_db).await {
error!(context, "delete_msgs: failed to update db: {e:#}.");
res = Err(e);
continue;
}
let keep_tombstone = match context.sql.transaction(update_db).await {
Ok(v) => v,
Err(e) => {
error!(context, "delete_msgs: failed to update db: {e:#}.");
res = Err(e);
continue;
}
};
msgs.push((msg_id, keep_tombstone));
}
res?;
@@ -1719,9 +1732,9 @@ pub async fn delete_msgs_ex(
.await?;
}
for &msg_id in msg_ids {
for (msg_id, keep_tombstone) in msgs {
let msg = Message::load_from_db(context, msg_id).await?;
delete_msg_locally(context, &msg).await?;
delete_msg_locally(context, &msg, keep_tombstone).await?;
}
delete_msgs_locally_done(context, msg_ids, modified_chat_ids).await?;
@@ -1731,6 +1744,23 @@ pub async fn delete_msgs_ex(
Ok(())
}
/// 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
.sql
.execute(
"DELETE FROM msgs
WHERE rfc724_mid=?
AND chat_id=?
AND NOT EXISTS (
SELECT * FROM imap WHERE msgs.rfc724_mid=rfc724_mid AND target!=''
)",
(rfc724_mid, DC_CHAT_ID_TRASH),
)
.await?;
Ok(())
}
/// Marks requested messages as seen.
pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()> {
if msg_ids.is_empty() {

34
src/receive_imf.rs
View File

@@ -44,7 +44,7 @@ use crate::securejoin::{self, handle_securejoin_handshake, observe_securejoin_on
use crate::simplify;
use crate::stock_str;
use crate::sync::Sync::*;
use crate::tools::{self, buf_compress, remove_subject_prefix};
use crate::tools::{self, buf_compress, remove_subject_prefix, time};
use crate::{chatlist_events, ensure_and_debug_assert, ensure_and_debug_assert_eq, location};
use crate::{contact, imap};
@@ -557,25 +557,27 @@ pub(crate) async fn receive_imf_inner(
// make sure, this check is done eg. before securejoin-processing.
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?;
}
replace_msg_id = Some(old_msg_id);
replace_chat_id = if let Some(msg) = Message::load_from_db_optional(context, old_msg_id)
.await?
.filter(|msg| msg.download_state() != DownloadState::Done)
{
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.");
Some(msg.chat_id)
replace_chat_id = Some(msg.chat_id);
} else {
// The message was already fully downloaded
// or cannot be loaded because it is deleted.
None
};
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,
@@ -2200,10 +2202,7 @@ RETURNING id
}
if let Some(replace_msg_id) = replace_msg_id {
// Trash the "replace" placeholder with a message that has no parts. If it has the original
// "Message-ID", mark the placeholder for server-side deletion so as if the user deletes the
// fully downloaded message later, the server-side deletion is issued.
let on_server = rfc724_mid == rfc724_mid_orig;
let on_server = false;
replace_msg_id.trash(context, on_server).await?;
}
@@ -2311,7 +2310,8 @@ async fn handle_edit_delete(
{
if let Some(msg) = Message::load_from_db_optional(context, msg_id).await? {
if msg.from_id == from_id {
message::delete_msg_locally(context, &msg).await?;
let keep_tombstone = false;
message::delete_msg_locally(context, &msg, keep_tombstone).await?;
msg_ids.push(msg.id);
modified_chat_ids.insert(msg.chat_id);
} else {
@@ -2322,6 +2322,14 @@ async fn handle_edit_delete(
}
} else {
warn!(context, "Delete message: {rfc724_mid:?} not found.");
context
.sql
.execute(
"INSERT INTO msgs (rfc724_mid, timestamp, chat_id)
VALUES (?, ?, ?)",
(rfc724_mid, time(), DC_CHAT_ID_TRASH),
)
.await?;
}
}
message::delete_msgs_locally_done(context, &msg_ids, modified_chat_ids).await?;

25
src/receive_imf/receive_imf_tests.rs
View File

@@ -815,6 +815,31 @@ async fn test_concat_multiple_ndns() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_receive_resent_deleted_msg() -> Result<()> {
let alice = &TestContext::new_alice().await;
let bob = &TestContext::new_bob().await;
let alice_bob_id = alice.add_or_lookup_contact_id(bob).await;
let alice_chat_id = ChatId::create_for_contact(alice, alice_bob_id).await?;
let sent = alice.send_text(alice_chat_id, "hi").await;
let alice_msg = Message::load_from_db(alice, sent.sender_msg_id).await?;
bob.sql
.execute(
"INSERT INTO imap (rfc724_mid, folder, uid, uidvalidity, target)
VALUES (?1, ?2, ?3, ?4, ?5)",
(&alice_msg.rfc724_mid, "INBOX", 1, 1, "INBOX"),
)
.await?;
let bob_msg = bob.recv_msg(&sent).await;
let bob_chat_id = bob_msg.chat_id;
message::delete_msgs(bob, &[bob_msg.id]).await?;
chat::resend_msgs(alice, &[alice_msg.id]).await?;
let bob_msg = bob.recv_msg(&alice.pop_sent_msg().await).await;
assert_eq!(bob_msg.chat_id, bob_chat_id);
assert_eq!(bob_msg.text, "hi");
Ok(())
}
async fn load_imf_email(context: &Context, imf_raw: &[u8]) -> Message {
context
.set_config(Config::ShowEmails, Some("2"))

13
src/sync.rs
View File

@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
use crate::chat::{self, ChatId};
use crate::config::Config;
use crate::constants::Blocked;
use crate::constants::{self, Blocked};
use crate::contact::ContactId;
use crate::context::Context;
use crate::log::LogExt;
@@ -317,14 +317,21 @@ impl Context {
for rfc724_mid in msgs {
if let Some((msg_id, _)) = message::rfc724_mid_exists(self, rfc724_mid).await? {
if let Some(msg) = Message::load_from_db_optional(self, msg_id).await? {
message::delete_msg_locally(self, &msg).await?;
let keep_tombstone = false;
message::delete_msg_locally(self, &msg, keep_tombstone).await?;
msg_ids.push(msg.id);
modified_chat_ids.insert(msg.chat_id);
} else {
warn!(self, "Sync message delete: Database entry does not exist.");
}
} else {
warn!(self, "Sync message delete: {rfc724_mid:?} not found.");
info!(self, "sync_message_deletion: {rfc724_mid:?} not found.");
self.sql
.execute(
"INSERT INTO msgs (rfc724_mid, timestamp, chat_id) VALUES (?, ?, ?)",
(rfc724_mid, time(), constants::DC_CHAT_ID_TRASH),
)
.await?;
}
}
message::delete_msgs_locally_done(self, &msg_ids, modified_chat_ids).await?;