From b7bbc4a12a29d2eabf03b14327ec0f7dd429ec98 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Thu, 11 Dec 2025 01:32:15 +0900 Subject: [PATCH] Add migration validation --- .../changepack_log_DmywO0Aa5wafZc2erMz7C.json | 1 + Cargo.lock | 14 +- crates/vespertide-cli/src/utils.rs | 6 +- crates/vespertide-planner/src/error.rs | 2 + crates/vespertide-planner/src/lib.rs | 2 +- crates/vespertide-planner/src/validate.rs | 128 +++++++++++++++++- 6 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 .changepacks/changepack_log_DmywO0Aa5wafZc2erMz7C.json diff --git a/.changepacks/changepack_log_DmywO0Aa5wafZc2erMz7C.json b/.changepacks/changepack_log_DmywO0Aa5wafZc2erMz7C.json new file mode 100644 index 0000000..3ed42bf --- /dev/null +++ b/.changepacks/changepack_log_DmywO0Aa5wafZc2erMz7C.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch"},"note":"Add migration validation","date":"2025-12-10T16:31:52.974551200Z"} \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 7c3f987..d6d25c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -913,14 +913,14 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "vespertide" -version = "0.1.0" +version = "0.1.1" dependencies = [ "vespertide-macro", ] [[package]] name = "vespertide-cli" -version = "0.1.0" +version = "0.1.1" dependencies = [ "anyhow", "chrono", @@ -940,7 +940,7 @@ dependencies = [ [[package]] name = "vespertide-config" -version = "0.1.0" +version = "0.1.1" dependencies = [ "clap", "serde", @@ -948,7 +948,7 @@ dependencies = [ [[package]] name = "vespertide-core" -version = "0.1.0" +version = "0.1.1" dependencies = [ "schemars", "serde", @@ -956,7 +956,7 @@ dependencies = [ [[package]] name = "vespertide-macro" -version = "0.1.0" +version = "0.1.1" dependencies = [ "thiserror", "vespertide-core", @@ -964,7 +964,7 @@ dependencies = [ [[package]] name = "vespertide-planner" -version = "0.1.0" +version = "0.1.1" dependencies = [ "rstest", "thiserror", @@ -973,7 +973,7 @@ dependencies = [ [[package]] name = "vespertide-query" -version = "0.1.0" +version = "0.1.1" dependencies = [ "rstest", "thiserror", diff --git a/crates/vespertide-cli/src/utils.rs b/crates/vespertide-cli/src/utils.rs index 002f0b0..0c98bfe 100644 --- a/crates/vespertide-cli/src/utils.rs +++ b/crates/vespertide-cli/src/utils.rs @@ -4,7 +4,7 @@ use std::path::PathBuf; use anyhow::{Context, Result}; use vespertide_config::{FileFormat, VespertideConfig}; use vespertide_core::{MigrationPlan, TableDef}; -use vespertide_planner::validate_schema; +use vespertide_planner::{validate_migration_plan, validate_schema}; /// Load vespertide.json config from current directory. pub fn load_config() -> Result { @@ -86,6 +86,10 @@ pub fn load_migrations(config: &VespertideConfig) -> Result> .with_context(|| format!("parse migration: {}", path.display()))? }; + // Validate the migration plan + validate_migration_plan(&plan) + .with_context(|| format!("validate migration: {}", path.display()))?; + plans.push(plan); } } diff --git a/crates/vespertide-planner/src/error.rs b/crates/vespertide-planner/src/error.rs index 566e62c..fe841b9 100644 --- a/crates/vespertide-planner/src/error.rs +++ b/crates/vespertide-planner/src/error.rs @@ -24,4 +24,6 @@ pub enum PlannerError { ConstraintColumnNotFound(String, String, String), #[error("constraint has empty column list: {0}.{1}")] EmptyConstraintColumns(String, String), + #[error("AddColumn requires fill_with when column is NOT NULL without default: {0}.{1}")] + MissingFillWith(String, String), } diff --git a/crates/vespertide-planner/src/lib.rs b/crates/vespertide-planner/src/lib.rs index 433722b..932f42d 100644 --- a/crates/vespertide-planner/src/lib.rs +++ b/crates/vespertide-planner/src/lib.rs @@ -10,4 +10,4 @@ pub use diff::diff_schemas; pub use error::PlannerError; pub use plan::plan_next_migration; pub use schema::schema_from_plans; -pub use validate::validate_schema; +pub use validate::{validate_migration_plan, validate_schema}; diff --git a/crates/vespertide-planner/src/validate.rs b/crates/vespertide-planner/src/validate.rs index 90289ef..759c9aa 100644 --- a/crates/vespertide-planner/src/validate.rs +++ b/crates/vespertide-planner/src/validate.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; -use vespertide_core::{IndexDef, TableConstraint, TableDef}; +use vespertide_core::{IndexDef, MigrationAction, MigrationPlan, TableConstraint, TableDef}; use crate::error::PlannerError; @@ -514,4 +514,130 @@ mod tests { } } } + + #[test] + fn validate_migration_plan_missing_fill_with() { + use vespertide_core::{ColumnDef, ColumnType, MigrationAction, MigrationPlan}; + + let plan = MigrationPlan { + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::AddColumn { + table: "users".into(), + column: ColumnDef { + name: "email".into(), + r#type: ColumnType::Text, + nullable: false, + default: None, + }, + fill_with: None, + }], + }; + + let result = validate_migration_plan(&plan); + assert!(result.is_err()); + match result.unwrap_err() { + PlannerError::MissingFillWith(table, column) => { + assert_eq!(table, "users"); + assert_eq!(column, "email"); + } + _ => panic!("expected MissingFillWith error"), + } + } + + #[test] + fn validate_migration_plan_with_fill_with() { + use vespertide_core::{ColumnDef, ColumnType, MigrationAction, MigrationPlan}; + + let plan = MigrationPlan { + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::AddColumn { + table: "users".into(), + column: ColumnDef { + name: "email".into(), + r#type: ColumnType::Text, + nullable: false, + default: None, + }, + fill_with: Some("default@example.com".into()), + }], + }; + + let result = validate_migration_plan(&plan); + assert!(result.is_ok()); + } + + #[test] + fn validate_migration_plan_nullable_column() { + use vespertide_core::{ColumnDef, ColumnType, MigrationAction, MigrationPlan}; + + let plan = MigrationPlan { + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::AddColumn { + table: "users".into(), + column: ColumnDef { + name: "email".into(), + r#type: ColumnType::Text, + nullable: true, + default: None, + }, + fill_with: None, + }], + }; + + let result = validate_migration_plan(&plan); + assert!(result.is_ok()); + } + + #[test] + fn validate_migration_plan_with_default() { + use vespertide_core::{ColumnDef, ColumnType, MigrationAction, MigrationPlan}; + + let plan = MigrationPlan { + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::AddColumn { + table: "users".into(), + column: ColumnDef { + name: "email".into(), + r#type: ColumnType::Text, + nullable: false, + default: Some("default@example.com".into()), + }, + fill_with: None, + }], + }; + + let result = validate_migration_plan(&plan); + assert!(result.is_ok()); + } +} + +/// Validate a migration plan for correctness. +/// Checks for: +/// - AddColumn actions with NOT NULL columns without default must have fill_with +pub fn validate_migration_plan(plan: &MigrationPlan) -> Result<(), PlannerError> { + for action in &plan.actions { + if let MigrationAction::AddColumn { + table, + column, + fill_with, + } = action + { + // If column is NOT NULL and has no default, fill_with is required + if !column.nullable && column.default.is_none() && fill_with.is_none() { + return Err(PlannerError::MissingFillWith( + table.clone(), + column.name.clone(), + )); + } + } + } + Ok(()) }