diff --git a/CHANGELOG.md b/CHANGELOG.md index df03616f3..010209ccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Fix a bug where a blocked contact could send a contact request #3218 - Make sure, videochat-room-names are always URL-safe #3231 - Try removing account folder multiple times in case of failure #3229 +- Ignore messages from all spam folders if there are many #3246 ### Changes diff --git a/python/src/deltachat/direct_imap.py b/python/src/deltachat/direct_imap.py index b8a2f2243..080d2e836 100644 --- a/python/src/deltachat/direct_imap.py +++ b/python/src/deltachat/direct_imap.py @@ -40,8 +40,6 @@ def dc_account_extra_configure(account): # We just deleted the folder, so we have to make DC forget about it, too if account.get_config("configured_sentbox_folder") == folder: account.set_config("configured_sentbox_folder", None) - if account.get_config("configured_spam_folder") == folder: - account.set_config("configured_spam_folder", None) setattr(account, "direct_imap", imap) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 66f3450b7..a152970a9 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -1468,6 +1468,15 @@ class TestOnlineAccount: Unknown message in Spam """.format(ac1.get_config("configured_addr"))) + ac1.direct_imap.append("Junk", """ + From: unknown.address@junk.org + Subject: subj + To: {} + Message-ID: + Content-Type: text/plain; charset=utf-8 + + Unknown message in Junk + """.format(ac1.get_config("configured_addr"))) ac1.set_config("scan_all_folders_debounce_secs", "0") lp.sec("All prepared, now let DC find the message") diff --git a/src/config.rs b/src/config.rs index 12c80de3c..d1c156f2f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -129,7 +129,6 @@ pub enum Config { ConfiguredInboxFolder, ConfiguredMvboxFolder, ConfiguredSentboxFolder, - ConfiguredSpamFolder, ConfiguredTimestamp, ConfiguredProvider, Configured, diff --git a/src/context.rs b/src/context.rs index e0a89af79..56f35c817 100644 --- a/src/context.rs +++ b/src/context.rs @@ -608,11 +608,6 @@ impl Context { Ok(mvbox.as_deref() == Some(folder_name)) } - pub async fn is_spam_folder(&self, folder_name: &str) -> Result { - let spam = self.get_config(Config::ConfiguredSpamFolder).await?; - Ok(spam.as_deref() == Some(folder_name)) - } - pub(crate) fn derive_blobdir(dbfile: &PathBuf) -> PathBuf { let mut blob_fname = OsString::new(); blob_fname.push(dbfile.file_name().unwrap_or_default()); diff --git a/src/imap.rs b/src/imap.rs index e52ffbd29..c6a72f2ba 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -131,7 +131,7 @@ impl FolderMeaning { fn to_config(self) -> Option { match self { FolderMeaning::Unknown => None, - FolderMeaning::Spam => Some(Config::ConfiguredSpamFolder), + FolderMeaning::Spam => None, FolderMeaning::Sent => Some(Config::ConfiguredSentboxFolder), FolderMeaning::Drafts => None, FolderMeaning::Other => None, @@ -512,7 +512,12 @@ impl Imap { /// /// Prefetches headers and downloads new message from the folder, moves messages away from the /// folder and deletes messages in the folder. - pub async fn fetch_move_delete(&mut self, context: &Context, watch_folder: &str) -> Result<()> { + pub async fn fetch_move_delete( + &mut self, + context: &Context, + watch_folder: &str, + is_spam_folder: bool, + ) -> Result<()> { if !context.sql.is_open().await { // probably shutdown bail!("IMAP operation attempted while it is torn down"); @@ -520,7 +525,7 @@ impl Imap { self.prepare(context).await?; let msgs_fetched = self - .fetch_new_messages(context, watch_folder, false) + .fetch_new_messages(context, watch_folder, is_spam_folder, false) .await .context("fetch_new_messages")?; if msgs_fetched && context.get_config_delete_device_after().await?.is_some() { @@ -734,9 +739,10 @@ impl Imap { &mut self, context: &Context, folder: &str, + is_spam_folder: bool, fetch_existing_msgs: bool, ) -> Result { - if should_ignore_folder(context, folder).await? { + if should_ignore_folder(context, folder, is_spam_folder).await? { info!(context, "Not fetching from {}", folder); return Ok(false); } @@ -779,7 +785,7 @@ impl Imap { // Get the Message-ID or generate a fake one to identify the message in the database. let message_id = prefetch_get_message_id(&headers).unwrap_or_else(dc_create_id); - let target = match target_folder(context, folder, &headers).await? { + let target = match target_folder(context, folder, is_spam_folder, &headers).await? { Some(config) => match context.get_config(config).await? { Some(target) => target, None => folder.to_string(), @@ -810,7 +816,7 @@ impl Imap { // If the sender is known, the message will be moved to the Inbox or Mvbox // and then we download the message from there. // Also see `spam_target_folder()`. - && !context.is_spam_folder(folder).await? + && !is_spam_folder && prefetch_should_download( context, &headers, @@ -1730,13 +1736,14 @@ async fn spam_target_folder( pub async fn target_folder( context: &Context, folder: &str, + is_spam_folder: bool, headers: &[mailparse::MailHeader<'_>], ) -> Result> { if context.is_mvbox(folder).await? { return Ok(None); } - if context.is_spam_folder(folder).await? { + if is_spam_folder { spam_target_folder(context, headers).await } else if needs_move_to_mvbox(context, headers).await? { Ok(Some(Config::ConfiguredMvboxFolder)) @@ -2182,7 +2189,11 @@ pub async fn get_config_last_seen_uid(context: &Context, folder: &str) -> Result /// /// This caters for the [`Config::OnlyFetchMvbox`] setting which means mails from folders /// not explicitly watched should not be fetched. -async fn should_ignore_folder(context: &Context, folder: &str) -> Result { +async fn should_ignore_folder( + context: &Context, + folder: &str, + is_spam_folder: bool, +) -> Result { if !context.get_config_bool(Config::OnlyFetchMvbox).await? { return Ok(false); } @@ -2190,7 +2201,7 @@ async fn should_ignore_folder(context: &Context, folder: &str) -> Result { // Still respect the SentboxWatch setting. return Ok(!context.get_config_bool(Config::SentboxWatch).await?); } - Ok(!(context.is_mvbox(folder).await? || context.is_spam_folder(folder).await?)) + Ok(!(context.is_mvbox(folder).await? || is_spam_folder)) } /// Builds a list of sequence/uid sets. The returned sets have each no more than around 1000 @@ -2366,9 +2377,6 @@ mod tests { folder, mvbox_move, chat_msg, accepted_chat, outgoing, setupmessage); let t = TestContext::new_alice().await; - t.ctx - .set_config(Config::ConfiguredSpamFolder, Some("Spam")) - .await?; t.ctx .set_config(Config::ConfiguredMvboxFolder, Some("DeltaChat")) .await?; @@ -2410,11 +2418,13 @@ mod tests { let (headers, _) = mailparse::parse_headers(bytes)?; - let actual = if let Some(config) = target_folder(&t, folder, &headers).await? { - t.get_config(config).await? - } else { - None - }; + let is_spam_folder = folder == "Spam"; + let actual = + if let Some(config) = target_folder(&t, folder, is_spam_folder, &headers).await? { + t.get_config(config).await? + } else { + None + }; let expected = if expected_destination == folder { None diff --git a/src/imap/idle.rs b/src/imap/idle.rs index f325c3648..dcf5ee6f4 100644 --- a/src/imap/idle.rs +++ b/src/imap/idle.rs @@ -157,7 +157,10 @@ impl Imap { // in anything. If so, we behave as if IDLE had data but // will have already fetched the messages so perform_*_fetch // will not find any new. - match self.fetch_new_messages(context, &watch_folder, false).await { + match self + .fetch_new_messages(context, &watch_folder, false, false) + .await + { Ok(res) => { info!(context, "fetch_new_messages returned {:?}", res); if res { diff --git a/src/imap/scan_folders.rs b/src/imap/scan_folders.rs index 176a17519..e1c77bd04 100644 --- a/src/imap/scan_folders.rs +++ b/src/imap/scan_folders.rs @@ -60,6 +60,9 @@ impl Imap { let is_drafts = folder_meaning == FolderMeaning::Drafts || (folder_meaning == FolderMeaning::Unknown && folder_name_meaning == FolderMeaning::Drafts); + let is_spam_folder = folder_meaning == FolderMeaning::Spam + || (folder_meaning == FolderMeaning::Unknown + && folder_name_meaning == FolderMeaning::Spam); // Don't scan folders that are watched anyway if !watched_folders.contains(&folder.name().to_string()) && !is_drafts { @@ -67,7 +70,7 @@ impl Imap { self.server_sent_unsolicited_exists(context)?; loop { - self.fetch_move_delete(context, folder.name()) + self.fetch_move_delete(context, folder.name(), is_spam_folder) .await .ok_or_log_msg(context, "Can't fetch new msgs in scanned folder"); @@ -79,16 +82,15 @@ impl Imap { } } - // We iterate over both folder meanings to make sure that if e.g. the "Sent" folder was deleted, - // `ConfiguredSentboxFolder` is set to `None`: - for config in &[ - Config::ConfiguredSentboxFolder, - Config::ConfiguredSpamFolder, - ] { - context - .set_config(*config, folder_configs.get(config).map(|s| s.as_str())) - .await?; - } + // Set the `ConfiguredSentboxFolder` or set it to `None` if the folder was deleted. + context + .set_config( + Config::ConfiguredSentboxFolder, + folder_configs + .get(&Config::ConfiguredSentboxFolder) + .map(|s| s.as_str()), + ) + .await?; last_scan.replace(Instant::now()); Ok(true) diff --git a/src/job.rs b/src/job.rs index 9e6995639..65c4b1e73 100644 --- a/src/job.rs +++ b/src/job.rs @@ -335,7 +335,7 @@ impl Job { Config::ConfiguredSentboxFolder, ] { if let Some(folder) = job_try!(context.get_config(*config).await) { - if let Err(e) = imap.fetch_new_messages(context, &folder, true).await { + if let Err(e) = imap.fetch_new_messages(context, &folder, false, true).await { // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: warn!(context, "Could not fetch messages, retrying: {:#}", e); return Status::RetryLater; diff --git a/src/scheduler.rs b/src/scheduler.rs index 3bd6f1c73..725e1c6be 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -157,7 +157,10 @@ async fn fetch(ctx: &Context, connection: &mut Imap) { } // fetch - if let Err(err) = connection.fetch_move_delete(ctx, &watch_folder).await { + if let Err(err) = connection + .fetch_move_delete(ctx, &watch_folder, false) + .await + { connection.trigger_reconnect(ctx).await; warn!(ctx, "{:#}", err); } @@ -194,7 +197,10 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int } // Fetch the watched folder. - if let Err(err) = connection.fetch_move_delete(ctx, &watch_folder).await { + if let Err(err) = connection + .fetch_move_delete(ctx, &watch_folder, false) + .await + { connection.trigger_reconnect(ctx).await; warn!(ctx, "{:#}", err); return InterruptInfo::new(false); @@ -230,7 +236,10 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int // 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. - if let Err(err) = connection.fetch_move_delete(ctx, &watch_folder).await { + if let Err(err) = connection + .fetch_move_delete(ctx, &watch_folder, false) + .await + { connection.trigger_reconnect(ctx).await; warn!(ctx, "{:#}", err); return InterruptInfo::new(false);