From 7bb6890f263c137c56bb75914e885095acfcfdc1 Mon Sep 17 00:00:00 2001 From: Alexander Krotov Date: Tue, 14 Jul 2020 20:11:42 +0300 Subject: [PATCH] Send MDNs for messages deleted on the server Now MarkseenMsgOnImap sends MDN even if it can't mark the message as seen on the server. To prevent multiple MDNs from being sent, MarkseenMsgOnImap is postponed until the message is detected in a folder from which it is not going to be moved. --- python/tests/test_account.py | 1 - src/context.rs | 32 +---------------------------- src/dc_receive_imf.rs | 39 ++++++++++++++++++++---------------- src/imap/mod.rs | 35 ++++++++++++++++++++++++-------- src/job.rs | 16 ++++++++++++++- src/message.rs | 35 ++++++++++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 58 deletions(-) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 4746f2a53..4cd70c8e0 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -1542,7 +1542,6 @@ class TestOnlineAccount: assert msg.is_encrypted(), "Message is not encrypted" assert msg.chat == ac2.create_chat(ac4) - @pytest.mark.xfail def test_immediate_autodelete(self, acfactory, lp): ac1 = acfactory.get_online_configuring_account() ac2 = acfactory.get_online_configuring_account(mvbox=False, move=False, sentbox=False) diff --git a/src/context.rs b/src/context.rs index 5458282bf..9e652a46f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -15,12 +15,10 @@ use crate::contact::*; use crate::dc_tools::duration_to_str; use crate::error::*; use crate::events::{Event, EventEmitter, Events}; -use crate::job::{self, Action}; use crate::key::{DcKey, SignedPublicKey}; use crate::login_param::LoginParam; use crate::lot::Lot; -use crate::message::{self, Message, MessengerMessage, MsgId}; -use crate::param::Params; +use crate::message::{self, MsgId}; use crate::scheduler::Scheduler; use crate::sql::Sql; use std::time::SystemTime; @@ -460,34 +458,6 @@ impl Context { self.get_config(Config::ConfiguredMvboxFolder).await == Some(folder_name.as_ref().to_string()) } - - pub async fn do_heuristics_moves(&self, folder: &str, msg_id: MsgId) { - if !self.get_config_bool(Config::MvboxMove).await { - return; - } - - if self.is_mvbox(folder).await { - return; - } - if let Ok(msg) = Message::load_from_db(self, msg_id).await { - if msg.is_setupmessage() { - // do not move setup messages; - // there may be a non-delta device that wants to handle it - return; - } - - match msg.is_dc_message { - MessengerMessage::No => {} - MessengerMessage::Yes | MessengerMessage::Reply => { - job::add( - self, - job::Job::new(Action::MoveMsg, msg.id.to_u32(), Params::new(), 0), - ) - .await; - } - } - } - } } impl InnerContext { diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 4440b4da8..0b5c72dd7 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -224,24 +224,29 @@ pub async fn dc_receive_imf( ) .await; } - } else { + } else if insert_msg_id + .needs_move(context, server_folder.as_ref()) + .await + .unwrap_or_default() + { // Move message if we don't delete it immediately. - context - .do_heuristics_moves(server_folder.as_ref(), insert_msg_id) - .await; - if !mime_parser.mdn_reports.is_empty() && mime_parser.has_chat_version() { - // This is a Delta Chat MDN. Mark as read. - job::add( - context, - job::Job::new( - Action::MarkseenMsgOnImap, - insert_msg_id.to_u32(), - Params::new(), - 0, - ), - ) - .await; - } + job::add( + context, + job::Job::new(Action::MoveMsg, insert_msg_id.to_u32(), Params::new(), 0), + ) + .await; + } else if !mime_parser.mdn_reports.is_empty() && mime_parser.has_chat_version() { + // This is a Delta Chat MDN. Mark as read. + job::add( + context, + job::Job::new( + Action::MarkseenMsgOnImap, + insert_msg_id.to_u32(), + Params::new(), + 0, + ), + ) + .await; } } diff --git a/src/imap/mod.rs b/src/imap/mod.rs index bb095d2e6..4b83ed924 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -23,7 +23,7 @@ use crate::events::Event; use crate::headerdef::{HeaderDef, HeaderDefMap}; use crate::job::{self, Action}; use crate::login_param::{CertificateChecks, LoginParam}; -use crate::message::{self, update_server_uid}; +use crate::message::{self, update_server_uid, MessageState}; use crate::mimeparser; use crate::oauth2::dc_get_oauth2_access_token; use crate::param::Params; @@ -1366,14 +1366,26 @@ async fn precheck_imf( let delete_server_after = context.get_config_delete_server_after().await; if delete_server_after != Some(0) { - context - .do_heuristics_moves(server_folder.as_ref(), msg_id) + if msg_id + .needs_move(context, server_folder) + .await + .unwrap_or_default() + { + // If the bcc-self message is not moved, directly + // add MarkSeen job, otherwise MarkSeen job is + // added after the Move Job completed. + job::add( + context, + job::Job::new(Action::MoveMsg, msg_id.to_u32(), Params::new(), 0), + ) .await; - job::add( - context, - job::Job::new(Action::MarkseenMsgOnImap, msg_id.to_u32(), Params::new(), 0), - ) - .await; + } else { + job::add( + context, + job::Job::new(Action::MarkseenMsgOnImap, msg_id.to_u32(), Params::new(), 0), + ) + .await; + } } } else if old_server_folder != server_folder { info!( @@ -1408,6 +1420,13 @@ async fn precheck_imf( if old_server_folder != server_folder || old_server_uid != server_uid { update_server_uid(context, rfc724_mid, server_folder, server_uid).await; + if let Ok(MessageState::InSeen) = msg_id.get_state(context).await { + job::add( + context, + job::Job::new(Action::MarkseenMsgOnImap, msg_id.to_u32(), Params::new(), 0), + ) + .await; + }; context .interrupt_inbox(InterruptInfo::new(false, Some(msg_id))) .await; diff --git a/src/job.rs b/src/job.rs index 86c26ce57..498a2b00f 100644 --- a/src/job.rs +++ b/src/job.rs @@ -639,7 +639,21 @@ impl Job { let msg = job_try!(Message::load_from_db(context, MsgId::new(self.foreign_id)).await); let folder = msg.server_folder.as_ref().unwrap(); - match imap.set_seen(context, folder, msg.server_uid).await { + + let result = if msg.server_uid == 0 { + // The message is moved or deleted by us. + // + // Do not call set_seen with zero UID, as it will return + // ImapActionResult::RetryLater, but we do not want to + // retry. If the message was moved, we will create another + // job to mark the message as seen later. If it was + // deleted, there is nothing to do. + ImapActionResult::Failed + } else { + imap.set_seen(context, folder, msg.server_uid).await + }; + + match result { ImapActionResult::RetryLater => Status::RetryLater, ImapActionResult::AlreadyDone => Status::Finished(Ok(())), ImapActionResult::Success | ImapActionResult::Failed => { diff --git a/src/message.rs b/src/message.rs index cadc025b9..fa87fdca3 100644 --- a/src/message.rs +++ b/src/message.rs @@ -6,6 +6,7 @@ use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; use crate::chat::{self, Chat, ChatId}; +use crate::config::Config; use crate::constants::*; use crate::contact::*; use crate::context::*; @@ -68,6 +69,40 @@ impl MsgId { self.0 == 0 } + /// Returns message state. + pub async fn get_state(self, context: &Context) -> crate::sql::Result { + let result = context + .sql + .query_get_value_result("SELECT state FROM msgs WHERE id=?", paramsv![self]) + .await? + .unwrap_or_default(); + Ok(result) + } + + /// Returns true if the message needs to be moved from `folder`. + pub async fn needs_move(self, context: &Context, folder: &str) -> Result { + if !context.get_config_bool(Config::MvboxMove).await { + return Ok(false); + } + + if context.is_mvbox(folder).await { + return Ok(false); + } + + let msg = Message::load_from_db(context, self).await?; + + if msg.is_setupmessage() { + // do not move setup messages; + // there may be a non-delta device that wants to handle it + return Ok(false); + } + + match msg.is_dc_message { + MessengerMessage::No => Ok(false), + MessengerMessage::Yes | MessengerMessage::Reply => Ok(true), + } + } + /// Put message into trash chat and delete message text. /// /// It means the message is deleted locally, but not on the server