From ddfadc748d73d0a5e0bd60b4a8aec9022836c604 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sun, 29 Nov 2020 15:34:30 +0200 Subject: [PATCH] melib/imap: don't fetch RFC822 except when requested In some cases when handling new server events, the entire body message was unnecessarily fetched. Closes #87 IMAP: don't fetch RFC822 except when requested --- melib/src/backends/imap/untagged.rs | 377 ++++++++++++++++------------ melib/src/backends/imap/watch.rs | 138 ++++++---- 2 files changed, 301 insertions(+), 214 deletions(-) diff --git a/melib/src/backends/imap/untagged.rs b/melib/src/backends/imap/untagged.rs index 9aeeeaa51..01ad1770a 100644 --- a/melib/src/backends/imap/untagged.rs +++ b/melib/src/backends/imap/untagged.rs @@ -28,7 +28,6 @@ use crate::backends::{ RefreshEvent, RefreshEventKind::{self, *}, }; -use crate::email::Envelope; use crate::error::*; use std::convert::TryInto; @@ -196,94 +195,120 @@ impl ImapConnection { debug!("exists {}", n); try_fail!( mailbox_hash, - self.send_command(format!("FETCH {} (UID FLAGS RFC822)", n).as_bytes()).await + self.send_command(format!("FETCH {} (UID FLAGS ENVELOPE BODY.PEEK[HEADER.FIELDS (REFERENCES)] BODYSTRUCTURE)", n).as_bytes()).await self.read_response(&mut response, RequiredResponses::FETCH_REQUIRED).await ); - match super::protocol_parser::fetch_responses(&response) { - Ok((_, v, _)) => { - 'fetch_responses: for FetchResponse { - uid, flags, body, .. - } in v - { - if uid.is_none() || flags.is_none() || body.is_none() { - continue; + let mut v = match super::protocol_parser::fetch_responses(&response) { + Ok((_, v, _)) => v, + Err(err) => { + debug!( + "Error when parsing FETCH response after untagged exists {:?}", + err + ); + return Ok(true); + } + }; + debug!("responses len is {}", v.len()); + for FetchResponse { + ref uid, + ref mut envelope, + ref mut flags, + ref references, + .. + } in &mut v + { + if uid.is_none() || flags.is_none() || envelope.is_none() { + continue; + } + let uid = uid.unwrap(); + let env = envelope.as_mut().unwrap(); + env.set_hash(generate_envelope_hash(&mailbox.imap_path(), &uid)); + if let Some(value) = references { + let parse_result = crate::email::parser::address::msg_id_list(value); + if let Ok((_, value)) = parse_result { + let prev_val = env.references.take(); + for v in value { + env.push_references(v); } - let uid = uid.unwrap(); - if self - .uid_store - .uid_index - .lock() - .unwrap() - .contains_key(&(mailbox_hash, uid)) - { - continue 'fetch_responses; - } - let env_hash = generate_envelope_hash(&mailbox.imap_path(), &uid); - self.uid_store - .msn_index - .lock() - .unwrap() - .entry(mailbox_hash) - .or_default() - .push(uid); - if let Ok(mut env) = - Envelope::from_bytes(body.unwrap(), flags.as_ref().map(|&(f, _)| f)) - { - env.set_hash(env_hash); - self.uid_store - .hash_index - .lock() - .unwrap() - .insert(env_hash, (uid, mailbox_hash)); - self.uid_store - .uid_index - .lock() - .unwrap() - .insert((mailbox_hash, uid), env_hash); - if let Some((_, keywords)) = flags { - let mut tag_lck = self.uid_store.tag_index.write().unwrap(); - for f in keywords { - let hash = tag_hash!(f); - if !tag_lck.contains_key(&hash) { - tag_lck.insert(hash, f); - } - env.labels_mut().push(hash); - } + if let Some(prev) = prev_val { + for v in prev.refs { + env.push_references(v); } - debug!( - "Create event {} {} {}", - env.hash(), - env.subject(), - mailbox.path(), - ); - if !env.is_seen() { - mailbox.unseen.lock().unwrap().insert_new(env.hash()); - } - mailbox.exists.lock().unwrap().insert_new(env.hash()); - let mut event: [(UID, RefreshEvent); 1] = [( - uid, - RefreshEvent { - account_hash: self.uid_store.account_hash, - mailbox_hash, - kind: Create(Box::new(env)), - }, - )]; - if self.uid_store.keep_offline_cache { - cache_handle.update(mailbox_hash, &event)?; - } - self.add_refresh_event(std::mem::replace( - &mut event[0].1, - RefreshEvent { - account_hash: self.uid_store.account_hash, - mailbox_hash, - kind: Rescan, - }, - )); } } + env.set_references(value); } - Err(e) => { - debug!(e); + let mut tag_lck = self.uid_store.tag_index.write().unwrap(); + if let Some((flags, keywords)) = flags { + env.set_flags(*flags); + if !env.is_seen() { + mailbox.unseen.lock().unwrap().insert_new(env.hash()); + } + for f in keywords { + let hash = tag_hash!(f); + if !tag_lck.contains_key(&hash) { + tag_lck.insert(hash, f.to_string()); + } + env.labels_mut().push(hash); + } + } + mailbox.exists.lock().unwrap().insert_new(env.hash()); + if !self + .uid_store + .uid_index + .lock() + .unwrap() + .contains_key(&(mailbox_hash, uid)) + { + self.uid_store + .msn_index + .lock() + .unwrap() + .entry(mailbox_hash) + .or_default() + .push(uid); + } + self.uid_store + .hash_index + .lock() + .unwrap() + .insert(env.hash(), (uid, mailbox_hash)); + self.uid_store + .uid_index + .lock() + .unwrap() + .insert((mailbox_hash, uid), env.hash()); + debug!( + "Create event {} {} {}", + env.hash(), + env.subject(), + mailbox.path(), + ); + } + if self.uid_store.keep_offline_cache { + if let Err(err) = cache_handle + .insert_envelopes(mailbox_hash, &v) + .chain_err_summary(|| { + format!( + "Could not save envelopes in cache for mailbox {}", + &mailbox.imap_path() + ) + }) + { + crate::log(err.to_string(), crate::INFO); + } + } + for response in v { + if let FetchResponse { + envelope: Some(envelope), + .. + } = response + { + self.add_refresh_event(RefreshEvent { + account_hash: self.uid_store.account_hash, + mailbox_hash, + kind: Create(Box::new(envelope)), + }); } } } @@ -308,96 +333,126 @@ impl ImapConnection { for ms in iter { accum = format!("{},{}", accum, to_str!(ms).trim()); } - format!("UID FETCH {} (FLAGS RFC822)", accum) + format!("UID FETCH {} (UID FLAGS ENVELOPE BODY.PEEK[HEADER.FIELDS (REFERENCES)] BODYSTRUCTURE)", accum) }; try_fail!( mailbox_hash, self.send_command(command.as_bytes()).await self.read_response(&mut response, RequiredResponses::FETCH_REQUIRED).await ); - debug!(&response); - match super::protocol_parser::fetch_responses(&response) { - Ok((_, v, _)) => { - for FetchResponse { - uid, flags, body, .. - } in v - { - if uid.is_none() || flags.is_none() || body.is_none() { - continue; + let mut v = match super::protocol_parser::fetch_responses(&response) { + Ok((_, v, _)) => v, + Err(err) => { + debug!( + "Error when parsing FETCH response after untagged recent {:?}", + err + ); + return Ok(true); + } + }; + debug!("responses len is {}", v.len()); + for FetchResponse { + ref uid, + ref mut envelope, + ref mut flags, + ref references, + .. + } in &mut v + { + if uid.is_none() || flags.is_none() || envelope.is_none() { + continue; + } + let uid = uid.unwrap(); + let env = envelope.as_mut().unwrap(); + env.set_hash(generate_envelope_hash(&mailbox.imap_path(), &uid)); + if let Some(value) = references { + let parse_result = + crate::email::parser::address::msg_id_list(value); + if let Ok((_, value)) = parse_result { + let prev_val = env.references.take(); + for v in value { + env.push_references(v); } - let uid = uid.unwrap(); - if !self - .uid_store - .uid_index - .lock() - .unwrap() - .contains_key(&(mailbox_hash, uid)) - { - if let Ok(mut env) = Envelope::from_bytes( - body.unwrap(), - flags.as_ref().map(|&(f, _)| f), - ) { - self.uid_store - .hash_index - .lock() - .unwrap() - .insert(env.hash(), (uid, mailbox_hash)); - self.uid_store - .uid_index - .lock() - .unwrap() - .insert((mailbox_hash, uid), env.hash()); - debug!( - "Create event {} {} {}", - env.hash(), - env.subject(), - mailbox.path(), - ); - if let Some((_, keywords)) = flags { - let mut tag_lck = - self.uid_store.tag_index.write().unwrap(); - for f in keywords { - let hash = tag_hash!(f); - if !tag_lck.contains_key(&hash) { - tag_lck.insert(hash, f); - } - env.labels_mut().push(hash); - } - } - if !env.is_seen() { - mailbox - .unseen - .lock() - .unwrap() - .insert_new(env.hash()); - } - - mailbox.exists.lock().unwrap().insert_new(env.hash()); - let mut event: [(UID, RefreshEvent); 1] = [( - uid, - RefreshEvent { - account_hash: self.uid_store.account_hash, - mailbox_hash, - kind: Create(Box::new(env)), - }, - )]; - if self.uid_store.keep_offline_cache { - cache_handle.update(mailbox_hash, &event)?; - } - self.add_refresh_event(std::mem::replace( - &mut event[0].1, - RefreshEvent { - account_hash: self.uid_store.account_hash, - mailbox_hash, - kind: Rescan, - }, - )); + if let Some(prev) = prev_val { + for v in prev.refs { + env.push_references(v); } } } + env.set_references(value); } - Err(e) => { - debug!(e); + let mut tag_lck = self.uid_store.tag_index.write().unwrap(); + if let Some((flags, keywords)) = flags { + env.set_flags(*flags); + if !env.is_seen() { + mailbox.unseen.lock().unwrap().insert_new(env.hash()); + } + for f in keywords { + let hash = tag_hash!(f); + if !tag_lck.contains_key(&hash) { + tag_lck.insert(hash, f.to_string()); + } + env.labels_mut().push(hash); + } + } + mailbox.exists.lock().unwrap().insert_new(env.hash()); + } + if self.uid_store.keep_offline_cache { + if let Err(err) = cache_handle + .insert_envelopes(mailbox_hash, &v) + .chain_err_summary(|| { + format!( + "Could not save envelopes in cache for mailbox {}", + &mailbox.imap_path() + ) + }) + { + crate::log(err.to_string(), crate::INFO); + } + } + for response in v { + if let FetchResponse { + envelope: Some(envelope), + uid: Some(uid), + .. + } = response + { + if !self + .uid_store + .uid_index + .lock() + .unwrap() + .contains_key(&(mailbox_hash, uid)) + { + self.uid_store + .msn_index + .lock() + .unwrap() + .entry(mailbox_hash) + .or_default() + .push(uid); + } + self.uid_store + .hash_index + .lock() + .unwrap() + .insert(envelope.hash(), (uid, mailbox_hash)); + self.uid_store + .uid_index + .lock() + .unwrap() + .insert((mailbox_hash, uid), envelope.hash()); + debug!( + "Create event {} {} {}", + envelope.hash(), + envelope.subject(), + mailbox.path(), + ); + self.add_refresh_event(RefreshEvent { + account_hash: self.uid_store.account_hash, + mailbox_hash, + kind: Create(Box::new(envelope)), + }); } } } diff --git a/melib/src/backends/imap/watch.rs b/melib/src/backends/imap/watch.rs index bbc23810e..b521970d5 100644 --- a/melib/src/backends/imap/watch.rs +++ b/melib/src/backends/imap/watch.rs @@ -267,14 +267,16 @@ pub async fn examine_updates( cmd.push_str(&n.to_string()); } } - cmd.push_str(" (UID FLAGS RFC822)"); + cmd.push_str( + " (UID FLAGS ENVELOPE BODY.PEEK[HEADER.FIELDS (REFERENCES)] BODYSTRUCTURE)", + ); conn.send_command(cmd.as_bytes()).await?; conn.read_response(&mut response, RequiredResponses::FETCH_REQUIRED) .await?; } else if debug!(select_response.exists > mailbox.exists.lock().unwrap().len()) { conn.send_command( format!( - "FETCH {}:* (UID FLAGS RFC822)", + "FETCH {}:* (UID FLAGS ENVELOPE BODY.PEEK[HEADER.FIELDS (REFERENCES)] BODYSTRUCTURE)", mailbox.exists.lock().unwrap().len() ) .as_bytes(), @@ -285,42 +287,70 @@ pub async fn examine_updates( } else { return Ok(()); } - debug!(&response); + debug!( + "fetch response is {} bytes and {} lines", + response.len(), + String::from_utf8_lossy(&response).lines().count() + ); let (_, mut v, _) = protocol_parser::fetch_responses(&response)?; + debug!("responses len is {}", v.len()); for FetchResponse { ref uid, - ref mut flags, - ref mut body, ref mut envelope, + ref mut flags, + ref references, .. } in v.iter_mut() { let uid = uid.unwrap(); - *envelope = Envelope::from_bytes(body.take().unwrap(), flags.as_ref().map(|&(f, _)| f)) - .map(|mut env| { - env.set_hash(generate_envelope_hash(&mailbox.imap_path(), &uid)); - if let Some((_, keywords)) = flags.take() { - let mut tag_lck = uid_store.tag_index.write().unwrap(); - for f in keywords { - let hash = tag_hash!(f); - if !tag_lck.contains_key(&hash) { - tag_lck.insert(hash, f); - } - env.labels_mut().push(hash); + let env = envelope.as_mut().unwrap(); + env.set_hash(generate_envelope_hash(&mailbox.imap_path(), &uid)); + if let Some(value) = references { + let parse_result = crate::email::parser::address::msg_id_list(value); + if let Ok((_, value)) = parse_result { + let prev_val = env.references.take(); + for v in value { + env.push_references(v); + } + if let Some(prev) = prev_val { + for v in prev.refs { + env.push_references(v); } } - env - }) - .map_err(|err| { - debug!("uid {} envelope parse error {}", uid, &err); - err - }) - .ok(); + } + env.set_references(value); + } + let mut tag_lck = uid_store.tag_index.write().unwrap(); + if let Some((flags, keywords)) = flags { + env.set_flags(*flags); + if !env.is_seen() { + mailbox.unseen.lock().unwrap().insert_new(env.hash()); + } + mailbox.exists.lock().unwrap().insert_new(env.hash()); + for f in keywords { + let hash = tag_hash!(f); + if !tag_lck.contains_key(&hash) { + tag_lck.insert(hash, f.to_string()); + } + env.labels_mut().push(hash); + } + } } if uid_store.keep_offline_cache { - cache_handle.insert_envelopes(mailbox_hash, &v)?; + debug!(cache_handle + .insert_envelopes(mailbox_hash, &v) + .chain_err_summary(|| { + format!( + "Could not save envelopes in cache for mailbox {}", + mailbox.imap_path() + ) + }))?; } - 'fetch_responses_c: for FetchResponse { uid, envelope, .. } in v { + + for FetchResponse { uid, envelope, .. } in v { + if uid.is_none() || envelope.is_none() { + continue; + } let uid = uid.unwrap(); if uid_store .uid_index @@ -328,35 +358,37 @@ pub async fn examine_updates( .unwrap() .contains_key(&(mailbox_hash, uid)) { - continue 'fetch_responses_c; - } - if let Some(env) = envelope { - uid_store - .hash_index - .lock() - .unwrap() - .insert(env.hash(), (uid, mailbox_hash)); - uid_store - .uid_index - .lock() - .unwrap() - .insert((mailbox_hash, uid), env.hash()); - debug!( - "Create event {} {} {}", - env.hash(), - env.subject(), - mailbox.path(), - ); - if !env.is_seen() { - mailbox.unseen.lock().unwrap().insert_new(env.hash()); - } - mailbox.exists.lock().unwrap().insert_new(env.hash()); - conn.add_refresh_event(RefreshEvent { - account_hash: uid_store.account_hash, - mailbox_hash, - kind: Create(Box::new(env)), - }); + continue; } + let env = envelope.unwrap(); + debug!( + "Create event {} {} {}", + env.hash(), + env.subject(), + mailbox.path(), + ); + uid_store + .msn_index + .lock() + .unwrap() + .entry(mailbox_hash) + .or_default() + .push(uid); + uid_store + .hash_index + .lock() + .unwrap() + .insert(env.hash(), (uid, mailbox_hash)); + uid_store + .uid_index + .lock() + .unwrap() + .insert((mailbox_hash, uid), env.hash()); + conn.add_refresh_event(RefreshEvent { + account_hash: uid_store.account_hash, + mailbox_hash, + kind: Create(Box::new(env)), + }); } } Ok(())