address @link2xt comment

and otherwise properly fetch the last messages to determine the last seen uid.
also add some tracing.

.gitignore

@@ -23,3 +23,5 @@ python/liveconfig*
 # ignore doxgen generated files
 deltachat-ffi/html
 deltachat-ffi/xml
+
+.rsynclist

appveyor.yml

@@ -1,5 +1,4 @@
 environment:
-  RUST_BACKTRACE: full
   matrix:
     - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
 

deltachat-ffi/src/lib.rs

@@ -24,7 +24,7 @@ use num_traits::{FromPrimitive, ToPrimitive};
 
 use deltachat::constants::DC_MSG_ID_LAST_SPECIAL;
 use deltachat::contact::Contact;
-use deltachat::context::{Context, ContextBuilder};
+use deltachat::context::Context;
 use deltachat::dc_tools::{
     as_path, dc_strdup, to_opt_string_lossy, to_string_lossy, OsStrExt, StrExt,
 };
@@ -248,15 +248,22 @@ pub unsafe extern "C" fn dc_open(
     }
     let ffi_context = &*context;
     let rust_cb = move |_ctx: &Context, evt: Event| ffi_context.translate_cb(evt);
-    let mut builder = ContextBuilder::new(
-        Box::new(rust_cb),
-        ffi_context.os_name.clone(),
-        as_path(dbfile).to_path_buf(),
-    );
-    if !blobdir.is_null() && *blobdir != 0 {
-        builder = builder.blobdir(as_path(blobdir).to_path_buf());
-    }
-    match builder.create() {
+
+    let ctx = if blobdir.is_null() || *blobdir == 0 {
+        Context::new(
+            Box::new(rust_cb),
+            ffi_context.os_name.clone(),
+            as_path(dbfile).to_path_buf(),
+        )
+    } else {
+        Context::with_blobdir(
+            Box::new(rust_cb),
+            ffi_context.os_name.clone(),
+            as_path(dbfile).to_path_buf(),
+            as_path(blobdir).to_path_buf(),
+        )
+    };
+    match ctx {
         Ok(ctx) => {
             let mut inner_guard = ffi_context.inner.write().unwrap();
             *inner_guard = Some(ctx);

examples/repl/main.rs

@@ -366,12 +366,11 @@ fn main_0(args: Vec<String>) -> Result<(), failure::Error> {
         println!("Error: Bad arguments, expected [db-name].");
         return Err(format_err!("No db-name specified"));
     }
-    let context = ContextBuilder::new(
+    let context = Context::new(
         Box::new(receive_event),
         "CLI".into(),
         Path::new(&args[1]).to_path_buf(),
-    )
-    .create()?;
+    )?;
 
     println!("Delta Chat Core is awaiting your commands.");
 

examples/simple.rs

@@ -39,9 +39,8 @@ fn main() {
     let dir = tempdir().unwrap();
     let dbfile = dir.path().join("db.sqlite");
     println!("creating database {:?}", dbfile);
-    let ctx = ContextBuilder::new(Box::new(cb), "FakeOs".into(), dbfile)
-        .create()
-        .expect("Failed to create context");
+    let ctx =
+        Context::new(Box::new(cb), "FakeOs".into(), dbfile).expect("Failed to create context");
     let running = Arc::new(RwLock::new(true));
     let info = ctx.get_info();
     let duration = time::Duration::from_millis(4000);

python/tests/test_account.py

@@ -794,6 +794,7 @@ class TestOnlineAccount:
         chat2 = ac2.qr_join_chat(qr)
         assert chat2.id >= 10
         wait_securejoin_inviter_progress(ac1, 1000)
+        ac1._evlogger.get_matching("DC_EVENT_SECUREJOIN_MEMBER_ADDED")
 
         lp.sec("ac2: read member added message")
         msg = ac2.wait_next_incoming_message()

src/aheader.rs

@@ -44,8 +44,8 @@ impl str::FromStr for EncryptPreference {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "mutual" => Ok(EncryptPreference::Mutual),
-            "reset" => Ok(EncryptPreference::Reset),
-            _ => Ok(EncryptPreference::NoPreference),
+            "nopreference" => Ok(EncryptPreference::NoPreference),
+            _ => Err(()),
         }
     }
 }

src/blob.rs

@@ -32,11 +32,11 @@ impl<'a> BlobObject<'a> {
     ///
     /// # Errors
     ///
-    /// [BlobErrorKind::CreateFailure] is used when the file could not
+    /// [BlobError::CreateFailure] is used when the file could not
     /// be created.  You can expect [BlobError.cause] to contain an
     /// underlying error.
     ///
-    /// [BlobErrorKind::WriteFailure] is used when the file could not
+    /// [BlobError::WriteFailure] is used when the file could not
     /// be written to.  You can expect [BlobError.cause] to contain an
     /// underlying error.
     pub fn create(
@@ -48,7 +48,12 @@ impl<'a> BlobObject<'a> {
         let (stem, ext) = BlobObject::sanitise_name(suggested_name.as_ref());
         let (name, mut file) = BlobObject::create_new_file(&blobdir, &stem, &ext)?;
         file.write_all(data)
-            .map_err(|err| BlobError::new_write_failure(blobdir, &name, err))?;
+            .map_err(|err| BlobError::WriteFailure {
+                blobdir: blobdir.to_path_buf(),
+                blobname: name.clone(),
+                cause: err,
+                backtrace: failure::Backtrace::new(),
+            })?;
         let blob = BlobObject {
             blobdir,
             name: format!("$BLOBDIR/{}", name),
@@ -71,18 +76,25 @@ impl<'a> BlobObject<'a> {
                 Ok(file) => return Ok((name, file)),
                 Err(err) => {
                     if attempt == max_attempt {
-                        return Err(BlobError::new_create_failure(dir, &name, err));
+                        return Err(BlobError::CreateFailure {
+                            blobdir: dir.to_path_buf(),
+                            blobname: name,
+                            cause: err,
+                            backtrace: failure::Backtrace::new(),
+                        });
                     } else {
                         name = format!("{}-{}{}", stem, rand::random::<u32>(), ext);
                     }
                 }
             }
         }
-        Err(BlobError::new_create_failure(
-            dir,
-            &name,
-            format_err!("Unreachable code - supposedly"),
-        ))
+        // This is supposed to be unreachable, but the compiler doesn't know.
+        Err(BlobError::CreateFailure {
+            blobdir: dir.to_path_buf(),
+            blobname: name,
+            cause: std::io::Error::new(std::io::ErrorKind::Other, "supposedly unreachable"),
+            backtrace: failure::Backtrace::new(),
+        })
     }
 
     /// Creates a new blob object with unique name by copying an existing file.
@@ -95,24 +107,35 @@ impl<'a> BlobObject<'a> {
     /// # Errors
     ///
     /// In addition to the errors in [BlobObject::create] the
-    /// [BlobErrorKind::CopyFailure] is used when the data can not be
+    /// [BlobError::CopyFailure] is used when the data can not be
     /// copied.
     pub fn create_and_copy(
         context: &'a Context,
         src: impl AsRef<Path>,
     ) -> std::result::Result<BlobObject<'a>, BlobError> {
-        let mut src_file = fs::File::open(src.as_ref()).map_err(|err| {
-            BlobError::new_copy_failure(context.get_blobdir(), "", src.as_ref(), err)
+        let mut src_file = fs::File::open(src.as_ref()).map_err(|err| BlobError::CopyFailure {
+            blobdir: context.get_blobdir().to_path_buf(),
+            blobname: String::from(""),
+            src: src.as_ref().to_path_buf(),
+            cause: err,
+            backtrace: failure::Backtrace::new(),
         })?;
         let (stem, ext) = BlobObject::sanitise_name(&src.as_ref().to_string_lossy());
         let (name, mut dst_file) = BlobObject::create_new_file(context.get_blobdir(), &stem, &ext)?;
+        let name_for_err = name.clone();
         std::io::copy(&mut src_file, &mut dst_file).map_err(|err| {
             {
                 // Attempt to remove the failed file, swallow errors resulting from that.
-                let path = context.get_blobdir().join(&name);
+                let path = context.get_blobdir().join(&name_for_err);
                 fs::remove_file(path).ok();
             }
-            BlobError::new_copy_failure(context.get_blobdir(), &name, src.as_ref(), err)
+            BlobError::CopyFailure {
+                blobdir: context.get_blobdir().to_path_buf(),
+                blobname: name_for_err,
+                src: src.as_ref().to_path_buf(),
+                cause: err,
+                backtrace: failure::Backtrace::new(),
+            }
         })?;
         let blob = BlobObject {
             blobdir: context.get_blobdir(),
@@ -156,10 +179,10 @@ impl<'a> BlobObject<'a> {
     ///
     /// # Errors
     ///
-    /// [BlobErrorKind::WrongBlobdir] is used if the path is not in
+    /// [BlobError::WrongBlobdir] is used if the path is not in
     /// the blob directory.
     ///
-    /// [BlobErrorKind::WrongName] is used if the file name does not
+    /// [BlobError::WrongName] is used if the file name does not
     /// remain identical after sanitisation.
     pub fn from_path(
         context: &Context,
@@ -168,13 +191,21 @@ impl<'a> BlobObject<'a> {
         let rel_path = path
             .as_ref()
             .strip_prefix(context.get_blobdir())
-            .map_err(|_| BlobError::new_wrong_blobdir(context.get_blobdir(), path.as_ref()))?;
+            .map_err(|_| BlobError::WrongBlobdir {
+                blobdir: context.get_blobdir().to_path_buf(),
+                src: path.as_ref().to_path_buf(),
+                backtrace: failure::Backtrace::new(),
+            })?;
         if !BlobObject::is_acceptible_blob_name(&rel_path) {
-            return Err(BlobError::new_wrong_name(path.as_ref()));
+            return Err(BlobError::WrongName {
+                blobname: path.as_ref().to_path_buf(),
+                backtrace: failure::Backtrace::new(),
+            });
         }
-        let name = rel_path
-            .to_str()
-            .ok_or_else(|| BlobError::new_wrong_name(path.as_ref()))?;
+        let name = rel_path.to_str().ok_or_else(|| BlobError::WrongName {
+            blobname: path.as_ref().to_path_buf(),
+            backtrace: failure::Backtrace::new(),
+        })?;
         BlobObject::from_name(context, name.to_string())
     }
 
@@ -187,7 +218,7 @@ impl<'a> BlobObject<'a> {
     ///
     /// # Errors
     ///
-    /// [BlobErrorKind::WrongName] is used if the name is not a valid
+    /// [BlobError::WrongName] is used if the name is not a valid
     /// blobname, i.e. if [BlobObject::sanitise_name] does modify the
     /// provided name.
     pub fn from_name(
@@ -199,7 +230,10 @@ impl<'a> BlobObject<'a> {
             false => name,
         };
         if !BlobObject::is_acceptible_blob_name(&name) {
-            return Err(BlobError::new_wrong_name(name));
+            return Err(BlobError::WrongName {
+                blobname: PathBuf::from(name),
+                backtrace: failure::Backtrace::new(),
+            });
         }
         Ok(BlobObject {
             blobdir: context.get_blobdir(),
@@ -324,171 +358,50 @@ impl<'a> fmt::Display for BlobObject<'a> {
 }
 
 /// Errors for the [BlobObject].
-///
-/// To keep the return type small and thus the happy path fast this
-/// stores everything on the heap.
-#[derive(Debug)]
-pub struct BlobError {
-    inner: Box<BlobErrorInner>,
-}
-
-#[derive(Debug)]
-struct BlobErrorInner {
-    kind: BlobErrorKind,
-    data: BlobErrorData,
-    backtrace: failure::Backtrace,
-}
-
-/// Error kind for [BlobError].
-///
-/// Each error kind has associated data in the [BlobErrorData].
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum BlobErrorKind {
-    /// Failed to create the blob.
-    CreateFailure,
-    /// Failed to write data to blob.
-    WriteFailure,
-    /// Failed to copy data to blob.
-    CopyFailure,
-    /// Blob is not in the blobdir.
-    WrongBlobdir,
-    /// Blob has a bad name.
-    ///
-    /// E.g. the name is not sanitised correctly or contains a
-    /// sub-directory.
-    WrongName,
-}
-
-/// Associated data for each [BlobError] error kind.
-///
-/// This is not stored directly on the [BlobErrorKind] so that the
-/// kind can stay trivially Copy and Eq.  It is however possible to
-/// create a [BlobError] with mismatching [BlobErrorKind] and
-/// [BlobErrorData], don't do that.
-///
-/// Any blobname stored here is the bare name, without the `$BLOBDIR`
-/// prefix.  All data is owned so that errors do not need to be tied
-/// to any lifetimes.
-#[derive(Debug)]
-enum BlobErrorData {
+#[derive(Fail, Debug)]
+pub enum BlobError {
     CreateFailure {
         blobdir: PathBuf,
         blobname: String,
-        cause: failure::Error,
+        #[cause]
+        cause: std::io::Error,
+        backtrace: failure::Backtrace,
     },
     WriteFailure {
         blobdir: PathBuf,
         blobname: String,
-        cause: failure::Error,
+        #[cause]
+        cause: std::io::Error,
+        backtrace: failure::Backtrace,
     },
     CopyFailure {
         blobdir: PathBuf,
         blobname: String,
         src: PathBuf,
-        cause: failure::Error,
+        #[cause]
+        cause: std::io::Error,
+        backtrace: failure::Backtrace,
     },
     WrongBlobdir {
         blobdir: PathBuf,
         src: PathBuf,
+        backtrace: failure::Backtrace,
     },
     WrongName {
         blobname: PathBuf,
+        backtrace: failure::Backtrace,
     },
 }
 
-impl BlobError {
-    pub fn kind(&self) -> BlobErrorKind {
-        self.inner.kind
-    }
-
-    fn new_create_failure(
-        blobdir: impl Into<PathBuf>,
-        blobname: impl Into<String>,
-        cause: impl Into<failure::Error>,
-    ) -> BlobError {
-        BlobError {
-            inner: Box::new(BlobErrorInner {
-                kind: BlobErrorKind::CreateFailure,
-                data: BlobErrorData::CreateFailure {
-                    blobdir: blobdir.into(),
-                    blobname: blobname.into(),
-                    cause: cause.into(),
-                },
-                backtrace: failure::Backtrace::new(),
-            }),
-        }
-    }
-
-    fn new_write_failure(
-        blobdir: impl Into<PathBuf>,
-        blobname: impl Into<String>,
-        cause: impl Into<failure::Error>,
-    ) -> BlobError {
-        BlobError {
-            inner: Box::new(BlobErrorInner {
-                kind: BlobErrorKind::WriteFailure,
-                data: BlobErrorData::WriteFailure {
-                    blobdir: blobdir.into(),
-                    blobname: blobname.into(),
-                    cause: cause.into(),
-                },
-                backtrace: failure::Backtrace::new(),
-            }),
-        }
-    }
-
-    fn new_copy_failure(
-        blobdir: impl Into<PathBuf>,
-        blobname: impl Into<String>,
-        src: impl Into<PathBuf>,
-        cause: impl Into<failure::Error>,
-    ) -> BlobError {
-        BlobError {
-            inner: Box::new(BlobErrorInner {
-                kind: BlobErrorKind::CopyFailure,
-                data: BlobErrorData::CopyFailure {
-                    blobdir: blobdir.into(),
-                    blobname: blobname.into(),
-                    src: src.into(),
-                    cause: cause.into(),
-                },
-                backtrace: failure::Backtrace::new(),
-            }),
-        }
-    }
-
-    fn new_wrong_blobdir(blobdir: impl Into<PathBuf>, src: impl Into<PathBuf>) -> BlobError {
-        BlobError {
-            inner: Box::new(BlobErrorInner {
-                kind: BlobErrorKind::WrongBlobdir,
-                data: BlobErrorData::WrongBlobdir {
-                    blobdir: blobdir.into(),
-                    src: src.into(),
-                },
-                backtrace: failure::Backtrace::new(),
-            }),
-        }
-    }
-
-    fn new_wrong_name(blobname: impl Into<PathBuf>) -> BlobError {
-        BlobError {
-            inner: Box::new(BlobErrorInner {
-                kind: BlobErrorKind::WrongName,
-                data: BlobErrorData::WrongName {
-                    blobname: blobname.into(),
-                },
-                backtrace: failure::Backtrace::new(),
-            }),
-        }
-    }
-}
-
+// Implementing Display is done by hand because the failure
+// #[fail(display = "...")] syntax does not allow using
+// `blobdir.display()`.
 impl fmt::Display for BlobError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         // Match on the data rather than kind, they are equivalent for
         // identifying purposes but contain the actual data we need.
-        match &self.inner.data {
-            BlobErrorData::CreateFailure {
+        match &self {
+            BlobError::CreateFailure {
                 blobdir, blobname, ..
             } => write!(
                 f,
@@ -496,7 +409,7 @@ impl fmt::Display for BlobError {
                 blobname,
                 blobdir.display()
             ),
-            BlobErrorData::WriteFailure {
+            BlobError::WriteFailure {
                 blobdir, blobname, ..
             } => write!(
                 f,
@@ -504,7 +417,7 @@ impl fmt::Display for BlobError {
                 blobname,
                 blobdir.display()
             ),
-            BlobErrorData::CopyFailure {
+            BlobError::CopyFailure {
                 blobdir,
                 blobname,
                 src,
@@ -516,34 +429,19 @@ impl fmt::Display for BlobError {
                 blobname,
                 blobdir.display(),
             ),
-            BlobErrorData::WrongBlobdir { blobdir, src } => write!(
+            BlobError::WrongBlobdir { blobdir, src, .. } => write!(
                 f,
                 "File path {} is not in blobdir {}",
                 src.display(),
                 blobdir.display(),
             ),
-            BlobErrorData::WrongName { blobname } => {
+            BlobError::WrongName { blobname, .. } => {
                 write!(f, "Blob has a bad name: {}", blobname.display(),)
             }
         }
     }
 }
 
-impl failure::Fail for BlobError {
-    fn cause(&self) -> Option<&dyn failure::Fail> {
-        match &self.inner.data {
-            BlobErrorData::CreateFailure { cause, .. }
-            | BlobErrorData::WriteFailure { cause, .. }
-            | BlobErrorData::CopyFailure { cause, .. } => Some(cause.as_fail()),
-            _ => None,
-        }
-    }
-
-    fn backtrace(&self) -> Option<&failure::Backtrace> {
-        Some(&self.inner.backtrace)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/chat.rs

@@ -5,7 +5,7 @@ use std::path::{Path, PathBuf};
 use itertools::Itertools;
 use num_traits::FromPrimitive;
 
-use crate::blob::{BlobErrorKind, BlobObject};
+use crate::blob::{BlobError, BlobObject};
 use crate::chatlist::*;
 use crate::config::*;
 use crate::constants::*;
@@ -637,7 +637,7 @@ pub fn create_or_lookup_by_contact_id(
         context,
         &context.sql,
         format!(
-            "INSERT INTO chats (type, name, param, blocked, grpid) VALUES({}, '{}', '{}', {}, '{}')",
+            "INSERT INTO chats (type, name, param, blocked, grpid, created_timestamp) VALUES({}, '{}', '{}', {}, '{}', {})",
             100,
             chat_name,
             match contact_id {
@@ -650,6 +650,7 @@ pub fn create_or_lookup_by_contact_id(
             },
             create_blocked as u8,
             contact.get_addr(),
+            time(),
         ),
         params![],
     )?;
@@ -1388,7 +1389,7 @@ pub fn create_group_chat(
     sql::execute(
         context,
         &context.sql,
-        "INSERT INTO chats (type, name, grpid, param) VALUES(?, ?, ?, \'U=1\');",
+        "INSERT INTO chats (type, name, grpid, param, created_timestamp) VALUES(?, ?, ?, \'U=1\', ?);",
         params![
             if verified != VerifiedStatus::Unverified {
                 Chattype::VerifiedGroup
@@ -1396,7 +1397,8 @@ pub fn create_group_chat(
                 Chattype::Group
             },
             chat_name.as_ref(),
-            grpid
+            grpid,
+            time(),
         ],
     )?;
 
@@ -1486,7 +1488,7 @@ pub(crate) fn add_contact_to_chat_ex(
     let self_addr = context
         .get_config(Config::ConfiguredAddr)
         .unwrap_or_default();
-    if contact.get_addr() == &self_addr {
+    if addr_cmp(contact.get_addr(), &self_addr) {
         // ourself is added using DC_CONTACT_ID_SELF, do not add this address explicitly.
         // if SELF is not in the group, members cannot be added at all.
         warn!(
@@ -1795,8 +1797,8 @@ pub fn set_chat_profile_image(
         ));
     } else {
         let image_blob = BlobObject::from_path(context, Path::new(new_image.as_ref())).or_else(
-            |err| match err.kind() {
-                BlobErrorKind::WrongBlobdir => {
+            |err| match err {
+                BlobError::WrongBlobdir { .. } => {
                     BlobObject::create_and_copy(context, Path::new(new_image.as_ref()))
                 }
                 _ => Err(err),

src/chatlist.rs

@@ -6,7 +6,7 @@ use crate::contact::*;
 use crate::context::*;
 use crate::error::Result;
 use crate::lot::Lot;
-use crate::message::{Message, MsgId};
+use crate::message::{Message, MessageState, MsgId};
 use crate::stock::StockMessage;
 
 /// An object representing a single chatlist in memory.
@@ -119,46 +119,42 @@ impl Chatlist {
         let mut ids = if let Some(query_contact_id) = query_contact_id {
             // show chats shared with a given contact
             context.sql.query_map(
-                concat!(
-                    "SELECT c.id, m.id",
-                    " FROM chats c",
-                    " LEFT JOIN msgs m",
-                    "        ON c.id=m.chat_id",
-                    "       AND m.timestamp=(",
-                    "               SELECT MAX(timestamp)",
-                    "                 FROM msgs",
-                    "                WHERE chat_id=c.id",
-                    "                  AND hidden=0)",
-                    " WHERE c.id>9",
-                    "   AND c.blocked=0",
-                    "   AND c.id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)",
-                    " GROUP BY c.id",
-                    " ORDER BY IFNULL(m.timestamp,0) DESC, m.id DESC;"
-                ),
-                params![query_contact_id as i32],
+                "SELECT c.id, m.id
+                 FROM chats c
+                 LEFT JOIN msgs m
+                        ON c.id=m.chat_id
+                       AND m.timestamp=(
+                               SELECT MAX(timestamp)
+                                 FROM msgs
+                                WHERE chat_id=c.id
+                                  AND (hidden=0 OR state=?))
+                 WHERE c.id>9
+                   AND c.blocked=0
+                   AND c.id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)
+                 GROUP BY c.id
+                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                params![MessageState::OutDraft, query_contact_id as i32],
                 process_row,
                 process_rows,
             )?
         } else if 0 != listflags & DC_GCL_ARCHIVED_ONLY {
             // show archived chats
             context.sql.query_map(
-                concat!(
-                    "SELECT c.id, m.id",
-                    " FROM chats c",
-                    " LEFT JOIN msgs m",
-                    "        ON c.id=m.chat_id",
-                    "       AND m.timestamp=(",
-                    "               SELECT MAX(timestamp)",
-                    "                 FROM msgs",
-                    "                WHERE chat_id=c.id",
-                    "                  AND hidden=0)",
-                    " WHERE c.id>9",
-                    "   AND c.blocked=0",
-                    "   AND c.archived=1",
-                    " GROUP BY c.id",
-                    " ORDER BY IFNULL(m.timestamp,0) DESC, m.id DESC;"
-                ),
-                params![],
+                "SELECT c.id, m.id
+                 FROM chats c
+                 LEFT JOIN msgs m
+                        ON c.id=m.chat_id
+                       AND m.timestamp=(
+                               SELECT MAX(timestamp)
+                                 FROM msgs
+                                WHERE chat_id=c.id
+                                  AND (hidden=0 OR state=?))
+                 WHERE c.id>9
+                   AND c.blocked=0
+                   AND c.archived=1
+                 GROUP BY c.id
+                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                params![MessageState::OutDraft],
                 process_row,
                 process_rows,
             )?
@@ -168,46 +164,42 @@ impl Chatlist {
 
             let str_like_cmd = format!("%{}%", query);
             context.sql.query_map(
-                concat!(
-                    "SELECT c.id, m.id",
-                    " FROM chats c",
-                    " LEFT JOIN msgs m",
-                    "        ON c.id=m.chat_id",
-                    "       AND m.timestamp=(",
-                    "               SELECT MAX(timestamp)",
-                    "                 FROM msgs",
-                    "                WHERE chat_id=c.id",
-                    "                  AND hidden=0)",
-                    " WHERE c.id>9",
-                    "   AND c.blocked=0",
-                    "   AND c.name LIKE ?",
-                    " GROUP BY c.id",
-                    " ORDER BY IFNULL(m.timestamp,0) DESC, m.id DESC;"
-                ),
-                params![str_like_cmd],
+                "SELECT c.id, m.id
+                 FROM chats c
+                 LEFT JOIN msgs m
+                        ON c.id=m.chat_id
+                       AND m.timestamp=(
+                               SELECT MAX(timestamp)
+                                 FROM msgs
+                                WHERE chat_id=c.id
+                                  AND (hidden=0 OR state=?))
+                 WHERE c.id>9
+                   AND c.blocked=0
+                   AND c.name LIKE ?
+                 GROUP BY c.id
+                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                params![MessageState::OutDraft, str_like_cmd],
                 process_row,
                 process_rows,
             )?
         } else {
             //  show normal chatlist
             let mut ids = context.sql.query_map(
-                concat!(
-                    "SELECT c.id, m.id",
-                    " FROM chats c",
-                    " LEFT JOIN msgs m",
-                    "        ON c.id=m.chat_id",
-                    "       AND m.timestamp=(",
-                    "               SELECT MAX(timestamp)",
-                    "                 FROM msgs",
-                    "                WHERE chat_id=c.id",
-                    "                  AND hidden=0)",
-                    " WHERE c.id>9",
-                    "   AND c.blocked=0",
-                    "   AND c.archived=0",
-                    " GROUP BY c.id",
-                    " ORDER BY IFNULL(m.timestamp,0) DESC, m.id DESC;"
-                ),
-                params![],
+                "SELECT c.id, m.id
+                 FROM chats c
+                 LEFT JOIN msgs m
+                        ON c.id=m.chat_id
+                       AND m.timestamp=(
+                               SELECT MAX(timestamp)
+                                 FROM msgs
+                                WHERE chat_id=c.id
+                                  AND (hidden=0 OR state=?))
+                 WHERE c.id>9
+                   AND c.blocked=0
+                   AND c.archived=0
+                 GROUP BY c.id
+                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                params![MessageState::OutDraft],
                 process_row,
                 process_rows,
             )?;
@@ -297,7 +289,7 @@ impl Chatlist {
         let lastmsg_id = self.ids[index].1;
         let mut lastcontact = None;
 
-        let mut lastmsg = if let Ok(lastmsg) = Message::load_from_db(context, lastmsg_id) {
+        let lastmsg = if let Ok(lastmsg) = Message::load_from_db(context, lastmsg_id) {
             if lastmsg.from_id != DC_CONTACT_ID_SELF
                 && (chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup)
             {
@@ -309,16 +301,6 @@ impl Chatlist {
             None
         };
 
-        if let Ok(draft) = get_draft(context, chat.id) {
-            if draft.is_some()
-                && (lastmsg.is_none()
-                    || draft.as_ref().unwrap().timestamp_sort
-                        > lastmsg.as_ref().unwrap().timestamp_sort)
-            {
-                lastmsg = draft;
-            }
-        }
-
         if chat.id == DC_CHAT_ID_ARCHIVED_LINK {
             ret.text2 = None;
         } else if lastmsg.is_none() || lastmsg.as_ref().unwrap().from_id == DC_CONTACT_ID_UNDEFINED
@@ -362,3 +344,44 @@ fn get_last_deaddrop_fresh_msg(context: &Context) -> Option<MsgId> {
         params![],
     )
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::chat;
+    use crate::test_utils::*;
+
+    #[test]
+    fn test_try_load() {
+        let t = dummy_context();
+        let chat_id1 = create_group_chat(&t.ctx, VerifiedStatus::Unverified, "a chat").unwrap();
+        let chat_id2 = create_group_chat(&t.ctx, VerifiedStatus::Unverified, "b chat").unwrap();
+        let chat_id3 = create_group_chat(&t.ctx, VerifiedStatus::Unverified, "c chat").unwrap();
+
+        // check that the chatlist starts with the most recent message
+        let chats = Chatlist::try_load(&t.ctx, 0, None, None).unwrap();
+        assert_eq!(chats.len(), 3);
+        assert_eq!(chats.get_chat_id(0), chat_id3);
+        assert_eq!(chats.get_chat_id(1), chat_id2);
+        assert_eq!(chats.get_chat_id(2), chat_id1);
+
+        // drafts are sorted to the top
+        let mut msg = Message::new(Viewtype::Text);
+        msg.set_text(Some("hello".to_string()));
+        set_draft(&t.ctx, chat_id2, Some(&mut msg));
+        let chats = Chatlist::try_load(&t.ctx, 0, None, None).unwrap();
+        assert_eq!(chats.get_chat_id(0), chat_id2);
+
+        // check chatlist query and archive functionality
+        let chats = Chatlist::try_load(&t.ctx, 0, Some("b"), None).unwrap();
+        assert_eq!(chats.len(), 1);
+
+        let chats = Chatlist::try_load(&t.ctx, DC_GCL_ARCHIVED_ONLY, None, None).unwrap();
+        assert_eq!(chats.len(), 0);
+
+        chat::archive(&t.ctx, chat_id1, true).ok();
+        let chats = Chatlist::try_load(&t.ctx, DC_GCL_ARCHIVED_ONLY, None, None).unwrap();
+        assert_eq!(chats.len(), 1);
+    }
+}

src/configure/mod.rs

@@ -355,13 +355,19 @@ pub fn JobConfigureImap(context: &Context) {
                 progress!(context, 900);
                 let create_mvbox = context.get_config_bool(Config::MvboxWatch)
                     || context.get_config_bool(Config::MvboxMove);
-                context
-                    .inbox_thread
-                    .read()
-                    .unwrap()
-                    .imap
-                    .configure_folders(context, create_mvbox);
-                true
+                let imap = &context.inbox_thread.read().unwrap().imap;
+                if let Err(err) = imap.ensure_configured_folders(context, create_mvbox) {
+                    warn!(context, "configuring folders failed: {:?}", err);
+                    false
+                } else {
+                    let res = imap.select_with_uidvalidity(context, "INBOX");
+                    if let Err(err) = res {
+                        error!(context, "could not read INBOX status: {:?}", err);
+                        false
+                    } else {
+                        true
+                    }
+                }
             }
             17 => {
                 progress!(context, 910);
@@ -383,11 +389,10 @@ pub fn JobConfigureImap(context: &Context) {
                 // (~30 seconds on a Moto G4 play) and might looks as if message sending is always that slow.
                 e2ee::ensure_secret_key_exists(context);
                 success = true;
-                info!(context, "Configure completed.");
+                info!(context, "key generation completed");
                 progress!(context, 940);
                 break; // We are done here
             }
-
             _ => {
                 error!(context, "Internal error: step counter out of bound",);
                 break;
@@ -410,24 +415,6 @@ pub fn JobConfigureImap(context: &Context) {
         context.smtp.clone().lock().unwrap().disconnect();
     }
 
-    /*
-    if !success {
-        // disconnect if configure did not succeed
-        if imap_connected_here {
-            // context.inbox.read().unwrap().disconnect(context);
-        }
-        if smtp_connected_here {
-            // context.smtp.clone().lock().unwrap().disconnect();
-        }
-    } else {
-        assert!(imap_connected_here && smtp_connected_here);
-        info!(
-            context,
-            0, "Keeping IMAP/SMTP connections open after successful configuration"
-        );
-    }
-    */
-
     // remember the entered parameters on success
     // and restore to last-entered on failure.
     // this way, the parameters visible to the ui are always in-sync with the current configuration.

src/contact.rs

@@ -272,7 +272,7 @@ impl Contact {
             .get_config(Config::ConfiguredAddr)
             .unwrap_or_default();
 
-        if addr_normalized == addr_self {
+        if addr_cmp(addr_normalized, addr_self) {
             return DC_CONTACT_ID_SELF;
         }
 
@@ -309,7 +309,7 @@ impl Contact {
             .get_config(Config::ConfiguredAddr)
             .unwrap_or_default();
 
-        if addr == addr_self {
+        if addr_cmp(addr, addr_self) {
             return Ok((DC_CONTACT_ID_SELF, sth_modified));
         }
 
@@ -1026,17 +1026,16 @@ fn cat_fingerprint(
 }
 
 pub fn addr_cmp(addr1: impl AsRef<str>, addr2: impl AsRef<str>) -> bool {
-    let norm1 = addr_normalize(addr1.as_ref());
-    let norm2 = addr_normalize(addr2.as_ref());
+    let norm1 = addr_normalize(addr1.as_ref()).to_lowercase();
+    let norm2 = addr_normalize(addr2.as_ref()).to_lowercase();
 
     norm1 == norm2
 }
 
 pub fn addr_equals_self(context: &Context, addr: impl AsRef<str>) -> bool {
     if !addr.as_ref().is_empty() {
-        let normalized_addr = addr_normalize(addr.as_ref());
         if let Some(self_addr) = context.get_config(Config::ConfiguredAddr) {
-            return normalized_addr == self_addr;
+            return addr_cmp(addr, self_addr);
         }
     }
     false
@@ -1088,6 +1087,10 @@ mod tests {
     fn test_normalize_addr() {
         assert_eq!(addr_normalize("mailto:john@doe.com"), "john@doe.com");
         assert_eq!(addr_normalize("  hello@world.com   "), "hello@world.com");
+
+        // normalisation preserves case to allow user-defined spelling.
+        // however, case is ignored on addr_cmp()
+        assert_ne!(addr_normalize("John@Doe.com"), "john@doe.com");
     }
 
     #[test]
@@ -1209,4 +1212,11 @@ mod tests {
         assert_eq!(contact.get_addr(), ""); // we're not configured
         assert!(!contact.is_blocked());
     }
+
+    #[test]
+    fn test_addr_cmp() {
+        assert!(addr_cmp("AA@AA.ORG", "aa@aa.ORG"));
+        assert!(addr_cmp(" aa@aa.ORG ", "AA@AA.ORG"));
+        assert!(addr_cmp(" mailto:AA@AA.ORG", "Aa@Aa.orG"));
+    }
 }

src/context.rs

@@ -15,7 +15,6 @@ use crate::imap::*;
 use crate::job::*;
 use crate::job_thread::JobThread;
 use crate::key::*;
-use crate::log;
 use crate::login_param::LoginParam;
 use crate::lot::Lot;
 use crate::message::{self, Message, MsgId};
@@ -63,7 +62,6 @@ pub struct Context {
     /// Mutex to avoid generating the key for the user more than once.
     pub generating_key_mutex: Mutex<()>,
     pub translated_stockstrings: RwLock<HashMap<usize, String>>,
-    pub logger: RwLock<log::Logger>,
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -96,92 +94,30 @@ pub fn get_info() -> HashMap<&'static str, String> {
     res
 }
 
-/// Builder for [Context].
-///
-/// # Example
-///
-/// ```
-/// use deltachat::context::ContextBuilder;
-/// let dir = tempfile::tempdir().unwrap();
-/// let dbfile = dir.path().join("my-context.db");
-/// let ctx = ContextBuilder::new(Box::new(|_, _| 0), "AppName".into(), dbfile)
-///     .blobdir(dir.path().join("my-context-blobs"))
-///     .logdir(dir.path().join("my-context-logs"))
-///     .create()
-///     .unwrap();
-/// assert_eq!(ctx.get_blobdir(), dir.path().join("my-context-blobs"));
-/// ```
-#[derive(DebugStub)]
-pub struct ContextBuilder {
-    #[debug_stub = "Callback"]
-    cb: Box<ContextCallback>,
-    os_name: String,
-    dbfile: PathBuf,
-    blobdir: PathBuf,
-    logdir: PathBuf,
-}
-
-impl ContextBuilder {
-    pub fn new(cb: Box<ContextCallback>, os_name: String, dbfile: PathBuf) -> Self {
-        let db_fname = OsString::from(dbfile.file_name().unwrap_or(&OsString::from("dc-context")));
-        let mut blob_fname = db_fname.clone();
-        blob_fname.push("-blobs");
-        let mut log_fname = db_fname.clone();
-        log_fname.push("-logs");
-        ContextBuilder {
-            cb,
-            os_name,
-            blobdir: dbfile.with_file_name(blob_fname),
-            logdir: dbfile.with_file_name(log_fname),
-            dbfile,
-        }
-    }
-
-    pub fn blobdir(mut self, path: PathBuf) -> Self {
-        self.blobdir = path;
-        self
-    }
-
-    pub fn logdir(mut self, path: PathBuf) -> Self {
-        self.logdir = path;
-        self
-    }
-
-    pub fn create(self) -> Result<Context> {
-        if !self.blobdir.exists() {
-            std::fs::create_dir_all(&self.blobdir)?;
-        }
-        if !self.logdir.exists() {
-            std::fs::create_dir_all(&self.logdir)?;
-        }
-        Context::new(
-            self.cb,
-            self.os_name,
-            self.dbfile,
-            self.blobdir,
-            self.logdir,
-        )
-    }
-}
-
 impl Context {
-    fn new(
+    /// Creates new context.
+    pub fn new(cb: Box<ContextCallback>, os_name: String, dbfile: PathBuf) -> Result<Context> {
+        let mut blob_fname = OsString::new();
+        blob_fname.push(dbfile.file_name().unwrap_or_default());
+        blob_fname.push("-blobs");
+        let blobdir = dbfile.with_file_name(blob_fname);
+        if !blobdir.exists() {
+            std::fs::create_dir_all(&blobdir)?;
+        }
+        Context::with_blobdir(cb, os_name, dbfile, blobdir)
+    }
+
+    pub fn with_blobdir(
         cb: Box<ContextCallback>,
         os_name: String,
         dbfile: PathBuf,
         blobdir: PathBuf,
-        logdir: PathBuf,
     ) -> Result<Context> {
         ensure!(
             blobdir.is_dir(),
             "Blobdir does not exist: {}",
             blobdir.display()
         );
-        ensure!(
-            logdir.is_dir(),
-            "Logdir does not exist: {}",
-            logdir.display()
-        );
         let ctx = Context {
             blobdir,
             dbfile,
@@ -214,7 +150,6 @@ impl Context {
             perform_inbox_jobs_needed: Arc::new(RwLock::new(false)),
             generating_key_mutex: Mutex::new(()),
             translated_stockstrings: RwLock::new(HashMap::new()),
-            logger: RwLock::new(log::Logger::new(logdir)?),
         };
 
         ensure!(
@@ -591,7 +526,7 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         std::fs::write(&dbfile, b"123").unwrap();
-        let res = ContextBuilder::new(Box::new(|_, _| 0), "FakeOs".into(), dbfile).create();
+        let res = Context::new(Box::new(|_, _| 0), "FakeOs".into(), dbfile);
         assert!(res.is_err());
     }
 
@@ -606,9 +541,7 @@ mod tests {
     fn test_blobdir_exists() {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
-        ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .create()
-            .unwrap();
+        Context::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile).unwrap();
         let blobdir = tmp.path().join("db.sqlite-blobs");
         assert!(blobdir.is_dir());
     }
@@ -619,7 +552,7 @@ mod tests {
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = tmp.path().join("db.sqlite-blobs");
         std::fs::write(&blobdir, b"123").unwrap();
-        let res = ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile).create();
+        let res = Context::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile);
         assert!(res.is_err());
     }
 
@@ -629,9 +562,7 @@ mod tests {
         let subdir = tmp.path().join("subdir");
         let dbfile = subdir.join("db.sqlite");
         let dbfile2 = dbfile.clone();
-        ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .create()
-            .unwrap();
+        Context::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile).unwrap();
         assert!(subdir.is_dir());
         assert!(dbfile2.is_file());
     }
@@ -641,9 +572,7 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = PathBuf::new();
-        let res = ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .blobdir(blobdir)
-            .create();
+        let res = Context::with_blobdir(Box::new(|_, _| 0), "FakeOS".into(), dbfile, blobdir);
         assert!(res.is_err());
     }
 
@@ -652,36 +581,10 @@ mod tests {
         let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
         let blobdir = tmp.path().join("blobs");
-        ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .blobdir(blobdir.clone())
-            .create()
-            .unwrap();
-        assert!(blobdir.is_dir());
-    }
-
-    #[test]
-    fn test_with_empty_logdir() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let logdir = PathBuf::new();
-        let res = ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .logdir(logdir)
-            .create();
+        let res = Context::with_blobdir(Box::new(|_, _| 0), "FakeOS".into(), dbfile, blobdir);
         assert!(res.is_err());
     }
 
-    #[test]
-    fn test_with_logdir_not_exists() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let logdir = tmp.path().join("logs");
-        ContextBuilder::new(Box::new(|_, _| 0), "FakeOS".into(), dbfile)
-            .logdir(logdir.clone())
-            .create()
-            .unwrap();
-        assert!(logdir.is_dir());
-    }
-
     #[test]
     fn no_crashes_on_context_deref() {
         let t = dummy_context();

src/dc_receive_imf.rs

@@ -392,6 +392,8 @@ unsafe fn add_parts(
             MessageState::InFresh
         };
         *to_id = DC_CONTACT_ID_SELF;
+        let mut needs_stop_ongoing_process = false;
+
         // handshake messages must be processed _before_ chats are created
         // (eg. contacs may be marked as verified)
         if mime_parser.lookup_field("Secure-Join").is_some() {
@@ -399,11 +401,24 @@ unsafe fn add_parts(
             msgrmsg = 1;
             *chat_id = 0;
             allow_creation = 1;
-            let handshake = handle_securejoin_handshake(context, mime_parser, *from_id);
-            if 0 != handshake & DC_HANDSHAKE_STOP_NORMAL_PROCESSING {
-                *hidden = 1;
-                *needs_delete_job = 0 != handshake & DC_HANDSHAKE_ADD_DELETE_JOB;
-                state = MessageState::InSeen;
+            match handle_securejoin_handshake(context, mime_parser, *from_id) {
+                Ok(ret) => {
+                    if ret.hide_this_msg {
+                        *hidden = 1;
+                        *needs_delete_job = ret.delete_this_msg;
+                        state = MessageState::InSeen;
+                    }
+                    if let Some(status) = ret.bob_securejoin_success {
+                        context.bob.write().unwrap().status = status as i32;
+                    }
+                    needs_stop_ongoing_process = ret.stop_ongoing_process;
+                }
+                Err(err) => {
+                    warn!(
+                        context,
+                        "Unexpected messaged passed to Secure-Join handshake protocol: {}", err
+                    );
+                }
             }
         }
 
@@ -501,6 +516,13 @@ unsafe fn add_parts(
         {
             state = MessageState::InNoticed;
         }
+
+        if needs_stop_ongoing_process {
+            // The Secure-Join protocol finished and the group
+            // creation handling is done.  Stopping the ongoing
+            // process will let dc_join_securejoin() return.
+            context.stop_ongoing();
+        }
     } else {
         // Outgoing
 
@@ -1439,7 +1461,7 @@ fn create_group_record(
     if sql::execute(
         context,
         &context.sql,
-        "INSERT INTO chats (type, name, grpid, blocked) VALUES(?, ?, ?, ?);",
+        "INSERT INTO chats (type, name, grpid, blocked, created_timestamp) VALUES(?, ?, ?, ?, ?);",
         params![
             if VerifiedStatus::Unverified != create_verified {
                 Chattype::VerifiedGroup
@@ -1449,6 +1471,7 @@ fn create_group_record(
             grpname.as_ref(),
             grpid.as_ref(),
             create_blocked,
+            time(),
         ],
     )
     .is_err()

src/dc_tools.rs

@@ -523,9 +523,13 @@ pub(crate) fn dc_create_folder(context: &Context, path: impl AsRef<std::path::Pa
 }
 
 /// Write a the given content to provied file path.
-pub(crate) fn dc_write_file(context: &Context, path: impl AsRef<Path>, buf: &[u8]) -> bool {
+pub(crate) fn dc_write_file(
+    context: &Context,
+    path: impl AsRef<Path>,
+    buf: &[u8],
+) -> Result<(), std::io::Error> {
     let path_abs = dc_get_abs_path(context, &path);
-    if let Err(err) = fs::write(&path_abs, buf) {
+    fs::write(&path_abs, buf).map_err(|err| {
         warn!(
             context,
             "Cannot write {} bytes to \"{}\": {}",
@@ -533,10 +537,8 @@ pub(crate) fn dc_write_file(context: &Context, path: impl AsRef<Path>, buf: &[u8
             path.as_ref().display(),
             err
         );
-        false
-    } else {
-        true
-    }
+        err
+    })
 }
 
 pub fn dc_read_file<P: AsRef<std::path::Path>>(
@@ -1323,7 +1325,7 @@ mod tests {
             dc_delete_file(context, "$BLOBDIR/foobar.dadada");
             dc_delete_file(context, "$BLOBDIR/foobar-folder");
         }
-        assert!(dc_write_file(context, "$BLOBDIR/foobar", b"content"));
+        assert!(dc_write_file(context, "$BLOBDIR/foobar", b"content").is_ok());
         assert!(dc_file_exist(context, "$BLOBDIR/foobar",));
         assert!(!dc_file_exist(context, "$BLOBDIR/foobarx"));
         assert_eq!(dc_get_filebytes(context, "$BLOBDIR/foobar"), 7);
@@ -1355,7 +1357,7 @@ mod tests {
         assert!(!dc_delete_file(context, "$BLOBDIR/foobar-folder"));
 
         let fn0 = "$BLOBDIR/data.data";
-        assert!(dc_write_file(context, &fn0, b"content"));
+        assert!(dc_write_file(context, &fn0, b"content").is_ok());
 
         assert!(dc_delete_file(context, &fn0));
         assert!(!dc_file_exist(context, &fn0));

src/error.rs

@@ -36,24 +36,6 @@ pub enum Error {
     InvalidMsgId,
     #[fail(display = "Watch folder not found {:?}", _0)]
     WatchFolderNotFound(String),
-    #[fail(display = "Connection Failed params: {}", _0)]
-    ImapConnectionFailed(String),
-    #[fail(display = "Could not get OAUTH token")]
-    ImapOauthError,
-    #[fail(display = "Could not login as {}", _0)]
-    ImapLoginFailed(String),
-    #[fail(display = "Cannot idle")]
-    ImapMissesIdle,
-    #[fail(display = "Imap IDLE protocol failed to init/complete")]
-    ImapIdleProtocolFailed(String),
-    #[fail(display = "Imap IDLE failed to select folder {:?}", _0)]
-    ImapSelectFailed(String),
-    #[fail(display = "Connect without configured params")]
-    ConnectWithoutConfigure,
-    #[fail(display = "imap operation attempted while imap is torn down")]
-    ImapInTeardown,
-    #[fail(display = "No IMAP Connection established")]
-    ImapNoConnection,
 }
 
 pub type Result<T> = std::result::Result<T, Error>;

src/imap.rs

@@ -18,7 +18,6 @@ use async_std::task;
 use crate::constants::*;
 use crate::context::Context;
 use crate::dc_receive_imf::dc_receive_imf;
-use crate::error::{Error, Result};
 use crate::events::Event;
 use crate::imap_client::*;
 use crate::job::{job_add, Action};
@@ -31,6 +30,77 @@ use crate::wrapmime;
 
 const DC_IMAP_SEEN: usize = 0x0001;
 
+type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Debug, Fail)]
+pub enum Error {
+    #[fail(display = "IMAP Could not obtain imap-session object.")]
+    NoSession,
+
+    #[fail(display = "IMAP Connect without configured params")]
+    ConnectWithoutConfigure,
+
+    #[fail(display = "IMAP Connection Failed params: {}", _0)]
+    ConnectionFailed(String),
+
+    #[fail(display = "IMAP No Connection established")]
+    NoConnection,
+
+    #[fail(display = "IMAP Could not get OAUTH token")]
+    OauthError,
+
+    #[fail(display = "IMAP Could not login as {}", _0)]
+    LoginFailed(String),
+
+    #[fail(display = "IMAP Could not fetch {}", _0)]
+    FetchFailed(#[cause] async_imap::error::Error),
+
+    #[fail(display = "IMAP IDLE protocol failed to init/complete")]
+    IdleProtocolFailed(#[cause] async_imap::error::Error),
+
+    #[fail(display = "IMAP server does not have IDLE capability")]
+    IdleAbilityMissing,
+
+    #[fail(display = "IMAP Connection Lost or no connection established")]
+    ConnectionLost,
+
+    #[fail(display = "IMAP close/expunge failed: {}", _0)]
+    CloseExpungeFailed(#[cause] async_imap::error::Error),
+
+    #[fail(display = "IMAP Folder name invalid: {:?}", _0)]
+    BadFolderName(String),
+
+    #[fail(display = "IMAP operation attempted while it is torn down")]
+    InTeardown,
+
+    #[fail(display = "IMAP operation attempted while it is torn down")]
+    SqlError(#[cause] rusqlite::Error),
+
+    #[fail(display = "IMAP got error from elsewhere: {:?}", _0)]
+    WrappedError(#[cause] crate::error::Error),
+
+    #[fail(display = "IMAP other error: {:?}", _0)]
+    Other(String),
+}
+
+impl From<rusqlite::Error> for Error {
+    fn from(err: rusqlite::Error) -> Error {
+        Error::SqlError(err)
+    }
+}
+
+impl From<crate::error::Error> for Error {
+    fn from(err: crate::error::Error) -> Error {
+        Error::WrappedError(err)
+    }
+}
+
+impl From<Error> for crate::error::Error {
+    fn from(err: Error) -> crate::error::Error {
+        crate::error::Error::Message(err.to_string())
+    }
+}
+
 #[derive(Debug, Display, Clone, Copy, PartialEq, Eq)]
 pub enum ImapActionResult {
     Failed,
@@ -40,6 +110,7 @@ pub enum ImapActionResult {
 }
 
 const PREFETCH_FLAGS: &str = "(UID ENVELOPE)";
+const JUST_UID: &str = "(UID)";
 const BODY_FLAGS: &str = "(FLAGS BODY.PEEK[])";
 const SELECT_ALL: &str = "1:*";
 
@@ -141,7 +212,7 @@ impl Imap {
     fn setup_handle_if_needed(&self, context: &Context) -> Result<()> {
         task::block_on(async move {
             if self.config.read().await.imap_server.is_empty() {
-                return Err(Error::ImapInTeardown);
+                return Err(Error::InTeardown);
             }
 
             if self.should_reconnect() {
@@ -203,7 +274,7 @@ impl Imap {
                             let res = client.authenticate("XOAUTH2", &auth).await;
                             res
                         } else {
-                            return Err(Error::ImapOauthError);
+                            return Err(Error::OauthError);
                         }
                     } else {
                         let res = client.login(imap_user, imap_pw).await;
@@ -223,7 +294,7 @@ impl Imap {
                     };
                     // IMAP connection failures are reported to users
                     emit_event!(context, Event::ErrorNetwork(message));
-                    return Err(Error::ImapConnectionFailed(err.to_string()));
+                    return Err(Error::ConnectionFailed(err.to_string()));
                 }
             };
 
@@ -244,10 +315,7 @@ impl Imap {
                         Event::ErrorNetwork(format!("{} ({})", message, err))
                     );
                     self.trigger_reconnect();
-                    Err(Error::ImapLoginFailed(format!(
-                        "cannot login as {}",
-                        imap_user
-                    )))
+                    Err(Error::LoginFailed(format!("cannot login as {}", imap_user)))
                 }
             }
         })
@@ -299,19 +367,10 @@ impl Imap {
         // the trailing underscore is correct
 
         if self.connect(context, &param) {
-            if context
-                .sql
-                .get_raw_config_int(context, "folders_configured")
-                .unwrap_or_default()
-                < 3
-            {
-                self.configure_folders(context, true);
-            }
-            return Ok(());
+            self.ensure_configured_folders(context, true)
+        } else {
+            Err(Error::ConnectionFailed(format!("{}", param).to_string()))
         }
-        return Err(Error::ImapConnectionFailed(
-            format!("{}", param).to_string(),
-        ));
     }
 
     /// tries connecting to imap account using the specific login
@@ -400,30 +459,30 @@ impl Imap {
         task::block_on(async move {
             if !context.sql.is_open() {
                 // probably shutdown
-                return Err(Error::ImapInTeardown);
+                return Err(Error::InTeardown);
             }
             while self
                 .fetch_from_single_folder(context, &watch_folder)
                 .await?
             {
-                // During the fetch commands new messages may arrive.  So we fetch until we do not
-                // get any more. If IDLE is called directly after, there is only a small chance that
-                // messages are missed and delayed until the next IDLE call
+                // We fetch until no more new messages are there.
             }
             Ok(())
         })
     }
 
+    /// select a folder, possibly update uid_validity and, if needed,
+    /// expunge the folder to remove delete-marked messages.
     async fn select_folder<S: AsRef<str>>(
         &self,
         context: &Context,
         folder: Option<S>,
-    ) -> ImapActionResult {
+    ) -> Result<()> {
         if self.session.lock().await.is_none() {
             let mut cfg = self.config.write().await;
             cfg.selected_folder = None;
             cfg.selected_folder_needs_expunge = false;
-            return ImapActionResult::Failed;
+            return Err(Error::NoSession);
         }
 
         // if there is a new folder and the new folder is equal to the selected one, there's nothing to do.
@@ -431,7 +490,7 @@ impl Imap {
         if let Some(ref folder) = folder {
             if let Some(ref selected_folder) = self.config.read().await.selected_folder {
                 if folder.as_ref() == selected_folder {
-                    return ImapActionResult::AlreadyDone;
+                    return Ok(());
                 }
             }
         }
@@ -450,12 +509,11 @@ impl Imap {
                             info!(context, "close/expunge succeeded");
                         }
                         Err(err) => {
-                            warn!(context, "failed to close session: {:?}", err);
-                            return ImapActionResult::Failed;
+                            return Err(Error::CloseExpungeFailed(err));
                         }
                     }
                 } else {
-                    return ImapActionResult::Failed;
+                    return Err(Error::NoSession);
                 }
             }
             self.config.write().await.selected_folder_needs_expunge = false;
@@ -464,31 +522,39 @@ impl Imap {
         // select new folder
         if let Some(ref folder) = folder {
             if let Some(ref mut session) = &mut *self.session.lock().await {
-                match session.select(folder).await {
+                let res = session.select(folder).await;
+
+                // https://tools.ietf.org/html/rfc3501#section-6.3.1
+                // says that if the server reports select failure we are in
+                // authenticated (not-select) state.
+
+                match res {
                     Ok(mailbox) => {
                         let mut config = self.config.write().await;
                         config.selected_folder = Some(folder.as_ref().to_string());
                         config.selected_mailbox = Some(mailbox);
+                        Ok(())
+                    }
+                    Err(async_imap::error::Error::ConnectionLost) => {
+                        self.trigger_reconnect();
+                        self.config.write().await.selected_folder = None;
+                        Err(Error::ConnectionLost)
+                    }
+                    Err(async_imap::error::Error::Validate(_)) => {
+                        Err(Error::BadFolderName(folder.as_ref().to_string()))
                     }
                     Err(err) => {
-                        warn!(
-                            context,
-                            "Cannot select folder: {}; {:?}.",
-                            folder.as_ref(),
-                            err
-                        );
-
                         self.config.write().await.selected_folder = None;
                         self.trigger_reconnect();
-                        return ImapActionResult::Failed;
+                        Err(Error::Other(err.to_string()))
                     }
                 }
             } else {
-                unreachable!();
+                Err(Error::NoSession)
             }
+        } else {
+            Ok(())
         }
-
-        ImapActionResult::Success
     }
 
     fn get_config_last_seen_uid<S: AsRef<str>>(&self, context: &Context, folder: S) -> (u32, u32) {
@@ -513,140 +579,154 @@ impl Imap {
         }
     }
 
-    async fn fetch_from_single_folder<S: AsRef<str>>(
+    /// return Result with (uid_validity, last_seen_uid) tuple.
+    pub(crate) fn select_with_uidvalidity(
         &self,
         context: &Context,
-        folder: S,
-    ) -> Result<bool> {
-        match self.select_folder(context, Some(&folder)).await {
-            ImapActionResult::Failed | ImapActionResult::RetryLater => {
-                bail!("Cannot select folder {:?} for fetching.", folder.as_ref());
+        folder: &str,
+    ) -> Result<(u32, u32)> {
+        task::block_on(async move {
+            self.select_folder(context, Some(folder)).await?;
+
+            // compare last seen UIDVALIDITY against the current one
+            let (uid_validity, last_seen_uid) = self.get_config_last_seen_uid(context, &folder);
+
+            let config = self.config.read().await;
+            let mailbox = config.selected_mailbox.as_ref().expect("just selected");
+
+            let new_uid_validity = match mailbox.uid_validity {
+                Some(v) => v,
+                None => {
+                    let s = format!("No UIDVALIDITY for folder {:?}", folder);
+                    return Err(Error::Other(s));
+                }
+            };
+
+            if new_uid_validity == uid_validity {
+                return Ok((uid_validity, last_seen_uid));
             }
-            ImapActionResult::Success | ImapActionResult::AlreadyDone => {}
-        }
-
-        // compare last seen UIDVALIDITY against the current one
-        let (mut uid_validity, mut last_seen_uid) = self.get_config_last_seen_uid(context, &folder);
-
-        let config = self.config.read().await;
-        let mailbox = config.selected_mailbox.as_ref().expect("just selected");
-
-        ensure!(
-            mailbox.uid_validity.is_some(),
-            "Cannot get UIDVALIDITY for folder {:?}",
-            folder.as_ref()
-        );
-
-        let new_uid_validity = mailbox.uid_validity.unwrap();
-        if new_uid_validity != uid_validity {
-            // First time this folder is selected or UIDVALIDITY has changed.
-            // Init lastseenuid and save it to config.
-            info!(
-                context,
-                "new_uid_validity={} current local uid_validity={} lastseenuid={}",
-                new_uid_validity,
-                uid_validity,
-                last_seen_uid
-            );
 
             if mailbox.exists == 0 {
-                info!(context, "Folder \"{}\" is empty.", folder.as_ref());
+                info!(context, "Folder \"{}\" is empty.", folder);
 
                 // set lastseenuid=0 for empty folders.
                 // id we do not do this here, we'll miss the first message
                 // as we will get in here again and fetch from lastseenuid+1 then
 
                 self.set_config_last_seen_uid(context, &folder, new_uid_validity, 0);
-                return Ok(false);
+                return Ok((new_uid_validity, 0));
             }
 
-            let list = if let Some(ref mut session) = &mut *self.session.lock().await {
-                // `FETCH <message sequence number> (UID)`
-                let set = format!("{}", mailbox.exists);
-                match session.fetch(set, PREFETCH_FLAGS).await {
-                    Ok(list) => list,
-                    Err(err) => {
-                        bail!("fetch failed: {}", err);
+            // uid_validity has changed or is being set the first time.
+            // find the last seen uid within the new uid_validity scope.
+            let new_last_seen_uid = match mailbox.uid_next {
+                Some(uid_next) => {
+                    uid_next - 1 // XXX could uid_next be 0?
+                }
+                None => {
+                    warn!(
+                        context,
+                        "IMAP folder has no uid_next, fall back to fetching"
+                    );
+                    if let Some(ref mut session) = &mut *self.session.lock().await {
+                        // note that we use fetch by sequence number
+                        // and thus we only need to get exactly the
+                        // last-index message.
+                        let set = format!("{}", mailbox.exists);
+                        match session.fetch(set, JUST_UID).await {
+                            Ok(list) => list[0].uid.unwrap_or_default(),
+                            Err(err) => {
+                                return Err(Error::FetchFailed(err));
+                            }
+                        }
+                    } else {
+                        return Err(Error::NoConnection);
                     }
                 }
-            } else {
-                return Err(Error::ImapNoConnection);
             };
 
-            last_seen_uid = list[0].uid.unwrap_or_else(|| 0);
-
-            // if the UIDVALIDITY has _changed_, decrease lastseenuid by one to avoid gaps (well add 1 below
-            if uid_validity > 0 && last_seen_uid > 1 {
-                last_seen_uid -= 1;
-            }
-
-            uid_validity = new_uid_validity;
-            self.set_config_last_seen_uid(context, &folder, uid_validity, last_seen_uid);
+            self.set_config_last_seen_uid(context, &folder, new_uid_validity, new_last_seen_uid);
             info!(
                 context,
-                "lastseenuid initialized to {} for {}@{}",
-                last_seen_uid,
-                folder.as_ref(),
+                "uid/validity change: new {}/{} current {}/{}",
+                new_last_seen_uid,
+                new_uid_validity,
                 uid_validity,
+                last_seen_uid
             );
-        }
+            Ok((new_uid_validity, new_last_seen_uid))
+        })
+    }
+
+    async fn fetch_from_single_folder<S: AsRef<str>>(
+        &self,
+        context: &Context,
+        folder: S,
+    ) -> Result<bool> {
+        let (uid_validity, last_seen_uid) =
+            self.select_with_uidvalidity(context, folder.as_ref())?;
 
         let mut read_cnt = 0;
-        let mut read_errors = 0;
-        let mut new_last_seen_uid = 0;
 
-        let list = if let Some(ref mut session) = &mut *self.session.lock().await {
+        let mut list = if let Some(ref mut session) = &mut *self.session.lock().await {
             // fetch messages with larger UID than the last one seen
             // (`UID FETCH lastseenuid+1:*)`, see RFC 4549
             let set = format!("{}:*", last_seen_uid + 1);
             match session.uid_fetch(set, PREFETCH_FLAGS).await {
                 Ok(list) => list,
                 Err(err) => {
-                    bail!("uid_fetch failed: {}", err);
+                    return Err(Error::FetchFailed(err));
                 }
             }
         } else {
-            return Err(Error::ImapNoConnection);
+            return Err(Error::NoConnection);
         };
 
-        // go through all mails in folder (this is typically _fast_ as we already have the whole list)
+        // prefetch info from all unfetched mails
+        let mut new_last_seen_uid = last_seen_uid;
+        let mut read_errors = 0;
+
+        list.sort_unstable_by_key(|msg| msg.uid.unwrap_or_default());
+
         for msg in &list {
-            let cur_uid = msg.uid.unwrap_or_else(|| 0);
-            if cur_uid > last_seen_uid {
-                read_cnt += 1;
+            let cur_uid = msg.uid.unwrap_or_default();
+            if cur_uid <= last_seen_uid {
+                warn!(
+                    context,
+                    "unexpected uid {}, last seen was {}", cur_uid, last_seen_uid
+                );
+                continue;
+            }
+            read_cnt += 1;
 
-                let message_id = prefetch_get_message_id(msg).unwrap_or_default();
+            let message_id = prefetch_get_message_id(msg).unwrap_or_default();
 
-                if !precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
-                    // check passed, go fetch the rest
-                    if self.fetch_single_msg(context, &folder, cur_uid).await == 0 {
-                        info!(
-                            context,
-                            "Read error for message {} from \"{}\", trying over later.",
-                            message_id,
-                            folder.as_ref()
-                        );
-
-                        read_errors += 1;
-                    }
-                } else {
-                    // check failed
+            if !precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
+                // check passed, go fetch the rest
+                if self.fetch_single_msg(context, &folder, cur_uid).await == 0 {
                     info!(
                         context,
-                        "Skipping message {} from \"{}\" by precheck.",
+                        "Read error for message {} from \"{}\", trying over later.",
                         message_id,
-                        folder.as_ref(),
+                        folder.as_ref()
                     );
+                    read_errors += 1;
                 }
-                if cur_uid > new_last_seen_uid {
-                    new_last_seen_uid = cur_uid
-                }
+            } else {
+                // we know the message-id already or don't want the message otherwise.
+                info!(
+                    context,
+                    "Skipping message {} from \"{}\" by precheck.",
+                    message_id,
+                    folder.as_ref(),
+                );
+            }
+            if read_errors == 0 {
+                new_last_seen_uid = cur_uid;
             }
         }
 
-        if 0 == read_errors && new_last_seen_uid > 0 {
-            // TODO: it might be better to increase the lastseenuid also on partial errors.
-            // however, this requires to sort the list before going through it above.
+        if new_last_seen_uid > last_seen_uid {
             self.set_config_last_seen_uid(context, &folder, uid_validity, new_last_seen_uid);
         }
 
@@ -757,18 +837,12 @@ impl Imap {
     pub fn idle(&self, context: &Context, watch_folder: Option<String>) -> Result<()> {
         task::block_on(async move {
             if !self.config.read().await.can_idle {
-                return Err(Error::ImapMissesIdle);
+                return Err(Error::IdleAbilityMissing);
             }
 
             self.setup_handle_if_needed(context)?;
 
-            match self.select_folder(context, watch_folder.clone()).await {
-                ImapActionResult::Success | ImapActionResult::AlreadyDone => {}
-
-                ImapActionResult::Failed | ImapActionResult::RetryLater => {
-                    return Err(Error::ImapSelectFailed(format!("{:?}", watch_folder)));
-                }
-            }
+            self.select_folder(context, watch_folder.clone()).await?;
 
             let session = self.session.lock().await.take();
             let timeout = Duration::from_secs(23 * 60);
@@ -778,7 +852,7 @@ impl Imap {
                     // typically also need to change the Insecure branch.
                     IdleHandle::Secure(mut handle) => {
                         if let Err(err) = handle.init().await {
-                            return Err(Error::ImapIdleProtocolFailed(err.to_string()));
+                            return Err(Error::IdleProtocolFailed(err));
                         }
 
                         let (idle_wait, interrupt) = handle.wait_with_timeout(timeout);
@@ -816,13 +890,13 @@ impl Imap {
                                 // means that we waited long (with idle_wait)
                                 // but the network went away/changed
                                 self.trigger_reconnect();
-                                return Err(Error::ImapIdleProtocolFailed(err.to_string()));
+                                return Err(Error::IdleProtocolFailed(err));
                             }
                         }
                     }
                     IdleHandle::Insecure(mut handle) => {
                         if let Err(err) = handle.init().await {
-                            return Err(Error::ImapIdleProtocolFailed(err.to_string()));
+                            return Err(Error::IdleProtocolFailed(err));
                         }
 
                         let (idle_wait, interrupt) = handle.wait_with_timeout(timeout);
@@ -860,7 +934,7 @@ impl Imap {
                                 // means that we waited long (with idle_wait)
                                 // but the network went away/changed
                                 self.trigger_reconnect();
-                                return Err(Error::ImapIdleProtocolFailed(err.to_string()));
+                                return Err(Error::IdleProtocolFailed(err));
                             }
                         }
                     }
@@ -1097,14 +1171,22 @@ impl Imap {
                 }
             }
             match self.select_folder(context, Some(&folder)).await {
-                ImapActionResult::Success | ImapActionResult::AlreadyDone => None,
-                res => {
-                    warn!(
-                        context,
-                        "Cannot select folder {} for preparing IMAP operation", folder
-                    );
-
-                    Some(res)
+                Ok(()) => None,
+                Err(Error::ConnectionLost) => {
+                    warn!(context, "Lost imap connection");
+                    Some(ImapActionResult::RetryLater)
+                }
+                Err(Error::NoSession) => {
+                    warn!(context, "no imap session");
+                    Some(ImapActionResult::Failed)
+                }
+                Err(Error::BadFolderName(folder_name)) => {
+                    warn!(context, "invalid folder name: {:?}", folder_name);
+                    Some(ImapActionResult::Failed)
+                }
+                Err(err) => {
+                    warn!(context, "failed to select folder: {:?}: {:?}", folder, err);
+                    Some(ImapActionResult::RetryLater)
                 }
             }
         })
@@ -1206,19 +1288,30 @@ impl Imap {
         })
     }
 
-    pub fn configure_folders(&self, context: &Context, create_mvbox: bool) {
+    pub fn ensure_configured_folders(&self, context: &Context, create_mvbox: bool) -> Result<()> {
+        let folders_configured = context
+            .sql
+            .get_raw_config_int(context, "folders_configured");
+        if folders_configured.unwrap_or_default() >= 3 {
+            // the "3" here we increase if we have future updates to
+            // to folder configuration
+            return Ok(());
+        }
+
         task::block_on(async move {
             if !self.is_connected().await {
-                return;
+                return Err(Error::NoConnection);
             }
 
             info!(context, "Configuring IMAP-folders.");
 
             if let Some(ref mut session) = &mut *self.session.lock().await {
-                let folders = self
-                    .list_folders(session, context)
-                    .await
-                    .expect("no folders found");
+                let folders = match self.list_folders(session, context).await {
+                    Some(f) => f,
+                    None => {
+                        return Err(Error::Other("list_folders failed".to_string()));
+                    }
+                };
 
                 let sentbox_folder =
                     folders
@@ -1249,7 +1342,7 @@ impl Imap {
                         Err(err) => {
                             warn!(
                                 context,
-                                "Cannot create MVBOX-folder, using trying INBOX subfolder. ({})",
+                                "Cannot create MVBOX-folder, trying to create INBOX subfolder. ({})",
                                 err
                             );
 
@@ -1271,35 +1364,34 @@ impl Imap {
                     // that may be used by other MUAs to list folders.
                     // for the LIST command, the folder is always visible.
                     if let Some(ref mvbox) = mvbox_folder {
-                        // TODO: better error handling
-                        session.subscribe(mvbox).await.expect("failed to subscribe");
+                        if let Err(err) = session.subscribe(mvbox).await {
+                            warn!(context, "could not subscribe to {:?}: {:?}", mvbox, err);
+                        }
                     }
                 }
                 context
                     .sql
-                    .set_raw_config(context, "configured_inbox_folder", Some("INBOX"))
-                    .ok();
+                    .set_raw_config(context, "configured_inbox_folder", Some("INBOX"))?;
                 if let Some(ref mvbox_folder) = mvbox_folder {
-                    context
-                        .sql
-                        .set_raw_config(context, "configured_mvbox_folder", Some(mvbox_folder))
-                        .ok();
+                    context.sql.set_raw_config(
+                        context,
+                        "configured_mvbox_folder",
+                        Some(mvbox_folder),
+                    )?;
                 }
                 if let Some(ref sentbox_folder) = sentbox_folder {
-                    context
-                        .sql
-                        .set_raw_config(
-                            context,
-                            "configured_sentbox_folder",
-                            Some(sentbox_folder.name()),
-                        )
-                        .ok();
+                    context.sql.set_raw_config(
+                        context,
+                        "configured_sentbox_folder",
+                        Some(sentbox_folder.name()),
+                    )?;
                 }
                 context
                     .sql
-                    .set_raw_config_int(context, "folders_configured", 3)
-                    .ok();
+                    .set_raw_config_int(context, "folders_configured", 3)?;
             }
+            info!(context, "FINISHED configuring IMAP-folders.");
+            Ok(())
         })
     }
 
@@ -1330,33 +1422,35 @@ impl Imap {
             info!(context, "emptying folder {}", folder);
 
             if folder.is_empty() {
-                warn!(context, "cannot perform empty, folder not set");
+                error!(context, "cannot perform empty, folder not set");
                 return;
             }
-            match self.select_folder(context, Some(&folder)).await {
-                ImapActionResult::Success | ImapActionResult::AlreadyDone => {
-                    if !self
-                        .add_flag_finalized_with_set(context, SELECT_ALL, "\\Deleted")
-                        .await
-                    {
-                        warn!(context, "Cannot empty folder {}", folder);
-                    } else {
-                        // we now trigger expunge to actually delete messages
-                        self.config.write().await.selected_folder_needs_expunge = true;
-                        if self.select_folder::<String>(context, None).await
-                            == ImapActionResult::Success
-                        {
-                            emit_event!(context, Event::ImapFolderEmptied(folder.to_string()));
-                        } else {
-                            warn!(
-                                context,
-                                "could not perform expunge on empty-marked folder {}", folder
-                            );
-                        }
-                    }
+            if let Err(err) = self.select_folder(context, Some(&folder)).await {
+                // we want to report all error to the user
+                // (no retry should be attempted)
+                error!(
+                    context,
+                    "Could not select {} for expunging: {:?}", folder, err
+                );
+                return;
+            }
+
+            if !self
+                .add_flag_finalized_with_set(context, SELECT_ALL, "\\Deleted")
+                .await
+            {
+                error!(context, "Cannot mark messages for deletion {}", folder);
+                return;
+            }
+
+            // we now trigger expunge to actually delete messages
+            self.config.write().await.selected_folder_needs_expunge = true;
+            match self.select_folder::<String>(context, None).await {
+                Ok(()) => {
+                    emit_event!(context, Event::ImapFolderEmptied(folder.to_string()));
                 }
-                ImapActionResult::Failed | ImapActionResult::RetryLater => {
-                    warn!(context, "could not select folder {}", folder);
+                Err(err) => {
+                    error!(context, "expunge failed {}: {:?}", folder, err);
                 }
             }
         });
@@ -1435,11 +1529,16 @@ fn precheck_imf(context: &Context, rfc724_mid: &str, server_folder: &str, server
 }
 
 fn prefetch_get_message_id(prefetch_msg: &Fetch) -> Result<String> {
-    ensure!(
-        prefetch_msg.envelope().is_some(),
-        "Fetched message has no envelope"
-    );
+    if prefetch_msg.envelope().is_none() {
+        return Err(Error::Other(
+            "prefectch: message has no envelope".to_string(),
+        ));
+    }
+
     let message_id = prefetch_msg.envelope().unwrap().message_id;
-    ensure!(message_id.is_some(), "No message ID found");
-    wrapmime::parse_message_id(&message_id.unwrap())
+    if message_id.is_none() {
+        return Err(Error::Other("prefetch: No message ID found".to_string()));
+    }
+
+    wrapmime::parse_message_id(&message_id.unwrap()).map_err(Into::into)
 }

src/imex.rs

@@ -476,14 +476,15 @@ fn import_backup(context: &Context, backup_to_import: impl AsRef<Path>) -> Resul
                 }
 
                 let path_filename = context.get_blobdir().join(file_name);
-                if dc_write_file(context, &path_filename, &file_blob) {
+                if dc_write_file(context, &path_filename, &file_blob).is_err() {
+                    bail!(
+                        "Storage full? Cannot write file {} with {} bytes.",
+                        path_filename.display(),
+                        file_blob.len(),
+                    );
+                } else {
                     continue;
                 }
-                bail!(
-                    "Storage full? Cannot write file {} with {} bytes.",
-                    path_filename.display(),
-                    file_blob.len(),
-                );
             }
             Ok(())
         },
@@ -686,14 +687,14 @@ fn export_self_keys(context: &Context, dir: impl AsRef<Path>) -> Result<()> {
                 let (id, public_key, private_key, is_default) = key_pair?;
                 let id = Some(id).filter(|_| is_default != 0);
                 if let Some(key) = public_key {
-                    if !export_key_to_asc_file(context, &dir, id, &key) {
+                    if export_key_to_asc_file(context, &dir, id, &key).is_err() {
                         export_errors += 1;
                     }
                 } else {
                     export_errors += 1;
                 }
                 if let Some(key) = private_key {
-                    if !export_key_to_asc_file(context, &dir, id, &key) {
+                    if export_key_to_asc_file(context, &dir, id, &key).is_err() {
                         export_errors += 1;
                     }
                 } else {
@@ -717,8 +718,7 @@ fn export_key_to_asc_file(
     dir: impl AsRef<Path>,
     id: Option<i64>,
     key: &Key,
-) -> bool {
-    let mut success = false;
+) -> std::io::Result<()> {
     let file_name = {
         let kind = if key.is_public() { "public" } else { "private" };
         let id = id.map_or("default".into(), |i| i.to_string());
@@ -728,14 +728,13 @@ fn export_key_to_asc_file(
     info!(context, "Exporting key {}", file_name.display());
     dc_delete_file(context, &file_name);
 
-    if !key.write_asc_to_file(&file_name, context) {
+    let res = key.write_asc_to_file(&file_name, context);
+    if res.is_err() {
         error!(context, "Cannot write key to {}", file_name.display());
     } else {
         context.call_cb(Event::ImexFileWritten(file_name));
-        success = true;
     }
-
-    success
+    res
 }
 
 #[cfg(test)]
@@ -799,7 +798,7 @@ mod tests {
         let base64 = include_str!("../test-data/key/public.asc");
         let key = Key::from_base64(base64, KeyType::Public).unwrap();
         let blobdir = "$BLOBDIR";
-        assert!(export_key_to_asc_file(&context.ctx, blobdir, None, &key));
+        assert!(export_key_to_asc_file(&context.ctx, blobdir, None, &key).is_ok());
         let blobdir = context.ctx.get_blobdir().to_str().unwrap();
         let filename = format!("{}/public-key-default.asc", blobdir);
         let bytes = std::fs::read(&filename).unwrap();

src/job.rs

@@ -20,6 +20,7 @@ use crate::message::MsgId;
 use crate::message::{self, Message, MessageState};
 use crate::mimefactory::{vec_contains_lowercase, Loaded, MimeFactory};
 use crate::param::*;
+use crate::smtp::SmtpError;
 use crate::sql;
 
 // results in ~3 weeks for the last backoff timespan
@@ -184,11 +185,22 @@ impl Job {
                     // otherwise might send it twice.
                     let mut smtp = context.smtp.lock().unwrap();
                     match smtp.send(context, recipients_list, body, self.job_id) {
-                        Err(err) => {
+                        Err(SmtpError::SendError(err)) => {
+                            // Remote error, retry later.
                             smtp.disconnect();
-                            warn!(context, "smtp failed: {}", err);
+                            info!(context, "SMTP failed to send: {}", err);
                             self.try_again_later(TryAgain::AtOnce, Some(err.to_string()));
                         }
+                        Err(SmtpError::EnvelopeError(err)) => {
+                            // Local error, job is invalid, do not retry.
+                            smtp.disconnect();
+                            warn!(context, "SMTP job is invalid: {}", err);
+                        }
+                        Err(SmtpError::NoTransport) => {
+                            // Should never happen.
+                            // It does not even make sense to disconnect here.
+                            error!(context, "SMTP job failed because SMTP has no transport");
+                        }
                         Ok(()) => {
                             // smtp success, update db ASAP, then delete smtp file
                             if 0 != self.foreign_id {
@@ -216,13 +228,10 @@ impl Job {
         let imap_inbox = &context.inbox_thread.read().unwrap().imap;
 
         if let Ok(msg) = Message::load_from_db(context, MsgId::new(self.foreign_id)) {
-            if context
-                .sql
-                .get_raw_config_int(context, "folders_configured")
-                .unwrap_or_default()
-                < 3
-            {
-                imap_inbox.configure_folders(context, true);
+            if let Err(err) = imap_inbox.ensure_configured_folders(context, true) {
+                self.try_again_later(TryAgain::StandardDelay, None);
+                warn!(context, "could not configure folders: {:?}", err);
+                return;
             }
             let dest_folder = context
                 .sql
@@ -343,13 +352,10 @@ impl Job {
             return;
         }
         if 0 != self.param.get_int(Param::AlsoMove).unwrap_or_default() {
-            if context
-                .sql
-                .get_raw_config_int(context, "folders_configured")
-                .unwrap_or_default()
-                < 3
-            {
-                imap_inbox.configure_folders(context, true);
+            if let Err(err) = imap_inbox.ensure_configured_folders(context, true) {
+                self.try_again_later(TryAgain::StandardDelay, None);
+                warn!(context, "configuring folders failed: {:?}", err);
+                return;
             }
             let dest_folder = context
                 .sql

src/job_thread.rs

@@ -104,7 +104,7 @@ impl JobThread {
                 if let Some(watch_folder) = self.get_watch_folder(context) {
                     let start = std::time::Instant::now();
                     info!(context, "{} started...", prefix);
-                    let res = self.imap.fetch(context, &watch_folder);
+                    let res = self.imap.fetch(context, &watch_folder).map_err(Into::into);
                     let elapsed = start.elapsed().as_millis();
                     info!(context, "{} done in {:.3} ms.", prefix, elapsed);
 
@@ -113,7 +113,7 @@ impl JobThread {
                     Err(Error::WatchFolderNotFound("not-set".to_string()))
                 }
             }
-            Err(err) => Err(err),
+            Err(err) => Err(crate::error::Error::Message(err.to_string())),
         }
     }
 
@@ -176,7 +176,7 @@ impl JobThread {
                 info!(context, "{} ended...", prefix);
                 match res {
                     Ok(()) => false,
-                    Err(Error::ImapMissesIdle) => true, // we have to do fake_idle
+                    Err(crate::imap::Error::IdleAbilityMissing) => true, // we have to do fake_idle
                     Err(err) => {
                         warn!(context, "{} failed: {} -> reconnecting", prefix, err);
                         // something is borked, let's start afresh on the next occassion

src/key.rs

@@ -217,15 +217,18 @@ impl Key {
             .expect("failed to serialize key")
     }
 
-    pub fn write_asc_to_file(&self, file: impl AsRef<Path>, context: &Context) -> bool {
+    pub fn write_asc_to_file(
+        &self,
+        file: impl AsRef<Path>,
+        context: &Context,
+    ) -> std::io::Result<()> {
         let file_content = self.to_asc(None).into_bytes();
 
-        if dc_write_file(context, &file, &file_content) {
-            true
-        } else {
+        let res = dc_write_file(context, &file, &file_content);
+        if res.is_err() {
             error!(context, "Cannot write key to {}", file.as_ref().display());
-            false
         }
+        res
     }
 
     pub fn fingerprint(&self) -> String {

src/lib.rs

@@ -22,7 +22,7 @@ extern crate jetscii;
 extern crate debug_stub_derive;
 
 #[macro_use]
-pub mod log;
+mod log;
 #[macro_use]
 pub mod error;
 

src/log.rs

@@ -1,167 +1,4 @@
-//! # Logging support
-
-use std::fmt;
-use std::fs;
-use std::io;
-use std::io::prelude::*;
-use std::path::{Path, PathBuf};
-
-/// A logger for a [Context].
-#[derive(Debug)]
-pub struct Logger {
-    created: std::time::Instant,
-    logdir: PathBuf,
-    logfile: String,
-    file_handle: fs::File,
-    max_files: u32,
-    max_filesize: usize,
-    bytes_written: usize,
-}
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub enum LogLevel {
-    Info,
-    Warning,
-    Error,
-}
-
-impl fmt::Display for LogLevel {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            LogLevel::Info => write!(f, "I"),
-            LogLevel::Warning => write!(f, "W"),
-            LogLevel::Error => write!(f, "E"),
-        }
-    }
-}
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Callsite<'a> {
-    pub file: &'a str,
-    pub line: u32,
-}
-
-impl fmt::Display for Callsite<'_> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}:{}", self.file, self.line)
-    }
-}
-
-impl Logger {
-    pub fn new(logdir: PathBuf) -> Result<Logger, io::Error> {
-        let (fname, file) = Self::open(&logdir)?;
-        let max_files = 5;
-        Self::prune(&logdir, max_files)?;
-        Ok(Logger {
-            created: std::time::Instant::now(),
-            logdir,
-            logfile: fname,
-            file_handle: file,
-            max_files,
-            max_filesize: 4 * 1024 * 1024, // 4 Mb
-            bytes_written: 0,
-        })
-    }
-
-    /// Opens a new logfile, returning a tuple of (file_name, file_handle).
-    ///
-    /// This tries to create a new logfile based on the current time,
-    /// creating .0.log, .1.log etc if this file already exists (up to 32).
-    fn open(logdir: &Path) -> Result<(String, fs::File), io::Error> {
-        let basename =
-            chrono::offset::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true);
-        let mut fname = sanitize_filename::sanitize_with_options(
-            format!("{}.log", &basename),
-            sanitize_filename::Options {
-                truncate: true,
-                windows: true,
-                replacement: ".",
-            },
-        );
-        let mut counter = 0;
-        loop {
-            match std::fs::OpenOptions::new()
-                .write(true)
-                .create_new(true)
-                .open(logdir.join(&fname))
-            {
-                Ok(file) => {
-                    return Ok((fname, file));
-                }
-                Err(e) => {
-                    if counter >= 32 {
-                        return Err(e);
-                    } else {
-                        counter += 1;
-                        fname =
-                            sanitize_filename::sanitize(format!("{}-{}.log", &basename, counter));
-                        continue;
-                    }
-                }
-            }
-        }
-    }
-
-    /// Cleans up old logfiles.
-    fn prune(logdir: &Path, max_files: u32) -> Result<(), io::Error> {
-        let mut names: Vec<std::ffi::OsString> = Vec::new();
-        for dirent in fs::read_dir(logdir)? {
-            names.push(dirent?.file_name());
-        }
-        // Sorting like this sorts: 23.log, 24.1.log, 24.2.log,
-        // 24.log, 25.log.  That is 24.log is out of sequence.  Oh well.
-        names.sort();
-        names.reverse();
-        while names.len() > max_files as usize {
-            if let Some(name) = names.pop() {
-                fs::remove_file(logdir.join(name))?;
-            }
-        }
-        Ok(())
-    }
-
-    pub fn log(
-        &mut self,
-        level: LogLevel,
-        callsite: Callsite,
-        msg: &str,
-    ) -> Result<(), std::io::Error> {
-        if self.bytes_written > self.max_filesize {
-            self.flush()?;
-            let (fname, handle) = Self::open(&self.logdir)?;
-            self.logfile = fname;
-            self.file_handle = handle;
-            Self::prune(&self.logdir, self.max_files)?;
-        }
-        let thread = std::thread::current();
-        let msg = format!(
-            "{time:8.2} {level} {thid:?}/{thname} [{callsite}]: {msg}\n",
-            time = self.created.elapsed().as_secs_f64(),
-            level = level,
-            thid = thread.id(),
-            thname = thread.name().unwrap_or("unnamed"),
-            callsite = callsite,
-            msg = msg,
-        );
-        self.file_handle.write_all(msg.as_bytes())?;
-        self.bytes_written += msg.len();
-        Ok(())
-    }
-
-    pub fn flush(&mut self) -> Result<(), std::io::Error> {
-        self.file_handle.flush()
-    }
-}
-
-#[macro_export]
-macro_rules! callsite {
-    () => {
-        $crate::log::Callsite {
-            file: file!(),
-            line: line!(),
-        }
-    };
-}
+//! # Logging macros
 
 #[macro_export]
 macro_rules! info {
@@ -170,9 +7,6 @@ macro_rules! info {
     };
     ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
         let formatted = format!($msg, $($args),*);
-        if let Ok(mut logger) = $ctx.logger.write() {
-            logger.log($crate::log::LogLevel::Info, callsite!(), &formatted).ok();
-        }
         emit_event!($ctx, $crate::Event::Info(formatted));
     }};
 }
@@ -184,9 +18,6 @@ macro_rules! warn {
     };
     ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
         let formatted = format!($msg, $($args),*);
-        if let Ok(mut logger) = $ctx.logger.write() {
-            logger.log($crate::log::LogLevel::Warning, callsite!(), &formatted).ok();
-        }
         emit_event!($ctx, $crate::Event::Warning(formatted));
     }};
 }
@@ -198,9 +29,6 @@ macro_rules! error {
     };
     ($ctx:expr, $msg:expr, $($args:expr),* $(,)?) => {{
         let formatted = format!($msg, $($args),*);
-        if let Ok(mut logger) = $ctx.logger.write() {
-            logger.log($crate::log::LogLevel::Error, callsite!(), &formatted).ok();
-        }
         emit_event!($ctx, $crate::Event::Error(formatted));
     }};
 }
@@ -211,80 +39,3 @@ macro_rules! emit_event {
         $ctx.call_cb($event);
     };
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_basic_logging() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dir = tmp.path();
-        let mut logger = Logger::new(dir.to_path_buf()).unwrap();
-        logger.log(LogLevel::Info, callsite!(), "foo").unwrap();
-        logger.log(LogLevel::Warning, callsite!(), "bar").unwrap();
-        logger.log(LogLevel::Error, callsite!(), "baz").unwrap();
-        logger.flush().unwrap();
-        let log = fs::read_to_string(logger.logdir.join(logger.logfile)).unwrap();
-        println!("{}", log);
-        let lines: Vec<&str> = log.lines().collect();
-
-        assert!(lines[0].contains(" I "));
-        assert!(lines[0].contains(format!("{:?}", std::thread::current().id()).as_str()));
-        assert!(lines[0]
-            .contains(format!("{}", std::thread::current().name().unwrap_or("unnamed")).as_str()));
-        assert!(lines[0].contains(&format!("src{}log.rs", std::path::MAIN_SEPARATOR)));
-        assert!(lines[0].contains("foo"));
-
-        assert!(lines[1].contains(" W "));
-        assert!(lines[1].contains(format!("{:?}", std::thread::current().id()).as_str()));
-        assert!(lines[1]
-            .contains(format!("{}", std::thread::current().name().unwrap_or("unnamed")).as_str()));
-        assert!(lines[1].contains(&format!("src{}log.rs", std::path::MAIN_SEPARATOR)));
-        assert!(lines[1].contains("bar"));
-
-        assert!(lines[2].contains(" E "));
-        assert!(lines[2].contains(format!("{:?}", std::thread::current().id()).as_str()));
-        assert!(lines[2]
-            .contains(format!("{}", std::thread::current().name().unwrap_or("unnamed")).as_str()));
-        assert!(lines[2].contains(&format!("src{}log.rs", std::path::MAIN_SEPARATOR)));
-        assert!(lines[2].contains("baz"));
-    }
-
-    #[test]
-    fn test_reopen_logfile() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dir = tmp.path();
-        let mut logger = Logger::new(dir.to_path_buf()).unwrap();
-        logger.max_filesize = 5;
-
-        let fname0 = logger.logfile.clone();
-        assert!(fname0.ends_with(".log"));
-        logger
-            .log(LogLevel::Info, callsite!(), "more than 5 bytes are written")
-            .unwrap();
-        logger.log(LogLevel::Info, callsite!(), "2nd msg").unwrap();
-        let fname1 = logger.logfile.clone();
-        assert!(fname1.ends_with("-1.log"));
-        assert_ne!(fname0, fname1);
-        let log0 = fs::read_to_string(logger.logdir.join(&fname0)).unwrap();
-        assert!(log0.contains("more than 5 bytes are written"));
-        let log1 = fs::read_to_string(logger.logdir.join(&fname1)).unwrap();
-        assert!(log1.contains("2nd msg"));
-    }
-
-    #[test]
-    fn test_prune() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dir = tmp.path();
-        Logger::new(dir.to_path_buf()).unwrap();
-        Logger::new(dir.to_path_buf()).unwrap();
-        Logger::new(dir.to_path_buf()).unwrap();
-        Logger::new(dir.to_path_buf()).unwrap();
-        let dirents0: Vec<fs::DirEntry> = fs::read_dir(&dir).unwrap().map(|r| r.unwrap()).collect();
-        assert_eq!(dirents0.len(), 4);
-        Logger::prune(&dir, 3).unwrap();
-        let dirents1: Vec<fs::DirEntry> = fs::read_dir(&dir).unwrap().map(|r| r.unwrap()).collect();
-        assert_eq!(dirents1.len(), 3);
-    }
-}

src/mimefactory.rs

@@ -707,7 +707,7 @@ impl<'a> MimeFactory<'a> {
                     .get_config(Config::ConfiguredAddr)
                     .unwrap_or_default();
 
-                if !email_to_remove.is_empty() && email_to_remove != self_addr {
+                if !email_to_remove.is_empty() && !addr_cmp(email_to_remove, self_addr) {
                     if !vec_contains_lowercase(&factory.recipients_addr, &email_to_remove) {
                         factory.recipients_names.push("".to_string());
                         factory.recipients_addr.push(email_to_remove.to_string());

src/oauth2.rs

@@ -355,6 +355,8 @@ fn normalize_addr(addr: &str) -> &str {
 mod tests {
     use super::*;
 
+    use crate::test_utils::*;
+
     #[test]
     fn test_normalize_addr() {
         assert_eq!(normalize_addr(" hello@mail.de  "), "hello@mail.de");
@@ -384,4 +386,24 @@ mod tests {
 
         assert_eq!(Oauth2::from_address("hello@web.de"), None);
     }
+
+    #[test]
+    fn test_dc_get_oauth2_addr() {
+        let ctx = dummy_context();
+        let addr = "dignifiedquire@gmail.com";
+        let code = "fail";
+        let res = dc_get_oauth2_addr(&ctx.ctx, addr, code);
+        // this should fail as it is an invalid password
+        assert_eq!(res, None);
+    }
+
+    #[test]
+    fn test_dc_get_oauth2_token() {
+        let ctx = dummy_context();
+        let addr = "dignifiedquire@gmail.com";
+        let code = "fail";
+        let res = dc_get_oauth2_access_token(&ctx.ctx, addr, code, false);
+        // this should fail as it is an invalid password
+        assert_eq!(res, None);
+    }
 }

src/param.rs

@@ -321,7 +321,6 @@ mod tests {
     use std::fs;
     use std::path::Path;
 
-    use crate::blob::BlobErrorKind;
     use crate::test_utils::*;
 
     #[test]
@@ -404,7 +403,10 @@ mod tests {
 
         // Blob does not exist yet, expect BlobError.
         let err = p.get_blob(Param::File, &t.ctx, false).unwrap_err();
-        assert_eq!(err.kind(), BlobErrorKind::WrongBlobdir);
+        match err {
+            BlobError::WrongBlobdir { .. } => (),
+            _ => panic!("wrong error type/variant: {:?}", err),
+        }
 
         fs::write(fname, b"boo").unwrap();
         let blob = p.get_blob(Param::File, &t.ctx, true).unwrap().unwrap();

src/securejoin.rs

@@ -242,7 +242,8 @@ pub fn dc_join_securejoin(context: &Context, qr: &str) -> u32 {
 
     // Bob -> Alice
     while !context.shall_stop_ongoing() {
-        std::thread::sleep(std::time::Duration::new(0, 3_000_000));
+        // Don't sleep too long, the user is waiting.
+        std::thread::sleep(std::time::Duration::from_millis(200));
     }
     cleanup(&context, contact_chat_id, true, join_vg)
 }
@@ -318,21 +319,40 @@ fn fingerprint_equals_sender(
     false
 }
 
+pub(crate) struct HandshakeMessageStatus {
+    pub(crate) hide_this_msg: bool,
+    pub(crate) delete_this_msg: bool,
+    pub(crate) stop_ongoing_process: bool,
+    pub(crate) bob_securejoin_success: Option<bool>,
+}
+
+impl Default for HandshakeMessageStatus {
+    fn default() -> Self {
+        Self {
+            hide_this_msg: true,
+            delete_this_msg: false,
+            stop_ongoing_process: false,
+            bob_securejoin_success: None,
+        }
+    }
+}
+
 /* library private: secure-join */
-pub fn handle_securejoin_handshake(
+pub(crate) fn handle_securejoin_handshake(
     context: &Context,
     mimeparser: &MimeParser,
     contact_id: u32,
-) -> libc::c_int {
+) -> Result<HandshakeMessageStatus, Error> {
     let own_fingerprint: String;
 
-    if contact_id <= DC_CONTACT_ID_LAST_SPECIAL {
-        return 0;
-    }
+    ensure!(
+        contact_id > DC_CONTACT_ID_LAST_SPECIAL,
+        "handle_securejoin_handshake(): called with special contact id"
+    );
     let step = match mimeparser.lookup_optional_field("Secure-Join") {
         Some(s) => s,
         None => {
-            return 0;
+            bail!("This message is not a Secure-Join message");
         }
     };
     info!(
@@ -345,8 +365,8 @@ pub fn handle_securejoin_handshake(
     if contact_chat_id_blocked != Blocked::Not {
         chat::unblock(context, contact_chat_id);
     }
-    let mut ret: libc::c_int = DC_HANDSHAKE_STOP_NORMAL_PROCESSING;
     let join_vg = step.starts_with("vg-");
+    let mut ret = HandshakeMessageStatus::default();
 
     match step.as_str() {
         "vg-request" | "vc-request" => {
@@ -362,12 +382,12 @@ pub fn handle_securejoin_handshake(
                 Some(n) => n,
                 None => {
                     warn!(context, "Secure-join denied (invitenumber missing).",);
-                    return ret;
+                    return Ok(ret);
                 }
             };
             if !token::exists(context, token::Namespace::InviteNumber, &invitenumber) {
                 warn!(context, "Secure-join denied (bad invitenumber).",);
-                return ret;
+                return Ok(ret);
             }
             info!(context, "Secure-join requested.",);
 
@@ -393,7 +413,7 @@ pub fn handle_securejoin_handshake(
             if cond {
                 warn!(context, "auth-required message out of sync.",);
                 // no error, just aborted somehow or a mail from another handshake
-                return ret;
+                return Ok(ret);
             }
             let scanned_fingerprint_of_alice = get_qr_attr!(context, fingerprint).to_string();
             let auth = get_qr_attr!(context, auth).to_string();
@@ -408,8 +428,9 @@ pub fn handle_securejoin_handshake(
                         "Not encrypted."
                     },
                 );
-                end_bobs_joining(context, DC_BOB_ERROR);
-                return ret;
+                ret.stop_ongoing_process = true;
+                ret.bob_securejoin_success = Some(false);
+                return Ok(ret);
             }
             if !fingerprint_equals_sender(context, &scanned_fingerprint_of_alice, contact_chat_id) {
                 could_not_establish_secure_connection(
@@ -417,8 +438,9 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Fingerprint mismatch on joiner-side.",
                 );
-                end_bobs_joining(context, DC_BOB_ERROR);
-                return ret;
+                ret.stop_ongoing_process = true;
+                ret.bob_securejoin_success = Some(false);
+                return Ok(ret);
             }
             info!(context, "Fingerprint verified.",);
             own_fingerprint = get_self_fingerprint(context).unwrap();
@@ -453,7 +475,7 @@ pub fn handle_securejoin_handshake(
                         contact_chat_id,
                         "Fingerprint not provided.",
                     );
-                    return ret;
+                    return Ok(ret);
                 }
             };
             if !encrypted_and_signed(mimeparser, &fingerprint) {
@@ -462,7 +484,7 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Auth not encrypted.",
                 );
-                return ret;
+                return Ok(ret);
             }
             if !fingerprint_equals_sender(context, &fingerprint, contact_chat_id) {
                 could_not_establish_secure_connection(
@@ -470,7 +492,7 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Fingerprint mismatch on inviter-side.",
                 );
-                return ret;
+                return Ok(ret);
             }
             info!(context, "Fingerprint verified.",);
             // verify that the `Secure-Join-Auth:`-header matches the secret written to the QR code
@@ -482,12 +504,12 @@ pub fn handle_securejoin_handshake(
                         contact_chat_id,
                         "Auth not provided.",
                     );
-                    return ret;
+                    return Ok(ret);
                 }
             };
             if !token::exists(context, token::Namespace::Auth, &auth_0) {
                 could_not_establish_secure_connection(context, contact_chat_id, "Auth invalid.");
-                return ret;
+                return Ok(ret);
             }
             if mark_peer_as_verified(context, fingerprint).is_err() {
                 could_not_establish_secure_connection(
@@ -495,7 +517,7 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Fingerprint mismatch on inviter-side.",
                 );
-                return ret;
+                return Ok(ret);
             }
             Contact::scaleup_origin_by_id(context, contact_id, Origin::SecurejoinInvited);
             info!(context, "Auth verified.",);
@@ -509,7 +531,7 @@ pub fn handle_securejoin_handshake(
                 let (group_chat_id, _, _) = chat::get_chat_id_by_grpid(context, &field_grpid);
                 if group_chat_id == 0 {
                     error!(context, "Chat {} not found.", &field_grpid);
-                    return ret;
+                    return Ok(ret);
                 } else {
                     if let Err(err) =
                         chat::add_contact_to_chat_ex(context, group_chat_id, contact_id, true)
@@ -524,11 +546,11 @@ pub fn handle_securejoin_handshake(
         }
         "vg-member-added" | "vc-contact-confirm" => {
             if join_vg {
-                ret = DC_HANDSHAKE_CONTINUE_NORMAL_PROCESSING;
+                ret.hide_this_msg = false;
             }
             if context.bob.read().unwrap().expects != DC_VC_CONTACT_CONFIRM {
                 info!(context, "Message belongs to a different handshake.",);
-                return ret;
+                return Ok(ret);
             }
             let cond = {
                 let bob = context.bob.read().unwrap();
@@ -540,7 +562,7 @@ pub fn handle_securejoin_handshake(
                     context,
                     "Message out of sync or belongs to a different handshake.",
                 );
-                return ret;
+                return Ok(ret);
             }
             let scanned_fingerprint_of_alice = get_qr_attr!(context, fingerprint).to_string();
 
@@ -564,8 +586,8 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Contact confirm message not encrypted.",
                 );
-                end_bobs_joining(context, DC_BOB_ERROR);
-                return ret;
+                ret.bob_securejoin_success = Some(false);
+                return Ok(ret);
             }
 
             if mark_peer_as_verified(context, &scanned_fingerprint_of_alice).is_err() {
@@ -574,7 +596,7 @@ pub fn handle_securejoin_handshake(
                     contact_chat_id,
                     "Fingerprint mismatch on joiner-side.",
                 );
-                return ret;
+                return Ok(ret);
             }
             Contact::scaleup_origin_by_id(context, contact_id, Origin::SecurejoinJoined);
             emit_event!(context, Event::ContactsChanged(None));
@@ -583,7 +605,7 @@ pub fn handle_securejoin_handshake(
                 .unwrap_or_default();
             if join_vg && !addr_equals_self(context, cg_member_added) {
                 info!(context, "Message belongs to a different handshake (scaled up contact anyway to allow creation of group).");
-                return ret;
+                return Ok(ret);
             }
             secure_connection_established(context, contact_chat_id);
             context.bob.write().unwrap().expects = 0;
@@ -597,7 +619,8 @@ pub fn handle_securejoin_handshake(
                     "",
                 );
             }
-            end_bobs_joining(context, DC_BOB_SUCCESS);
+            ret.stop_ongoing_process = true;
+            ret.bob_securejoin_success = Some(true);
         }
         "vg-member-added-received" => {
             /* ============================================================
@@ -607,7 +630,7 @@ pub fn handle_securejoin_handshake(
             if let Ok(contact) = Contact::get_by_id(context, contact_id) {
                 if contact.is_verified(context) == VerifiedStatus::Unverified {
                     warn!(context, "vg-member-added-received invalid.",);
-                    return ret;
+                    return Ok(ret);
                 }
                 inviter_progress!(context, contact_id, 800);
                 inviter_progress!(context, contact_id, 1000);
@@ -621,22 +644,17 @@ pub fn handle_securejoin_handshake(
                 });
             } else {
                 warn!(context, "vg-member-added-received invalid.",);
-                return ret;
+                return Ok(ret);
             }
         }
         _ => {
             warn!(context, "invalid step: {}", step);
         }
     }
-    if ret == DC_HANDSHAKE_STOP_NORMAL_PROCESSING {
-        ret |= DC_HANDSHAKE_ADD_DELETE_JOB;
+    if ret.hide_this_msg {
+        ret.delete_this_msg = true;
     }
-    ret
-}
-
-fn end_bobs_joining(context: &Context, status: libc::c_int) {
-    context.bob.write().unwrap().status = status;
-    context.stop_ongoing();
+    Ok(ret)
 }
 
 fn secure_connection_established(context: &Context, contact_chat_id: u32) {

src/smtp.rs

@@ -1,6 +1,8 @@
 use lettre::smtp::client::net::*;
 use lettre::*;
 
+use failure::Fail;
+
 use crate::constants::*;
 use crate::context::Context;
 use crate::error::Error;
@@ -17,6 +19,16 @@ pub struct Smtp {
     from: Option<EmailAddress>,
 }
 
+#[derive(Debug, Fail)]
+pub enum SmtpError {
+    #[fail(display = "Envelope error: {}", _0)]
+    EnvelopeError(#[cause] lettre::error::Error),
+    #[fail(display = "Send error: {}", _0)]
+    SendError(#[cause] lettre::smtp::error::Error),
+    #[fail(display = "SMTP has no transport")]
+    NoTransport,
+}
+
 impl Smtp {
     /// Create a new Smtp instances.
     pub fn new() -> Self {
@@ -144,7 +156,7 @@ impl Smtp {
         recipients: Vec<EmailAddress>,
         message: Vec<u8>,
         job_id: u32,
-    ) -> Result<(), Error> {
+    ) -> Result<(), SmtpError> {
         let message_len = message.len();
 
         let recipients_display = recipients
@@ -154,36 +166,28 @@ impl Smtp {
             .join(",");
 
         if let Some(ref mut transport) = self.transport {
-            let envelope = match Envelope::new(self.from.clone(), recipients) {
-                Ok(env) => env,
-                Err(err) => {
-                    bail!("{}", err);
-                }
-            };
+            let envelope =
+                Envelope::new(self.from.clone(), recipients).map_err(SmtpError::EnvelopeError)?;
             let mail = SendableEmail::new(
                 envelope,
                 format!("{}", job_id), // only used for internal logging
                 message,
             );
 
-            match transport.send(mail) {
-                Ok(_) => {
-                    context.call_cb(Event::SmtpMessageSent(format!(
-                        "Message len={} was smtp-sent to {}",
-                        message_len, recipients_display
-                    )));
-                    self.transport_connected = true;
-                    Ok(())
-                }
-                Err(err) => {
-                    bail!("SMTP failed len={}: error: {}", message_len, err);
-                }
-            }
+            transport.send(mail).map_err(SmtpError::SendError)?;
+
+            context.call_cb(Event::SmtpMessageSent(format!(
+                "Message len={} was smtp-sent to {}",
+                message_len, recipients_display
+            )));
+            self.transport_connected = true;
+            Ok(())
         } else {
-            bail!(
-                "uh? SMTP has no transport,  failed to send to {:?}",
-                recipients_display
+            warn!(
+                context,
+                "uh? SMTP has no transport, failed to send to {}", recipients_display
             );
+            return Err(SmtpError::NoTransport);
         }
     }
 }

src/sql.rs

@@ -829,6 +829,14 @@ fn open(
             update_icons = true;
             sql.set_raw_config_int(context, "dbversion", 59)?;
         }
+        if dbversion < 60 {
+            info!(context, "[migration] v60");
+            sql.execute(
+                "ALTER TABLE chats ADD COLUMN created_timestamp INTEGER DEFAULT 0;",
+                NO_PARAMS,
+            )?;
+            sql.set_raw_config_int(context, "dbversion", 60)?;
+        }
 
         // (2) updates that require high-level objects
         // (the structure is complete now and all objects are usable)

src/test_utils.rs

@@ -7,7 +7,7 @@ use tempfile::{tempdir, TempDir};
 
 use crate::config::Config;
 use crate::constants::KeyType;
-use crate::context::{Context, ContextBuilder, ContextCallback};
+use crate::context::{Context, ContextCallback};
 use crate::events::Event;
 use crate::key;
 
@@ -33,9 +33,7 @@ pub fn test_context(callback: Option<Box<ContextCallback>>) -> TestContext {
         Some(cb) => cb,
         None => Box::new(|_, _| 0),
     };
-    let ctx = ContextBuilder::new(cb, "FakeOs".into(), dbfile)
-        .create()
-        .unwrap();
+    let ctx = Context::new(cb, "FakeOs".into(), dbfile).unwrap();
     TestContext { ctx: ctx, dir: dir }
 }
 

tests/stress.rs

@@ -219,9 +219,7 @@ struct TestContext {
 fn create_test_context() -> TestContext {
     let dir = tempdir().unwrap();
     let dbfile = dir.path().join("db.sqlite");
-    let ctx = ContextBuilder::new(Box::new(cb), "FakeOs".into(), dbfile)
-        .create()
-        .unwrap();
+    let ctx = Context::new(Box::new(cb), "FakeOs".into(), dbfile).unwrap();
     TestContext { ctx, dir }
 }
 
@@ -235,26 +233,6 @@ fn test_dc_get_oauth2_url() {
     assert_eq!(res, Some("https://accounts.google.com/o/oauth2/auth?client_id=959970109878%2D4mvtgf6feshskf7695nfln6002mom908%2Eapps%2Egoogleusercontent%2Ecom&redirect_uri=chat%2Edelta%3A%2Fcom%2Eb44t%2Emessenger&response_type=code&scope=https%3A%2F%2Fmail.google.com%2F%20email&access_type=offline".into()));
 }
 
-#[test]
-fn test_dc_get_oauth2_addr() {
-    let ctx = create_test_context();
-    let addr = "dignifiedquire@gmail.com";
-    let code = "fail";
-    let res = dc_get_oauth2_addr(&ctx.ctx, addr, code);
-    // this should fail as it is an invalid password
-    assert_eq!(res, None);
-}
-
-#[test]
-fn test_dc_get_oauth2_token() {
-    let ctx = create_test_context();
-    let addr = "dignifiedquire@gmail.com";
-    let code = "fail";
-    let res = dc_get_oauth2_access_token(&ctx.ctx, addr, code, false);
-    // this should fail as it is an invalid password
-    assert_eq!(res, None);
-}
-
 #[test]
 fn test_stress_tests() {
     unsafe {