diff --git a/Cargo.lock b/Cargo.lock index 57b1badb..e9d18fbb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -384,11 +384,12 @@ dependencies = [ "nanoid", "p256", "pollster", + "r2d2", + "r2d2_sqlite", "ring", "rmp-serde", "robusta_jni", "rstest", - "rusqlite", "rusqlite_migration", "security-framework", "security-framework-sys", @@ -1355,7 +1356,17 @@ checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" dependencies = [ "instant", "lock_api", - "parking_lot_core", + "parking_lot_core 0.8.6", +] + +[[package]] +name = "parking_lot" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70d58bf43669b5795d1576d0641cfb6fbb2057bf629506267a92807158584a13" +dependencies = [ + "lock_api", + "parking_lot_core 0.9.11", ] [[package]] @@ -1367,11 +1378,24 @@ dependencies = [ "cfg-if", "instant", "libc", - "redox_syscall", + "redox_syscall 0.2.16", "smallvec", "winapi", ] +[[package]] +name = "parking_lot_core" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc838d2a56b5b1a6c25f55575dfc605fabb63bb2365f6c2353ef9159aa69e4a5" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.5.17", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "password-hash" version = "0.5.0" @@ -1583,6 +1607,28 @@ version = "5.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" +[[package]] +name = "r2d2" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51de85fb3fb6524929c8a2eb85e6b6d363de4e8c48f9e2c2eac4944abc181c93" +dependencies = [ + "log", + "parking_lot 0.12.4", + "scheduled-thread-pool", +] + +[[package]] +name = "r2d2_sqlite" +version = "0.31.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63417e83dc891797eea3ad379f52a5986da4bca0d6ef28baf4d14034dd111b0c" +dependencies = [ + "r2d2", + "rusqlite", + "uuid", +] + [[package]] name = "rand" version = "0.8.5" @@ -1651,6 +1697,15 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "redox_syscall" +version = "0.5.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5407465600fb0548f1442edf71dd20683c6ed326200ace4b1ef0763521bb3b77" +dependencies = [ + "bitflags 2.9.1", +] + [[package]] name = "regex" version = "1.11.1" @@ -1883,6 +1938,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scheduled-thread-pool" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3cbc66816425a074528352f5789333ecff06ca41b36b0b0efdfbb29edc391a19" +dependencies = [ + "parking_lot 0.12.4", +] + [[package]] name = "scoped-tls" version = "1.0.1" @@ -2047,7 +2111,7 @@ dependencies = [ "fxhash", "libc", "log", - "parking_lot", + "parking_lot 0.11.2", ] [[package]] @@ -2611,6 +2675,18 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" +[[package]] +name = "uuid" +version = "1.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3cf4199d1e5d15ddd86a694e4d0dffa9c323ce759fea589f00fef9d81cc1931d" +dependencies = [ + "getrandom 0.3.3", + "js-sys", + "rand 0.9.1", + "wasm-bindgen", +] + [[package]] name = "valuable" version = "0.1.1" @@ -2662,6 +2738,7 @@ checksum = "1edc8929d7499fc4e8f0be2262a241556cfc54a0bea223790e71446f2aab1ef5" dependencies = [ "cfg-if", "once_cell", + "rustversion", "wasm-bindgen-macro", ] diff --git a/Cargo.toml b/Cargo.toml index 6fbdc118..95f6dbab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,8 +95,9 @@ zeroize = { version = "1.8.1", features = ["derive"] } itertools = "0.14.0" rmp-serde = "1.3.0" security-framework-sys = { version = "2.14.0", optional = true } -rusqlite = { version = "0.37.0", features = ["bundled"] } rusqlite_migration = "2.3.0" +r2d2 = "0.8.10" +r2d2_sqlite = { version = "0.31.0", features = ["bundled"] } [dev-dependencies] color-eyre = "0.6.3" diff --git a/cSpell.json b/cSpell.json index f2168cdf..1ff622c2 100644 --- a/cSpell.json +++ b/cSpell.json @@ -35,10 +35,13 @@ "rngs", "robusta", "rstest", + "rusqlite", "rustup", "securestore", "serde", "sodiumoxide", + "sqlite", + "Sqlite", "Swatinem", "Taskfile", "thiserror", diff --git a/src/storage/mod.rs b/src/storage/mod.rs index c3c03c7f..d9834c5f 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -276,13 +276,16 @@ pub(crate) enum StorageField { #[cfg(test)] mod test { + use super::*; + + use std::sync::LazyLock; use nanoid::nanoid; use rstest::{fixture, rstest}; - use crate::prelude::KeySpec; + use crate::{prelude::KeySpec, tests::TestStore}; - use super::*; + static TEST_KV_STORE: LazyLock = LazyLock::new(TestStore::new); const DUMMY_SPEC: Spec = Spec::KeySpec(KeySpec { cipher: crate::prelude::Cipher::AesGcm256, @@ -300,9 +303,7 @@ mod test { #[fixture] fn storage_manager() -> StorageManager { - let config = vec![AdditionalConfig::FileStoreConfig { - db_dir: "testfolder".to_owned(), - }]; + let config = TEST_KV_STORE.impl_config().additional_config; StorageManager::new(nanoid!(), &config).unwrap().unwrap() } diff --git a/src/storage/storage_backend/mod.rs b/src/storage/storage_backend/mod.rs index b1fe0dde..a6a38051 100644 --- a/src/storage/storage_backend/mod.rs +++ b/src/storage/storage_backend/mod.rs @@ -15,7 +15,7 @@ use crate::{ key::ScopedKey, storage_backend::{ kv_store::KvStorageBackendError, - sqlite_store::{SqliteBackend, SqliteBackendError}, + sqlite_store::{SqliteBackend, SqliteBackendError, SqliteBackendInitializationError}, }, StorageManagerInitializationError, }, @@ -32,7 +32,7 @@ pub enum StorageBackendError { #[derive(Debug, Error)] pub enum StorageBackendInitializationError { #[error(transparent)] - Sqlite(#[from] SqliteBackendError), + Sqlite(#[from] SqliteBackendInitializationError), } #[enum_dispatch] @@ -142,7 +142,6 @@ mod test { fn create_file_storage_backend() -> StorageBackendExplicit { let mut db_dir = TEST_TMP_DIR.path().to_path_buf(); db_dir.push(&nanoid!()); - std::fs::create_dir(&db_dir).expect("should be able to create dir"); let additional_configs = vec![AdditionalConfig::FileStoreConfig { db_dir: db_dir.to_string_lossy().to_string(), }]; diff --git a/src/storage/storage_backend/sqlite_store.rs b/src/storage/storage_backend/sqlite_store.rs index d458384e..4af9e647 100644 --- a/src/storage/storage_backend/sqlite_store.rs +++ b/src/storage/storage_backend/sqlite_store.rs @@ -1,11 +1,16 @@ use core::fmt; use std::{ - path::Path, - sync::{Arc, Mutex}, + fs::create_dir_all, + path::{Path, PathBuf}, + sync::Arc, }; use itertools::Itertools; -use rusqlite::{named_params, Connection}; +use r2d2::{Builder, Pool}; +use r2d2_sqlite::{ + rusqlite::{self, named_params}, + SqliteConnectionManager, +}; use rusqlite_migration::{Migrations, M}; use thiserror::Error; @@ -16,17 +21,34 @@ use crate::storage::{ #[derive(Error, Debug)] pub enum SqliteBackendError { - #[error("Failed to initialize database.")] - InitialisationError(String), - #[error("Failed to execute query.")] + #[error("Failed to execute sql query.")] SqlError(#[from] rusqlite::Error), #[error("Key not found.")] NoKeyError, + #[error("Failed to acquire SQLite pooled connection.")] + Acquire { source: r2d2::Error }, +} + +#[derive(Error, Debug)] +pub enum SqliteBackendInitializationError { + #[error("Failed database migration: {source}")] + Migration { source: rusqlite_migration::Error }, + #[error("Failed creating database dir '{path:?}' with error: {source}")] + Mkdir { + source: std::io::Error, + path: PathBuf, + }, + #[error("Failed to set SQLite pragmas: {source}")] + Pragma { source: rusqlite::Error }, + #[error("Failed opening sqlite database with path '{path:?}' and error: {source}")] + Open { source: r2d2::Error, path: PathBuf }, + #[error("Failed to acquire SQLite pooled connection.")] + Acquire { source: r2d2::Error }, } #[derive(Clone)] pub(in crate::storage) struct SqliteBackend { - conn: Arc>, + pool: Arc>, } const MIGRATIONS_SLICE: &[M<'_>] = &[ @@ -36,25 +58,33 @@ const MIGRATIONS: Migrations<'_> = Migrations::from_slice(MIGRATIONS_SLICE); impl SqliteBackend { pub(super) fn new(path: impl AsRef) -> Result { + create_dir_all(&path).map_err(|err| SqliteBackendInitializationError::Mkdir { + path: path.as_ref().to_path_buf(), + source: err, + })?; + let path = path.as_ref().join("keys.db"); tracing::trace!("opening sql db: {:?}", path); - let mut conn = Connection::open(&path).map_err(|_| { - StorageBackendInitializationError::Sqlite(SqliteBackendError::InitialisationError( - format!("Can't open path: {:?}", path), - )) - })?; + let manager = SqliteConnectionManager::file(&path).with_init(|conn| { + conn.pragma_update(None, "journal_mode", "wal")?; + conn.pragma_update(None, "synchronous", "normal")?; - conn.pragma_update(None, "journal_mode", "wal") - .map_err(|e| { - StorageBackendInitializationError::Sqlite(SqliteBackendError::SqlError(e)) - })?; + Ok(()) + }); + let pool = Builder::new().build(manager).map_err(|err| { + SqliteBackendInitializationError::Open { + source: err, + path: path, + } + })?; - conn.pragma_update(None, "synchronous", "normal") - .map_err(|e| { - StorageBackendInitializationError::Sqlite(SqliteBackendError::SqlError(e)) - })?; + // This is ok, as conn only is mut to hinder nested transactions on one connection. + // See `rusqlite::Transaction` for more info. + let mut conn = pool + .get() + .map_err(|err| SqliteBackendInitializationError::Acquire { source: err })?; match MIGRATIONS.to_latest(&mut conn) { Ok(_) => (), @@ -69,18 +99,11 @@ impl SqliteBackend { _, ), }) => tracing::warn!("Cant run sqlite migration: {:?}", e), - Err(e) => { - return Err(StorageBackendInitializationError::Sqlite( - SqliteBackendError::InitialisationError(format!( - "Can't run sqlite migration: {:?}", - e - )), - )) - } + Err(e) => return Err(SqliteBackendInitializationError::Migration { source: e }.into()), } Ok(Self { - conn: Arc::new(Mutex::new(conn)), + pool: Arc::new(pool), }) } } @@ -93,9 +116,9 @@ impl fmt::Debug for SqliteBackend { impl StorageBackend for SqliteBackend { fn store(&self, key: ScopedKey, data: &[u8]) -> Result<(), StorageBackendError> { - self.conn - .lock() - .unwrap() + self.pool + .get() + .map_err(|err| SqliteBackendError::Acquire { source: err })? .execute( "INSERT INTO keys (id, provider, encryption_key_id, signature_key_id, data_blob) VALUES (:id, :provider, :encryption_key_id, :signature_key_id, :data_blob) @@ -117,16 +140,20 @@ impl StorageBackend for SqliteBackend { let query = "SELECT data_blob FROM keys WHERE id = :id AND provider = :provider AND encryption_key_id = :encryption_key_id AND signature_key_id = :signature_key_id;"; - let result = self.conn.lock().unwrap().query_one( - query, - named_params! { - ":id": key.key_id, - ":provider": key.provider_scope, - ":encryption_key_id": key.encryption_scope, - ":signature_key_id": key.signature_scope, - }, - |row| row.get(0), - ); + let result = self + .pool + .get() + .map_err(|err| SqliteBackendError::Acquire { source: err })? + .query_one( + query, + named_params! { + ":id": key.key_id, + ":provider": key.provider_scope, + ":encryption_key_id": key.encryption_scope, + ":signature_key_id": key.signature_scope, + }, + |row| row.get(0), + ); match result { Ok(v) => Ok(v), @@ -141,9 +168,9 @@ impl StorageBackend for SqliteBackend { let query = "DELETE FROM keys WHERE id = :id AND provider = :provider AND encryption_key_id = :encryption_key_id AND signature_key_id = :signature_key_id;"; - self.conn - .lock() - .unwrap() + self.pool + .get() + .map_err(|err| SqliteBackendError::Acquire { source: err })? .execute( query, named_params! { @@ -158,7 +185,10 @@ impl StorageBackend for SqliteBackend { } fn keys(&self) -> Vec> { - let conn = self.conn.lock().unwrap(); + let conn = match self.pool.get() { + Ok(c) => c, + Err(err) => return vec![Err(SqliteBackendError::Acquire { source: err }.into())], + }; let query = "SELECT id, provider, encryption_key_id, signature_key_id FROM keys;"; let statement = conn.prepare(query); @@ -198,3 +228,48 @@ fn flatten_res(res: Result, E>) -> Result { Err(e) => Err(e), } } + +#[cfg(test)] +mod test { + use std::sync::LazyLock; + + use nanoid::nanoid; + use tempfile::{tempdir_in, TempDir}; + + use super::*; + + const TARGET_FOLDER: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/target"); + static TEST_TMP_DIR: LazyLock = LazyLock::new(|| tempdir_in(TARGET_FOLDER).unwrap()); + + #[test] + fn test_file_store_creation() { + let _storage = SqliteBackend::new(TEST_TMP_DIR.path().join("test_file_store_creation")) + .expect("Failed to create a file store"); + } + + #[test] + fn test_multi_file_store_creation_same_file() { + let db_dir = TEST_TMP_DIR + .path() + .join("test_multi_file_store_creation_same_file"); + + let store1 = SqliteBackend::new(&db_dir).expect("Failed to create a file store 1"); + + let store2 = SqliteBackend::new(db_dir).expect("Failed to create a file store 2"); + + let key = ScopedKey { + key_id: nanoid!(), + provider_scope: nanoid!(), + encryption_scope: nanoid!(), + signature_scope: nanoid!(), + }; + + let data = b"Hello"; + + store1.store(key.clone(), data).unwrap(); + + let fetched_data = store2.get(key).unwrap(); + + assert_eq!(&fetched_data, data); + } +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 93a2ee40..e64c4d92 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -8,10 +8,8 @@ use std::sync::{Arc, RwLock}; use std::{io, vec}; use color_eyre::install; -use tracing_subscriber::{ - filter::{EnvFilter, LevelFilter}, - fmt, -}; +use color_eyre::owo_colors::OwoColorize; +use tracing_subscriber::{filter::EnvFilter, fmt}; use crate::common::config::{AdditionalConfig, ProviderImplConfig}; use crate::common::KeyPairHandle; @@ -43,6 +41,18 @@ fn setup() { SETUP_INITIALIZATION.call_once(|| { install().unwrap(); + let env_filter = EnvFilter::builder().try_from_env().unwrap_or_else(|err| { + eprintln!( + "{} {} | {}", + "Failed to parse env-filter directives with:".blue(), + err.purple(), + "Logging with default directives.".yellow() + ); + EnvFilter::builder() + .parse("error,crypto_layer=warn") + .unwrap() + }); + // Please change this subscriber as you see fit. fmt() // .with_max_level(LevelFilter::DEBUG) @@ -50,11 +60,7 @@ fn setup() { .with_line_number(true) // .with_span_events(FmtSpan::ACTIVE) .with_writer(io::stderr) - .with_env_filter( - EnvFilter::builder() - .with_default_directive(LevelFilter::DEBUG.into()) - .from_env_lossy(), - ) + .with_env_filter(env_filter) .init(); }); }