introduce DC_IMAP_MESSAGE_MOVED event and try harder to send out MDNs

This commit is contained in:
holger krekel
2019-10-01 15:30:33 +02:00
parent 96066712bd
commit 509a21ff05
7 changed files with 173 additions and 214 deletions

View File

@@ -3905,7 +3905,7 @@ int64_t dc_lot_get_timestamp (const dc_lot_t* lot);
#define DC_EVENT_SMTP_MESSAGE_SENT 103 #define DC_EVENT_SMTP_MESSAGE_SENT 103
/** /**
* Emitted when a message was successfully marked as deleted on the SMTP server. * Emitted when a message was successfully marked as deleted on the IMAP server.
* *
* @param data1 0 * @param data1 0
* @param data2 (const char*) Info string in english language. * @param data2 (const char*) Info string in english language.
@@ -3914,6 +3914,15 @@ int64_t dc_lot_get_timestamp (const dc_lot_t* lot);
*/ */
#define DC_EVENT_IMAP_MESSAGE_DELETED 104 #define DC_EVENT_IMAP_MESSAGE_DELETED 104
/**
* Emitted when a message was successfully moved on IMAP.
*
* @param data1 0
* @param data2 (const char*) Info string in english language.
* Must not be free()'d or modified and is valid only until the callback returns.
* @return 0
*/
#define DC_EVENT_IMAP_MESSAGE_MOVED 105
/** /**
* The library-user should write a warning string to the log. * The library-user should write a warning string to the log.

View File

@@ -126,6 +126,7 @@ impl ContextWrapper {
| Event::ImapConnected(msg) | Event::ImapConnected(msg)
| Event::SmtpMessageSent(msg) | Event::SmtpMessageSent(msg)
| Event::ImapMessageDeleted(msg) | Event::ImapMessageDeleted(msg)
| Event::ImapMessageMoved(msg)
| Event::Warning(msg) | Event::Warning(msg)
| Event::Error(msg) | Event::Error(msg)
| Event::ErrorNetwork(msg) | Event::ErrorNetwork(msg)

View File

@@ -62,6 +62,7 @@ DC_EVENT_SMTP_CONNECTED = 101
DC_EVENT_IMAP_CONNECTED = 102 DC_EVENT_IMAP_CONNECTED = 102
DC_EVENT_SMTP_MESSAGE_SENT = 103 DC_EVENT_SMTP_MESSAGE_SENT = 103
DC_EVENT_IMAP_MESSAGE_DELETED = 104 DC_EVENT_IMAP_MESSAGE_DELETED = 104
DC_EVENT_IMAP_MESSAGE_MOVED = 105
DC_EVENT_WARNING = 300 DC_EVENT_WARNING = 300
DC_EVENT_ERROR = 400 DC_EVENT_ERROR = 400
DC_EVENT_ERROR_NETWORK = 401 DC_EVENT_ERROR_NETWORK = 401

View File

@@ -394,6 +394,17 @@ class TestOnlineAccount:
ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED") ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
assert ev[2] > const.DC_CHAT_ID_LAST_SPECIAL assert ev[2] > const.DC_CHAT_ID_LAST_SPECIAL
def test_move_works(self, acfactory):
ac1 = acfactory.get_online_configuring_account()
ac2 = acfactory.get_online_configuring_account(mvbox=True)
wait_configuration_progress(ac2, 1000)
wait_configuration_progress(ac1, 1000)
chat = self.get_chat(ac1, ac2)
chat.send_text("message1")
ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
assert ev[2] > const.DC_CHAT_ID_LAST_SPECIAL
ev = ac2._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
def test_forward_messages(self, acfactory): def test_forward_messages(self, acfactory):
ac1, ac2 = acfactory.get_two_online_accounts() ac1, ac2 = acfactory.get_two_online_accounts()
chat = self.get_chat(ac1, ac2) chat = self.get_chat(ac1, ac2)

View File

@@ -48,6 +48,12 @@ pub enum Event {
#[strum(props(id = "104"))] #[strum(props(id = "104"))]
ImapMessageDeleted(String), ImapMessageDeleted(String),
/// Emitted when an IMAP message has been moved
///
/// @return 0
#[strum(props(id = "105"))]
ImapMessageMoved(String),
/// The library-user should write a warning string to the log. /// The library-user should write a warning string to the log.
/// Passed to the callback given to dc_context_new(). /// Passed to the callback given to dc_context_new().
/// ///

View File

@@ -10,7 +10,7 @@ use crate::context::Context;
use crate::dc_receive_imf::dc_receive_imf; use crate::dc_receive_imf::dc_receive_imf;
use crate::error::Error; use crate::error::Error;
use crate::events::Event; use crate::events::Event;
use crate::job::{job_add, Action}; use crate::job::{connect_to_inbox, job_add, Action};
use crate::login_param::LoginParam; use crate::login_param::LoginParam;
use crate::message::{self, update_msg_move_state, update_server_uid}; use crate::message::{self, update_msg_move_state, update_server_uid};
use crate::oauth2::dc_get_oauth2_access_token; use crate::oauth2::dc_get_oauth2_access_token;
@@ -930,6 +930,7 @@ impl Imap {
} else { } else {
let msg = &msgs[0]; let msg = &msgs[0];
// XXX put flags into a set and pass them to dc_receive_imf
let is_deleted = msg let is_deleted = msg
.flags() .flags()
.iter() .iter()
@@ -1116,107 +1117,76 @@ impl Imap {
cvar.notify_one(); cvar.notify_one();
} }
pub fn mv<S1: AsRef<str>, S2: AsRef<str>>( pub fn mv(
&self, &self,
context: &Context, context: &Context,
folder: S1, folder: &str,
uid: u32, uid: u32,
dest_folder: S2, dest_folder: &str,
dest_uid: &mut u32, dest_uid: &mut u32,
) -> ImapResult { ) -> ImapResult {
let mut res = ImapResult::RetryLater; if folder == dest_folder {
info!(
context,
"Skip moving message; message {}/{} is already in {}...", folder, uid, dest_folder,
);
return ImapResult::AlreadyDone;
}
if let Some(imapresult) = self.prepare_imap_operation_on_msg(context, folder, uid) {
return imapresult;
}
// we are connected, and the folder is selected
// XXX Rust-Imap provides no target uid on mv, so just set it to 0
*dest_uid = 0;
let set = format!("{}", uid); let set = format!("{}", uid);
let display_folder_id = format!("{}/{}", folder, uid);
if uid == 0 { if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
res = ImapResult::Failed; match session.uid_mv(&set, &dest_folder) {
} else if folder.as_ref() == dest_folder.as_ref() { Ok(_) => {
info!( emit_event!(
context, context,
"Skip moving message; message {}/{} is already in {}...", Event::ImapMessageMoved(format!(
folder.as_ref(), "IMAP Message {} moved to {}",
uid, display_folder_id, dest_folder
dest_folder.as_ref() ))
); );
return ImapResult::Success;
res = ImapResult::AlreadyDone; }
} else { Err(err) => {
info!( warn!(
context, context,
"Moving message {}/{} to {}...", "Cannot move message, fallback to COPY/DELETE {}/{} to {}: {}",
folder.as_ref(), folder,
uid, uid,
dest_folder.as_ref() dest_folder,
); err
);
if self.select_folder(context, Some(folder.as_ref())) == 0 {
warn!(
context,
"Cannot select folder {} for moving message.",
folder.as_ref()
);
} else {
let moved = if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
match session.uid_mv(&set, &dest_folder) {
Ok(_) => {
res = ImapResult::Success;
true
}
Err(err) => {
info!(
context,
"Cannot move message, fallback to COPY/DELETE {}/{} to {}: {}",
folder.as_ref(),
uid,
dest_folder.as_ref(),
err
);
false
}
}
} else {
unreachable!();
};
if !moved {
let copied = if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
match session.uid_copy(&set, &dest_folder) {
Ok(_) => true,
Err(err) => {
eprintln!("error copy: {:?}", err);
info!(context, "Cannot copy message.",);
false
}
}
} else {
unreachable!();
};
if copied {
if !self.add_flag_finalized(context, uid, "\\Deleted") {
warn!(context, "Giving up: cannot mark {} as \"Deleted\".", uid);
}
self.config.write().unwrap().selected_folder_needs_expunge = true;
res = ImapResult::Success;
}
} }
} }
} } else {
unreachable!();
};
if res == ImapResult::Success { if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
// TODO: is this correct? match session.uid_copy(&set, &dest_folder) {
*dest_uid = uid; Ok(_) => {
} if !self.add_flag_finalized(context, uid, "\\Deleted") {
warn!(context, "Cannot mark {} as \"Deleted\" after copy.", uid);
if res == ImapResult::RetryLater { ImapResult::Failed
if self.should_reconnect() { } else {
ImapResult::RetryLater self.config.write().unwrap().selected_folder_needs_expunge = true;
} else { ImapResult::Success
ImapResult::Failed }
}
Err(err) => {
warn!(context, "Could not copy message: {}", err);
ImapResult::Failed
}
} }
} else { } else {
res unreachable!();
} }
} }
@@ -1257,10 +1227,14 @@ impl Imap {
uid: u32, uid: u32,
) -> Option<ImapResult> { ) -> Option<ImapResult> {
if uid == 0 { if uid == 0 {
Some(ImapResult::Failed) return Some(ImapResult::Failed);
} else if !self.is_connected() { } else if !self.is_connected() {
Some(ImapResult::RetryLater) connect_to_inbox(context, &self);
} else if self.select_folder(context, Some(&folder)) == 0 { if !self.is_connected() {
return Some(ImapResult::RetryLater);
}
}
if self.select_folder(context, Some(&folder)) == 0 {
warn!( warn!(
context, context,
"Cannot select folder {} for preparing IMAP operation", folder "Cannot select folder {} for preparing IMAP operation", folder
@@ -1290,96 +1264,76 @@ impl Imap {
} }
// only returns 0 on connection problems; we should try later again in this case * // only returns 0 on connection problems; we should try later again in this case *
pub fn delete_msg<S1: AsRef<str>, S2: AsRef<str>>( pub fn delete_msg(
&self, &self,
context: &Context, context: &Context,
message_id: S1, message_id: &str,
folder: S2, folder: &str,
server_uid: &mut u32, uid: &mut u32,
) -> usize { ) -> ImapResult {
let mut success = false; if let Some(imapresult) = self.prepare_imap_operation_on_msg(context, folder, *uid) {
if *server_uid == 0 { return imapresult;
success = true }
} else { // we are connected, and the folder is selected
info!(
context,
"Marking message \"{}\", {}/{} for deletion...",
message_id.as_ref(),
folder.as_ref(),
server_uid,
);
if self.select_folder(context, Some(&folder)) == 0 { let set = format!("{}", uid);
warn!( let display_imap_id = format!("{}/{}", folder, uid);
context,
"Cannot select folder {} for deleting message.",
folder.as_ref()
);
} else {
let set = format!("{}", server_uid);
let display_imap_id = format!("{}/{}", folder.as_ref(), server_uid);
if let Some(ref mut session) = &mut *self.session.lock().unwrap() { // double-check that we are deleting the correct message-id
match session.uid_fetch(set, PREFETCH_FLAGS) { // this comes at the expense of another imap query
Ok(msgs) => { if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
if msgs.is_empty() { match session.uid_fetch(set, PREFETCH_FLAGS) {
warn!( Ok(msgs) => {
context, if msgs.is_empty() {
"Cannot delete on IMAP, {}: message-id gone '{}'", warn!(
display_imap_id, context,
message_id.as_ref(), "Cannot delete on IMAP, {}: imap entry gone '{}'",
); display_imap_id,
} else { message_id,
let remote_message_id = );
prefetch_get_message_id(msgs.first().unwrap()) return ImapResult::Failed;
.unwrap_or_default();
if remote_message_id != message_id.as_ref() {
warn!(
context,
"Cannot delete on IMAP, {}: remote message-id '{}' != '{}'",
display_imap_id,
remote_message_id,
message_id.as_ref(),
);
}
}
*server_uid = 0;
}
Err(err) => {
eprintln!("fetch error: {:?}", err);
warn!(
context,
"Cannot delete on IMAP, {} not found.",
display_imap_id,
);
*server_uid = 0;
}
} }
} let remote_message_id =
prefetch_get_message_id(msgs.first().unwrap()).unwrap_or_default();
// mark the message for deletion if remote_message_id != message_id.as_ref() {
if !self.add_flag_finalized(context, *server_uid, "\\Deleted") { warn!(
warn!(context, "Cannot mark message as \"Deleted\"."); context,
} else { "Cannot delete on IMAP, {}: remote message-id '{}' != '{}'",
emit_event!( display_imap_id,
remote_message_id,
message_id,
);
}
*uid = 0;
}
Err(err) => {
warn!(
context, context,
Event::ImapMessageDeleted(format!( "Cannot delete {} on IMAP: {}", display_imap_id, err
"IMAP Message {} marked as deleted [{}]",
display_imap_id, message_id.as_ref()
))
); );
self.config.write().unwrap().selected_folder_needs_expunge = true; *uid = 0;
success = true
} }
} }
} }
if success { // mark the message for deletion
1 if !self.add_flag_finalized(context, *uid, "\\Deleted") {
warn!(
context,
"Cannot mark message {} as \"Deleted\".", display_imap_id
);
ImapResult::Failed
} else { } else {
self.is_connected() as usize emit_event!(
context,
Event::ImapMessageDeleted(format!(
"IMAP Message {} marked as deleted [{}]",
display_imap_id, message_id
))
);
self.config.write().unwrap().selected_folder_needs_expunge = true;
ImapResult::Success
} }
} }

View File

@@ -216,13 +216,6 @@ impl Job {
fn do_DC_JOB_MOVE_MSG(&mut self, context: &Context) { fn do_DC_JOB_MOVE_MSG(&mut self, context: &Context) {
let inbox = context.inbox.read().unwrap(); let inbox = context.inbox.read().unwrap();
if !inbox.is_connected() {
connect_to_inbox(context, &inbox);
if !inbox.is_connected() {
self.try_again_later(3, Some("could not connect".into()));
return;
}
}
if let Ok(msg) = Message::load_from_db(context, self.foreign_id) { if let Ok(msg) = Message::load_from_db(context, self.foreign_id) {
if context if context
.sql .sql
@@ -269,26 +262,18 @@ impl Job {
if let Ok(mut msg) = Message::load_from_db(context, self.foreign_id) { if let Ok(mut msg) = Message::load_from_db(context, self.foreign_id) {
if !msg.rfc724_mid.is_empty() { if !msg.rfc724_mid.is_empty() {
/* eg. device messages have no Message-ID */ /* eg. device messages have no Message-ID */
let mut delete_from_server = true; if message::rfc724_mid_cnt(context, &msg.rfc724_mid) > 1 {
if message::rfc724_mid_cnt(context, &msg.rfc724_mid) != 1 {
info!( info!(
context, context,
"The message is deleted from the server when all parts are deleted.", "The message is deleted from the server when all parts are deleted.",
); );
delete_from_server = false; } else {
} /* if this is the last existing part of the message,
/* if this is the last existing part of the message, we delete the message from the server */ we delete the message from the server */
if delete_from_server {
if !inbox.is_connected() {
connect_to_inbox(context, &inbox);
if !inbox.is_connected() {
self.try_again_later(3i32, None);
return;
}
}
let mid = msg.rfc724_mid; let mid = msg.rfc724_mid;
let server_folder = msg.server_folder.as_ref().unwrap(); let server_folder = msg.server_folder.as_ref().unwrap();
if 0 == inbox.delete_msg(context, &mid, server_folder, &mut msg.server_uid) { let res = inbox.delete_msg(context, &mid, server_folder, &mut msg.server_uid);
if res == ImapResult::RetryLater {
self.try_again_later(-1i32, None); self.try_again_later(-1i32, None);
return; return;
} }
@@ -302,25 +287,24 @@ impl Job {
fn do_DC_JOB_MARKSEEN_MSG_ON_IMAP(&mut self, context: &Context) { fn do_DC_JOB_MARKSEEN_MSG_ON_IMAP(&mut self, context: &Context) {
let inbox = context.inbox.read().unwrap(); let inbox = context.inbox.read().unwrap();
if !inbox.is_connected() {
connect_to_inbox(context, &inbox);
if !inbox.is_connected() {
self.try_again_later(3i32, None);
return;
}
}
if let Ok(msg) = Message::load_from_db(context, self.foreign_id) { if let Ok(msg) = Message::load_from_db(context, self.foreign_id) {
let folder = msg.server_folder.as_ref().unwrap(); let folder = msg.server_folder.as_ref().unwrap();
match inbox.set_seen(context, folder, msg.server_uid) { match inbox.set_seen(context, folder, msg.server_uid) {
ImapResult::Failed => {}
ImapResult::RetryLater => { ImapResult::RetryLater => {
self.try_again_later(3i32, None); self.try_again_later(3i32, None);
} }
_ => { ImapResult::AlreadyDone => {}
ImapResult::Success | ImapResult::Failed => {
// XXX the message might just have been moved
// we want to send out an MDN anyway
// The job will not be retried so locally
// there is no risk of double-sending MDNs.
if 0 != msg.param.get_int(Param::WantsMdn).unwrap_or_default() if 0 != msg.param.get_int(Param::WantsMdn).unwrap_or_default()
&& context.get_config_bool(Config::MdnsEnabled) && context.get_config_bool(Config::MdnsEnabled)
{ {
send_mdn(context, msg.id); if let Err(err) = send_mdn(context, msg.id) {
warn!(context, "could not send out mdn for {}: {}", msg.id, err);
}
} }
} }
} }
@@ -336,16 +320,9 @@ impl Job {
.to_string(); .to_string();
let uid = self.param.get_int(Param::ServerUid).unwrap_or_default() as u32; let uid = self.param.get_int(Param::ServerUid).unwrap_or_default() as u32;
let inbox = context.inbox.read().unwrap(); let inbox = context.inbox.read().unwrap();
if inbox.set_seen(context, &folder, uid) == ImapResult::RetryLater {
if !inbox.is_connected() {
connect_to_inbox(context, &inbox);
if !inbox.is_connected() {
self.try_again_later(3, None);
return;
}
}
if inbox.set_seen(context, &folder, uid) == ImapResult::Failed {
self.try_again_later(3i32, None); self.try_again_later(3i32, None);
return;
} }
if 0 != self.param.get_int(Param::AlsoMove).unwrap_or_default() { if 0 != self.param.get_int(Param::AlsoMove).unwrap_or_default() {
if context if context
@@ -360,7 +337,7 @@ impl Job {
if let Some(dest_folder) = dest_folder { if let Some(dest_folder) = dest_folder {
let mut dest_uid = 0; let mut dest_uid = 0;
if ImapResult::RetryLater if ImapResult::RetryLater
== inbox.mv(context, folder, uid, dest_folder, &mut dest_uid) == inbox.mv(context, &folder, uid, &dest_folder, &mut dest_uid)
{ {
self.try_again_later(3, None); self.try_again_later(3, None);
} }
@@ -941,7 +918,7 @@ fn suspend_smtp_thread(context: &Context, suspend: bool) {
} }
} }
fn connect_to_inbox(context: &Context, inbox: &Imap) -> libc::c_int { pub fn connect_to_inbox(context: &Context, inbox: &Imap) -> libc::c_int {
let ret_connected = dc_connect_to_configured_imap(context, inbox); let ret_connected = dc_connect_to_configured_imap(context, inbox);
if 0 != ret_connected { if 0 != ret_connected {
inbox.set_watch_folder("INBOX".into()); inbox.set_watch_folder("INBOX".into());