refac: make migrations simpler

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("up.sql").exists() && path.join("down.sql").exists()
-        && path.join(DOWNGRADE_MIGRATION_FILE_NAME).exists()
 }
 
 pub fn get_all_migrations(dir_path: &Path) -> MigraResult<migration::List> {

migra/src/managers.rs

@@ -1,6 +1,5 @@
 use crate::errors::{DbKind, Error, MigraResult, StdResult};
-use crate::migration::{self, Migration};
+use crate::migration;
-use std::path::Path;
 
 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<migration::List>;
 
-    fn get_extended_applied_migrations(&mut self, prefix: &Path) -> MigraResult<migration::List> {
+    fn run_upgrade_migration(&mut self, name: &str, content: &str) -> MigraResult<()> {
-        self.get_applied_migrations()
+        self.apply_sql(content)?;
-            .map(|migrations| migrations.extend_with_path_prefix(prefix))
+        self.insert_migration(name)?;
-    }
-
-    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())?;
-
         Ok(())
     }
 
-    fn apply_downgrade_migration(&mut self, migration: &Migration) -> MigraResult<()> {
+    fn run_downgrade_migration(&mut self, name: &str, content: &str) -> MigraResult<()> {
-        let content = migration.read_downgrade_migration_sql()?;
+        self.apply_sql(content)?;
-
+        self.delete_migration(name)?;
-        self.apply_sql(&content)?;
-        self.delete_migration(migration.name())?;
-
         Ok(())
     }
 }

migra/src/migration.rs

@@ -1,58 +1,24 @@
-use std::fs;
+use crate::errors::MigraResult;
-use std::io;
+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 {
+    pub fn new(name: &str) -> 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 {
         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<String> {
-        fs::read_to_string(self.path.join(UPGRADE_MIGRATION_FILE_NAME))
-    }
-
-    pub fn read_downgrade_migration_sql(&self) -> io::Result<String> {
-        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<Migration>,
 }
 
-impl<T: AsRef<Path>> From<Vec<T>> for List {
+impl<T: AsRef<std::path::Path>> From<Vec<T>> for List {
     fn from(list: Vec<T>) -> 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<P: AsRef<Path>>(&mut self, path: P) {
+    pub fn push_name(&mut self, name: &str) {
-        self.inner.push(Migration::new(path.as_ref()))
+        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 should_run_upgrade_migration(
-    pub fn extend_with_path_prefix(&self, prefix: &Path) -> Self {
+        &mut self,
-        self.inner
+        client: &mut dyn ManageMigrations,
-            .iter()
+        name: &str,
-            .map(|m| m.extend_with_path_prefix(prefix))
+        content: &str,
-            .collect()
+    ) -> MigraResult<bool> {
+        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<bool> {
+        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!(
+        assert_eq!(list.contains(&Migration::new(FIRST_MIGRATION)), true);
-            list.contains(&Migration::new(&PathBuf::from(FIRST_MIGRATION))),
+        assert_eq!(list.contains(&Migration::new(SECOND_MIGRATION)), false);
-            true
-        );
-        assert_eq!(
-            list.contains(&Migration::new(&PathBuf::from(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]);

migra_cli/src/commands/downgrade.rs

@@ -12,9 +12,9 @@ pub(crate) fn rollback_applied_migrations(
 
     client.create_migrations_table()?;
 
-    let applied_migrations =
+    let migrations_dir_path = config.migration_dir_path();
-        client.get_extended_applied_migrations(&config.migration_dir_path())?;
+    let applied_migrations = client.get_applied_migrations()?;
-    let all_migrations = migra::fs::get_all_migrations(&config.migration_dir_path())?;
+    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::<Result<Vec<_>, _>>()?;
 
     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| {
+                .try_for_each(|(migration_name, content)| {
-                    if all_migrations.contains_name(applied_migration.name()) {
+                    if all_migrations.contains_name(migration_name) {
-                        println!("downgrade {}...", applied_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(())

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
+            let applied_migrations = client.get_applied_migrations().unwrap_or_else(|err| {
-                .get_applied_migrations()
+                dbg!(err);
-                .unwrap_or_else(|_| migration::List::new());
+                migration::List::new()
+            });
 
             show_applied_migrations(&applied_migrations);
 

migra_cli/src/commands/upgrade.rs

@@ -12,9 +12,9 @@ pub(crate) fn upgrade_pending_migrations(
 
     client.create_migrations_table()?;
 
-    let applied_migration_names =
+    let migrations_dir_path = config.migration_dir_path();
-        client.get_extended_applied_migrations(&config.migration_dir_path())?;
+    let applied_migration_names = client.get_applied_migrations()?;
-    let all_migrations = migra::fs::get_all_migrations(&config.migration_dir_path())?;
+    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::<Result<Vec<_>, _>>()?;
+
     database::maybe_with_transaction(
         opts.transaction_opts.single_transaction,
         &mut client,
         &mut |mut client| {
-            migrations
+            migrations_with_content
                 .iter()
-                .try_for_each(|migration| {
+                .try_for_each(|(migration_name, content)| {
-                    print_migration_info(migration);
+                    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());
-}