From a90f425d32af866974eec58b43c02f302ae3376a Mon Sep 17 00:00:00 2001 From: Quentin Dufour Date: Mon, 8 Jan 2024 21:18:45 +0100 Subject: [PATCH] Futures must be ordered --- src/imap/index.rs | 11 +++++-- src/imap/mail_view.rs | 4 +-- src/imap/mailbox_view.rs | 5 +-- src/imap/search.rs | 2 +- src/mail/mailbox.rs | 2 +- src/mail/query.rs | 68 +++++++++++----------------------------- 6 files changed, 34 insertions(+), 58 deletions(-) diff --git a/src/imap/index.rs b/src/imap/index.rs index f9228fe..4853374 100644 --- a/src/imap/index.rs +++ b/src/imap/index.rs @@ -104,16 +104,21 @@ impl<'a> Index<'a> { return Ok(vec![]); } let iter_strat = sequence::Strategy::Naive { - largest: self.last().expect("The mailbox is not empty").uid, + largest: NonZeroU32::try_from(self.imap_index.len() as u32)?, }; - sequence_set + let mut acc = sequence_set .iter(iter_strat) .map(|wanted_id| { self.imap_index .get((wanted_id.get() as usize) - 1) .ok_or(anyhow!("Mail not found")) }) - .collect::>>() + .collect::>>()?; + + // Sort the result to be consistent with UID + acc.sort_by(|a, b| a.i.cmp(&b.i)); + + Ok(acc) } pub fn fetch( diff --git a/src/imap/mail_view.rs b/src/imap/mail_view.rs index 879166d..a593b1a 100644 --- a/src/imap/mail_view.rs +++ b/src/imap/mail_view.rs @@ -27,13 +27,13 @@ use crate::imap::response::Body; pub struct MailView<'a> { pub in_idx: &'a MailIndex<'a>, - pub query_result: &'a QueryResult<'a>, + pub query_result: &'a QueryResult, pub content: FetchedMail<'a>, } impl<'a> MailView<'a> { pub fn new( - query_result: &'a QueryResult<'a>, + query_result: &'a QueryResult, in_idx: &'a MailIndex<'a>, ) -> Result> { Ok(Self { diff --git a/src/imap/mailbox_view.rs b/src/imap/mailbox_view.rs index 027947f..513567f 100644 --- a/src/imap/mailbox_view.rs +++ b/src/imap/mailbox_view.rs @@ -1,7 +1,7 @@ use std::num::NonZeroU32; use std::sync::Arc; -use anyhow::{anyhow, Context, Error, Result}; +use anyhow::{anyhow, Error, Result}; use futures::stream::{FuturesOrdered, StreamExt}; @@ -259,6 +259,7 @@ impl MailboxView { true => QueryScope::Full, _ => QueryScope::Partial, }; + tracing::debug!("Query scope {:?}", query_scope); let idx = self.index()?; let mail_idx_list = idx.fetch(sequence_set, *is_uid_fetch)?; @@ -544,7 +545,6 @@ mod tests { let rfc822 = b"Subject: hello\r\nFrom: a@a.a\r\nTo: b@b.b\r\nDate: Thu, 12 Oct 2023 08:45:28 +0000\r\n\r\nhello world"; let qr = QueryResult::FullResult { uuid: mail_in_idx.uuid.clone(), - index: &index_entry, metadata: meta, content: rfc822.to_vec(), }; @@ -619,6 +619,7 @@ mod tests { seq: NonZeroU32::new(1).unwrap(), items: NonEmptyVec::from(MessageDataItem::Body(mime_view::bodystructure( &message.child, + false, )?)), }); let test_bytes = ResponseCodec::new().encode(&test_repr).dump(); diff --git a/src/imap/search.rs b/src/imap/search.rs index 22afd0c..c4888d0 100644 --- a/src/imap/search.rs +++ b/src/imap/search.rs @@ -134,7 +134,7 @@ impl<'a> Criteria<'a> { pub fn filter_on_query<'b>( &self, midx_list: &[&'b MailIndex<'b>], - query_result: &'b Vec>, + query_result: &'b Vec, ) -> Result>> { Ok(midx_list .iter() diff --git a/src/mail/mailbox.rs b/src/mail/mailbox.rs index 7eed34f..aab200b 100644 --- a/src/mail/mailbox.rs +++ b/src/mail/mailbox.rs @@ -486,7 +486,7 @@ fn dump(uid_index: &Bayou) { /// The metadata of a message that is stored in K2V /// at pk = mail/, sk = -#[derive(Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct MailMeta { /// INTERNALDATE field (milliseconds since epoch) pub internaldate: u64, diff --git a/src/mail/query.rs b/src/mail/query.rs index 91bd6c1..a8a23b2 100644 --- a/src/mail/query.rs +++ b/src/mail/query.rs @@ -1,9 +1,8 @@ use super::mailbox::MailMeta; use super::snapshot::FrozenMailbox; -use super::uidindex::IndexEntry; use super::unique_ident::UniqueIdent; -use anyhow::{anyhow, Result}; -use futures::stream::{FuturesUnordered, StreamExt}; +use anyhow::Result; +use futures::stream::{FuturesOrdered, StreamExt}; /// Query is in charge of fetching efficiently /// requested data for a list of emails @@ -13,7 +12,7 @@ pub struct Query<'a, 'b> { pub scope: QueryScope, } -#[allow(dead_code)] +#[derive(Debug)] pub enum QueryScope { Index, Partial, @@ -30,41 +29,26 @@ impl QueryScope { } impl<'a, 'b> Query<'a, 'b> { - pub async fn fetch(&self) -> Result>> { + pub async fn fetch(&self) -> Result> { match self.scope { - QueryScope::Index => self.index(), - QueryScope::Partial => self.partial().await, + QueryScope::Index => Ok(self.emails.iter().map(|&uuid| QueryResult::IndexResult { uuid }).collect()), + QueryScope::Partial =>self.partial().await, QueryScope::Full => self.full().await, } } // --- functions below are private *for reasons* - fn index(&self) -> Result>> { - self.emails - .iter() - .map(|uuid| { - self.frozen - .snapshot - .table - .get(uuid) - .map(|index| QueryResult::IndexResult { uuid: *uuid, index }) - .ok_or(anyhow!("missing email in index")) - }) - .collect::, _>>() - } - - async fn partial(&self) -> Result>> { + async fn partial(&self) -> Result> { let meta = self.frozen.mailbox.fetch_meta(self.emails).await?; let result = meta .into_iter() - .zip(self.index()?) - .map(|(metadata, index)| { - index - .into_partial(metadata) - .expect("index to be IndexResult") + .zip(self.emails.iter()) + .map(|(metadata, &uuid)| { + QueryResult::PartialResult { uuid, metadata } }) .collect::>(); + Ok(result) } @@ -72,7 +56,7 @@ impl<'a, 'b> Query<'a, 'b> { /// AND GENERATE SO MUCH NETWORK TRAFFIC. /// THIS FUNCTION SHOULD BE REWRITTEN, FOR EXAMPLE WITH /// SOMETHING LIKE AN ITERATOR - async fn full(&self) -> Result>> { + async fn full(&self) -> Result> { let meta_list = self.partial().await?; meta_list .into_iter() @@ -91,7 +75,7 @@ impl<'a, 'b> Query<'a, 'b> { Ok(meta.into_full(content).expect("meta to be PartialResult")) }) - .collect::>() + .collect::>() .collect::>() .await .into_iter() @@ -99,24 +83,22 @@ impl<'a, 'b> Query<'a, 'b> { } } -pub enum QueryResult<'a> { +#[derive(Debug)] +pub enum QueryResult { IndexResult { uuid: UniqueIdent, - index: &'a IndexEntry, }, PartialResult { uuid: UniqueIdent, - index: &'a IndexEntry, metadata: MailMeta, }, FullResult { uuid: UniqueIdent, - index: &'a IndexEntry, metadata: MailMeta, content: Vec, }, } -impl<'a> QueryResult<'a> { +impl QueryResult { pub fn uuid(&self) -> &UniqueIdent { match self { Self::IndexResult { uuid, .. } => uuid, @@ -125,16 +107,7 @@ impl<'a> QueryResult<'a> { } } - #[allow(dead_code)] - pub fn index(&self) -> &IndexEntry { - match self { - Self::IndexResult { index, .. } => index, - Self::PartialResult { index, .. } => index, - Self::FullResult { index, .. } => index, - } - } - - pub fn metadata(&'a self) -> Option<&'a MailMeta> { + pub fn metadata(&self) -> Option<&MailMeta> { match self { Self::IndexResult { .. } => None, Self::PartialResult { metadata, .. } => Some(metadata), @@ -143,7 +116,7 @@ impl<'a> QueryResult<'a> { } #[allow(dead_code)] - pub fn content(&'a self) -> Option<&'a [u8]> { + pub fn content(&self) -> Option<&[u8]> { match self { Self::FullResult { content, .. } => Some(content), _ => None, @@ -152,9 +125,8 @@ impl<'a> QueryResult<'a> { fn into_partial(self, metadata: MailMeta) -> Option { match self { - Self::IndexResult { uuid, index } => Some(Self::PartialResult { + Self::IndexResult { uuid } => Some(Self::PartialResult { uuid, - index, metadata, }), _ => None, @@ -165,11 +137,9 @@ impl<'a> QueryResult<'a> { match self { Self::PartialResult { uuid, - index, metadata, } => Some(Self::FullResult { uuid, - index, metadata, content, }),