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

fix: Delete user-deleted messages on the server even if they show up on IMAP later

Browse Source 
Before, if the user deleted a message too quickly after sending, it was deleted only locally. The
fix is to remember for tombstones that the corresponding message should be deleted on the server
too.
This commit is contained in:
iequidoo
2024-03-27 22:09:55 -03:00
committed by iequidoo
parent f436e915d3
commit 13f58e0ca5
8 changed files with 71 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
python/tests/test_0_complex_or_slow.py
View File

@@ -687,5 +687,6 @@ def test_deleted_msgs_dont_reappear(acfactory):
ac1._evtracker.get_matching("DC_EVENT_SMTP_MESSAGE_SENT")
ac1.delete_messages([msg])
ac1._evtracker.get_matching("DC_EVENT_MSG_DELETED")
ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_DELETED")
time.sleep(5)
assert len(chat.get_messages()) == 0

4
src/ephemeral.rs
View File

@@ -1024,7 +1024,7 @@ mod tests {
t.send_text(self_chat.id, "Saved message, which we delete manually")
.await;
let msg = t.get_last_msg_in(self_chat.id).await;
msg.id.trash(&t).await?;
msg.id.trash(&t, false).await?;
check_msg_is_deleted(&t, &self_chat, msg.id).await;
self_chat
@@ -1304,7 +1304,7 @@ mod tests {
let msg = alice.get_last_msg().await;
// Message is deleted when its timer expires.
msg.id.trash(&alice).await?;
msg.id.trash(&alice, false).await?;
// Message with Message-ID <third@example.com>, referencing <first@example.com> and
// <second@example.com>, is received. The message <second@example.come> is not in the

18
src/imap.rs
View File

@@ -599,20 +599,26 @@ impl Imap {
// in the `INBOX.DeltaChat` folder again.
let _target;
let target = if let Some(message_id) = &message_id {
let is_dup = if let Some((_, ts_sent_old)) =
message::rfc724_mid_exists(context, message_id).await?
{
let msg_info =
message::rfc724_mid_exists_ex(context, message_id, "deleted=1").await?;
let delete = if let Some((_, _, true)) = msg_info {
info!(context, "Deleting locally deleted message {message_id}.");
true
} else if let Some((_, ts_sent_old, _)) = msg_info {
let is_chat_msg = headers.get_header_value(HeaderDef::ChatVersion).is_some();
let ts_sent = headers
.get_header_value(HeaderDef::Date)
.and_then(|v| mailparse::dateparse(&v).ok())
.unwrap_or_default();
is_dup_msg(is_chat_msg, ts_sent, ts_sent_old)
let is_dup = is_dup_msg(is_chat_msg, ts_sent, ts_sent_old);
if is_dup {
info!(context, "Deleting duplicate message {message_id}.");
}
is_dup
} else {
false
};
if is_dup {
info!(context, "Deleting duplicate message {message_id}.");
if delete {
&delete_target
} else if context
.sql

60
src/message.rs
View File

@@ -103,23 +103,31 @@ impl MsgId {
/// 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
pub async fn trash(self, context: &Context) -> Result<()> {
///
/// * `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()`.
pub async fn trash(self, context: &Context, on_server: bool) -> Result<()> {
let chat_id = DC_CHAT_ID_TRASH;
let deleted_subst = match on_server {
true => ", deleted=1",
false => "",
};
context
.sql
.execute(
// If you change which information is removed here, also change delete_expired_messages() and
// which information receive_imf::add_parts() still adds to the db if the chat_id is TRASH
r#"
UPDATE msgs
SET
chat_id=?, txt='', txt_normalized=NULL,
subject='', txt_raw='',
mime_headers='',
from_id=0, to_id=0,
param=''
WHERE id=?;
"#,
&format!(
"UPDATE msgs SET \
chat_id=?, txt='', txt_normalized=NULL, \
subject='', txt_raw='', \
mime_headers='', \
from_id=0, to_id=0, \
param=''{deleted_subst} \
WHERE id=?"
),
(chat_id, self),
)
.await?;
@@ -1549,8 +1557,9 @@ pub async fn delete_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> {
if msg.location_id > 0 {
delete_poi_location(context, msg.location_id).await?;
}
let on_server = true;
msg_id
.trash(context)
.trash(context, on_server)
.await
.with_context(|| format!("Unable to trash message {msg_id}"))?;
@@ -1894,23 +1903,26 @@ pub async fn estimate_deletion_cnt(
Ok(cnt)
}
/// See [`rfc724_mid_exists_and()`].
/// See [`rfc724_mid_exists_ex()`].
pub(crate) async fn rfc724_mid_exists(
context: &Context,
rfc724_mid: &str,
) -> Result<Option<(MsgId, i64)>> {
rfc724_mid_exists_and(context, rfc724_mid, "1").await
Ok(rfc724_mid_exists_ex(context, rfc724_mid, "1")
.await?
.map(|(id, ts_sent, _)| (id, ts_sent)))
}
/// Returns [MsgId] and "sent" timestamp of the message with given `rfc724_mid` (Message-ID header)
/// if it exists in the db.
/// Returns [MsgId] and "sent" timestamp of the most recent message with given `rfc724_mid`
/// (Message-ID header) and bool `expr` result if such messages exists in the db.
///
/// @param cond SQL subexpression for filtering messages.
pub(crate) async fn rfc724_mid_exists_and(
/// * `expr`: SQL expression additionally passed into `SELECT`. Evaluated to `true` iff it is true
/// for all messages with the given `rfc724_mid`.
pub(crate) async fn rfc724_mid_exists_ex(
context: &Context,
rfc724_mid: &str,
cond: &str,
) -> Result<Option<(MsgId, i64)>> {
expr: &str,
) -> Result<Option<(MsgId, i64, bool)>> {
let rfc724_mid = rfc724_mid.trim_start_matches('<').trim_end_matches('>');
if rfc724_mid.is_empty() {
warn!(context, "Empty rfc724_mid passed to rfc724_mid_exists");
@@ -1920,13 +1932,15 @@ pub(crate) async fn rfc724_mid_exists_and(
let res = context
.sql
.query_row_optional(
&("SELECT id, timestamp_sent FROM msgs WHERE rfc724_mid=? AND ".to_string() + cond),
&("SELECT id, timestamp_sent, MIN(".to_string()
+ expr
+ ") FROM msgs WHERE rfc724_mid=? ORDER BY timestamp_sent DESC"),
(rfc724_mid,),
|row| {
let msg_id: MsgId = row.get(0)?;
let timestamp_sent: i64 = row.get(1)?;
Ok((msg_id, timestamp_sent))
let expr_res: bool = row.get(2)?;
Ok((msg_id, timestamp_sent, expr_res))
},
)
.await?;

2
src/mimefactory.rs
View File

@@ -2112,7 +2112,7 @@ mod tests {
let incoming_msg = get_chat_msg(&t, new_msg.chat_id, 0, 2).await;
if delete_original_msg {
incoming_msg.id.trash(&t).await.unwrap();
incoming_msg.id.trash(&t, false).await.unwrap();
}
if message_arrives_inbetween {

12
src/receive_imf.rs
View File

@@ -27,7 +27,7 @@ use crate::headerdef::{HeaderDef, HeaderDefMap};
use crate::imap::{markseen_on_imap_table, GENERATED_PREFIX};
use crate::log::LogExt;
use crate::message::{
self, rfc724_mid_exists, rfc724_mid_exists_and, Message, MessageState, MessengerMessage, MsgId,
self, rfc724_mid_exists, rfc724_mid_exists_ex, Message, MessageState, MessengerMessage, MsgId,
Viewtype,
};
use crate::mimeparser::{parse_message_ids, AvatarAction, MimeMessage, SystemMessage};
@@ -1652,8 +1652,11 @@ RETURNING id
}
if let Some(replace_msg_id) = replace_msg_id {
// "Replace" placeholder with a message that has no parts.
replace_msg_id.trash(context).await?;
// 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;
replace_msg_id.trash(context, on_server).await?;
}
chat_id.unarchive_if_not_muted(context, state).await?;
@@ -2058,8 +2061,9 @@ async fn apply_group_changes(
|| match mime_parser.get_header(HeaderDef::InReplyTo) {
// If we don't know the referenced message, we missed some messages.
// Maybe they added/removed members, so we need to recreate our member list.
Some(reply_to) => rfc724_mid_exists_and(context, reply_to, "download_state=0")
Some(reply_to) => rfc724_mid_exists_ex(context, reply_to, "download_state=0")
.await?
.filter(|(_, _, downloaded)| *downloaded)
.is_none(),
None => false,
}

2
src/receive_imf/tests.rs
View File

@@ -2043,7 +2043,7 @@ async fn test_dont_assign_to_trash_by_parent() {
assert_eq!(msg.text, "Hi – hello");
println!("\n========= Delete the message ==========");
msg.id.trash(&t).await.unwrap();
msg.id.trash(&t, false).await.unwrap();
let msgs = chat::get_chat_msgs(&t.ctx, chat_id).await.unwrap();
assert_eq!(msgs.len(), 0);

9
src/sql/migrations.rs
View File

@@ -907,6 +907,15 @@ CREATE INDEX msgs_status_updates_index2 ON msgs_status_updates (uid);
)
.await?;
}
if dbversion < 111 {
// Whether the message part doesn't need to be stored on the server. If all parts are marked
// deleted, a server-side deletion is issued.
sql.execute_migration(
"ALTER TABLE msgs ADD COLUMN deleted INTEGER NOT NULL DEFAULT 0",
111,
)
.await?;
}
if dbversion < 111 {
sql.execute_migration(