diff --git a/src/cli/generate/mod.rs b/src/cli/generate/mod.rs index 3b5beb1..f23c556 100644 --- a/src/cli/generate/mod.rs +++ b/src/cli/generate/mod.rs @@ -13,7 +13,9 @@ use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::{BreakingChange, MitigationStrategy, analyze_breaking_changes}; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::pglite::SchemaLoader; use crate::db::state::{Migration, StateBackend, StateHash}; @@ -70,6 +72,8 @@ pub struct GenerateOutput { /// Breaking changes with details. #[serde(skip_serializing_if = "Vec::is_empty")] pub breaking_changes: Vec, + /// Whether the migration can be reversed with `down`. + pub is_reversible: bool, } /// Breaking change output for JSON serialization. @@ -117,6 +121,11 @@ impl std::fmt::Display for GenerateOutput { " To state: {}", &self.target_state_hash[..16.min(self.target_state_hash.len())] )?; + writeln!( + f, + " Reversible: {}", + if self.is_reversible { "yes" } else { "no" } + )?; if self.has_breaking_changes { writeln!(f)?; @@ -127,6 +136,12 @@ impl std::fmt::Display for GenerateOutput { } } + if !self.is_reversible { + writeln!(f)?; + writeln!(f, "NOTE: This migration contains irreversible operations.")?; + writeln!(f, " Running 'tern down' will not be possible.")?; + } + if self.dry_run { writeln!(f)?; writeln!(f, "This was a dry run. No migration was recorded.")?; @@ -195,6 +210,7 @@ impl Generate { target_state_hash: target_hash.to_hex(), dry_run: self.dry_run, breaking_changes: vec![], + is_reversible: true, // Empty migration is trivially reversible }; print_json(&output); } @@ -232,10 +248,15 @@ impl Generate { // Get the breaking changes for the migration let breaking_changes = analysis.into_changes(); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create the migration let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.clone(), @@ -250,13 +271,14 @@ impl Generate { let output = GenerateOutput { migration_id: migration.id.to_hex(), description: self.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), has_breaking_changes: !breaking_changes.is_empty(), has_destructive_changes: destructive_count > 0, source_state_hash: source_hash.to_hex(), target_state_hash: target_hash.to_hex(), dry_run: self.dry_run, breaking_changes: breaking_change_outputs, + is_reversible: migration.is_reversible(), }; // Record the migration (unless dry run) @@ -308,6 +330,7 @@ mod tests { operation_count: 3, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "0".repeat(64), target_state_hash: "1".repeat(64), dry_run: false, @@ -328,6 +351,7 @@ mod tests { operation_count: 1, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "0".repeat(64), target_state_hash: "1".repeat(64), dry_run: true, @@ -346,6 +370,7 @@ mod tests { operation_count: 1, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "src".to_string(), target_state_hash: "tgt".to_string(), dry_run: false, diff --git a/src/cli/history/mod.rs b/src/cli/history/mod.rs index 515ac23..364f029 100644 --- a/src/cli/history/mod.rs +++ b/src/cli/history/mod.rs @@ -230,6 +230,7 @@ mod tests { let m = Migration::new( "Second migration", vec![], + vec![], current_hash, crate::db::state::StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/cli/import/mod.rs b/src/cli/import/mod.rs index e32d1b4..c67a831 100644 --- a/src/cli/import/mod.rs +++ b/src/cli/import/mod.rs @@ -12,7 +12,9 @@ use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::analyze_breaking_changes; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::query::{PostgresCatalog, load_namespace}; use crate::db::state::{Migration, StateBackend, StateHash}; use crate::db::{self}; @@ -256,9 +258,14 @@ impl Import { let source_hash = StateHash::from_namespace(&expected_schema); let target_hash = StateHash::from_namespace(&live_schema); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes, @@ -280,7 +287,7 @@ impl Import { has_changes: true, migration_id, description: self.description, - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), dry_run: self.dry_run, summary, }; diff --git a/src/cli/inspect/mod.rs b/src/cli/inspect/mod.rs index d10f2b5..0648dc5 100644 --- a/src/cli/inspect/mod.rs +++ b/src/cli/inspect/mod.rs @@ -184,10 +184,10 @@ fn inspect_json_migration(path: &PathBuf) -> miette::Result { .into_diagnostic() .wrap_err("Failed to parse migration JSON")?; - // Try to generate SQL from operations - let sql_statements = if !migration.operations.is_empty() { + // Try to generate SQL from up_operations + let sql_statements = if !migration.up_operations.is_empty() { use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let renderer = PostgresRenderer::new(RenderConfig::default()); let script = plan.render(&renderer); Some( diff --git a/src/cli/migrate/down.rs b/src/cli/migrate/down.rs new file mode 100644 index 0000000..f1dbbcf --- /dev/null +++ b/src/cli/migrate/down.rs @@ -0,0 +1,437 @@ +//! Migrate down command. +//! +//! This command reverts the most recently applied migration using the +//! pre-computed down_operations stored in the migration file. + +use std::path::PathBuf; + +use anstream::println; +use clap::Args; +use miette::{Context, IntoDiagnostic}; +use serde::Serialize; + +use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; +use crate::db::execution::MigrationTracker; +use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::state::{Migration, StateBackend}; +use crate::db::{self}; + +/// Revert the most recently applied migration. +/// +/// Connects to a database and reverts the last applied migration. +/// Only one migration is reverted at a time for safety. +#[derive(Debug, Clone, Args)] +pub struct Down { + /// PostgreSQL connection string + #[arg(env = "DATABASE_URL")] + pub url: Option, + + /// PostgreSQL connection string (alternative to positional) + #[arg(long, env = "DATABASE_URL")] + pub database_url: Option, + + /// The database schema to migrate + #[arg(long, default_value = "public")] + pub schema: String, + + /// Path to the state directory + #[arg(long)] + pub path: Option, + + /// Show what would be reverted without actually reverting + #[arg(long)] + pub dry_run: bool, + + /// Output format + #[arg(long, default_value = "text")] + pub format: OutputFormat, +} + +/// Output structure for down command JSON format. +#[derive(Debug, Serialize)] +pub struct DownOutput { + /// Whether the operation was successful. + pub success: bool, + /// Whether this was a dry run. + pub dry_run: bool, + /// The migration that was reverted (or would be reverted). + #[serde(skip_serializing_if = "Option::is_none")] + pub migration: Option, + /// Error message if failed. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Information about a reverted migration. +#[derive(Debug, Serialize)] +pub struct RevertedMigrationInfo { + /// Migration ID (hex). + pub id: String, + /// Migration description. + pub description: String, + /// Number of up operations in the migration. + pub operation_count: usize, + /// Number of SQL statements executed to revert. + #[serde(skip_serializing_if = "Option::is_none")] + pub statement_count: Option, +} + +impl std::fmt::Display for DownOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(migration) = &self.migration { + if self.dry_run { + writeln!(f, "Would revert migration (dry run):")?; + writeln!(f)?; + writeln!( + f, + " [{}...] {}", + &migration.id[..12.min(migration.id.len())], + migration.description + )?; + writeln!(f, " Operations to revert: {}", migration.operation_count)?; + writeln!(f)?; + writeln!(f, "Run without --dry-run to revert this migration.")?; + } else if self.success { + writeln!(f, "Reverted migration:")?; + writeln!(f)?; + writeln!( + f, + " [{}...] {}", + &migration.id[..12.min(migration.id.len())], + migration.description + )?; + if let Some(stmt_count) = migration.statement_count { + writeln!(f, " SQL statements executed: {}", stmt_count)?; + } + writeln!(f)?; + writeln!(f, "Migration reverted successfully.")?; + } else { + writeln!(f, "Migration revert failed!")?; + writeln!(f)?; + if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } + } + } else if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } else { + writeln!(f, "No migrations to revert.")?; + } + + Ok(()) + } +} + +impl Down { + /// Dispatch the down command. + pub async fn dispatch(self) -> miette::Result<()> { + // Resolve database URL (positional > --database-url > env) + let db_url = self + .url + .or(self.database_url) + .ok_or_else(|| miette::miette!("Database URL required. Provide as argument, --database-url, or set DATABASE_URL environment variable."))?; + + let backend = load_backend(self.path.as_deref()); + ensure_backend_initialized(&backend).await?; + + if !matches!(self.format, OutputFormat::Json) { + println!("Connecting to database..."); + } + + // Connect to the database + let client = db::connect(&db_url) + .await + .into_diagnostic() + .wrap_err("Failed to connect to database")?; + + // Create tracker + let tracker = MigrationTracker::new(&client, &self.schema); + + // Ensure tracking infrastructure exists + tracker.ensure_schema().await.into_diagnostic()?; + + // Get the current migration ID from the database + let current_id = match tracker.get_current_migration_id().await.into_diagnostic()? { + Some(id) => id, + None => { + let output = DownOutput { + success: false, + dry_run: self.dry_run, + migration: None, + error: Some("No migrations have been applied to revert.".to_string()), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- No migrations to revert"), + } + std::process::exit(1); + } + }; + + // Find the migration in the local backend + let migration = find_migration_by_hex_id(&backend, ¤t_id) + .await + .into_diagnostic() + .wrap_err("Failed to find migration in local state")?; + + let migration = match migration { + Some(m) => m, + None => { + return Err(miette::miette!( + "Migration {} is recorded in the database but not found in local state.\n\nThis may indicate state corruption or that migrations were applied from a different source.", + current_id + )); + } + }; + + // Check if this is a baseline migration + if migration.is_baseline() { + return Err(miette::miette!( + "Cannot revert the baseline migration.\n\nThe baseline migration represents the initial database state and cannot be reverted." + )); + } + + // Check if the migration is reversible (has pre-computed down_operations) + if !migration.is_reversible() { + let output = DownOutput { + success: false, + dry_run: self.dry_run, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.up_operations.len(), + statement_count: None, + }), + error: Some( + "This migration contains irreversible operations and cannot be reverted." + .to_string(), + ), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- Migration is not reversible"), + } + std::process::exit(1); + } + + // Use the pre-computed down_operations + let down_ops = migration.down_operations.clone(); + + if self.dry_run { + // Just show what would be reverted + let output = DownOutput { + success: true, + dry_run: true, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.up_operations.len(), + statement_count: None, + }), + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!( + "-- Dry run: would revert migration {}", + migration.id.to_short_hex() + ); + // Render the down operations to show what SQL would be executed + let renderer = PostgresRenderer::new(RenderConfig::default()); + let plan = MigrationPlan::from_operations(down_ops); + let script = plan.render(&renderer); + println!("{}", script.to_sql()); + } + } + } else { + // Execute the revert + if !matches!(self.format, OutputFormat::Json) { + println!("Reverting migration {}...", migration.id.to_short_hex()); + } + + // Render migration to SQL + let renderer = PostgresRenderer::new(RenderConfig::default()); + let plan = MigrationPlan::from_operations(down_ops); + let script = plan.render(&renderer); + let sql = script.to_sql(); + let statement_count = script.all_statements().len(); + + // Begin transaction + client + .execute("BEGIN", &[]) + .await + .into_diagnostic() + .wrap_err("Failed to begin transaction")?; + + // Set search path to target schema + let set_search_path = format!("SET search_path TO {}", self.schema); + if let Err(e) = client.batch_execute(&set_search_path).await { + let _ = client.execute("ROLLBACK", &[]).await; + return Err(miette::miette!("Failed to set search_path: {}", e)); + } + + // Execute the down operations + if !sql.is_empty() + && let Err(e) = client.batch_execute(&sql).await + { + let _ = client.execute("ROLLBACK", &[]).await; + let output = DownOutput { + success: false, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.up_operations.len(), + statement_count: Some(statement_count), + }), + error: Some(e.to_string()), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- Revert failed: {}", e), + } + std::process::exit(1); + } + + // Remove migration record from tracking tables + if let Err(e) = tracker.unrecord_migration().await { + let _ = client.execute("ROLLBACK", &[]).await; + return Err(miette::miette!("Failed to unrecord migration: {}", e)); + } + + // Commit transaction + client + .execute("COMMIT", &[]) + .await + .into_diagnostic() + .wrap_err("Failed to commit transaction")?; + + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.up_operations.len(), + statement_count: Some(statement_count), + }), + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!("-- Migration reverted successfully"); + } + } + } + + Ok(()) + } +} + +/// Find a migration by its hex ID string. +async fn find_migration_by_hex_id( + backend: &crate::db::state::LocalFileBackend, + hex_id: &str, +) -> Result, crate::db::state::StateError> { + let all_migrations = backend.get_all_migrations().await?; + Ok(all_migrations.into_iter().find(|m| m.id.to_hex() == hex_id)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn down_output_no_migrations_display() { + let output = DownOutput { + success: false, + dry_run: false, + migration: None, + error: Some("No migrations to revert".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("No migrations to revert")); + } + + #[test] + fn down_output_dry_run_display() { + let output = DownOutput { + success: true, + dry_run: true, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: None, + }), + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Would revert migration (dry run)")); + assert!(display.contains("Add users table")); + assert!(display.contains("Run without --dry-run")); + } + + #[test] + fn down_output_success_display() { + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: Some(5), + }), + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Reverted migration")); + assert!(display.contains("Migration reverted successfully")); + } + + #[test] + fn down_output_failure_display() { + let output = DownOutput { + success: false, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: None, + }), + error: Some("relation 'users' does not exist".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("Migration revert failed")); + assert!(display.contains("relation 'users' does not exist")); + } + + #[test] + fn down_output_serializes() { + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc".to_string(), + description: "Test".to_string(), + operation_count: 1, + statement_count: Some(2), + }), + error: None, + }; + let json = serde_json::to_string(&output).unwrap(); + assert!(json.contains("\"success\":true")); + assert!(json.contains("\"operation_count\":1")); + } +} diff --git a/src/cli/migrate/mod.rs b/src/cli/migrate/mod.rs new file mode 100644 index 0000000..18e86d0 --- /dev/null +++ b/src/cli/migrate/mod.rs @@ -0,0 +1,38 @@ +//! Migrate command with up/down subcommands. +//! +//! This module provides the `tern migrate` command which offers precise control +//! over database migrations. + +pub mod down; +pub mod up; + +pub use down::Down; +pub use up::Up; + +use clap::Subcommand; + +/// Migrate subcommands for precise migration control. +#[derive(Debug, Subcommand, Clone)] +pub enum MigrateAction { + /// Run pending migrations against a database + /// + /// Connects to a database and applies all migrations that haven't been + /// applied yet. Each migration runs in its own transaction. + Up(Up), + + /// Revert the most recently applied migration + /// + /// Connects to a database and reverts the last applied migration. + /// Only one migration is reverted at a time for safety. + Down(Down), +} + +impl MigrateAction { + /// Dispatch migrate subcommands. + pub async fn dispatch(self) -> miette::Result<()> { + match self { + MigrateAction::Up(args) => args.dispatch().await, + MigrateAction::Down(args) => args.dispatch().await, + } + } +} diff --git a/src/cli/migrate/up.rs b/src/cli/migrate/up.rs new file mode 100644 index 0000000..5e16b6c --- /dev/null +++ b/src/cli/migrate/up.rs @@ -0,0 +1,387 @@ +//! Migrate up command. +//! +//! This command runs pending migrations against a live database. + +use std::path::PathBuf; + +use anstream::println; +use clap::Args; +use miette::{Context, IntoDiagnostic}; +use serde::Serialize; + +use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; +use crate::db::execution::MigrationExecutor; +use crate::db::state::StateBackend; +use crate::db::{self}; + +/// Run pending migrations against a live database. +/// +/// Connects to a database and applies all migrations that haven't been +/// applied yet. Each migration runs in its own transaction. +#[derive(Debug, Clone, Args)] +pub struct Up { + /// PostgreSQL connection string + #[arg(env = "DATABASE_URL")] + pub url: Option, + + /// PostgreSQL connection string (alternative to positional) + #[arg(long, env = "DATABASE_URL")] + pub database_url: Option, + + /// The database schema to migrate + #[arg(long, default_value = "public")] + pub schema: String, + + /// Path to the state directory + #[arg(long)] + pub path: Option, + + /// Show pending migrations without applying + #[arg(long)] + pub dry_run: bool, + + /// Output format + #[arg(long, default_value = "text")] + pub format: OutputFormat, +} + +/// Output structure for up command JSON format. +#[derive(Debug, Serialize)] +pub struct UpOutput { + /// Whether the operation was successful. + pub success: bool, + /// Whether this was a dry run. + pub dry_run: bool, + /// Number of migrations applied or pending. + pub migration_count: usize, + /// Details of each migration. + pub migrations: Vec, + /// Error message if failed. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Information about a single migration. +#[derive(Debug, Serialize)] +pub struct MigrationInfo { + /// Migration ID (hex). + pub id: String, + /// Migration description. + pub description: String, + /// Number of SQL statements (if applied). + #[serde(skip_serializing_if = "Option::is_none")] + pub statement_count: Option, + /// Schema hash after applying (if applied). + #[serde(skip_serializing_if = "Option::is_none")] + pub schema_hash: Option, + /// Whether this migration was applied. + pub applied: bool, +} + +impl std::fmt::Display for UpOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.migration_count == 0 { + writeln!(f, "Database is up to date. No pending migrations.")?; + return Ok(()); + } + + if self.dry_run { + writeln!(f, "Pending migrations (dry run):")?; + writeln!(f)?; + for (i, m) in self.migrations.iter().enumerate() { + writeln!( + f, + " {}. [{}...] {}", + i + 1, + &m.id[..12.min(m.id.len())], + m.description + )?; + } + writeln!(f)?; + writeln!(f, "Run without --dry-run to apply these migrations.")?; + } else if self.success { + writeln!(f, "Applying {} pending migration(s):", self.migration_count)?; + writeln!(f)?; + for (i, m) in self.migrations.iter().enumerate() { + let status = if m.applied { "OK" } else { "SKIPPED" }; + // Pad description to align status + let desc = if m.description.len() > 40 { + format!("{}...", &m.description[..37]) + } else { + m.description.clone() + }; + writeln!( + f, + " [{}/{}] {:.<50} {}", + i + 1, + self.migration_count, + desc, + status + )?; + } + writeln!(f)?; + writeln!(f, "All migrations applied successfully.")?; + } else { + writeln!(f, "Migration failed!")?; + writeln!(f)?; + if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } + writeln!(f)?; + writeln!( + f, + "Applied {} of {} migration(s) before failure.", + self.migrations.iter().filter(|m| m.applied).count(), + self.migration_count + )?; + } + + Ok(()) + } +} + +impl Up { + /// Dispatch the up command. + pub async fn dispatch(self) -> miette::Result<()> { + // Resolve database URL (positional > --database-url > env) + let db_url = self + .url + .or(self.database_url) + .ok_or_else(|| miette::miette!("Database URL required. Provide as argument, --database-url, or set DATABASE_URL environment variable."))?; + + let backend = load_backend(self.path.as_deref()); + ensure_backend_initialized(&backend).await?; + + // Check that we have migrations to apply + let index = backend.get_migration_index().await.into_diagnostic()?; + if index.is_empty() { + return Err(miette::miette!( + "No migrations found in state backend.\n\nRun 'tern generate' to create a migration first." + )); + } + + if !matches!(self.format, OutputFormat::Json) { + println!("Connecting to database..."); + } + + // Connect to the database + let client = db::connect(&db_url) + .await + .into_diagnostic() + .wrap_err("Failed to connect to database")?; + + // Create executor + let executor = MigrationExecutor::new(&client, &backend, &self.schema); + + if self.dry_run { + // Just show pending migrations + if !matches!(self.format, OutputFormat::Json) { + println!("Checking migration status..."); + } + + let pending = executor + .get_pending() + .await + .into_diagnostic() + .wrap_err("Failed to get pending migrations")?; + + let migrations: Vec = pending + .iter() + .map(|m| MigrationInfo { + id: m.id.to_hex(), + description: m.description.clone(), + statement_count: None, + schema_hash: None, + applied: false, + }) + .collect(); + + let output = UpOutput { + success: true, + dry_run: true, + migration_count: migrations.len(), + migrations, + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!("-- Dry run: showing pending migrations"); + for m in pending { + println!("-- Migration: {} - {}", m.id.to_short_hex(), m.description); + } + } + } + } else { + // Execute pending migrations + if !matches!(self.format, OutputFormat::Json) { + println!("Checking migration status..."); + } + + let result = executor + .execute_pending() + .await + .into_diagnostic() + .wrap_err("Failed to execute migrations")?; + + let migrations: Vec = result + .applied + .iter() + .map(|r| MigrationInfo { + id: r.migration_id.to_hex(), + description: r.description.clone(), + statement_count: Some(r.statement_count), + schema_hash: Some(r.schema_hash.clone()), + applied: true, + }) + .collect(); + + let output = UpOutput { + success: result.success, + dry_run: false, + migration_count: migrations.len(), + migrations, + error: result.error.clone(), + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + if result.success { + println!("-- All migrations applied successfully"); + } else if let Some(err) = &result.error { + println!("-- Migration failed: {}", err); + } + } + } + + // Exit with error if migrations failed + if !result.success { + std::process::exit(1); + } + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn up_output_no_migrations_display() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 0, + migrations: vec![], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Database is up to date")); + } + + #[test] + fn up_output_dry_run_display() { + let output = UpOutput { + success: true, + dry_run: true, + migration_count: 2, + migrations: vec![ + MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: None, + schema_hash: None, + applied: false, + }, + MigrationInfo { + id: "789xyz123456".to_string(), + description: "Add orders table".to_string(), + statement_count: None, + schema_hash: None, + applied: false, + }, + ], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Pending migrations (dry run)")); + assert!(display.contains("Add users table")); + assert!(display.contains("Add orders table")); + assert!(display.contains("Run without --dry-run")); + } + + #[test] + fn up_output_success_display() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 2, + migrations: vec![ + MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: Some(3), + schema_hash: Some("hash1".to_string()), + applied: true, + }, + MigrationInfo { + id: "789xyz123456".to_string(), + description: "Add orders table".to_string(), + statement_count: Some(2), + schema_hash: Some("hash2".to_string()), + applied: true, + }, + ], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Applying 2 pending migration(s)")); + assert!(display.contains("All migrations applied successfully")); + } + + #[test] + fn up_output_failure_display() { + let output = UpOutput { + success: false, + dry_run: false, + migration_count: 2, + migrations: vec![MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: Some(3), + schema_hash: Some("hash1".to_string()), + applied: true, + }], + error: Some("relation 'foo' does not exist".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("Migration failed")); + assert!(display.contains("relation 'foo' does not exist")); + } + + #[test] + fn up_output_serializes() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 1, + migrations: vec![MigrationInfo { + id: "abc".to_string(), + description: "Test".to_string(), + statement_count: Some(1), + schema_hash: Some("hash".to_string()), + applied: true, + }], + error: None, + }; + let json = serde_json::to_string(&output).unwrap(); + assert!(json.contains("\"success\":true")); + assert!(json.contains("\"migration_count\":1")); + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 39536bf..4b1fb66 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -12,6 +12,7 @@ pub mod history; pub mod import; pub mod init; pub mod inspect; +pub mod migrate; pub mod print_migrations; pub mod record; pub mod schema; @@ -162,11 +163,23 @@ pub enum CliCommand { /// and generates a migration that would transform the schema. Generate(generate::Generate), - /// Run pending migrations against a database + /// Migration commands (up/down) + /// + /// Commands for running and reverting database migrations. + #[command(subcommand)] + Migrate(migrate::MigrateAction), + + /// Run pending migrations against a database (alias for 'migrate up') /// /// Connects to a database and applies all migrations that haven't been /// applied yet. Each migration runs in its own transaction. - Up(up::Up), + Up(migrate::Up), + + /// Revert the most recently applied migration (alias for 'migrate down') + /// + /// Connects to a database and reverts the last applied migration. + /// Only one migration is reverted at a time for safety. + Down(migrate::Down), /// List migration history /// @@ -289,7 +302,9 @@ impl CliCommand { CliCommand::Check(action) => action.dispatch().await, CliCommand::Import(args) => args.dispatch().await, CliCommand::Generate(args) => args.dispatch().await, + CliCommand::Migrate(action) => action.dispatch().await, CliCommand::Up(args) => args.dispatch().await, + CliCommand::Down(args) => args.dispatch().await, CliCommand::History(args) => args.dispatch().await, CliCommand::Show(args) => args.dispatch().await, CliCommand::Verify(args) => args.dispatch().await, diff --git a/src/cli/record/mod.rs b/src/cli/record/mod.rs index 9f694b1..fbe4d0f 100644 --- a/src/cli/record/mod.rs +++ b/src/cli/record/mod.rs @@ -95,10 +95,10 @@ impl Record { let new_state = if let Some(ref state) = migration.checkpoint_state { state.clone() } else { - // We need to apply the operations to the current state + // We need to apply the up_operations to the current state let current_state = backend.get_current_state().await.into_diagnostic()?; current_state - .apply(&migration.operations) + .apply(&migration.up_operations) .into_diagnostic() .wrap_err("Failed to apply migration operations")? }; @@ -205,7 +205,15 @@ pub fn create_sync_migration( parent_hash: StateHash, resulting_hash: StateHash, ) -> Migration { - Migration::new(description, vec![], parent_hash, resulting_hash, vec![]) + // Sync migrations have no operations, so they're trivially reversible + Migration::new( + description, + vec![], + vec![], + parent_hash, + resulting_hash, + vec![], + ) } #[cfg(test)] @@ -261,6 +269,7 @@ mod tests { let migration = Migration::new( "Test migration", vec![], + vec![], current_hash, StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/cli/schema/migrate/mod.rs b/src/cli/schema/migrate/mod.rs index 1d3caed..ec6b207 100644 --- a/src/cli/schema/migrate/mod.rs +++ b/src/cli/schema/migrate/mod.rs @@ -14,7 +14,9 @@ use super::diff::BreakingChangeOutput; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::{MitigationStrategy, analyze_breaking_changes}; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::pglite::SchemaLoader; use crate::db::state::{Migration, StateBackend, StateHash}; @@ -146,10 +148,15 @@ impl Migrate { // Get the breaking changes for the migration let breaking_changes = analysis.into_changes(); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create the migration let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.clone(), @@ -164,7 +171,7 @@ impl Migrate { let output = SchemaMigrateOutput { migration_id: migration.id.to_hex(), description: self.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), has_breaking_changes: !breaking_changes.is_empty(), has_destructive_changes: destructive_count > 0, source_state_hash: source_hash.to_hex(), diff --git a/src/cli/show/mod.rs b/src/cli/show/mod.rs index eeb1f9b..18060c8 100644 --- a/src/cli/show/mod.rs +++ b/src/cli/show/mod.rs @@ -132,7 +132,7 @@ impl Show { // Generate SQL if needed let sql_statements = if matches!(self.format, OutputFormat::Sql) { - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let renderer = PostgresRenderer::new(RenderConfig::default()); let script = plan.render(&renderer); Some( diff --git a/src/db/compile/codegen.rs b/src/db/compile/codegen.rs index a77b09a..e4418a0 100644 --- a/src/db/compile/codegen.rs +++ b/src/db/compile/codegen.rs @@ -637,6 +637,7 @@ mod tests { Migration::new( "Test migration", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -653,6 +654,7 @@ mod tests { let migration = Migration::new( "Add users table", ops.clone(), + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -777,6 +779,7 @@ mod tests { let migration = Migration::new( "Drop users table", ops.clone(), + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), breaking_changes, @@ -810,6 +813,7 @@ mod tests { let migration = Migration::new( "Destructive changes", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), breaking_changes, @@ -927,6 +931,7 @@ mod tests { let migration = Migration::new( "Test with 'quotes' and \"double quotes\"", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -950,9 +955,10 @@ mod tests { let compiler = MigrationCompiler::with_config(CompilerConfig::macro_only()); // Create a migration with SQL containing special characters - let migration = Migration::new( + let _migration = Migration::new( "Test SQL", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -978,6 +984,7 @@ mod tests { let migration = Migration::new( "Line 1\nLine 2\nLine 3", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/db/compile/mod.rs b/src/db/compile/mod.rs index 9c937ad..d379b24 100644 --- a/src/db/compile/mod.rs +++ b/src/db/compile/mod.rs @@ -219,7 +219,7 @@ impl std::fmt::Display for PackageFormat { use crate::db::diff::breaking::analyze_breaking_changes; use crate::db::diff::diff_namespaces; -use crate::db::migrate::MigrationPlan; +use crate::db::migrate::{MigrationPlan, compute_inverse_operations}; use crate::db::model::Namespace; use crate::db::state::{Migration, StateHash}; @@ -399,10 +399,15 @@ pub fn compile_migration( let source_hash = StateHash::from_namespace(source); let target_hash = StateHash::from_namespace(target); + // Step 4.5: Compute inverse operations for down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Step 5: Create the migration record let migration = Migration::new( &options.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.into_changes(), @@ -553,6 +558,7 @@ mod tests { let migration = Migration::new( "Add email column to users table", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], // No breaking changes for adding nullable column @@ -617,6 +623,7 @@ mod tests { let migration = Migration::new( "Drop users table", plan.operations.clone(), + vec![], source_hash, target_hash, breaking_changes.into_changes(), @@ -682,6 +689,7 @@ mod tests { let migration = Migration::new( "Create users table with status enum", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], @@ -714,6 +722,7 @@ mod tests { let migration = Migration::new( r#"Add "quoted" column with 'single quotes' and \backslashes\"#, plan.operations.clone(), + vec![], source_hash, target_hash, vec![], @@ -746,6 +755,7 @@ mod tests { let migration = Migration::new( "Empty migration", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], diff --git a/src/db/execution/error.rs b/src/db/execution/error.rs index b8ad7c6..61f3d07 100644 --- a/src/db/execution/error.rs +++ b/src/db/execution/error.rs @@ -74,6 +74,30 @@ pub enum ExecutionError { #[error("query error: {0}")] #[diagnostic(code(tern::execution::query))] Query(String), + + /// No migrations to revert. + #[error("no migrations have been applied to the database")] + #[diagnostic( + code(tern::execution::no_migrations_to_revert), + help("Run 'tern up' to apply migrations first before attempting to revert.") + )] + NoMigrationsToRevert, + + /// Revert operation failed. + #[error("failed to revert migration {migration_id}: {message}")] + #[diagnostic(code(tern::execution::revert_failed))] + RevertFailed { + migration_id: String, + message: String, + }, + + /// Cannot revert baseline migration. + #[error("cannot revert baseline migration {0}")] + #[diagnostic( + code(tern::execution::cannot_revert_baseline), + help("The baseline migration represents the initial state and cannot be reverted.") + )] + CannotRevertBaseline(String), } /// Represents the result of applying a single migration. diff --git a/src/db/execution/executor.rs b/src/db/execution/executor.rs index 01dabe0..976b42b 100644 --- a/src/db/execution/executor.rs +++ b/src/db/execution/executor.rs @@ -182,7 +182,7 @@ impl<'a> MigrationExecutor<'a> { ) -> Result { // Render migration to SQL let renderer = PostgresRenderer::new(RenderConfig::default()); - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let script = plan.render(&renderer); let sql = script.to_sql(); let statement_count = script.all_statements().len(); @@ -250,16 +250,16 @@ impl<'a> MigrationExecutor<'a> { } } -/// Computes the BLAKE3 hash of a migration's operations. +/// Computes the BLAKE3 hash of a migration's up_operations. /// /// This hash is used to detect if a migration has been modified since it was -/// applied. Only the operations array is hashed, not the description or +/// applied. Only the up_operations array is hashed, not the description or /// timestamps, allowing descriptions to be updated without triggering /// divergence errors. fn compute_migration_hash(migration: &Migration) -> String { let mut hasher = blake3::Hasher::new(); let ops_json = - serde_json::to_vec(&migration.operations).expect("operations should be serializable"); + serde_json::to_vec(&migration.up_operations).expect("operations should be serializable"); hasher.update(&ops_json); hasher.finalize().to_hex().to_string() } @@ -273,14 +273,16 @@ mod tests { fn compute_migration_hash_is_deterministic() { let migration1 = Migration::new( "Test migration", - vec![], + vec![], // up_operations + vec![], // down_operations StateHash::zero(), StateHash::zero(), vec![], ); let migration2 = Migration::new( "Different description", // Different description - vec![], // Same operations + vec![], // Same up_operations + vec![], // down_operations StateHash::zero(), StateHash::zero(), vec![], @@ -289,7 +291,7 @@ mod tests { let hash1 = compute_migration_hash(&migration1); let hash2 = compute_migration_hash(&migration2); - // Hashes should be the same since only operations matter + // Hashes should be the same since only up_operations matter assert_eq!(hash1, hash2); } } diff --git a/src/db/execution/tracker.rs b/src/db/execution/tracker.rs index 09ff8ae..f16596e 100644 --- a/src/db/execution/tracker.rs +++ b/src/db/execution/tracker.rs @@ -260,6 +260,68 @@ impl<'a> MigrationTracker<'a> { Ok(row.get(0)) } + + /// Removes the most recent migration record from the tracking tables. + /// + /// This should be called within a transaction after successfully + /// reverting the migration operations. + /// + /// # Returns + /// + /// Returns the ID of the removed migration if successful. + pub async fn unrecord_migration(&self) -> Result { + // Get the current (most recent) migration + let current_id = self + .get_current_migration_id() + .await? + .ok_or(ExecutionError::NoMigrationsToRevert)?; + + // Get the current migration's sequence to find the previous one + let current_migration = self + .get_migration(¤t_id) + .await? + .ok_or(ExecutionError::MigrationNotFound(current_id.clone()))?; + + // Find the previous migration (if any) + let previous = if current_migration.sequence > 1 { + let row = self + .client + .query_opt( + "SELECT id FROM tern.migrations WHERE sequence = $1", + &[&(current_migration.sequence - 1)], + ) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + row.map(|r| r.get::<_, String>(0)) + } else { + None + }; + + // Delete the current migration record + self.client + .execute("DELETE FROM tern.migrations WHERE id = $1", &[¤t_id]) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + + // Update the current pointer + if let Some(prev_id) = previous { + self.client + .execute( + "UPDATE tern.current SET migration_id = $1, updated_at = now()", + &[&prev_id], + ) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + } else { + // No previous migration - delete the current pointer + self.client + .execute("DELETE FROM tern.current", &[]) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + } + + Ok(current_id) + } } #[cfg(test)] diff --git a/src/db/migrate/inverse.rs b/src/db/migrate/inverse.rs new file mode 100644 index 0000000..4896f96 --- /dev/null +++ b/src/db/migrate/inverse.rs @@ -0,0 +1,388 @@ +//! Inverse operation computation for migration reversal. +//! +//! This module provides functions to compute the inverse (reverse) operations +//! needed to revert a migration. Inverse operations are computed at migration +//! generation time and stored alongside the forward operations. + +use super::Operation; + +/// Error type for inverse operation computation. +#[derive(Debug, Clone, thiserror::Error)] +pub enum InverseError { + /// The operation cannot be reversed. + #[error("{0}")] + Irreversible(String), +} + +/// Result of computing inverse operations. +#[derive(Debug, Clone)] +pub struct InverseResult { + /// The computed inverse operations, in reverse order. + pub operations: Vec, + + /// Warnings about operations that could not be reversed. + /// If this is non-empty, the `operations` list will be empty. + pub errors: Vec, +} + +impl InverseResult { + /// Returns true if all operations were successfully inverted. + pub fn is_success(&self) -> bool { + self.errors.is_empty() + } + + /// Returns true if any operations could not be inverted. + pub fn has_errors(&self) -> bool { + !self.errors.is_empty() + } +} + +/// Computes the inverse operations needed to revert a migration. +/// +/// Returns the operations in reverse order (last applied first reverted). +/// If any operation is irreversible, returns an empty operations list with errors. +pub fn compute_inverse_operations(operations: &[Operation]) -> InverseResult { + let mut inverse_ops = Vec::new(); + let mut errors = Vec::new(); + + // Process operations in reverse order + for op in operations.iter().rev() { + match compute_single_inverse(op) { + Ok(inverse) => inverse_ops.push(inverse), + Err(e) => errors.push(e), + } + } + + // If there were any errors, return empty operations + if !errors.is_empty() { + InverseResult { + operations: vec![], + errors, + } + } else { + InverseResult { + operations: inverse_ops, + errors: vec![], + } + } +} + +/// Computes the inverse of a single operation. +fn compute_single_inverse(op: &Operation) -> Result { + match op { + // Enum operations + Operation::CreateEnum { schema, enum_type } => Ok(Operation::DropEnum { + schema: schema.clone(), + name: enum_type.name.clone(), + }), + Operation::DropEnum { .. } => Err(InverseError::Irreversible( + "Cannot revert DropEnum: enum definition is not preserved".to_string(), + )), + Operation::RenameEnum { schema, from, to } => Ok(Operation::RenameEnum { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AddEnumValue { .. } => Err(InverseError::Irreversible( + "Cannot revert AddEnumValue: PostgreSQL does not support removing enum values" + .to_string(), + )), + + // Sequence operations + Operation::CreateSequence { schema, sequence } => Ok(Operation::DropSequence { + schema: schema.clone(), + name: sequence.name.clone(), + }), + Operation::DropSequence { .. } => Err(InverseError::Irreversible( + "Cannot revert DropSequence: sequence definition is not preserved".to_string(), + )), + Operation::RenameSequence { schema, from, to } => Ok(Operation::RenameSequence { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterSequence { .. } => Err(InverseError::Irreversible( + "Cannot revert AlterSequence: previous values are not preserved".to_string(), + )), + + // Table operations + Operation::CreateTable { schema, table } => Ok(Operation::DropTable { + schema: schema.clone(), + name: table.name.clone(), + }), + Operation::DropTable { .. } => Err(InverseError::Irreversible( + "Cannot revert DropTable: table definition and data are not preserved".to_string(), + )), + Operation::RenameTable { schema, from, to } => Ok(Operation::RenameTable { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Column operations + Operation::AddColumn { + schema, + table, + column, + } => Ok(Operation::DropColumn { + schema: schema.clone(), + table: table.clone(), + name: column.name.clone(), + }), + Operation::DropColumn { .. } => Err(InverseError::Irreversible( + "Cannot revert DropColumn: column definition and data are not preserved".to_string(), + )), + Operation::RenameColumn { + schema, + table, + from, + to, + } => Ok(Operation::RenameColumn { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterColumn { .. } => Err(InverseError::Irreversible( + "Cannot revert AlterColumn: previous values are not preserved".to_string(), + )), + + // Constraint operations + Operation::AddConstraint { + schema, + table, + constraint, + } => Ok(Operation::DropConstraint { + schema: schema.clone(), + table: table.clone(), + name: constraint.name.clone(), + }), + Operation::DropConstraint { .. } => Err(InverseError::Irreversible( + "Cannot revert DropConstraint: constraint definition is not preserved".to_string(), + )), + Operation::RenameConstraint { + schema, + table, + from, + to, + } => Ok(Operation::RenameConstraint { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Index operations + Operation::CreateIndex { + schema, + index, + concurrently, + .. + } => Ok(Operation::DropIndex { + schema: schema.clone(), + name: index.name.clone(), + concurrently: *concurrently, + }), + Operation::DropIndex { .. } => Err(InverseError::Irreversible( + "Cannot revert DropIndex: index definition is not preserved".to_string(), + )), + Operation::RenameIndex { schema, from, to } => Ok(Operation::RenameIndex { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // View operations + Operation::CreateView { schema, view } => Ok(Operation::DropView { + schema: schema.clone(), + name: view.name.clone(), + is_materialized: view.is_materialized, + }), + Operation::DropView { .. } => Err(InverseError::Irreversible( + "Cannot revert DropView: view definition is not preserved".to_string(), + )), + Operation::RenameView { + schema, + from, + to, + is_materialized, + } => Ok(Operation::RenameView { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + is_materialized: *is_materialized, + }), + Operation::ReplaceView { .. } => Err(InverseError::Irreversible( + "Cannot revert ReplaceView: previous view definition is not preserved".to_string(), + )), + Operation::RefreshMaterializedView { .. } => Err(InverseError::Irreversible( + "Cannot revert RefreshMaterializedView: this operation cannot be undone".to_string(), + )), + + // Comment operations + Operation::SetComment { .. } => Err(InverseError::Irreversible( + "Cannot revert SetComment: previous comment is not preserved".to_string(), + )), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::model::{EnumType, Table, TableKind}; + use crate::db::schema::{Oid, SchemaName, TableName, TypeName}; + + fn test_schema() -> SchemaName { + SchemaName::try_new("public".to_string()).unwrap() + } + + fn test_table_name() -> TableName { + TableName::try_new("users".to_string()).unwrap() + } + + fn test_table() -> Table { + Table { + oid: Oid::new(1), + name: test_table_name(), + kind: TableKind::Regular, + columns: vec![], + constraints: vec![], + indexes: vec![], + comment: None, + } + } + + fn test_enum() -> EnumType { + EnumType { + oid: Oid::new(1), + name: TypeName::try_new("status".to_string()).unwrap(), + values: vec!["active".to_string(), "inactive".to_string()], + comment: None, + } + } + + #[test] + fn create_table_inverse_is_drop_table() { + let op = Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert_eq!(result.operations.len(), 1); + assert!(matches!( + &result.operations[0], + Operation::DropTable { schema, name } + if schema.as_ref() == "public" && name.as_ref() == "users" + )); + } + + #[test] + fn drop_table_is_irreversible() { + let op = Operation::DropTable { + schema: test_schema(), + name: test_table_name(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + assert_eq!(result.errors.len(), 1); + } + + #[test] + fn rename_table_swaps_names() { + let op = Operation::RenameTable { + schema: test_schema(), + from: TableName::try_new("old_name".to_string()).unwrap(), + to: TableName::try_new("new_name".to_string()).unwrap(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert!(matches!( + &result.operations[0], + Operation::RenameTable { from, to, .. } + if from.as_ref() == "new_name" && to.as_ref() == "old_name" + )); + } + + #[test] + fn create_enum_inverse_is_drop_enum() { + let op = Operation::CreateEnum { + schema: test_schema(), + enum_type: test_enum(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert!(matches!( + &result.operations[0], + Operation::DropEnum { name, .. } + if name.as_ref() == "status" + )); + } + + #[test] + fn add_enum_value_is_irreversible() { + let op = Operation::AddEnumValue { + schema: test_schema(), + enum_name: TypeName::try_new("status".to_string()).unwrap(), + value: "pending".to_string(), + position: crate::db::migrate::EnumValuePosition::End, + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + } + + #[test] + fn multiple_operations_reversed_in_order() { + let ops = vec![ + Operation::CreateEnum { + schema: test_schema(), + enum_type: test_enum(), + }, + Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }, + ]; + let result = compute_inverse_operations(&ops); + + assert!(result.is_success()); + assert_eq!(result.operations.len(), 2); + + // Order should be reversed: DropTable first, then DropEnum + assert!(matches!(&result.operations[0], Operation::DropTable { .. })); + assert!(matches!(&result.operations[1], Operation::DropEnum { .. })); + } + + #[test] + fn mixed_reversible_irreversible_fails() { + let ops = vec![ + Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }, + Operation::DropTable { + schema: test_schema(), + name: TableName::try_new("other".to_string()).unwrap(), + }, + ]; + let result = compute_inverse_operations(&ops); + + // Should fail because DropTable is irreversible + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + } + + #[test] + fn empty_operations_returns_empty() { + let result = compute_inverse_operations(&[]); + assert!(result.is_success()); + assert!(result.operations.is_empty()); + } +} diff --git a/src/db/migrate/mod.rs b/src/db/migrate/mod.rs index dedc1b0..7000131 100644 --- a/src/db/migrate/mod.rs +++ b/src/db/migrate/mod.rs @@ -33,6 +33,7 @@ mod collector; mod error; +mod inverse; mod operation; mod ordering; mod plan; @@ -41,6 +42,7 @@ mod script; pub use collector::{CollectorConfig, OperationCollector}; pub use error::MigrationError; +pub use inverse::{InverseError, InverseResult, compute_inverse_operations}; pub use operation::{ ColumnChanges, CommentTarget, DefaultChange, EnumValuePosition, GeneratedChange, IdentityChange, ObjectKind, Operation, OperationId, SequenceChanges, SetColumnType, diff --git a/src/db/state/init.rs b/src/db/state/init.rs index 08eebcb..e48d994 100644 --- a/src/db/state/init.rs +++ b/src/db/state/init.rs @@ -9,7 +9,7 @@ #![allow(unused_assignments)] use crate::db::diff::diff_namespaces; -use crate::db::migrate::MigrationPlan; +use crate::db::migrate::{MigrationPlan, compute_inverse_operations}; use crate::db::model::Namespace; use crate::db::query::{Catalog, QueryError, load_namespace}; @@ -118,8 +118,13 @@ where let diff = diff_namespaces(&empty, &namespace); let plan = MigrationPlan::from_diff(&diff); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create and save baseline migration with operations - let baseline = Migration::baseline_with_operations(namespace.clone(), plan.operations); + let baseline = + Migration::baseline_with_operations(namespace.clone(), plan.operations, down_operations); backend.save_migration(&baseline).await?; // Save current state @@ -252,7 +257,7 @@ mod tests { assert!(baseline.is_baseline()); assert!(baseline.is_checkpoint()); assert!(baseline.parent_state_hash.is_zero()); - assert_eq!(baseline.operations.len(), 0); + assert_eq!(baseline.up_operations.len(), 0); // Verify backend state assert!(backend.is_initialized().await.unwrap()); @@ -307,8 +312,8 @@ mod tests { assert!(baseline.parent_state_hash.is_zero()); assert!(baseline.is_checkpoint()); - // NEW: Baseline now has operations to create the schema from scratch - assert!(!baseline.operations.is_empty()); + // NEW: Baseline now has up_operations to create the schema from scratch + assert!(!baseline.up_operations.is_empty()); // Should have at least one operation to create the table assert!(baseline.operation_count() >= 1); diff --git a/src/db/state/local.rs b/src/db/state/local.rs index ddd8308..8d1a6fd 100644 --- a/src/db/state/local.rs +++ b/src/db/state/local.rs @@ -483,10 +483,10 @@ impl StateBackend for LocalFileBackend { // Find nearest checkpoint and apply operations let (mut state, start_idx) = self.find_nearest_checkpoint(&migrations, target_idx)?; - // Apply operations from checkpoint to target (exclusive of checkpoint migration itself + // Apply up_operations from checkpoint to target (exclusive of checkpoint migration itself // since its state is already included) for migration in &migrations[start_idx..=target_idx] { - state = state.apply(&migration.operations).map_err(|e| { + state = state.apply(&migration.up_operations).map_err(|e| { StateError::ReconstructionFailed { id: migration.id, message: e.to_string(), @@ -688,6 +688,7 @@ mod tests { let m2 = Migration::new( "Second migration", vec![], + vec![], m1.resulting_state_hash, test_state_hash(2), vec![], @@ -697,6 +698,7 @@ mod tests { let m3 = Migration::new( "Third migration", vec![], + vec![], m2.resulting_state_hash, test_state_hash(3), vec![], @@ -735,7 +737,7 @@ mod tests { // After second migration let result_hash = test_state_hash(99); - let m2 = Migration::new("Second", vec![], expected_hash, result_hash, vec![]); + let m2 = Migration::new("Second", vec![], vec![], expected_hash, result_hash, vec![]); backend.save_migration(&m2).await.unwrap(); assert_eq!(backend.get_current_state_hash().await.unwrap(), result_hash); @@ -757,6 +759,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -790,6 +793,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -816,6 +820,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -825,6 +830,7 @@ mod tests { let m3 = Migration::new( "Third", vec![], + vec![], m2.resulting_state_hash, test_state_hash(33), vec![], @@ -923,6 +929,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(2), vec![], @@ -932,6 +939,7 @@ mod tests { let m3 = Migration::new( "Third", vec![], + vec![], m2.resulting_state_hash, test_state_hash(3), vec![], diff --git a/src/db/state/mod.rs b/src/db/state/mod.rs index 09fb5f1..7df4e9d 100644 --- a/src/db/state/mod.rs +++ b/src/db/state/mod.rs @@ -380,7 +380,7 @@ mod tests { })?; for migration in &migrations[start_idx..=target_idx] { - state = state.apply(&migration.operations).map_err(|e| { + state = state.apply(&migration.up_operations).map_err(|e| { StateError::ReconstructionFailed { id: migration.id, message: e.to_string(), @@ -465,6 +465,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, StateHash::from_bytes([22u8; 32]), vec![], diff --git a/src/db/state/types.rs b/src/db/state/types.rs index 2ffbf23..9bb5119 100644 --- a/src/db/state/types.rs +++ b/src/db/state/types.rs @@ -300,8 +300,12 @@ pub struct Migration { /// When the migration was created (not applied). pub created_at: Timestamp, - /// The operations that make up this migration. - pub operations: Vec, + /// The forward operations that make up this migration (applied with `up`). + pub up_operations: Vec, + + /// The reverse operations to undo this migration (applied with `down`). + /// Empty if the migration contains irreversible operations. + pub down_operations: Vec, /// Hash of the schema state before this migration. pub parent_state_hash: StateHash, @@ -321,23 +325,25 @@ pub struct Migration { impl Migration { /// Creates a new migration with the given parameters. /// - /// The migration ID is computed automatically from the content. + /// The migration ID is computed automatically from the up_operations content. #[must_use] pub fn new( description: impl Into, - operations: Vec, + up_operations: Vec, + down_operations: Vec, parent_state_hash: StateHash, resulting_state_hash: StateHash, breaking_changes: Vec, ) -> Self { let description = description.into(); - let id = MigrationId::from_content(&operations, &parent_state_hash, &description); + let id = MigrationId::from_content(&up_operations, &parent_state_hash, &description); Self { id, description, created_at: Timestamp::now(), - operations, + up_operations, + down_operations, parent_state_hash, resulting_state_hash, breaking_changes, @@ -359,7 +365,8 @@ impl Migration { id, description, created_at: Timestamp::now(), - operations: vec![], + up_operations: vec![], + down_operations: vec![], parent_state_hash: StateHash::zero(), resulting_state_hash: resulting_hash, breaking_changes: vec![], @@ -376,18 +383,24 @@ impl Migration { /// # Arguments /// /// * `namespace` - The target schema state - /// * `operations` - Operations to create the schema from empty + /// * `up_operations` - Operations to create the schema from empty + /// * `down_operations` - Operations to revert to empty (typically drop all) #[must_use] - pub fn baseline_with_operations(namespace: Namespace, operations: Vec) -> Self { + pub fn baseline_with_operations( + namespace: Namespace, + up_operations: Vec, + down_operations: Vec, + ) -> Self { let description = "Baseline migration from existing database".to_string(); - let id = MigrationId::from_content(&operations, &StateHash::zero(), &description); + let id = MigrationId::from_content(&up_operations, &StateHash::zero(), &description); let resulting_hash = StateHash::from_namespace(&namespace); Self { id, description, created_at: Timestamp::now(), - operations, + up_operations, + down_operations, parent_state_hash: StateHash::zero(), resulting_state_hash: resulting_hash, breaking_changes: vec![], @@ -418,16 +431,24 @@ impl Migration { self.checkpoint_state.is_some() } - /// Returns true if this is a baseline migration (no operations). + /// Returns true if this is a baseline migration (no up operations). #[must_use] pub fn is_baseline(&self) -> bool { - self.operations.is_empty() && self.parent_state_hash.is_zero() + self.up_operations.is_empty() && self.parent_state_hash.is_zero() } - /// Returns the number of operations in this migration. + /// Returns the number of up operations in this migration. #[must_use] pub fn operation_count(&self) -> usize { - self.operations.len() + self.up_operations.len() + } + + /// Returns true if this migration can be reverted. + /// + /// A migration can be reverted if it has down operations defined. + #[must_use] + pub fn is_reversible(&self) -> bool { + !self.down_operations.is_empty() } } @@ -734,8 +755,8 @@ mod tests { fn new_migration_computes_id() { let parent = StateHash::zero(); let result = StateHash::from_bytes([1u8; 32]); - let m1 = Migration::new("test", vec![], parent, result, vec![]); - let m2 = Migration::new("test", vec![], parent, result, vec![]); + let m1 = Migration::new("test", vec![], vec![], parent, result, vec![]); + let m2 = Migration::new("test", vec![], vec![], parent, result, vec![]); // Same content should produce same ID assert_eq!(m1.id, m2.id); @@ -745,13 +766,33 @@ mod tests { fn with_checkpoint_sets_state() { let parent = StateHash::zero(); let result = StateHash::from_bytes([1u8; 32]); - let migration = Migration::new("test", vec![], parent, result, vec![]); + let migration = Migration::new("test", vec![], vec![], parent, result, vec![]); assert!(!migration.is_checkpoint()); let ns = Namespace::empty("public"); let migration = migration.with_checkpoint(ns); assert!(migration.is_checkpoint()); } + + #[test] + fn is_reversible_with_down_operations() { + use crate::db::schema::{SchemaName, TableName}; + + let parent = StateHash::zero(); + let result = StateHash::from_bytes([1u8; 32]); + + // Migration without down operations is not reversible + let migration = Migration::new("test", vec![], vec![], parent, result, vec![]); + assert!(!migration.is_reversible()); + + // Migration with down operations is reversible + let down_ops = vec![Operation::DropTable { + schema: SchemaName::try_new("public".to_string()).unwrap(), + name: TableName::try_new("users".to_string()).unwrap(), + }]; + let migration = Migration::new("test", vec![], down_ops, parent, result, vec![]); + assert!(migration.is_reversible()); + } } mod migration_index_tests { diff --git a/tests/compile_integration_tests.rs b/tests/compile_integration_tests.rs index 6ded796..ff65513 100644 --- a/tests/compile_integration_tests.rs +++ b/tests/compile_integration_tests.rs @@ -778,6 +778,7 @@ mod low_level_api { let migration = Migration::new( "Create users table", plan.operations.clone(), + vec![], source_hash, target_hash, breaking_changes.into_changes(), @@ -811,6 +812,7 @@ mod low_level_api { let migration = Migration::new( "Create users", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], diff --git a/tests/state_reconstruction_tests.rs b/tests/state_reconstruction_tests.rs index 45c54f9..20bff9c 100644 --- a/tests/state_reconstruction_tests.rs +++ b/tests/state_reconstruction_tests.rs @@ -44,6 +44,7 @@ fn build_migration( let migration = Migration::new( description, plan.operations, + vec![], parent_hash, resulting_hash, vec![],