diff --git a/migra/src/errors.rs b/migra/src/errors.rs index 1a346a2..59af0ec 100644 --- a/migra/src/errors.rs +++ b/migra/src/errors.rs @@ -12,6 +12,7 @@ pub type MigraResult = Result; /// Migra error #[derive(Debug)] +#[non_exhaustive] pub enum Error { /// Represents database errors. Db(DbError), @@ -54,6 +55,7 @@ impl Error { /// All kinds of errors with witch this crate works. #[derive(Debug)] +#[non_exhaustive] pub enum DbKind { /// Failed to database connection. DatabaseConnection, diff --git a/migra/src/migration.rs b/migra/src/migration.rs index 460792e..77ca120 100644 --- a/migra/src/migration.rs +++ b/migra/src/migration.rs @@ -29,9 +29,6 @@ impl Migration { /// /// Can be presented as a list of all migrations, a list of pending migrations /// or a list of applied migrations, depending on the implementation. -/// -/// -/// #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct List { inner: Vec, @@ -219,16 +216,16 @@ mod tests { fn contains_migration() { let list = List::from(vec![FIRST_MIGRATION]); - assert_eq!(list.contains(&Migration::new(FIRST_MIGRATION)), true); - assert_eq!(list.contains(&Migration::new(SECOND_MIGRATION)), false); + assert!(list.contains(&Migration::new(FIRST_MIGRATION))); + assert!(!list.contains(&Migration::new(SECOND_MIGRATION))); } #[test] fn contains_migration_name() { let list = List::from(vec![FIRST_MIGRATION]); - assert_eq!(list.contains_name(FIRST_MIGRATION), true); - assert_eq!(list.contains_name(SECOND_MIGRATION), false); + assert!(list.contains_name(FIRST_MIGRATION)); + assert!(!list.contains_name(SECOND_MIGRATION)); } #[test] diff --git a/migra_cli/src/commands/apply.rs b/migra_cli/src/commands/apply.rs index 07d69f1..542c319 100644 --- a/migra_cli/src/commands/apply.rs +++ b/migra_cli/src/commands/apply.rs @@ -20,22 +20,12 @@ pub(crate) fn apply_sql(app: &App, cmd_opts: &ApplyCommandOpt) -> migra::StdResu .map(std::fs::read_to_string) .collect::, _>>()?; - database::should_run_in_transaction( - &mut client, - cmd_opts.transaction_opts.single_transaction, - |client| { - file_contents - .iter() - .try_for_each(|content| { - database::should_run_in_transaction( - client, - !cmd_opts.transaction_opts.single_transaction, - |client| client.apply_sql(content), - ) - }) - .map_err(From::from) - }, - )?; + database::run_in_transaction(&mut client, |client| { + file_contents + .iter() + .try_for_each(|content| client.apply_sql(content)) + .map_err(From::from) + })?; Ok(()) } diff --git a/migra_cli/src/commands/downgrade.rs b/migra_cli/src/commands/downgrade.rs index 7c2eeeb..af7b3bb 100644 --- a/migra_cli/src/commands/downgrade.rs +++ b/migra_cli/src/commands/downgrade.rs @@ -32,27 +32,19 @@ pub(crate) fn rollback_applied_migrations( }) .collect::, _>>()?; - database::should_run_in_transaction( - &mut client, - opts.transaction_opts.single_transaction, - |client| { - migrations_with_content - .iter() - .try_for_each(|(migration_name, content)| { - if all_migrations.contains_name(migration_name) { - println!("downgrade {}...", migration_name); - database::should_run_in_transaction( - client, - !opts.transaction_opts.single_transaction, - |client| client.run_downgrade_migration(migration_name, &content), - ) - } else { - Ok(()) - } - }) - .map_err(From::from) - }, - )?; + database::run_in_transaction(&mut client, |client| { + migrations_with_content + .iter() + .try_for_each(|(migration_name, content)| { + if all_migrations.contains_name(migration_name) { + println!("downgrade {}...", migration_name); + client.run_downgrade_migration(migration_name, &content) + } else { + Ok(()) + } + }) + .map_err(From::from) + })?; Ok(()) } diff --git a/migra_cli/src/commands/upgrade.rs b/migra_cli/src/commands/upgrade.rs index 73cb6a6..f927e06 100644 --- a/migra_cli/src/commands/upgrade.rs +++ b/migra_cli/src/commands/upgrade.rs @@ -52,23 +52,15 @@ pub(crate) fn upgrade_pending_migrations( }) .collect::, _>>()?; - database::should_run_in_transaction( - &mut client, - opts.transaction_opts.single_transaction, - |client| { - migrations_with_content - .iter() - .try_for_each(|(migration_name, content)| { - println!("upgrade {}...", migration_name); - database::should_run_in_transaction( - client, - !opts.transaction_opts.single_transaction, - |client| client.run_upgrade_migration(migration_name, &content), - ) - }) - .map_err(From::from) - }, - )?; + database::run_in_transaction(&mut client, |client| { + migrations_with_content + .iter() + .try_for_each(|(migration_name, content)| { + println!("upgrade {}...", migration_name); + client.run_upgrade_migration(migration_name, &content) + }) + .map_err(From::from) + })?; Ok(()) } diff --git a/migra_cli/src/config.rs b/migra_cli/src/config.rs index 1b73bed..479c9e4 100644 --- a/migra_cli/src/config.rs +++ b/migra_cli/src/config.rs @@ -50,7 +50,7 @@ fn is_sqlite_database_file(filename: &str) -> bool { .rsplit('.') .next() .map(|ext| ext.eq_ignore_ascii_case("db")) - == Some(true) + .unwrap_or_default() } fn default_database_connection_env() -> String { diff --git a/migra_cli/src/database.rs b/migra_cli/src/database.rs index 6d71e06..883bd7f 100644 --- a/migra_cli/src/database.rs +++ b/migra_cli/src/database.rs @@ -53,18 +53,3 @@ where .and_then(|res| client.commit_transaction().and(Ok(res))) .or_else(|err| client.rollback_transaction().and(Err(err))) } - -pub fn should_run_in_transaction( - client: &mut AnyClient, - is_needed: bool, - trx_fn: TrxFnMut, -) -> migra::Result<()> -where - TrxFnMut: FnOnce(&mut AnyClient) -> migra::Result<()>, -{ - if is_needed { - run_in_transaction(client, trx_fn) - } else { - trx_fn(client) - } -} diff --git a/migra_cli/src/opts.rs b/migra_cli/src/opts.rs index 58a0db7..53b13ed 100644 --- a/migra_cli/src/opts.rs +++ b/migra_cli/src/opts.rs @@ -32,19 +32,10 @@ pub(crate) enum Command { Completions(CompletionsShell), } -#[derive(Debug, StructOpt, Clone)] -pub(crate) struct TransactionOpts { - #[structopt(long = "single-transaction")] - pub single_transaction: bool, -} - #[derive(Debug, StructOpt, Clone)] pub(crate) struct ApplyCommandOpt { #[structopt(parse(from_os_str), required = true)] pub file_paths: Vec, - - #[structopt(flatten)] - pub transaction_opts: TransactionOpts, } #[derive(Debug, StructOpt, Clone)] @@ -64,9 +55,6 @@ pub(crate) struct UpgradeCommandOpt { /// How many existing migrations do we have to update. #[structopt(long = "number", short = "n")] pub migrations_number: Option, - - #[structopt(flatten)] - pub transaction_opts: TransactionOpts, } #[derive(Debug, StructOpt, Clone)] @@ -78,9 +66,6 @@ pub(crate) struct DowngradeCommandOpt { /// Rolls back all applied migrations. Ignores --number option. #[structopt(long = "all")] pub all_migrations: bool, - - #[structopt(flatten)] - pub transaction_opts: TransactionOpts, } #[derive(Debug, StructOpt, Clone)] diff --git a/migra_cli/tests/commands.rs b/migra_cli/tests/commands.rs index ea5ed75..46fa09a 100644 --- a/migra_cli/tests/commands.rs +++ b/migra_cli/tests/commands.rs @@ -485,62 +485,6 @@ mod upgrade { Ok(()) } - #[test] - fn partial_applied_invalid_migrations() -> TestResult { - fn inner(database_name: &'static str, validate: ValidateFn) -> TestResult - where - ValidateFn: Fn() -> TestResult, - { - let manifest_path = database_manifest_path(database_name); - - Command::cargo_bin("migra")? - .arg("-c") - .arg(&manifest_path) - .arg("up") - .assert() - .failure(); - - validate()?; - - Command::cargo_bin("migra")? - .arg("-c") - .arg(&manifest_path) - .arg("down") - .assert() - .success(); - - Ok(()) - } - - #[cfg(feature = "postgres")] - inner("postgres_invalid", || { - let mut conn = client_postgres::Client::connect(POSTGRES_URL, client_postgres::NoTls)?; - let articles_res = conn.query("SELECT a.id FROM articles AS a", &[]); - let persons_res = conn.query("SELECT p.id FROM persons AS p", &[]); - - assert!(articles_res.is_ok()); - assert!(persons_res.is_err()); - - Ok(()) - })?; - - #[cfg(feature = "sqlite")] - remove_sqlite_db().and_then(|_| { - inner("sqlite_invalid", || { - let conn = client_rusqlite::Connection::open(SQLITE_URL)?; - let articles_res = conn.execute_batch("SELECT a.id FROM articles AS a"); - let persons_res = conn.execute_batch("SELECT p.id FROM persons AS p"); - - assert!(articles_res.is_ok()); - assert!(persons_res.is_err()); - - Ok(()) - }) - })?; - - Ok(()) - } - #[test] fn cannot_applied_invalid_migrations_in_single_transaction() -> TestResult { fn inner(database_name: &'static str, validate: ValidateFn) -> TestResult @@ -588,6 +532,8 @@ mod upgrade { }) })?; + // mysql doesn't support DDL in transaction 🤷 + Ok(()) } }