Skip to content

Commit

Permalink
refactor(imap): remove Session from Imap structure
Browse files Browse the repository at this point in the history
Connection establishment now happens only in one place in each IMAP loop.
Now all connection establishment happens in one place
and is limited by the ratelimit.

Backoff was removed from fake_idle
as it does not establish connections anymore.
If connection fails, fake_idle will return an error.
We then drop the connection and get back to the beginning of IMAP
loop.

Backoff may be still nice to have to delay retries
in case of constant connection failures
so we don't immediately hit ratelimit if the network is unusable
and returns immediate error on each connection attempt
(e.g. ICMP network unreachable error),
but adding backoff for connection failures is out of scope for this change.
  • Loading branch information
link2xt committed Mar 1, 2024
1 parent b08a4d6 commit 07870a6
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 381 deletions.
19 changes: 9 additions & 10 deletions src/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use tokio::task;
use crate::config::{self, Config};
use crate::contact::addr_cmp;
use crate::context::Context;
use crate::imap::Imap;
use crate::imap::{session::Session as ImapSession, Imap};
use crate::log::LogExt;
use crate::login_param::{CertificateChecks, LoginParam, ServerLoginParam};
use crate::message::{Message, Viewtype};
Expand Down Expand Up @@ -395,7 +395,7 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {

// Configure IMAP

let mut imap: Option<Imap> = None;
let mut imap: Option<(Imap, ImapSession)> = None;
let imap_servers: Vec<&ServerParams> = servers
.iter()
.filter(|params| params.protocol == Protocol::Imap)
Expand Down Expand Up @@ -433,7 +433,7 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
600 + (800 - 600) * (1 + imap_server_index) / imap_servers_count
);
}
let mut imap = match imap {
let (mut imap, mut imap_session) = match imap {
Some(imap) => imap,
None => bail!(nicer_configuration_error(ctx, errors).await),
};
Expand All @@ -454,11 +454,10 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {

let create_mvbox = ctx.should_watch_mvbox().await?;

imap.configure_folders(ctx, create_mvbox).await?;
imap.configure_folders(ctx, &mut imap_session, create_mvbox)
.await?;

imap.session
.as_mut()
.context("no IMAP connection established")?
imap_session
.select_with_uidvalidity(ctx, "INBOX")
.await
.context("could not read INBOX status")?;
Expand Down Expand Up @@ -579,7 +578,7 @@ async fn try_imap_one_param(
socks5_config: &Option<Socks5Config>,
addr: &str,
provider_strict_tls: bool,
) -> Result<Imap, ConfigurationError> {
) -> Result<(Imap, ImapSession), ConfigurationError> {
let inf = format!(
"imap: {}@{}:{} security={} certificate_checks={} oauth2={} socks5_config={}",
param.user,
Expand Down Expand Up @@ -617,9 +616,9 @@ async fn try_imap_one_param(
msg: format!("{err:#}"),
})
}
Ok(()) => {
Ok(session) => {
info!(context, "success: {}", inf);
Ok(imap)
Ok((imap, session))
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,13 @@ impl Context {

// connection
let mut connection = Imap::new_configured(self, channel::bounded(1).1).await?;
connection.prepare(self).await?;
let mut session = connection.prepare(self).await?;

// fetch imap folders
for folder_meaning in [FolderMeaning::Inbox, FolderMeaning::Mvbox] {
let (_, watch_folder) = convert_folder_meaning(self, folder_meaning).await?;
connection
.fetch_move_delete(self, &watch_folder, folder_meaning)
.fetch_move_delete(self, &mut session, &watch_folder, folder_meaning)
.await?;
}

Expand All @@ -484,10 +484,8 @@ impl Context {
};

if quota_needs_update {
if let Some(session) = connection.session.as_mut() {
if let Err(err) = self.update_recent_quota(session).await {
warn!(self, "Failed to update quota: {err:#}.");
}
if let Err(err) = self.update_recent_quota(&mut session).await {
warn!(self, "Failed to update quota: {err:#}.");
}
}

Expand Down
19 changes: 9 additions & 10 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};

use crate::config::Config;
use crate::context::Context;
use crate::imap::{Imap, ImapActionResult};
use crate::imap::{session::Session, ImapActionResult};
use crate::message::{Message, MsgId, Viewtype};
use crate::mimeparser::{MimeMessage, Part};
use crate::tools::time;
Expand Down Expand Up @@ -129,9 +129,11 @@ impl Message {
/// Actually download a message partially downloaded before.
///
/// Most messages are downloaded automatically on fetch instead.
pub(crate) async fn download_msg(context: &Context, msg_id: MsgId, imap: &mut Imap) -> Result<()> {
imap.prepare(context).await?;

pub(crate) async fn download_msg(
context: &Context,
msg_id: MsgId,
session: &mut Session,
) -> Result<()> {
let msg = Message::load_from_db(context, msg_id).await?;
let row = context
.sql
Expand All @@ -152,7 +154,7 @@ pub(crate) async fn download_msg(context: &Context, msg_id: MsgId, imap: &mut Im
return Err(anyhow!("Call download_full() again to try over."));
};

match imap
match session
.fetch_single_msg(
context,
&server_folder,
Expand All @@ -169,7 +171,7 @@ pub(crate) async fn download_msg(context: &Context, msg_id: MsgId, imap: &mut Im
}
}

impl Imap {
impl Session {
/// Download a single message and pipe it to receive_imf().
///
/// receive_imf() is not directly aware that this is a result of a call to download_msg(),
Expand All @@ -194,10 +196,7 @@ impl Imap {

let mut uid_message_ids: BTreeMap<u32, String> = BTreeMap::new();
uid_message_ids.insert(uid, rfc724_mid);
let Some(session) = self.session.as_mut() else {
return ImapActionResult::Failed;
};
let (last_uid, _received) = match session
let (last_uid, _received) = match self
.fetch_many_msgs(
context,
folder,
Expand Down
Loading

0 comments on commit 07870a6

Please sign in to comment.