Automatically abort when a migration fails

src/lib.rs

@@ -71,29 +71,43 @@ impl Reshape {
 
         println!(" Applying {} migrations\n", remaining_migrations.len());
 
-        let target_migration = remaining_migrations.last().unwrap().name.to_string();
+        helpers::setup_helpers(&mut self.db, current_migration)?;
 
         let mut new_schema = Schema::new();
-
-        helpers::setup_helpers(&mut self.db, current_migration)?;
+        let mut processed_migrations: &[Migration] = &[];
+        let mut result: anyhow::Result<()> = Ok(());
 
         for (migration_index, migration) in remaining_migrations.iter().enumerate() {
             println!("Migrating '{}':", migration.name);
+            processed_migrations = &remaining_migrations[..migration_index + 1];
 
             for (action_index, action) in migration.actions.iter().enumerate() {
                 print!("  + {} ", action.describe());
 
                 let ctx = Context::new(migration_index, action_index);
-                action.run(&ctx, &mut self.db, &new_schema)?;
-                action.update_schema(&ctx, &mut new_schema);
+                result = action.run(&ctx, &mut self.db, &new_schema);
 
-                println!("{}", "done".green());
+                if result.is_ok() {
+                    action.update_schema(&ctx, &mut new_schema);
+                    println!("{}", "done".green());
+                } else {
+                    println!("{}", "failed".red());
+                    break;
+                }
             }
 
             println!("");
         }
 
+        // If a migration failed, we abort all the migrations that were applied
+        if let Err(err) = result {
+            println!("A migration failed, aborting migrations that have already been applied");
+            abort_migrations(&mut self.db, &processed_migrations)?;
+            return Err(err);
+        }
+
         // Create schema and views for migration
+        let target_migration = remaining_migrations.last().unwrap().name.to_string();
         self.create_schema_for_migration(&target_migration, &new_schema)?;
 
         // Update state once migrations have been performed
@@ -268,15 +282,8 @@ impl Reshape {
             schema_name_for_migration(&target_migration)
         ))?;
 
-        // Abort all pending migrations in reverse order
-        for (migration_index, migration) in remaining_migrations.iter().rev().enumerate() {
-            print!("Aborting'{}' ", migration.name);
-            for (action_index, action) in migration.actions.iter().rev().enumerate() {
-                let ctx = Context::new(migration_index, action_index);
-                action.abort(&ctx, &mut transaction)?;
-            }
-            println!("{}", "done".green());
-        }
+        // Abort all pending migrations
+        abort_migrations(&mut transaction, &remaining_migrations)?;
 
         let keep_count = self.state.migrations.len() - remaining_migrations.len();
         self.state.migrations.truncate(keep_count);
@@ -289,6 +296,19 @@ impl Reshape {
     }
 }
 
+fn abort_migrations(db: &mut dyn Conn, migrations: &[Migration]) -> anyhow::Result<()> {
+    // Abort all migrations in reverse order
+    for (migration_index, migration) in migrations.iter().rev().enumerate() {
+        print!("Aborting '{}' ", migration.name);
+        for (action_index, action) in migration.actions.iter().rev().enumerate() {
+            let ctx = Context::new(migration_index, action_index);
+            action.abort(&ctx, db)?;
+        }
+        println!("{}", "done".green());
+    }
+    Ok(())
+}
+
 pub fn latest_schema_from_migrations(migrations: &[Migration]) -> Option<String> {
     migrations
         .last()

tests/failure.rs

@@ -0,0 +1,81 @@
+use reshape::migrations::{AddColumn, Column, CreateTable, Migration};
+use reshape::Status;
+
+mod common;
+
+#[test]
+fn invalid_migration() {
+    let (mut reshape, mut old_db, mut new_db) = common::setup();
+
+    let create_users_table = Migration::new("create_users_table", None).with_action(CreateTable {
+        name: "users".to_string(),
+        primary_key: vec!["id".to_string()],
+        foreign_keys: vec![],
+        columns: vec![
+            Column {
+                name: "id".to_string(),
+                data_type: "SERIAL".to_string(),
+                nullable: true,
+                default: None,
+                generated: None,
+            },
+            Column {
+                name: "name".to_string(),
+                data_type: "TEXT".to_string(),
+                nullable: false,
+                default: None,
+                generated: None,
+            },
+        ],
+    });
+    let add_first_column = Migration::new("add_first_column", None).with_action(AddColumn {
+        table: "users".to_string(),
+        column: Column {
+            name: "first".to_string(),
+            data_type: "TEXT".to_string(),
+            nullable: false,
+            default: None,
+            generated: None,
+        },
+        up: Some("INVALID SQL".to_string()),
+    });
+
+    let first_migrations = vec![create_users_table.clone()];
+    let second_migrations = vec![create_users_table.clone(), add_first_column.clone()];
+
+    // Run first migration, should automatically finish
+    reshape.migrate(first_migrations.clone()).unwrap();
+    assert!(matches!(reshape.state.status, Status::Idle));
+    assert_eq!(
+        Some(&create_users_table.name),
+        reshape.state.current_migration.as_ref()
+    );
+
+    // Update search paths
+    old_db
+        .simple_query(&reshape::schema_query_for_migration(
+            &first_migrations.last().unwrap().name,
+        ))
+        .unwrap();
+    new_db
+        .simple_query(&reshape::schema_query_for_migration(
+            &first_migrations.last().unwrap().name,
+        ))
+        .unwrap();
+
+    // Insert a test user
+    new_db
+        .simple_query(
+            "
+            INSERT INTO users (id, name) VALUES
+                (1, 'John Doe')
+            ",
+        )
+        .unwrap();
+
+    // Run second migration and ensure it fails
+    assert!(
+        reshape.migrate(second_migrations.clone()).is_err(),
+        "invalid migration succeeded"
+    );
+}