From 6506fffb9427ba13ba4368cd6b2c0dba12e5294c Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Fri, 24 Nov 2023 12:58:21 +0200 Subject: [PATCH] Rewrite email flag modifications Flag and tag modifications are now somewhat typed better, and the frontend applies them on its own on success. This means that if you set an unseen mail as seen but it was already seen in the backend, you will see the change locally. Previously it would remain unseen. Signed-off-by: Manos Pitsidianakis --- meli/src/accounts.rs | 208 ++++++++++++++----------------- meli/src/accounts/backend_ops.rs | 86 +++++++++++++ meli/src/mail/listing.rs | 113 +++++------------ meli/src/mail/view.rs | 34 ++--- melib/src/backends.rs | 33 ++++- melib/src/email.rs | 9 +- melib/src/imap/cache/sync.rs | 9 +- melib/src/imap/mod.rs | 55 ++++---- melib/src/imap/untagged.rs | 4 +- melib/src/imap/watch.rs | 2 +- melib/src/jmap/mod.rs | 60 ++++----- melib/src/maildir/backend.rs | 14 ++- melib/src/mbox/mod.rs | 2 +- melib/src/nntp/mod.rs | 8 +- melib/src/notmuch/message.rs | 2 +- melib/src/notmuch/mod.rs | 43 +++---- 16 files changed, 354 insertions(+), 328 deletions(-) create mode 100644 meli/src/accounts/backend_ops.rs diff --git a/meli/src/accounts.rs b/meli/src/accounts.rs index d413c0f3..23838846 100644 --- a/meli/src/accounts.rs +++ b/meli/src/accounts.rs @@ -61,6 +61,8 @@ use crate::{ MainLoopHandler, StatusEvent, ThreadEvent, }; +mod backend_ops; + #[macro_export] macro_rules! try_recv_timeout { ($oneshot:expr) => {{ @@ -242,6 +244,7 @@ pub enum JobRequest { }, SetFlags { env_hashes: EnvelopeHashBatch, + flags: SmallVec<[FlagOp; 8]>, handle: JoinHandle>, }, SaveMessage { @@ -325,7 +328,13 @@ impl std::fmt::Debug for JobRequest { } JobRequest::IsOnline { .. } => write!(f, "JobRequest::IsOnline"), JobRequest::Refresh { .. } => write!(f, "JobRequest::Refresh"), - JobRequest::SetFlags { .. } => write!(f, "JobRequest::SetFlags"), + JobRequest::SetFlags { + env_hashes, flags, .. + } => f + .debug_struct(stringify!(JobRequest::SetFlags)) + .field("env_hashes", &env_hashes) + .field("flags", &flags) + .finish(), JobRequest::SaveMessage { .. } => write!(f, "JobRequest::SaveMessage"), JobRequest::DeleteMessages { .. } => write!(f, "JobRequest::DeleteMessages"), JobRequest::CreateMailbox { .. } => write!(f, "JobRequest::CreateMailbox"), @@ -356,11 +365,14 @@ impl std::fmt::Display for JobRequest { JobRequest::Fetch { .. } => write!(f, "Mailbox fetch"), JobRequest::IsOnline { .. } => write!(f, "Online status check"), JobRequest::Refresh { .. } => write!(f, "Refresh mailbox"), - JobRequest::SetFlags { env_hashes, .. } => write!( + JobRequest::SetFlags { + env_hashes, flags, .. + } => write!( f, - "Set flags for {} message{}", + "Set flags for {} message{}: {:?}", env_hashes.len(), - if env_hashes.len() == 1 { "" } else { "s" } + if env_hashes.len() == 1 { "" } else { "s" }, + flags ), JobRequest::SaveMessage { .. } => write!(f, "Save message"), JobRequest::DeleteMessages { env_hashes, .. } => write!( @@ -777,42 +789,7 @@ impl Account { ); } #[cfg(feature = "sqlite3")] - if self.settings.conf.search_backend == crate::conf::SearchBackend::Sqlite3 { - match crate::sqlite3::remove(old_hash).map(|_| { - crate::sqlite3::insert( - (*envelope).clone(), - self.backend.clone(), - self.name.clone(), - ) - }) { - Err(err) => { - log::error!( - "Failed to update envelope {} in cache: {}", - envelope.message_id_display(), - err - ); - } - Ok(job) => { - let handle = self - .main_loop_handler - .job_executor - .spawn_blocking("sqlite3::update".into(), job); - self.insert_job( - handle.job_id, - JobRequest::Generic { - name: format!( - "Update envelope {} in sqlite3 cache", - envelope.message_id_display() - ) - .into(), - handle, - log_level: LogLevel::TRACE, - on_finish: None, - }, - ); - } - } - } + self.update_cached_env(*envelope.clone(), Some(old_hash)); self.collection.update(old_hash, *envelope, mailbox_hash); return Some(EnvelopeUpdate(old_hash)); } @@ -833,43 +810,18 @@ impl Account { entry.set_flags(flags); }); #[cfg(feature = "sqlite3")] - if self.settings.conf.search_backend == crate::conf::SearchBackend::Sqlite3 { - match crate::sqlite3::remove(env_hash).map(|_| { - crate::sqlite3::insert( - self.collection.envelopes.read().unwrap()[&env_hash].clone(), - self.backend.clone(), - self.name.clone(), - ) - }) { - Ok(job) => { - let handle = self - .main_loop_handler - .job_executor - .spawn_blocking("sqlite3::remove".into(), job); - self.insert_job( - handle.job_id, - JobRequest::Generic { - name: format!( - "Update envelope {} in sqlite3 cache", - self.collection.envelopes.read().unwrap()[&env_hash] - .message_id_display() - ) - .into(), - handle, - log_level: LogLevel::TRACE, - on_finish: None, - }, - ); - } - Err(err) => { - log::error!( - "Failed to update envelope {} in cache: {}", - self.collection.envelopes.read().unwrap()[&env_hash] - .message_id_display(), - err - ); - } - } + if let Some(env) = { + let temp = self + .collection + .envelopes + .read() + .unwrap() + .get(&env_hash) + .cloned(); + + temp + } { + self.update_cached_env(env, None); } self.collection.update_flags(env_hash, mailbox_hash); return Some(EnvelopeUpdate(env_hash)); @@ -880,43 +832,18 @@ impl Account { return Some(EnvelopeRename(old_hash, new_hash)); } #[cfg(feature = "sqlite3")] - if self.settings.conf.search_backend == crate::conf::SearchBackend::Sqlite3 { - match crate::sqlite3::remove(old_hash).map(|_| { - crate::sqlite3::insert( - self.collection.envelopes.read().unwrap()[&new_hash].clone(), - self.backend.clone(), - self.name.clone(), - ) - }) { - Err(err) => { - log::error!( - "Failed to update envelope {} in cache: {}", - &self.collection.envelopes.read().unwrap()[&new_hash] - .message_id_display(), - err - ); - } - Ok(job) => { - let handle = self - .main_loop_handler - .job_executor - .spawn_blocking("sqlite3::rename".into(), job); - self.insert_job( - handle.job_id, - JobRequest::Generic { - name: format!( - "Update envelope {} in sqlite3 cache", - self.collection.envelopes.read().unwrap()[&new_hash] - .message_id_display() - ) - .into(), - handle, - log_level: LogLevel::TRACE, - on_finish: None, - }, - ); - } - } + if let Some(env) = { + let temp = self + .collection + .envelopes + .read() + .unwrap() + .get(&new_hash) + .cloned(); + + temp + } { + self.update_cached_env(env, Some(old_hash)); } return Some(EnvelopeRename(old_hash, new_hash)); } @@ -1952,8 +1879,12 @@ impl Account { } } } - JobRequest::SetFlags { ref mut handle, .. } => { - if let Ok(Some(Err(err))) = handle.chan.try_recv() { + JobRequest::SetFlags { + ref mut handle, + ref env_hashes, + ref flags, + } => match handle.chan.try_recv() { + Ok(Some(Err(err))) => { self.main_loop_handler .job_executor .set_job_success(job_id, false); @@ -1964,7 +1895,50 @@ impl Account { Some(crate::types::NotificationType::Error(err.kind)), ))); } - } + Ok(Some(Ok(()))) => { + for env_hash in env_hashes.iter() { + if !self.collection.contains_key(&env_hash) { + continue; + } + let mut env_lck = self.collection.envelopes.write().unwrap(); + env_lck.entry(env_hash).and_modify(|entry| { + for op in flags.iter() { + match op { + FlagOp::Set(f) => { + let mut flags = entry.flags(); + flags.set(*f, true); + entry.set_flags(flags); + } + FlagOp::UnSet(f) => { + let mut flags = entry.flags(); + flags.set(*f, false); + entry.set_flags(flags); + } + FlagOp::SetTag(t) => { + entry + .tags_mut() + .insert(TagHash::from_bytes(t.as_bytes())); + } + FlagOp::UnSetTag(t) => { + entry + .tags_mut() + .remove(&TagHash::from_bytes(t.as_bytes())); + } + } + } + }); + #[cfg(feature = "sqlite3")] + if let Some(env) = env_lck.get(&env_hash).cloned() { + drop(env_lck); + self.update_cached_env(env, None); + } + + self.main_loop_handler + .send(ThreadEvent::UIEvent(UIEvent::EnvelopeUpdate(env_hash))); + } + } + _ => {} + }, JobRequest::SaveMessage { ref mut handle, ref bytes, diff --git a/meli/src/accounts/backend_ops.rs b/meli/src/accounts/backend_ops.rs new file mode 100644 index 00000000..3876a592 --- /dev/null +++ b/meli/src/accounts/backend_ops.rs @@ -0,0 +1,86 @@ +/* + * meli - accounts module. + * + * Copyright 2023 Manos Pitsidianakis + * + * This file is part of meli. + * + * meli is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * meli is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with meli. If not, see . + */ + +//! Account mail backend operations. + +use super::*; + +impl Account { + pub fn set_flags( + &mut self, + env_hashes: EnvelopeHashBatch, + mailbox_hash: MailboxHash, + flags: SmallVec<[FlagOp; 8]>, + ) -> Result { + let fut = self.backend.write().unwrap().set_flags( + env_hashes.clone(), + mailbox_hash, + flags.clone(), + )?; + let handle = self + .main_loop_handler + .job_executor + .spawn_specialized("set_unseen".into(), fut); + let job_id = handle.job_id; + self.insert_job( + job_id, + JobRequest::SetFlags { + env_hashes, + flags, + handle, + }, + ); + + Ok(job_id) + } + + #[cfg(not(feature = "sqlite3"))] + pub(super) fn update_cached_env(&mut self, _: Envelope, _: Option) {} + + #[cfg(feature = "sqlite3")] + pub(super) fn update_cached_env(&mut self, env: Envelope, old_hash: Option) { + if self.settings.conf.search_backend == crate::conf::SearchBackend::Sqlite3 { + let msg_id = env.message_id_display().to_string(); + match crate::sqlite3::remove(old_hash.unwrap_or(env.hash())) + .map(|_| crate::sqlite3::insert(env, self.backend.clone(), self.name.clone())) + { + Ok(job) => { + let handle = self + .main_loop_handler + .job_executor + .spawn_blocking("sqlite3::remove".into(), job); + self.insert_job( + handle.job_id, + JobRequest::Generic { + name: format!("Update envelope {} in sqlite3 cache", msg_id).into(), + handle, + log_level: LogLevel::TRACE, + on_finish: None, + }, + ); + } + Err(err) => { + log::error!("Failed to update envelope {} in cache: {}", msg_id, err); + } + } + } + } +} diff --git a/meli/src/mail/listing.rs b/meli/src/mail/listing.rs index 3775624f..e64a431f 100644 --- a/meli/src/mail/listing.rs +++ b/meli/src/mail/listing.rs @@ -31,7 +31,8 @@ use std::{ use futures::future::try_join_all; use melib::{ - backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Address, UnixTimestamp, + backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Address, FlagOp, + UnixTimestamp, }; use smallvec::SmallVec; @@ -505,99 +506,47 @@ pub trait MailListingTrait: ListingTrait { let account = &mut context.accounts[&account_hash]; match a { ListingAction::SetSeen => { - let job = account.backend.write().unwrap().set_flags( + if let Err(err) = account.set_flags( env_hashes.clone(), mailbox_hash, - smallvec::smallvec![(Ok(Flag::SEEN), true)], - ); - match job { - Err(err) => { - context.replies.push_back(UIEvent::StatusEvent( - StatusEvent::DisplayMessage(err.to_string()), - )); - } - Ok(fut) => { - let handle = account - .main_loop_handler - .job_executor - .spawn_specialized("set_seen".into(), fut); - account.insert_job( - handle.job_id, - JobRequest::SetFlags { env_hashes, handle }, - ); - } + smallvec::smallvec![FlagOp::Set(Flag::SEEN)], + ) { + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::DisplayMessage(err.to_string()), + )); } } ListingAction::SetUnseen => { - let job = account.backend.write().unwrap().set_flags( + if let Err(err) = account.set_flags( env_hashes.clone(), mailbox_hash, - smallvec::smallvec![(Ok(Flag::SEEN), false)], - ); - match job { - Err(err) => { - context.replies.push_back(UIEvent::StatusEvent( - StatusEvent::DisplayMessage(err.to_string()), - )); - } - Ok(fut) => { - let handle = account - .main_loop_handler - .job_executor - .spawn_specialized("set_unseen".into(), fut); - account.insert_job( - handle.job_id, - JobRequest::SetFlags { env_hashes, handle }, - ); - } - } - } - ListingAction::Tag(Remove(ref tag_str)) => { - let job = account.backend.write().unwrap().set_flags( - env_hashes.clone(), - mailbox_hash, - smallvec::smallvec![(Err(tag_str.to_string()), false)], - ); - match job { - Err(err) => { - context.replies.push_back(UIEvent::StatusEvent( - StatusEvent::DisplayMessage(err.to_string()), - )); - } - Ok(fut) => { - let handle = account - .main_loop_handler - .job_executor - .spawn_specialized("remove_tag".into(), fut); - account.insert_job( - handle.job_id, - JobRequest::SetFlags { env_hashes, handle }, - ); - } + smallvec::smallvec![FlagOp::UnSet(Flag::SEEN)], + ) { + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::DisplayMessage(err.to_string()), + )); } } ListingAction::Tag(Add(ref tag_str)) => { - let job = account.backend.write().unwrap().set_flags( + if let Err(err) = account.set_flags( env_hashes.clone(), mailbox_hash, - smallvec::smallvec![(Err(tag_str.to_string()), true)], - ); - match job { - Err(err) => { - context.replies.push_back(UIEvent::StatusEvent( - StatusEvent::DisplayMessage(err.to_string()), - )); - } - Ok(fut) => { - let handle = account - .main_loop_handler - .job_executor - .spawn_specialized("add_tag".into(), fut); - account.insert_job( - handle.job_id, - JobRequest::SetFlags { env_hashes, handle }, - ); - } + smallvec::smallvec![FlagOp::SetTag(tag_str.into())], + ) { + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::DisplayMessage(err.to_string()), + )); + } + } + ListingAction::Tag(Remove(ref tag_str)) => { + if let Err(err) = account.set_flags( + env_hashes.clone(), + mailbox_hash, + smallvec::smallvec![FlagOp::UnSetTag(tag_str.into())], + ) { + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::DisplayMessage(err.to_string()), + )); } } ListingAction::Delete => { diff --git a/meli/src/mail/view.rs b/meli/src/mail/view.rs index 4af31463..fab7fb28 100644 --- a/meli/src/mail/view.rs +++ b/meli/src/mail/view.rs @@ -29,7 +29,7 @@ use std::{ use melib::{ email::attachment_types::ContentType, list_management, mailto::Mailto, parser::BytesExt, - utils::datetime, Card, Draft, HeaderName, SpecialUsageMailbox, + utils::datetime, Card, Draft, FlagOp, HeaderName, SpecialUsageMailbox, }; use smallvec::SmallVec; @@ -310,32 +310,16 @@ impl Component for MailView { { let account = &mut context.accounts[&coordinates.0]; if !account.collection.get_env(coordinates.2).is_seen() { - let job = account.backend.write().unwrap().set_flags( + if let Err(err) = account.set_flags( coordinates.2.into(), coordinates.1, - smallvec::smallvec![(Ok(Flag::SEEN), true)], - ); - match job { - Ok(fut) => { - let handle = account - .main_loop_handler - .job_executor - .spawn_specialized("set_flags".into(), fut); - account.insert_job( - handle.job_id, - JobRequest::SetFlags { - env_hashes: coordinates.2.into(), - handle, - }, - ); - } - Err(err) => { - context.replies.push_back(UIEvent::StatusEvent( - StatusEvent::DisplayMessage(format!( - "Could not set message as seen: {err}", - )), - )); - } + smallvec::smallvec![FlagOp::Set(Flag::SEEN)], + ) { + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::DisplayMessage(format!( + "Could not set message as seen: {err}", + )), + )); } } } diff --git a/melib/src/backends.rs b/melib/src/backends.rs index 81c82a96..fbc6d3d1 100644 --- a/melib/src/backends.rs +++ b/melib/src/backends.rs @@ -320,6 +320,37 @@ impl Deref for BackendEventConsumer { } } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum FlagOp { + Set(Flag), + SetTag(String), + UnSet(Flag), + UnSetTag(String), +} + +impl From<&FlagOp> for bool { + fn from(val: &FlagOp) -> Self { + matches!(val, FlagOp::Set(_) | FlagOp::SetTag(_)) + } +} + +impl FlagOp { + #[inline] + pub fn is_flag(&self) -> bool { + matches!(self, Self::Set(_) | Self::UnSet(_)) + } + + #[inline] + pub fn is_tag(&self) -> bool { + matches!(self, Self::SetTag(_) | Self::UnSetTag(_)) + } + + #[inline] + pub fn as_bool(&self) -> bool { + self.into() + } +} + #[derive(Debug, Clone)] pub struct MailBackendCapabilities { pub is_async: bool, @@ -376,7 +407,7 @@ pub trait MailBackend: ::std::fmt::Debug + Send + Sync { &mut self, env_hashes: EnvelopeHashBatch, mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()>; fn delete_messages( diff --git a/melib/src/email.rs b/melib/src/email.rs index b7057754..c42e8cee 100644 --- a/melib/src/email.rs +++ b/melib/src/email.rs @@ -116,6 +116,7 @@ use imap_codec::imap_types::{ fetch::{MacroOrMessageDataItemNames, MessageDataItemName, Section}, flag::Flag as ImapCodecFlag, }; +use indexmap::IndexSet; use smallvec::SmallVec; use crate::{ @@ -287,7 +288,7 @@ pub struct Envelope { pub thread: ThreadNodeHash, pub flags: Flag, pub has_attachments: bool, - pub tags: SmallVec<[TagHash; 8]>, + pub tags: IndexSet, } impl std::fmt::Debug for Envelope { @@ -330,7 +331,7 @@ impl Envelope { thread: ThreadNodeHash::null(), has_attachments: false, flags: Flag::default(), - tags: SmallVec::new(), + tags: IndexSet::new(), } } @@ -851,11 +852,11 @@ impl Envelope { self.has_attachments } - pub fn tags(&self) -> &SmallVec<[TagHash; 8]> { + pub fn tags(&self) -> &IndexSet { &self.tags } - pub fn tags_mut(&mut self) -> &mut SmallVec<[TagHash; 8]> { + pub fn tags_mut(&mut self) -> &mut IndexSet { &mut self.tags } } diff --git a/melib/src/imap/cache/sync.rs b/melib/src/imap/cache/sync.rs index 4a16f9a2..29420179 100644 --- a/melib/src/imap/cache/sync.rs +++ b/melib/src/imap/cache/sync.rs @@ -25,6 +25,7 @@ use imap_codec::imap_types::{ sequence::SequenceSet, status::StatusDataItemName, }; +use indexmap::IndexSet; use super::*; @@ -173,7 +174,7 @@ impl ImapConnection { for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } } @@ -266,7 +267,7 @@ impl ImapConnection { != &tags .iter() .map(|t| TagHash::from_bytes(t.as_bytes())) - .collect::>() + .collect::>() { env_lck.entry(env_hash).and_modify(|entry| { entry.inner.set_flags(flags); @@ -466,7 +467,7 @@ impl ImapConnection { for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } } @@ -556,7 +557,7 @@ impl ImapConnection { != &tags .iter() .map(|t| TagHash::from_bytes(t.as_bytes())) - .collect::>() + .collect::>() { env_lck.entry(env_hash).and_modify(|entry| { entry.inner.set_flags(flags); diff --git a/melib/src/imap/mod.rs b/melib/src/imap/mod.rs index 380cada6..a0b0d66a 100644 --- a/melib/src/imap/mod.rs +++ b/melib/src/imap/mod.rs @@ -701,7 +701,7 @@ impl MailBackend for ImapType { &mut self, env_hashes: EnvelopeHashBatch, mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { let connection = self.connection.clone(); let uid_store = self.uid_store.clone(); @@ -724,7 +724,7 @@ impl MailBackend for ImapType { let mut conn = connection.lock().await; conn.select_mailbox(mailbox_hash, &mut response, false) .await?; - if flags.iter().any(|(_, b)| *b) { + if flags.iter().any(::from) { /* Set flags/tags to true */ let mut set_seen = false; let command = { @@ -734,28 +734,25 @@ impl MailBackend for ImapType { cmd = format!("{},{}", cmd, uid); } cmd = format!("{} +FLAGS (", cmd); - for (f, v) in flags.iter() { - if !*v { - continue; - } - match f { - Ok(flag) if *flag == Flag::REPLIED => { + for op in flags.iter().filter(|op| ::from(*op)) { + match op { + FlagOp::Set(flag) if *flag == Flag::REPLIED => { cmd.push_str("\\Answered "); } - Ok(flag) if *flag == Flag::FLAGGED => { + FlagOp::Set(flag) if *flag == Flag::FLAGGED => { cmd.push_str("\\Flagged "); } - Ok(flag) if *flag == Flag::TRASHED => { + FlagOp::Set(flag) if *flag == Flag::TRASHED => { cmd.push_str("\\Deleted "); } - Ok(flag) if *flag == Flag::SEEN => { + FlagOp::Set(flag) if *flag == Flag::SEEN => { cmd.push_str("\\Seen "); set_seen = true; } - Ok(flag) if *flag == Flag::DRAFT => { + FlagOp::Set(flag) if *flag == Flag::DRAFT => { cmd.push_str("\\Draft "); } - Ok(_) => { + FlagOp::Set(_) => { log::error!( "Application error: more than one flag bit set in set_flags: \ {:?}", @@ -768,12 +765,13 @@ impl MailBackend for ImapType { )) .set_kind(crate::ErrorKind::Bug)); } - Err(tag) => { + FlagOp::SetTag(tag) => { let hash = TagHash::from_bytes(tag.as_bytes()); tag_lck.entry(hash).or_insert_with(|| tag.to_string()); cmd.push_str(tag); cmd.push(' '); } + _ => {} } } // pop last space @@ -794,7 +792,7 @@ impl MailBackend for ImapType { } } } - if flags.iter().any(|(_, b)| !*b) { + if flags.iter().any(|b| !::from(b)) { let mut set_unseen = false; /* Set flags/tags to false */ let command = { @@ -803,28 +801,25 @@ impl MailBackend for ImapType { cmd = format!("{},{}", cmd, uid); } cmd = format!("{} -FLAGS (", cmd); - for (f, v) in flags.iter() { - if *v { - continue; - } - match f { - Ok(flag) if *flag == Flag::REPLIED => { + for op in flags.iter().filter(|op| !::from(*op)) { + match op { + FlagOp::UnSet(flag) if *flag == Flag::REPLIED => { cmd.push_str("\\Answered "); } - Ok(flag) if *flag == Flag::FLAGGED => { + FlagOp::UnSet(flag) if *flag == Flag::FLAGGED => { cmd.push_str("\\Flagged "); } - Ok(flag) if *flag == Flag::TRASHED => { + FlagOp::UnSet(flag) if *flag == Flag::TRASHED => { cmd.push_str("\\Deleted "); } - Ok(flag) if *flag == Flag::SEEN => { + FlagOp::UnSet(flag) if *flag == Flag::SEEN => { cmd.push_str("\\Seen "); set_unseen = true; } - Ok(flag) if *flag == Flag::DRAFT => { + FlagOp::UnSet(flag) if *flag == Flag::DRAFT => { cmd.push_str("\\Draft "); } - Ok(_) => { + FlagOp::UnSet(_) => { log::error!( "Application error: more than one flag bit set in set_flags: \ {:?}", @@ -836,12 +831,14 @@ impl MailBackend for ImapType { flags ))); } - Err(tag) => { + FlagOp::UnSetTag(tag) => { cmd.push_str(tag); cmd.push(' '); } + _ => {} } } + // [ref:TODO] there should be a check that cmd is not empty here. // pop last space cmd.pop(); cmd.push(')'); @@ -872,7 +869,7 @@ impl MailBackend for ImapType { let flag_future = self.set_flags( env_hashes, mailbox_hash, - smallvec::smallvec![(Ok(Flag::TRASHED), true)], + smallvec::smallvec![FlagOp::Set(Flag::TRASHED)], )?; let connection = self.connection.clone(); Ok(Box::pin(async move { @@ -1811,7 +1808,7 @@ async fn fetch_hlpr(state: &mut FetchState) -> Result> { for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } } diff --git a/melib/src/imap/untagged.rs b/melib/src/imap/untagged.rs index 5e3dffd5..6ff155db 100644 --- a/melib/src/imap/untagged.rs +++ b/melib/src/imap/untagged.rs @@ -246,7 +246,7 @@ impl ImapConnection { for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } mailbox.exists.lock().unwrap().insert_new(env.hash()); @@ -379,7 +379,7 @@ impl ImapConnection { for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } mailbox.exists.lock().unwrap().insert_new(env.hash()); diff --git a/melib/src/imap/watch.rs b/melib/src/imap/watch.rs index 3734aac3..99fc2ab9 100644 --- a/melib/src/imap/watch.rs +++ b/melib/src/imap/watch.rs @@ -379,7 +379,7 @@ pub async fn examine_updates( for f in keywords { let hash = TagHash::from_bytes(f.as_bytes()); tag_lck.entry(hash).or_insert_with(|| f.to_string()); - env.tags_mut().push(hash); + env.tags_mut().insert(hash); } } } diff --git a/melib/src/jmap/mod.rs b/melib/src/jmap/mod.rs index b6017a40..cfe98f6b 100644 --- a/melib/src/jmap/mod.rs +++ b/melib/src/jmap/mod.rs @@ -29,7 +29,7 @@ use std::{ }; use futures::{lock::Mutex as FutureMutex, Stream}; -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use isahc::{config::RedirectPolicy, AsyncReadResponseExt, HttpClient}; use serde_json::{json, Value}; use smallvec::SmallVec; @@ -209,7 +209,7 @@ pub struct Store { impl Store { pub fn add_envelope(&self, obj: EmailObject) -> Envelope { let mut flags = Flag::default(); - let mut labels: SmallVec<[TagHash; 8]> = SmallVec::new(); + let mut labels: IndexSet = IndexSet::new(); let mut tag_lck = self.collection.tag_index.write().unwrap(); for t in obj.keywords().keys() { match t.as_str() { @@ -229,7 +229,7 @@ impl Store { _ => { let tag_hash = TagHash::from_bytes(t.as_bytes()); tag_lck.entry(tag_hash).or_insert_with(|| t.to_string()); - labels.push(tag_hash); + labels.insert(tag_hash); } } } @@ -240,7 +240,7 @@ impl Store { drop(tag_lck); let mut ret: Envelope = obj.into(); ret.set_flags(flags); - ret.tags_mut().append(&mut labels); + ret.tags_mut().extend(labels); let mut id_store_lck = self.id_store.lock().unwrap(); let mut reverse_id_store_lck = self.reverse_id_store.lock().unwrap(); @@ -760,7 +760,7 @@ impl MailBackend for JmapType { &mut self, env_hashes: EnvelopeHashBatch, mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { let store = self.store.clone(); let connection = self.connection.clone(); @@ -769,9 +769,9 @@ impl MailBackend for JmapType { let mut ids: Vec> = Vec::with_capacity(env_hashes.rest.len() + 1); let mut id_map: IndexMap, EnvelopeHash> = IndexMap::default(); let mut update_keywords: IndexMap = IndexMap::default(); - for (flag, value) in flags.iter() { - match flag { - Ok(f) => { + for op in flags.iter() { + match op { + FlagOp::Set(f) => { update_keywords.insert( format!( "keywords/{}", @@ -785,23 +785,32 @@ impl MailBackend for JmapType { _ => continue, // [ref:VERIFY] } ), - if *value { - serde_json::json!(true) - } else { - serde_json::json!(null) - }, + serde_json::json!(true), ); } - Err(t) => { + FlagOp::UnSet(f) => { update_keywords.insert( - format!("keywords/{}", t), - if *value { - serde_json::json!(true) - } else { - serde_json::json!(null) - }, + format!( + "keywords/{}", + match *f { + Flag::DRAFT => "$draft", + Flag::FLAGGED => "$flagged", + Flag::SEEN => "$seen", + Flag::REPLIED => "$answered", + Flag::TRASHED => "$junk", + Flag::PASSED => "$passed", + _ => continue, // [ref:VERIFY] + } + ), + serde_json::json!(null), ); } + FlagOp::SetTag(t) => { + update_keywords.insert(format!("keywords/{}", t), serde_json::json!(true)); + } + FlagOp::UnSetTag(t) => { + update_keywords.insert(format!("keywords/{}", t), serde_json::json!(null)); + } } } { @@ -866,14 +875,9 @@ impl MailBackend for JmapType { { let mut tag_index_lck = store.collection.tag_index.write().unwrap(); - for (flag, value) in flags.iter() { - match flag { - Ok(_) => {} - Err(t) => { - if *value { - tag_index_lck.insert(TagHash::from_bytes(t.as_bytes()), t.clone()); - } - } + for op in flags.iter() { + if let FlagOp::SetTag(t) = op { + tag_index_lck.insert(TagHash::from_bytes(t.as_bytes()), t.clone()); } } drop(tag_index_lck); diff --git a/melib/src/maildir/backend.rs b/melib/src/maildir/backend.rs index 4b9b6ae3..d6bbfa36 100644 --- a/melib/src/maildir/backend.rs +++ b/melib/src/maildir/backend.rs @@ -809,10 +809,10 @@ impl MailBackend for MaildirType { &mut self, env_hashes: EnvelopeHashBatch, mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { let hash_index = self.hash_indexes.clone(); - if flags.iter().any(|(f, _)| f.is_err()) { + if flags.iter().any(|op| op.is_tag()) { return Err(Error::new("Maildir doesn't support tags.")); } @@ -841,8 +841,10 @@ impl MailBackend for MaildirType { .ok_or_else(|| Error::new(format!("Invalid email filename: {:?}", path)))? + 3; let mut new_name: String = path[..idx].to_string(); - for (f, value) in flags.iter() { - env_flags.set(*f.as_ref().unwrap(), *value); + for op in flags.iter() { + if let FlagOp::Set(f) | FlagOp::UnSet(f) = op { + env_flags.set(*f, op.as_bool()); + } } if !(env_flags & Flag::DRAFT).is_empty() { @@ -867,9 +869,9 @@ impl MailBackend for MaildirType { hash_index.entry(env_hash).or_default().modified = Some(PathMod::Path(new_name.clone())); - debug!("renaming {:?} to {:?}", path, new_name); + log::debug!("renaming {:?} to {:?}", path, new_name); fs::rename(path, &new_name)?; - debug!("success in rename"); + log::debug!("success in rename"); } Ok(()) })) diff --git a/melib/src/mbox/mod.rs b/melib/src/mbox/mod.rs index f3ca5c69..04483f60 100644 --- a/melib/src/mbox/mod.rs +++ b/melib/src/mbox/mod.rs @@ -1152,7 +1152,7 @@ impl MailBackend for MboxType { &mut self, _env_hashes: EnvelopeHashBatch, _mailbox_hash: MailboxHash, - _flags: SmallVec<[(std::result::Result, bool); 8]>, + _flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { Err(Error::new( "Setting flags is currently unimplemented for mbox backend", diff --git a/melib/src/nntp/mod.rs b/melib/src/nntp/mod.rs index a3a63fd3..c2198bdb 100644 --- a/melib/src/nntp/mod.rs +++ b/melib/src/nntp/mod.rs @@ -438,7 +438,7 @@ impl MailBackend for NntpType { &mut self, env_hashes: EnvelopeHashBatch, mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { let uid_store = self.uid_store.clone(); Ok(Box::pin(async move { @@ -461,11 +461,11 @@ impl MailBackend for NntpType { let fsets = &uid_store.mailboxes.lock().await[&mailbox_hash]; let store_lck = uid_store.store.lock().await; if let Some(s) = store_lck.as_ref() { - for (flag, on) in flags { - if let Ok(f) = flag { + for op in flags { + if let FlagOp::Set(f) | FlagOp::UnSet(f) = op { for (env_hash, uid) in &uids { let mut current_val = s.flags(*env_hash, mailbox_hash, *uid)?; - current_val.set(f, on); + current_val.set(f, ::from(&op)); if !current_val.intersects(Flag::SEEN) { fsets.unseen.lock().unwrap().insert_new(*env_hash); } else { diff --git a/melib/src/notmuch/message.rs b/melib/src/notmuch/message.rs index 9f988a67..ca3279c5 100644 --- a/melib/src/notmuch/message.rs +++ b/melib/src/notmuch/message.rs @@ -101,7 +101,7 @@ impl<'m> Message<'m> { for tag in tags { let num = TagHash::from_bytes(tag.as_bytes()); tag_lock.entry(num).or_insert(tag); - env.tags_mut().push(num); + env.tags_mut().insert(num); } unsafe { use crate::email::parser::address::rfc2822address_list; diff --git a/melib/src/notmuch/mod.rs b/melib/src/notmuch/mod.rs index ad9a0005..a70c711c 100644 --- a/melib/src/notmuch/mod.rs +++ b/melib/src/notmuch/mod.rs @@ -850,7 +850,7 @@ impl MailBackend for NotmuchDb { &mut self, env_hashes: EnvelopeHashBatch, _mailbox_hash: MailboxHash, - flags: SmallVec<[(std::result::Result, bool); 8]>, + flags: SmallVec<[FlagOp; 8]>, ) -> ResultFuture<()> { let database = Self::new_connection( self.path.as_path(), @@ -906,32 +906,29 @@ impl MailBackend for NotmuchDb { }}; } - for (f, v) in flags.iter() { - let value = *v; - debug!(&f); - debug!(&value); - match f { - Ok(Flag::DRAFT) if value => add_tag!(b"draft\0"), - Ok(Flag::DRAFT) => remove_tag!(b"draft\0"), - Ok(Flag::FLAGGED) if value => add_tag!(b"flagged\0"), - Ok(Flag::FLAGGED) => remove_tag!(b"flagged\0"), - Ok(Flag::PASSED) if value => add_tag!(b"passed\0"), - Ok(Flag::PASSED) => remove_tag!(b"passed\0"), - Ok(Flag::REPLIED) if value => add_tag!(b"replied\0"), - Ok(Flag::REPLIED) => remove_tag!(b"replied\0"), - Ok(Flag::SEEN) if value => remove_tag!(b"unread\0"), - Ok(Flag::SEEN) => add_tag!(b"unread\0"), - Ok(Flag::TRASHED) if value => add_tag!(b"trashed\0"), - Ok(Flag::TRASHED) => remove_tag!(b"trashed\0"), - Ok(_) => debug!("flags is {:?} value = {}", f, value), - Err(tag) if value => { + for op in flags.iter() { + match op { + FlagOp::Set(Flag::DRAFT) => add_tag!(b"draft\0"), + FlagOp::UnSet(Flag::DRAFT) => remove_tag!(b"draft\0"), + FlagOp::Set(Flag::FLAGGED) => add_tag!(b"flagged\0"), + FlagOp::UnSet(Flag::FLAGGED) => remove_tag!(b"flagged\0"), + FlagOp::Set(Flag::PASSED) => add_tag!(b"passed\0"), + FlagOp::UnSet(Flag::PASSED) => remove_tag!(b"passed\0"), + FlagOp::Set(Flag::REPLIED) => add_tag!(b"replied\0"), + FlagOp::UnSet(Flag::REPLIED) => remove_tag!(b"replied\0"), + FlagOp::Set(Flag::SEEN) => remove_tag!(b"unread\0"), + FlagOp::UnSet(Flag::SEEN) => add_tag!(b"unread\0"), + FlagOp::Set(Flag::TRASHED) => add_tag!(b"trashed\0"), + FlagOp::UnSet(Flag::TRASHED) => remove_tag!(b"trashed\0"), + FlagOp::SetTag(tag) => { let c_tag = CString::new(tag.as_str()).unwrap(); add_tag!(&c_tag.as_ref()); } - Err(tag) => { + FlagOp::UnSetTag(tag) => { let c_tag = CString::new(tag.as_str()).unwrap(); remove_tag!(&c_tag.as_ref()); } + _ => debug!("flag_op is {:?}", op), } } @@ -943,8 +940,8 @@ impl MailBackend for NotmuchDb { *p = msg_id.into(); } } - for (f, v) in flags.iter() { - if let (Err(tag), true) = (f, v) { + for op in flags.iter() { + if let FlagOp::SetTag(tag) = op { let hash = TagHash::from_bytes(tag.as_bytes()); tag_index.write().unwrap().insert(hash, tag.to_string()); }