diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs b/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs index 0a1d38c50df..f3305e986fe 100644 --- a/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs +++ b/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs @@ -74,7 +74,7 @@ impl CliState { pub fn is_database_path(&self, path: &Path) -> bool { let database_configuration = self.database_configuration().ok(); match database_configuration { - Some(c) => c.path() == Some(path), + Some(c) => c.path() == Some(path.to_path_buf()), None => false, } } @@ -125,17 +125,24 @@ impl CliState { self.delete_all_named_identities().await?; self.delete_all_nodes(true).await?; self.delete_all_named_vaults().await?; - self.delete() + self.delete().await } /// Removes all the directories storing state without loading the current state + /// The database data is only removed if the database is a SQLite one pub fn hard_reset() -> Result<()> { let dir = Self::default_dir()?; Self::delete_at(&dir) } /// Delete the local database and log files - pub fn delete(&self) -> Result<()> { + pub async fn delete(&self) -> Result<()> { + self.database.drop_tables().await?; + Ok(Self::delete_at(&self.dir)?) + } + + /// Delete the local data on disk: sqlite database file and log files + pub fn delete_local_data(&self) -> Result<()> { Self::delete_at(&self.dir) } @@ -146,7 +153,8 @@ impl CliState { } /// Backup and reset is used to save aside - /// some corrupted local state for later inspection and then reset the state + /// some corrupted local state for later inspection and then reset the state. + /// The database is backed-up only if it is a SQLite database. pub fn backup_and_reset() -> Result<()> { let dir = Self::default_dir()?; @@ -239,8 +247,8 @@ impl CliState { pub(super) fn make_database_configuration(root_path: &Path) -> Result { match DatabaseConfiguration::postgres()? { Some(configuration) => Ok(configuration), - None => Ok(DatabaseConfiguration::Sqlite( - root_path.join("database.sqlite3"), + None => Ok(DatabaseConfiguration::sqlite( + root_path.join("database.sqlite3").as_path(), )), } } @@ -251,8 +259,8 @@ impl CliState { ) -> Result { match DatabaseConfiguration::postgres()? { Some(configuration) => Ok(configuration), - None => Ok(DatabaseConfiguration::Sqlite( - root_path.join("application_database.sqlite3"), + None => Ok(DatabaseConfiguration::sqlite( + root_path.join("application_database.sqlite3").as_path(), )), } } @@ -272,7 +280,7 @@ impl CliState { // Delete the nodes database, keep the application database let _ = match Self::make_database_configuration(root_path)? { DatabaseConfiguration::Sqlite(path) => std::fs::remove_file(path)?, - _ => (), + DatabaseConfiguration::Postgres { .. } => (), }; Ok(()) } diff --git a/implementations/rust/ockam/ockam_api/src/test_utils/mod.rs b/implementations/rust/ockam/ockam_api/src/test_utils/mod.rs index 96f30381a77..767ba757017 100644 --- a/implementations/rust/ockam/ockam_api/src/test_utils/mod.rs +++ b/implementations/rust/ockam/ockam_api/src/test_utils/mod.rs @@ -41,7 +41,9 @@ pub struct NodeManagerHandle { impl Drop for NodeManagerHandle { fn drop(&mut self) { - self.cli_state.delete().expect("cannot delete cli state"); + self.cli_state + .delete_local_data() + .expect("cannot delete cli state"); } } diff --git a/implementations/rust/ockam/ockam_api/tests/common/common.rs b/implementations/rust/ockam/ockam_api/tests/common/common.rs index 72774b955a7..5388d4d53cf 100644 --- a/implementations/rust/ockam/ockam_api/tests/common/common.rs +++ b/implementations/rust/ockam/ockam_api/tests/common/common.rs @@ -31,7 +31,7 @@ pub async fn default_configuration() -> Result { let mut configuration = authority_node::Configuration { identifier: "I4dba4b2e53b2ed95967b3bab350b6c9ad9c624e5a1b2c3d4e5f6a6b5c4d3e2f1" .try_into()?, - database_configuration: DatabaseConfiguration::Sqlite(database_path), + database_configuration: DatabaseConfiguration::sqlite(database_path.as_path()), project_identifier: "123456".to_string(), tcp_listener_address: InternetAddress::new(&format!("127.0.0.1:{}", port)).unwrap(), secure_channel_listener_name: None, diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/database_configuration.rs b/implementations/rust/ockam/ockam_node/src/storage/database/database_configuration.rs index ca36534dd96..cb54d94be82 100644 --- a/implementations/rust/ockam/ockam_node/src/storage/database/database_configuration.rs +++ b/implementations/rust/ockam/ockam_node/src/storage/database/database_configuration.rs @@ -1,3 +1,4 @@ +use ockam_core::compat::rand::random_string; use ockam_core::env::get_env; use ockam_core::errcode::{Kind, Origin}; use ockam_core::{Error, Result}; @@ -20,7 +21,7 @@ pub const OCKAM_POSTGRES_PASSWORD: &str = "OCKAM_POSTGRES_PASSWORD"; #[derive(Clone, Debug, PartialEq, Eq)] pub enum DatabaseConfiguration { /// Configuration for a SQLite database - Sqlite(PathBuf), + Sqlite(String), /// Configuration for a Postgres database Postgres { /// Database host name @@ -102,7 +103,12 @@ impl DatabaseConfiguration { /// Create a local sqlite configuration pub fn sqlite(path: &Path) -> DatabaseConfiguration { - DatabaseConfiguration::Sqlite(path.to_path_buf()) + DatabaseConfiguration::Sqlite(Self::create_sqlite_on_disk_connection_string(path)) + } + + /// Create an in-memory sqlite configuration + pub fn sqlite_in_memory() -> DatabaseConfiguration { + DatabaseConfiguration::Sqlite(Self::create_sqlite_in_memory_connection_string()) } /// Return the type of database that has been configured @@ -113,10 +119,28 @@ impl DatabaseConfiguration { } } + /// Return the type of database that has been configured + pub fn connection_string(&self) -> String { + match self { + DatabaseConfiguration::Sqlite(path) => path.clone(), + DatabaseConfiguration::Postgres { + host, + port, + database_name, + user, + } => Self::create_postgres_connection_string( + host.clone(), + *port, + database_name.clone(), + user.clone(), + ), + } + } + /// Create a directory for the SQLite database file if necessary pub fn create_directory_if_necessary(&self) -> Result<()> { match self { - DatabaseConfiguration::Sqlite(path) => match path.parent() { + DatabaseConfiguration::Sqlite(path) => match PathBuf::from(path).parent() { Some(parent) => { if !parent.exists() { create_dir_all(parent) @@ -132,17 +156,37 @@ impl DatabaseConfiguration { /// Return true if the path for a SQLite database exists pub fn exists(&self) -> bool { - match self { - DatabaseConfiguration::Sqlite(path) => path.exists(), - _ => false, - } + self.path().map(|p| p.exists()).unwrap_or(false) } /// Return the database path if the database is a SQLite file. - pub fn path(&self) -> Option<&Path> { + pub fn path(&self) -> Option { match self { - DatabaseConfiguration::Sqlite(path) => Some(path.as_path()), + DatabaseConfiguration::Sqlite(path) => Some(PathBuf::from(path)), _ => None, } } + + fn create_sqlite_in_memory_connection_string() -> String { + let file_name = random_string(); + format!("sqlite:file:{file_name}?mode=memory&cache=shared") + } + + fn create_sqlite_on_disk_connection_string(path: &Path) -> String { + let url_string = &path.to_string_lossy().to_string(); + format!("sqlite:file://{url_string}?mode=rwc") + } + + fn create_postgres_connection_string( + host: String, + port: u16, + database_name: String, + user: Option, + ) -> String { + let user_password = match user { + Some(user) => format!("{}:{}@", user.user_name(), user.password()), + None => "".to_string(), + }; + format!("postgres://{user_password}{host}:{port}/{database_name}") + } } diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/application_migration_set.rs b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/application_migration_set.rs index 72960ee741e..227ef43da3b 100644 --- a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/application_migration_set.rs +++ b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/application_migration_set.rs @@ -39,10 +39,8 @@ mod tests { async fn test() -> Result<()> { let db_file = NamedTempFile::new().unwrap(); - let db = SqlxDatabase::create_no_migration(DatabaseConfiguration::Sqlite( - db_file.path().to_path_buf(), - )) - .await?; + let db = SqlxDatabase::create_no_migration(DatabaseConfiguration::sqlite(db_file.path())) + .await?; ApplicationMigrationSet::new(DatabaseType::Sqlite) .create_migrator()? diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/sql/postgres/20240613100000_project_journey.sql b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/sql/postgres/20240613110000_project_journey.sql similarity index 100% rename from implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/sql/postgres/20240613100000_project_journey.sql rename to implementations/rust/ockam/ockam_node/src/storage/database/migrations/application_migrations/sql/postgres/20240613110000_project_journey.sql diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/migration_support/migrator.rs b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/migration_support/migrator.rs index 23a4ee165c5..e316d1d8d54 100644 --- a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/migration_support/migrator.rs +++ b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/migration_support/migrator.rs @@ -240,8 +240,8 @@ impl NextMigration<'_> { Origin::Node, Kind::Conflict, format!( - "Checksum mismatch for sql migration for version {}", - migration.version + "Checksum mismatch for sql migration '{}' for version {}", + migration.description, migration.version, ), )); } diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/node_migrations/node_migration_set.rs b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/node_migrations/node_migration_set.rs index 7acc5229439..ac28e7f132d 100644 --- a/implementations/rust/ockam/ockam_node/src/storage/database/migrations/node_migrations/node_migration_set.rs +++ b/implementations/rust/ockam/ockam_node/src/storage/database/migrations/node_migrations/node_migration_set.rs @@ -61,10 +61,8 @@ mod tests { async fn test() -> Result<()> { let db_file = NamedTempFile::new().unwrap(); - let db = SqlxDatabase::create_no_migration(DatabaseConfiguration::Sqlite( - db_file.path().to_path_buf(), - )) - .await?; + let db = SqlxDatabase::create_no_migration(DatabaseConfiguration::sqlite(db_file.path())) + .await?; NodeMigrationSet::new(DatabaseType::Sqlite) .create_migrator()? diff --git a/implementations/rust/ockam/ockam_node/src/storage/database/sqlx_database.rs b/implementations/rust/ockam/ockam_node/src/storage/database/sqlx_database.rs index 178dd01c9f1..90676ed40dd 100644 --- a/implementations/rust/ockam/ockam_node/src/storage/database/sqlx_database.rs +++ b/implementations/rust/ockam/ockam_node/src/storage/database/sqlx_database.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use ockam_core::errcode::{Kind, Origin}; use sqlx::any::{install_default_drivers, AnyConnectOptions}; use sqlx::pool::PoolOptions; -use sqlx::{Any, ConnectOptions, Pool}; +use sqlx::{Any, ConnectOptions, Pool, Row}; use tempfile::NamedTempFile; use tokio_retry::strategy::{jitter, FixedInterval}; use tokio_retry::Retry; @@ -18,7 +18,7 @@ use crate::database::database_configuration::DatabaseConfiguration; use crate::database::migrations::application_migration_set::ApplicationMigrationSet; use crate::database::migrations::node_migration_set::NodeMigrationSet; use crate::database::migrations::MigrationSet; -use crate::database::{DatabaseType, DatabaseUser}; +use crate::database::DatabaseType; use ockam_core::compat::rand::random_string; use ockam_core::compat::sync::Arc; use ockam_core::{Error, Result}; @@ -33,7 +33,7 @@ use ockam_core::{Error, Result}; pub struct SqlxDatabase { /// Pool of connections to the database pub pool: Arc>, - configuration: Option, + configuration: DatabaseConfiguration, } impl Debug for SqlxDatabase { @@ -71,12 +71,12 @@ impl SqlxDatabase { /// Constructor for a sqlite database pub async fn create_sqlite(path: &Path) -> Result { - Self::create(DatabaseConfiguration::Sqlite(path.to_path_buf())).await + Self::create(DatabaseConfiguration::sqlite(path)).await } /// Constructor for a sqlite application database pub async fn create_application_sqlite(path: &Path) -> Result { - Self::create_application_database(DatabaseConfiguration::Sqlite(path.to_path_buf())).await + Self::create_application_database(DatabaseConfiguration::sqlite(path)).await } /// Constructor for a local postgres database with no data @@ -84,7 +84,7 @@ impl SqlxDatabase { match DatabaseConfiguration::postgres()? { Some(configuration) => { let db = Self::create_no_migration(configuration.clone()).await?; - db.drop_postgres_tables().await?; + db.drop_tables().await?; SqlxDatabase::create(configuration).await }, None => Err(Error::new(Origin::Core, Kind::NotFound, "There is no postgres database configuration, or it is incomplete. Please run ockam environment to check the database environment variables".to_string())), @@ -96,7 +96,7 @@ impl SqlxDatabase { match DatabaseConfiguration::postgres()? { Some(configuration) => { let db = Self::create_application_no_migration(configuration.clone()).await?; - db.drop_postgres_tables().await?; + db.drop_tables().await?; SqlxDatabase::create_application_database(configuration).await }, None => Err(Error::new(Origin::Core, Kind::NotFound, "There is no postgres database configuration, or it is incomplete. Please run ockam environment to check the database environment variables".to_string())), @@ -137,7 +137,13 @@ impl SqlxDatabase { .take(10); // limit to 10 retries let db = Retry::spawn(retry_strategy, || async { - Self::create_at(configuration.clone()).await + match Self::create_at(configuration.clone()).await { + Ok(db) => Ok(db), + Err(e) => { + println!("{e:?}"); + Err(e) + } + } }) .await?; @@ -169,13 +175,14 @@ impl SqlxDatabase { migration_set: impl MigrationSet, ) -> Result { debug!("create an in memory database for {usage}"); + let configuration = DatabaseConfiguration::sqlite_in_memory(); let pool = Self::create_in_memory_connection_pool().await?; let migrator = migration_set.create_migrator()?; migrator.migrate(&pool).await?; // FIXME: We should be careful if we run multiple nodes in one process let db = SqlxDatabase { pool: Arc::new(pool), - configuration: None, + configuration, }; Ok(db) } @@ -185,7 +192,7 @@ impl SqlxDatabase { let pool = Self::create_connection_pool(configuration.clone()).await?; Ok(SqlxDatabase { pool: Arc::new(pool), - configuration: Some(configuration), + configuration, }) } @@ -193,19 +200,9 @@ impl SqlxDatabase { configuration: DatabaseConfiguration, ) -> Result> { install_default_drivers(); - let connection_url = match configuration { - DatabaseConfiguration::Postgres { - host, - port, - database_name, - user, - } => Self::create_postgres_connection_url(host, port, database_name, user)?, - DatabaseConfiguration::Sqlite(path) => { - Self::create_sqlite_connection_url(path.as_path())? - } - }; - debug!("connecting to {connection_url}"); - let options = AnyConnectOptions::from_str(&connection_url) + let connection_string = configuration.connection_string(); + debug!("connecting to {connection_string}"); + let options = AnyConnectOptions::from_str(&connection_string) .map_err(Self::map_sql_err)? .log_statements(LevelFilter::Debug); let pool = Pool::connect_with(options) @@ -216,31 +213,7 @@ impl SqlxDatabase { /// Create a connection for a SQLite database pub async fn create_sqlite_connection_pool(path: &Path) -> Result> { - Self::create_connection_pool(DatabaseConfiguration::Sqlite(path.to_path_buf())).await - } - - fn create_postgres_connection_url( - host: String, - port: u16, - database_name: String, - user: Option, - ) -> Result { - let user_password = match user { - Some(user) => format!("{}:{}@", user.user_name(), user.password()), - None => "".to_string(), - }; - Ok(format!( - "postgres://{user_password}{host}:{port}/{database_name}" - )) - } - - fn create_sqlite_connection_url(path: &Path) -> Result { - let url_string = &path.as_os_str().to_str().ok_or(Error::new( - Origin::Api, - Kind::Invalid, - format!("incorrect database url {path:?}"), - ))?; - Ok(format!("sqlite:file://{url_string}?mode=rwc")) + Self::create_connection_pool(DatabaseConfiguration::sqlite(path)).await } pub(crate) async fn create_in_memory_connection_pool() -> Result> { @@ -258,11 +231,8 @@ impl SqlxDatabase { } /// Path to the db file if there is one - pub fn path(&self) -> Option<&Path> { - match &self.configuration { - Some(DatabaseConfiguration::Sqlite(path)) => Some(path.as_path()), - _ => None, - } + pub fn path(&self) -> Option { + self.configuration.path() } /// Map a sqlx error into an ockam error @@ -277,21 +247,39 @@ impl SqlxDatabase { Error::new(Origin::Application, Kind::Io, err) } - /// Drop all tables if the database is Postgres - pub async fn drop_postgres_tables(&self) -> Result<()> { - sqlx::query( - r#"DO $$ -DECLARE - r RECORD; -BEGIN - FOR r IN (SELECT tablename FROM pg_tables WHERE schemaname = 'public') LOOP - EXECUTE 'DROP TABLE IF EXISTS ' || quote_ident(r.tablename) || ' CASCADE'; - END LOOP; -END $$;"#, - ) - .execute(&*self.pool) - .await - .void() + /// Drop all the database tables + pub async fn drop_tables(&self) -> Result<()> { + match self.configuration { + DatabaseConfiguration::Sqlite(_) => { + let tables: Vec = + sqlx::query("SELECT name FROM sqlite_master WHERE type='table';") + .fetch_all(&*self.pool) + .await.into_core()? + .into_iter() + .map(|row| row.get(0)) + .collect(); + + // Generate and execute DROP TABLE statements for each table + for table in tables { + let drop_query = format!("DROP TABLE IF EXISTS {};", table); + sqlx::query(&drop_query).execute(&*self.pool).await.void()?; + } + Ok(()) + } + DatabaseConfiguration::Postgres { .. } => sqlx::query( + r#"DO $$ + DECLARE + r RECORD; + BEGIN + FOR r IN (SELECT tablename FROM pg_tables WHERE schemaname = 'public') LOOP + EXECUTE 'DROP TABLE IF EXISTS ' || quote_ident(r.tablename) || ' CASCADE'; + END LOOP; + END $$;"#, + ) + .execute(&*self.pool) + .await + .void(), + } } } @@ -433,7 +421,7 @@ pub mod tests { match DatabaseConfiguration::postgres()? { Some(configuration) => { let db = SqlxDatabase::create_no_migration(configuration.clone()).await?; - db.drop_postgres_tables().await?; + db.drop_tables().await?; let db = SqlxDatabase::create(configuration).await?; let inserted = insert_identity(&db).await.unwrap();