From 6b7498a4b12d8ee3cfb9e7cb2a5ac7c3d5b9b090 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Sat, 7 Sep 2019 14:07:56 +0200 Subject: [PATCH 1/4] fix(contact): fix logic for create or add contact Closes #448 --- src/contact.rs | 58 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/src/contact.rs b/src/contact.rs index a29af19d1..dea4761b7 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -341,19 +341,22 @@ impl<'a> Contact<'a> { let row_id = row.get(0)?; let row_name: String = row.get(1)?; let row_addr: String = row.get(2)?; - let row_origin = row.get(3)?; + let row_origin: Origin = row.get(3)?; let row_authname: String = row.get(4)?; - if !name.as_ref().is_empty() && !row_name.is_empty() { - if origin >= row_origin && name.as_ref() != row_name { + if !name.as_ref().is_empty() { + if !row_name.is_empty() { + if origin >= row_origin && name.as_ref() != row_name { + update_name = true; + } + } else { update_name = true; } - } else { - update_name = true; - } - if origin == Origin::IncomingUnknownFrom && name.as_ref() != row_authname { - update_authname = true; + if origin == Origin::IncomingUnknownFrom && name.as_ref() != row_authname { + update_authname = true; + } } + Ok((row_id, row_name, row_addr, row_origin, row_authname)) }, ) { @@ -393,7 +396,7 @@ impl<'a> Contact<'a> { context, &context.sql, "UPDATE chats SET name=? WHERE type=? AND id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?);", - params![name.as_ref(), 100, row_id] + params![name.as_ref(), Chattype::Single, row_id] ).ok(); } sth_modified = Modifier::Modified; @@ -431,19 +434,13 @@ impl<'a> Contact<'a> { /// To add a single contact entered by the user, you should prefer `Contact::create`, /// however, for adding a bunch of addresses, this function is _much_ faster. /// - /// The `adr_book` is a multiline string in the format `Name one\nAddress one\nName two\nAddress two`. + /// The `addr_book` is a multiline string in the format `Name one\nAddress one\nName two\nAddress two`. /// /// Returns the number of modified contacts. - pub fn add_address_book(context: &Context, adr_book: impl AsRef) -> Result { + pub fn add_address_book(context: &Context, addr_book: impl AsRef) -> Result { let mut modify_cnt = 0; - for chunk in &adr_book.as_ref().lines().chunks(2) { - let chunk = chunk.collect::>(); - if chunk.len() < 2 { - break; - } - let name = chunk[0]; - let addr = chunk[1]; + for (name, addr) in split_address_book(addr_book.as_ref()).into_iter() { let name = normalize_name(name); let (_, modified) = Contact::add_or_lookup(context, name, addr, Origin::AdressBook)?; if modified != Modifier::None { @@ -1043,6 +1040,21 @@ pub fn addr_equals_self(context: &Context, addr: impl AsRef) -> bool { false } +fn split_address_book(book: &str) -> Vec<(&str, &str)> { + book.lines() + .chunks(2) + .into_iter() + .filter_map(|mut chunk| { + let name = chunk.next().unwrap(); + let addr = match chunk.next() { + Some(a) => a, + None => return None, + }; + Some((name, addr)) + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -1078,4 +1090,14 @@ mod tests { fn test_get_first_name() { assert_eq!(get_first_name("John Doe"), "John"); } + + #[test] + fn test_split_address_book() { + let book = "Name one\nAddress one\nName two\nAddress two\nrest name"; + let list = split_address_book(&book); + assert_eq!( + list, + vec![("Name one", "Address one"), ("Name two", "Address two")] + ) + } } From d8630b50297b6c8acff58c6db4d506cc6373cf7e Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sat, 7 Sep 2019 22:21:38 +0200 Subject: [PATCH 2/4] fix reading of unknown/outdated origin --- src/contact.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/contact.rs b/src/contact.rs index dea4761b7..974d5ddbd 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -109,7 +109,10 @@ impl ToSql for Origin { impl FromSql for Origin { fn column_result(col: ValueRef) -> FromSqlResult { let inner = FromSql::column_result(col)?; - FromPrimitive::from_i64(inner).ok_or(FromSqlError::InvalidType) + match FromPrimitive::from_i64(inner) { + Some(res) => Ok(res), + None => Ok(Origin::Unknown) + } } } From d07ef01204ff48820dd2102dce899942258b678f Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sat, 7 Sep 2019 23:25:19 +0200 Subject: [PATCH 3/4] cargo fmt --- src/contact.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contact.rs b/src/contact.rs index 974d5ddbd..8f8e0d805 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -111,7 +111,7 @@ impl FromSql for Origin { let inner = FromSql::column_result(col)?; match FromPrimitive::from_i64(inner) { Some(res) => Ok(res), - None => Ok(Origin::Unknown) + None => Ok(Origin::Unknown), } } } From 00e5ddd6f00d373a3cebc38fc181367517a8666b Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Sun, 8 Sep 2019 11:29:40 +0200 Subject: [PATCH 4/4] make enum reading from the db more robust --- deltachat_derive/src/lib.rs | 2 +- src/constants.rs | 12 ++++++++++++ src/contact.rs | 27 +++++++-------------------- src/job.rs | 26 ++++++++++++++++++++++++++ src/message.rs | 6 ++++++ 5 files changed, 52 insertions(+), 21 deletions(-) diff --git a/deltachat_derive/src/lib.rs b/deltachat_derive/src/lib.rs index 6fde011b7..f911b9fa9 100644 --- a/deltachat_derive/src/lib.rs +++ b/deltachat_derive/src/lib.rs @@ -36,7 +36,7 @@ pub fn from_sql_derive(input: TokenStream) -> TokenStream { impl rusqlite::types::FromSql for #name { fn column_result(col: rusqlite::types::ValueRef) -> rusqlite::types::FromSqlResult { let inner = rusqlite::types::FromSql::column_result(col)?; - num_traits::FromPrimitive::from_i64(inner).ok_or(rusqlite::types::FromSqlError::InvalidType) + Ok(num_traits::FromPrimitive::from_i64(inner).unwrap_or_default()) } } }; diff --git a/src/constants.rs b/src/constants.rs index 595fcab41..6a066351e 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -18,6 +18,12 @@ pub enum MoveState { Moving = 3, } +impl Default for MoveState { + fn default() -> Self { + MoveState::Undefined + } +} + // some defaults const DC_E2EE_DEFAULT_ENABLED: i32 = 1; pub const DC_MDNS_DEFAULT_ENABLED: i32 = 1; @@ -220,6 +226,12 @@ pub enum Viewtype { File = 60, } +impl Default for Viewtype { + fn default() -> Self { + Viewtype::Unknown + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/contact.rs b/src/contact.rs index 8f8e0d805..0e7371ae5 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -1,7 +1,6 @@ +use deltachat_derive::*; use itertools::Itertools; -use num_traits::{FromPrimitive, ToPrimitive}; use rusqlite; -use rusqlite::types::*; use crate::aheader::EncryptPreference; use crate::config::Config; @@ -58,7 +57,9 @@ pub struct Contact<'a> { } /// Possible origins of a contact. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, FromPrimitive, ToPrimitive)] +#[derive( + Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, FromPrimitive, ToPrimitive, FromSql, ToSql, +)] #[repr(i32)] pub enum Origin { Unknown = 0, @@ -96,23 +97,9 @@ pub enum Origin { ManuallyCreated = 0x4000000, } -impl ToSql for Origin { - fn to_sql(&self) -> rusqlite::Result { - let num: i64 = self - .to_i64() - .expect("impossible: Origin -> i64 conversion failed"); - - Ok(ToSqlOutput::Owned(Value::Integer(num))) - } -} - -impl FromSql for Origin { - fn column_result(col: ValueRef) -> FromSqlResult { - let inner = FromSql::column_result(col)?; - match FromPrimitive::from_i64(inner) { - Some(res) => Ok(res), - None => Ok(Origin::Unknown), - } +impl Default for Origin { + fn default() -> Self { + Origin::Unknown } } diff --git a/src/job.rs b/src/job.rs index bacb73c1b..905d270cd 100644 --- a/src/job.rs +++ b/src/job.rs @@ -25,13 +25,22 @@ use crate::x::*; #[derive(Debug, Display, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive, FromSql, ToSql)] #[repr(i32)] enum Thread { + Unknown = 0, Imap = 100, Smtp = 5000, } +impl Default for Thread { + fn default() -> Self { + Thread::Unknown + } +} + #[derive(Debug, Display, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive, FromSql, ToSql)] #[repr(i32)] pub enum Action { + Unknown = 0, + // Jobs in the INBOX-thread, range from DC_IMAP_THREAD..DC_IMAP_THREAD+999 Housekeeping = 105, // low priority ... DeleteMsgOnImap = 110, @@ -50,11 +59,19 @@ pub enum Action { SendMsgToSmtp = 5901, // ... high priority } +impl Default for Action { + fn default() -> Self { + Action::Unknown + } +} + impl From for Thread { fn from(action: Action) -> Thread { use Action::*; match action { + Unknown => Thread::Unknown, + Housekeeping => Thread::Imap, DeleteMsgOnImap => Thread::Imap, MarkseenMdnOnImap => Thread::Imap, @@ -844,6 +861,9 @@ fn job_perform(context: &Context, thread: Thread, probe_network: bool) { job.try_again = 0; match job.action { + Action::Unknown => { + warn!(context, 0, "Unknown job id found"); + } Action::SendMsgToSmtp => job.do_DC_JOB_SEND(context), Action::DeleteMsgOnImap => job.do_DC_JOB_DELETE_MSG_ON_IMAP(context), Action::MarkseenMsgOnImap => job.do_DC_JOB_MARKSEEN_MSG_ON_IMAP(context), @@ -1062,6 +1082,11 @@ pub fn job_add( param: Params, delay_seconds: i64, ) { + if action != Action::Unknown { + error!(context, 0, "Invalid action passed to job_add"); + return; + } + let timestamp = time(); let thread: Thread = action.into(); @@ -1082,6 +1107,7 @@ pub fn job_add( match thread { Thread::Imap => interrupt_imap_idle(context), Thread::Smtp => interrupt_smtp_idle(context), + Thread::Unknown => {} } } diff --git a/src/message.rs b/src/message.rs index 89ecd00b3..92227cdca 100644 --- a/src/message.rs +++ b/src/message.rs @@ -38,6 +38,12 @@ pub enum MessageState { OutMdnRcvd = 28, } +impl Default for MessageState { + fn default() -> Self { + MessageState::Undefined + } +} + impl From for LotState { fn from(s: MessageState) -> Self { use MessageState::*;