diff --git a/CHANGELOG.md b/CHANGELOG.md index 3abb9ca41..621d24ac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Start SQL transactions with IMMEDIATE behaviour rather than default DEFERRED one. #4063 - Fix a problem with Gmail where (auto-)deleted messages would get archived instead of deleted. Move them to the Trash folder for Gmail which auto-deletes trashed messages in 30 days #3972 +- Clear config cache after backup import. This bug sometimes resulted in the import to seemingly work at first. #4067 ### API-Changes diff --git a/src/imex.rs b/src/imex.rs index a3de4c201..ffc2589dd 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -427,8 +427,6 @@ async fn import_backup( context.get_dbfile().display() ); - context.sql.config_cache.write().await.clear(); - let mut archive = Archive::new(backup_file); let mut entries = archive.entries()?; @@ -785,7 +783,10 @@ where #[cfg(test)] mod tests { + use std::time::Duration; + use ::pgp::armor::BlockType; + use tokio::task; use super::*; use crate::pgp::{split_armored_data, HEADER_AUTOCRYPT, HEADER_SETUPCODE}; @@ -933,6 +934,46 @@ mod tests { Ok(()) } + /// This is a regression test for + /// https://github.com/deltachat/deltachat-android/issues/2263 + /// where the config cache wasn't reset properly after a backup. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_import_backup_reset_config_cache() -> Result<()> { + let backup_dir = tempfile::tempdir()?; + let context1 = TestContext::new_alice().await; + let context2 = TestContext::new().await; + assert!(!context2.is_configured().await?); + + // export from context1 + imex(&context1, ImexMode::ExportBackup, backup_dir.path(), None).await?; + + // import to context2 + let backup = has_backup(&context2, backup_dir.path()).await?; + let context2_cloned = context2.clone(); + let handle = task::spawn(async move { + imex( + &context2_cloned, + ImexMode::ImportBackup, + backup.as_ref(), + None, + ) + .await + .unwrap(); + }); + + while !handle.is_finished() { + // The database is still unconfigured; + // fill the config cache with the old value. + context2.is_configured().await.ok(); + tokio::time::sleep(Duration::from_micros(1)).await; + } + + // Assert that the config cache has the new value now. + assert!(context2.is_configured().await?); + + Ok(()) + } + #[test] fn test_normalize_setup_code() { let norm = normalize_setup_code("123422343234423452346234723482349234"); diff --git a/src/sql.rs b/src/sql.rs index d96c85b66..163d4e989 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -129,43 +129,49 @@ impl Sql { .to_str() .with_context(|| format!("path {path:?} is not valid unicode"))? .to_string(); - self.call(move |conn| { - // Check that backup passphrase is correct before resetting our database. - conn.execute( - "ATTACH DATABASE ? AS backup KEY ?", - paramsv![path_str, passphrase], - ) - .context("failed to attach backup database")?; - if let Err(err) = conn - .query_row("SELECT count(*) FROM sqlite_master", [], |_row| Ok(())) - .context("backup passphrase is not correct") - { + let res = self + .call(move |conn| { + // Check that backup passphrase is correct before resetting our database. + conn.execute( + "ATTACH DATABASE ? AS backup KEY ?", + paramsv![path_str, passphrase], + ) + .context("failed to attach backup database")?; + if let Err(err) = conn + .query_row("SELECT count(*) FROM sqlite_master", [], |_row| Ok(())) + .context("backup passphrase is not correct") + { + conn.execute("DETACH DATABASE backup", []) + .context("failed to detach backup database")?; + return Err(err); + } + + // Reset the database without reopening it. We don't want to reopen the database because we + // don't have main database passphrase at this point. + // See for documentation. + // Without resetting import may fail due to existing tables. + conn.set_db_config(DbConfig::SQLITE_DBCONFIG_RESET_DATABASE, true) + .context("failed to set SQLITE_DBCONFIG_RESET_DATABASE")?; + conn.execute("VACUUM", []) + .context("failed to vacuum the database")?; + conn.set_db_config(DbConfig::SQLITE_DBCONFIG_RESET_DATABASE, false) + .context("failed to unset SQLITE_DBCONFIG_RESET_DATABASE")?; + let res = conn + .query_row("SELECT sqlcipher_export('main', 'backup')", [], |_row| { + Ok(()) + }) + .context("failed to import from attached backup database"); conn.execute("DETACH DATABASE backup", []) .context("failed to detach backup database")?; - return Err(err); - } + res?; + Ok(()) + }) + .await; - // Reset the database without reopening it. We don't want to reopen the database because we - // don't have main database passphrase at this point. - // See for documentation. - // Without resetting import may fail due to existing tables. - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_RESET_DATABASE, true) - .context("failed to set SQLITE_DBCONFIG_RESET_DATABASE")?; - conn.execute("VACUUM", []) - .context("failed to vacuum the database")?; - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_RESET_DATABASE, false) - .context("failed to unset SQLITE_DBCONFIG_RESET_DATABASE")?; - let res = conn - .query_row("SELECT sqlcipher_export('main', 'backup')", [], |_row| { - Ok(()) - }) - .context("failed to import from attached backup database"); - conn.execute("DETACH DATABASE backup", []) - .context("failed to detach backup database")?; - res?; - Ok(()) - }) - .await + // The config cache is wrong now that we have a different database + self.config_cache.write().await.clear(); + + res } /// Creates a new connection pool.