fix: Don't try to do fetch_move_delete() if Trash is needed but not yet configured

This fixes things for Gmail f.e. Before, `Imap::fetch_move_delete()` was called before looking for
Trash and returned an error because of that failing the whole `fetch_idle()` which prevented
configuring Trash in turn.
This commit is contained in:
iequidoo
2024-04-04 04:36:01 -03:00
committed by iequidoo
parent d39cbcdc8d
commit e9cfcd9d1b
3 changed files with 59 additions and 22 deletions

View File

@@ -2128,17 +2128,21 @@ def test_delete_multiple_messages(acfactory, lp):
def test_trash_multiple_messages(acfactory, lp):
ac1, ac2 = acfactory.get_online_accounts(2)
ac2.stop_io()
# Create the Trash folder on IMAP server
# and recreate the account so Trash folder is configured.
# Create the Trash folder on IMAP server and configure deletion to it. There was a bug that if
# Trash wasn't configured initially, it can't be configured later, let's check this.
lp.sec("Creating trash folder")
ac2.direct_imap.create_folder("Trash")
lp.sec("Creating new accounts")
ac2 = acfactory.new_online_configuring_account(cloned_from=ac2)
acfactory.bring_accounts_online()
ac2.set_config("delete_to_trash", "1")
assert ac2.get_config("configured_trash_folder")
lp.sec("Check that Trash can be configured initially as well")
ac3 = acfactory.new_online_configuring_account(cloned_from=ac2)
acfactory.bring_accounts_online()
assert ac3.get_config("configured_trash_folder")
ac3.stop_io()
ac2.start_io()
chat12 = acfactory.get_accepted_chat(ac1, ac2)
lp.sec("ac1: sending 3 messages")
@@ -2153,6 +2157,9 @@ def test_trash_multiple_messages(acfactory, lp):
assert msg.text in texts
if text != "second":
to_delete.append(msg)
# ac2 has received some messages, this is impossible w/o the trash folder configured, let's
# check the configuration.
assert ac2.get_config("configured_trash_folder") == "Trash"
lp.sec("ac2: deleting all messages except second")
assert len(to_delete) == len(texts) - 1

View File

@@ -89,9 +89,16 @@ impl Imap {
Config::ConfiguredSentboxFolder,
Config::ConfiguredTrashFolder,
] {
context
.set_config_internal(conf, folder_configs.get(&conf).map(|s| s.as_str()))
.await?;
let val = folder_configs.get(&conf).map(|s| s.as_str());
let interrupt = conf == Config::ConfiguredTrashFolder
&& val.is_some()
&& context.get_config(conf).await?.is_none();
context.set_config_internal(conf, val).await?;
if interrupt {
// `Imap::fetch_move_delete()` is possible now for other folders (NB: we are in the
// Inbox loop).
context.scheduler.interrupt_oboxes().await;
}
}
last_scan.replace(tools::Time::now());

View File

@@ -247,6 +247,14 @@ impl SchedulerState {
}
}
/// Interrupt optional boxes (mvbox, sentbox) loops.
pub(crate) async fn interrupt_oboxes(&self) {
let inner = self.inner.read().await;
if let InnerSchedulerState::Started(ref scheduler) = *inner {
scheduler.interrupt_oboxes();
}
}
pub(crate) async fn interrupt_smtp(&self) {
let inner = self.inner.read().await;
if let InnerSchedulerState::Started(ref scheduler) = *inner {
@@ -565,19 +573,28 @@ async fn fetch_idle(
.context("store_seen_flags_on_imap")?;
}
// Fetch the watched folder.
connection
.fetch_move_delete(ctx, &mut session, &watch_folder, folder_meaning)
.await
.context("fetch_move_delete")?;
if !ctx.should_delete_to_trash().await?
|| ctx
.get_config(Config::ConfiguredTrashFolder)
.await?
.is_some()
{
// Fetch the watched folder.
connection
.fetch_move_delete(ctx, &mut session, &watch_folder, folder_meaning)
.await
.context("fetch_move_delete")?;
// Mark expired messages for deletion. Marked messages will be deleted from the server
// on the next iteration of `fetch_move_delete`. `delete_expired_imap_messages` is not
// called right before `fetch_move_delete` because it is not well optimized and would
// otherwise slow down message fetching.
delete_expired_imap_messages(ctx)
.await
.context("delete_expired_imap_messages")?;
// Mark expired messages for deletion. Marked messages will be deleted from the server
// on the next iteration of `fetch_move_delete`. `delete_expired_imap_messages` is not
// called right before `fetch_move_delete` because it is not well optimized and would
// otherwise slow down message fetching.
delete_expired_imap_messages(ctx)
.await
.context("delete_expired_imap_messages")?;
} else if folder_config == Config::ConfiguredInboxFolder {
ctx.last_full_folder_scan.lock().await.take();
}
// Scan additional folders only after finishing fetching the watched folder.
//
@@ -911,6 +928,12 @@ impl Scheduler {
self.inbox.conn_state.interrupt();
}
fn interrupt_oboxes(&self) {
for b in &self.oboxes {
b.conn_state.interrupt();
}
}
fn interrupt_smtp(&self) {
self.smtp.interrupt();
}