Merge pull request #458 from deltachat/fix-unknown

fix reading unknown origin

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<Self> {
                 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())
             }
         }
     };

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::*;

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,20 +97,9 @@ pub enum Origin {
     ManuallyCreated = 0x4000000,
 }
 
-impl ToSql for Origin {
+impl Default for Origin {
-    fn to_sql(&self) -> rusqlite::Result<ToSqlOutput> {
+    fn default() -> Self {
-        let num: i64 = self
+        Origin::Unknown
-            .to_i64()
-            .expect("impossible: Origin -> i64 conversion failed");
-
-        Ok(ToSqlOutput::Owned(Value::Integer(num)))
-    }
-}
-
-impl FromSql for Origin {
-    fn column_result(col: ValueRef) -> FromSqlResult<Self> {
-        let inner = FromSql::column_result(col)?;
-        FromPrimitive::from_i64(inner).ok_or(FromSqlError::InvalidType)
     }
 }
 
@@ -341,19 +331,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 !name.as_ref().is_empty() {
-                    if origin >= row_origin && name.as_ref() != row_name {
+                    if !row_name.is_empty() {
+                        if origin >= row_origin && name.as_ref() != row_name {
+                            update_name = true;
+                        }
+                    } else {
                         update_name = true;
                     }
-                } else {
+                    if origin == Origin::IncomingUnknownFrom && name.as_ref() != row_authname {
-                    update_name = true;
+                        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 +386,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 +424,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<str>) -> Result<usize> {
+    pub fn add_address_book(context: &Context, addr_book: impl AsRef<str>) -> Result<usize> {
         let mut modify_cnt = 0;
 
-        for chunk in &adr_book.as_ref().lines().chunks(2) {
+        for (name, addr) in split_address_book(addr_book.as_ref()).into_iter() {
-            let chunk = chunk.collect::<Vec<_>>();
-            if chunk.len() < 2 {
-                break;
-            }
-            let name = chunk[0];
-            let addr = chunk[1];
             let name = normalize_name(name);
             let (_, modified) = Contact::add_or_lookup(context, name, addr, Origin::AdressBook)?;
             if modified != Modifier::None {
@@ -1043,6 +1030,21 @@ pub fn addr_equals_self(context: &Context, addr: impl AsRef<str>) -> 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 +1080,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")]
+        )
+    }
 }

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<Action> for Thread {
     fn from(action: Action) -> Thread {
         use Action::*;
 
         match action {
+            Unknown => Thread::Unknown,
+
             Housekeeping => Thread::Imap,
             DeleteMsgOnImap => Thread::Imap,
             MarkseenMdnOnImap => Thread::Imap,
@@ -842,6 +859,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),
@@ -1060,6 +1080,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();
 
@@ -1080,6 +1105,7 @@ pub fn job_add(
     match thread {
         Thread::Imap => interrupt_imap_idle(context),
         Thread::Smtp => interrupt_smtp_idle(context),
+        Thread::Unknown => {}
     }
 }
 

src/message.rs

@@ -38,6 +38,12 @@ pub enum MessageState {
     OutMdnRcvd = 28,
 }
 
+impl Default for MessageState {
+    fn default() -> Self {
+        MessageState::Undefined
+    }
+}
+
 impl From<MessageState> for LotState {
     fn from(s: MessageState) -> Self {
         use MessageState::*;