feat: Remove "jobs" from imap_markseen if folder doesn't exist (#5870)

Add a `create` param to `select_with_uidvalidity()` instead of always trying to create the folder
and return `Ok(false)` from it if the folder doesn't exist and shouldn't be created, and handle this
in `store_seen_flags_on_imap()` by just removing "jobs" from the `imap_markseen` table. Also don't
create the folder in other code paths where it's not necessary.
This commit is contained in:
iequidoo
2024-08-18 17:49:29 -03:00
committed by iequidoo
parent f1302c3bc4
commit 3d9aee1368
6 changed files with 133 additions and 68 deletions

View File

@@ -452,8 +452,9 @@ async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result<Configure
imap.configure_folders(ctx, &mut imap_session, create_mvbox) imap.configure_folders(ctx, &mut imap_session, create_mvbox)
.await?; .await?;
let create = true;
imap_session imap_session
.select_with_uidvalidity(ctx, "INBOX") .select_with_uidvalidity(ctx, "INBOX", create)
.await .await
.context("could not read INBOX status")?; .context("could not read INBOX status")?;

View File

@@ -3,7 +3,7 @@
use std::cmp::max; use std::cmp::max;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use anyhow::{anyhow, bail, Result}; use anyhow::{anyhow, bail, ensure, Result};
use deltachat_derive::{FromSql, ToSql}; use deltachat_derive::{FromSql, ToSql};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@@ -201,7 +201,11 @@ impl Session {
bail!("Attempt to fetch UID 0"); bail!("Attempt to fetch UID 0");
} }
self.select_with_uidvalidity(context, folder).await?; let create = false;
let folder_exists = self
.select_with_uidvalidity(context, folder, create)
.await?;
ensure!(folder_exists, "No folder {folder}");
// we are connected, and the folder is selected // we are connected, and the folder is selected
info!(context, "Downloading message {}/{} fully...", folder, uid); info!(context, "Downloading message {}/{} fully...", folder, uid);

View File

@@ -13,7 +13,7 @@ use std::{
time::{Duration, UNIX_EPOCH}, time::{Duration, UNIX_EPOCH},
}; };
use anyhow::{bail, format_err, Context as _, Result}; use anyhow::{bail, ensure, format_err, Context as _, Result};
use async_channel::Receiver; use async_channel::Receiver;
use async_imap::types::{Fetch, Flag, Name, NameAttribute, UnsolicitedResponse}; use async_imap::types::{Fetch, Flag, Name, NameAttribute, UnsolicitedResponse};
use deltachat_contact_tools::ContactAddress; use deltachat_contact_tools::ContactAddress;
@@ -540,10 +540,14 @@ impl Imap {
return Ok(false); return Ok(false);
} }
session let create = false;
.select_with_uidvalidity(context, folder) let folder_exists = session
.select_with_uidvalidity(context, folder, create)
.await .await
.with_context(|| format!("Failed to select folder {folder:?}"))?; .with_context(|| format!("Failed to select folder {folder:?}"))?;
if !folder_exists {
return Ok(false);
}
if !session.new_mail && !fetch_existing_msgs { if !session.new_mail && !fetch_existing_msgs {
info!(context, "No new emails in folder {folder:?}."); info!(context, "No new emails in folder {folder:?}.");
@@ -835,45 +839,52 @@ impl Session {
folder: &str, folder: &str,
folder_meaning: FolderMeaning, folder_meaning: FolderMeaning,
) -> Result<()> { ) -> Result<()> {
let uid_validity;
// Collect pairs of UID and Message-ID. // Collect pairs of UID and Message-ID.
let mut msgs = BTreeMap::new(); let mut msgs = BTreeMap::new();
self.select_with_uidvalidity(context, folder).await?; let create = false;
let folder_exists = self
.select_with_uidvalidity(context, folder, create)
.await?;
if folder_exists {
let mut list = self
.uid_fetch("1:*", RFC724MID_UID)
.await
.with_context(|| format!("Can't resync folder {folder}"))?;
while let Some(fetch) = list.try_next().await? {
let headers = match get_fetch_headers(&fetch) {
Ok(headers) => headers,
Err(err) => {
warn!(context, "Failed to parse FETCH headers: {}", err);
continue;
}
};
let message_id = prefetch_get_message_id(&headers);
let mut list = self if let (Some(uid), Some(rfc724_mid)) = (fetch.uid, message_id) {
.uid_fetch("1:*", RFC724MID_UID) msgs.insert(
.await uid,
.with_context(|| format!("can't resync folder {folder}"))?; (
while let Some(fetch) = list.try_next().await? { rfc724_mid,
let headers = match get_fetch_headers(&fetch) { target_folder(context, folder, folder_meaning, &headers).await?,
Ok(headers) => headers, ),
Err(err) => { );
warn!(context, "Failed to parse FETCH headers: {}", err);
continue;
} }
};
let message_id = prefetch_get_message_id(&headers);
if let (Some(uid), Some(rfc724_mid)) = (fetch.uid, message_id) {
msgs.insert(
uid,
(
rfc724_mid,
target_folder(context, folder, folder_meaning, &headers).await?,
),
);
} }
info!(
context,
"resync_folder_uids: Collected {} message IDs in {folder}.",
msgs.len(),
);
uid_validity = get_uidvalidity(context, folder).await?;
} else {
warn!(context, "resync_folder_uids: No folder {folder}.");
uid_validity = 0;
} }
info!(
context,
"Resync: collected {} message IDs in folder {}",
msgs.len(),
folder,
);
let uid_validity = get_uidvalidity(context, folder).await?;
// Write collected UIDs to SQLite database. // Write collected UIDs to SQLite database.
context context
.sql .sql
@@ -1039,7 +1050,11 @@ impl Session {
// MOVE/DELETE operations. This does not result in multiple SELECT commands // MOVE/DELETE operations. This does not result in multiple SELECT commands
// being sent because `select_folder()` does nothing if the folder is already // being sent because `select_folder()` does nothing if the folder is already
// selected. // selected.
self.select_with_uidvalidity(context, folder).await?; let create = false;
let folder_exists = self
.select_with_uidvalidity(context, folder, create)
.await?;
ensure!(folder_exists, "No folder {folder}");
// Empty target folder name means messages should be deleted. // Empty target folder name means messages should be deleted.
if target.is_empty() { if target.is_empty() {
@@ -1133,30 +1148,40 @@ impl Session {
.await?; .await?;
for (folder, rowid_set, uid_set) in UidGrouper::from(rows) { for (folder, rowid_set, uid_set) in UidGrouper::from(rows) {
if let Err(err) = self.select_with_uidvalidity(context, &folder).await { let create = false;
warn!(context, "store_seen_flags_on_imap: Failed to select {folder}, will retry later: {err:#}."); let folder_exists = match self.select_with_uidvalidity(context, &folder, create).await {
Err(err) => {
warn!(
context,
"store_seen_flags_on_imap: Failed to select {folder}, will retry later: {err:#}.");
continue;
}
Ok(folder_exists) => folder_exists,
};
if !folder_exists {
warn!(context, "store_seen_flags_on_imap: No folder {folder}.");
} else if let Err(err) = self.add_flag_finalized_with_set(&uid_set, "\\Seen").await { } else if let Err(err) = self.add_flag_finalized_with_set(&uid_set, "\\Seen").await {
warn!( warn!(
context, context,
"Cannot mark messages {uid_set} in {folder} as seen, will retry later: {err:#}."); "Cannot mark messages {uid_set} in {folder} as seen, will retry later: {err:#}.");
continue;
} else { } else {
info!( info!(
context, context,
"Marked messages {} in folder {} as seen.", uid_set, folder "Marked messages {} in folder {} as seen.", uid_set, folder
); );
context
.sql
.transaction(|transaction| {
let mut stmt =
transaction.prepare("DELETE FROM imap_markseen WHERE id = ?")?;
for rowid in rowid_set {
stmt.execute((rowid,))?;
}
Ok(())
})
.await
.context("Cannot remove messages marked as seen from imap_markseen table")?;
} }
context
.sql
.transaction(|transaction| {
let mut stmt = transaction.prepare("DELETE FROM imap_markseen WHERE id = ?")?;
for rowid in rowid_set {
stmt.execute((rowid,))?;
}
Ok(())
})
.await
.context("Cannot remove messages marked as seen from imap_markseen table")?;
} }
Ok(()) Ok(())
@@ -1172,9 +1197,14 @@ impl Session {
return Ok(()); return Ok(());
} }
self.select_with_uidvalidity(context, folder) let create = false;
let folder_exists = self
.select_with_uidvalidity(context, folder, create)
.await .await
.context("failed to select folder")?; .context("Failed to select folder")?;
if !folder_exists {
return Ok(());
}
let mailbox = self let mailbox = self
.selected_mailbox .selected_mailbox
@@ -1630,7 +1660,7 @@ fn format_setmetadata(folder: &str, device_token: &str) -> String {
impl Session { impl Session {
/// Returns success if we successfully set the flag or we otherwise /// Returns success if we successfully set the flag or we otherwise
/// think add_flag should not be retried: Disconnection during setting /// think add_flag should not be retried: Disconnection during setting
/// the flag, or other imap-errors, returns true as well. /// the flag, or other imap-errors, returns Ok as well.
/// ///
/// Returning error means that the operation can be retried. /// Returning error means that the operation can be retried.
async fn add_flag_finalized_with_set(&mut self, uid_set: &str, flag: &str) -> Result<()> { async fn add_flag_finalized_with_set(&mut self, uid_set: &str, flag: &str) -> Result<()> {
@@ -1677,7 +1707,11 @@ impl Session {
self.close().await?; self.close().await?;
// Before moving emails to the mvbox we need to remember its UIDVALIDITY, otherwise // Before moving emails to the mvbox we need to remember its UIDVALIDITY, otherwise
// emails moved before that wouldn't be fetched but considered "old" instead. // emails moved before that wouldn't be fetched but considered "old" instead.
self.select_with_uidvalidity(context, folder).await?; let create = false;
let folder_exists = self
.select_with_uidvalidity(context, folder, create)
.await?;
ensure!(folder_exists, "No MVBOX folder {:?}??", &folder);
return Ok(Some(folder)); return Ok(Some(folder));
} }
} }
@@ -1688,7 +1722,10 @@ impl Session {
// Some servers require namespace-style folder names like "INBOX.DeltaChat", so we try all // Some servers require namespace-style folder names like "INBOX.DeltaChat", so we try all
// the variants here. // the variants here.
for folder in folders { for folder in folders {
match self.select_with_uidvalidity(context, folder).await { match self
.select_with_uidvalidity(context, folder, create_mvbox)
.await
{
Ok(_) => { Ok(_) => {
info!(context, "MVBOX-folder {} created.", folder); info!(context, "MVBOX-folder {} created.", folder);
return Ok(Some(folder)); return Ok(Some(folder));
@@ -2539,10 +2576,14 @@ async fn add_all_recipients_as_contacts(
); );
return Ok(()); return Ok(());
}; };
session let create = false;
.select_with_uidvalidity(context, &mailbox) let folder_exists = session
.select_with_uidvalidity(context, &mailbox, create)
.await .await
.with_context(|| format!("could not select {mailbox}"))?; .with_context(|| format!("could not select {mailbox}"))?;
if !folder_exists {
return Ok(());
}
let recipients = session let recipients = session
.get_all_recipients(context) .get_all_recipients(context)

View File

@@ -29,7 +29,9 @@ impl Session {
) -> Result<Self> { ) -> Result<Self> {
use futures::future::FutureExt; use futures::future::FutureExt;
self.select_with_uidvalidity(context, folder).await?; let create = true;
self.select_with_uidvalidity(context, folder, create)
.await?;
if self.drain_unsolicited_responses(context)? { if self.drain_unsolicited_responses(context)? {
self.new_mail = true; self.new_mail = true;

View File

@@ -34,6 +34,7 @@ impl Imap {
let watched_folders = get_watched_folders(context).await?; let watched_folders = get_watched_folders(context).await?;
let mut folder_configs = BTreeMap::new(); let mut folder_configs = BTreeMap::new();
let mut folder_names = Vec::new();
for folder in folders { for folder in folders {
let folder_meaning = get_folder_meaning_by_attrs(folder.attributes()); let folder_meaning = get_folder_meaning_by_attrs(folder.attributes());
@@ -44,6 +45,7 @@ impl Imap {
// already been moved and left it in the inbox. // already been moved and left it in the inbox.
continue; continue;
} }
folder_names.push(folder.name().to_string());
let folder_name_meaning = get_folder_meaning_by_name(folder.name()); let folder_name_meaning = get_folder_meaning_by_name(folder.name());
if let Some(config) = folder_meaning.to_config() { if let Some(config) = folder_meaning.to_config() {
@@ -91,6 +93,7 @@ impl Imap {
} }
} }
info!(context, "Found folders: {folder_names:?}.");
last_scan.replace(tools::Time::now()); last_scan.replace(tools::Time::now());
Ok(true) Ok(true)
} }

View File

@@ -111,7 +111,8 @@ impl ImapSession {
} }
} }
/// Selects a folder and takes care of UIDVALIDITY changes. /// Selects a folder optionally creating it and takes care of UIDVALIDITY changes. Returns false
/// iff `folder` doesn't exist.
/// ///
/// When selecting a folder for the first time, sets the uid_next to the current /// When selecting a folder for the first time, sets the uid_next to the current
/// mailbox.uid_next so that no old emails are fetched. /// mailbox.uid_next so that no old emails are fetched.
@@ -123,11 +124,24 @@ impl ImapSession {
&mut self, &mut self,
context: &Context, context: &Context,
folder: &str, folder: &str,
) -> Result<()> { create: bool,
let newly_selected = self ) -> Result<bool> {
.select_or_create_folder(context, folder) let newly_selected = if create {
.await self.select_or_create_folder(context, folder)
.with_context(|| format!("failed to select or create folder {folder}"))?; .await
.with_context(|| format!("failed to select or create folder {folder}"))?
} else {
match self.select_folder(context, folder).await {
Ok(newly_selected) => newly_selected,
Err(err) => match err {
Error::NoFolder(..) => return Ok(false),
_ => {
return Err(err)
.with_context(|| format!("failed to select folder {folder}"))?
}
},
}
};
let mailbox = self let mailbox = self
.selected_mailbox .selected_mailbox
.as_mut() .as_mut()
@@ -199,7 +213,7 @@ impl ImapSession {
} }
} }
return Ok(()); return Ok(true);
} }
// UIDVALIDITY is modified, reset highest seen MODSEQ. // UIDVALIDITY is modified, reset highest seen MODSEQ.
@@ -233,7 +247,7 @@ impl ImapSession {
old_uid_next, old_uid_next,
old_uid_validity, old_uid_validity,
); );
Ok(()) Ok(true)
} }
} }