Use anyhow's with_context

Otherwise you do not know which account needs unlocking if the
selected account is locked.

CHANGELOG.md

@@ -1,5 +1,29 @@
 # Changelog
 
+## Unreleased
+
+### Changes
+- refactorings #3026
+- move messages in batches #3058
+- working with encrypted storage drastically changed to be RAII #3063
+
+### Fixes
+- avoid archived, fresh chats #3053
+- treat "NO" IMAP response to MOVE and COPY commands as an error #3058
+
+
+## 1.75.0
+
+### Changes
+- optimize `delete_expired_imap_messages()` #3047
+
+
+## 1.74.0
+
+### Fixes
+- avoid reconnection loop when message without Message-ID is marked as seen #3044
+
+
 ## 1.73.0
 
 ### API changes

Cargo.lock

@@ -347,9 +347,9 @@ dependencies = [
 
 [[package]]
 name = "async-std-resolver"
-version = "0.20.3"
+version = "0.20.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ed4e2c3da14d8ad45acb1e3191db7a918e9505b6f155b218e70a7c9a1a48c638"
+checksum = "dbf3e776afdf3a2477ef4854b85ba0dff3bd85792f685fb3c68948b4d304e4f0"
 dependencies = [
  "async-std",
  "async-trait",
@@ -1063,7 +1063,7 @@ dependencies = [
 
 [[package]]
 name = "deltachat"
-version = "1.73.0"
+version = "1.75.0"
 dependencies = [
  "ansi_term",
  "anyhow",
@@ -1143,7 +1143,7 @@ dependencies = [
 
 [[package]]
 name = "deltachat_ffi"
-version = "1.73.0"
+version = "1.75.0"
 dependencies = [
  "anyhow",
  "async-std",
@@ -3870,9 +3870,9 @@ dependencies = [
 
 [[package]]
 name = "trust-dns-proto"
-version = "0.20.3"
+version = "0.20.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ad0d7f5db438199a6e2609debe3f69f808d074e0a2888ee0bccb45fe234d03f4"
+checksum = "ca94d4e9feb6a181c690c4040d7a24ef34018d8313ac5044a61d21222ae24e31"
 dependencies = [
  "async-trait",
  "cfg-if 1.0.0",
@@ -3894,9 +3894,9 @@ dependencies = [
 
 [[package]]
 name = "trust-dns-resolver"
-version = "0.20.3"
+version = "0.20.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f6ad17b608a64bd0735e67bde16b0636f8aa8591f831a25d18443ed00a699770"
+checksum = "ecae383baad9995efaa34ce8e57d12c3f305e545887472a492b838f4b5cfb77a"
 dependencies = [
  "cfg-if 1.0.0",
  "futures-util",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "deltachat"
-version = "1.73.0"
+version = "1.75.0"
 authors = ["Delta Chat Developers (ML) <delta@codespeak.net>"]
 edition = "2018"
 license = "MPL-2.0"

deltachat-ffi/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "deltachat_ffi"
-version = "1.73.0"
+version = "1.75.0"
 description = "Deltachat FFI"
 authors = ["Delta Chat Developers (ML) <delta@codespeak.net>"]
 edition = "2018"

deltachat-ffi/deltachat.h

@@ -179,10 +179,12 @@ typedef struct _dc_accounts_event_emitter dc_accounts_event_emitter_t;
 // create/open/config/information
 
 /**
- * Create a new context object and try to open it without passphrase. If
- * database is encrypted, the result is the same as using
- * dc_context_new_closed() and the database should be opened with
- * dc_context_open() before using.
+ * Create a new context object and try to open it without passphrase.
+ *
+ * If database is encrypted NULL is returned, encrypted databases should be
+ * opened using dc_context_new_encrypted().
+ *
+ * If the database does not yet exist a new one will be created.
  *
  * @memberof dc_context_t
  * @param os_name Deprecated, pass NULL or empty string here.
@@ -192,54 +194,34 @@ typedef struct _dc_accounts_event_emitter dc_accounts_event_emitter_t;
  * @return A context object with some public members.
  *     The object must be passed to the other context functions
  *     and must be freed using dc_context_unref() after usage.
+ *     On failure NULL is returned.
+ *
+ * If you want to use multiple context objects at the same time,
+ * this can be managed using dc_accounts_t.
  */
 dc_context_t*   dc_context_new               (const char* os_name, const char* dbfile, const char* blobdir);
 
 
 /**
- * Create a new context object.  After creation it is usually opened with
- * dc_context_open() and started with dc_start_io() so it is connected and
- * mails are fetched.
+ * Create a new context object and try to open it with a passphrase.
+ *
+ * If the database does not yet exist a new one will be created.  If the
+ * database is not encrypted this will fail.
  *
  * @memberof dc_context_t
  * @param dbfile The file to use to store the database,
  *     something like `~/file` won't work, use absolute paths.
+ * @param passphrase The passphrase to use.  This MUST be non-NULL and MUST be valid
+ *     UTF-8, if either of this is not true this fails and NULL is retruned.
  * @return A context object with some public members.
  *     The object must be passed to the other context functions
  *     and must be freed using dc_context_unref() after usage.
+ *     On failure NULL is returned.
  *
  * If you want to use multiple context objects at the same time,
  * this can be managed using dc_accounts_t.
  */
-dc_context_t*   dc_context_new_closed        (const char* dbfile);
-
-
-/**
- * Opens the database with the given passphrase. This can only be used on
- * closed context, such as created by dc_context_new_closed(). If the database
- * is new, this operation sets the database passphrase. For existing databases
- * the passphrase should be the one used to encrypt the database the first
- * time.
- *
- * @memberof dc_context_t
- * @param context The context object.
- * @param passphrase The passphrase to use with the database. Pass NULL or
- * empty string to use no passphrase and no encryption.
- * @return 1 if the database is opened with this passphrase, 0 if the
- * passphrase is incorrect and on error.
- */
-int             dc_context_open              (dc_context_t *context, const char* passphrase);
-
-
-/**
- * Returns 1 if database is open.
- *
- * @memberof dc_context_t
- * @param context The context object.
- * @return 1 if database is open, 0 if database is closed
- */
-int             dc_context_is_open           (dc_context_t *context);
-
+dc_context_t*   dc_context_new_encrypted   (const char* dbfile, const char* passphrase);
 
 /**
  * Free a context object.
@@ -343,12 +325,12 @@ char*           dc_get_blobdir               (const dc_context_t* context);
  *                    and watch the `DeltaChat` folder for updates (default),
  *                    0=do not move chat-messages
  *                    changes require restarting IO by calling dc_stop_io() and then dc_start_io().
- * - `only_fetch_mvbox`
- *                  = 1=Do not fetch messages from folders other than the
- *                      `DeltaChat` folder. Messages will still be fetched from the
- *                       spam folder and `sendbox_watch` will also still be respected
- *                       if enabled.
+ * - `only_fetch_mvbox` = 1=Do not fetch messages from folders other than the
+ *                    `DeltaChat` folder. Messages will still be fetched from the
+ *                    spam folder and `sendbox_watch` will also still be respected
+ *                    if enabled.
  *                    0=watch all folders normally (default)
+ *                    changes require restarting IO by calling dc_stop_io() and then dc_start_io().
  * - `show_emails`  = DC_SHOW_EMAILS_OFF (0)=
  *                    show direct replies to chats only (default),
  *                    DC_SHOW_EMAILS_ACCEPTED_CONTACTS (1)=
@@ -2637,21 +2619,22 @@ void           dc_accounts_unref                (dc_accounts_t* accounts);
 uint32_t       dc_accounts_add_account          (dc_accounts_t* accounts);
 
 /**
- * Add a new closed account to the account manager.
- * Internally, dc_context_new_closed() is called using a unique database-name
- * in the directory specified at dc_accounts_new().
+ * Add a new account to the account manager, encrypted with a passphrase.
  *
- * If the function succeeds,
- * dc_accounts_get_all() will return one more account
- * and you can access the newly created account using dc_accounts_get_account().
- * Moreover, the newly created account will be the selected one.
+ * The account's database will be encrypted with a key derived from the
+ * passphrase and will need to be opened with the passphrase to be usable.
+ *
+ * Encrypted accounts are not automatically opened when the account
+ * manager is created.  They need to be opened using dc_accounts_load_encrypted(),
+ * to find unloaded encrypted accounts use dc_accounts_get_encrypted().
  *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
- * @return Account-id, use dc_accounts_get_account() to get the context object.
+ * @return Account-id, use dc_accounts_get_encrypted_account() to get the
+ *     context object.
  *     On errors, 0 is returned.
  */
-uint32_t       dc_accounts_add_closed_account   (dc_accounts_t* accounts);
+uint32_t       dc_accounts_add_encrypted_account(dc_accounts_t* accounts, char* passphrase);
 
 /**
  * Migrate independent accounts into accounts managed by the account manager.
@@ -2687,6 +2670,10 @@ int            dc_accounts_remove_account       (dc_accounts_t* accounts, uint32
 /**
  * List all accounts.
  *
+ * Only lists loaded accounts, some accounts might be encrypted and need to be
+ * explicitly loaded using dc_accounts_get_encrypted() and
+ * dc_accounts_load_encrypted().
+ *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
  * @return An array containing all account-ids,
@@ -2694,10 +2681,27 @@ int            dc_accounts_remove_account       (dc_accounts_t* accounts, uint32
  */
 dc_array_t*    dc_accounts_get_all              (dc_accounts_t* accounts);
 
+/**
+ * Lists encrypted accounts.
+ *
+ * This returns all *locked* encrypted accounts which must be activated using
+ * dc_accounts_load_encrypted().  Once an encrypted account has been loaded it will
+ * show up in dc_accounts_get_all() and can be retrieved using
+ * dc_accounts_get_account().
+ *
+ * @memberof dc_accounts_t
+ * @param accounts Account manager as created by dc_accounts_new().
+ * @return An array containing account-ids, use dc_array_get_id() to get the ids.
+ */
+dc_array_t*    dc_accounts_get_encrypted        (dc_accounts_t* accounts);
 
 /**
  * Get an account-context from an account-id.
  *
+ * Only accounts returned by dc_accounts_get_all() can be retrieved with this
+ * function, encrypted accounts first need to be loaded using
+ * dc_accounts_load_encrypted().
+ *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
  * @param account_id The account-id as returned e.g. by dc_accounts_get_all() or dc_accounts_add_account().
@@ -2708,9 +2712,30 @@ dc_array_t*    dc_accounts_get_all              (dc_accounts_t* accounts);
  */
 dc_context_t*  dc_accounts_get_account          (dc_accounts_t* accounts, uint32_t account_id);
 
+/**
+ * Loads an encrypted account into the Account manager.
+ *
+ * Once the account has been loaded you can also access it using
+ * dc_accounts_get_account() like any other account.  Use *dc_accounts_get_encrypted()
+ * to find accounts which need to be loaded like this.
+ *
+ * @memberof dc_accounts_t
+ * @param accounts Account manager as created by dc_accounts_new().
+ * @param account_id The account-id as returned e.g. by dc_accounts_get_encrypted(),
+ *     dc_accounts_add_encrypted_account().
+ * @param passphrase The passphrase to decrypt the account with.
+ * @return The account-context, this can be used most similar as a normal,
+ *     unmanaged account-context as created by dc_context_new().
+ *     Once you do no longer need the context-object, you have to call dc_context_unref() on it,
+ *     which, however, will not close the account but only decrease a reference counter.
+ *     On failure NULL is returned.
+ */
+dc_context_t*  dc_accounts_load_encrypted (dc_accounts_t* accounts, uint32_t account_id, char* passphrase);
+
 
 /**
- * Get the currently selected account.
+ * Get the currently selected account ID.
+ *
  * If there is at least one account in the account-manager,
  * there is always a selected one.
  * To change the selected account, use dc_accounts_select_account();
@@ -2718,13 +2743,15 @@ dc_context_t*  dc_accounts_get_account          (dc_accounts_t* accounts, uint32
  *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
- * @return The account-context, this can be used most similar as a normal,
- *     unmanaged account-context as created by dc_context_new().
- *     Once you do no longer need the context-object, you have to call dc_context_unref() on it,
- *     which, however, will not close the account but only decrease a reference counter.
- *     If there is no selected account, NULL is returned.
+ * @return The account ID of the selected account.  The context of
+ *     the selected account can then be retrieved using dc_accounts_get_account().
+ *     Note that if the selected account is encrypted you may first have to
+ *     load it using dc_accounts_load_encrypted(), you can verify if this is needed
+ *     using dc_accounts_get_encrypted().
+ *     If no account is selected 0 is returned.  However this is only possible if
+ *     there is no single account in the account manager.
  */
-dc_context_t*  dc_accounts_get_selected_account (dc_accounts_t* accounts);
+uint32_t*  dc_accounts_get_selected_account_id (dc_accounts_t* accounts);
 
 
 /**

deltachat-ffi/src/lib.rs

@@ -16,6 +16,7 @@ extern crate serde_json;
 
 use std::collections::BTreeMap;
 use std::convert::TryFrom;
+use std::ffi::CStr;
 use std::fmt::Write;
 use std::ops::Deref;
 use std::ptr;
@@ -94,20 +95,33 @@ pub unsafe extern "C" fn dc_context_new(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_context_new_closed(dbfile: *const libc::c_char) -> *mut dc_context_t {
+pub unsafe extern "C" fn dc_context_new_encrypted(
+    dbfile: *const libc::c_char,
+    passphrase: *const libc::c_char,
+) -> *mut dc_context_t {
     setup_panic!();
-
-    if dbfile.is_null() {
-        eprintln!("ignoring careless call to dc_context_new_closed()");
+    if dbfile.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_context_new_encrypted()");
         return ptr::null_mut();
     }
-
+    // Generate random ID if dc_accounts_t is not used.
     let id = rand::thread_rng().gen();
-    match block_on(Context::new_closed(
-        as_path(dbfile).to_path_buf().into(),
+    let dbfile = as_path(dbfile).to_path_buf();
+    let cstr = CStr::from_ptr(passphrase);
+    let passphrase = match cstr.to_str() {
+        Ok(s) => s,
+        Err(err) => {
+            eprintln!("passphrase was not UTF-8: {:#}", err);
+            return ptr::null_mut();
+        }
+    };
+    let ctx = block_on(Context::new_encrypted(
+        dbfile.into(),
         id,
-    )) {
-        Ok(context) => Box::into_raw(Box::new(context)),
+        passphrase.to_string(),
+    ));
+    match ctx {
+        Ok(ctx) => Box::into_raw(Box::new(ctx)),
         Err(err) => {
             eprintln!("failed to create context: {:#}", err);
             ptr::null_mut()
@@ -115,35 +129,6 @@ pub unsafe extern "C" fn dc_context_new_closed(dbfile: *const libc::c_char) -> *
     }
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn dc_context_open(
-    context: *mut dc_context_t,
-    passphrase: *const libc::c_char,
-) -> libc::c_int {
-    if context.is_null() {
-        eprintln!("ignoring careless call to dc_context_open()");
-        return 0;
-    }
-
-    let ctx = &*context;
-    let passphrase = to_string_lossy(passphrase);
-    block_on(ctx.open(passphrase))
-        .log_err(ctx, "dc_context_open() failed")
-        .map(|b| b as libc::c_int)
-        .unwrap_or(0)
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn dc_context_is_open(context: *mut dc_context_t) -> libc::c_int {
-    if context.is_null() {
-        eprintln!("ignoring careless call to dc_context_is_open()");
-        return 0;
-    }
-
-    let ctx = &*context;
-    block_on(ctx.is_open()) as libc::c_int
-}
-
 /// Release the context structure.
 ///
 /// This function releases the memory of the `dc_context_t` structure.
@@ -4083,18 +4068,17 @@ pub unsafe extern "C" fn dc_accounts_get_account(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_accounts_get_selected_account(
+pub unsafe extern "C" fn dc_accounts_get_selected_account_id(
     accounts: *mut dc_accounts_t,
-) -> *mut dc_context_t {
+) -> u32 {
     if accounts.is_null() {
         eprintln!("ignoring careless call to dc_accounts_get_selected_account()");
-        return ptr::null_mut();
+        return 0;
     }
 
     let accounts = &*accounts;
-    block_on(async move { accounts.read().await.get_selected_account().await })
-        .map(|ctx| Box::into_raw(Box::new(ctx)))
-        .unwrap_or_else(std::ptr::null_mut)
+    block_on(async move { accounts.read().await.get_selected_account_id().await })
+        .unwrap_or(0)
 }
 
 #[no_mangle]
@@ -4148,17 +4132,29 @@ pub unsafe extern "C" fn dc_accounts_add_account(accounts: *mut dc_accounts_t) -
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_accounts_add_closed_account(accounts: *mut dc_accounts_t) -> u32 {
-    if accounts.is_null() {
-        eprintln!("ignoring careless call to dc_accounts_add_closed_account()");
+pub unsafe extern "C" fn dc_accounts_add_encrypted_account(
+    accounts: *mut dc_accounts_t,
+    passphrase: *const libc::c_char,
+) -> u32 {
+    if accounts.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_accounts_add_with_password()");
         return 0;
     }
-
-    let accounts = &mut *accounts;
-
+    let accounts: &AccountsWrapper = &mut *accounts;
+    let cstr = CStr::from_ptr(passphrase);
     block_on(async move {
         let mut accounts = accounts.write().await;
-        match accounts.add_closed_account().await {
+        let passphrase = match cstr.to_str() {
+            Ok(s) => s,
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Passphrase was not UTF-8: {:#}",
+                    err
+                )));
+                return 0;
+            }
+        };
+        match accounts.add_encrypted_account(passphrase.to_string()).await {
             Ok(id) => id,
             Err(err) => {
                 accounts.emit_event(EventType::Error(format!(
@@ -4243,6 +4239,60 @@ pub unsafe extern "C" fn dc_accounts_get_all(accounts: *mut dc_accounts_t) -> *m
     Box::into_raw(Box::new(array))
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn dc_accounts_get_encrypted(
+    accounts: *mut dc_accounts_t,
+) -> *mut dc_array_t {
+    if accounts.is_null() {
+        eprintln!("ignoring careless call to dc_accounts_get_all()");
+        return ptr::null_mut();
+    }
+    let accounts: &AccountsWrapper = &*accounts;
+    let list = block_on(async move { accounts.read().await.get_encrypted() });
+    let array: dc_array_t = list.into();
+    Box::into_raw(Box::new(array))
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn dc_accounts_load_encrypted(
+    accounts: *mut dc_accounts_t,
+    account_id: u32,
+    passphrase: *const libc::c_char,
+) -> *mut dc_context_t {
+    if accounts.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_context_new_encrypted()");
+        return ptr::null_mut();
+    }
+    let accounts: &AccountsWrapper = &*accounts;
+    let cstr = CStr::from_ptr(passphrase);
+    block_on(async move {
+        let mut accounts = accounts.write().await;
+        let passphrase = match cstr.to_str() {
+            Ok(s) => s,
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Passphrase was not UTF-8: {:#}",
+                    err
+                )));
+                return ptr::null_mut();
+            }
+        };
+        match accounts
+            .load_encrypted_account(account_id, passphrase.to_string())
+            .await
+        {
+            Ok(ctx) => Box::into_raw(Box::new(ctx)),
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Failed to load encrypted account: {:#}",
+                    err
+                )));
+                ptr::null_mut()
+            }
+        }
+    })
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn dc_accounts_all_work_done(accounts: *mut dc_accounts_t) -> libc::c_int {
     if accounts.is_null() {

python/src/deltachat/direct_imap.py

@@ -224,9 +224,7 @@ class DirectImap:
         """ (blocking) wait for next idle message from server. """
         assert self._idling
         self.account.log("imap-direct: calling idle_check")
-        res = self.conn.idle_check(timeout=30)
-        if len(res) == 0:
-            raise TimeoutError
+        res = self.conn.idle_check()
         if terminate:
             self.idle_done()
         self.account.log("imap-direct: idle_check returned {!r}".format(res))

python/src/deltachat/testplugin.py

@@ -241,7 +241,6 @@ def acfactory(pytestconfig, tmpdir, request, session_liveconfig, data):
         def make_account(self, path, logid, quiet=False):
             ac = Account(path, logging=self._logging)
             ac._evtracker = ac.add_account_plugin(FFIEventTracker(ac))
-            ac._evtracker.set_timeout(30)
             ac.addr = ac.get_self_contact().addr
             ac.set_config("displayname", logid)
             if not quiet:
@@ -483,7 +482,7 @@ class BotProcess:
     def kill(self) -> None:
         self.popen.kill()
 
-    def wait(self, timeout=30) -> None:
+    def wait(self, timeout=None) -> None:
         self.popen.wait(timeout=timeout)
 
     def fnmatch_lines(self, pattern_lines):
@@ -492,7 +491,7 @@ class BotProcess:
             print("+++FNMATCH:", next_pattern)
             ignored = []
             while 1:
-                line = self.stdout_queue.get(timeout=15)
+                line = self.stdout_queue.get()
                 if line is None:
                     if ignored:
                         print("BOT stdout terminated after these lines")

python/tests/test_account.py

@@ -652,8 +652,6 @@ class TestOnlineAccount:
             pre_generated_key=False,
             config={"key_gen_type": str(const.DC_KEY_GEN_ED25519)}
         )
-        # rsa key gen can be slow especially on CI, adjust timeout
-        ac1._evtracker.set_timeout(240)
         acfactory.wait_configure_and_start_io()
         chat = acfactory.get_accepted_chat(ac1, ac2)
 
@@ -892,11 +890,11 @@ class TestOnlineAccount:
 
         chat = acfactory.get_accepted_chat(ac1, ac2)
         chat.send_text("message1")
+        ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
         chat.send_text("message2")
+        ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
         chat.send_text("message3")
         ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
-        ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
-        ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
 
     def test_forward_messages(self, acfactory, lp):
         ac1, ac2 = acfactory.get_two_online_accounts()
@@ -1834,7 +1832,6 @@ class TestOnlineAccount:
         lp.sec("trigger ac setup message and return setupcode")
         assert ac1.get_info()["fingerprint"] != ac2.get_info()["fingerprint"]
         setup_code = ac1.initiate_key_transfer()
-        ac2._evtracker.set_timeout(30)
         ev = ac2._evtracker.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
         msg = ac2.get_message_by_id(ev.data2)
         assert msg.is_setup_message()
@@ -1851,7 +1848,6 @@ class TestOnlineAccount:
     def test_ac_setup_message_twice(self, acfactory, lp):
         ac1 = acfactory.get_online_configuring_account()
         ac2 = acfactory.clone_online_account(ac1)
-        ac2._evtracker.set_timeout(30)
         acfactory.wait_configure_and_start_io()
 
         lp.sec("trigger ac setup message but ignore")
@@ -2025,7 +2021,7 @@ class TestOnlineAccount:
 
         lp.sec("ac1: send a message to group chat to promote the group")
         chat.send_text("afterwards promoted")
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "chat-modified"
         assert chat.is_promoted()
         assert sorted(x.addr for x in chat.get_contacts()) == \
@@ -2035,29 +2031,29 @@ class TestOnlineAccount:
         # note that if the above create_chat() would not
         # happen we would not receive a proper member_added event
         contact2 = chat.add_contact("devnull@testrun.org")
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "chat-modified"
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "chat-modified"
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "added"
         assert ev.message.get_sender_contact().addr == ac1_addr
         assert ev.contact.addr == "devnull@testrun.org"
 
         lp.sec("ac1: remove address2")
         chat.remove_contact(contact2)
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "chat-modified"
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "removed"
         assert ev.contact.addr == contact2.addr
         assert ev.message.get_sender_contact().addr == ac1_addr
 
         lp.sec("ac1: remove ac2 contact from chat")
         chat.remove_contact(ac2)
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "chat-modified"
-        ev = in_list.get(timeout=10)
+        ev = in_list.get()
         assert ev.action == "removed"
         assert ev.message.get_sender_contact().addr == ac1_addr
 
@@ -2682,7 +2678,13 @@ class TestOnlineAccount:
         ac1.direct_imap.select_config_folder("inbox")
         ac1.direct_imap.idle_start()
         acfactory.get_accepted_chat(ac2, ac1).send_text("hello")
-        ac1.direct_imap.idle_check(terminate=True)
+        while True:
+            if len(ac1.direct_imap.idle_check(terminate=True)) > 1:
+                # If length is 1, it's [(b'OK', b'Still here')]
+                # Could happen on very slow network.
+                #
+                # More is usually [(1, b'EXISTS'), (1, b'RECENT')]
+                break
         ac1.direct_imap.conn.move(["*"], folder)  # "*" means "biggest UID in mailbox"
 
         lp.sec("Everything prepared, now see if DeltaChat finds the message (" + variant + ")")

src/accounts.rs

@@ -1,6 +1,6 @@
 //! # Account manager module.
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use async_std::channel::{self, Receiver, Sender};
 use async_std::fs;
@@ -12,7 +12,7 @@ use uuid::Uuid;
 use anyhow::{ensure, Context as _, Result};
 use serde::{Deserialize, Serialize};
 
-use crate::context::Context;
+use crate::context::{Context, ContextError};
 use crate::events::{Event, EventType, Events};
 
 /// Account manager, that can handle multiple accounts in a single place.
@@ -50,7 +50,7 @@ impl Accounts {
 
     /// Opens an existing accounts structure. Will error if the folder doesn't exist,
     /// no account exists and no config exists.
-    pub async fn open(dir: PathBuf) -> Result<Self> {
+    async fn open(dir: PathBuf) -> Result<Self> {
         ensure!(dir.exists().await, "directory does not exist");
 
         let config_file = dir.join(CONFIG_NAME);
@@ -91,6 +91,9 @@ impl Accounts {
     }
 
     /// Get the currently selected account.
+    ///
+    /// If the selected account is encrypted and not yet loaded using
+    /// [`Accounts::load_encrypted_account`] `None` will be returned.
     pub async fn get_selected_account(&self) -> Option<Context> {
         let id = self.config.get_selected_account().await;
         self.accounts.get(&id).cloned()
@@ -124,17 +127,41 @@ impl Accounts {
         Ok(account_config.id)
     }
 
-    /// Adds a new closed account.
-    pub async fn add_closed_account(&mut self) -> Result<u32> {
+    /// Adds an new encrypted account and opens it.
+    ///
+    /// Creates a new account with encrypted database using the provided password.  Returns
+    /// the account ID of the opened account.
+    pub async fn add_encrypted_account(&mut self, passphrase: String) -> Result<u32> {
         let account_config = self.config.new_account(&self.dir).await?;
-
-        let ctx = Context::new_closed(account_config.dbfile().into(), account_config.id).await?;
+        let ctx = Context::new_encrypted(
+            account_config.dbfile().into(),
+            account_config.id,
+            passphrase,
+        )
+        .await?;
         self.emitter.add_account(&ctx).await?;
         self.accounts.insert(account_config.id, ctx);
-
         Ok(account_config.id)
     }
 
+    /// Decrypts and open an existing account.
+    pub async fn load_encrypted_account(&mut self, id: u32, passphrase: String) -> Result<Context> {
+        let account_config = self
+            .config
+            .get_account(id)
+            .await
+            .with_context(|| format!("No such account with id {}", id))?;
+        let ctx = Context::new_encrypted(
+            account_config.dbfile().into(),
+            account_config.id,
+            passphrase,
+        )
+        .await?;
+        self.emitter.add_account(&ctx).await?;
+        self.accounts.insert(id, ctx.clone());
+        Ok(ctx)
+    }
+
     /// Remove an account.
     pub async fn remove_account(&mut self, id: u32) -> Result<()> {
         let ctx = self.accounts.remove(&id);
@@ -228,6 +255,21 @@ impl Accounts {
         self.accounts.keys().copied().collect()
     }
 
+    /// Returns all encrypted accounts.
+    ///
+    /// Note that we can't really distinguish between unreadable/corrupted accounts and
+    /// encrypted accounts.  We consider all known accounts which failed to load encrypted,
+    /// they can be loaded using [`Accounts::load_encrypted_account`].
+    pub fn get_encrypted(&self) -> Vec<u32> {
+        let configured_ids: BTreeSet<u32> = self
+            .config
+            .all_configured_accounts()
+            .map(|cfg| cfg.id)
+            .collect();
+        let loaded_ids: BTreeSet<u32> = self.accounts.keys().copied().collect();
+        configured_ids.difference(&loaded_ids).copied().collect()
+    }
+
     /// This is meant especially for iOS, because iOS needs to tell the system when its background work is done.
     ///
     /// Returns whether all accounts finished their background work.
@@ -400,19 +442,29 @@ impl Config {
         Ok(Config { file, inner })
     }
 
+    /// Returns all account configurations.
+    fn all_configured_accounts(&self) -> impl Iterator<Item = &AccountConfig> {
+        self.inner.accounts.iter()
+    }
+
+    /// Loads all unencrypted accounts.
     pub async fn load_accounts(&self) -> Result<BTreeMap<u32, Context>> {
         let mut accounts = BTreeMap::new();
         for account_config in &self.inner.accounts {
-            let ctx = Context::new(account_config.dbfile().into(), account_config.id)
-                .await
-                .with_context(|| {
-                    format!(
-                        "failed to create context from file {:?}",
-                        account_config.dbfile()
-                    )
-                })?;
-
-            accounts.insert(account_config.id, ctx);
+            match Context::new(account_config.dbfile().into(), account_config.id).await {
+                Ok(ctx) => {
+                    accounts.insert(account_config.id, ctx);
+                }
+                Err(ContextError::WrongKey) => {
+                    continue;
+                }
+                Err(ContextError::Other(err)) => {
+                    return Err(err.context(format!(
+                        "failed to create context from file {}",
+                        account_config.dbfile().display()
+                    )));
+                }
+            }
         }
 
         Ok(accounts)
@@ -746,36 +798,73 @@ mod tests {
 
         assert_eq!(accounts.accounts.len(), 0);
         let account_id = accounts
-            .add_closed_account()
+            .add_encrypted_account("foobar".to_string())
             .await
-            .context("failed to add closed account")?;
+            .context("failed to add encrypted account")?;
         let account = accounts
             .get_selected_account()
             .await
             .context("failed to get account")?;
         assert_eq!(account.id, account_id);
-        let passphrase_set_success = account
-            .open("foobar".to_string())
-            .await
-            .context("failed to set passphrase")?;
-        assert!(passphrase_set_success);
         drop(accounts);
 
-        let accounts = Accounts::new(p.clone())
+        let mut accounts = Accounts::new(p.clone())
             .await
             .context("failed to create second accounts manager")?;
+        assert!(accounts.get_selected_account().await.is_none());
+        let id = accounts
+            .get_selected_account_id()
+            .await
+            .context("failed to get selected account id")?;
+
+        // Try wrong passphrase
+        assert!(accounts
+            .load_encrypted_account(id, "barfoo".to_string())
+            .await
+            .is_err());
+
+        let loaded_account = accounts
+            .load_encrypted_account(id, "foobar".to_string())
+            .await
+            .context("failed to load encrypted account")?;
+
         let account = accounts
             .get_selected_account()
             .await
             .context("failed to get account")?;
-        assert_eq!(account.is_open().await, false);
+        assert_eq!(loaded_account.id, account.id);
 
-        // Try wrong passphrase.
-        assert_eq!(account.open("barfoo".to_string()).await?, false);
-        assert_eq!(account.open("".to_string()).await?, false);
+        Ok(())
+    }
 
-        assert_eq!(account.open("foobar".to_string()).await?, true);
-        assert_eq!(account.is_open().await, true);
+    #[async_std::test]
+    async fn test_get_encrypted() -> Result<()> {
+        let dir = tempfile::tempdir().context("failed to create tempdir")?;
+        let p: PathBuf = dir.path().join("accounts").into();
+        let mut accounts = Accounts::new(p.clone())
+            .await
+            .context("failed to create accounts manager")?;
+        let account_id = accounts
+            .add_encrypted_account("secret".to_string())
+            .await
+            .context("failed to add encrypted account")?;
+        drop(accounts);
+
+        let mut accounts = Accounts::new(p.clone())
+            .await
+            .context("failed to create second accounts manager")?;
+        let encrypted_ids = accounts.get_encrypted();
+        assert_eq!(vec![account_id], encrypted_ids);
+
+        for id in encrypted_ids {
+            let res = accounts
+                .load_encrypted_account(id, "secret".to_string())
+                .await;
+            assert!(res.is_ok());
+        }
+
+        let encrypted_ids = accounts.get_encrypted();
+        assert!(encrypted_ids.is_empty());
 
         Ok(())
     }

src/chat.rs

@@ -474,22 +474,21 @@ impl ChatId {
             self
         );
 
-        if visibility == ChatVisibility::Archived {
-            context
-                .sql
-                .execute(
-                    "UPDATE msgs SET state=? WHERE chat_id=? AND state=?;",
-                    paramsv![MessageState::InNoticed, self, MessageState::InFresh],
-                )
-                .await?;
-        }
-
         context
             .sql
-            .execute(
-                "UPDATE chats SET archived=? WHERE id=?;",
-                paramsv![visibility, self],
-            )
+            .transaction(move |transaction| {
+                if visibility == ChatVisibility::Archived {
+                    transaction.execute(
+                        "UPDATE msgs SET state=? WHERE chat_id=? AND state=?;",
+                        paramsv![MessageState::InNoticed, self, MessageState::InFresh],
+                    )?;
+                }
+                transaction.execute(
+                    "UPDATE chats SET archived=? WHERE id=?;",
+                    paramsv![visibility, self],
+                )?;
+                Ok(())
+            })
             .await?;
 
         context.emit_event(EventType::MsgsChanged {
@@ -961,7 +960,7 @@ impl std::fmt::Display for ChatId {
 /// well as query for a [ChatId].
 impl rusqlite::types::ToSql for ChatId {
     fn to_sql(&self) -> rusqlite::Result<rusqlite::types::ToSqlOutput> {
-        let val = rusqlite::types::Value::Integer(self.0 as i64);
+        let val = rusqlite::types::Value::Integer(i64::from(self.0));
         let out = rusqlite::types::ToSqlOutput::Owned(val);
         Ok(out)
     }
@@ -971,7 +970,7 @@ impl rusqlite::types::ToSql for ChatId {
 impl rusqlite::types::FromSql for ChatId {
     fn column_result(value: rusqlite::types::ValueRef) -> rusqlite::types::FromSqlResult<Self> {
         i64::column_result(value).and_then(|val| {
-            if 0 <= val && val <= std::u32::MAX as i64 {
+            if 0 <= val && val <= i64::from(std::u32::MAX) {
                 Ok(ChatId::new(val as u32))
             } else {
                 Err(rusqlite::types::FromSqlError::OutOfRange(val))

src/config.rs

@@ -245,7 +245,7 @@ impl Context {
         match self.get_config_int(Config::DeleteServerAfter).await? {
             0 => Ok(None),
             1 => Ok(Some(0)),
-            x => Ok(Some(x as i64)),
+            x => Ok(Some(i64::from(x))),
         }
     }
 
@@ -267,7 +267,7 @@ impl Context {
     pub async fn get_config_delete_device_after(&self) -> Result<Option<i64>> {
         match self.get_config_int(Config::DeleteDeviceAfter).await? {
             0 => Ok(None),
-            x => Ok(Some(x as i64)),
+            x => Ok(Some(i64::from(x))),
         }
     }
 

src/context.rs

@@ -5,7 +5,7 @@ use std::ffi::OsString;
 use std::ops::Deref;
 use std::time::{Instant, SystemTime};
 
-use anyhow::{bail, ensure, Result};
+use anyhow::{bail, Context as _, Result};
 use async_std::{
     channel::{self, Receiver, Sender},
     path::{Path, PathBuf},
@@ -25,7 +25,7 @@ use crate::message::{self, MessageState, MsgId};
 use crate::quota::QuotaInfo;
 use crate::scheduler::Scheduler;
 use crate::securejoin::Bob;
-use crate::sql::Sql;
+use crate::sql::{Sql, SqlOpenError};
 
 #[derive(Clone, Debug)]
 pub struct Context {
@@ -103,74 +103,62 @@ pub fn get_info() -> BTreeMap<&'static str, String> {
     res
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum ContextError {
+    #[error("wrong passphrase or unencrypted context")]
+    WrongKey,
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
+impl From<SqlOpenError> for ContextError {
+    fn from(source: SqlOpenError) -> Self {
+        match source {
+            SqlOpenError::WrongKey => Self::WrongKey,
+            SqlOpenError::Other(err) => Self::Other(err),
+        }
+    }
+}
+
 impl Context {
     /// Creates new context and opens the database.
-    pub async fn new(dbfile: PathBuf, id: u32) -> Result<Context> {
-        let context = Self::new_closed(dbfile, id).await?;
-
-        // Open the database if is not encrypted.
-        if context.check_passphrase("".to_string()).await? {
-            context.sql.open(&context, "".to_string()).await?;
-        }
-        Ok(context)
+    pub async fn new(dbfile: PathBuf, id: u32) -> Result<Context, ContextError> {
+        Context::new_common(dbfile, id, None).await
     }
 
-    /// Creates new context without opening the database.
-    pub async fn new_closed(dbfile: PathBuf, id: u32) -> Result<Context> {
+    pub async fn new_encrypted(
+        dbfile: PathBuf,
+        id: u32,
+        passphrase: String,
+    ) -> Result<Context, ContextError> {
+        Context::new_common(dbfile, id, Some(passphrase)).await
+    }
+
+    async fn new_common(
+        dbfile: PathBuf,
+        id: u32,
+        passphrase: Option<String>,
+    ) -> Result<Context, ContextError> {
         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().await {
-            async_std::fs::create_dir_all(&blobdir).await?;
+            async_std::fs::create_dir_all(&blobdir)
+                .await
+                .context("Failed to create blobdir")?;
         }
-        let context = Context::with_blobdir(dbfile, blobdir, id).await?;
-        Ok(context)
-    }
 
-    /// Opens the database with the given passphrase.
-    ///
-    /// Returns true if passphrase is correct, false is passphrase is not correct. Fails on other
-    /// errors.
-    pub async fn open(&self, passphrase: String) -> Result<bool> {
-        if self.sql.check_passphrase(passphrase.clone()).await? {
-            self.sql.open(self, passphrase).await?;
-            Ok(true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    /// Returns true if database is open.
-    pub async fn is_open(&self) -> bool {
-        self.sql.is_open().await
-    }
-
-    /// Tests the database passphrase.
-    ///
-    /// Returns true if passphrase is correct.
-    ///
-    /// Fails if database is already open.
-    pub(crate) async fn check_passphrase(&self, passphrase: String) -> Result<bool> {
-        self.sql.check_passphrase(passphrase).await
-    }
-
-    pub(crate) async fn with_blobdir(
-        dbfile: PathBuf,
-        blobdir: PathBuf,
-        id: u32,
-    ) -> Result<Context> {
-        ensure!(
-            blobdir.is_dir().await,
-            "Blobdir does not exist: {}",
-            blobdir.display()
-        );
+        let sql = match passphrase {
+            Some(passphrase) => Sql::new_encrypted(dbfile, passphrase).await?,
+            None => Sql::new(dbfile).await?,
+        };
 
         let inner = InnerContext {
             id,
             blobdir,
             running_state: RwLock::new(Default::default()),
-            sql: Sql::new(dbfile),
+            sql,
             bob: Default::default(),
             last_smeared_timestamp: RwLock::new(0),
             generating_key_mutex: Mutex::new(()),
@@ -189,6 +177,7 @@ impl Context {
         let ctx = Context {
             inner: Arc::new(inner),
         };
+        ctx.sql.run_migrations(&ctx).await?;
 
         Ok(ctx)
     }
@@ -680,21 +669,16 @@ mod tests {
     use crate::dc_tools::dc_create_outgoing_rfc724_mid;
     use crate::message::Message;
     use crate::test_utils::TestContext;
-    use anyhow::Context as _;
     use std::time::Duration;
     use strum::IntoEnumIterator;
-    use tempfile::tempdir;
 
     #[async_std::test]
-    async fn test_wrong_db() -> Result<()> {
-        let tmp = tempfile::tempdir()?;
+    async fn test_wrong_db() {
+        let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
-        std::fs::write(&dbfile, b"123")?;
-        let res = Context::new(dbfile.into(), 1).await?;
-
-        // Broken database is indistinguishable from encrypted one.
-        assert_eq!(res.is_open().await, false);
-        Ok(())
+        std::fs::write(&dbfile, b"123").unwrap();
+        let res = Context::new(dbfile.into(), 1).await;
+        assert!(res.is_err());
     }
 
     #[async_std::test]
@@ -849,16 +833,6 @@ mod tests {
         assert!(blobdir.is_dir());
     }
 
-    #[async_std::test]
-    async fn test_wrong_blogdir() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = tmp.path().join("db.sqlite-blobs");
-        std::fs::write(&blobdir, b"123").unwrap();
-        let res = Context::new(dbfile.into(), 1).await;
-        assert!(res.is_err());
-    }
-
     #[async_std::test]
     async fn test_sqlite_parent_not_exists() {
         let tmp = tempfile::tempdir().unwrap();
@@ -870,24 +844,6 @@ mod tests {
         assert!(dbfile2.is_file());
     }
 
-    #[async_std::test]
-    async fn test_with_empty_blobdir() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = PathBuf::new();
-        let res = Context::with_blobdir(dbfile.into(), blobdir, 1).await;
-        assert!(res.is_err());
-    }
-
-    #[async_std::test]
-    async fn test_with_blobdir_not_exists() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = tmp.path().join("blobs");
-        let res = Context::with_blobdir(dbfile.into(), blobdir.into(), 1).await;
-        assert!(res.is_err());
-    }
-
     #[async_std::test]
     async fn no_crashes_on_context_deref() {
         let t = TestContext::new().await;
@@ -1048,27 +1004,42 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn test_check_passphrase() -> Result<()> {
-        let dir = tempdir()?;
+    async fn test_reopen_unencrypted() {
+        let dir = tempfile::tempdir().unwrap();
         let dbfile = dir.path().join("db.sqlite");
 
-        let id = 1;
-        let context = Context::new_closed(dbfile.clone().into(), id)
-            .await
-            .context("failed to create context")?;
-        assert_eq!(context.open("foo".to_string()).await?, true);
-        assert_eq!(context.is_open().await, true);
-        drop(context);
+        // First time: creates new db.
+        Context::new(dbfile.clone().into(), 1).await.unwrap();
 
-        let id = 2;
-        let context = Context::new(dbfile.into(), id)
-            .await
-            .context("failed to create context")?;
-        assert_eq!(context.is_open().await, false);
-        assert_eq!(context.check_passphrase("bar".to_string()).await?, false);
-        assert_eq!(context.open("false".to_string()).await?, false);
-        assert_eq!(context.open("foo".to_string()).await?, true);
+        // Opening it encrypted should now fail.
+        let res = Context::new_encrypted(dbfile.clone().into(), 1, "secret".to_string()).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
 
-        Ok(())
+        // But opening normally still works.
+        let res = Context::new(dbfile.into(), 1).await;
+        assert!(res.is_ok());
+    }
+
+    #[async_std::test]
+    async fn test_reopen_ecrypted() {
+        let dir = tempfile::tempdir().unwrap();
+        let dbfile = dir.path().join("db.sqlite");
+
+        // First time: creates new db.
+        Context::new_encrypted(dbfile.clone().into(), 1, "secret".to_string())
+            .await
+            .unwrap();
+
+        // Opening it unencrypted should now fail.
+        let res = Context::new(dbfile.clone().into(), 1).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
+
+        // Wrong password also fails.
+        let res = Context::new_encrypted(dbfile.clone().into(), 1, "oops".to_string()).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
+
+        // Finally using the right password still works.
+        let res = Context::new_encrypted(dbfile.into(), 1, "secret".to_string()).await;
+        assert!(res.is_ok());
     }
 }

src/dc_receive_imf.rs

@@ -1156,8 +1156,8 @@ INSERT INTO msgs
         stmt.execute(paramsv![
             rfc724_mid,
             chat_id,
-            if trash { 0 } else { from_id as i32 },
-            if trash { 0 } else { to_id as i32 },
+            if trash { 0 } else { i64::from(from_id) },
+            if trash { 0 } else { i64::from(to_id) },
             sort_timestamp,
             sent_timestamp,
             rcvd_timestamp,

src/dc_tools.rs

@@ -71,7 +71,7 @@ pub(crate) fn dc_gm2local_offset() -> i64 {
     /* returns the offset that must be _added_ to an UTC/GMT-time to create the localtime.
     the function may return negative values. */
     let lt = Local::now();
-    lt.offset().local_minus_utc() as i64
+    i64::from(lt.offset().local_minus_utc())
 }
 
 // timesmearing

src/ephemeral.rs

@@ -451,12 +451,11 @@ pub(crate) async fn delete_expired_imap_messages(context: &Context) -> Result<()
         .execute(
             "UPDATE imap
              SET target=''
-             WHERE EXISTS (
-               SELECT * FROM msgs
-               WHERE rfc724_mid=imap.rfc724_mid
-               AND ((download_state = 0 AND timestamp < ?) OR
-                    (download_state != 0 AND timestamp < ?) OR
-                    (ephemeral_timestamp != 0 AND ephemeral_timestamp <= ?))
+             WHERE rfc724_mid IN (
+               SELECT rfc724_mid FROM msgs
+               WHERE ((download_state = 0 AND timestamp < ?) OR
+                      (download_state != 0 AND timestamp < ?) OR
+                      (ephemeral_timestamp != 0 AND ephemeral_timestamp <= ?))
              )",
             paramsv![threshold_timestamp, threshold_timestamp_extended, now],
         )

src/imap.rs

@@ -9,7 +9,7 @@ use std::{
     collections::{BTreeMap, BTreeSet},
 };
 
-use anyhow::{anyhow, bail, format_err, Context as _, Result};
+use anyhow::{bail, format_err, Context as _, Result};
 use async_imap::types::{
     Fetch, Flag, Mailbox, Name, NameAttribute, Quota, QuotaRoot, UnsolicitedResponse,
 };
@@ -827,18 +827,98 @@ impl Imap {
         Ok(read_cnt > 0)
     }
 
+    /// Moves batch of messages identified by their UID from the currently
+    /// selected folder to the target folder.
+    async fn move_message_batch(
+        &mut self,
+        context: &Context,
+        set: &str,
+        row_ids: Vec<i64>,
+        target: &str,
+    ) -> Result<()> {
+        if self.config.can_move {
+            let session = self
+                .session
+                .as_mut()
+                .context("no session while attempting to MOVE messages")?;
+            match session.uid_mv(set, &target).await {
+                Ok(()) => {
+                    // Messages are moved or don't exist, IMAP returns OK response in both cases.
+                    context
+                        .sql
+                        .execute(
+                            format!(
+                                "DELETE FROM imap WHERE id IN ({})",
+                                row_ids.iter().map(|_| "?").collect::<Vec<&str>>().join(",")
+                            ),
+                            rusqlite::params_from_iter(row_ids),
+                        )
+                        .await
+                        .context("cannot delete moved messages from imap table")?;
+                    context.emit_event(EventType::ImapMessageMoved(format!(
+                        "IMAP messages {} moved to {}",
+                        set, target
+                    )));
+                    return Ok(());
+                }
+                Err(err) => {
+                    warn!(
+                        context,
+                        "Cannot move message, fallback to COPY/DELETE {} to {}: {}",
+                        set,
+                        target,
+                        err
+                    );
+                }
+            }
+        } else {
+            info!(
+                context,
+                "Server does not support MOVE, fallback to COPY/DELETE {} to {}", set, target
+            );
+        }
+
+        // Server does not support MOVE or MOVE failed.
+        // Copy the message to the destination folder and mark the record for deletion.
+        let session = self
+            .session
+            .as_mut()
+            .context("no session while attempting to COPY messages")?;
+        match session.uid_copy(&set, &target).await {
+            Ok(()) => {
+                context
+                    .sql
+                    .execute(
+                        format!(
+                            "UPDATE imap SET target='' WHERE id IN ({})",
+                            row_ids.iter().map(|_| "?").collect::<Vec<&str>>().join(",")
+                        ),
+                        rusqlite::params_from_iter(row_ids),
+                    )
+                    .await
+                    .context("cannot plan deletion of copied messages")?;
+                context.emit_event(EventType::ImapMessageMoved(format!(
+                    "IMAP messages {} copied to {}",
+                    set, target
+                )));
+                Ok(())
+            }
+            Err(err) => Err(err.into()),
+        }
+    }
+
     /// Moves messages.
     ///
     /// This is the only place where messages are moved on the IMAP server.
     async fn move_messages(&mut self, context: &Context, folder: &str) -> Result<()> {
-        let rows = context
+        let mut rows = context
             .sql
             .query_map(
                 "SELECT id, uid, target FROM imap
         WHERE folder = ?
         AND target != folder
         AND target != '' -- Not planned for deletion.
-        ORDER BY id",
+        ORDER BY target, uid",
                 paramsv![folder],
                 |row| {
                     let rowid: i64 = row.get(0)?;
@@ -848,105 +928,57 @@ impl Imap {
                 },
                 |rows| rows.collect::<Result<Vec<_>, _>>().map_err(Into::into),
             )
-            .await?;
+            .await?
+            .into_iter()
+            .peekable();
 
         self.prepare(context).await?;
         self.select_folder(context, Some(folder)).await?;
 
-        for (rowid, uid, target) in rows {
-            // TODO: batch moves of messages with the same destination.
-            let set = uid.to_string();
+        while let Some((_, _, target)) = rows.peek().cloned() {
+            // Construct next request for the target folder.
+            let mut uid_set = String::new();
+            let mut rowid_set = Vec::new();
 
-            if self.config.can_move {
-                if let Some(session) = &mut self.session {
-                    match session.uid_mv(&set, &target).await {
-                        Ok(_) => {
-                            context.emit_event(EventType::ImapMessageMoved(format!(
-                                "IMAP message {}/{} moved to {}",
-                                folder, uid, target
-                            )));
-                            context
-                                .sql
-                                .execute("DELETE FROM imap WHERE id=?", paramsv![rowid])
-                                .await?;
-                            continue;
-                        }
-                        Err(async_imap::error::Error::No(text)) => {
-                            // "NO" response, probably the message is moved already.
-                            info!(
-                                context,
-                                "IMAP message {}/{} cannot be moved: {}", folder, uid, text
-                            );
-                            context
-                                .sql
-                                .execute("DELETE FROM imap WHERE id=?", paramsv![rowid])
-                                .await?;
-                            continue;
-                        }
-                        Err(err) => {
-                            warn!(
-                                context,
-                                "Cannot move message, fallback to COPY/DELETE {}/{} to {}: {}",
-                                folder,
-                                uid,
-                                target,
-                                err
-                            );
-                        }
+            while uid_set.len() < 1000 {
+                // Construct a new range.
+                if let Some((start_rowid, start_uid, _)) =
+                    rows.next_if(|(_, _, start_target)| start_target == &target)
+                {
+                    rowid_set.push(start_rowid);
+                    let mut end_uid = start_uid;
+
+                    while let Some((next_rowid, next_uid, _)) =
+                        rows.next_if(|(_, next_uid, next_target)| {
+                            next_target == &target && *next_uid == end_uid + 1
+                        })
+                    {
+                        end_uid = next_uid;
+                        rowid_set.push(next_rowid);
                     }
+
+                    let uid_range = UidRange {
+                        start: start_uid,
+                        end: end_uid,
+                    };
+                    if !uid_set.is_empty() {
+                        uid_set.push(',');
+                    }
+                    uid_set.push_str(&uid_range.to_string());
                 } else {
-                    bail!("No session while attempting to move the message");
+                    break;
                 }
-            } else {
-                info!(
-                    context,
-                    "Server does not support MOVE, fallback to COPY/DELETE {}/{} to {}",
-                    folder,
-                    uid,
-                    target
-                );
             }
 
-            // Server does not support MOVE or MOVE failed.
-            // Copy the message to the destination folder and mark the record for deletion.
-            if let Some(session) = &mut self.session {
-                match session.uid_copy(&set, &target).await {
-                    Ok(_) => {
-                        context.emit_event(EventType::ImapMessageMoved(format!(
-                            "IMAP message {}/{} copied to {}",
-                            folder, uid, target
-                        )));
-                        // Plan deletion of the original message.
-                        context
-                            .sql
-                            .execute("UPDATE imap SET target='' WHERE id=?", paramsv![rowid])
-                            .await?;
-                    }
-                    Err(async_imap::error::Error::No(text)) => {
-                        // "NO" response, probably the message is moved already.
-                        info!(
-                            context,
-                            "IMAP message {}/{} cannot be copied: {}", folder, uid, text
-                        );
-                        context
-                            .sql
-                            .execute("DELETE FROM imap WHERE id=?", paramsv![rowid])
-                            .await?;
-                        continue;
-                    }
-                    Err(err) => {
-                        warn!(
-                            context,
-                            "Could not copy message {}/{}: {}", folder, uid, err
-                        );
-                        // Break the loop to avoid moving messages out of order.
-                        // We can't proceed until this message is moved or copied.
-                        break;
-                    }
-                }
-            } else {
-                bail!("No session while attempting to copy the message");
-            }
+            // Execute request.
+            self.move_message_batch(context, &uid_set, rowid_set, &target)
+                .await
+                .with_context(|| {
+                    format!(
+                        "cannot move batch of messages {:?} to folder {:?}",
+                        &uid_set, target
+                    )
+                })?;
         }
 
         Ok(())
@@ -1283,7 +1315,8 @@ impl Imap {
                 let folder = folder.clone();
 
                 // safe, as we checked above that there is a body.
-                let body = body.unwrap();
+                let body = body
+                    .context("we checked that message has body right above, but it has vanished")?;
                 let is_seen = msg.flags().any(|flag| flag == Flag::Seen);
 
                 match dc_receive_imf_inner(
@@ -1338,7 +1371,7 @@ impl Imap {
             bail!("Can't set flag, should reconnect");
         }
 
-        let session = self.session.as_mut().context("No session").unwrap();
+        let session = self.session.as_mut().context("No session")?;
         let query = format!("+FLAGS ({})", flag);
         let mut responses = session
             .uid_store(uid_set, &query)
@@ -1598,16 +1631,13 @@ impl Imap {
         self.config.can_check_quota
     }
 
-    pub async fn get_quota_roots(
+    pub(crate) async fn get_quota_roots(
         &mut self,
         mailbox_name: &str,
     ) -> Result<(Vec<QuotaRoot>, Vec<Quota>)> {
-        if let Some(session) = self.session.as_mut() {
-            let quota_roots = session.get_quota_root(mailbox_name).await?;
-            Ok(quota_roots)
-        } else {
-            Err(anyhow!("Not connected to IMAP, no session"))
-        }
+        let session = self.session.as_mut().context("no session")?;
+        let quota_roots = session.get_quota_root(mailbox_name).await?;
+        Ok(quota_roots)
     }
 }
 
@@ -1980,7 +2010,7 @@ async fn mark_seen_by_uid(
         .sql
         .query_row_optional(
             "SELECT id, chat_id FROM msgs
-                 WHERE rfc724_mid IN (
+                 WHERE id > 9 AND rfc724_mid IN (
                    SELECT rfc724_mid FROM imap
                    WHERE folder=?1
                    AND uidvalidity=?2

src/job.rs

@@ -197,7 +197,7 @@ impl Job {
                     "UPDATE jobs SET desired_timestamp=?, tries=?, param=? WHERE id=?;",
                     paramsv![
                         self.desired_timestamp,
-                        self.tries as i64,
+                        i64::from(self.tries),
                         self.param.to_string(),
                         self.job_id as i32,
                     ],
@@ -676,7 +676,7 @@ fn get_backoff_time_offset(tries: u32, action: Action) -> i64 {
             if seconds < 1 {
                 seconds = 1;
             }
-            seconds as i64
+            i64::from(seconds)
         }
     }
 }

src/lib.rs

@@ -7,7 +7,8 @@
     clippy::all,
     clippy::indexing_slicing,
     clippy::wildcard_imports,
-    clippy::needless_borrow
+    clippy::needless_borrow,
+    clippy::cast_lossless
 )]
 #![allow(
     clippy::match_bool,

src/login_param.rs

@@ -253,7 +253,8 @@ impl LoginParam {
         sql.set_raw_config(key, Some(&self.imap.server)).await?;
 
         let key = format!("{}mail_port", prefix);
-        sql.set_raw_config_int(key, self.imap.port as i32).await?;
+        sql.set_raw_config_int(key, i32::from(self.imap.port))
+            .await?;
 
         let key = format!("{}mail_user", prefix);
         sql.set_raw_config(key, Some(&self.imap.user)).await?;
@@ -273,7 +274,8 @@ impl LoginParam {
         sql.set_raw_config(key, Some(&self.smtp.server)).await?;
 
         let key = format!("{}send_port", prefix);
-        sql.set_raw_config_int(key, self.smtp.port as i32).await?;
+        sql.set_raw_config_int(key, i32::from(self.smtp.port))
+            .await?;
 
         let key = format!("{}send_user", prefix);
         sql.set_raw_config(key, Some(&self.smtp.user)).await?;

src/message.rs

@@ -183,7 +183,7 @@ impl rusqlite::types::ToSql for MsgId {
                 format_err!("Invalid MsgId {}", self.0).into(),
             ));
         }
-        let val = rusqlite::types::Value::Integer(self.0 as i64);
+        let val = rusqlite::types::Value::Integer(i64::from(self.0));
         let out = rusqlite::types::ToSqlOutput::Owned(val);
         Ok(out)
     }
@@ -194,7 +194,7 @@ impl rusqlite::types::FromSql for MsgId {
     fn column_result(value: rusqlite::types::ValueRef) -> rusqlite::types::FromSqlResult<Self> {
         // Would be nice if we could use match here, but alas.
         i64::column_result(value).and_then(|val| {
-            if 0 <= val && val <= std::u32::MAX as i64 {
+            if 0 <= val && val <= i64::from(std::u32::MAX) {
                 Ok(MsgId::new(val as u32))
             } else {
                 Err(rusqlite::types::FromSqlError::OutOfRange(val))

src/scheduler.rs

@@ -11,6 +11,7 @@ use crate::dc_tools::maybe_add_time_based_warnings;
 use crate::ephemeral::delete_expired_imap_messages;
 use crate::imap::Imap;
 use crate::job::{self, Thread};
+use crate::log::LogExt;
 use crate::smtp::{send_smtp_messages, Smtp};
 
 use self::connectivity::ConnectivityStore;
@@ -206,15 +207,11 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int
             }
 
             // Synchronize Seen flags.
-            if let Err(err) = connection
+            connection
                 .sync_seen_flags(ctx, &watch_folder)
                 .await
                 .context("sync_seen_flags")
-            {
-                connection.trigger_reconnect(ctx).await;
-                warn!(ctx, "{:#}", err);
-                return InterruptInfo::new(false);
-            }
+                .ok_or_log(ctx);
 
             connection.connectivity.set_connected(ctx).await;
 

src/sql.rs

@@ -7,10 +7,10 @@ use std::collections::HashSet;
 use std::convert::TryFrom;
 use std::time::Duration;
 
-use anyhow::{bail, Context as _, Result};
+use anyhow::{anyhow, Context as _, Result};
 use async_std::path::PathBuf;
 use async_std::prelude::*;
-use rusqlite::{config::DbConfig, Connection, OpenFlags};
+use rusqlite::{config::DbConfig, OpenFlags};
 
 use crate::blob::BlobObject;
 use crate::chat::{add_device_msg, update_device_icon, update_saved_messages_icon};
@@ -49,40 +49,47 @@ pub struct Sql {
     is_encrypted: RwLock<Option<bool>>,
 }
 
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum SqlOpenError {
+    #[error("wrong passphrase or unencrypted context")]
+    WrongKey,
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
 impl Sql {
-    pub fn new(dbfile: PathBuf) -> Sql {
-        Self {
+    /// Opens or creates a new (unencrypted) database.
+    ///
+    /// Note after creating this you **MUST** call [`Sql::run_migrations`].
+    pub(crate) async fn new(dbfile: PathBuf) -> Result<Sql, SqlOpenError> {
+        let sql = Self {
             dbfile,
             pool: Default::default(),
             is_encrypted: Default::default(),
-        }
+        };
+        sql.open("".into()).await.map(|_| sql)
     }
 
-    /// Tests SQLCipher passphrase.
+    /// Opens or creates an encrypted database.
     ///
-    /// Returns true if passphrase is correct, i.e. the database is new or can be unlocked with
-    /// this passphrase, and false if the database is already encrypted with another passphrase or
-    /// corrupted.
+    /// If the database does not exist this creates a new encrypted database with the
+    /// provided passphrase.  If the database does exist attempts to open it with the
+    /// provided passphrase.
     ///
-    /// Fails if database is already open.
-    pub async fn check_passphrase(&self, passphrase: String) -> Result<bool> {
-        if self.is_open().await {
-            bail!("Database is already opened.");
+    /// Note after creating this you **MUST** call [`Sql::run_migrations`].
+    pub(crate) async fn new_encrypted(
+        dbfile: PathBuf,
+        passphrase: String,
+    ) -> Result<Sql, SqlOpenError> {
+        if passphrase.is_empty() {
+            return Err(anyhow!("Empty passphrase").into());
         }
-
-        // Hold the lock to prevent other thread from opening the database.
-        let _lock = self.pool.write().await;
-
-        // Test that the key is correct using a single connection.
-        let connection = Connection::open(&self.dbfile)?;
-        connection
-            .pragma_update(None, "key", &passphrase)
-            .context("failed to set PRAGMA key")?;
-        let key_is_correct = connection
-            .query_row("SELECT count(*) FROM sqlite_master", [], |_row| Ok(()))
-            .is_ok();
-
-        Ok(key_is_correct)
+        let sql = Self {
+            dbfile,
+            pool: Default::default(),
+            is_encrypted: Default::default(),
+        };
+        sql.open(passphrase).await.map(|_| sql)
     }
 
     /// Checks if there is currently a connection to the underlying Sqlite database.
@@ -194,11 +201,18 @@ impl Sql {
         Ok(pool)
     }
 
-    async fn try_open(&self, context: &Context, dbfile: &Path, passphrase: String) -> Result<()> {
-        *self.pool.write().await = Some(Self::new_pool(dbfile, passphrase.to_string())?);
+    async fn try_open(&self, dbfile: &Path, passphrase: String) -> Result<(), SqlOpenError> {
+        *self.pool.write().await = Some(Self::new_pool(dbfile, passphrase)?);
 
         {
             let conn = self.get_conn().await?;
+            let res = conn.query_row("SELECT count(*) FROM sqlite_master", params![], |_row| {
+                Ok(())
+            });
+            if res.is_err() {
+                // This hides SqliteFailure "NotADatabase"
+                return Err(SqlOpenError::WrongKey);
+            }
 
             // Try to enable auto_vacuum. This will only be
             // applied if the database is new or after successful
@@ -206,21 +220,22 @@ impl Sql {
             // When auto_vacuum is INCREMENTAL, it is possible to
             // use PRAGMA incremental_vacuum to return unused
             // database pages to the filesystem.
-            conn.pragma_update(None, "auto_vacuum", &"INCREMENTAL".to_string())?;
+            conn.pragma_update(None, "auto_vacuum", &"INCREMENTAL".to_string())
+                .context("auto_vacuum pragma failed")?;
 
             // journal_mode is persisted, it is sufficient to change it only for one handle.
-            conn.pragma_update(None, "journal_mode", &"WAL".to_string())?;
+            conn.pragma_update(None, "journal_mode", &"WAL".to_string())
+                .context("journal_mode pragma failed")?;
 
             // Default synchronous=FULL is much slower. NORMAL is sufficient for WAL mode.
-            conn.pragma_update(None, "synchronous", &"NORMAL".to_string())?;
+            conn.pragma_update(None, "synchronous", &"NORMAL".to_string())
+                .context("synchronous pragma failed")?;
         }
 
-        self.run_migrations(context).await?;
-
         Ok(())
     }
 
-    pub async fn run_migrations(&self, context: &Context) -> Result<()> {
+    pub(crate) async fn run_migrations(&self, context: &Context) -> Result<()> {
         // (1) update low-level database structure.
         // this should be done before updates that use high-level objects that
         // rely themselves on the low-level structure.
@@ -290,28 +305,31 @@ impl Sql {
             }
         }
 
+        info!(context, "Migrations finished");
         Ok(())
     }
 
-    /// Opens the provided database and runs any necessary migrations.
+    /// Opens the provided database.
+    ///
+    /// To open an unencrypted database provide an empty passphrase, if the passphrase is
+    /// wrong an error is returned.
+    ///
     /// If a database is already open, this will return an error.
-    pub async fn open(&self, context: &Context, passphrase: String) -> Result<()> {
+    async fn open(&self, passphrase: String) -> Result<(), SqlOpenError> {
         if self.is_open().await {
-            error!(
-                context,
-                "Cannot open, database \"{:?}\" already opened.", self.dbfile,
-            );
-            bail!("SQL database is already opened.");
+            return Err(anyhow!("SQL database is already opened.").into());
         }
 
         let passphrase_nonempty = !passphrase.is_empty();
-        if let Err(err) = self.try_open(context, &self.dbfile, passphrase).await {
-            self.close().await;
-            Err(err)
-        } else {
-            info!(context, "Opened database {:?}.", self.dbfile);
-            *self.is_encrypted.write().await = Some(passphrase_nonempty);
-            Ok(())
+        match self.try_open(&self.dbfile, passphrase).await {
+            Ok(()) => {
+                *self.is_encrypted.write().await = Some(passphrase_nonempty);
+                Ok(())
+            }
+            Err(err) => {
+                self.close().await;
+                Err(err)
+            }
         }
     }
 
@@ -888,7 +906,7 @@ mod tests {
 
         t.sql.close().await;
         housekeeping(&t).await.unwrap_err(); // housekeeping should fail as the db is closed
-        t.sql.open(&t, "".to_string()).await.unwrap();
+        t.sql.open("".to_string()).await.unwrap();
 
         let a = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
         assert_eq!(avatar_bytes, &async_std::fs::read(&a).await.unwrap()[..]);
@@ -906,54 +924,10 @@ mod tests {
         }
     }
 
-    /// Regression test.
-    ///
-    /// Previously the code checking for existence of `config` table
-    /// checked it with `PRAGMA table_info("config")` but did not
-    /// drain `SqlitePool.fetch` result, only using the first row
-    /// returned. As a result, prepared statement for `PRAGMA` was not
-    /// finalized early enough, leaving reader connection in a broken
-    /// state after reopening the database, when `config` table
-    /// existed and `PRAGMA` returned non-empty result.
-    ///
-    /// Statements were not finalized due to a bug in sqlx:
-    /// <https://github.com/launchbadge/sqlx/issues/1147>
-    #[async_std::test]
-    async fn test_db_reopen() -> Result<()> {
-        use tempfile::tempdir;
-
-        // The context is used only for logging.
-        let t = TestContext::new().await;
-
-        // Create a separate empty database for testing.
-        let dir = tempdir()?;
-        let dbfile = dir.path().join("testdb.sqlite");
-        let sql = Sql::new(dbfile.into());
-
-        // Create database with all the tables.
-        sql.open(&t, "".to_string()).await.unwrap();
-        sql.close().await;
-
-        // Reopen the database
-        sql.open(&t, "".to_string()).await?;
-        sql.execute(
-            "INSERT INTO config (keyname, value) VALUES (?, ?);",
-            paramsv!("foo", "bar"),
-        )
-        .await?;
-
-        let value: Option<String> = sql
-            .query_get_value("SELECT value FROM config WHERE keyname=?;", paramsv!("foo"))
-            .await?;
-        assert_eq!(value.unwrap(), "bar");
-
-        Ok(())
-    }
-
     #[async_std::test]
     async fn test_migration_flags() -> Result<()> {
         let t = TestContext::new().await;
-        t.evtracker.get_info_contains("Opened database").await;
+        t.evtracker.get_info_contains("Migrations finished").await;
 
         // as migrations::run() was already executed on context creation,
         // another call should not result in any action needed.
@@ -988,36 +962,4 @@ mod tests {
 
         Ok(())
     }
-
-    #[async_std::test]
-    async fn test_check_passphrase() -> Result<()> {
-        use tempfile::tempdir;
-
-        // The context is used only for logging.
-        let t = TestContext::new().await;
-
-        // Create a separate empty database for testing.
-        let dir = tempdir()?;
-        let dbfile = dir.path().join("testdb.sqlite");
-        let sql = Sql::new(dbfile.clone().into());
-
-        sql.check_passphrase("foo".to_string()).await?;
-        sql.open(&t, "foo".to_string())
-            .await
-            .context("failed to open the database first time")?;
-        sql.close().await;
-
-        // Reopen the database
-        let sql = Sql::new(dbfile.into());
-
-        // Test that we can't open encrypted database without a passphrase.
-        assert!(sql.open(&t, "".to_string()).await.is_err());
-
-        // Now open the database with passpharse, it should succeed.
-        sql.check_passphrase("foo".to_string()).await?;
-        sql.open(&t, "foo".to_string())
-            .await
-            .context("failed to open the database second time")?;
-        Ok(())
-    }
 }

src/webxdc.rs

@@ -75,7 +75,7 @@ impl StatusUpdateId {
 
 impl rusqlite::types::ToSql for StatusUpdateId {
     fn to_sql(&self) -> rusqlite::Result<rusqlite::types::ToSqlOutput> {
-        let val = rusqlite::types::Value::Integer(self.0 as i64);
+        let val = rusqlite::types::Value::Integer(i64::from(self.0));
         let out = rusqlite::types::ToSqlOutput::Owned(val);
         Ok(out)
     }