From e9cfcd9d1b4bd0fd70c5773728885e060c0a7bd5 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 4 Apr 2024 04:36:01 -0300 Subject: [PATCH] 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. --- python/tests/test_1_online.py | 21 ++++++++++------ src/imap/scan_folders.rs | 13 +++++++--- src/scheduler.rs | 47 ++++++++++++++++++++++++++--------- 3 files changed, 59 insertions(+), 22 deletions(-) diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index a3185db82..12cc98412 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -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 diff --git a/src/imap/scan_folders.rs b/src/imap/scan_folders.rs index 1ad74e84e..a4e17d04d 100644 --- a/src/imap/scan_folders.rs +++ b/src/imap/scan_folders.rs @@ -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()); diff --git a/src/scheduler.rs b/src/scheduler.rs index 9a3592fe7..a621f960c 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -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(); }