fix: do not miss new messages while expunging the folder

This should fix flaky `test_verified_group_vs_delete_server_after`.
This commit is contained in:
link2xt
2024-06-04 15:41:39 +00:00
parent 216b295f52
commit 380116d107
5 changed files with 49 additions and 38 deletions

View File

@@ -184,7 +184,7 @@ impl Session {
bail!("Attempt to fetch UID 0");
}
self.select_folder(context, folder).await?;
self.select_with_uidvalidity(context, folder).await?;
// we are connected, and the folder is selected
info!(context, "Downloading message {}/{} fully...", folder, uid);

View File

@@ -543,15 +543,16 @@ impl Imap {
return Ok(false);
}
let new_emails = session
session
.select_with_uidvalidity(context, folder)
.await
.with_context(|| format!("Failed to select folder {folder:?}"))?;
if !new_emails && !fetch_existing_msgs {
if !session.new_mail && !fetch_existing_msgs {
info!(context, "No new emails in folder {folder:?}.");
return Ok(false);
}
session.new_mail = false;
let uid_validity = get_uidvalidity(context, folder).await?;
let old_uid_next = get_uid_next(context, folder).await?;
@@ -838,7 +839,7 @@ impl Session {
// Collect pairs of UID and Message-ID.
let mut msgs = BTreeMap::new();
self.select_folder(context, folder).await?;
self.select_with_uidvalidity(context, folder).await?;
let mut list = self
.uid_fetch("1:*", RFC724MID_UID)
@@ -1039,7 +1040,7 @@ impl Session {
// MOVE/DELETE operations. This does not result in multiple SELECT commands
// being sent because `select_folder()` does nothing if the folder is already
// selected.
self.select_folder(context, folder).await?;
self.select_with_uidvalidity(context, folder).await?;
// Empty target folder name means messages should be deleted.
if target.is_empty() {
@@ -1087,7 +1088,7 @@ impl Session {
.await?;
for (folder, rowid_set, uid_set) in UidGrouper::from(rows) {
self.select_folder(context, &folder)
self.select_with_uidvalidity(context, &folder)
.await
.context("failed to select folder")?;
@@ -1131,7 +1132,7 @@ impl Session {
return Ok(());
}
self.select_folder(context, folder)
self.select_with_uidvalidity(context, folder)
.await
.context("failed to select folder")?;

View File

@@ -29,9 +29,13 @@ impl Session {
) -> Result<Self> {
use futures::future::FutureExt;
self.select_folder(context, folder).await?;
self.select_with_uidvalidity(context, folder).await?;
if self.server_sent_unsolicited_exists(context)? {
self.new_mail = true;
}
if self.new_mail {
return Ok(self);
}
@@ -92,6 +96,9 @@ impl Session {
session.as_mut().set_read_timeout(Some(IMAP_TIMEOUT));
self.inner = session;
// Fetch mail once we exit IDLE.
self.new_mail = true;
Ok(self)
}
}

View File

@@ -39,6 +39,7 @@ impl ImapSession {
info!(context, "close/expunge succeeded");
self.selected_folder = None;
self.selected_folder_needs_expunge = false;
self.new_mail = false;
}
}
Ok(())
@@ -47,11 +48,7 @@ impl ImapSession {
/// Selects a folder, possibly updating uid_validity and, if needed,
/// expunging the folder to remove delete-marked messages.
/// Returns whether a new folder was selected.
pub(crate) async fn select_folder(
&mut self,
context: &Context,
folder: &str,
) -> Result<NewlySelected> {
async fn select_folder(&mut self, context: &Context, folder: &str) -> Result<NewlySelected> {
// if there is a new folder and the new folder is equal to the selected one, there's nothing to do.
// if there is _no_ new folder, we continue as we might want to expunge below.
if let Some(selected_folder) = &self.selected_folder {
@@ -119,13 +116,14 @@ impl ImapSession {
/// 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.
///
/// Returns Result<new_emails> (i.e. whether new emails arrived),
/// if in doubt, returns new_emails=true so emails are fetched.
/// Updates `self.new_mail` if folder was previously unselected
/// and new mails are detected after selecting,
/// i.e. UIDNEXT advanced while the folder was closed.
pub(crate) async fn select_with_uidvalidity(
&mut self,
context: &Context,
folder: &str,
) -> Result<bool> {
) -> Result<()> {
let newly_selected = self
.select_or_create_folder(context, folder)
.await
@@ -182,28 +180,26 @@ impl ImapSession {
mailbox.uid_next = new_uid_next;
if new_uid_validity == old_uid_validity {
let new_emails = if newly_selected == NewlySelected::No {
// The folder was not newly selected i.e. no SELECT command was run. This means that mailbox.uid_next
// was not updated and may contain an incorrect value. So, just return true so that
// the caller tries to fetch new messages (we could of course run a SELECT command now, but trying to fetch
// new messages is only one command, just as a SELECT command)
true
} else if let Some(new_uid_next) = new_uid_next {
if new_uid_next < old_uid_next {
warn!(
context,
"The server illegally decreased the uid_next of folder {folder:?} from {old_uid_next} to {new_uid_next} without changing validity ({new_uid_validity}), resyncing UIDs...",
);
set_uid_next(context, folder, new_uid_next).await?;
context.schedule_resync().await?;
}
new_uid_next != old_uid_next // If UIDNEXT changed, there are new emails
} else {
// We have no UIDNEXT and if in doubt, return true.
true
};
if newly_selected == NewlySelected::Yes {
if let Some(new_uid_next) = new_uid_next {
if new_uid_next < old_uid_next {
warn!(
context,
"The server illegally decreased the uid_next of folder {folder:?} from {old_uid_next} to {new_uid_next} without changing validity ({new_uid_validity}), resyncing UIDs...",
);
set_uid_next(context, folder, new_uid_next).await?;
context.schedule_resync().await?;
}
return Ok(new_emails);
// If UIDNEXT changed, there are new emails.
self.new_mail |= new_uid_next != old_uid_next;
} else {
warn!(context, "Folder {folder} was just selected but we failed to determine UIDNEXT, assume that it has new mail.");
self.new_mail = true;
}
}
return Ok(());
}
// UIDVALIDITY is modified, reset highest seen MODSEQ.
@@ -214,6 +210,7 @@ impl ImapSession {
let new_uid_next = new_uid_next.unwrap_or_default();
set_uid_next(context, folder, new_uid_next).await?;
set_uidvalidity(context, folder, new_uid_validity).await?;
self.new_mail = true;
// Collect garbage entries in `imap` table.
context
@@ -236,7 +233,7 @@ impl ImapSession {
old_uid_next,
old_uid_validity,
);
Ok(false)
Ok(())
}
}

View File

@@ -40,6 +40,11 @@ pub(crate) struct Session {
pub selected_mailbox: Option<Mailbox>,
pub selected_folder_needs_expunge: bool,
/// True if currently selected folder has new messages.
///
/// Should be false if no folder is currently selected.
pub new_mail: bool,
}
impl Deref for Session {
@@ -67,6 +72,7 @@ impl Session {
selected_folder: None,
selected_mailbox: None,
selected_folder_needs_expunge: false,
new_mail: false,
}
}