Skip to content

Commit

Permalink
Crypto store: clear db before integ tests (#3644)
Browse files Browse the repository at this point in the history
It's currently possible for integ test results to leak from one test run to the
next (for example, the indexeddb stores hang around in the browser), causing
bad test results.

Extend the test setup routine to clear out the store before the test starts.
  • Loading branch information
richvdh authored Jul 3, 2024
1 parent 99e284d commit d60ec55
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 34 deletions.
65 changes: 48 additions & 17 deletions crates/matrix-sdk-crypto/src/store/integration_tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
/// A macro which will run the CryptoStore integration test suite.
///
/// You need to provide a `async fn get_store() -> StoreResult<impl StateStore>`
/// providing a fresh store on the same level you invoke the macro.
///
/// ## Usage Example:
/// ```no_run
/// # use matrix_sdk_crypto::store::{
/// # MemoryStore as MyCryptoStore,
/// # };
///
/// #[cfg(test)]
/// mod tests {
/// use super::MyCryptoStore;
///
/// async fn get_store(
/// name: &str,
/// passphrase: Option<&str>,
/// clear_data: bool,
/// ) -> MyCryptoStore {
/// let store = MyCryptoStore::new();
/// if clear_data {
/// store.clear();
/// }
/// store
/// }
///
/// cryptostore_integration_tests!();
/// }
/// ```
#[allow(unused_macros)]
#[macro_export]
macro_rules! cryptostore_integration_tests {
Expand Down Expand Up @@ -61,7 +92,7 @@ macro_rules! cryptostore_integration_tests {
}

pub async fn get_loaded_store(name: &str) -> (Account, impl CryptoStore) {
let store = get_store(name, None).await;
let store = get_store(name, None, true).await;
let account = get_account();

store.save_pending_changes(PendingChanges { account: Some(account.deep_clone()), }).await.expect("Can't save account");
Expand Down Expand Up @@ -93,7 +124,7 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn save_account_via_generic_save() {
let store = get_store("save_account_via_generic", None).await;
let store = get_store("save_account_via_generic", None, true).await;
assert!(store.get_static_account().is_none());
assert!(store.load_account().await.unwrap().is_none());
let account = get_account();
Expand All @@ -107,7 +138,7 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn save_account() {
let store = get_store("save_account", None).await;
let store = get_store("save_account", None, true).await;
assert!(store.get_static_account().is_none());
assert!(store.load_account().await.unwrap().is_none());
let account = get_account();
Expand All @@ -121,7 +152,7 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn load_account() {
let store = get_store("load_account", None).await;
let store = get_store("load_account", None, true).await;
let account = get_account();

store
Expand All @@ -137,8 +168,8 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn load_account_with_passphrase() {
let store =
get_store("load_account_with_passphrase", Some("secret_passphrase")).await;
let passphrase = Some("secret_passphrase");
let store = get_store("load_account_with_passphrase", passphrase, true).await;
let account = get_account();

store
Expand All @@ -154,7 +185,7 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn save_and_share_account() {
let store = get_store("save_and_share_account", None).await;
let store = get_store("save_and_share_account", None, true).await;
let mut account = get_account();

store
Expand All @@ -179,7 +210,7 @@ macro_rules! cryptostore_integration_tests {

#[async_test]
async fn load_sessions() {
let store = get_store("load_sessions", None).await;
let store = get_store("load_sessions", None, true).await;
let (account, session) = get_account_and_session().await;
store
.save_pending_changes(PendingChanges { account: Some(account.deep_clone()) })
Expand All @@ -206,7 +237,7 @@ macro_rules! cryptostore_integration_tests {

// Given we created a session and saved it in the store
let (session_id, account, sender_key) = {
let store = get_store(store_name, None).await;
let store = get_store(store_name, None, true).await;
let (account, session) = get_account_and_session().await;
let sender_key = session.sender_key.to_base64();
let session_id = session.session_id().to_owned();
Expand Down Expand Up @@ -241,7 +272,7 @@ macro_rules! cryptostore_integration_tests {
};

// When we reload the store
let store = get_store(store_name, None).await;
let store = get_store(store_name, None, false).await;

// Then the same account and session info was reloaded
let loaded_account = store.load_account().await.unwrap().unwrap();
Expand Down Expand Up @@ -293,7 +324,7 @@ macro_rules! cryptostore_integration_tests {
}

// When we reload the account
let store = get_store(dir, None).await;
let store = get_store(dir, None, false).await;
store.load_account().await.unwrap();

// Then the saved session is restored
Expand Down Expand Up @@ -512,7 +543,7 @@ macro_rules! cryptostore_integration_tests {

drop(store);

let store = get_store(dir, None).await;
let store = get_store(dir, None, false).await;

store.load_account().await.unwrap();

Expand Down Expand Up @@ -564,7 +595,7 @@ macro_rules! cryptostore_integration_tests {

drop(store);

let store = get_store(dir.clone(), None).await;
let name = dir.clone();let store = get_store(name, None, false).await;
let loaded = store.load_tracked_users().await.unwrap();
check_loaded_users(loaded);
}
Expand Down Expand Up @@ -615,7 +646,7 @@ macro_rules! cryptostore_integration_tests {

drop(store);

let store = get_store(dir, None).await;
let store = get_store(dir, None, false).await;

store.load_account().await.unwrap();

Expand Down Expand Up @@ -666,7 +697,7 @@ macro_rules! cryptostore_integration_tests {
store.save_changes(changes).await.unwrap();
drop(store);

let store = get_store(dir, None).await;
let store = get_store(dir, None, false).await;

store.load_account().await.unwrap();

Expand All @@ -683,7 +714,7 @@ macro_rules! cryptostore_integration_tests {
let user_id = user_id!("@example:localhost");
let device_id: &DeviceId = "WSKKLTJZCL".into();

let store = get_store(dir, None).await;
let store = get_store(dir, None, true).await;

let account = Account::with_device_id(&user_id, device_id);

Expand All @@ -705,7 +736,7 @@ macro_rules! cryptostore_integration_tests {

drop(store);

let store = get_store(dir, None).await;
let store = get_store(dir, None, false).await;

store.load_account().await.unwrap();

Expand Down
22 changes: 15 additions & 7 deletions crates/matrix-sdk-crypto/src/store/memorystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1132,20 +1132,28 @@ mod integration_tests {
/// dropping this store won't destroy its data, since
/// [PersistentMemoryStore] is a reference-counted smart pointer
/// to an underlying [MemoryStore].
async fn get_store(name: &str, _passphrase: Option<&str>) -> PersistentMemoryStore {
async fn get_store(
name: &str,
_passphrase: Option<&str>,
clear_data: bool,
) -> PersistentMemoryStore {
// Holds on to one [PersistentMemoryStore] per test, so even if the test drops
// the store, we keep its data alive. This simulates the behaviour of
// the other stores, which keep their data in a real DB, allowing us to
// test MemoryStore using the same code.
static STORES: OnceLock<Mutex<HashMap<String, PersistentMemoryStore>>> = OnceLock::new();
let stores = STORES.get_or_init(|| Mutex::new(HashMap::new()));

stores
.lock()
.unwrap()
.entry(name.to_owned())
.or_insert_with(PersistentMemoryStore::new)
.clone()
let mut stores = stores.lock().unwrap();

if clear_data {
// Create a new PersistentMemoryStore
let new_store = PersistentMemoryStore::new();
stores.insert(name.to_owned(), new_store.clone());
new_store
} else {
stores.entry(name.to_owned()).or_insert_with(PersistentMemoryStore::new).clone()
}
}

/// Forwards all methods to the underlying [MemoryStore].
Expand Down
34 changes: 28 additions & 6 deletions crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,14 @@ impl IndexeddbCryptoStore {
IndexeddbCryptoStore::open_with_store_cipher(name, None).await
}

/// Delete the IndexedDB databases for the given name.
#[cfg(test)]
pub fn delete_stores(prefix: &str) -> Result<()> {
IdbDatabase::delete_by_name(&format!("{prefix:0}::matrix-sdk-crypto-meta"))?;
IdbDatabase::delete_by_name(&format!("{prefix:0}::matrix-sdk-crypto"))?;
Ok(())
}

fn get_static_account(&self) -> Option<StaticAccountData> {
self.static_account.read().unwrap().clone()
}
Expand Down Expand Up @@ -1735,7 +1743,14 @@ mod tests {

wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

async fn get_store(name: &str, passphrase: Option<&str>) -> IndexeddbCryptoStore {
async fn get_store(
name: &str,
passphrase: Option<&str>,
clear_data: bool,
) -> IndexeddbCryptoStore {
if clear_data {
IndexeddbCryptoStore::delete_stores(name).unwrap();
}
match passphrase {
Some(pass) => IndexeddbCryptoStore::open_with_passphrase(name, pass)
.await
Expand All @@ -1748,7 +1763,7 @@ mod tests {

#[async_test]
async fn cache_cleared_after_device_update() {
let store = get_store("cache_cleared_after_device_update", None).await;
let store = get_store("cache_cleared_after_device_update", None, true).await;
// Given we created a session and saved it in the store
let (account, session) = cryptostore_integration_tests::get_account_and_session().await;
let sender_key = session.sender_key.to_base64();
Expand Down Expand Up @@ -1800,11 +1815,17 @@ mod encrypted_tests {

wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

async fn get_store(name: &str, passphrase: Option<&str>) -> IndexeddbCryptoStore {
async fn get_store(
name: &str,
passphrase: Option<&str>,
clear_data: bool,
) -> IndexeddbCryptoStore {
if clear_data {
IndexeddbCryptoStore::delete_stores(name).unwrap();
}

let pass = passphrase.unwrap_or(name);
// make sure to use a different store name than the equivalent unencrypted test
let store_name = name.to_owned() + "_enc";
IndexeddbCryptoStore::open_with_passphrase(&store_name, pass)
IndexeddbCryptoStore::open_with_passphrase(&name, pass)
.await
.expect("Can't create a passphrase protected store")
}
Expand All @@ -1819,6 +1840,7 @@ mod encrypted_tests {
let b64_passdata = base64_encode(passdata);

// Initialise the store with some account data
IndexeddbCryptoStore::delete_stores(store_name).unwrap();
let store = IndexeddbCryptoStore::open_with_passphrase(&store_name, &b64_passdata)
.await
.expect("Can't create a passphrase-protected store");
Expand Down
26 changes: 22 additions & 4 deletions crates/matrix-sdk-sqlite/src/crypto_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,14 +1326,23 @@ mod tests {
use matrix_sdk_crypto::{cryptostore_integration_tests, cryptostore_integration_tests_time};
use once_cell::sync::Lazy;
use tempfile::{tempdir, TempDir};
use tokio::fs;

use super::SqliteCryptoStore;

static TMP_DIR: Lazy<TempDir> = Lazy::new(|| tempdir().unwrap());

async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore {
async fn get_store(
name: &str,
passphrase: Option<&str>,
clear_data: bool,
) -> SqliteCryptoStore {
let tmpdir_path = TMP_DIR.path().join(name);

if clear_data {
let _ = fs::remove_dir_all(&tmpdir_path).await;
}

SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), passphrase)
.await
.expect("Can't create a passphrase protected store")
Expand All @@ -1353,23 +1362,32 @@ mod encrypted_tests {
use matrix_sdk_test::async_test;
use once_cell::sync::Lazy;
use tempfile::{tempdir, TempDir};
use tokio::fs;

use super::SqliteCryptoStore;

static TMP_DIR: Lazy<TempDir> = Lazy::new(|| tempdir().unwrap());

async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore {
async fn get_store(
name: &str,
passphrase: Option<&str>,
clear_data: bool,
) -> SqliteCryptoStore {
let tmpdir_path = TMP_DIR.path().join(name);
let pass = passphrase.unwrap_or("default_test_password");

if clear_data {
let _ = fs::remove_dir_all(&tmpdir_path).await;
}

SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), Some(pass))
.await
.expect("Can't create a passphrase protected store")
}

#[async_test]
async fn cache_cleared() {
let store = get_store("cache_cleared", None).await;
let store = get_store("cache_cleared", None, true).await;
// Given we created a session and saved it in the store
let (account, session) = cryptostore_integration_tests::get_account_and_session().await;
let sender_key = session.sender_key.to_base64();
Expand All @@ -1396,7 +1414,7 @@ mod encrypted_tests {

#[async_test]
async fn cache_cleared_after_device_update() {
let store = get_store("cache_cleared_after_device_update", None).await;
let store = get_store("cache_cleared_after_device_update", None, true).await;
// Given we created a session and saved it in the store
let (account, session) = cryptostore_integration_tests::get_account_and_session().await;
let sender_key = session.sender_key.to_base64();
Expand Down

0 comments on commit d60ec55

Please sign in to comment.