diff --git a/deltachat-rpc-client/tests/test_folders.py b/deltachat-rpc-client/tests/test_folders.py index 8700f1b09..d2e60d5d1 100644 --- a/deltachat-rpc-client/tests/test_folders.py +++ b/deltachat-rpc-client/tests/test_folders.py @@ -26,70 +26,6 @@ def test_move_works(acfactory, direct_imap): assert msg.text == "message1" -def test_move_avoids_loop(acfactory, direct_imap): - """Test that the message is only moved from INBOX to DeltaChat. - - This is to avoid busy loop if moved message reappears in the Inbox - or some scanned folder later. - For example, this happens on servers that alias `INBOX.DeltaChat` to `DeltaChat` folder, - so the message moved to `DeltaChat` appears as a new message in the `INBOX.DeltaChat` folder. - We do not want to move this message from `INBOX.DeltaChat` to `DeltaChat` again. - """ - ac1, ac2 = acfactory.get_online_accounts(2) - ac2_direct_imap = direct_imap(ac2) - ac2_direct_imap.create_folder("DeltaChat") - ac2.set_config("mvbox_move", "1") - ac2.set_config("delete_server_after", "0") - ac2.bring_online() - - # Create INBOX.DeltaChat folder and make sure - # it is detected by full folder scan. - ac2_direct_imap = direct_imap(ac2) - ac2_direct_imap.create_folder("INBOX.DeltaChat") - ac2.stop_io() - ac2.start_io() - - while True: - event = ac2.wait_for_event() - # Wait until the end of folder scan. - if event.kind == EventType.INFO and "Found folders:" in event.msg: - break - - ac1_chat = acfactory.get_accepted_chat(ac1, ac2) - ac1_chat.send_text("Message 1") - - # Message is moved to the DeltaChat folder and downloaded. - ac2_msg1 = ac2.wait_for_incoming_msg().get_snapshot() - assert ac2_msg1.text == "Message 1" - - # Move the message to the INBOX.DeltaChat again. - # We assume that test server uses "." as the delimiter. - ac2_direct_imap.select_folder("DeltaChat") - ac2_direct_imap.conn.move(["*"], "INBOX.DeltaChat") - - ac1_chat.send_text("Message 2") - ac2_msg2 = ac2.wait_for_incoming_msg().get_snapshot() - assert ac2_msg2.text == "Message 2" - - # Stop and start I/O to trigger folder scan. - ac2.stop_io() - ac2.start_io() - while True: - event = ac2.wait_for_event() - # Wait until the end of folder scan. - if event.kind == EventType.INFO and "Found folders:" in event.msg: - break - - # Check that Message 1 is still in the INBOX.DeltaChat folder - # and Message 2 is in the DeltaChat folder. - ac2_direct_imap.select_folder("INBOX") - assert len(ac2_direct_imap.get_all_messages()) == 0 - ac2_direct_imap.select_folder("DeltaChat") - assert len(ac2_direct_imap.get_all_messages()) == 1 - ac2_direct_imap.select_folder("INBOX.DeltaChat") - assert len(ac2_direct_imap.get_all_messages()) == 1 - - def test_reactions_for_a_reordering_move(acfactory, direct_imap): """When a batch of messages is moved from Inbox to DeltaChat folder with a single MOVE command, their UIDs may be reordered (e.g. Gmail is known for that) which led to that messages were @@ -238,77 +174,6 @@ def test_markseen_message_and_mdn(acfactory, direct_imap, mvbox_move): assert len(list(ac2_direct_imap.conn.fetch(AND(seen=True), mark_seen=False))) == 1 -@pytest.mark.parametrize( - ("folder", "move", "expected_destination"), - [ - ( - "xyz", - False, - "xyz", - ), # Test that emails aren't found in a random folder - ( - "xyz", - True, - "xyz", - ), # ...emails are found in a random folder and downloaded without moving - ( - "Spam", - False, - "INBOX", - ), # ...emails are moved from the spam folder to the Inbox - ], -) -# Testrun.org does not support the CREATE-SPECIAL-USE capability, which means that we can't create a folder with -# the "\Junk" flag (see https://tools.ietf.org/html/rfc6154). So, we can't test spam folder detection by flag. -def test_scan_folders(acfactory, log, direct_imap, folder, move, expected_destination): - """Delta Chat periodically scans all folders for new messages to make sure we don't miss any.""" - variant = folder + "-" + str(move) + "-" + expected_destination - log.section("Testing variant " + variant) - ac1, ac2 = acfactory.get_online_accounts(2) - ac1.set_config("delete_server_after", "0") - if move: - ac1.set_config("mvbox_move", "1") - ac1.bring_online() - - ac1.stop_io() - ac1_direct_imap = direct_imap(ac1) - ac1_direct_imap.create_folder(folder) - - # Wait until each folder was selected once and we are IDLEing: - ac1.start_io() - ac1.bring_online() - - ac1.stop_io() - assert folder in ac1_direct_imap.list_folders() - - log.section("Send a message from ac2 to ac1 and manually move it to `folder`") - ac1_direct_imap.select_config_folder("inbox") - with ac1_direct_imap.idle() as idle1: - acfactory.get_accepted_chat(ac2, ac1).send_text("hello") - idle1.wait_for_new_message() - ac1_direct_imap.conn.move(["*"], folder) # "*" means "biggest UID in mailbox" - - log.section("start_io() and see if DeltaChat finds the message (" + variant + ")") - ac1.set_config("scan_all_folders_debounce_secs", "0") - ac1.start_io() - chat = ac1.create_chat(ac2) - n_msgs = 1 # "Messages are end-to-end encrypted." - if folder == "Spam": - msg = ac1.wait_for_incoming_msg().get_snapshot() - assert msg.text == "hello" - n_msgs += 1 - else: - ac1.wait_for_event(EventType.IMAP_INBOX_IDLE) - assert len(chat.get_messages()) == n_msgs - - # The message has reached its destination. - ac1_direct_imap.select_folder(expected_destination) - assert len(ac1_direct_imap.get_all_messages()) == 1 - if folder != expected_destination: - ac1_direct_imap.select_folder(folder) - assert len(ac1_direct_imap.get_all_messages()) == 0 - - def test_trash_multiple_messages(acfactory, direct_imap, log): ac1, ac2 = acfactory.get_online_accounts(2) ac2.stop_io() diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index e6205d446..236da0cea 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -932,7 +932,6 @@ def test_set_get_group_image(acfactory, data, lp): def test_connectivity(acfactory, lp): ac1, ac2 = acfactory.get_online_accounts(2) - ac1.set_config("scan_all_folders_debounce_secs", "0") ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_CONNECTED) diff --git a/src/config.rs b/src/config.rs index d7757144d..ef7bf3269 100644 --- a/src/config.rs +++ b/src/config.rs @@ -328,10 +328,6 @@ pub enum Config { /// Timestamp of the last `CantDecryptOutgoingMsgs` notification. LastCantDecryptOutgoingMsgs, - /// To how many seconds to debounce scan_all_folders. Used mainly in tests, to disable debouncing completely. - #[strum(props(default = "60"))] - ScanAllFoldersDebounceSecs, - /// Whether to avoid using IMAP IDLE even if the server supports it. /// /// This is a developer option for testing "fake idle". diff --git a/src/context.rs b/src/context.rs index 02a1604da..22b12bafa 100644 --- a/src/context.rs +++ b/src/context.rs @@ -999,12 +999,6 @@ impl Context { .await? .to_string(), ); - res.insert( - "scan_all_folders_debounce_secs", - self.get_config_int(Config::ScanAllFoldersDebounceSecs) - .await? - .to_string(), - ); res.insert( "quota_exceeding", self.get_config_int(Config::QuotaExceeding) diff --git a/src/imap.rs b/src/imap.rs index b5c86f1c9..4365cb6a2 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -53,7 +53,6 @@ use crate::transport::{ pub(crate) mod capabilities; mod client; mod idle; -pub mod scan_folders; pub mod select_folder; pub(crate) mod session; @@ -2421,5 +2420,23 @@ impl std::fmt::Display for UidRange { } } +pub(crate) async fn get_watched_folder_configs(context: &Context) -> Result> { + let mut res = vec![Config::ConfiguredInboxFolder]; + if context.should_watch_mvbox().await? { + res.push(Config::ConfiguredMvboxFolder); + } + Ok(res) +} + +pub(crate) async fn get_watched_folders(context: &Context) -> Result> { + let mut res = Vec::new(); + for folder_config in get_watched_folder_configs(context).await? { + if let Some(folder) = context.get_config(folder_config).await? { + res.push(folder); + } + } + Ok(res) +} + #[cfg(test)] mod imap_tests; diff --git a/src/imap/scan_folders.rs b/src/imap/scan_folders.rs deleted file mode 100644 index 389347780..000000000 --- a/src/imap/scan_folders.rs +++ /dev/null @@ -1,108 +0,0 @@ -use std::collections::BTreeMap; - -use anyhow::{Context as _, Result}; - -use super::{get_folder_meaning_by_attrs, get_folder_meaning_by_name}; -use crate::config::Config; -use crate::imap::{Imap, session::Session}; -use crate::log::LogExt; -use crate::tools::{self, time_elapsed}; -use crate::{context::Context, imap::FolderMeaning}; - -impl Imap { - /// Returns true if folders were scanned, false if scanning was postponed. - pub(crate) async fn scan_folders( - &mut self, - context: &Context, - session: &mut Session, - ) -> Result { - // First of all, debounce to once per minute: - { - let mut last_scan = session.last_full_folder_scan.lock().await; - if let Some(last_scan) = *last_scan { - let elapsed_secs = time_elapsed(&last_scan).as_secs(); - let debounce_secs = context - .get_config_u64(Config::ScanAllFoldersDebounceSecs) - .await?; - - if elapsed_secs < debounce_secs { - return Ok(false); - } - } - - // Update the timestamp before scanning the folders - // to avoid holding the lock for too long. - // This means next scan is delayed even if - // the current one fails. - last_scan.replace(tools::Time::now()); - } - info!(context, "Starting full folder scan"); - - let folders = session.list_folders().await?; - let watched_folders = get_watched_folders(context).await?; - - let mut folder_configs = BTreeMap::new(); - let mut folder_names = Vec::new(); - - for folder in folders { - let folder_meaning = get_folder_meaning_by_attrs(folder.attributes()); - if folder_meaning == FolderMeaning::Virtual { - // Gmail has virtual folders that should be skipped. For example, - // emails appear in the inbox and under "All Mail" as soon as it is - // received. The code used to wrongly conclude that the email had - // already been moved and left it in the inbox. - continue; - } - folder_names.push(folder.name().to_string()); - let folder_name_meaning = get_folder_meaning_by_name(folder.name()); - - if let Some(config) = folder_meaning.to_config() { - // Always takes precedence - folder_configs.insert(config, folder.name().to_string()); - } else if let Some(config) = folder_name_meaning.to_config() { - // only set if none has been already set - folder_configs - .entry(config) - .or_insert_with(|| folder.name().to_string()); - } - - let folder_meaning = match folder_meaning { - FolderMeaning::Unknown => folder_name_meaning, - _ => folder_meaning, - }; - - // Don't scan folders that are watched anyway - if !watched_folders.contains(&folder.name().to_string()) - && folder_meaning != FolderMeaning::Trash - && folder_meaning != FolderMeaning::Unknown - { - self.fetch_move_delete(context, session, folder.name(), folder_meaning) - .await - .context("Can't fetch new msgs in scanned folder") - .log_err(context) - .ok(); - } - } - - info!(context, "Found folders: {folder_names:?}."); - Ok(true) - } -} - -pub(crate) async fn get_watched_folder_configs(context: &Context) -> Result> { - let mut res = vec![Config::ConfiguredInboxFolder]; - if context.should_watch_mvbox().await? { - res.push(Config::ConfiguredMvboxFolder); - } - Ok(res) -} - -pub(crate) async fn get_watched_folders(context: &Context) -> Result> { - let mut res = Vec::new(); - for folder_config in get_watched_folder_configs(context).await? { - if let Some(folder) = context.get_config(folder_config).await? { - res.push(folder); - } - } - Ok(res) -} diff --git a/src/imap/session.rs b/src/imap/session.rs index 9d0943086..583fc51ff 100644 --- a/src/imap/session.rs +++ b/src/imap/session.rs @@ -5,11 +5,9 @@ use anyhow::{Context as _, Result}; use async_imap::Session as ImapSession; use async_imap::types::Mailbox; use futures::TryStreamExt; -use tokio::sync::Mutex; use crate::imap::capabilities::Capabilities; use crate::net::session::SessionStream; -use crate::tools; /// Prefetch: /// - Message-ID to check if we already have the message. @@ -46,8 +44,6 @@ pub(crate) struct Session { pub selected_folder_needs_expunge: bool, - pub(crate) last_full_folder_scan: Mutex>, - /// True if currently selected folder has new messages. /// /// Should be false if no folder is currently selected. @@ -84,7 +80,6 @@ impl Session { selected_folder: None, selected_mailbox: None, selected_folder_needs_expunge: false, - last_full_folder_scan: Mutex::new(None), new_mail: false, resync_request_sender, } diff --git a/src/quota.rs b/src/quota.rs index f8c70d71d..ba6adbf01 100644 --- a/src/quota.rs +++ b/src/quota.rs @@ -9,7 +9,7 @@ use async_imap::types::{Quota, QuotaResource}; use crate::chat::add_device_msg_with_importance; use crate::config::Config; use crate::context::Context; -use crate::imap::scan_folders::get_watched_folders; +use crate::imap::get_watched_folders; use crate::imap::session::Session as ImapSession; use crate::log::warn; use crate::message::Message; diff --git a/src/scheduler.rs b/src/scheduler.rs index 7546f4f48..b69d360e3 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -530,38 +530,6 @@ async fn fetch_idle( .await .context("download_msgs")?; - // Scan additional folders only after finishing fetching the watched folder. - // - // On iOS the application has strictly limited time to work in background, so we may not - // be able to scan all folders before time is up if there are many of them. - if folder_config == Config::ConfiguredInboxFolder { - // Only scan on the Inbox thread in order to prevent parallel scans, which might lead to duplicate messages - match connection - .scan_folders(ctx, &mut session) - .await - .context("scan_folders") - { - Err(err) => { - // Don't reconnect, if there is a problem with the connection we will realize this when IDLEing - // but maybe just one folder can't be selected or something - warn!(ctx, "{:#}", err); - } - Ok(true) => { - // Fetch the watched folder again in case scanning other folder moved messages - // there. - // - // In most cases this will select the watched folder and return because there are - // no new messages. We want to select the watched folder anyway before going IDLE - // there, so this does not take additional protocol round-trip. - connection - .fetch_move_delete(ctx, &mut session, &watch_folder, folder_meaning) - .await - .context("fetch_move_delete after scan_folders")?; - } - Ok(false) => {} - } - } - // Synchronize Seen flags. session .sync_seen_flags(ctx, &watch_folder) diff --git a/src/scheduler/connectivity.rs b/src/scheduler/connectivity.rs index 414d0ed2c..78cd8f6c9 100644 --- a/src/scheduler/connectivity.rs +++ b/src/scheduler/connectivity.rs @@ -6,7 +6,7 @@ use anyhow::Result; use humansize::{BINARY, format_size}; use crate::events::EventType; -use crate::imap::{FolderMeaning, scan_folders::get_watched_folder_configs}; +use crate::imap::{FolderMeaning, get_watched_folder_configs}; use crate::quota::{QUOTA_ERROR_THRESHOLD_PERCENTAGE, QUOTA_WARN_THRESHOLD_PERCENTAGE}; use crate::stock_str; use crate::{context::Context, log::LogExt};