diff --git a/CHANGELOG.md b/CHANGELOG.md index c59aeae..7052aeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - ORM: **migration preflight guards — nothing runs until the whole plan is validated.** Duplicate versions (two files, or a file shadowing a coded migration), empty or non-portable version/name strings (also closes `write_migration_file` writing outside its directory via a `../`-shaped version), an applied migration that was *renamed* in code (the checksum guard already caught edits; `repair` now re-stamps names too), and an **out-of-order pending migration** — one sorting before a version already applied, the merged-stale-branch hazard whose DDL would run against a schema later migrations already reshaped. Out-of-order is a hard error naming the versions; teams that genuinely interleave opt in with `Migrator::allow_out_of_order()`. The guard anchors only on versions the migrator itself defines, so two apps sharing one database (and one `_sutegi_migrations` table) don't read each other's history as staleness. -- ORM: **`Migrator::rollback` preflights the whole batch before undoing anything.** It used to discover a forward-only or code-deleted migration *mid-batch*, erroring with the newer half already rolled back — the one state with no clean way forward or back. Now that discovery happens up front and the database is untouched. +- ORM: **`Migrator::rollback` preflights the whole batch before undoing anything.** It used to discover a forward-only or code-deleted migration *mid-batch*, erroring with the newer half already rolled back — the one state with no clean way forward or back. Now that discovery happens up front and the database is untouched. Rollback also only targets batches containing versions **this migrator defines**, so an app sharing a database (and the `_sutegi_migrations` table) with another can no longer pick the *other* app's newest batch as its rollback victim. - ORM: **`Migrator::plan_run(&db)` — the dry run.** The pending migrations in apply order, each with the exact SQL a declarative migration would execute (rendered for the backend's dialect against the live schema, table-rebuild expansions included); closure bodies report `None` rather than pretending. Read-only, so it's safe to wire into a deploy pipeline's review step. - ORM: **`Migration::no_transaction()`** for DDL that refuses to run inside a transaction — Postgres `CREATE INDEX CONCURRENTLY` being the canonical case. The trade is explicit and documented: a crash between the body and its history row re-runs the body next time, so such migrations must be idempotent. `Migrator::lock_timeout(...)` tunes how long a runner waits for a busy migration lock (default 300 s) instead of hanging a deploy forever, and `MigrationOps` gained `dialect()` so a closure migration can write dialect-specific SQL without guessing. diff --git a/crates/sutegi-orm/src/migrate.rs b/crates/sutegi-orm/src/migrate.rs index 68c6fb0..9779971 100644 --- a/crates/sutegi-orm/src/migrate.rs +++ b/crates/sutegi-orm/src/migrate.rs @@ -797,9 +797,13 @@ impl Migrator { /// migration atomically (its `down` and its history delete in one /// transaction). Returns the versions rolled back. /// - /// The whole batch is **preflighted before anything is undone**: if any - /// victim is forward-only or no longer defined in code, the rollback - /// errors with the database untouched — never a half-rolled-back batch. + /// Only batches containing at least one version **this migrator defines** + /// are candidates — another app sharing the database (and the history + /// table) can't have *its* newest batch picked as this app's rollback + /// target. The whole batch is then **preflighted before anything is + /// undone**: if any victim is forward-only or no longer defined in code, + /// the rollback errors with the database untouched — never a + /// half-rolled-back batch. pub fn rollback( &self, conn: &B, @@ -813,8 +817,17 @@ impl Migrator { return Ok(Vec::new()); } - // The `batches` highest distinct batch numbers. - let mut batch_nums: Vec = applied.iter().map(|r| r.batch).collect(); + // The `batches` highest distinct batch numbers among batches that + // contain at least one version defined here (a batch is written by a + // single run, so this keeps another app's batches out of scope while + // still surfacing a code-deleted migration inside our own batch). + let defined: std::collections::BTreeSet<&str> = + self.migrations.iter().map(|m| m.version.as_str()).collect(); + let mut batch_nums: Vec = applied + .iter() + .filter(|r| defined.contains(r.version.as_str())) + .map(|r| r.batch) + .collect(); batch_nums.sort_unstable(); batch_nums.dedup(); let target: std::collections::BTreeSet = @@ -1730,6 +1743,41 @@ mod tests { assert!(db.select(&QueryBuilder::table("posts")).is_ok()); } + #[test] + fn rollback_targets_only_this_migrators_batches() { + // Two apps share one database (and one history table). App B's + // rollback must pick B's newest batch, not A's globally-newest one. + let db = Db::memory().unwrap(); + let app_a = Migrator::new().add(Migration::reversible( + "a_0001", + "a1", + |db| { + db.execute("CREATE TABLE a1 (id INTEGER PRIMARY KEY)", &[]) + .map(|_| ()) + }, + |db| db.execute("DROP TABLE a1", &[]).map(|_| ()), + )); + let app_b = Migrator::new().add(Migration::reversible( + "b_0001", + "b1", + |db| { + db.execute("CREATE TABLE b1 (id INTEGER PRIMARY KEY)", &[]) + .map(|_| ()) + }, + |db| db.execute("DROP TABLE b1", &[]).map(|_| ()), + )); + app_b.run(&db).unwrap(); + app_a.run(&db).unwrap(); // batch 2 — the globally newest + + // B rolls back: undoes b_0001 (batch 1), leaves A's batch 2 alone. + assert_eq!(app_b.rollback(&db, 1).unwrap(), vec!["b_0001"]); + assert!(db.select(&QueryBuilder::table("a1")).is_ok()); + assert!(db.select(&QueryBuilder::table("b1")).is_err()); + let a_status = app_a.status(&db).unwrap(); + let a1 = a_status.iter().find(|s| s.version == "a_0001").unwrap(); + assert!(a1.applied); + } + #[test] fn plan_run_previews_sql_without_executing() { let db = Db::memory().unwrap();