fix export: write backup_time to the destination not the source sql file

and perform slightly cleaner teardown in python

python/src/deltachat/account.py

@@ -153,6 +153,14 @@ class Account(object):
         self.check_is_configured()
         return from_dc_charpointer(lib.dc_get_info(self._dc_context))
 
+    def get_latest_backupfile(self, backupdir):
+        """ return the latest backup file in a given directory.
+        """
+        res = lib.dc_imex_has_backup(self._dc_context, as_dc_charpointer(backupdir))
+        if res == ffi.NULL:
+            return None
+        return from_dc_charpointer(res)
+
     def get_blobdir(self):
         """ return the directory for files.
 
@@ -477,8 +485,9 @@ class Account(object):
 
     def stop_threads(self, wait=True):
         """ stop IMAP/SMTP threads. """
-        self.stop_ongoing()
-        self._threads.stop(wait=wait)
+        if self._threads.is_started():
+            self.stop_ongoing()
+            self._threads.stop(wait=wait)
 
     def shutdown(self, wait=True):
         """ stop threads and close and remove underlying dc_context and callbacks. """

python/tests/test_account.py

@@ -2,6 +2,7 @@ from __future__ import print_function
 import pytest
 import os
 import queue
+import time
 from deltachat import const, Account
 from deltachat.message import Message
 from datetime import datetime, timedelta
@@ -641,18 +642,29 @@ class TestOnlineAccount:
         assert os.path.exists(msg_in.filename)
         assert os.stat(msg_in.filename).st_size == os.stat(path).st_size
 
-    def test_import_export_online_all(self, acfactory, tmpdir):
+    def test_import_export_online_all(self, acfactory, tmpdir, lp):
         ac1 = acfactory.get_online_configuring_account()
         wait_configuration_progress(ac1, 1000)
 
+        lp.sec("create some chat content")
         contact1 = ac1.create_contact("some1@hello.com", name="some1")
         chat = ac1.create_chat_by_contact(contact1)
         chat.send_text("msg1")
         backupdir = tmpdir.mkdir("backup")
+
+        lp.sec("export all to {}".format(backupdir))
         path = ac1.export_all(backupdir.strpath)
         assert os.path.exists(path)
+        t = time.time()
 
+        lp.sec("get fresh empty account")
         ac2 = acfactory.get_unconfigured_account()
+
+        lp.sec("get latest backup file")
+        path2 = ac2.get_latest_backupfile(backupdir.strpath)
+        assert path2 == path
+
+        lp.sec("import backup and check it's proper")
         ac2.import_all(path)
         contacts = ac2.get_contacts(query="some1")
         assert len(contacts) == 1
@@ -663,6 +675,18 @@ class TestOnlineAccount:
         assert len(messages) == 1
         assert messages[0].text == "msg1"
 
+        pytest.xfail("cannot export twice yet, probably due to interrupt_idle failing")
+        # wait until a second passed since last backup
+        # because get_latest_backupfile() shall return the latest backup
+        # from a UI it's unlikely anyone manages to export two
+        # backups in one second.
+        time.sleep(max(0, 1 - (time.time() - t)))
+        lp.sec("Second-time export all to {}".format(backupdir))
+        path2 = ac1.export_all(backupdir.strpath)
+        assert os.path.exists(path2)
+        assert path2 != path
+        assert ac2.get_latest_backupfile(backupdir.strpath) == path2
+
     def test_ac_setup_message(self, acfactory, lp):
         # note that the receiving account needs to be configured and running
         # before ther setup message is send. DC does not read old messages

src/imex.rs

@@ -1,5 +1,5 @@
 use core::cmp::{max, min};
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 use num_traits::FromPrimitive;
 use rand::{thread_rng, Rng};
@@ -75,7 +75,7 @@ pub fn imex(context: &Context, what: ImexMode, param1: Option<impl AsRef<Path>>)
     job_add(context, Action::ImexImap, 0, param, 0);
 }
 
-/// Returns the filename of the backup found (otherwise an error) 
+/// Returns the filename of the backup found (otherwise an error)
 pub fn has_backup(context: &Context, dir_name: impl AsRef<Path>) -> Result<String> {
     let dir_name = dir_name.as_ref();
     let dir_iter = std::fs::read_dir(dir_name)?;
@@ -90,13 +90,15 @@ pub fn has_backup(context: &Context, dir_name: impl AsRef<Path>) -> Result<Strin
                 if name.starts_with("delta-chat") && name.ends_with(".bak") {
                     let sql = Sql::new();
                     if sql.open(context, &path, true) {
-                        let curr_backup_time =
-                            sql.get_raw_config_int(context, "backup_time")
-                                .unwrap_or_default() as u64;
+                        let curr_backup_time = sql
+                            .get_raw_config_int(context, "backup_time")
+                            .unwrap_or_default();
                         if curr_backup_time > newest_backup_time {
                             newest_backup_path = Some(path);
                             newest_backup_time = curr_backup_time;
                         }
+                        info!(context, "backup_time of {} is {}", name, curr_backup_time);
+                        sql.close(&context);
                     }
                 }
             }
@@ -483,10 +485,13 @@ fn export_backup(context: &Context, dir: impl AsRef<Path>) -> Result<()> {
     // let dest_path_filename = dc_get_next_backup_file(context, dir, res);
     let now = time();
     let dest_path_filename = dc_get_next_backup_path(dir, now)?;
+    let dest_path_string = dest_path_filename.to_string_lossy().to_string();
 
     sql::housekeeping(context);
 
     sql::try_execute(context, &context.sql, "VACUUM;").ok();
+
+    // we close the database during the copy of the dbfile
     context.sql.close(context);
     info!(
         context,
@@ -496,38 +501,40 @@ fn export_backup(context: &Context, dir: impl AsRef<Path>) -> Result<()> {
     );
     let copied = dc_copy_file(context, context.get_dbfile(), &dest_path_filename);
     context.sql.open(&context, &context.get_dbfile(), false);
+
     if !copied {
-        let s = dest_path_filename.to_string_lossy().to_string();
         bail!(
             "could not copy file from '{}' to '{}'",
             context.get_dbfile().display(),
-            s
+            dest_path_string
         );
     }
-    match add_files_to_export(context, &dest_path_filename) {
+    let dest_sql = Sql::new();
+    ensure!(
+        dest_sql.open(context, &dest_path_filename, false),
+        "could not open exported database {}",
+        dest_path_string
+    );
+    let res = match add_files_to_export(context, &dest_sql) {
         Err(err) => {
             dc_delete_file(context, &dest_path_filename);
             error!(context, "backup failed: {}", err);
             Err(err)
         }
         Ok(()) => {
-            context
-                .sql
-                .set_raw_config_int(context, "backup_time", now as i32)?;
+            dest_sql.set_raw_config_int(context, "backup_time", now as i32)?;
             context.call_cb(Event::ImexFileWritten(dest_path_filename.clone()));
             Ok(())
         }
-    }
+    };
+    dest_sql.close(context);
+
+    res
 }
 
-fn add_files_to_export(context: &Context, dest_path_filename: &PathBuf) -> Result<()> {
+fn add_files_to_export(context: &Context, sql: &Sql) -> Result<()> {
     // add all files as blobs to the database copy (this does not require
     // the source to be locked, neigher the destination as it is used only here)
-    let sql = Sql::new();
-    ensure!(
-        sql.open(context, &dest_path_filename, false),
-        "could not open db"
-    );
     if !sql.table_exists("backup_blobs") {
         sql::execute(
             context,