From 687062fd7658effb82625fb439cc12d4678da175 Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Thu, 10 Jun 2021 23:40:30 +0300 Subject: [PATCH] refac: make migrations simpler --- migra/src/fs.rs | 4 +- migra/src/managers.rs | 26 ++---- migra/src/migration.rs | 133 +++++++++++----------------- migra_cli/src/commands/downgrade.rs | 26 ++++-- migra_cli/src/commands/list.rs | 7 +- migra_cli/src/commands/upgrade.rs | 27 +++--- 6 files changed, 95 insertions(+), 128 deletions(-) diff --git a/migra/src/fs.rs b/migra/src/fs.rs index 0992d02..5bfb9e1 100644 --- a/migra/src/fs.rs +++ b/migra/src/fs.rs @@ -1,13 +1,11 @@ use crate::errors::MigraResult; use crate::migration; -use crate::migration::{DOWNGRADE_MIGRATION_FILE_NAME, UPGRADE_MIGRATION_FILE_NAME}; use std::io; use std::path::Path; #[must_use] pub fn is_migration_dir(path: &Path) -> bool { - path.join(UPGRADE_MIGRATION_FILE_NAME).exists() - && path.join(DOWNGRADE_MIGRATION_FILE_NAME).exists() + path.join("up.sql").exists() && path.join("down.sql").exists() } pub fn get_all_migrations(dir_path: &Path) -> MigraResult { diff --git a/migra/src/managers.rs b/migra/src/managers.rs index 5cb1e6a..e05a764 100644 --- a/migra/src/managers.rs +++ b/migra/src/managers.rs @@ -1,6 +1,5 @@ use crate::errors::{DbKind, Error, MigraResult, StdResult}; -use crate::migration::{self, Migration}; -use std::path::Path; +use crate::migration; pub trait BatchExecute { fn batch_execute(&mut self, sql: &str) -> StdResult<()>; @@ -37,26 +36,15 @@ pub trait ManageMigrations: BatchExecute { fn get_applied_migrations(&mut self) -> MigraResult; - fn get_extended_applied_migrations(&mut self, prefix: &Path) -> MigraResult { - self.get_applied_migrations() - .map(|migrations| migrations.extend_with_path_prefix(prefix)) - } - - fn apply_upgrade_migration(&mut self, migration: &Migration) -> MigraResult<()> { - let content = migration.read_upgrade_migration_sql()?; - - self.apply_sql(&content)?; - self.insert_migration(migration.name())?; - + fn run_upgrade_migration(&mut self, name: &str, content: &str) -> MigraResult<()> { + self.apply_sql(content)?; + self.insert_migration(name)?; Ok(()) } - fn apply_downgrade_migration(&mut self, migration: &Migration) -> MigraResult<()> { - let content = migration.read_downgrade_migration_sql()?; - - self.apply_sql(&content)?; - self.delete_migration(migration.name())?; - + fn run_downgrade_migration(&mut self, name: &str, content: &str) -> MigraResult<()> { + self.apply_sql(content)?; + self.delete_migration(name)?; Ok(()) } } diff --git a/migra/src/migration.rs b/migra/src/migration.rs index bdd9db9..7234477 100644 --- a/migra/src/migration.rs +++ b/migra/src/migration.rs @@ -1,58 +1,24 @@ -use std::fs; -use std::io; +use crate::errors::MigraResult; +use crate::managers::ManageMigrations; use std::iter::FromIterator; -use std::path::{Path, PathBuf}; - -pub(crate) const UPGRADE_MIGRATION_FILE_NAME: &str = "up.sql"; -pub(crate) const DOWNGRADE_MIGRATION_FILE_NAME: &str = "down.sql"; #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Migration { - path: PathBuf, name: String, } impl Migration { #[must_use] - pub fn new(path: &Path) -> Self { - Migration::with_name( - path, - path.file_name() - .and_then(std::ffi::OsStr::to_str) - .expect("Cannot read migration name"), - ) - } - - #[must_use] - pub fn with_name(path: &Path, name: &str) -> Self { + pub fn new(name: &str) -> Self { Migration { - path: PathBuf::from(path), name: name.to_owned(), } } - #[must_use] - pub fn path(&self) -> &Path { - &self.path - } - #[must_use] pub fn name(&self) -> &String { &self.name } - - #[must_use] - pub fn extend_with_path_prefix(&self, prefix: &Path) -> Self { - Migration::with_name(&prefix.join(self.path()), self.name()) - } - - pub fn read_upgrade_migration_sql(&self) -> io::Result { - fs::read_to_string(self.path.join(UPGRADE_MIGRATION_FILE_NAME)) - } - - pub fn read_downgrade_migration_sql(&self) -> io::Result { - fs::read_to_string(self.path.join(DOWNGRADE_MIGRATION_FILE_NAME)) - } } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -60,10 +26,19 @@ pub struct List { inner: Vec, } -impl> From> for List { +impl> From> for List { fn from(list: Vec) -> Self { List { - inner: list.iter().map(AsRef::as_ref).map(Migration::new).collect(), + inner: list + .iter() + .map(AsRef::as_ref) + .map(|path| { + path.file_name() + .and_then(std::ffi::OsStr::to_str) + .expect("Cannot read migration name") + }) + .map(Migration::new) + .collect(), } } } @@ -116,8 +91,8 @@ impl List { self.inner.push(migration) } - pub fn push_name>(&mut self, path: P) { - self.inner.push(Migration::new(path.as_ref())) + pub fn push_name(&mut self, name: &str) { + self.inner.push(Migration::new(name)) } #[must_use] @@ -132,24 +107,6 @@ impl List { self.inner.iter().any(|migration| migration.name() == name) } - #[must_use] - pub fn maybe_contains<'a>(&self, name: &'a str) -> Option<&'a str> { - if self.contains_name(name) { - Some(name) - } else { - None - } - } - - #[must_use] - pub fn maybe_missed<'a>(&self, name: &'a str) -> Option<&'a str> { - if self.contains_name(name) { - None - } else { - Some(name) - } - } - #[must_use] pub fn exclude(&self, list: &List) -> List { self.inner @@ -158,12 +115,36 @@ impl List { .collect() } - #[must_use] - pub fn extend_with_path_prefix(&self, prefix: &Path) -> Self { - self.inner - .iter() - .map(|m| m.extend_with_path_prefix(prefix)) - .collect() + pub fn should_run_upgrade_migration( + &mut self, + client: &mut dyn ManageMigrations, + name: &str, + content: &str, + ) -> MigraResult { + let is_missed = !self.contains_name(name); + + if is_missed { + client.run_upgrade_migration(name, content)?; + self.push_name(name); + } + + Ok(is_missed) + } + + pub fn should_run_downgrade_migration( + &mut self, + client: &mut dyn ManageMigrations, + name: &str, + content: &str, + ) -> MigraResult { + let is_latest = self.inner.last() == Some(&Migration::new(name)); + + if is_latest { + client.run_downgrade_migration(name, content)?; + self.inner.pop(); + } + + Ok(is_latest) } } @@ -178,10 +159,10 @@ mod tests { fn push_migration_to_list() { let mut list = List::new(); - list.push(Migration::new(&PathBuf::from(FIRST_MIGRATION))); + list.push(Migration::new(FIRST_MIGRATION)); assert_eq!(list, List::from(vec![FIRST_MIGRATION])); - list.push(Migration::new(&PathBuf::from(SECOND_MIGRATION))); + list.push(Migration::new(SECOND_MIGRATION)); assert_eq!(list, List::from(vec![FIRST_MIGRATION, SECOND_MIGRATION])) } @@ -200,14 +181,8 @@ mod tests { fn contains_migration() { let list = List::from(vec![FIRST_MIGRATION]); - assert_eq!( - list.contains(&Migration::new(&PathBuf::from(FIRST_MIGRATION))), - true - ); - assert_eq!( - list.contains(&Migration::new(&PathBuf::from(SECOND_MIGRATION))), - false - ); + assert_eq!(list.contains(&Migration::new(FIRST_MIGRATION)), true); + assert_eq!(list.contains(&Migration::new(SECOND_MIGRATION)), false); } #[test] @@ -218,14 +193,6 @@ mod tests { assert_eq!(list.contains_name(SECOND_MIGRATION), false); } - #[test] - fn maybe_next_migration_name() { - let list = List::from(vec![FIRST_MIGRATION]); - - assert_eq!(list.maybe_missed(FIRST_MIGRATION), None); - assert_eq!(list.maybe_missed(SECOND_MIGRATION), Some(SECOND_MIGRATION)); - } - #[test] fn create_excluded_migration_list() { let all_migrations = List::from(vec![FIRST_MIGRATION, SECOND_MIGRATION]); diff --git a/migra_cli/src/commands/downgrade.rs b/migra_cli/src/commands/downgrade.rs index 5630dfc..039d2a5 100644 --- a/migra_cli/src/commands/downgrade.rs +++ b/migra_cli/src/commands/downgrade.rs @@ -12,9 +12,9 @@ pub(crate) fn rollback_applied_migrations( client.create_migrations_table()?; - let applied_migrations = - client.get_extended_applied_migrations(&config.migration_dir_path())?; - let all_migrations = migra::fs::get_all_migrations(&config.migration_dir_path())?; + let migrations_dir_path = config.migration_dir_path(); + let applied_migrations = client.get_applied_migrations()?; + let all_migrations = migra::fs::get_all_migrations(&migrations_dir_path)?; let rollback_migrations_number = if opts.all_migrations { applied_migrations.len() @@ -22,21 +22,29 @@ pub(crate) fn rollback_applied_migrations( cmp::min(opts.migrations_number, applied_migrations.len()) }; - dbg!(&rollback_migrations_number); + let migrations = applied_migrations[..rollback_migrations_number].to_vec(); + let migrations_with_content = migrations + .iter() + .map(|migration| { + let migration_name = migration.name(); + let migration_file_path = migrations_dir_path.join(migration_name).join("down.sql"); + std::fs::read_to_string(migration_file_path).map(|content| (migration_name, content)) + }) + .collect::, _>>()?; database::maybe_with_transaction( opts.transaction_opts.single_transaction, &mut client, &mut |mut client| { - applied_migrations[..rollback_migrations_number] + migrations_with_content .iter() - .try_for_each(|applied_migration| { - if all_migrations.contains_name(applied_migration.name()) { - println!("downgrade {}...", applied_migration.name()); + .try_for_each(|(migration_name, content)| { + if all_migrations.contains_name(migration_name) { + println!("downgrade {}...", migration_name); database::maybe_with_transaction( !opts.transaction_opts.single_transaction, &mut client, - &mut |client| client.apply_downgrade_migration(&applied_migration), + &mut |client| client.run_downgrade_migration(migration_name, &content), ) } else { Ok(()) diff --git a/migra_cli/src/commands/list.rs b/migra_cli/src/commands/list.rs index 8858b3b..11bfdcd 100644 --- a/migra_cli/src/commands/list.rs +++ b/migra_cli/src/commands/list.rs @@ -14,9 +14,10 @@ pub(crate) fn print_migration_lists(app: &App) -> migra::StdResult<()> { database_connection_string, &config.migrations.table_name(), )?; - let applied_migrations = client - .get_applied_migrations() - .unwrap_or_else(|_| migration::List::new()); + let applied_migrations = client.get_applied_migrations().unwrap_or_else(|err| { + dbg!(err); + migration::List::new() + }); show_applied_migrations(&applied_migrations); diff --git a/migra_cli/src/commands/upgrade.rs b/migra_cli/src/commands/upgrade.rs index a6c01e4..92a55d8 100644 --- a/migra_cli/src/commands/upgrade.rs +++ b/migra_cli/src/commands/upgrade.rs @@ -12,9 +12,9 @@ pub(crate) fn upgrade_pending_migrations( client.create_migrations_table()?; - let applied_migration_names = - client.get_extended_applied_migrations(&config.migration_dir_path())?; - let all_migrations = migra::fs::get_all_migrations(&config.migration_dir_path())?; + let migrations_dir_path = config.migration_dir_path(); + let applied_migration_names = client.get_applied_migrations()?; + let all_migrations = migra::fs::get_all_migrations(&migrations_dir_path)?; let pending_migrations = migra::fs::filter_pending_migrations(&all_migrations, &applied_migration_names); @@ -44,18 +44,27 @@ pub(crate) fn upgrade_pending_migrations( .into() }; + let migrations_with_content = migrations + .iter() + .map(|migration| { + let migration_name = migration.name(); + let migration_file_path = migrations_dir_path.join(migration_name).join("up.sql"); + std::fs::read_to_string(migration_file_path).map(|content| (migration_name, content)) + }) + .collect::, _>>()?; + database::maybe_with_transaction( opts.transaction_opts.single_transaction, &mut client, &mut |mut client| { - migrations + migrations_with_content .iter() - .try_for_each(|migration| { - print_migration_info(migration); + .try_for_each(|(migration_name, content)| { + println!("upgrade {}...", migration_name); database::maybe_with_transaction( !opts.transaction_opts.single_transaction, &mut client, - &mut |client| client.apply_upgrade_migration(migration), + &mut |client| client.run_upgrade_migration(migration_name, &content), ) }) .map_err(From::from) @@ -64,7 +73,3 @@ pub(crate) fn upgrade_pending_migrations( Ok(()) } - -fn print_migration_info(migration: &migra::Migration) { - println!("upgrade {}...", migration.name()); -}