From 5ace1c438bb911e24aed6240fcbb8c41174ad141 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Mon, 17 Aug 2026 08:43:25 -0400 Subject: [PATCH 1/2] add checksum drift detection and repeatable migrations Drift detection: an up run verifies that every already-applied migration still matches its recorded checksum before applying anything, aborting with `Error::ChecksumMismatch`. Opt out with `Migrator::allow_checksum_mismatch` / `--allow-checksum-mismatch`. Repeatable migrations: a migration declared with `repeatable()` or a `-- migrant:repeatable` directive re-runs whenever its up-SQL checksum changes instead of applying once. They run after the pending versioned migrations, at most once per run, keep a single bookkeeping row updated in place, and are exempt from the drift, unknown-tag, and out-of-order checks. They are forward-only: a down run never selects them, and declaring one with no checksum or with a down direction is rejected at registration. `Migrator::rerun_repeatable` / `--rerun-repeatable` re-runs them regardless of checksum, and `redo` warns when it will not revert one. The bookkeeping table gains an `is_repeatable` column, and `down.sql` is now optional for file-discovered migrations. See the changelogs for the on-disk schema upgrade note. --- CHANGELOG.md | 31 +- README.md | 8 +- docs/src/cli.md | 56 +- docs/src/migration-types.md | 29 + docs/src/migrations.md | 54 +- migrant_lib/CHANGELOG.md | 28 +- migrant_lib/src/config/mod.rs | 67 +- migrant_lib/src/drivers/mod.rs | 153 ++++- migrant_lib/src/drivers/mysql.rs | 84 ++- migrant_lib/src/drivers/pg.rs | 94 ++- migrant_lib/src/drivers/sqlite.rs | 146 ++++- migrant_lib/src/errors.rs | 11 + migrant_lib/src/lib.rs | 34 +- migrant_lib/src/migratable.rs | 165 +++++ migrant_lib/src/migration.rs | 235 ++++++- migrant_lib/src/migrator.rs | 1007 +++++++++++++++++++++++++++-- migrant_lib/src/ops.rs | 283 ++++++-- migrant_lib/tests/server_dbs.rs | 183 +++++- migrant_lib/tests/sqlite.rs | 792 +++++++++++++++++++++++ spec/README.md | 2 + spec/checksum-drift-detection.md | 50 ++ spec/cli-migration-management.md | 32 +- spec/database-backends.md | 13 +- spec/error-handling-api.md | 18 +- spec/migration-types.md | 22 + spec/migrator-api.md | 22 +- spec/repeatable-migrations.md | 141 ++++ src/cli.rs | 40 +- src/main.rs | 54 +- src/status.rs | 147 ++++- src/tui.rs | 19 +- tests/migrant.rs | 428 +++++++++++- 32 files changed, 4127 insertions(+), 321 deletions(-) create mode 100644 spec/checksum-drift-detection.md create mode 100644 spec/repeatable-migrations.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 8160d27..26a6e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ ## [Unreleased] ### Added +- An up run verifies that each already-applied migration still matches its recorded checksum + and aborts with `ChecksumMismatch` if one changed since it was applied. `apply` and `redo` + accept `--allow-checksum-mismatch` (library: `Migrator::allow_checksum_mismatch`) to apply + despite the drift. Programmatic migrations and rows with a null recorded checksum are not + checked +- Repeatable migrations: a migration whose up-SQL carries the `-- migrant:repeatable` directive + re-runs whenever its checksum changes, instead of applying exactly once. They run after all + pending versioned migrations, keep a single bookkeeping row updated in place, and are never + reverted by `apply --down` or `redo`. `new --repeatable ` creates one (an `up.sql` seeded + with the directive, and no `down.sql`) +- `apply` and `redo` accept `--rerun-repeatable` to re-run every repeatable migration even if + its SQL has not changed, since editing the file is otherwise the only trigger +- `redo` prints a note naming the applied repeatable migrations it will not revert, since it + targets the most recent versioned migration instead +- `status` reports whether each migration is repeatable and whether it is stale (due to re-run), + with a `stale` summary count. Text output annotates repeatable rows and marks stale ones `[~]`; + the JSON rows gain `repeatable` and `stale` fields - `apply` accepts `--step N` to apply exactly N migrations, in either direction - `apply` and `redo` accept `--allow-unknown-tags` to permit a run when the database has an applied tag not present in the defined migration set, and `--allow-out-of-order` to permit @@ -12,11 +29,14 @@ - `apply` applies all pending migrations by default, instead of just the next one. Use `--step 1`, or `--down` (which remains single-step by default), to move one migration at a time +- A migration directory no longer needs a `down.sql`. A migration with no down file is a no-op + in the down direction: reverting it removes its bookkeeping row without running SQL - The `__migrant_migrations` bookkeeping table is now multi-column (`id`, `tag`, `checksum`, - `applied_at`) instead of a single `tag` column. Each applied migration now records a sha256 - checksum of its up-SQL (null for programmatic migrations) and an applied-at timestamp, and - applied order is tracked by `id` rather than inferred from file/tag order. `redo` and - `apply --down` now target the most-recently-applied migration by this recorded order. + `applied_at`, `is_repeatable`) instead of a single `tag` column. Each applied migration now + records a sha256 checksum of its up-SQL (null for programmatic migrations), an applied-at + timestamp, and whether it is repeatable, and applied order is tracked by `id` rather than + inferred from file/tag order. `redo` and `apply --down` now target the most-recently-applied + migration by this recorded order. This is a one-time breaking change to the on-disk schema. If you don't need to preserve applied history, the simplest upgrade is to drop the existing `__migrant_migrations` table @@ -28,16 +48,19 @@ alter table __migrant_migrations add column id bigserial; alter table __migrant_migrations add column checksum text; alter table __migrant_migrations add column applied_at timestamptz not null default now(); + alter table __migrant_migrations add column is_repeatable boolean not null default false; -- sqlite alter table __migrant_migrations add column id integer; alter table __migrant_migrations add column checksum text; alter table __migrant_migrations add column applied_at text not null default (datetime('now')); + alter table __migrant_migrations add column is_repeatable boolean not null default false; -- mysql alter table __migrant_migrations add column id bigint unsigned auto_increment unique; alter table __migrant_migrations add column checksum text; alter table __migrant_migrations add column applied_at timestamp not null default current_timestamp; + alter table __migrant_migrations add column is_repeatable boolean not null default false; ``` Since applied order was not previously tracked, `id` will not necessarily reflect the diff --git a/README.md b/README.md index 23c07bc..ede208f 100644 --- a/README.md +++ b/README.md @@ -69,17 +69,17 @@ When run interactively (without `--no-confirm`), `setup` will be run automatical `migrant setup` - Verify database info/credentials and setup a `__migrant_migrations` table if missing. -`migrant new ` - Generate new up & down files with the given `` under the specified `migration_location`. +`migrant new [--repeatable]` - Generate new up & down files with the given `` under the specified `migration_location`. `--repeatable` generates only an `up.sql`, carrying the `-- migrant:repeatable` directive, for a migration that re-runs whenever its SQL changes. `migrant edit [--down]` - Edit the `up` [or `down`] migration file with the given ``. `migrant list` - Display all available .sql files and mark those applied. -`migrant status [--format ]` - Report every managed migration's applied/pending state with summary counts, as pretty text (default) or JSON. +`migrant status [--format ]` - Report every managed migration's applied/pending state with summary counts, as pretty text (default) or JSON. Repeatable migrations are annotated, and marked stale when due to re-run. -`migrant apply [--down, --step N, --force, --fake, --no-sync]` - Apply all pending migrations. `--down` reverses direction and applies a single step by default; `--step N` applies exactly N steps in either direction. +`migrant apply [--down, --step N, --force, --fake, --no-sync, --rerun-repeatable, --allow-unknown-tags, --allow-out-of-order, --allow-checksum-mismatch]` - Apply all pending migrations. `--down` reverses direction and applies a single step by default; `--step N` applies exactly N steps in either direction. `--rerun-repeatable` re-runs every repeatable migration even if its SQL is unchanged. The `--allow-*` flags each bypass one otherwise-fatal consistency check. -`migrant redo [--all, --force, --no-sync]` - Re-apply the latest migration (down then up). +`migrant redo [--all, --force, --no-sync, --rerun-repeatable]` - Re-apply the latest migration (down then up). Repeatable migrations are forward-only, so `redo` does not revert them and says so. `migrant tui` - Open an interactive terminal UI for viewing and applying migrations. diff --git a/docs/src/cli.md b/docs/src/cli.md index ac0557d..c804945 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -25,9 +25,12 @@ run from anywhere inside the project; migrant searches upward for the config. ## Migrations -`migrant new ` +`migrant new [--repeatable]` : Generate a timestamped `_/` directory with empty `up.sql` and - `down.sql`. Tags may contain `[a-z0-9-]`. + `down.sql`. Tags may contain `[a-z0-9-]`. `--repeatable` instead writes only an + `up.sql`, seeded with the `-- migrant:repeatable` directive, for a migration + that re-runs whenever its SQL changes (see + [Writing migrations](migrations.md)). `migrant edit [--down]` : Open the `up.sql` (or `down.sql` with `--down`) for a migration matching @@ -38,25 +41,38 @@ run from anywhere inside the project; migrant searches upward for the config. `migrant status [--format ]` : Report every managed migration with its applied/pending state and summary - counts. `--format text` (the default) prints a summary line plus a `[✓]`/`[ ]` - row per migration; `--format json` prints the same data as JSON - (`{ total, applied, pending, migrations: [{ tag, applied }] }`) for scripting. - -`migrant apply [--down] [--all] [--force[=]] [--fake] [--no-sync]` -: Apply the next migration. `--down` reverts instead of applying. `--all` runs - every remaining migration in the chosen direction. `--force` continues past a - failed migration: bare `--force` (or `--force=accept-failures`) records the - failed migration as applied anyway, so it is not retried on later runs; - `--force=skip-failures` leaves it unrecorded and retries it on the next run. - `--fake` records the migration as (un)applied without running its SQL. - `--no-sync` disables the cross-process advisory lock that is otherwise on by - default for PostgreSQL/MySQL; use it when migrations are already serialized - by an external mechanism. - -`migrant redo [--all] [--force[=]] [--no-sync]` + counts. `--format text` (the default) prints a summary line plus a + `[✓]`/`[ ]`/`[~]` row per migration; `--format json` prints the same data as + JSON + (`{ total, applied, pending, stale, migrations: [{ tag, applied, repeatable, stale }] }`) + for scripting. A repeatable migration is annotated, and marked `[~]` when it is + due to re-run. The summary `stale` count covers applied migrations that will + re-run; migrations with no row yet are counted in `pending`. + +`migrant apply [--down] [--step ] [--force[=]] [--fake] [--no-sync] [--rerun-repeatable] [--allow-unknown-tags] [--allow-out-of-order] [--allow-checksum-mismatch]` +: Apply every pending migration. `--down` reverts instead of applying, and + defaults to a single migration. `--step N` limits either direction to N. + `--force` continues past a failed migration: bare `--force` (or + `--force=accept-failures`) records the failed migration as applied anyway, so + it is not retried on later runs; `--force=skip-failures` leaves it unrecorded + and retries it on the next run. `--fake` records the migration as (un)applied + without running its SQL. `--no-sync` disables the cross-process advisory lock + that is otherwise on by default for PostgreSQL/MySQL; use it when migrations + are already serialized by an external mechanism. `--rerun-repeatable` re-runs + every repeatable migration even if its SQL is unchanged. The three `--allow-*` flags + each bypass one otherwise-fatal consistency check: an applied tag missing from + the migration set, a migration applied out of order, and an already-applied + migration whose SQL has changed since it was recorded. + +`migrant redo [--all] [--force[=]] [--no-sync] [--rerun-repeatable] [--allow-unknown-tags] [--allow-out-of-order] [--allow-checksum-mismatch]` : Shortcut for the latest `down` then `up`. Useful while iterating on a migration - you are still writing. `--no-sync` disables the advisory lock for both the - down and up runs. + you are still writing. `--all` redoes every applied migration. `--no-sync` + disables the advisory lock for both the down and up runs. Repeatable + migrations are forward-only, so the down phase skips them and targets the most + recent versioned migration instead; the up phase then re-runs a repeatable + migration only if its SQL changed, or unconditionally with + `--rerun-repeatable`. `redo` prints a note naming the repeatable migrations it + will not revert. ## Inspect and connect diff --git a/docs/src/migration-types.md b/docs/src/migration-types.md index 38addde..f2fff3f 100644 --- a/docs/src/migration-types.md +++ b/docs/src/migration-types.md @@ -60,6 +60,35 @@ FnMigration::with_tag("seed-users") # } ``` +## Repeatable migrations + +`Migratable::is_repeatable()` marks a migration that re-runs whenever its up-SQL +checksum changes, instead of applying exactly once. The default is `false`. + +`FileMigration` and `EmbeddedMigration` declare it with the `repeatable()` +builder method, or with a `-- migrant:repeatable` directive in the up-SQL (the +form the CLI reads off disk). Either declares it. + +```rust +use migrant_lib::EmbeddedMigration; + +# fn run() { +EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('admin') on conflict do nothing;") + .boxed(); +# } +``` + +A repeatable migration must have up-SQL to hash and must not define a down +direction; `use_migrations` rejects either with `Error::Migration`. `FnMigration` +has no SQL to hash, so it cannot be repeatable. + +Within a run they apply after every pending versioned migration, at most once +each. `Report::repeatable_tags()` lists the ones a run re-ran, and +`MigrationStatus::repeatable()`/`stale()` report the state of each. See +[Writing migrations](migrations.md) for the full rules. + ## Transactions per migration `Migratable::use_transaction(direction)` decides whether migrant wraps a diff --git a/docs/src/migrations.md b/docs/src/migrations.md index 7dbfc6d..cb2f40c 100644 --- a/docs/src/migrations.md +++ b/docs/src/migrations.md @@ -1,7 +1,7 @@ # Writing migrations -A CLI migration is a directory holding an `up.sql` and a `down.sql`, named with a -timestamp and a tag: +A CLI migration is a directory holding an `up.sql` and, optionally, a +`down.sql`, named with a timestamp and a tag: ``` migrations/ @@ -22,6 +22,9 @@ way up, newest-first on the way down. Tags may contain `[a-z0-9-]`. `up.sql` moves the schema forward; `down.sql` reverses it. Keep them inverses so `apply --down` cleanly undoes `apply`. +`down.sql` is optional. A migration with no down file is a no-op in the down +direction: reverting it removes its tracking-table row without running any SQL. + ```sql -- 20260714101500_add-users-email/up.sql alter table users add column email text; @@ -47,8 +50,8 @@ Current Migration Status: -> [ ] 20260714101500_add-users-email ``` -`apply` runs the next unapplied migration in timestamp order; `apply --all` runs -the rest. `apply --down` reverts the most recently applied one. +`apply` runs every pending migration in timestamp order; `apply --step N` limits +a run to N. `apply --down` reverts the most recently applied one. ## Editing and iterating @@ -56,6 +59,49 @@ the rest. `apply --down` reverts the most recently applied one. - `migrant redo` re-runs the latest migration (down then up) so you can iterate on SQL you are still writing. +Editing a migration that has already been applied is drift: the next run aborts +with a checksum mismatch rather than silently building on changed SQL. Either +revert the edit and write a new migration, or pass `--allow-checksum-mismatch`. +Repeatable migrations invert this, see below. + +## Repeatable migrations + +A repeatable migration re-runs whenever its `up.sql` changes, instead of applying +exactly once. Use them for idempotent data work (seeding, backfills, refreshing +views) rather than schema versioning. + +`migrant new --repeatable ` creates one: an `up.sql` carrying the directive, +and no `down.sql`. + +```sql +-- migrant:repeatable +insert into roles (name) values ('admin') on conflict do nothing; +``` + +The rules: + +- They run after every pending versioned migration in a run, in timestamp order + among themselves, and at most once per run. +- A checksum change is the signal to re-run, not drift, so editing the file is + how you make it run again. An unchanged file is skipped. +- They keep one row in the tracking table, updated in place. +- They are forward-only: they must not have a `down.sql`, and `apply --down` + never reverts them. `redo` reverts and re-applies the most recent *versioned* + migration, which may not be the one you just edited; its up phase then re-runs + a repeatable migration only if its SQL changed, like any other run. `redo` + prints a note when it skips one. +- To run one whose SQL has not changed, pass `--rerun-repeatable` to `apply` or + `redo`. It re-runs every repeatable migration, still after the versioned ones + and still at most once per run. + +`migrant status` marks one that is due to re-run: + +``` +Migration status: 2 applied, 0 pending, 1 stale (2 total) + [✓] 20260713094500_create-roles + [~] 20260714101500_seed-roles (repeatable, will re-run) +``` + ## Non-transactional DDL Some statements cannot run inside a transaction (for example PostgreSQL diff --git a/migrant_lib/CHANGELOG.md b/migrant_lib/CHANGELOG.md index dda9950..098990b 100644 --- a/migrant_lib/CHANGELOG.md +++ b/migrant_lib/CHANGELOG.md @@ -8,10 +8,36 @@ has an applied tag not present in the defined migration set, instead of erroring - `Migrator::allow_out_of_order(bool)` (default `false`) lets a run apply migrations out of their defined order, instead of erroring +- `Migrator::allow_checksum_mismatch(bool)` (default `false`) lets a run proceed when an + already-applied migration's current checksum no longer matches the recorded one, instead of + erroring with the new `Error::ChecksumMismatch` +- `Error::ChecksumMismatch` and its `is_checksum_mismatch()` predicate +- Repeatable migrations: `Migratable::is_repeatable()` (default `false`) marks a migration that + re-runs whenever its up-SQL checksum changes instead of applying once. `EmbeddedMigration` and + `FileMigration` declare it with a `repeatable()` builder method or a `-- migrant:repeatable` + directive in their up-SQL (either form declares it). Repeatable migrations run after all + pending versioned ones, are exempt from the unknown-tag, ordering, and drift checks, keep a + single bookkeeping row updated in place, and are never selected by a `Down` run. Registering + one with no checksum, or with a down direction, is an `Error::Migration` +- `Migratable::defines_down()` (default `false`) reports whether a migration has a down direction + to run, used to reject a down on a repeatable migration +- `Migrator::rerun_repeatable(bool)` (default `false`) re-runs every repeatable migration on an + `Up` run regardless of checksum, so re-running an unedited one does not require touching its + SQL. Still at most once per run, and it never re-applies a versioned migration +- `Report::repeatable_tags()` lists the repeatable tags a run re-ran, a subset of `tags()` +- `MigrationStatus::repeatable()` and `MigrationStatus::stale()` report whether a migration is + repeatable and whether it will run on the next `Up` run +- `create_repeatable_migration` creates a migration directory with only an `up.sql`, seeded with + the `-- migrant:repeatable` directive ### Changed - `migrant_lib::new` is renamed to `migrant_lib::create_migration` and now returns a - `NewMigration` (with `dir()`/`up_path()`/`down_path()` accessors) instead of `()` + `NewMigration` (with `dir()`/`up_path()`/`down_path()` accessors) instead of `()`. + `NewMigration::down_path()` returns `Option<&Path>`, since a repeatable migration has none +- A file-discovered migration no longer requires a `down.sql`. A migration with no down file + reports `defines_down() == false` and is a no-op in the down direction +- `pending_migrations` now lists stale repeatable tags after the pending versioned ones, in the + order a run would apply them - `migrant_lib::list` moved to `migrant_lib::cli::list` - `SqliteSettingsBuilder`/`PostgresSettingsBuilder`/`MySqlSettingsBuilder` setters `database_path`/`migration_location` are now infallible: they take and return `Self` instead diff --git a/migrant_lib/src/config/mod.rs b/migrant_lib/src/config/mod.rs index 3476cec..d5da900 100644 --- a/migrant_lib/src/config/mod.rs +++ b/migrant_lib/src/config/mod.rs @@ -1,7 +1,7 @@ /*! Configuration */ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::env; use std::fs; use std::path::{Path, PathBuf}; @@ -10,10 +10,10 @@ use std::sync::{Arc, Mutex, PoisonError}; use log::{debug, error}; -use crate::drivers::DbConnection; +use crate::drivers::{AppliedRecord, DbConnection}; use crate::errors::*; use crate::macros::{bail, err}; -use crate::migratable::Migratable; +use crate::migratable::{validate_migrations, Migratable}; use crate::{tags, DbKind, SQLITE_MEMORY_PATH}; mod builders; @@ -38,6 +38,15 @@ pub struct Config { pub(crate) settings: Settings, pub(crate) settings_path: Option, pub(crate) applied: Vec, + /// Checksum recorded per applied tag (`None` where the column is NULL), + /// loaded alongside `applied`. Used to detect drift of an already-applied + /// migration whose current checksum no longer matches what was recorded, + /// and to decide whether a repeatable migration needs to re-run. + pub(crate) recorded_checksums: HashMap>, + /// Applied tags whose row is marked `is_repeatable`, loaded alongside + /// `applied`. Lets the migrator recognize a recorded repeatable migration + /// even when its tag is no longer in the available set. + pub(crate) recorded_repeatable: HashSet, pub(crate) migrations: Option>>, pub(crate) cli_compatible: bool, conn: Arc>>, @@ -56,6 +65,8 @@ impl Config { settings, settings_path, applied: vec![], + recorded_checksums: HashMap::new(), + recorded_repeatable: HashSet::new(), migrations: None, cli_compatible: false, conn: Arc::new(Mutex::new(None)), @@ -297,6 +308,10 @@ impl Config { bail!(TagError, "Tags must be unique. Found duplicate: {}", tag) } } + // A repeatable migration needs a checksum and must not define a down + // direction; reject an unusable declaration here rather than at apply + // time. + validate_migrations(migrations)?; self.migrations = Some(migrations.to_vec()); Ok(self) } @@ -394,7 +409,7 @@ impl Config { }; config.cli_compatible = self.cli_compatible; config.migrations = self.migrations.clone(); - config.applied = config.load_applied()?; + config.refresh_applied()?; Ok(config) } @@ -403,16 +418,24 @@ impl Config { /// `Config::reload`). Used by the migrator so a run stays on the /// connection its advisory lock was acquired on. pub(crate) fn refresh_applied(&mut self) -> Result<()> { - self.applied = self.load_applied()?; + let records = self.load_applied_records()?; + self.applied = records.iter().map(|r| r.tag.clone()).collect(); + self.recorded_repeatable = records + .iter() + .filter(|r| r.repeatable) + .map(|r| r.tag.clone()) + .collect(); + self.recorded_checksums = records.into_iter().map(|r| (r.tag, r.checksum)).collect(); Ok(()) } /// Load the applied migrations from the database migration table. /// - /// The tags are returned in recorded application order (`order by id`), + /// The records are returned in recorded application order (`order by id`), /// which is authoritative -- no re-sorting is done. Each tag is still - /// validated against the active naming rules. - pub(crate) fn load_applied(&self) -> Result> { + /// validated against the active naming rules. The checksum is `None` where + /// the column is NULL (programmatic migrations, or legacy rows). + pub(crate) fn load_applied_records(&self) -> Result> { if !self.migration_table_exists()? { bail!( Migration, @@ -420,11 +443,11 @@ impl Config { ) } - let applied = self.with_conn(|conn| conn.applied_tags())?; - for tag in &applied { - self.check_saved_tag(tag)?; + let records = self.with_conn(|conn| conn.applied_records())?; + for record in &records { + self.check_saved_tag(&record.tag)?; } - Ok(applied) + Ok(records) } /// Check if a `__migrant_migrations` table exists @@ -432,10 +455,22 @@ impl Config { self.with_conn(|conn| conn.migration_table_exists()) } - /// Insert given tag (and its optional checksum) into the database migration - /// table. `applied_at` is populated by the column default. - pub(crate) fn insert_migration_tag(&self, tag: &str, checksum: Option<&str>) -> Result<()> { - self.with_conn(|conn| conn.insert_tag(tag, checksum)) + /// Insert given tag (its optional checksum, and whether it is repeatable) + /// into the database migration table. `applied_at` is populated by the + /// column default. + pub(crate) fn insert_migration_tag( + &self, + tag: &str, + checksum: Option<&str>, + repeatable: bool, + ) -> Result<()> { + self.with_conn(|conn| conn.insert_tag(tag, checksum, repeatable)) + } + + /// Update an already-recorded tag's checksum and `applied_at` in place, + /// used when a repeatable migration re-runs. + pub(crate) fn update_migration_tag(&self, tag: &str, checksum: Option<&str>) -> Result<()> { + self.with_conn(|conn| conn.update_tag(tag, checksum)) } /// Remove a given tag from the database migration table diff --git a/migrant_lib/src/drivers/mod.rs b/migrant_lib/src/drivers/mod.rs index e5c3297..f17c722 100644 --- a/migrant_lib/src/drivers/mod.rs +++ b/migrant_lib/src/drivers/mod.rs @@ -17,27 +17,64 @@ pub(crate) mod sql { // The bookkeeping table carries, besides the migration `tag`: a surrogate // `id` whose ascending order is the authoritative recorded application // order, an optional `checksum` (lowercase hex sha256 of the raw up-direction - // SQL, NULL for programmatic migrations that have no SQL to hash), and an - // `applied_at` timestamp populated by the column default. - pub static PG_CREATE_TABLE: &str = "create table __migrant_migrations(id serial primary key, tag text unique not null, checksum text, applied_at timestamptz not null default now());"; - pub static SQLITE_CREATE_TABLE: &str = "create table __migrant_migrations(id integer primary key autoincrement, tag text unique not null, checksum text, applied_at timestamp not null default current_timestamp);"; - pub static MYSQL_CREATE_TABLE: &str = "create table __migrant_migrations(id integer primary key auto_increment, tag varchar(512) unique not null, checksum text null, applied_at timestamp not null default current_timestamp);"; + // SQL, NULL for programmatic migrations that have no SQL to hash), an + // `applied_at` timestamp populated by the column default, and `is_repeatable` + // marking rows that record a repeatable migration (re-run on every checksum + // change). The column is named `is_repeatable` rather than `repeatable` + // because the latter is a keyword on both postgres and mysql. + pub static PG_CREATE_TABLE: &str = "create table __migrant_migrations(id serial primary key, tag text unique not null, checksum text, applied_at timestamptz not null default now(), is_repeatable boolean not null default false);"; + // Sqlite uses `0`/`1` rather than the `false`/`true` keywords, which its + // parser only gained in 3.23.0. `migrant_lib` links whatever libsqlite3 the + // consumer provides (its rusqlite dependency is not `bundled`), so the + // portable spelling is the safe one. + pub static SQLITE_CREATE_TABLE: &str = "create table __migrant_migrations(id integer primary key autoincrement, tag text unique not null, checksum text, applied_at timestamp not null default current_timestamp, is_repeatable boolean not null default 0);"; + pub static MYSQL_CREATE_TABLE: &str = "create table __migrant_migrations(id integer primary key auto_increment, tag varchar(512) unique not null, checksum text null, applied_at timestamp not null default current_timestamp, is_repeatable boolean not null default false);"; // Recorded application order is authoritative, so order by the surrogate id. - pub static GET_MIGRATIONS: &str = "select tag from __migrant_migrations order by id;"; + // Each row carries the tag, its recorded checksum (NULL for programmatic + // migrations), and whether it records a repeatable migration. Checksum drift + // of an already-applied versioned migration is detected by comparing the + // recorded checksum against the migration's current one; for a repeatable + // row the same difference is instead the signal to re-run. + pub static GET_MIGRATIONS: &str = + "select tag, checksum, is_repeatable from __migrant_migrations order by id;"; pub static INSERT_MIGRATION_PG_SQLITE: &str = - "insert into __migrant_migrations (tag, checksum) values ($1, $2)"; + "insert into __migrant_migrations (tag, checksum, is_repeatable) values ($1, $2, $3)"; pub static REMOVE_MIGRATION_PG_SQLITE: &str = "delete from __migrant_migrations where tag = $1"; pub static INSERT_MIGRATION_MYSQL: &str = - "insert into __migrant_migrations (tag, checksum) values (?, ?)"; + "insert into __migrant_migrations (tag, checksum, is_repeatable) values (?, ?, ?)"; pub static REMOVE_MIGRATION_MYSQL: &str = "delete from __migrant_migrations where tag = ?"; + // A repeatable migration keeps one row, updated in place on each re-run, so + // its `id` (and therefore recorded application order) is preserved. + // `applied_at` is refreshed explicitly because the column default only + // applies on insert. `is_repeatable` is set rather than left alone so a row + // first recorded for a versioned migration becomes correctly marked once + // that migration is declared repeatable; only a repeatable re-run takes + // this path. + pub static UPDATE_MIGRATION_PG: &str = "update __migrant_migrations set checksum = $1, applied_at = now(), is_repeatable = true where tag = $2"; + pub static UPDATE_MIGRATION_SQLITE: &str = "update __migrant_migrations set checksum = $1, applied_at = current_timestamp, is_repeatable = 1 where tag = $2"; + pub static UPDATE_MIGRATION_MYSQL: &str = "update __migrant_migrations set checksum = ?, applied_at = current_timestamp, is_repeatable = true where tag = ?"; + pub static SQLITE_MIGRATION_TABLE_EXISTS: &str = "select exists(select 1 from sqlite_master where type = 'table' and name = '__migrant_migrations');"; pub static PG_MIGRATION_TABLE_EXISTS: &str = "select exists(select 1 from pg_tables where tablename = '__migrant_migrations');"; pub static MYSQL_MIGRATION_TABLE_EXISTS: &str = "select exists(select 1 from information_schema.tables where table_name='__migrant_migrations' and table_schema = database()) as tag;"; } +/// One row of the `__migrant_migrations` bookkeeping table, in recorded +/// application order. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct AppliedRecord { + /// The recorded migration tag + pub(crate) tag: String, + /// The checksum recorded when the migration was applied, `None` where the + /// column is NULL (programmatic migrations, or legacy rows) + pub(crate) checksum: Option, + /// Whether the row records a repeatable migration + pub(crate) repeatable: bool, +} + #[cfg(feature = "mysql")] pub(crate) mod mysql; #[cfg(feature = "postgres")] @@ -141,15 +178,29 @@ impl DbConnection { dispatch!(self, c => c.setup_migration_table()) } - /// Select all applied migration tags - pub(crate) fn applied_tags(&mut self) -> Result> { - dispatch!(self, c => c.applied_tags()) + /// Select all applied migrations as [`AppliedRecord`]s, in recorded + /// application order. + pub(crate) fn applied_records(&mut self) -> Result> { + dispatch!(self, c => c.applied_records()) } - /// Record a migration tag as applied, along with its optional checksum - /// (`applied_at` is populated by the column default) - pub(crate) fn insert_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { - dispatch!(self, c => c.insert_tag(tag, checksum)) + /// Record a migration tag as applied, along with its optional checksum and + /// whether it is repeatable (`applied_at` is populated by the column + /// default) + pub(crate) fn insert_tag( + &mut self, + tag: &str, + checksum: Option<&str>, + repeatable: bool, + ) -> Result<()> { + dispatch!(self, c => c.insert_tag(tag, checksum, repeatable)) + } + + /// Update an already-recorded tag's checksum and `applied_at` in place, and + /// mark it repeatable. Used when a repeatable migration re-runs, so the row + /// keeps its `id` and recorded application order is unchanged. + pub(crate) fn update_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { + dispatch!(self, c => c.update_tag(tag, checksum)) } /// Remove a migration tag from the applied set @@ -209,37 +260,75 @@ mod tests { } /// Every backend's create-table statement must carry the `tag`, `checksum`, - /// and `applied_at` columns, and applied tags must be selected in recorded - /// order (`order by id`) so the recorded application order is authoritative. + /// `applied_at`, and `is_repeatable` columns, and applied tags must be + /// selected in recorded order (`order by id`) so the recorded application + /// order is authoritative. #[test] - fn create_table_statements_carry_checksum_and_applied_at() { + fn create_table_statements_carry_every_bookkeeping_column() { for (name, ddl) in [ ("pg", sql::PG_CREATE_TABLE), ("sqlite", sql::SQLITE_CREATE_TABLE), ("mysql", sql::MYSQL_CREATE_TABLE), + ] { + for column in ["tag", "checksum", "applied_at", "is_repeatable"] { + assert!( + ddl.contains(column), + "{name} ddl must have a {column} column: {ddl}" + ); + } + } + assert!( + sql::GET_MIGRATIONS.contains("order by id"), + "GET_MIGRATIONS must order by id: {}", + sql::GET_MIGRATIONS + ); + for column in ["tag", "checksum", "is_repeatable"] { + assert!( + sql::GET_MIGRATIONS.contains(column), + "GET_MIGRATIONS must select {column}: {}", + sql::GET_MIGRATIONS + ); + assert!( + sql::INSERT_MIGRATION_PG_SQLITE.contains(column) + && sql::INSERT_MIGRATION_MYSQL.contains(column), + "inserts must carry the {column} column" + ); + } + } + + /// REPEAT-6: every backend's in-place update must refresh `checksum` and + /// `applied_at`, mark the row repeatable, and be scoped to one tag. Without + /// the `where tag` clause it would rewrite the whole table. + #[test] + fn update_statements_refresh_the_row_in_place_for_one_tag() { + for (name, update) in [ + ("pg", sql::UPDATE_MIGRATION_PG), + ("sqlite", sql::UPDATE_MIGRATION_SQLITE), + ("mysql", sql::UPDATE_MIGRATION_MYSQL), ] { assert!( - ddl.contains("tag"), - "{name} ddl must have a tag column: {ddl}" + update.contains("checksum ="), + "{name} update must set checksum: {update}" ); assert!( - ddl.contains("checksum"), - "{name} ddl must have a checksum column: {ddl}" + update.contains("applied_at ="), + "{name} update must refresh applied_at: {update}" ); assert!( - ddl.contains("applied_at"), - "{name} ddl must have an applied_at column: {ddl}" + update.contains("is_repeatable ="), + "{name} update must mark the row repeatable: {update}" + ); + assert!( + update.contains("where tag ="), + "{name} update must be scoped to a single tag: {update}" ); } + // The sqlite parser only gained the `true`/`false` keywords in 3.23.0, + // and `migrant_lib` links the consumer's libsqlite3. assert!( - sql::GET_MIGRATIONS.contains("order by id"), - "GET_MIGRATIONS must order by id: {}", - sql::GET_MIGRATIONS - ); - assert!( - sql::INSERT_MIGRATION_PG_SQLITE.contains("checksum") - && sql::INSERT_MIGRATION_MYSQL.contains("checksum"), - "inserts must carry the checksum column" + !sql::SQLITE_CREATE_TABLE.contains("false") + && !sql::UPDATE_MIGRATION_SQLITE.contains("true"), + "sqlite statements must use 0/1 rather than the boolean keywords" ); } } diff --git a/migrant_lib/src/drivers/mysql.rs b/migrant_lib/src/drivers/mysql.rs index 8d138f8..54de7eb 100644 --- a/migrant_lib/src/drivers/mysql.rs +++ b/migrant_lib/src/drivers/mysql.rs @@ -3,7 +3,7 @@ MySQL driver */ use mysql::{prelude::Queryable, Conn, Opts}; -use super::sql; +use super::{sql, AppliedRecord}; use crate::errors::*; use crate::macros::{bail, err}; @@ -47,13 +47,34 @@ impl MySqlConn { Ok(true) } - pub(crate) fn applied_tags(&mut self) -> Result> { - Ok(self.conn.query(sql::GET_MIGRATIONS)?) + pub(crate) fn applied_records(&mut self) -> Result> { + // `is_repeatable` is a mysql `boolean`, i.e. `tinyint(1)`, which the + // driver decodes into `bool`. + let rows: Vec<(String, Option, bool)> = self.conn.query(sql::GET_MIGRATIONS)?; + Ok(rows + .into_iter() + .map(|(tag, checksum, repeatable)| AppliedRecord { + tag, + checksum, + repeatable, + }) + .collect()) } - pub(crate) fn insert_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { + pub(crate) fn insert_tag( + &mut self, + tag: &str, + checksum: Option<&str>, + repeatable: bool, + ) -> Result<()> { self.conn - .exec_drop(sql::INSERT_MIGRATION_MYSQL, (tag, checksum))?; + .exec_drop(sql::INSERT_MIGRATION_MYSQL, (tag, checksum, repeatable))?; + Ok(()) + } + + pub(crate) fn update_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { + self.conn + .exec_drop(sql::UPDATE_MIGRATION_MYSQL, (checksum, tag))?; Ok(()) } @@ -118,6 +139,14 @@ impl MySqlConn { mod tests { use super::*; + fn record(tag: &str, checksum: Option<&str>, repeatable: bool) -> AppliedRecord { + AppliedRecord { + tag: tag.to_string(), + checksum: checksum.map(str::to_string), + repeatable, + } + } + /// Requires a running mysql instance; set `MYSQL_TEST_CONN_STR` /// (e.g. `mysql://user:pass@localhost/db`) to run #[test] @@ -143,24 +172,20 @@ mod tests { assert!(!conn.setup_migration_table().unwrap(), "setup idempotent"); assert!(conn.migration_table_exists().unwrap(), "table exists"); - conn.insert_tag("initial", Some("abc123")).unwrap(); - conn.insert_tag("alter1", None).unwrap(); - conn.insert_tag("alter2", Some("def456")).unwrap(); - // Recorded order is authoritative: tags come back in insertion (id) order. + conn.insert_tag("initial", Some("abc123"), false).unwrap(); + conn.insert_tag("alter1", None, false).unwrap(); + conn.insert_tag("alter2", Some("def456"), true).unwrap(); + // Recorded order is authoritative: records come back in insertion (id) + // order, each carrying its tag, checksum (NULL where None), and kind. assert_eq!( - vec!["initial", "alter1", "alter2"], - conn.applied_tags().unwrap() + vec![ + record("initial", Some("abc123"), false), + record("alter1", None, false), + record("alter2", Some("def456"), true), + ], + conn.applied_records().unwrap() ); - // The checksum column carries the inserted value (NULL where None). - let checksums: Vec> = conn - .conn - .query("select checksum from __migrant_migrations order by id") - .unwrap(); - assert_eq!( - vec![Some("abc123".to_string()), None, Some("def456".to_string())], - checksums - ); // `applied_at` is populated by the column default. let stamped: Option = conn .conn @@ -168,12 +193,27 @@ mod tests { .unwrap(); assert_eq!(Some(3), stamped); + // REPEAT-6: a repeatable row is updated in place, keeping its id. + let id_before: Option = conn + .conn + .query_first("select id from __migrant_migrations where tag = 'alter2'") + .unwrap(); + conn.update_tag("alter2", Some("ghi789")).unwrap(); + let records = conn.applied_records().unwrap(); + assert_eq!(3, records.len(), "an update must not insert another row"); + assert_eq!(record("alter2", Some("ghi789"), true), records[2]); + let id_after: Option = conn + .conn + .query_first("select id from __migrant_migrations where tag = 'alter2'") + .unwrap(); + assert_eq!(id_before, id_after, "the row keeps its recorded order"); + conn.remove_tag("alter2").unwrap(); - assert_eq!(2, conn.applied_tags().unwrap().len()); + assert_eq!(2, conn.applied_records().unwrap().len()); conn.remove_tag("alter1").unwrap(); conn.remove_tag("initial").unwrap(); - assert_eq!(0, conn.applied_tags().unwrap().len()); + assert_eq!(0, conn.applied_records().unwrap().len()); conn.execute_batch("drop table __migrant_migrations;") .unwrap(); diff --git a/migrant_lib/src/drivers/pg.rs b/migrant_lib/src/drivers/pg.rs index e4352df..64c4720 100644 --- a/migrant_lib/src/drivers/pg.rs +++ b/migrant_lib/src/drivers/pg.rs @@ -5,7 +5,7 @@ use std::path::Path; use postgres::{Client, NoTls}; -use super::sql; +use super::{sql, AppliedRecord}; use crate::errors::*; use crate::macros::err; @@ -98,14 +98,34 @@ impl PgConn { Ok(true) } - pub(crate) fn applied_tags(&mut self) -> Result> { + pub(crate) fn applied_records(&mut self) -> Result> { let rows = self.client.query(sql::GET_MIGRATIONS, &[])?; - Ok(rows.iter().map(|row| row.get(0)).collect()) + Ok(rows + .iter() + .map(|row| AppliedRecord { + tag: row.get(0), + checksum: row.get(1), + repeatable: row.get(2), + }) + .collect()) + } + + pub(crate) fn insert_tag( + &mut self, + tag: &str, + checksum: Option<&str>, + repeatable: bool, + ) -> Result<()> { + self.client.execute( + sql::INSERT_MIGRATION_PG_SQLITE, + &[&tag, &checksum, &repeatable], + )?; + Ok(()) } - pub(crate) fn insert_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { + pub(crate) fn update_tag(&mut self, tag: &str, checksum: Option<&str>) -> Result<()> { self.client - .execute(sql::INSERT_MIGRATION_PG_SQLITE, &[&tag, &checksum])?; + .execute(sql::UPDATE_MIGRATION_PG, &[&checksum, &tag])?; Ok(()) } @@ -163,6 +183,14 @@ impl PgConn { mod tests { use super::*; + fn record(tag: &str, checksum: Option<&str>, repeatable: bool) -> AppliedRecord { + AppliedRecord { + tag: tag.to_string(), + checksum: checksum.map(str::to_string), + repeatable, + } + } + #[test] fn sslmode_value_parsing() { assert_eq!(sslmode_value("postgres://user:pass@localhost/db"), None); @@ -229,27 +257,20 @@ mod tests { assert!(!conn.setup_migration_table().unwrap(), "setup idempotent"); assert!(conn.migration_table_exists().unwrap(), "table exists"); - conn.insert_tag("initial", Some("abc123")).unwrap(); - conn.insert_tag("alter1", None).unwrap(); - conn.insert_tag("alter2", Some("def456")).unwrap(); - // Recorded order is authoritative: tags come back in insertion (id) order. + conn.insert_tag("initial", Some("abc123"), false).unwrap(); + conn.insert_tag("alter1", None, false).unwrap(); + conn.insert_tag("alter2", Some("def456"), true).unwrap(); + // Recorded order is authoritative: records come back in insertion (id) + // order, each carrying its tag, checksum (NULL where None), and kind. assert_eq!( - vec!["initial", "alter1", "alter2"], - conn.applied_tags().unwrap() + vec![ + record("initial", Some("abc123"), false), + record("alter1", None, false), + record("alter2", Some("def456"), true), + ], + conn.applied_records().unwrap() ); - // The checksum column carries the value we inserted (and NULL where None). - let checksums: Vec> = conn - .client - .query("select checksum from __migrant_migrations order by id", &[]) - .unwrap() - .iter() - .map(|row| row.get(0)) - .collect(); - assert_eq!( - vec![Some("abc123".to_string()), None, Some("def456".to_string())], - checksums - ); // `applied_at` is populated by the column default. let stamped: i64 = conn .client @@ -261,12 +282,35 @@ mod tests { .get(0); assert_eq!(3, stamped); + // REPEAT-6: a repeatable row is updated in place, keeping its id. + let id_before: i32 = conn + .client + .query_one( + "select id from __migrant_migrations where tag = 'alter2'", + &[], + ) + .unwrap() + .get(0); + conn.update_tag("alter2", Some("ghi789")).unwrap(); + let records = conn.applied_records().unwrap(); + assert_eq!(3, records.len(), "an update must not insert another row"); + assert_eq!(record("alter2", Some("ghi789"), true), records[2]); + let id_after: i32 = conn + .client + .query_one( + "select id from __migrant_migrations where tag = 'alter2'", + &[], + ) + .unwrap() + .get(0); + assert_eq!(id_before, id_after, "the row keeps its recorded order"); + conn.remove_tag("alter2").unwrap(); - assert_eq!(2, conn.applied_tags().unwrap().len()); + assert_eq!(2, conn.applied_records().unwrap().len()); conn.remove_tag("alter1").unwrap(); conn.remove_tag("initial").unwrap(); - assert_eq!(0, conn.applied_tags().unwrap().len()); + assert_eq!(0, conn.applied_records().unwrap().len()); conn.execute_batch("drop table __migrant_migrations;") .unwrap(); diff --git a/migrant_lib/src/drivers/sqlite.rs b/migrant_lib/src/drivers/sqlite.rs index 165327d..d95f0ac 100644 --- a/migrant_lib/src/drivers/sqlite.rs +++ b/migrant_lib/src/drivers/sqlite.rs @@ -8,7 +8,7 @@ use std::sync::{Arc, Mutex, MutexGuard}; use rusqlite::Connection; -use super::sql; +use super::{sql, AppliedRecord}; use crate::errors::*; use crate::macros::err; @@ -63,19 +63,38 @@ impl SqliteConn { Ok(true) } - pub(crate) fn applied_tags(&self) -> Result> { + pub(crate) fn applied_records(&self) -> Result> { let conn = self.lock(); let mut stmt = conn.prepare(sql::GET_MIGRATIONS)?; - let tags = stmt - .query_map([], |row| row.get(0))? - .collect::, _>>()?; - Ok(tags) + let records = stmt + .query_map([], |row| { + Ok(AppliedRecord { + tag: row.get(0)?, + checksum: row.get(1)?, + repeatable: row.get(2)?, + }) + })? + .collect::, _>>()?; + Ok(records) } - pub(crate) fn insert_tag(&self, tag: &str, checksum: Option<&str>) -> Result<()> { + pub(crate) fn insert_tag( + &self, + tag: &str, + checksum: Option<&str>, + repeatable: bool, + ) -> Result<()> { self.lock().execute( sql::INSERT_MIGRATION_PG_SQLITE, - rusqlite::params![tag, checksum], + rusqlite::params![tag, checksum, repeatable], + )?; + Ok(()) + } + + pub(crate) fn update_tag(&self, tag: &str, checksum: Option<&str>) -> Result<()> { + self.lock().execute( + sql::UPDATE_MIGRATION_SQLITE, + rusqlite::params![checksum, tag], )?; Ok(()) } @@ -134,6 +153,14 @@ impl SqliteConn { mod tests { use super::*; + fn record(tag: &str, checksum: Option<&str>, repeatable: bool) -> AppliedRecord { + AppliedRecord { + tag: tag.to_string(), + checksum: checksum.map(str::to_string), + repeatable, + } + } + #[test] fn migration_table_lifecycle() { let conn = SqliteConn::open(MEMORY_PATH).unwrap(); @@ -146,32 +173,20 @@ mod tests { assert!(!conn.setup_migration_table().unwrap(), "setup idempotent"); assert!(conn.migration_table_exists().unwrap(), "table exists"); - conn.insert_tag("initial", Some("abc123")).unwrap(); - conn.insert_tag("alter1", None).unwrap(); - conn.insert_tag("alter2", Some("def456")).unwrap(); - // Recorded order is authoritative: tags come back in insertion (id) order. + conn.insert_tag("initial", Some("abc123"), false).unwrap(); + conn.insert_tag("alter1", None, false).unwrap(); + conn.insert_tag("alter2", Some("def456"), false).unwrap(); + // Recorded order is authoritative: records come back in insertion (id) + // order, each carrying its tag and checksum (NULL where None). assert_eq!( - vec!["initial", "alter1", "alter2"], - conn.applied_tags().unwrap() + vec![ + record("initial", Some("abc123"), false), + record("alter1", None, false), + record("alter2", Some("def456"), false), + ], + conn.applied_records().unwrap() ); - // The checksum column carries the inserted value (NULL where None). - let checksums: Vec> = { - let guard = conn.lock(); - let mut stmt = guard - .prepare("select checksum from __migrant_migrations order by id") - .unwrap(); - let rows = stmt - .query_map([], |row| row.get::<_, Option>(0)) - .unwrap() - .collect::, _>>() - .unwrap(); - rows - }; - assert_eq!( - vec![Some("abc123".to_string()), None, Some("def456".to_string())], - checksums - ); // `applied_at` is populated by the column default. let stamped: i64 = conn .lock() @@ -184,11 +199,76 @@ mod tests { assert_eq!(3, stamped); conn.remove_tag("alter2").unwrap(); - assert_eq!(2, conn.applied_tags().unwrap().len()); + assert_eq!(2, conn.applied_records().unwrap().len()); conn.remove_tag("alter1").unwrap(); conn.remove_tag("initial").unwrap(); - assert_eq!(0, conn.applied_tags().unwrap().len()); + assert_eq!(0, conn.applied_records().unwrap().len()); + } + + // REPEAT-6 + #[test] + fn repeatable_rows_update_in_place_keeping_their_id() { + let conn = SqliteConn::open(MEMORY_PATH).unwrap(); + conn.setup_migration_table().unwrap(); + + conn.insert_tag("initial", Some("abc123"), false).unwrap(); + conn.insert_tag("seed-roles", Some("sum-v1"), true).unwrap(); + conn.insert_tag("later", Some("def456"), false).unwrap(); + + // The repeatable row records its kind. + assert_eq!( + vec![ + record("initial", Some("abc123"), false), + record("seed-roles", Some("sum-v1"), true), + record("later", Some("def456"), false), + ], + conn.applied_records().unwrap() + ); + + let id_before: i64 = conn + .lock() + .query_row( + "select id from __migrant_migrations where tag = 'seed-roles'", + [], + |row| row.get(0), + ) + .unwrap(); + + conn.update_tag("seed-roles", Some("sum-v2")).unwrap(); + + // One row still, with the new checksum, still marked repeatable, and + // still in the same position in recorded order. + assert_eq!( + vec![ + record("initial", Some("abc123"), false), + record("seed-roles", Some("sum-v2"), true), + record("later", Some("def456"), false), + ], + conn.applied_records().unwrap() + ); + let id_after: i64 = conn + .lock() + .query_row( + "select id from __migrant_migrations where tag = 'seed-roles'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!( + id_before, id_after, + "an in-place re-run must not change the row's id" + ); + + // A row first recorded for a versioned migration is marked repeatable + // by the update, so a migration converted from versioned to repeatable + // ends up with a truthful row rather than a stale `false`. + conn.update_tag("later", Some("ghi789")).unwrap(); + assert_eq!( + record("later", Some("ghi789"), true), + conn.applied_records().unwrap()[2], + "an in-place update marks the row repeatable" + ); } #[test] diff --git a/migrant_lib/src/errors.rs b/migrant_lib/src/errors.rs index c529402..f0734f8 100644 --- a/migrant_lib/src/errors.rs +++ b/migrant_lib/src/errors.rs @@ -26,6 +26,11 @@ pub enum Error { #[error("MigrationOrdering: {0}")] MigrationOrdering(String), + /// An already-applied migration's current checksum no longer matches the + /// checksum recorded when it was applied + #[error("ChecksumMismatch: {0}")] + ChecksumMismatch(String), + /// Failure while running an external command (editor, database shell) #[error("ShellCommandError: {0}")] ShellCommand(String), @@ -99,6 +104,11 @@ impl Error { matches!(self, Error::MigrationOrdering(_)) } + /// `true` for [`Error::ChecksumMismatch`] + pub fn is_checksum_mismatch(&self) -> bool { + matches!(self, Error::ChecksumMismatch(_)) + } + /// `true` for [`Error::ShellCommand`] pub fn is_shell_command(&self) -> bool { matches!(self, Error::ShellCommand(_)) @@ -129,6 +139,7 @@ mod tests { assert!(Error::TagError("dup".to_string()).is_tag_error()); assert!(Error::MigrationNotFound("x".to_string()).is_migration_not_found()); assert!(Error::MigrationOrdering("y".to_string()).is_migration_ordering()); + assert!(Error::ChecksumMismatch("z".to_string()).is_checksum_mismatch()); assert!(Error::FeatureRequired("sqlite").is_feature_required()); } diff --git a/migrant_lib/src/lib.rs b/migrant_lib/src/lib.rs index 8943e18..31825e8 100644 --- a/migrant_lib/src/lib.rs +++ b/migrant_lib/src/lib.rs @@ -80,6 +80,36 @@ config.use_migrations(&[ ``` +## Repeatable migrations + +A migration declared repeatable re-runs whenever its up-SQL checksum changes, instead of +applying exactly once. This suits idempotent data work (seeding, backfills, refreshing views) +rather than schema versioning, where a changed checksum is instead reported as drift. + +Declare one with the `repeatable()` builder method, or with a `-- migrant:repeatable` directive +on a comment line in the up-SQL (the form the `migrant` CLI reads off disk): + +```rust,no_run +# use migrant_lib::EmbeddedMigration; +EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('admin') on conflict do nothing;"); +``` + +Run semantics: + +- An `Up` run applies every pending versioned migration first, then re-runs the repeatable + migrations whose checksum changed (or that have never run), in definition order. +- A repeatable migration runs at most once per run. +- It keeps a single bookkeeping row, updated in place, and is exempt from the checksum-drift, + unknown-tag, and out-of-order checks. +- It is forward-only: a `Down` run never selects it and never removes its row, so `redo` never + reverts one (its `Up` phase re-runs one only if the checksum changed, like any other run). +- Changing the SQL is the usual re-run trigger; `Migrator::rerun_repeatable(true)` re-runs them + all regardless of checksum. +- Declaring one with no up-SQL to hash, or with a down direction, is an error. +- `fake(true)` records the new checksum without running the SQL, marking it up to date. + ## In-memory sqlite databases With the `sqlite` feature, the special database path `:memory:` selects an @@ -149,8 +179,8 @@ pub use crate::migratable::Migratable; pub use crate::migration::{noop, EmbeddedMigration, FileMigration, FnMigration}; pub use crate::migrator::{Direction, ForceMode, Migrator, Report}; pub use crate::ops::{ - create_migration, migration_statuses, pending_migrations, search_for_settings_file, - MigrationStatus, NewMigration, + create_migration, create_repeatable_migration, migration_statuses, pending_migrations, + search_for_settings_file, MigrationStatus, NewMigration, }; /// Interactive, terminal-oriented operations used by the `migrant` CLI. diff --git a/migrant_lib/src/migratable.rs b/migrant_lib/src/migratable.rs index 3d40b6e..1778e62 100644 --- a/migrant_lib/src/migratable.rs +++ b/migrant_lib/src/migratable.rs @@ -3,6 +3,8 @@ The `Migratable` trait */ use std::fmt; +use crate::errors::*; +use crate::macros::bail; use crate::migrator::Direction; use crate::Config; @@ -82,6 +84,82 @@ pub trait Migratable: MigratableClone { let _ = direction; true } + + /// Whether this migration re-runs its up-direction every time its + /// [`checksum`](Migratable::checksum) changes, instead of applying exactly + /// once. + /// + /// Defaults to `false` (a versioned migration). A repeatable migration is + /// re-run whenever its current checksum differs from the one recorded in + /// `__migrant_migrations`, which suits idempotent data work (seeding, + /// backfills, refreshing views) rather than one-time schema versioning. + /// [`EmbeddedMigration`](crate::EmbeddedMigration) and + /// [`FileMigration`](crate::FileMigration) declare it with their + /// `repeatable()` builder method or a `-- migrant:repeatable` directive in + /// their up-SQL. + /// + /// A repeatable migration must report a `checksum` (there is otherwise no + /// way to tell that it changed) and must not define a down direction; both + /// are rejected when the migration set is registered. + /// + /// Named `is_repeatable` because `EmbeddedMigration`/`FileMigration` expose + /// a `repeatable()` *builder* method, mirroring the + /// `no_transaction()`/[`use_transaction`](Migratable::use_transaction) + /// split. + fn is_repeatable(&self) -> bool { + false + } + + /// Whether this migration has a down direction to run. + /// + /// Defaults to `false`. This exists so a + /// [`repeatable`](Migratable::is_repeatable) migration that also defines a + /// down (which would never run, since a `Down` + /// run never selects a repeatable migration) is rejected rather than + /// silently ignored. Implementations that are never repeatable can leave it + /// at the default. + fn defines_down(&self) -> bool { + false + } +} + +/// Reject migration sets that declare a combination the migrator cannot honor. +/// +/// A repeatable migration must have a checksum to compare against the recorded +/// one, and must not define a down direction it would never run. Checked when +/// an explicit set is registered with +/// [`Config::use_migrations`](crate::Config::use_migrations) and again when the +/// migrator loads the available set, so file-discovered migrations declaring +/// `-- migrant:repeatable` are covered too. +pub(crate) fn validate_migrations(migrations: &[Box]) -> Result<()> { + for migration in migrations { + if !migration.is_repeatable() { + continue; + } + let tag = migration.tag(); + if migration.checksum().is_none() { + bail!( + Migration, + "Repeatable migration `{}` has no checksum, so a change to it \ + could never be detected. Check that its up-SQL is set and, for \ + a file migration, that the up file exists and is readable. \ + Programmatic migrations have no SQL to hash and cannot be \ + repeatable.", + tag + ) + } + if migration.defines_down() { + bail!( + Migration, + "Repeatable migration `{}` defines a down direction, which would \ + never run: repeatable migrations are forward-only. Remove the \ + down direction (for a file migration, delete its `down.sql`; an \ + empty file still counts), or drop the repeatable declaration.", + tag + ) + } + } + Ok(()) } impl Clone for Box { @@ -95,3 +173,90 @@ impl fmt::Debug for Box { write!(f, "Migration: {}", self.tag()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::migration::{noop, EmbeddedMigration, FnMigration}; + + // REPEAT-7 + #[test] + fn a_repeatable_migration_without_a_checksum_is_rejected() { + // A repeatable migration with no up-SQL has nothing to hash, so a + // change to it could never be detected. + let migrations = vec![EmbeddedMigration::with_tag("seed").repeatable().boxed()]; + match validate_migrations(&migrations) { + Err(Error::Migration(msg)) => { + assert!(msg.contains("seed"), "message should name the tag: {msg}"); + assert!( + msg.contains("checksum"), + "message should explain the missing checksum: {msg}" + ); + } + other => panic!("expected Error::Migration, got: {other:?}"), + } + } + + // REPEAT-7 + #[test] + fn a_programmatic_migration_cannot_be_repeatable() { + // `FnMigration` has no SQL to hash. It reports `is_repeatable() == + // false` by default, so the set is valid; a custom `Migratable` that + // returns `true` without a checksum is what the check catches. + #[derive(Clone)] + struct AlwaysRepeatable; + impl Migratable for AlwaysRepeatable { + fn tag(&self) -> String { + "custom".to_string() + } + fn is_repeatable(&self) -> bool { + true + } + } + + let programmatic = vec![FnMigration::with_tag("f").up(noop).down(noop).boxed()]; + validate_migrations(&programmatic).expect("a non-repeatable FnMigration is fine"); + + let custom: Vec> = vec![Box::new(AlwaysRepeatable)]; + assert!( + validate_migrations(&custom).is_err(), + "a checksum-less custom migration cannot be repeatable" + ); + } + + // REPEAT-4 + #[test] + fn a_repeatable_migration_with_a_down_is_rejected() { + let migrations = vec![EmbeddedMigration::with_tag("seed") + .repeatable() + .up("select 1;") + .down("select -1;") + .boxed()]; + match validate_migrations(&migrations) { + Err(Error::Migration(msg)) => { + assert!(msg.contains("seed"), "message should name the tag: {msg}"); + assert!( + msg.contains("down"), + "message should explain the down direction: {msg}" + ); + } + other => panic!("expected Error::Migration, got: {other:?}"), + } + } + + #[test] + fn valid_sets_pass() { + let migrations = vec![ + EmbeddedMigration::with_tag("a") + .up("select 1;") + .down("select -1;") + .boxed(), + // Repeatable: up-SQL to hash, and no down direction. + EmbeddedMigration::with_tag("seed") + .repeatable() + .up("select 2;") + .boxed(), + ]; + validate_migrations(&migrations).expect("a well-formed set must pass"); + } +} diff --git a/migrant_lib/src/migration.rs b/migrant_lib/src/migration.rs index 80e343e..617b334 100644 --- a/migrant_lib/src/migration.rs +++ b/migrant_lib/src/migration.rs @@ -25,6 +25,20 @@ use crate::DT_FORMAT; /// ``` pub(crate) const NO_TRANSACTION_DIRECTIVE: &str = "migrant:no-transaction"; +/// SQL comment directive that declares a migration repeatable: its up-direction +/// re-runs whenever its checksum changes, instead of applying exactly once. +/// +/// Placed on its own `--` comment line in the up-direction SQL, e.g: +/// +/// ```sql +/// -- migrant:repeatable +/// insert into roles (name) values ('admin') on conflict do nothing; +/// ``` +/// +/// The directive line is part of the up-SQL, so it is included in the migration's +/// checksum like any other byte. +pub(crate) const REPEATABLE_DIRECTIVE: &str = "migrant:repeatable"; + /// Compute the lowercase hex sha256 of the given raw bytes. Used to fingerprint /// a migration's up-direction SQL for the `checksum` bookkeeping column. pub(crate) fn sha256_hex(bytes: &[u8]) -> String { @@ -39,28 +53,37 @@ pub(crate) fn sha256_hex(bytes: &[u8]) -> String { out } -/// Return `true` if `sql` carries the [`NO_TRANSACTION_DIRECTIVE`] on a comment -/// line. It is matched case-insensitively as the first token of a `--` line -/// comment, so a trailing explanation is allowed -/// (`-- migrant:no-transaction (enum add)`). -pub(crate) fn sql_opts_out_of_transaction(sql: &str) -> bool { +/// Return `true` if `sql` carries `directive` on a comment line. It is matched +/// case-insensitively as the first token of a `--` line comment, so a trailing +/// explanation is allowed (`-- migrant:no-transaction (enum add)`). +pub(crate) fn sql_declares(sql: &str, directive: &str) -> bool { sql.lines().any(|line| { matches!( line.trim() .strip_prefix("--") .and_then(|rest| rest.split_whitespace().next()), - Some(token) if token.eq_ignore_ascii_case(NO_TRANSACTION_DIRECTIVE) + Some(token) if token.eq_ignore_ascii_case(directive) ) }) } -/// Whether the migration file at `path` declares the no-transaction directive. -/// A missing or unreadable file is treated as not opting out; the subsequent -/// apply surfaces any real read error. -fn file_opts_out_of_transaction(path: &Option) -> bool { +/// Return `true` if `sql` carries the [`NO_TRANSACTION_DIRECTIVE`]. +pub(crate) fn sql_opts_out_of_transaction(sql: &str) -> bool { + sql_declares(sql, NO_TRANSACTION_DIRECTIVE) +} + +/// Return `true` if `sql` carries the [`REPEATABLE_DIRECTIVE`]. +pub(crate) fn sql_declares_repeatable(sql: &str) -> bool { + sql_declares(sql, REPEATABLE_DIRECTIVE) +} + +/// Whether the migration file at `path` declares `directive`. A missing or +/// unreadable file is treated as not declaring it; the subsequent apply +/// surfaces any real read error. +fn file_declares(path: &Option, directive: &str) -> bool { match path { Some(p) => std::fs::read_to_string(p) - .map(|sql| sql_opts_out_of_transaction(&sql)) + .map(|sql| sql_declares(&sql, directive)) .unwrap_or(false), None => false, } @@ -81,6 +104,10 @@ fn file_opts_out_of_transaction(path: &Option) -> bool { /// or call [`no_transaction`](FileMigration::no_transaction) to opt out both /// directions. A file directive takes precedence over the builder flag, so it /// works for migrations discovered from disk by the `migrant` CLI. +/// +/// *Note:* The down direction is optional. A migration with no down file is a +/// silent no-op in that direction: reverting it removes its bookkeeping row +/// without running SQL. #[derive(Clone, Debug)] pub struct FileMigration { pub(crate) tag: String, @@ -88,6 +115,7 @@ pub struct FileMigration { pub(crate) down: Option, pub(crate) stamp: Option>, pub(crate) no_transaction: bool, + pub(crate) repeatable: bool, } impl FileMigration { @@ -99,9 +127,23 @@ impl FileMigration { down: None, stamp: None, no_transaction: false, + repeatable: false, } } + /// Declare this migration repeatable: its up file re-runs whenever the + /// file's checksum changes, instead of applying exactly once. + /// + /// Equivalent to putting the `-- migrant:repeatable` directive on a comment + /// line in the up file, which is the form the `migrant` CLI can pick up from + /// disk. A repeatable migration must have an up file and must not define a + /// down file; both are checked when the migration set is registered. See + /// [`Migratable::is_repeatable`]. + pub fn repeatable(mut self) -> Self { + self.repeatable = true; + self + } + /// Opt both of this migration's directions out of the migrator's automatic /// transaction wrapping. /// @@ -196,11 +238,23 @@ impl Migratable for FileMigration { }; // A directive in the migration file takes precedence over the // builder-level `no_transaction` flag. - if file_opts_out_of_transaction(file) { + if file_declares(file, NO_TRANSACTION_DIRECTIVE) { return false; } !self.no_transaction } + + fn is_repeatable(&self) -> bool { + // Either form declares it, so migrations discovered from disk (which + // have no builder call) can declare themselves repeatable. Unlike + // `no_transaction` there is no precedence: neither can un-declare the + // other. + file_declares(&self.up, REPEATABLE_DIRECTIVE) || self.repeatable + } + + fn defines_down(&self) -> bool { + self.down.is_some() + } } /// Define an embedded migration @@ -252,6 +306,7 @@ pub struct EmbeddedMigration { pub(crate) up: Option>, pub(crate) down: Option>, pub(crate) no_transaction: bool, + pub(crate) repeatable: bool, } impl EmbeddedMigration { @@ -262,9 +317,30 @@ impl EmbeddedMigration { up: None, down: None, no_transaction: false, + repeatable: false, } } + /// Declare this migration repeatable: its up-SQL re-runs whenever its + /// checksum changes, instead of applying exactly once. + /// + /// Equivalent to putting the `-- migrant:repeatable` directive on a comment + /// line in the up-SQL, which travels with an `include_str!`ed file. A + /// repeatable migration must have up-SQL and must not define a down + /// direction; both are checked when the migration set is registered. See + /// [`Migratable::is_repeatable`]. + /// + /// ```rust,no_run + /// # use migrant_lib::EmbeddedMigration; + /// EmbeddedMigration::with_tag("seed-roles") + /// .repeatable() + /// .up("insert into roles (name) values ('admin') on conflict do nothing;"); + /// ``` + pub fn repeatable(mut self) -> Self { + self.repeatable = true; + self + } + /// Opt both of this migration's directions out of the migrator's automatic /// transaction wrapping. /// @@ -341,6 +417,19 @@ impl Migratable for EmbeddedMigration { } !self.no_transaction } + + fn is_repeatable(&self) -> bool { + // Either form declares it; neither can un-declare the other. + let declared = match self.up { + Some(ref up) => sql_declares_repeatable(up), + None => false, + }; + declared || self.repeatable + } + + fn defines_down(&self) -> bool { + self.down.is_some() + } } /// No-op to use with `FnMigration` @@ -451,6 +540,10 @@ where fn use_transaction(&self, _direction: Direction) -> bool { false } + + fn defines_down(&self) -> bool { + self.down.is_some() + } } #[cfg(test)] @@ -573,6 +666,124 @@ mod tests { assert_eq!(None, no_up.checksum()); } + // REPEAT-3 + #[test] + fn detects_repeatable_directive() { + assert!(sql_declares_repeatable( + "-- migrant:repeatable\ninsert into roles values ('admin');" + )); + assert!(sql_declares_repeatable("--migrant:repeatable")); + assert!(sql_declares_repeatable("-- MIGRANT:Repeatable")); + assert!(sql_declares_repeatable( + "-- migrant:repeatable (re-seed roles)\nselect 1;" + )); + // The two directives are matched independently of each other. + assert!(!sql_declares_repeatable("-- migrant:no-transaction")); + assert!(!sql_opts_out_of_transaction("-- migrant:repeatable")); + // Same near-miss rules as the no-transaction directive. + assert!(!sql_declares_repeatable("-- migrant:repeatable-please")); + assert!(!sql_declares_repeatable("select 'migrant:repeatable';")); + assert!(!sql_declares_repeatable("select 1;")); + assert!(!sql_declares_repeatable("")); + } + + // REPEAT-3 + #[test] + fn embedded_declares_repeatable_by_builder_or_directive() { + // Neither: a plain versioned migration. + let versioned = EmbeddedMigration::with_tag("m").up("select 1;"); + assert!(!versioned.is_repeatable()); + + // Builder flag. + let built = EmbeddedMigration::with_tag("m") + .up("select 1;") + .repeatable(); + assert!(built.is_repeatable()); + + // Directive in the up-SQL, no builder flag. + let declared = EmbeddedMigration::with_tag("m").up("-- migrant:repeatable\nselect 1;"); + assert!(declared.is_repeatable()); + + // The directive is only read from the up direction: a down-SQL + // directive does not make the migration repeatable. + let down_only = EmbeddedMigration::with_tag("m") + .up("select 1;") + .down("-- migrant:repeatable\nselect 1;"); + assert!(!down_only.is_repeatable()); + } + + // REPEAT-3 + #[test] + fn file_declares_repeatable_by_builder_or_up_file_directive() { + let dir = tempfile::tempdir().unwrap(); + let plain = dir.path().join("plain.sql"); + let declared = dir.path().join("declared.sql"); + std::fs::write(&plain, b"select 1;").unwrap(); + std::fs::write(&declared, b"-- migrant:repeatable\nselect 1;").unwrap(); + + assert!(!FileMigration::with_tag("m").up(&plain).is_repeatable()); + assert!(FileMigration::with_tag("m") + .up(&plain) + .repeatable() + .is_repeatable()); + // The directive in the up file is what the CLI reads off disk. + assert!(FileMigration::with_tag("m").up(&declared).is_repeatable()); + // A directive in the down file is not consulted. + assert!(!FileMigration::with_tag("m") + .up(&plain) + .down(&declared) + .is_repeatable()); + } + + // REPEAT-4, REPEAT-7 + #[test] + fn defines_down_reports_the_down_direction() { + assert!(!EmbeddedMigration::with_tag("m") + .up("select 1;") + .defines_down()); + assert!(EmbeddedMigration::with_tag("m") + .up("select 1;") + .down("select -1;") + .defines_down()); + + let dir = tempfile::tempdir().unwrap(); + let up = dir.path().join("up.sql"); + let down = dir.path().join("down.sql"); + std::fs::write(&up, b"select 1;").unwrap(); + std::fs::write(&down, b"select -1;").unwrap(); + assert!(!FileMigration::with_tag("m").up(&up).defines_down()); + assert!(FileMigration::with_tag("m") + .up(&up) + .down(&down) + .defines_down()); + + // Programmatic migrations report their down function too, so the + // predicate means the same thing for every migration type. + type Noop = fn(ConnConfig) -> std::result::Result<(), Box>; + assert!(FnMigration::with_tag("f") + .up(noop) + .down(noop) + .defines_down()); + let no_down: FnMigration = FnMigration::with_tag("f").up(noop); + assert!(!no_down.defines_down()); + } + + // REPEAT-3 + #[test] + fn repeatable_directive_is_part_of_the_checksum() { + // The directive travels with the SQL, so adding it changes the + // migration's fingerprint like any other edit. + let plain = EmbeddedMigration::with_tag("m").up("select 1;"); + let declared = EmbeddedMigration::with_tag("m").up("-- migrant:repeatable\nselect 1;"); + assert_ne!(plain.checksum(), declared.checksum()); + + // The builder flag is not part of the SQL, so it does not. + let built = EmbeddedMigration::with_tag("m") + .up("select 1;") + .repeatable(); + assert_eq!(plain.checksum(), built.checksum()); + } + #[test] fn embedded_use_transaction_is_per_direction_and_directive_wins() { // `up` opts out via directive, `down` does not: transactionality differs diff --git a/migrant_lib/src/migrator.rs b/migrant_lib/src/migrator.rs index 13979c4..49bbce3 100644 --- a/migrant_lib/src/migrator.rs +++ b/migrant_lib/src/migrator.rs @@ -1,13 +1,13 @@ /*! Migration application */ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fmt; use crate::config::Config; use crate::errors::*; use crate::macros::bail; -use crate::migratable::Migratable; +use crate::migratable::{validate_migrations, Migratable}; use crate::ops; use crate::util::print_flush; use crate::DbKind; @@ -85,6 +85,7 @@ impl std::str::FromStr for ForceMode { pub struct Report { direction: Direction, tags: Vec, + repeatable_tags: Vec, } impl Report { @@ -92,6 +93,7 @@ impl Report { Self { direction, tags: Vec::new(), + repeatable_tags: Vec::new(), } } @@ -101,10 +103,29 @@ impl Report { } /// The migration tags whose bookkeeping this run changed, in order. + /// + /// A tag can appear because a repeatable migration was *re-run* rather than + /// applied for the first time, so this is not a count of newly-applied + /// migrations. Use [`Report::repeatable_tags`] to tell the two apart. pub fn tags(&self) -> &[String] { &self.tags } + /// The repeatable migration tags this run re-ran, in order. A subset of + /// [`Report::tags`]; always empty for a `Down` run, which never selects a + /// repeatable migration. + pub fn repeatable_tags(&self) -> &[String] { + &self.repeatable_tags + } + + /// Record a tag this run changed the bookkeeping of. + fn record(&mut self, tag: String, repeatable: bool) { + if repeatable { + self.repeatable_tags.push(tag.clone()); + } + self.tags.push(tag); + } + /// `true` if nothing ran (the database was already up to date, or fully /// reverted for a `Down` run). pub fn is_empty(&self) -> bool { @@ -120,14 +141,62 @@ impl Report { /// Outcome of attempting the next migration in a run. enum Step { /// A migration's bookkeeping was changed (applied/reverted, faked, or - /// force-recorded); carries its tag. - Applied(String), + /// force-recorded); carries its tag and whether it is repeatable. + Applied { tag: String, repeatable: bool }, /// A migration failed and was skipped under `ForceMode::SkipFailures`. Skipped, /// No further migration is available in this direction. Complete, } +/// Which `Up`-run consistency checks are bypassed on a run. Grouped so the +/// selection helpers stay within argument-count limits and pass the set as a +/// unit. +#[derive(Debug, Clone, Copy)] +struct StrictChecks { + allow_unknown_tags: bool, + allow_out_of_order: bool, + allow_checksum_mismatch: bool, +} + +/// The recorded bookkeeping state that migration selection and the `Up`-run +/// consistency checks are decided against, grouped so it passes as a unit. +#[derive(Debug, Clone, Copy)] +struct AppliedState<'a> { + /// Applied tags, in recorded application order + tags: &'a [String], + /// Checksum recorded per applied tag (`None` where the column is NULL) + checksums: &'a HashMap>, + /// Applied tags whose row is marked repeatable. Recognizes a recorded + /// repeatable migration even when its tag is no longer available. + repeatable: &'a HashSet, +} + +impl AppliedState<'_> { + fn contains(&self, tag: &str) -> bool { + self.tags.iter().any(|t| t == tag) + } + + /// Whether a repeatable migration needs to re-run: it has no row yet, or + /// its current checksum differs from the recorded one. + fn is_stale(&self, migration: &dyn Migratable) -> bool { + match self.checksums.get(&migration.tag()) { + None => true, + Some(recorded) => recorded.as_deref() != migration.checksum().as_deref(), + } + } +} + +/// Tags excluded from selection for the remainder of a run: those that failed +/// under `ForceMode::SkipFailures`, and the repeatable migrations already +/// re-run this run (a repeatable migration runs at most once per run, which +/// also keeps an `all` run from looping on one). +#[derive(Debug, Clone, Copy)] +struct RunExclusions<'a> { + skipped: &'a HashSet, + ran_repeatable: &'a HashSet, +} + /// Migration applicator /// /// By default each migration's SQL and its `__migrant_migrations` bookkeeping @@ -151,6 +220,8 @@ pub struct Migrator { synchronized: bool, allow_unknown_tags: bool, allow_out_of_order: bool, + allow_checksum_mismatch: bool, + rerun_repeatable: bool, } impl Migrator { @@ -166,6 +237,8 @@ impl Migrator { synchronized: true, allow_unknown_tags: false, allow_out_of_order: false, + allow_checksum_mismatch: false, + rerun_repeatable: false, } } @@ -253,6 +326,40 @@ impl Migrator { self } + /// Allow an already-applied migration whose current checksum no longer + /// matches the checksum recorded when it was applied. Default is `false`. + /// + /// By default an `Up` run aborts with [`Error::ChecksumMismatch`] before + /// applying anything if an already-applied migration's up-SQL has changed + /// since it was recorded -- the situation that arises when a migration file + /// is edited after it has run somewhere. The comparison only fires when both + /// the recorded and current checksums are present, so programmatic + /// migrations (which record no checksum) and legacy rows recorded before + /// checksums existed are never flagged. Set this to `true` to apply despite + /// such drift. Independent of `allow_unknown_tags` and `allow_out_of_order`. + pub fn allow_checksum_mismatch(mut self, allow: bool) -> Self { + self.allow_checksum_mismatch = allow; + self + } + + /// Re-run every repeatable migration on this `Up` run, whether or not its + /// checksum changed. Default is `false`. + /// + /// Normally a repeatable migration re-runs only when its up-SQL changed + /// (see [`Migratable::is_repeatable`](crate::Migratable::is_repeatable)), + /// so re-running an unedited one otherwise means touching its SQL. Set this + /// to `true` to run them all regardless, for example to re-seed after + /// restoring a database. Everything else is unchanged: they still run after + /// the pending versioned migrations, still at most once per run, and each + /// still records its checksum afterwards. + /// + /// Has no effect on a `Down` run, which never selects a repeatable + /// migration. + pub fn rerun_repeatable(mut self, rerun: bool) -> Self { + self.rerun_repeatable = rerun; + self + } + /// Apply migrations using the current configuration. /// /// Returns a [`Report`] of the migration tags whose bookkeeping this run @@ -296,12 +403,20 @@ impl Migrator { // Tags that failed under `ForceMode::SkipFailures`, excluded from // migration selection for the remainder of this run. let mut skipped = HashSet::new(); + // Repeatable tags already re-run this run. A repeatable migration runs + // at most once per run, so an `all` run cannot loop on one whose + // bookkeeping did not end up matching (a `fake` or force-recorded run + // still records, but this keeps the loop bounded regardless). + let mut ran_repeatable = HashSet::new(); let mut report = Report::new(self.direction); loop { self.check_lock_still_held(&config, lock_generation)?; - match self.apply_next(&config, &mut skipped, lock_generation)? { - Step::Applied(tag) => { - report.tags.push(tag); + match self.apply_next(&config, &mut skipped, &ran_repeatable, lock_generation)? { + Step::Applied { tag, repeatable } => { + if repeatable { + ran_repeatable.insert(tag.clone()); + } + report.record(tag, repeatable); if !self.all { return Ok(report); } @@ -339,7 +454,7 @@ impl Migrator { /// The set of migrations being managed: either those explicitly defined /// on the config, or file-migrations discovered under `migration_location` fn available_migrations(config: &Config) -> Result>> { - Ok(match config.migrations { + let migrations: Vec> = match config.migrations { Some(ref migrations) => migrations.clone(), None => { let location = config.migration_location()?; @@ -348,48 +463,81 @@ impl Migrator { .map(|fm| fm.boxed()) .collect() } - }) + }; + // Explicit sets are validated when registered, but file-discovered ones + // declare themselves repeatable through a directive in their SQL, so the + // same rules are enforced here. + validate_migrations(&migrations)?; + Ok(migrations) } /// Return the next available up or down migration, excluding any tags - /// skipped earlier in this run (`ForceMode::SkipFailures`). + /// skipped earlier in this run (`ForceMode::SkipFailures`) or already re-run + /// this run (repeatable migrations). /// /// For an `Up` run this first enforces the strictness checks (unknown applied - /// tags, out-of-order application) unless they have been opted out of. + /// tags, out-of-order application, checksum drift) unless they have been + /// opted out of. Pending versioned migrations are selected first, in + /// definition order; only once none remain are the stale repeatable + /// migrations selected, also in definition order. `rerun_repeatable` treats + /// every repeatable migration as stale. fn next_available<'a>( direction: Direction, available: &'a [Box], - applied: &[String], - skipped: &HashSet, - allow_unknown_tags: bool, - allow_out_of_order: bool, + state: AppliedState<'_>, + exclusions: RunExclusions<'_>, + checks: StrictChecks, + rerun_repeatable: bool, ) -> Result> { Ok(match direction { Direction::Up => { - Self::check_applied_consistency( - available, - applied, - skipped, - allow_unknown_tags, - allow_out_of_order, - )?; + Self::check_applied_consistency(available, state, exclusions.skipped, checks)?; + let pending = available.iter().find(|m| { + !m.is_repeatable() + && !state.contains(&m.tag()) + && !exclusions.skipped.contains(&m.tag()) + }); + if let Some(pending) = pending { + return Ok(Some(pending.as_ref())); + } + // Every pending versioned migration has been applied, so the + // repeatable migrations whose SQL changed (or that have never + // run) go next. `rerun_repeatable` takes them all. available .iter() - .find(|m| !applied.contains(&m.tag()) && !skipped.contains(&m.tag())) + .find(|m| { + m.is_repeatable() + && !exclusions.skipped.contains(&m.tag()) + && !exclusions.ran_repeatable.contains(&m.tag()) + && (rerun_repeatable || state.is_stale(m.as_ref())) + }) .map(AsRef::as_ref) } Direction::Down => { // Recorded application order is authoritative, so the most - // recently applied migration is `applied.last()`. Walk backwards, - // skipping tags that failed earlier this run, and return the - // corresponding available migration. A target tag absent from the - // available set is a hard error, matching the previous behavior. - for tag in applied.iter().rev() { - if skipped.contains(tag) { + // recently applied migration is the last applied tag. Walk + // backwards, skipping tags that failed earlier this run, and + // return the corresponding available migration. A target tag + // absent from the available set is a hard error, matching the + // previous behavior. + for tag in state.tags.iter().rev() { + if exclusions.skipped.contains(tag) { continue; } match available.iter().find(|m| &m.tag() == tag) { + // Repeatable migrations are forward-only: a `Down` run + // neither reverts them nor removes their bookkeeping + // row. The available set is authoritative on kind for a + // tag it still defines, so a migration converted back to + // versioned is reverted normally despite what its row + // recorded. + Some(m) if m.is_repeatable() => continue, Some(m) => return Ok(Some(m.as_ref())), + // Absent from the available set, so only the recorded + // row says what kind it was: a repeatable one is not + // part of the versioned sequence and is skipped rather + // than treated as a missing down target. + None if state.repeatable.contains(tag) => continue, None => bail!( MigrationNotFound, "Applied migration not found in available migrations: {}", @@ -408,16 +556,21 @@ impl Migrator { /// self-defeating. fn check_applied_consistency( available: &[Box], - applied: &[String], + state: AppliedState<'_>, skipped: &HashSet, - allow_unknown_tags: bool, - allow_out_of_order: bool, + checks: StrictChecks, ) -> Result<()> { - if !allow_unknown_tags { - for tag in applied { + if !checks.allow_unknown_tags { + for tag in state.tags { if skipped.contains(tag) { continue; } + // A repeatable tag is not part of the versioned sequence, so a + // recorded repeatable row is never an unknown versioned tag + // even once it is gone from the available set. + if state.repeatable.contains(tag) { + continue; + } if !available.iter().any(|m| &m.tag() == tag) { bail!( MigrationNotFound, @@ -428,7 +581,7 @@ impl Migrator { } } } - if !allow_out_of_order { + if !checks.allow_out_of_order { // Walk definition order tracking the first un-applied (and un-skipped) // migration. An applied migration appearing after it was applied out // of order. @@ -438,7 +591,12 @@ impl Migrator { if skipped.contains(&tag) { continue; } - if applied.contains(&tag) { + // Repeatable migrations run after the versioned sequence and + // re-run repeatedly, so they take no part in its ordering. + if m.is_repeatable() { + continue; + } + if state.contains(&tag) { if let Some(ref earlier) = first_unapplied { bail!( MigrationOrdering, @@ -454,6 +612,45 @@ impl Migrator { } } } + if !checks.allow_checksum_mismatch { + // Compare each already-applied migration's current checksum against + // the checksum recorded when it was applied. The checksum map is + // keyed by applied tag, so a lookup hit means the migration is both + // applied and available. The comparison only fires when both the + // recorded and current checksums are present -- a null on either + // side (programmatic migration, or a legacy/backfilled row) is not a + // mismatch. + for m in available { + let tag = m.tag(); + if skipped.contains(&tag) { + continue; + } + // For a repeatable migration a changed checksum is the signal to + // re-run, not drift. This walks the available migrations, which + // are authoritative on kind: a migration converted back to + // versioned is drift-checked again even though its recorded row + // still says repeatable. + if m.is_repeatable() { + continue; + } + if let Some(Some(recorded)) = state.checksums.get(&tag) { + if let Some(current) = m.checksum() { + if *recorded != current { + bail!( + ChecksumMismatch, + "Migration `{}` has changed since it was applied: recorded \ + checksum `{}` does not match its current checksum `{}`. Revert \ + the change, or pass `allow_checksum_mismatch(true)` to apply \ + despite the drift.", + tag, + recorded, + current + ) + } + } + } + } + } Ok(()) } @@ -462,33 +659,53 @@ impl Migrator { &self, config: &Config, skipped: &mut HashSet, + ran_repeatable: &HashSet, lock_generation: Option, ) -> Result { let migrations = Self::available_migrations(config)?; let next = match Self::next_available( self.direction, &migrations, - &config.applied, - skipped, - self.allow_unknown_tags, - self.allow_out_of_order, + AppliedState { + tags: &config.applied, + checksums: &config.recorded_checksums, + repeatable: &config.recorded_repeatable, + }, + RunExclusions { + skipped, + ran_repeatable, + }, + StrictChecks { + allow_unknown_tags: self.allow_unknown_tags, + allow_out_of_order: self.allow_out_of_order, + allow_checksum_mismatch: self.allow_checksum_mismatch, + }, + self.rerun_repeatable, )? { Some(next) => next, None => return Ok(Step::Complete), }; + let tag = next.tag(); + let repeatable = next.is_repeatable(); + // A repeatable migration that already has a bookkeeping row is being + // re-run rather than applied for the first time. + let verb = if repeatable && config.applied.contains(&tag) { + "Re-applying" + } else { + "Applying" + }; self.print(&format!( - "Applying[{}]: {}", + "{}[{}]: {}", + verb, self.direction, next.description(self.direction) )); - let tag = next.tag(); - if self.fake { self.println(" ✓ (fake)"); self.record_tag(config, next)?; - return Ok(Step::Applied(tag)); + return Ok(Step::Applied { tag, repeatable }); } // Wrap the migration's SQL and its bookkeeping row in one transaction so @@ -505,7 +722,7 @@ impl Migrator { config.commit_transaction()?; } self.println(" ✓"); - Ok(Step::Applied(tag)) + Ok(Step::Applied { tag, repeatable }) } Err(msg) => { if transactional { @@ -530,7 +747,7 @@ impl Migrator { // The transaction (if any) was rolled back, so this // bookkeeping row stands alone. self.record_tag(config, next)?; - Ok(Step::Applied(tag)) + Ok(Step::Applied { tag, repeatable }) } ForceMode::SkipFailures => { self.println(&format!( @@ -565,11 +782,22 @@ impl Migrator { /// Record the migration as applied (`Up`) or un-applied (`Down`) in the /// `__migrant_migrations` table. An `Up` record carries the migration's - /// checksum (`None` for programmatic migrations, stored as NULL). + /// checksum (`None` for programmatic migrations, stored as NULL) and + /// whether it is repeatable. + /// + /// A repeatable migration that already has a row is updated in place rather + /// than inserted again, so it keeps one row (and its recorded application + /// order) across re-runs. fn record_tag(&self, config: &Config, next: &dyn Migratable) -> Result<()> { let tag = next.tag(); + let checksum = next.checksum(); match self.direction { - Direction::Up => config.insert_migration_tag(&tag, next.checksum().as_deref()), + Direction::Up if next.is_repeatable() && config.applied.contains(&tag) => { + config.update_migration_tag(&tag, checksum.as_deref()) + } + Direction::Up => { + config.insert_migration_tag(&tag, checksum.as_deref(), next.is_repeatable()) + } Direction::Down => config.delete_migration_tag(&tag), } } @@ -632,14 +860,128 @@ mod tests { strs.iter().map(|s| (*s).to_owned()).collect() } - /// Strict selection with both opt-outs off -- the default the migrator uses. + /// Empty recorded-checksum map: tag-only embedded migrations record no + /// checksum, so the drift check is a no-op for these selection tests. + fn no_checksums() -> HashMap> { + HashMap::new() + } + + /// No applied row is marked repeatable. + fn no_repeatable() -> HashSet { + HashSet::new() + } + + /// All consistency checks enabled (no opt-outs) -- the migrator default. + fn all_checks() -> StrictChecks { + StrictChecks { + allow_unknown_tags: false, + allow_out_of_order: false, + allow_checksum_mismatch: false, + } + } + + /// Nothing excluded: no run-skipped tags and no repeatable migration run yet. + fn no_exclusions<'a>( + skipped: &'a HashSet, + ran_repeatable: &'a HashSet, + ) -> RunExclusions<'a> { + RunExclusions { + skipped, + ran_repeatable, + } + } + + /// Strict selection with all opt-outs off -- the default the migrator uses. fn next_strict<'a>( direction: Direction, available: &'a [Box], applied: &[String], skipped: &HashSet, ) -> Result> { - Migrator::next_available(direction, available, applied, skipped, false, false) + let checksums = no_checksums(); + let repeatable = no_repeatable(); + let ran = no_skips(); + Migrator::next_available( + direction, + available, + AppliedState { + tags: applied, + checksums: &checksums, + repeatable: &repeatable, + }, + no_exclusions(skipped, &ran), + all_checks(), + false, + ) + } + + /// Selection with a specific set of check opt-outs, nothing else excluded. + fn next_with_checks<'a>( + direction: Direction, + available: &'a [Box], + applied: &[String], + checks: StrictChecks, + ) -> Result> { + let checksums = no_checksums(); + let repeatable = no_repeatable(); + let skipped = no_skips(); + let ran = no_skips(); + Migrator::next_available( + direction, + available, + AppliedState { + tags: applied, + checksums: &checksums, + repeatable: &repeatable, + }, + no_exclusions(&skipped, &ran), + checks, + false, + ) + } + + /// Selection against a full recorded state, for the repeatable cases where + /// the recorded checksums decide what runs next. + fn next_with_state<'a>( + direction: Direction, + available: &'a [Box], + applied: &[String], + checksums: &HashMap>, + ran_repeatable: &HashSet, + ) -> Result> { + next_with_state_rerunning( + direction, + available, + applied, + checksums, + ran_repeatable, + false, + ) + } + + /// As `next_with_state`, with control over the `rerun_repeatable` override. + fn next_with_state_rerunning<'a>( + direction: Direction, + available: &'a [Box], + applied: &[String], + checksums: &HashMap>, + ran_repeatable: &HashSet, + rerun_repeatable: bool, + ) -> Result> { + let repeatable = no_repeatable(); + let skipped = no_skips(); + Migrator::next_available( + direction, + available, + AppliedState { + tags: applied, + checksums, + repeatable: &repeatable, + }, + no_exclusions(&skipped, ran_repeatable), + all_checks(), + rerun_repeatable, + ) } #[test] @@ -790,10 +1132,17 @@ mod tests { let applied = tags(&["a", "x"]); // With `allow_unknown_tags`, the unknown `x` is ignored and the next // available migration `b` is selected. - let next = - Migrator::next_available(Direction::Up, &avail, &applied, &no_skips(), true, false) - .unwrap() - .expect("expected an un-applied migration"); + let next = next_with_checks( + Direction::Up, + &avail, + &applied, + StrictChecks { + allow_unknown_tags: true, + ..all_checks() + }, + ) + .unwrap() + .expect("expected an un-applied migration"); assert_eq!(next.tag(), "b"); } @@ -823,10 +1172,17 @@ mod tests { let avail = available(&["a", "b", "c"]); let applied = tags(&["a", "c"]); // With `allow_out_of_order`, the intervening `b` is selected next. - let next = - Migrator::next_available(Direction::Up, &avail, &applied, &no_skips(), false, true) - .unwrap() - .expect("expected an un-applied migration"); + let next = next_with_checks( + Direction::Up, + &avail, + &applied, + StrictChecks { + allow_out_of_order: true, + ..all_checks() + }, + ) + .unwrap() + .expect("expected an un-applied migration"); assert_eq!(next.tag(), "b"); } @@ -869,7 +1225,15 @@ mod tests { // ordering check catches `c` applied ahead of the earlier migrations. let avail = available(&["a", "b", "c"]); let applied = tags(&["x", "c"]); - match Migrator::next_available(Direction::Up, &avail, &applied, &no_skips(), true, false) { + match next_with_checks( + Direction::Up, + &avail, + &applied, + StrictChecks { + allow_unknown_tags: true, + ..all_checks() + }, + ) { Err(Error::MigrationOrdering(_)) => {} other => panic!( "ordering check must remain active when only unknown tags are allowed, got: {:?}", @@ -885,7 +1249,15 @@ mod tests { // `MigrationNotFound` even with `allow_out_of_order`. let avail = available(&["a", "b"]); let applied = tags(&["a", "x"]); - match Migrator::next_available(Direction::Up, &avail, &applied, &no_skips(), false, true) { + match next_with_checks( + Direction::Up, + &avail, + &applied, + StrictChecks { + allow_out_of_order: true, + ..all_checks() + }, + ) { Err(Error::MigrationNotFound(_)) => {} other => panic!( "unknown-tag check must remain active when only ordering is allowed, got: {:?}", @@ -939,4 +1311,519 @@ mod tests { ), } } + + /// Available migrations carrying up-SQL, so each reports a real `checksum()`. + fn available_with_up(pairs: &[(&str, &str)]) -> Vec> { + pairs + .iter() + .map(|(tag, up)| EmbeddedMigration::with_tag(tag).up(up.to_string()).boxed()) + .collect() + } + + fn recorded(pairs: &[(&str, Option<&str>)]) -> HashMap> { + pairs + .iter() + .map(|(tag, sum)| ((*tag).to_owned(), sum.map(|s| s.to_owned()))) + .collect() + } + + /// Run the Up consistency checks with only the drift check able to fire. + fn check_drift( + available: &[Box], + applied: &[String], + recorded: &HashMap>, + allow_checksum_mismatch: bool, + ) -> Result<()> { + let repeatable = no_repeatable(); + Migrator::check_applied_consistency( + available, + AppliedState { + tags: applied, + checksums: recorded, + repeatable: &repeatable, + }, + &no_skips(), + StrictChecks { + allow_checksum_mismatch, + ..all_checks() + }, + ) + } + + #[test] + fn up_checksum_mismatch_aborts_by_default() { + let avail = available_with_up(&[("a", "select 1;")]); + let applied = tags(&["a"]); + // `a` was recorded with a different checksum than its current up-SQL. + let recorded = recorded(&[("a", Some("stale-checksum"))]); + match check_drift(&avail, &applied, &recorded, false) { + Err(Error::ChecksumMismatch(msg)) => { + assert!( + msg.contains("a"), + "message should name the drifted tag: {msg}" + ); + } + other => panic!("expected ChecksumMismatch, got: {other:?}"), + } + } + + #[test] + fn up_checksum_match_passes() { + let avail = available_with_up(&[("a", "select 1;")]); + let applied = tags(&["a"]); + // Record the migration's actual current checksum: no drift. + let current = avail[0].checksum().expect("embedded up-SQL has a checksum"); + let recorded = recorded(&[("a", Some(¤t))]); + check_drift(&avail, &applied, &recorded, false).expect("matching checksum must pass"); + } + + #[test] + fn up_null_recorded_checksum_skips_drift_check() { + // A recorded NULL checksum (programmatic migration, or a legacy row) is + // not comparable, so it is skipped rather than treated as a mismatch. + let avail = available_with_up(&[("a", "select 1;")]); + let applied = tags(&["a"]); + let recorded = recorded(&[("a", None)]); + check_drift(&avail, &applied, &recorded, false).expect("null recorded checksum skips"); + } + + #[test] + fn up_null_current_checksum_skips_drift_check() { + // A tag-only migration reports no current checksum, so there is nothing + // to compare against a recorded value: skipped, not a mismatch. + let avail = available(&["a"]); + let applied = tags(&["a"]); + let recorded = recorded(&[("a", Some("anything"))]); + check_drift(&avail, &applied, &recorded, false).expect("null current checksum skips"); + } + + #[test] + fn up_checksum_mismatch_allowed_when_opted_out() { + let avail = available_with_up(&[("a", "select 1;")]); + let applied = tags(&["a"]); + let recorded = recorded(&[("a", Some("stale-checksum"))]); + check_drift(&avail, &applied, &recorded, true) + .expect("allow_checksum_mismatch applies despite drift"); + } + + #[test] + fn up_checksum_drift_of_unapplied_migration_is_ignored() { + // Drift only concerns already-applied migrations. A recorded checksum for + // a tag that is not applied (absent from the applied set, so not in the + // recorded map) never fires the check. + let avail = available_with_up(&[("a", "select 1;"), ("b", "select 2;")]); + let applied = tags(&["a"]); + let current = avail[0].checksum().unwrap(); + let recorded = recorded(&[("a", Some(¤t))]); + // `b` is pending; its checksum is irrelevant to drift. + check_drift(&avail, &applied, &recorded, false).expect("pending migration is not checked"); + } + + /// A mixed available set: versioned migrations plus repeatable ones, + /// identified by the trailing `repeatable` flag of each entry. + fn available_mixed(entries: &[(&str, &str, bool)]) -> Vec> { + entries + .iter() + .map(|(tag, up, repeatable)| { + let m = EmbeddedMigration::with_tag(tag).up(up.to_string()); + if *repeatable { m.repeatable() } else { m }.boxed() + }) + .collect() + } + + /// The current checksum of the available migration with the given tag. + fn current_sum(available: &[Box], tag: &str) -> String { + available + .iter() + .find(|m| m.tag() == tag) + .expect("tag is available") + .checksum() + .expect("embedded up-SQL has a checksum") + } + + // REPEAT-5 + #[test] + fn up_runs_pending_versioned_migrations_before_repeatable_ones() { + // `seed` is repeatable and stale (never run), but the pending versioned + // `b` must still be selected first: repeatables run after the versioned + // sequence even when they come earlier in definition order. + let avail = available_mixed(&[ + ("seed", "insert into roles values ('admin');", true), + ("a", "select 1;", false), + ("b", "select 2;", false), + ]); + let applied = tags(&["a"]); + let checksums = recorded(&[("a", Some(¤t_sum(&avail, "a")))]); + let next = next_with_state(Direction::Up, &avail, &applied, &checksums, &no_skips()) + .unwrap() + .expect("expected a migration"); + assert_eq!(next.tag(), "b"); + } + + // REPEAT-1 + #[test] + fn up_runs_a_repeatable_migration_that_has_never_run() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + let applied = tags(&["a"]); + // Only `a` has a row; `seed` has never run, so it is stale. + let checksums = recorded(&[("a", Some(¤t_sum(&avail, "a")))]); + let next = next_with_state(Direction::Up, &avail, &applied, &checksums, &no_skips()) + .unwrap() + .expect("expected the repeatable migration"); + assert_eq!(next.tag(), "seed"); + } + + // REPEAT-1 + #[test] + fn up_reruns_a_repeatable_migration_whose_checksum_changed() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + let applied = tags(&["a", "seed"]); + // `seed` ran before with different SQL: its recorded checksum no longer + // matches, which is the signal to re-run it (not drift). + let checksums = recorded(&[ + ("a", Some(¤t_sum(&avail, "a"))), + ("seed", Some("checksum-of-the-old-sql")), + ]); + let next = next_with_state(Direction::Up, &avail, &applied, &checksums, &no_skips()) + .unwrap() + .expect("expected the repeatable migration to re-run"); + assert_eq!(next.tag(), "seed"); + } + + // REPEAT-1 + #[test] + fn up_skips_a_repeatable_migration_whose_checksum_is_unchanged() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + let applied = tags(&["a", "seed"]); + let checksums = recorded(&[ + ("a", Some(¤t_sum(&avail, "a"))), + ("seed", Some(¤t_sum(&avail, "seed"))), + ]); + let next = + next_with_state(Direction::Up, &avail, &applied, &checksums, &no_skips()).unwrap(); + assert!( + next.is_none(), + "an up-to-date repeatable migration must not re-run" + ); + } + + // REPEAT-12 + #[test] + fn rerun_repeatable_runs_an_unchanged_repeatable_migration() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + let applied = tags(&["a", "seed"]); + // Both checksums match, so nothing is stale. + let checksums = recorded(&[ + ("a", Some(¤t_sum(&avail, "a"))), + ("seed", Some(¤t_sum(&avail, "seed"))), + ]); + assert!( + next_with_state(Direction::Up, &avail, &applied, &checksums, &no_skips()) + .unwrap() + .is_none(), + "nothing is stale without the override" + ); + + let next = next_with_state_rerunning( + Direction::Up, + &avail, + &applied, + &checksums, + &no_skips(), + true, + ) + .unwrap() + .expect("the override re-runs it regardless of checksum"); + assert_eq!(next.tag(), "seed"); + } + + // REPEAT-12 + #[test] + fn rerun_repeatable_still_runs_each_at_most_once_and_leaves_versioned_alone() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + // `a` is not applied, so it is selected first even with the override: + // the override only makes repeatable migrations eligible, it does not + // reorder the run or re-apply versioned migrations. + let next = next_with_state_rerunning( + Direction::Up, + &avail, + &tags(&[]), + &no_checksums(), + &no_skips(), + true, + ) + .unwrap() + .expect("expected the pending versioned migration"); + assert_eq!(next.tag(), "a"); + + // An applied versioned migration is never re-selected by the override. + let applied = tags(&["a", "seed"]); + let checksums = recorded(&[ + ("a", Some(¤t_sum(&avail, "a"))), + ("seed", Some(¤t_sum(&avail, "seed"))), + ]); + // Once `seed` has run this run, the override does not select it again, + // so an `all` run still terminates. + let ran = skips(&["seed"]); + assert!( + next_with_state_rerunning(Direction::Up, &avail, &applied, &checksums, &ran, true) + .unwrap() + .is_none(), + "the override must not defeat the once-per-run guard" + ); + } + + // REPEAT-12 + #[test] + fn rerun_repeatable_has_no_effect_on_a_down_run() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + let applied = tags(&["a", "seed"]); + let next = next_with_state_rerunning( + Direction::Down, + &avail, + &applied, + &no_checksums(), + &no_skips(), + true, + ) + .unwrap() + .expect("expected a down migration"); + assert_eq!( + next.tag(), + "a", + "a Down run never selects a repeatable migration, override or not" + ); + } + + // REPEAT-1 + #[test] + fn up_runs_a_repeatable_migration_at_most_once_per_run() { + // Having re-run `seed` this run, it is excluded for the rest of the run + // even though the recorded checksum here still looks stale. This is what + // keeps an `all` run from looping on a single repeatable migration. + let avail = available_mixed(&[("seed", "select 2;", true)]); + let applied = tags(&["seed"]); + let checksums = recorded(&[("seed", Some("stale"))]); + let ran = skips(&["seed"]); + let next = next_with_state(Direction::Up, &avail, &applied, &checksums, &ran).unwrap(); + assert!(next.is_none(), "a repeatable migration runs once per run"); + } + + // REPEAT-5 + #[test] + fn up_runs_stale_repeatable_migrations_in_definition_order() { + let avail = + available_mixed(&[("seed-b", "select 2;", true), ("seed-a", "select 1;", true)]); + // Neither has ever run, so both are stale; definition order decides. + let next = next_with_state( + Direction::Up, + &avail, + &tags(&[]), + &no_checksums(), + &no_skips(), + ) + .unwrap() + .expect("expected a repeatable migration"); + assert_eq!(next.tag(), "seed-b"); + } + + // REPEAT-4 + #[test] + fn down_never_selects_a_repeatable_migration() { + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", true)]); + // `seed` was applied most recently, but a Down run walks past it to the + // versioned `a`. + let applied = tags(&["a", "seed"]); + let next = next_with_state( + Direction::Down, + &avail, + &applied, + &no_checksums(), + &no_skips(), + ) + .unwrap() + .expect("expected a down migration"); + assert_eq!(next.tag(), "a"); + } + + // REPEAT-4 + #[test] + fn down_with_only_repeatable_migrations_applied_returns_none() { + let avail = available_mixed(&[("seed", "select 2;", true)]); + let applied = tags(&["seed"]); + let next = next_with_state( + Direction::Down, + &avail, + &applied, + &no_checksums(), + &no_skips(), + ) + .unwrap(); + assert!( + next.is_none(), + "there is nothing to revert when only repeatable rows are recorded" + ); + } + + /// Run the Up consistency checks against a state that also carries the + /// applied tags recorded as repeatable. + fn check_with_recorded_repeatable( + available: &[Box], + applied: &[String], + checksums: &HashMap>, + recorded_repeatable: &HashSet, + ) -> Result<()> { + Migrator::check_applied_consistency( + available, + AppliedState { + tags: applied, + checksums, + repeatable: recorded_repeatable, + }, + &no_skips(), + all_checks(), + ) + } + + // REPEAT-2 + #[test] + fn repeatable_checksum_change_is_not_drift() { + // The same recorded-vs-current mismatch that aborts a versioned run is + // the expected re-run signal for a repeatable migration, so the drift + // check must not fire on it. + let avail = available_mixed(&[("seed", "select 2;", true)]); + let applied = tags(&["seed"]); + let checksums = recorded(&[("seed", Some("checksum-of-the-old-sql"))]); + check_with_recorded_repeatable(&avail, &applied, &checksums, &no_repeatable()) + .expect("a repeatable migration's changed checksum is not drift"); + } + + // REPEAT-5 + #[test] + fn repeatable_migrations_do_not_trigger_the_ordering_check() { + // `seed` is repeatable, applied, and sits before the un-applied `b` in + // definition order. For a versioned migration that is an out-of-order + // violation; a repeatable one takes no part in the versioned sequence. + let avail = available_mixed(&[ + ("a", "select 1;", false), + ("seed", "select 2;", true), + ("b", "select 3;", false), + ]); + let applied = tags(&["seed"]); + let checksums = recorded(&[("seed", Some(¤t_sum(&avail, "seed")))]); + check_with_recorded_repeatable(&avail, &applied, &checksums, &no_repeatable()) + .expect("a repeatable migration is not part of the versioned ordering"); + } + + // REPEAT-5 + #[test] + fn a_removed_repeatable_tag_is_not_an_unknown_tag() { + // `seed` was recorded as repeatable and has since been dropped from the + // available set. Its row is recognized by the `is_repeatable` column, so + // it is not reported as an unknown versioned tag. + let avail = available_mixed(&[("a", "select 1;", false)]); + let applied = tags(&["a", "seed"]); + let checksums = recorded(&[("a", Some(¤t_sum(&avail, "a")))]); + let recorded_repeatable = skips(&["seed"]); + check_with_recorded_repeatable(&avail, &applied, &checksums, &recorded_repeatable) + .expect("a recorded repeatable row is never an unknown versioned tag"); + + // A removed *versioned* tag is still an error, so the exemption is not + // blanket. + match check_with_recorded_repeatable(&avail, &applied, &checksums, &no_repeatable()) { + Err(Error::MigrationNotFound(_)) => {} + other => panic!("expected MigrationNotFound for a removed versioned tag: {other:?}"), + } + } + + // REPEAT-6 + #[test] + fn the_available_set_outranks_a_stale_recorded_kind() { + // A migration recorded as repeatable that is now declared versioned + // must be drift-checked again: the available set is authoritative on + // kind for a tag it still defines, so the recorded flag cannot become a + // one-way door that disables drift detection forever. + let avail = available_with_up(&[("a", "select 1;")]); + let applied = tags(&["a"]); + let recorded = recorded(&[("a", Some("checksum-from-when-it-was-repeatable"))]); + let still_marked_repeatable = skips(&["a"]); + match check_with_recorded_repeatable(&avail, &applied, &recorded, &still_marked_repeatable) + { + Err(Error::ChecksumMismatch(_)) => {} + other => panic!("a now-versioned migration must be drift-checked: {other:?}"), + } + } + + // REPEAT-4, REPEAT-6 + #[test] + fn down_reverts_a_migration_converted_back_to_versioned() { + // Mirror case for selection: the row still says repeatable, but the + // available set now declares it versioned, so Down targets it. + let avail = available_mixed(&[("a", "select 1;", false), ("seed", "select 2;", false)]); + let applied = tags(&["a", "seed"]); + let checksums = no_checksums(); + let still_marked_repeatable = skips(&["seed"]); + let skipped = no_skips(); + let ran = no_skips(); + let next = Migrator::next_available( + Direction::Down, + &avail, + AppliedState { + tags: &applied, + checksums: &checksums, + repeatable: &still_marked_repeatable, + }, + no_exclusions(&skipped, &ran), + all_checks(), + false, + ) + .unwrap() + .expect("expected a down migration"); + assert_eq!( + next.tag(), + "seed", + "the available set decides kind for a tag it still defines" + ); + } + + // REPEAT-5 + #[test] + fn down_skips_a_removed_repeatable_tag_instead_of_erroring() { + // The tag is gone from the available set, so only its recorded row says + // what it was. A repeatable one is skipped rather than raising + // MigrationNotFound for a down target that never existed. + let avail = available_mixed(&[("a", "select 1;", false)]); + let applied = tags(&["a", "seed"]); + let checksums = no_checksums(); + let recorded_repeatable = skips(&["seed"]); + let skipped = no_skips(); + let ran = no_skips(); + let next = Migrator::next_available( + Direction::Down, + &avail, + AppliedState { + tags: &applied, + checksums: &checksums, + repeatable: &recorded_repeatable, + }, + no_exclusions(&skipped, &ran), + all_checks(), + false, + ) + .unwrap() + .expect("expected a down migration"); + assert_eq!(next.tag(), "a"); + } + + // REPEAT-8 + #[test] + fn report_separates_repeatable_tags_from_the_full_tag_list() { + let mut report = Report::new(Direction::Up); + report.record("a".to_string(), false); + report.record("seed".to_string(), true); + assert_eq!(report.tags(), ["a", "seed"]); + assert_eq!(report.repeatable_tags(), ["seed"]); + assert_eq!(report.len(), 2); + assert!(!report.is_empty()); + } } diff --git a/migrant_lib/src/ops.rs b/migrant_lib/src/ops.rs index 7d0b70f..60d030c 100644 --- a/migrant_lib/src/ops.rs +++ b/migrant_lib/src/ops.rs @@ -16,7 +16,7 @@ use walkdir::WalkDir; use crate::config::{Config, DbSettings}; use crate::errors::*; use crate::macros::{bail, err}; -use crate::migratable::Migratable; +use crate::migratable::{validate_migrations, Migratable}; use crate::migration::FileMigration; use crate::migrator::Direction; use crate::util::{open_file_in_fg, prompt}; @@ -39,7 +39,9 @@ pub fn search_for_settings_file>(base: T) -> Option { /// Search for available migrations in the given migration directory /// /// Migration directories are expected to be named `<14-digit-timestamp>_` -/// and contain `up.sql` / `down.sql` files. +/// and contain an `up.sql` file. A `down.sql` is optional: a migration without +/// one is a no-op in the down direction (reverting it removes its bookkeeping +/// row without running SQL), which is the shape repeatable migrations require. /// /// Intended only for use with `FileMigration`s not managed directly in source /// with `Config::use_migrations`. @@ -98,19 +100,15 @@ pub(crate) fn search_for_migrations(mig_root: &Path) -> Result bool { self.applied } + + /// Whether the migration is repeatable: it re-runs whenever its up-SQL + /// checksum changes, instead of applying exactly once. See + /// [`Migratable::is_repeatable`](crate::Migratable::is_repeatable). + pub fn repeatable(&self) -> bool { + self.repeatable + } + + /// Whether the migration would run on the next `Up` run: a versioned + /// migration that is not yet applied, or a repeatable migration whose + /// current checksum differs from the recorded one (or that has never run). + pub fn stale(&self) -> bool { + self.stale + } } /// Return the status of all migrations being managed: either those explicitly @@ -151,35 +170,63 @@ impl MigrationStatus { /// Make sure the `Config` has been `reload`ed so its set of applied /// migrations is current. pub fn migration_statuses(config: &Config) -> Result> { - let available = match config.migrations { - Some(ref migs) => migs.iter().map(|m| m.tag()).collect::>(), + let available: Vec> = match config.migrations { + Some(ref migs) => migs.clone(), None => { let location = config.migration_location()?; search_for_migrations(&location)? .into_iter() - .map(|m| m.tag()) + .map(|m| m.boxed()) .collect() } }; + // Report the same errors a run would, so `status`/`list` never render an + // unusable migration set as healthy and leave `apply` to be the one that + // fails. + validate_migrations(&available)?; Ok(available .into_iter() - .map(|tag| { + .map(|m| { + let tag = m.tag(); let applied = config.applied.contains(&tag); - MigrationStatus { tag, applied } + let repeatable = m.is_repeatable(); + // A repeatable migration is due whenever its recorded checksum no + // longer matches its current one (or it has never run); a versioned + // one is due only until it is applied. + let stale = if repeatable { + match config.recorded_checksums.get(&tag) { + None => true, + Some(recorded) => recorded.as_deref() != m.checksum().as_deref(), + } + } else { + !applied + }; + MigrationStatus { + tag, + applied, + repeatable, + stale, + } }) .collect()) } -/// Preview the managed migrations that have not yet been applied, in the order -/// they would be applied (definition order for explicit migrations, timestamp -/// order for file migrations). +/// Preview the managed migrations that would run on the next `Up` run, in the +/// order they would be applied: pending versioned migrations first (definition +/// order for explicit migrations, timestamp order for file migrations), then +/// the stale repeatable migrations. /// /// This does not apply anything. Make sure the `Config` has been `reload`ed so /// its set of applied migrations is current. pub fn pending_migrations(config: &Config) -> Result> { - Ok(migration_statuses(config)? + let statuses = migration_statuses(config)?; + let (repeatable, versioned): (Vec<_>, Vec<_>) = statuses + .into_iter() + .filter(|status| status.stale) + .partition(|status| status.repeatable); + Ok(versioned .into_iter() - .filter(|status| !status.applied) + .chain(repeatable) .map(|status| status.tag) .collect()) } @@ -201,20 +248,44 @@ pub fn list(config: &Config) -> Result<()> { println!("Current Migration Status:"); for mig in &statuses { println!( - " -> [{x}] {name}", - x = if mig.applied { '✓' } else { ' ' }, - name = mig.tag + " -> [{x}] {name}{note}", + x = mark(mig), + name = mig.tag, + note = note(mig) ); } Ok(()) } -/// The migration directory and files created by [`create_migration`]. +/// The status mark for a migration row: applied, pending, or a repeatable +/// migration that is due to re-run. +fn mark(status: &MigrationStatus) -> char { + match (status.applied, status.stale) { + (true, true) => '~', + (true, false) => '✓', + (false, _) => ' ', + } +} + +/// The trailing annotation for a migration row. Empty for versioned migrations. +/// A repeatable migration that has never run "will run"; only one with a +/// recorded row it no longer matches "will re-run". +fn note(status: &MigrationStatus) -> &'static str { + match (status.repeatable, status.stale, status.applied) { + (false, _, _) => "", + (true, true, true) => " (repeatable, will re-run)", + (true, true, false) => " (repeatable, will run)", + (true, false, _) => " (repeatable)", + } +} + +/// The migration directory and files created by [`create_migration`] or +/// [`create_repeatable_migration`]. #[derive(Debug, Clone)] pub struct NewMigration { dir: PathBuf, up: PathBuf, - down: PathBuf, + down: Option, } impl NewMigration { @@ -228,9 +299,10 @@ impl NewMigration { &self.up } - /// The created `down.sql` file path - pub fn down_path(&self) -> &Path { - &self.down + /// The created `down.sql` file path, `None` for a repeatable migration + /// (which is forward-only and has no down direction) + pub fn down_path(&self) -> Option<&Path> { + self.down.as_deref() } } @@ -243,6 +315,39 @@ impl NewMigration { /// where migrations (`FileMigration`s) are all files with names following /// the expected timestamp formatted name. pub fn create_migration(config: &Config, tag: &str) -> Result { + let (mig_dir, up) = create_migration_dir(config, tag)?; + let down = mig_dir.join("down.sql"); + fs::File::create(&down)?; + Ok(NewMigration { + dir: mig_dir, + up, + down: Some(down), + }) +} + +/// Create a new repeatable migration with the given tag, returning the paths +/// that were created. +/// +/// Only an `up.sql` is created, seeded with the `-- migrant:repeatable` +/// directive: a repeatable migration re-runs whenever that file's checksum +/// changes and is forward-only, so it must not have a down direction. See +/// [`Migratable::is_repeatable`](crate::Migratable::is_repeatable). +pub fn create_repeatable_migration(config: &Config, tag: &str) -> Result { + let (mig_dir, up) = create_migration_dir(config, tag)?; + fs::write( + &up, + format!("-- {}\n", crate::migration::REPEATABLE_DIRECTIVE), + )?; + Ok(NewMigration { + dir: mig_dir, + up, + down: None, + }) +} + +/// Create the timestamped migration directory and its (empty) `up.sql`, +/// returning both paths. +fn create_migration_dir(config: &Config, tag: &str) -> Result<(PathBuf, PathBuf)> { if !tags::is_valid_simple_tag(tag) { bail!( Migration, @@ -257,14 +362,8 @@ pub fn create_migration(config: &Config, tag: &str) -> Result { fs::create_dir_all(&mig_dir)?; let up = mig_dir.join("up.sql"); - let down = mig_dir.join("down.sql"); fs::File::create(&up)?; - fs::File::create(&down)?; - Ok(NewMigration { - dir: mig_dir, - up, - down, - }) + Ok((mig_dir, up)) } /// Open a repl connection to the given `Config` settings @@ -507,16 +606,35 @@ mod tests { let status = MigrationStatus { tag: "20200101000000_first".to_string(), applied: true, + repeatable: false, + stale: false, }; assert_eq!(status.tag(), "20200101000000_first"); assert!(status.applied()); + assert!(!status.repeatable()); + assert!(!status.stale()); let unapplied = MigrationStatus { tag: "20200102000000_second".to_string(), applied: false, + repeatable: false, + stale: true, }; assert_eq!(unapplied.tag(), "20200102000000_second"); assert!(!unapplied.applied()); + assert!(unapplied.stale()); + + // A repeatable migration that has run but whose SQL has since changed + // is both applied and stale. + let re_run = MigrationStatus { + tag: "20200103000000_seed".to_string(), + applied: true, + repeatable: true, + stale: true, + }; + assert!(re_run.applied()); + assert!(re_run.repeatable()); + assert!(re_run.stale()); } #[test] @@ -533,13 +651,16 @@ mod tests { let created = create_migration(&config, "add-widgets").unwrap(); // The returned paths are the ones that now exist on disk. + let down = created + .down_path() + .expect("a versioned migration has a down"); assert!(created.dir().is_dir(), "the migration dir must be created"); assert!(created.up_path().is_file(), "up.sql must be created"); - assert!(created.down_path().is_file(), "down.sql must be created"); + assert!(down.is_file(), "down.sql must be created"); assert_eq!(created.up_path().file_name().unwrap(), "up.sql"); - assert_eq!(created.down_path().file_name().unwrap(), "down.sql"); + assert_eq!(down.file_name().unwrap(), "down.sql"); assert_eq!(created.up_path().parent().unwrap(), created.dir()); - assert_eq!(created.down_path().parent().unwrap(), created.dir()); + assert_eq!(down.parent().unwrap(), created.dir()); // The generated folder is `<14-digit-stamp>_`. let folder = created.dir().file_name().unwrap().to_str().unwrap(); assert!( @@ -627,16 +748,94 @@ mod tests { } } + // MIGTYPE-9 #[test] - fn migration_search_requires_up_and_down() { + fn migration_search_requires_up_but_not_down() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - let d = root.join("20190101000000_first"); - fs::create_dir_all(&d).unwrap(); - fs::write(d.join("up.sql"), "select 1;").unwrap(); + + // A directory with only `up.sql` is a valid migration with no down + // direction, which is the shape a repeatable migration must have. + let up_only = root.join("20190101000000_first"); + fs::create_dir_all(&up_only).unwrap(); + fs::write(up_only.join("up.sql"), "select 1;").unwrap(); + let migs = search_for_migrations(root).unwrap(); + assert_eq!(1, migs.len()); + assert_eq!("20190101000000_first", migs[0].tag()); + assert!( + !migs[0].defines_down(), + "a migration with no down.sql defines no down direction" + ); + + // A missing `up.sql` is still an error. + let down_only = root.join("20190101000001_second"); + fs::create_dir_all(&down_only).unwrap(); + fs::write(down_only.join("down.sql"), "select 1;").unwrap(); assert!(search_for_migrations(root).is_err()); } + // REPEAT-3, REPEAT-10 + #[test] + fn discovered_migrations_declare_repeatable_through_the_directive() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + let versioned = root.join("20190101000000_first"); + fs::create_dir_all(&versioned).unwrap(); + fs::write(versioned.join("up.sql"), "select 1;").unwrap(); + fs::write(versioned.join("down.sql"), "select -1;").unwrap(); + + let repeatable = root.join("20190101000001_seed-roles"); + fs::create_dir_all(&repeatable).unwrap(); + fs::write( + repeatable.join("up.sql"), + "-- migrant:repeatable\nselect 2;", + ) + .unwrap(); + + let migs = search_for_migrations(root).unwrap(); + assert_eq!(2, migs.len()); + assert!(!migs[0].is_repeatable(), "no directive, so not repeatable"); + assert!( + migs[1].is_repeatable(), + "the up-file directive declares the migration repeatable" + ); + } + + // REPEAT-10 + #[test] + fn create_repeatable_migration_writes_only_a_seeded_up_file() { + let dir = tempfile::tempdir().unwrap(); + let settings = crate::config::Settings::configure_sqlite() + .database_path("/abs/some.db") + .migration_location(dir.path()) + .build() + .unwrap(); + let config = Config::with_settings(settings); + + let created = create_repeatable_migration(&config, "seed-roles").unwrap(); + + assert!(created.up_path().is_file(), "up.sql must be created"); + assert_eq!( + None, + created.down_path(), + "a repeatable migration has no down direction" + ); + assert!( + !created.dir().join("down.sql").exists(), + "no down.sql may be written" + ); + + // The seeded up file declares the migration repeatable, so a discovery + // of this directory reports it as such. + let up = fs::read_to_string(created.up_path()).unwrap(); + assert_eq!("-- migrant:repeatable\n", up); + let migs = search_for_migrations(dir.path()).unwrap(); + assert_eq!(1, migs.len()); + assert!(migs[0].is_repeatable()); + assert!(!migs[0].defines_down()); + } + // A password with characters that must be percent-encoded in a URL: `@` // becomes `%40` and the space becomes `%20`. If the raw or encoded form // ever leaks into `argv`, these tests catch it. diff --git a/migrant_lib/tests/server_dbs.rs b/migrant_lib/tests/server_dbs.rs index dfdd4d8..78175ed 100644 --- a/migrant_lib/tests/server_dbs.rs +++ b/migrant_lib/tests/server_dbs.rs @@ -75,6 +75,149 @@ fn apply_and_unapply(settings: &Settings) { assert!(statuses.iter().all(|m| !m.applied())); } +/// An already-applied migration whose up-SQL later changes is detected as drift: +/// an up run aborts with `ChecksumMismatch` before applying, and +/// `allow_checksum_mismatch(true)` proceeds past it. Backend-agnostic, so it +/// runs as a phase of both the postgres and mysql end-to-end tests, sharing +/// their database. +fn assert_checksum_drift_detected(settings: &Settings) { + let mut config = Config::with_settings(settings.clone()); + config.setup().unwrap(); + config + .use_migrations(&[EmbeddedMigration::with_tag("drift-probe") + .up("create table if not exists drift_probe (x integer);") + .down("drop table drift_probe;") + .boxed()]) + .unwrap(); + let config = config.reload().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + + // Same tag, edited up-SQL: the recorded checksum no longer matches. + let mut config = Config::with_settings(settings.clone()); + config + .use_migrations(&[EmbeddedMigration::with_tag("drift-probe") + .up("create table if not exists drift_probe (x integer, y integer);") + .down("drop table drift_probe;") + .boxed()]) + .unwrap(); + let config = config.reload().unwrap(); + let err = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap_err(); + assert!( + err.is_checksum_mismatch(), + "an edited already-applied migration must abort the run, got: {err:?}" + ); + + // Opting out bypasses the check; nothing is pending, so the run is a no-op. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .allow_checksum_mismatch(true) + .apply() + .unwrap(); + assert!(report.is_empty()); + + // Revert the probe migration so a re-run starts clean (Down does not run the + // Up-only drift check; the opt-out is set only for symmetry). + Migrator::with_config(&config) + .direction(Direction::Down) + .all(true) + .show_output(false) + .allow_checksum_mismatch(true) + .apply() + .unwrap(); +} + +/// REPEAT-1/REPEAT-2/REPEAT-6: a repeatable migration re-runs when its up-SQL +/// changes (rather than aborting as drift), keeps a single bookkeeping row +/// updated in place, and is left alone by a `Down` run. +/// +/// Backend-agnostic, so both the postgres and mysql end-to-end tests run it as +/// a phase against their own database. +#[cfg(any(feature = "postgres", feature = "mysql"))] +fn assert_repeatable_reruns_in_place(settings: &Settings) { + fn config_with_seed(settings: &Settings, seed: &'static str) -> Config { + let mut config = Config::with_settings(settings.clone()); + config + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table if not exists roles (name varchar(64));") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up(seed) + .boxed(), + ]) + .unwrap(); + config + } + + let config = config_with_seed(settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + let config = config.reload().unwrap(); + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["create-roles", "seed-roles"]); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + + // Unchanged SQL: nothing re-runs. + let config = config.reload().unwrap(); + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert!( + report.is_empty(), + "an unchanged repeatable migration must not re-run" + ); + + // Changed SQL re-runs it instead of aborting with a checksum mismatch, with + // the drift check left at its strict default. + let changed = config_with_seed(settings, "insert into roles (name) values ('editor');") + .reload() + .unwrap(); + let report = Migrator::with_config(&changed) + .all(true) + .show_output(false) + .apply() + .expect("a repeatable migration's changed checksum is not drift"); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + + // A Down run reverts only the versioned migration, leaving the repeatable + // row recorded. + let changed = changed.reload().unwrap(); + let report = Migrator::with_config(&changed) + .direction(Direction::Down) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["create-roles"]); + assert!(report.repeatable_tags().is_empty()); + let changed = changed.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&changed).unwrap(); + let seed = statuses + .iter() + .find(|s| s.tag() == "seed-roles") + .expect("the repeatable migration is still managed"); + assert!( + seed.applied() && seed.repeatable(), + "the repeatable row survives a full down run: {seed:?}" + ); +} + /// Drop the migration table so the next run starts from a clean database. #[cfg(feature = "postgres")] fn drop_pg_migration_table(conn_str: &str) { @@ -95,6 +238,26 @@ fn drop_mysql_migration_table(conn_str: &str) { .expect("drop mysql migration table"); } +/// Drop the `roles` table the repeatable phase migrates, so it starts clean. +#[cfg(feature = "postgres")] +fn drop_pg_roles_table(conn_str: &str) { + let mut client = postgres::Client::connect(conn_str, postgres::NoTls) + .expect("connect to drop postgres roles table"); + client + .batch_execute("drop table if exists roles;") + .expect("drop postgres roles table"); +} + +/// Drop the `roles` table the repeatable phase migrates, so it starts clean. +#[cfg(feature = "mysql")] +fn drop_mysql_roles_table(conn_str: &str) { + use mysql::prelude::Queryable; + let opts = mysql::Opts::from_url(conn_str).expect("parse mysql connection string"); + let mut conn = mysql::Conn::new(opts).expect("connect to drop mysql roles table"); + conn.query_drop("drop table if exists roles;") + .expect("drop mysql roles table"); +} + #[cfg(feature = "postgres")] #[test] fn postgres_end_to_end() { @@ -130,6 +293,14 @@ fn postgres_end_to_end() { // synchronized(false) phase, also against the same database assert_unsynchronized_run_skips_lock(&conn_str, &settings); drop_pg_migration_table(&conn_str); + // checksum-drift phase, also against the same database + assert_checksum_drift_detected(&settings); + drop_pg_migration_table(&conn_str); + // repeatable re-run phase, also against the same database + drop_pg_roles_table(&conn_str); + assert_repeatable_reruns_in_place(&settings); + drop_pg_roles_table(&conn_str); + drop_pg_migration_table(&conn_str); } /// With `synchronized(false)` a run must not take the migration advisory lock: @@ -220,7 +391,7 @@ fn assert_pg_schema_records_checksum_and_order(conn_str: &str, settings: &Settin let mut client = postgres::Client::connect(conn_str, postgres::NoTls).unwrap(); // The new columns exist. - for col in ["id", "tag", "checksum", "applied_at"] { + for col in ["id", "tag", "checksum", "applied_at", "is_repeatable"] { let exists: bool = client .query_one( "select exists(select 1 from information_schema.columns \ @@ -398,6 +569,14 @@ fn mysql_end_to_end() { // schema (checksum/applied_at) + recorded-order phase, same database assert_mysql_schema_records_checksum_and_order(&conn_str, &settings); drop_mysql_migration_table(&conn_str); + // checksum-drift phase, also against the same database + assert_checksum_drift_detected(&settings); + drop_mysql_migration_table(&conn_str); + // repeatable re-run phase, also against the same database + drop_mysql_roles_table(&conn_str); + assert_repeatable_reruns_in_place(&settings); + drop_mysql_roles_table(&conn_str); + drop_mysql_migration_table(&conn_str); } /// After applying two embedded migrations, the `__migrant_migrations` table on @@ -444,7 +623,7 @@ fn assert_mysql_schema_records_checksum_and_order(conn_str: &str, settings: &Set let mut conn = mysql::Conn::new(opts).unwrap(); // The new columns exist. - for col in ["id", "tag", "checksum", "applied_at"] { + for col in ["id", "tag", "checksum", "applied_at", "is_repeatable"] { let exists: Option = conn .exec_first( "select count(*) from information_schema.columns \ diff --git a/migrant_lib/tests/sqlite.rs b/migrant_lib/tests/sqlite.rs index 24576a4..b64b17a 100644 --- a/migrant_lib/tests/sqlite.rs +++ b/migrant_lib/tests/sqlite.rs @@ -846,3 +846,795 @@ fn custom_migratable_applies_and_records_null_checksum() { "a custom migration inherits the default `checksum` of None (NULL)" ); } + +#[test] +fn checksum_drift_aborts_up_run_and_opt_out_applies() { + // Apply a migration, then swap in a same-tag migration whose up-SQL differs + // (as if the file had been edited after it ran). The recorded checksum no + // longer matches the current one, so the next up run aborts before applying. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let mut config = Config::with_settings(settings); + config + .use_migrations(&[EmbeddedMigration::with_tag("t") + .up("create table t (x integer);") + .down("drop table t;") + .boxed()]) + .unwrap(); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + let recorded = recorded_rows(&config)[0].1.clone(); + assert!( + recorded.is_some(), + "the up-SQL migration records a checksum" + ); + + // Same tag, edited up-SQL: a different checksum, on the same connection. + config + .use_migrations(&[EmbeddedMigration::with_tag("t") + .up("create table t (x integer, y integer);") + .down("drop table t;") + .boxed()]) + .unwrap(); + + let err = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap_err(); + assert!( + err.is_checksum_mismatch(), + "an edited already-applied migration must abort the run, got: {err:?}" + ); + assert!( + format!("{err}").contains("t"), + "the error should name the drifted tag: {err}" + ); + + // The recorded checksum is unchanged: the aborted run recorded nothing. + assert_eq!( + recorded, + recorded_rows(&config)[0].1, + "an aborted drift run must not rewrite the recorded checksum" + ); + + // Opting out bypasses the check. The migration is already applied, so the + // run has nothing to do and simply reports empty instead of erroring. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .allow_checksum_mismatch(true) + .apply() + .unwrap(); + assert!( + report.is_empty(), + "opting out applies despite drift; nothing was pending here" + ); +} + +#[test] +fn matching_checksum_does_not_trip_drift_check() { + // Re-running with the identical migration set must not be flagged as drift. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let mut config = Config::with_settings(settings); + config + .use_migrations(&[EmbeddedMigration::with_tag("t") + .up("create table t (x integer);") + .down("drop table t;") + .boxed()]) + .unwrap(); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + // A second up run over the unchanged set validates checksums and finds no + // drift, returning an empty report rather than erroring. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert!(report.is_empty(), "unchanged checksums must not error"); +} + +#[test] +fn null_recorded_checksum_is_not_treated_as_drift() { + // A programmatic migration records a NULL checksum. Re-running after it is + // applied must not flag drift even though the migration reports no checksum + // to compare against. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = migrations_config(&settings); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + // `seed-users` is an `FnMigration` recorded with a NULL checksum. + let rows = recorded_rows(&config); + assert!( + rows.iter() + .any(|(tag, sum)| tag == "seed-users" && sum.is_none()), + "the programmatic migration records a NULL checksum: {rows:?}" + ); + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert!( + report.is_empty(), + "a NULL recorded checksum is skipped, not treated as drift" + ); +} + +/// Read the full bookkeeping rows in recorded (`order by id`) order, including +/// each row's id and repeatable flag. +fn recorded_repeatable_rows(config: &Config) -> Vec<(i64, String, Option, bool)> { + let handle = config.sqlite_connection().unwrap(); + let conn = handle.lock().unwrap(); + let mut stmt = conn + .prepare("select id, tag, checksum, is_repeatable from __migrant_migrations order by id") + .unwrap(); + let rows = stmt + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?))) + .unwrap(); + rows.collect::, _>>().unwrap() +} + +/// An in-memory config with one versioned migration creating a `roles` table +/// and one repeatable migration seeding it with the given SQL. +fn repeatable_config(settings: &Settings, seed_sql: &'static str) -> Config { + let mut config = Config::with_settings(settings.clone()); + config + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up(seed_sql) + .boxed(), + ]) + .unwrap(); + config +} + +fn role_names(config: &Config) -> Vec { + let handle = config.sqlite_connection().unwrap(); + let conn = handle.lock().unwrap(); + let mut stmt = conn + .prepare("select name from roles order by name") + .unwrap(); + let rows = stmt.query_map([], |r| r.get(0)).unwrap(); + rows.collect::, _>>().unwrap() +} + +// REPEAT-1, REPEAT-5, REPEAT-6 +#[test] +fn repeatable_migration_reruns_only_when_its_checksum_changes() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + + // First run: the versioned migration, then the repeatable one. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["create-roles", "seed-roles"]); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!(role_names(&config), ["admin"]); + + // Second run over unchanged SQL: nothing re-runs. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert!( + report.is_empty(), + "an unchanged repeatable migration must not re-run" + ); + assert_eq!(role_names(&config), ["admin"]); + + let before = recorded_repeatable_rows(&config); + assert_eq!(2, before.len()); + assert!(before[1].3, "the repeatable row records its kind"); + assert!(!before[0].3, "the versioned row does not"); + + // Changing the repeatable migration's SQL is the signal to re-run it. The + // config carries the same live in-memory connection, so the seeded rows + // survive into the new config. + let mut changed = config.clone(); + changed + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor');") + .boxed(), + ]) + .unwrap(); + let report = Migrator::with_config(&changed) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["seed-roles"]); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!(role_names(&changed), ["admin", "editor"]); + + // REPEAT-6: still one row for the tag, updated in place with the new + // checksum and keeping its original id. + let after = recorded_repeatable_rows(&changed); + assert_eq!(2, after.len(), "a re-run must not insert a second row"); + assert_eq!(before[1].0, after[1].0, "the row keeps its id"); + assert_eq!("seed-roles", after[1].1); + assert_ne!(before[1].2, after[1].2, "the checksum is updated"); + assert!(after[1].3, "the row is still marked repeatable"); +} + +// REPEAT-2 +#[test] +fn a_changed_repeatable_migration_is_not_checksum_drift() { + // The same recorded-vs-current checksum mismatch that aborts a run for a + // versioned migration drives the re-run for a repeatable one, with the + // drift check left at its strict default. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + + let mut changed = config.clone(); + changed + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor');") + .boxed(), + ]) + .unwrap(); + let report = Migrator::with_config(&changed) + .all(true) + .show_output(false) + .apply() + .expect("a repeatable migration's changed checksum is not drift"); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); +} + +// REPEAT-4 +#[test] +fn down_runs_never_revert_repeatable_migrations() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + let config = config.reload().unwrap(); + assert_eq!(applied_tags(&config), ["create-roles", "seed-roles"]); + + // Reverting everything targets only the versioned migration; the + // repeatable row is left in place. + let report = Migrator::with_config(&config) + .direction(Direction::Down) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["create-roles"]); + assert!(report.repeatable_tags().is_empty()); + let config = config.reload().unwrap(); + assert_eq!(applied_tags(&config), ["seed-roles"]); + assert!(!table_exists(&config, "roles"), "the down migration ran"); + + // REPEAT-11: re-applying restores the versioned migration, and the + // unchanged repeatable one stays skipped. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["create-roles"]); + assert!( + report.repeatable_tags().is_empty(), + "an unchanged repeatable migration stays skipped after a down/up cycle" + ); +} + +// REPEAT-1 +#[test] +fn an_all_run_terminates_with_repeatable_migrations_present() { + // A repeatable migration runs at most once per run, so an `all` run cannot + // loop on one. This test would hang rather than fail if that broke. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!(role_names(&config), ["admin"], "the seed ran exactly once"); +} + +// REPEAT-8 +#[test] +fn statuses_report_repeatable_and_stale_state() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + + // Before any run both migrations are stale (they will run next). + let config = config.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&config).unwrap(); + assert!(statuses.iter().all(|s| s.stale() && !s.applied())); + assert!(!statuses[0].repeatable()); + assert!(statuses[1].repeatable()); + assert_eq!( + migrant_lib::pending_migrations(&config).unwrap(), + ["create-roles", "seed-roles"] + ); + + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + let config = config.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&config).unwrap(); + assert!( + statuses.iter().all(|s| s.applied() && !s.stale()), + "everything is applied and up to date after a run" + ); + assert!(migrant_lib::pending_migrations(&config).unwrap().is_empty()); + + // Changing the repeatable migration's SQL makes it stale again while it + // stays applied. + let mut changed = config.clone(); + changed + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor');") + .boxed(), + ]) + .unwrap(); + let changed = changed.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&changed).unwrap(); + assert!(!statuses[0].stale(), "the versioned migration is unchanged"); + assert!( + statuses[1].applied() && statuses[1].stale() && statuses[1].repeatable(), + "the repeatable migration is recorded but due to re-run: {statuses:?}" + ); + assert_eq!( + migrant_lib::pending_migrations(&changed).unwrap(), + ["seed-roles"], + "a stale repeatable migration is pending" + ); +} + +// REPEAT-5 +#[test] +fn repeatable_migrations_run_after_all_pending_versioned_ones() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let mut config = Config::with_settings(settings); + config + .use_migrations(&[ + // Declared first, but must still run last. + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('admin');") + .boxed(), + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("add-index") + .up("create index roles_name on roles (name);") + .down("drop index roles_name;") + .boxed(), + ]) + .unwrap(); + config.setup().unwrap(); + + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!( + report.tags(), + ["create-roles", "add-index", "seed-roles"], + "the repeatable migration runs after the versioned sequence" + ); +} + +// REPEAT-9 +#[test] +fn a_failed_repeatable_rerun_rolls_back_with_its_bookkeeping() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + let before = recorded_repeatable_rows(&config); + + // Re-run with SQL that inserts a row and then fails. Both the insert and + // the in-place bookkeeping update must roll back together. + let mut broken = config.clone(); + broken + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor'); insert into nope values (1);") + .boxed(), + ]) + .unwrap(); + let res = Migrator::with_config(&broken) + .all(true) + .show_output(false) + .apply(); + assert!(res.is_err(), "the failing re-run must abort the run"); + assert_eq!( + role_names(&broken), + ["admin"], + "the partial insert rolled back" + ); + assert_eq!( + before, + recorded_repeatable_rows(&broken), + "the recorded checksum is unchanged, so the migration is retried next run" + ); +} + +// REPEAT-9 +#[test] +fn fake_records_a_repeatable_rerun_without_running_its_sql() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(role_names(&config), ["admin"]); + + // A faked re-run updates the recorded checksum without executing the SQL, + // so the migration is no longer stale but nothing was seeded. + let mut changed = config.clone(); + changed + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor');") + .boxed(), + ]) + .unwrap(); + let report = Migrator::with_config(&changed) + .all(true) + .fake(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!( + role_names(&changed), + ["admin"], + "a faked re-run must not run the SQL" + ); + + // The recorded checksum was updated, so it is no longer stale. + let changed = changed.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&changed).unwrap(); + assert!(!statuses[1].stale(), "a faked re-run records the checksum"); +} + +// REPEAT-9 +#[test] +fn force_skip_failures_retries_a_repeatable_rerun_next_run() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + + // A failing re-run under skip-failures leaves the recorded checksum alone, + // and the run still terminates rather than retrying the same migration. + let mut broken = config.clone(); + broken + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into nope values (1);") + .boxed(), + ]) + .unwrap(); + let report = Migrator::with_config(&broken) + .all(true) + .force(ForceMode::SkipFailures) + .show_output(false) + .apply() + .unwrap(); + assert!( + report.is_empty(), + "a skipped re-run records nothing: {report:?}" + ); + + // Still stale, so the next run retries it. + let broken = broken.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&broken).unwrap(); + assert!( + statuses[1].stale(), + "an unrecorded failed re-run is retried on the next run" + ); +} + +// REPEAT-9 +#[test] +fn force_accept_failures_records_a_failed_repeatable_rerun_in_place() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + let before = recorded_repeatable_rows(&config); + + // Under accept-failures the re-run fails, its SQL is rolled back, but the + // bookkeeping is still updated: the migration is marked up to date and is + // NOT retried on the next run. + let mut broken = config.clone(); + broken + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("insert into roles (name) values ('editor'); insert into nope values (1);") + .boxed(), + ]) + .unwrap(); + let report = Migrator::with_config(&broken) + .all(true) + .force(ForceMode::AcceptFailures) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!( + role_names(&broken), + ["admin"], + "the failed re-run's SQL rolled back" + ); + + // The row was updated in place: still one row, same id, new checksum. + let after = recorded_repeatable_rows(&broken); + assert_eq!(before.len(), after.len(), "no extra row was inserted"); + assert_eq!(before[1].0, after[1].0, "the row keeps its id"); + assert_ne!(before[1].2, after[1].2, "the checksum was rewritten"); + + // Marked up to date despite having failed, which is the documented + // accept-failures tradeoff. + let broken = broken.reload().unwrap(); + let statuses = migrant_lib::migration_statuses(&broken).unwrap(); + assert!( + !statuses[1].stale(), + "accept-failures records the re-run, so it is not retried" + ); +} + +// REPEAT-12 +#[test] +fn rerun_repeatable_reruns_an_unchanged_migration() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(role_names(&config), ["admin"]); + let before = recorded_repeatable_rows(&config); + + // Without the override an unchanged repeatable migration is skipped. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert!(report.is_empty()); + + // With it, the SQL runs again even though nothing changed. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .rerun_repeatable(true) + .apply() + .unwrap(); + assert_eq!(report.tags(), ["seed-roles"]); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!( + role_names(&config), + ["admin", "admin"], + "the seed ran a second time" + ); + + // Bookkeeping is still one row updated in place, with the same (unchanged) + // checksum and the same id. + let after = recorded_repeatable_rows(&config); + assert_eq!(before.len(), after.len(), "no extra row was inserted"); + assert_eq!(before[1].0, after[1].0, "the row keeps its id"); + assert_eq!(before[1].2, after[1].2, "the checksum is unchanged"); + assert!(after[1].3); + + // The run terminates: the override does not defeat the once-per-run guard. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .rerun_repeatable(true) + .apply() + .unwrap(); + assert_eq!( + report.repeatable_tags(), + ["seed-roles"], + "each override run re-runs it exactly once" + ); + assert_eq!(role_names(&config), ["admin", "admin", "admin"]); +} + +// REPEAT-12 +#[test] +fn rerun_repeatable_does_not_re_apply_versioned_migrations() { + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let config = repeatable_config(&settings, "insert into roles (name) values ('admin');"); + config.setup().unwrap(); + Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + + // `create-roles` would fail if re-applied (the table exists), so a passing + // run proves the override left it alone. + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .rerun_repeatable(true) + .apply() + .unwrap(); + assert_eq!( + report.tags(), + ["seed-roles"], + "only the repeatable migration re-ran" + ); +} + +// REPEAT-7 +#[test] +fn registering_an_invalid_repeatable_migration_errors() { + // A repeatable migration with a down direction. + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let mut config = Config::with_settings(settings.clone()); + let res = config.use_migrations(&[EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .up("select 1;") + .down("select -1;") + .boxed()]); + assert!( + res.is_err(), + "a repeatable migration must not define a down direction" + ); + + // A repeatable migration with no up-SQL, so no checksum. + let mut config = Config::with_settings(settings); + let res = config.use_migrations(&[EmbeddedMigration::with_tag("seed-roles") + .repeatable() + .boxed()]); + assert!( + res.is_err(), + "a repeatable migration must have a checksum to compare" + ); +} + +// REPEAT-3 +#[test] +fn file_migrations_declare_repeatable_through_the_up_file_directive() { + // The directive is the CLI's route to a repeatable migration: the file set + // is discovered from disk, with no builder call available. + let dir = tempfile::tempdir().unwrap(); + let up = dir.path().join("up.sql"); + std::fs::write( + &up, + b"-- migrant:repeatable\ninsert into roles (name) values ('admin');", + ) + .unwrap(); + + let settings = Settings::configure_sqlite().memory().build().unwrap(); + let mut config = Config::with_settings(settings); + config + .use_migrations(&[ + EmbeddedMigration::with_tag("create-roles") + .up("create table roles (name text);") + .down("drop table roles;") + .boxed(), + FileMigration::with_tag("seed-roles").up(&up).boxed(), + ]) + .unwrap(); + config.setup().unwrap(); + + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!( + report.repeatable_tags(), + ["seed-roles"], + "the up-file directive declares the migration repeatable" + ); + + // Editing the file changes its checksum, which re-runs it. + std::fs::write( + &up, + b"-- migrant:repeatable\ninsert into roles (name) values ('editor');", + ) + .unwrap(); + let report = Migrator::with_config(&config) + .all(true) + .show_output(false) + .apply() + .unwrap(); + assert_eq!(report.repeatable_tags(), ["seed-roles"]); + assert_eq!(role_names(&config), ["admin", "editor"]); +} diff --git a/spec/README.md b/spec/README.md index e66927d..3d516da 100644 --- a/spec/README.md +++ b/spec/README.md @@ -27,6 +27,8 @@ can be built). Keep each row's status current with `spec.py set`. | Config File and Env Resolution | done | [config-file-and-env-resolution.md](config-file-and-env-resolution.md) | | Error Handling API | done | [error-handling-api.md](error-handling-api.md) | | Distribution | done | [distribution.md](distribution.md) | +| Repeatable Migrations | done | [repeatable-migrations.md](repeatable-migrations.md) | +| Checksum Drift Detection | done | [checksum-drift-detection.md](checksum-drift-detection.md) | ## Conventions diff --git a/spec/checksum-drift-detection.md b/spec/checksum-drift-detection.md new file mode 100644 index 0000000..7caf2ed --- /dev/null +++ b/spec/checksum-drift-detection.md @@ -0,0 +1,50 @@ +# Checksum Drift Detection + +An `Up` run verifies that every already-applied migration still matches the `checksum` +recorded in `__migrant_migrations` (see [migration-types.md](migration-types.md) MIGTYPE-6) +before applying anything, so an edit to already-applied SQL is caught instead of silently +building on a changed migration. This is the enforcement counterpart to +[repeatable-migrations.md](repeatable-migrations.md), where a checksum change is instead the +signal to re-run. + +## DRIFT-1 + +Before an `Up` run applies any migration, migrant compares each already-applied migration's +current `checksum()` against the checksum recorded for its tag. A difference is drift: the +run aborts with [`Error::ChecksumMismatch`](error-handling-api.md) before applying anything, +naming the drifted tag. The check runs alongside the unknown-tags and out-of-order checks +(see [migrator-api.md](migrator-api.md) MIGRATOR-7) and, like them, runs even when no +migrations are pending (so `apply` on an up-to-date database still validates checksums). + +## DRIFT-2 + +The comparison only fires when both sides are present: the recorded checksum is non-null and +the migration's current `checksum()` is `Some`. A null on either side is skipped, not a +mismatch. This covers programmatic migrations (`FnMigration`, which records a null checksum) +and legacy rows recorded before checksums existed or backfilled null during an in-place +schema upgrade. + +## DRIFT-3 + +The check covers only tags that are both applied and present in the available migration set. +An applied tag absent from the available set is handled by the unknown-tags check +(MIGRATOR-7), not here. + +## DRIFT-4 + +`Migrator::allow_checksum_mismatch(bool)` (default `false`) opts out of the check, applying +despite drift. It is independent of `allow_unknown_tags` and `allow_out_of_order`: each +check is enabled or bypassed on its own. + +## DRIFT-5 + +The check is part of the `Up`-run consistency checks. A pure `Down` run (`apply --down`) +does not trigger it; `redo` does, via its `Up` phase. + +## DRIFT-6 + +The CLI exposes the opt-out as `--allow-checksum-mismatch` on `apply` and `redo` +(see [cli-migration-management.md](cli-migration-management.md)), off by default. + +Coverage: `migrant_lib/tests/sqlite.rs`, `server_dbs.rs`, `tests/migrant.rs`; unit tests in +`migrant_lib/src/migrator.rs`. diff --git a/spec/cli-migration-management.md b/spec/cli-migration-management.md index b9c9188..349badc 100644 --- a/spec/cli-migration-management.md +++ b/spec/cli-migration-management.md @@ -5,7 +5,9 @@ new, edit, list, apply, and redo subcommands for creating and running migrations ## CLIMIG-1 `migrant new ` generates a timestamped up/down migration file pair under the configured -migration location. +migration location. `--repeatable` instead generates only an `up.sql`, seeded with the +`-- migrant:repeatable` directive (see +[repeatable-migrations.md](repeatable-migrations.md) REPEAT-10). ## CLIMIG-2 @@ -30,21 +32,33 @@ migrations is the default behavior. ## CLIMIG-5 `migrant redo` unapplies then reapplies the latest migration (`--down` then up); `--all` -redoes all applied migrations. Down-migrations run in reverse application order. +redoes all applied migrations. Down-migrations run in reverse application order. `redo` warns +when it will not revert applied repeatable migrations (see +[repeatable-migrations.md](repeatable-migrations.md) REPEAT-13). ## CLIMIG-6 `migrant status` reports every managed migration with its applied/pending state plus summary -counts (total, applied, pending). `--format text` (the default) prints a summary line followed -by a `[✓]`/`[ ]` row per migration; `--format json` prints the same data as pretty-printed JSON -(`{ total, applied, pending, migrations: [{ tag, applied }] }`) for scripting. +counts (total, applied, pending, stale). The summary `stale` count covers only applied +migrations that will re-run (repeatable ones whose SQL changed), so `applied + pending` still +equals `total`; the per-row `stale` field is broader, true for anything that would run next +including a versioned migration with no row yet. `--format text` (the default) prints a summary line +followed by a `[✓]`/`[ ]` row per migration; a repeatable migration is annotated +`(repeatable)` and, when stale, marked `[~]` with `(repeatable, will re-run)`. The summary +line reports the stale count only when it is non-zero. `--format json` prints the same data +as pretty-printed JSON +(`{ total, applied, pending, stale, migrations: [{ tag, applied, repeatable, stale }] }`) +for scripting. ## CLIMIG-7 -`apply` and `redo` accept `--allow-unknown-tags` and `--allow-out-of-order`, both off by -default. `--allow-unknown-tags` permits a run when the database has an applied tag not -present in the defined migration set, instead of erroring. `--allow-out-of-order` permits a -run to apply migrations out of their defined order, instead of erroring. +`apply` and `redo` accept `--allow-unknown-tags`, `--allow-out-of-order`, and +`--allow-checksum-mismatch`, all off by default. `--allow-unknown-tags` permits a run when the +database has an applied tag not present in the defined migration set, instead of erroring. +`--allow-out-of-order` permits a run to apply migrations out of their defined order, instead of +erroring. `--allow-checksum-mismatch` permits a run when an already-applied migration's SQL has +changed since it was recorded (see [checksum-drift-detection.md](checksum-drift-detection.md)), +instead of erroring. Coverage: `tests/migrant.rs` (kitchen_sink, new_rejects_invalid_tag, apply_fake_records_without_running, force_modes_through_the_cli, status_reports_text_and_json), diff --git a/spec/database-backends.md b/spec/database-backends.md index 44463c4..880ca1d 100644 --- a/spec/database-backends.md +++ b/spec/database-backends.md @@ -26,10 +26,17 @@ Invoking an operation whose backend feature is disabled returns ## BACKEND-6 -The `__migrant_migrations` bookkeeping table has four columns on all three backends: `id` +The `__migrant_migrations` bookkeeping table has five columns on all three backends: `id` (an auto-incrementing key recording applied order), `tag` (the migration tag), `checksum` -(a sha256 checksum of the migration's up-SQL, null for programmatic migrations), and -`applied_at` (a timestamp of when the migration was applied). +(a sha256 checksum of the migration's up-SQL, null for programmatic migrations), +`applied_at` (a timestamp of when the migration was applied), and `is_repeatable` (whether +the row records a repeatable migration, default false). The column is named `is_repeatable` +rather than `repeatable` because the latter is a keyword on both postgres and mysql and +would need per-backend quoting. + +A repeatable migration's row is updated in place on each re-run rather than re-inserted, so +`checksum` and `applied_at` change while `id` (and therefore recorded application order) +does not. See [repeatable-migrations.md](repeatable-migrations.md) REPEAT-6. Coverage: `migrant_lib/tests/sqlite.rs`; `server_dbs.rs` (postgres/mysql end-to-end, gated on POSTGRES_TEST_CONN_STR/MYSQL_TEST_CONN_STR, run via `test.sh` against docker diff --git a/spec/error-handling-api.md b/spec/error-handling-api.md index ce41f93..6ca26f4 100644 --- a/spec/error-handling-api.md +++ b/spec/error-handling-api.md @@ -5,6 +5,9 @@ Typed Error variants and helpers. ## ERRORH-1 `Error` variants cover the main failure modes: `Migration`, `MigrationNotFound`, +`MigrationOrdering` (an applied migration is out of definition order), +`ChecksumMismatch` (an already-applied migration's SQL changed since it was +recorded; see [checksum-drift-detection.md](checksum-drift-detection.md)), `TagError` (invalid tag format), `ShellCommand`, `PathError`, `InvalidDbKind`, `FeatureRequired` (operation needs a disabled cargo feature), and `Config`. The enum is `#[non_exhaustive]`. There is no "nothing to apply" error variant: a run @@ -14,7 +17,8 @@ with nothing pending returns an empty `Report` (see [migrator-api.md](migrator-a `Error` exposes predicate methods for branching without matching the `#[non_exhaustive]` enum: `is_config`, `is_migration`, `is_migration_not_found`, -`is_shell_command`, `is_tag_error`, `is_invalid_db_kind`, `is_feature_required`. +`is_migration_ordering`, `is_checksum_mismatch`, `is_shell_command`, `is_tag_error`, +`is_invalid_db_kind`, `is_feature_required`. ## ERRORH-3 @@ -24,8 +28,16 @@ expressions on either enum must include a wildcard arm. ## ERRORH-4 -`MigrationStatus`'s `tag` and `applied` fields are private, accessed via -`tag(&self) -> &str` and `applied(&self) -> bool`. +`MigrationStatus`'s fields are private, accessed via `tag(&self) -> &str`, +`applied(&self) -> bool`, `repeatable(&self) -> bool`, and `stale(&self) -> bool` +(see [repeatable-migrations.md](repeatable-migrations.md) REPEAT-8). + +## ERRORH-5 + +An unusable repeatable declaration (a repeatable migration with no checksum, or one that +defines a down direction) is reported as `Error::Migration`, from `Config::use_migrations` for +explicitly defined migrations and from the migrator's load of the available set for +file-discovered ones. See [repeatable-migrations.md](repeatable-migrations.md) REPEAT-7. Coverage: unit tests in `migrant_lib/src/errors.rs`, `tags.rs`; exercised throughout the integration tests. diff --git a/spec/migration-types.md b/spec/migration-types.md index 667f9f0..0696959 100644 --- a/spec/migration-types.md +++ b/spec/migration-types.md @@ -42,4 +42,26 @@ compute it from their up-SQL; `FnMigration` (a programmatic migration with no SQ `Migratable::description(Direction)` takes `Direction` by value instead of by reference. +## MIGTYPE-8 + +`Migratable::is_repeatable()` (default `false`) reports whether a migration re-runs on every +checksum change instead of applying once. `FileMigration` and `EmbeddedMigration` declare it +with a `repeatable()` builder method or a `-- migrant:repeatable` directive in their up-SQL +(either declares it; neither can un-declare the other), mirroring MIGTYPE-5's two forms. The +trait predicate carries the `is_` +prefix because the builder method occupies the plain name on the concrete types, the same +split as `no_transaction()` and `use_transaction()`. See +[repeatable-migrations.md](repeatable-migrations.md). + +`Migratable::defines_down()` (default `false`) reports whether a migration has a down +direction to run, and exists so a repeatable migration that also defines a down can be +rejected (REPEAT-7). + +## MIGTYPE-9 + +The down direction is optional for file-discovered migrations: a migration directory needs +only `up.sql`, and `down.sql` may be absent. Reverting a migration with no down direction +removes its bookkeeping row without running SQL, the same silent no-op `FnMigration` already +has for a missing function. + Coverage: `migrant_lib/tests/sqlite.rs`, `server_dbs.rs`, `reload_memory.rs`. diff --git a/spec/migrator-api.md b/spec/migrator-api.md index 07f9db5..35085f7 100644 --- a/spec/migrator-api.md +++ b/spec/migrator-api.md @@ -77,8 +77,26 @@ By default, a run errors before applying anything if either check fails: - Out of order: a pending migration would apply out of the order defined by the migration set, given what is already recorded as applied. -`Migrator::allow_unknown_tags(bool)` (default `false`) and `Migrator::allow_out_of_order(bool)` -(default `false`) each opt out of the corresponding check. +- Checksum drift: an already-applied migration's current checksum no longer matches the one + recorded when it was applied. See [checksum-drift-detection.md](checksum-drift-detection.md). + +`Migrator::allow_unknown_tags(bool)`, `Migrator::allow_out_of_order(bool)`, and +`Migrator::allow_checksum_mismatch(bool)` (each default `false`) opt out of the corresponding +check, independently of one another. + +Repeatable migrations are exempt from all three checks: a checksum change is their re-run +signal, and their tags are not part of the versioned sequence. See +[repeatable-migrations.md](repeatable-migrations.md) REPEAT-2 and REPEAT-5. + +## MIGRATOR-8 + +An `Up` run applies pending versioned migrations first, then re-runs the stale repeatable +migrations (REPEAT-5). `Report::repeatable_tags()` returns the repeatable tags the run +re-ran, a subset of `Report::tags()`. A `Down` run never selects a repeatable migration. + +`Migrator::rerun_repeatable(bool)` (default `false`) re-runs every repeatable migration on an +`Up` run regardless of checksum. See +[repeatable-migrations.md](repeatable-migrations.md) REPEAT-12. Coverage: `migrant_lib/tests/sqlite.rs`, `server_dbs.rs`, `reload_memory.rs`, `tests/migrant.rs`; unit tests in `migrant_lib/src/migrator.rs`. diff --git a/spec/repeatable-migrations.md b/spec/repeatable-migrations.md new file mode 100644 index 0000000..be3c990 --- /dev/null +++ b/spec/repeatable-migrations.md @@ -0,0 +1,141 @@ +# Repeatable Migrations + +Data-migration scripts that re-run whenever their up-SQL checksum changes, as the +counterpart to versioned migrations. A versioned migration applies once and its recorded +`checksum` (see [migration-types.md](migration-types.md) MIGTYPE-6) is expected to stay +stable, so a later change is drift. A repeatable migration inverts this: a checksum change +is the signal to re-run. These suit idempotent data work (seeding, backfills, refreshing +views, re-granting privileges) rather than one-time schema versioning. + +## REPEAT-1 + +A repeatable migration re-runs its up-direction whenever its current `checksum()` differs +from the value last recorded in `__migrant_migrations`, or when no row exists for its tag +yet. After a successful run the recorded checksum is updated to the new value. A repeatable +migration whose current checksum equals the recorded value is skipped. Within a single run a +repeatable migration runs at most once. + +## REPEAT-2 + +Repeatable and versioned migrations share the `checksum` bookkeeping column but interpret a +mismatch oppositely. For a versioned migration a recorded-vs-current mismatch is drift and +aborts the run (see [checksum-drift-detection.md](checksum-drift-detection.md) DRIFT-1); for +a repeatable migration a mismatch is expected and drives the re-run in REPEAT-1. Repeatable +migrations are therefore excluded from the drift check entirely. + +The available migration set is authoritative on kind for any tag it still defines: a migration +converted back to versioned is drift-checked again even though its recorded row still says +repeatable. The `is_repeatable` column decides only for tags that are no longer in the +available set, where nothing else records what they were (the unknown-tags check and `Down` +selection, REPEAT-5). + +## REPEAT-3 + +A migration is declared repeatable either by the `-- migrant:repeatable` directive on a +comment line in its up-SQL, or by the `repeatable()` builder method on +[`FileMigration`](migration-types.md)/`EmbeddedMigration`. This mirrors the +`-- migrant:no-transaction` mechanism (MIGTYPE-5): the directive travels with the SQL, so it +is the only form available to migrations the `migrant` CLI discovers from disk. Either form +declares the migration repeatable; unlike `no-transaction` there is no precedence between +them, because neither can un-declare the other. + +`Migratable::is_repeatable(&self) -> bool` (default `false`) reports the result, so the migrator +branches on kind through the trait and custom `Migratable` implementations can opt in. The trait +predicate is `is_repeatable` rather than `repeatable` because the builder method holds the plain +name on the concrete types, the same split as `no_transaction()` and `use_transaction()`. + +## REPEAT-4 + +Repeatable migrations are forward-only: they have no meaningful `down`. A `Down` run never +selects them, so `redo` never reverts one. `redo`'s `Up` phase is an ordinary run, so it +re-runs a repeatable migration only under the REPEAT-1 rule (its checksum changed), the same +as any other `Up` run. + +Defining a down direction on a repeatable migration is an error, not a silent no-op: +registration rejects it (REPEAT-7). Because of this the down direction of a file-discovered +migration is now optional -- `down.sql` may be absent, and a migration with no down file +reverts by removing its bookkeeping row without running SQL (see MIGTYPE-8). + +## REPEAT-5 + +Repeatable migrations run after all pending versioned migrations in a run, and re-run in +definition order among themselves each time. They are excluded from the unknown-tags and +out-of-order checks (see [migrator-api.md](migrator-api.md) MIGRATOR-7): a repeatable tag is +not part of the versioned sequence. + +Per REPEAT-2, kind comes from the available set where it defines the tag (the out-of-order +check, which walks that set) and from the recorded `is_repeatable` column where it does not +(the unknown-tags check, and `Down` selection, which both walk applied tags). A repeatable +migration removed from the codebase therefore leaves a row that is skipped rather than +reported as an unknown tag or treated as a missing `Down` target. + +## REPEAT-6 + +A repeatable migration keeps a single row in `__migrant_migrations`, updated in place +(`checksum` and `applied_at`) on each re-run, rather than inserting a new row per run. The +bookkeeping table carries an `is_repeatable` column (see +[database-backends.md](database-backends.md) BACKEND-6) recording the kind of each row, so a +re-run updates rather than duplicates and so drift/unknown-tag logic treats each kind +correctly even for a tag that has since been removed from the available set. The row keeps +its original `id`, so a re-run does not change recorded application order. + +## REPEAT-7 + +Registering a migration that is repeatable but whose `checksum()` is `None` is an error: +without a checksum there is no basis for "changed since last run", so the declaration is +rejected rather than silently always-running or never-running. Declaring both `repeatable()` +and a down direction is likewise an error (REPEAT-4). Both are reported as +`Error::Migration` from `Config::use_migrations` for explicitly defined migrations, and from +the migrator's own load of the available set for file-discovered ones. In practice this +means `FnMigration` (which has no SQL to hash) cannot be repeatable. + +## REPEAT-8 + +`status` and `list` ([cli-migration-management.md](cli-migration-management.md) CLIMIG-3, +CLIMIG-6) surface repeatable migrations and whether each is up-to-date (recorded checksum +matches) or stale (will re-run). `MigrationStatus` exposes `repeatable()` and `stale()` +alongside `applied()`, and `pending_migrations` lists stale repeatable tags after the pending +versioned ones, in the order a run would apply them. A run's `Report` +([migrator-api.md](migrator-api.md) MIGRATOR-1) reports the repeatable tags it re-ran through +`Report::repeatable_tags()`, a subset of `tags()`. + +## REPEAT-9 + +Repeatable migrations follow the same transactional wrapping as versioned ones (see +[transactional-migrations.md](transactional-migrations.md) and MIGTYPE-5): the re-run and +its in-place bookkeeping update commit or roll back together, subject to the usual +`no_transaction` opt-outs. `fake` and the `ForceMode` behaviors apply unchanged, with +"recorded" meaning the in-place update of REPEAT-6. + +## REPEAT-10 + +`migrant new --repeatable ` creates a migration directory containing only an `up.sql` +seeded with the `-- migrant:repeatable` directive, and no `down.sql` -- the shape REPEAT-4 +and REPEAT-7 require. + +## REPEAT-11 + +A `Down` run leaves repeatable rows in place (REPEAT-4), so after reverting and re-applying +every versioned migration a repeatable migration whose SQL has not changed stays skipped. +Re-running it is triggered by changing its SQL, which is the same signal as REPEAT-1. + +## REPEAT-12 + +`Migrator::rerun_repeatable(bool)` (default `false`) re-runs every repeatable migration on an +`Up` run whether or not its checksum changed, so re-running an unedited one does not require +touching its SQL. Everything else is unchanged: they still run after the pending versioned +migrations, still at most once per run (so an `all` run still terminates), and each still +records its checksum afterwards. It never re-applies a versioned migration, and has no effect +on a `Down` run. The CLI exposes it as `--rerun-repeatable` on `apply` and `redo`. + +## REPEAT-13 + +Because a `Down` run walks past repeatable migrations (REPEAT-4), `redo` reverts and re-applies +the most recent *versioned* migration, which may not be the migration the user was iterating +on. `migrant redo` therefore prints a note to stderr naming the applied repeatable migrations +it will not revert, and pointing at `--rerun-repeatable`. The note is suppressed when there are +no applied repeatable migrations, or when `--rerun-repeatable` was passed. + +Coverage: `migrant_lib/tests/sqlite.rs`, `server_dbs.rs`, `tests/migrant.rs`; unit tests in +`migrant_lib/src/migration.rs`, `migratable.rs`, `migrator.rs`, `ops.rs`, `drivers/mod.rs`, +`drivers/sqlite.rs`, and `src/status.rs`. diff --git a/src/cli.rs b/src/cli.rs index c035403..9bd8211 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -52,6 +52,28 @@ fn allow_out_of_order_arg() -> Arg { ) } +/// `--rerun-repeatable`: re-run repeatable migrations even if their SQL is unchanged. +fn rerun_repeatable_arg() -> Arg { + Arg::new("rerun-repeatable") + .long("rerun-repeatable") + .action(ArgAction::SetTrue) + .help( + "Re-run every repeatable migration even if its SQL has not changed, instead of \ + only the ones whose checksum differs", + ) +} + +/// `--allow-checksum-mismatch`: tolerate an already-applied migration whose SQL changed. +fn allow_checksum_mismatch_arg() -> Arg { + Arg::new("allow-checksum-mismatch") + .long("allow-checksum-mismatch") + .action(ArgAction::SetTrue) + .help( + "Tolerate an already-applied migration whose current checksum no longer matches \ + the recorded one, instead of aborting", + ) +} + pub fn build_cli() -> Command { Command::new("migrant") .version(env!("CARGO_PKG_VERSION")) @@ -175,8 +197,10 @@ pub fn build_cli() -> Command { .help("Updates the migration table without running the migration"), ) .arg(no_sync_arg()) + .arg(rerun_repeatable_arg()) .arg(allow_unknown_tags_arg()) - .arg(allow_out_of_order_arg()), + .arg(allow_out_of_order_arg()) + .arg(allow_checksum_mismatch_arg()), ) .subcommand( Command::new("redo") @@ -190,8 +214,10 @@ pub fn build_cli() -> Command { ) .arg(force_arg()) .arg(no_sync_arg()) + .arg(rerun_repeatable_arg()) .arg(allow_unknown_tags_arg()) - .arg(allow_out_of_order_arg()), + .arg(allow_out_of_order_arg()) + .arg(allow_checksum_mismatch_arg()), ) .subcommand( Command::new("new") @@ -200,6 +226,16 @@ pub fn build_cli() -> Command { Arg::new("tag") .required(true) .help("tag to use for new migration"), + ) + .arg( + Arg::new("repeatable") + .long("repeatable") + .action(ArgAction::SetTrue) + .help( + "Create a repeatable migration: an `up.sql` carrying the \ + `-- migrant:repeatable` directive and no `down.sql`. It re-runs \ + whenever the file's checksum changes", + ), ), ) .subcommand(Command::new("shell").about("Open a repl connection")) diff --git a/src/main.rs b/src/main.rs index 416c096..96f3b7d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -110,9 +110,15 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { let config = config.reload()?; let tag = matches.get_one::("tag").expect("required arg"); - let new_migration = migrant_lib::create_migration(&config, tag)?; + let new_migration = if matches.get_flag("repeatable") { + migrant_lib::create_repeatable_migration(&config, tag)? + } else { + migrant_lib::create_migration(&config, tag)? + }; println!("Created: {}", new_migration.up_path().display()); - println!("Created: {}", new_migration.down_path().display()); + if let Some(down) = new_migration.down_path() { + println!("Created: {}", down.display()); + } migrant_lib::cli::list(&config)?; } Some(("apply", matches)) => { @@ -124,6 +130,8 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { let no_sync = matches.get_flag("no-sync"); let allow_unknown_tags = matches.get_flag("allow-unknown-tags"); let allow_out_of_order = matches.get_flag("allow-out-of-order"); + let allow_checksum_mismatch = matches.get_flag("allow-checksum-mismatch"); + let rerun_repeatable = matches.get_flag("rerun-repeatable"); let direction = if matches.get_flag("down") { Direction::Down } else { @@ -141,8 +149,10 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .fake(fake) .all(false) .synchronized(!no_sync) + .rerun_repeatable(rerun_repeatable) .allow_unknown_tags(allow_unknown_tags) .allow_out_of_order(allow_out_of_order) + .allow_checksum_mismatch(allow_checksum_mismatch) .apply()?; if report.is_empty() { break; @@ -159,8 +169,10 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .fake(fake) .all(all) .synchronized(!no_sync) + .rerun_repeatable(rerun_repeatable) .allow_unknown_tags(allow_unknown_tags) .allow_out_of_order(allow_out_of_order) + .allow_checksum_mismatch(allow_checksum_mismatch) .apply()?; } } @@ -177,6 +189,10 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { let no_sync = matches.get_flag("no-sync"); let allow_unknown_tags = matches.get_flag("allow-unknown-tags"); let allow_out_of_order = matches.get_flag("allow-out-of-order"); + let allow_checksum_mismatch = matches.get_flag("allow-checksum-mismatch"); + let rerun_repeatable = matches.get_flag("rerun-repeatable"); + + warn_redo_skips_repeatable(&config, rerun_repeatable)?; Migrator::with_config(&config) .direction(Direction::Down) @@ -185,6 +201,7 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .synchronized(!no_sync) .allow_unknown_tags(allow_unknown_tags) .allow_out_of_order(allow_out_of_order) + .allow_checksum_mismatch(allow_checksum_mismatch) .apply()?; let config = config.reload()?; migrant_lib::cli::list(&config)?; @@ -194,8 +211,10 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .force(force) .all(all) .synchronized(!no_sync) + .rerun_repeatable(rerun_repeatable) .allow_unknown_tags(allow_unknown_tags) .allow_out_of_order(allow_out_of_order) + .allow_checksum_mismatch(allow_checksum_mismatch) .apply()?; let config = config.reload()?; migrant_lib::cli::list(&config)?; @@ -228,6 +247,37 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { Ok(()) } +/// Warn that `redo` will not revert the applied repeatable migrations. +/// +/// `redo` is the command people reach for while iterating on a migration, but a +/// repeatable migration is forward-only: the down phase walks past it to the +/// most recent *versioned* migration, so `redo` can revert and re-apply +/// something the user did not mean to touch. Say so before running rather than +/// leaving them to infer it from the reverted tag. +/// +/// Silent when nothing is affected, or when `--rerun-repeatable` already makes +/// the up phase re-run them. +fn warn_redo_skips_repeatable(config: &migrant_lib::Config, rerun_repeatable: bool) -> Result<()> { + if rerun_repeatable { + return Ok(()); + } + let applied_repeatable = migrant_lib::migration_statuses(config)? + .into_iter() + .filter(|s| s.repeatable() && s.applied()) + .map(|s| s.tag().to_string()) + .collect::>(); + if applied_repeatable.is_empty() { + return Ok(()); + } + eprintln!( + "Note: `redo` does not revert repeatable migrations ({}); it targets the most \ + recent versioned migration instead. The up phase re-runs a repeatable migration \ + only if its SQL changed -- pass `--rerun-repeatable` to run them regardless.", + applied_repeatable.join(", ") + ); + Ok(()) +} + /// Map the optional-valued `--force[=]` flag to a `ForceMode`. /// Bare `--force` carries the default-missing-value `accept-failures`. fn force_mode(matches: &clap::ArgMatches) -> Result { diff --git a/src/status.rs b/src/status.rs index 8125c02..e3ca40c 100644 --- a/src/status.rs +++ b/src/status.rs @@ -8,11 +8,37 @@ use migrant_lib::MigrationStatus; use serde::Serialize; -/// A single migration's tag and whether it is currently applied. +/// A single migration's tag, whether it is currently applied, and (for +/// repeatable migrations) whether it is due to re-run. #[derive(Debug, Clone, Serialize)] pub struct StatusRow { pub tag: String, pub applied: bool, + pub repeatable: bool, + pub stale: bool, +} + +impl StatusRow { + /// The status mark: applied, pending, or applied but due to re-run. + fn mark(&self) -> char { + match (self.applied, self.stale) { + (true, true) => '~', + (true, false) => '✓', + (false, _) => ' ', + } + } + + /// The trailing annotation, empty for versioned migrations. A repeatable + /// migration that has never run "will run"; only one with a recorded row it + /// no longer matches "will re-run". + fn note(&self) -> &'static str { + match (self.repeatable, self.stale, self.applied) { + (false, _, _) => "", + (true, true, true) => " (repeatable, will re-run)", + (true, true, false) => " (repeatable, will run)", + (true, false, _) => " (repeatable)", + } + } } /// The full migration-table status: per-migration rows plus summary counts. @@ -21,6 +47,13 @@ pub struct StatusReport { pub total: usize, pub applied: usize, pub pending: usize, + /// Applied repeatable migrations that will re-run on the next `Up` run. + /// + /// Note this is narrower than the per-row `stale` flag, which is true for + /// anything that would run next (including a versioned migration with no + /// row yet). Those are counted in `pending` instead, so `applied + pending` + /// still equals `total` and no migration is counted twice. + pub stale: usize, pub migrations: Vec, } @@ -33,30 +66,36 @@ impl StatusReport { .map(|s| StatusRow { tag: s.tag().to_string(), applied: s.applied(), + repeatable: s.repeatable(), + stale: s.stale(), }) .collect(); let applied = migrations.iter().filter(|r| r.applied).count(); + let stale = migrations.iter().filter(|r| r.applied && r.stale).count(); StatusReport { total: migrations.len(), applied, pending: migrations.len() - applied, + stale, migrations, } } /// Render the report as human-readable text: a summary line followed by one - /// `[✓]`/`[ ]` row per migration. + /// `[✓]`/`[ ]`/`[~]` row per migration. pub fn render_text(&self) -> String { let mut out = format!( - "Migration status: {} applied, {} pending ({} total)", - self.applied, self.pending, self.total + "Migration status: {} applied, {} pending", + self.applied, self.pending ); + // Only mentioned when there is something to report, so the summary line + // is unchanged for projects with no repeatable migrations. + if self.stale > 0 { + out.push_str(&format!(", {} stale", self.stale)); + } + out.push_str(&format!(" ({} total)", self.total)); for row in &self.migrations { - out.push_str(&format!( - "\n [{}] {}", - if row.applied { '✓' } else { ' ' }, - row.tag - )); + out.push_str(&format!("\n [{}] {}{}", row.mark(), row.tag, row.note())); } out } @@ -76,25 +115,59 @@ mod tests { StatusRow { tag: "20170812145327_initial".to_string(), applied: true, + repeatable: false, + stale: false, }, StatusRow { tag: "20171126194042_second".to_string(), applied: false, + repeatable: false, + stale: true, }, ] } - fn report() -> StatusReport { - let migrations = rows(); + fn build(migrations: Vec) -> StatusReport { let applied = migrations.iter().filter(|r| r.applied).count(); + let stale = migrations.iter().filter(|r| r.applied && r.stale).count(); StatusReport { total: migrations.len(), applied, pending: migrations.len() - applied, + stale, migrations, } } + fn report() -> StatusReport { + build(rows()) + } + + /// An up-to-date repeatable migration and a stale one, alongside an applied + /// versioned migration. + fn repeatable_report() -> StatusReport { + build(vec![ + StatusRow { + tag: "20170812145327_initial".to_string(), + applied: true, + repeatable: false, + stale: false, + }, + StatusRow { + tag: "20171126194042_seed-roles".to_string(), + applied: true, + repeatable: true, + stale: false, + }, + StatusRow { + tag: "20171126194043_refresh-views".to_string(), + applied: true, + repeatable: true, + stale: true, + }, + ]) + } + #[test] fn counts_reflect_rows() { let r = report(); @@ -122,6 +195,7 @@ mod tests { total: 0, applied: 0, pending: 0, + stale: 0, migrations: vec![], }; let text = r.render_text(); @@ -135,8 +209,59 @@ mod tests { assert_eq!(value["total"], 2); assert_eq!(value["applied"], 1); assert_eq!(value["pending"], 1); + assert_eq!(value["stale"], 0); assert_eq!(value["migrations"][0]["tag"], "20170812145327_initial"); assert_eq!(value["migrations"][0]["applied"], true); + assert_eq!(value["migrations"][0]["repeatable"], false); + assert_eq!(value["migrations"][0]["stale"], false); assert_eq!(value["migrations"][1]["applied"], false); } + + // REPEAT-8 + #[test] + fn repeatable_rows_are_annotated_and_stale_ones_marked() { + let text = repeatable_report().render_text(); + // An up-to-date repeatable migration is applied and annotated. + assert!( + text.contains("[✓] 20171126194042_seed-roles (repeatable)"), + "unexpected up-to-date repeatable row: {text}" + ); + // A stale one keeps its row but is marked as due to re-run. + assert!( + text.contains("[~] 20171126194043_refresh-views (repeatable, will re-run)"), + "unexpected stale repeatable row: {text}" + ); + // Versioned migrations are unannotated. + assert!(text.contains("[✓] 20170812145327_initial\n")); + } + + // REPEAT-8 + #[test] + fn summary_reports_stale_only_when_non_zero() { + // A stale repeatable migration is applied (it has a row) and counted + // separately from `pending`, which covers migrations with no row yet. + let text = repeatable_report().render_text(); + assert!( + text.starts_with("Migration status: 3 applied, 0 pending, 1 stale (3 total)"), + "unexpected summary line: {text}" + ); + // With nothing stale the summary is unchanged from a project with no + // repeatable migrations at all. + assert!(report() + .render_text() + .starts_with("Migration status: 1 applied, 1 pending (2 total)")); + } + + // REPEAT-8 + #[test] + fn json_carries_repeatable_and_stale_fields() { + let json = repeatable_report().render_json().unwrap(); + let value: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(value["stale"], 1); + assert_eq!(value["migrations"][1]["repeatable"], true); + assert_eq!(value["migrations"][1]["stale"], false); + assert_eq!(value["migrations"][2]["repeatable"], true); + assert_eq!(value["migrations"][2]["stale"], true); + assert_eq!(value["migrations"][2]["applied"], true); + } } diff --git a/src/tui.rs b/src/tui.rs index 4916752..f3f7d49 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -175,14 +175,25 @@ impl App { .statuses .iter() .map(|mig| { - let (mark, style) = if mig.applied() { - ("✓", Style::new().fg(Color::Green)) - } else { - (" ", Style::new().fg(Color::DarkGray)) + // An applied repeatable migration whose SQL changed is marked + // stale: it is recorded, but the next up run re-runs it. + let (mark, style) = match (mig.applied(), mig.stale()) { + (true, true) => ("~", Style::new().fg(Color::Yellow)), + (true, false) => ("✓", Style::new().fg(Color::Green)), + (false, _) => (" ", Style::new().fg(Color::DarkGray)), + }; + // Matches the `list`/`status` annotations so the three + // renderings of a migration row agree. + let note = match (mig.repeatable(), mig.stale(), mig.applied()) { + (false, _, _) => "", + (true, true, true) => " (repeatable, will re-run)", + (true, true, false) => " (repeatable, will run)", + (true, false, _) => " (repeatable)", }; ListItem::new(Line::from(vec![ Span::styled(format!("[{}] ", mark), style), Span::raw(mig.tag().to_string()), + Span::styled(note, Style::new().fg(Color::DarkGray)), ])) }) .collect::>(); diff --git a/tests/migrant.rs b/tests/migrant.rs index 562fe81..9d4da09 100644 --- a/tests/migrant.rs +++ b/tests/migrant.rs @@ -10,6 +10,7 @@ #![cfg(all(feature = "integration_tests", feature = "sqlite"))] use assert_cmd::Command; +use predicates::prelude::PredicateBooleanExt; use predicates::str::contains; fn migrant() -> Command { @@ -239,6 +240,22 @@ fn remove_migration(dir: &std::path::Path, tag: &str) { std::fs::remove_dir_all(&mig_dir).expect("remove migration dir"); } +/// Rewrite the up.sql of the on-disk migration whose name ends in `_`, +/// changing its checksum as if the file were edited after it was applied. +fn edit_migration_up(dir: &std::path::Path, tag: &str, up: &str) { + let migrations = dir.join("migrations"); + let mig_dir = std::fs::read_dir(&migrations) + .expect("read migrations dir") + .map(|e| e.expect("dir entry").path()) + .find(|p| { + p.file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.ends_with(&format!("_{}", tag))) + }) + .unwrap_or_else(|| panic!("migration dir for `{}` not found", tag)); + std::fs::write(mig_dir.join("up.sql"), up).expect("write up.sql"); +} + // CLIMIG: `apply --step N --down` reverts at most N applied migrations // (newest-first) and stops early once nothing remains, mirroring the up // direction. Only the up direction was exercised before. @@ -417,6 +434,54 @@ fn apply_out_of_order_rejected_then_allowed() { .stdout(predicates::str::is_match(r"\[✓\] \d{14}_a-bad").expect("valid regex")); } +// DRIFT-1/DRIFT-6: an already-applied migration whose up.sql is edited aborts a +// following `apply` with a checksum mismatch, and `--allow-checksum-mismatch` +// proceeds past the drift. +#[test] +fn apply_checksum_drift_rejected_then_allowed() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "seed", + "create table seed (x integer);", + "drop table seed;", + ); + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + + // Edit the applied migration's up.sql: its checksum no longer matches. + edit_migration_up( + dir.path(), + "seed", + "create table seed (x integer, y integer);", + ); + + // A plain up run detects the drift and aborts before applying anything. + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .failure() + .stderr(contains("ChecksumMismatch")) + .stderr(contains("has changed since it was applied")); + + // Opting out lets the run proceed (the migration is already applied, so + // there is nothing left to do). + migrant() + .current_dir(dir.path()) + .args(["apply", "--allow-checksum-mismatch"]) + .assert() + .success(); +} + // CLIMIG-3: `new` reports the created up/down file paths on stdout. #[test] fn new_reports_created_paths() { @@ -440,6 +505,347 @@ fn new_reports_created_paths() { ); } +/// Create a repeatable migration via `migrant new --repeatable` and overwrite +/// its up file, keeping the `-- migrant:repeatable` directive. +fn new_repeatable_migration(dir: &std::path::Path, tag: &str, up: &str) { + migrant() + .current_dir(dir) + .args(["new", "--repeatable", tag]) + .assert() + .success(); + edit_migration_up(dir, tag, &format!("-- migrant:repeatable\n{}", up)); +} + +// CLIMIG-1/REPEAT-10: `new --repeatable` creates only a seeded up.sql. +#[test] +fn new_repeatable_creates_only_a_seeded_up_file() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + + migrant() + .current_dir(dir.path()) + .args(["new", "--repeatable", "seed-roles"]) + .assert() + .success() + .stdout( + predicates::str::is_match(r"Created: .*_seed-roles[\\/]+up\.sql").expect("valid regex"), + ) + .stdout(contains("down.sql").not()); + + let mig_dir = std::fs::read_dir(dir.path().join("migrations")) + .expect("read migrations dir") + .map(|e| e.expect("dir entry").path()) + .find(|p| { + p.file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.ends_with("_seed-roles")) + }) + .expect("migration dir created"); + assert!(!mig_dir.join("down.sql").exists(), "no down.sql is created"); + let up = std::fs::read_to_string(mig_dir.join("up.sql")).expect("read up.sql"); + assert_eq!("-- migrant:repeatable\n", up); +} + +// REPEAT-1/REPEAT-8/CLIMIG-6: a repeatable migration re-runs when its up file +// changes, and `status` reports it as repeatable and stale beforehand. +#[test] +fn repeatable_migration_reruns_through_the_cli() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "create-roles", + "create table roles (name text);", + "drop table roles;", + ); + new_repeatable_migration( + dir.path(), + "seed-roles", + "insert into roles (name) values ('admin');", + ); + + // Before the first run the repeatable migration is pending and stale. + migrant() + .current_dir(dir.path()) + .args(["status", "--format", "json"]) + .assert() + .success() + .stdout(contains("\"repeatable\": true")) + .stdout(contains("\"stale\": true")); + + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + + // Applied and up to date: annotated but not marked for a re-run. + migrant() + .current_dir(dir.path()) + .arg("status") + .assert() + .success() + .stdout(contains("(repeatable)")) + .stdout(contains("will re-run").not()) + .stdout(contains("0 pending")); + + // Re-running with nothing changed does not re-apply it. + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success() + .stdout(contains("Re-applying").not()); + + // Editing the up file changes its checksum, which is the re-run signal + // rather than the drift error a versioned migration would raise. + edit_migration_up( + dir.path(), + "seed-roles", + "-- migrant:repeatable\ninsert into roles (name) values ('editor');", + ); + migrant() + .current_dir(dir.path()) + .arg("status") + .assert() + .success() + .stdout(contains("(repeatable, will re-run)")) + .stdout(contains("1 stale")); + + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success() + .stdout(contains("Re-applying[Up]")); + + // Both seeds ran, and the tag still has exactly one bookkeeping row. + let db = rusqlite::Connection::open(dir.path().join("db.db")).expect("open db"); + let names: Vec = db + .prepare("select name from roles order by name") + .expect("prepare") + .query_map([], |r| r.get(0)) + .expect("query") + .collect::>() + .expect("rows"); + assert_eq!(names, ["admin", "editor"]); + let rows: i64 = db + .query_row( + "select count(*) from __migrant_migrations where tag like '%seed-roles'", + [], + |r| r.get(0), + ) + .expect("count rows"); + assert_eq!(1, rows, "a re-run updates the row in place"); + let repeatable: bool = db + .query_row( + "select is_repeatable from __migrant_migrations where tag like '%seed-roles'", + [], + |r| r.get(0), + ) + .expect("read is_repeatable"); + assert!(repeatable, "the row records the migration's kind"); +} + +// REPEAT-12: `apply --rerun-repeatable` re-runs an unchanged repeatable +// migration; without it nothing runs. +#[test] +fn apply_rerun_repeatable_runs_unchanged_migrations() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "create-roles", + "create table roles (name text);", + "drop table roles;", + ); + new_repeatable_migration( + dir.path(), + "seed-roles", + "insert into roles (name) values ('admin');", + ); + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + + // Nothing changed, so a plain re-apply does nothing. + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success() + .stdout(contains("Re-applying").not()); + + // The flag runs it anyway. + migrant() + .current_dir(dir.path()) + .args(["apply", "--rerun-repeatable"]) + .assert() + .success() + .stdout(contains("Re-applying[Up]")); + + let db = rusqlite::Connection::open(dir.path().join("db.db")).expect("open db"); + let count: i64 = db + .query_row("select count(*) from roles", [], |r| r.get(0)) + .expect("count roles"); + assert_eq!(2, count, "the seed ran a second time"); + // Still one bookkeeping row for the tag. + let rows: i64 = db + .query_row( + "select count(*) from __migrant_migrations where tag like '%seed-roles'", + [], + |r| r.get(0), + ) + .expect("count rows"); + assert_eq!(1, rows); +} + +// REPEAT-13: `redo` warns that it does not revert repeatable migrations. +#[test] +fn redo_warns_that_it_skips_repeatable_migrations() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "create-roles", + "create table roles (name text);", + "drop table roles;", + ); + + // With no repeatable migrations there is nothing to warn about. + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + migrant() + .current_dir(dir.path()) + .arg("redo") + .assert() + .success() + .stderr(contains("does not revert repeatable").not()); + + // Once one is applied, `redo` says so, naming it, and points at the flag. + new_repeatable_migration( + dir.path(), + "seed-roles", + "insert into roles (name) values ('admin');", + ); + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + migrant() + .current_dir(dir.path()) + .arg("redo") + .assert() + .success() + .stderr(contains("does not revert repeatable migrations")) + .stderr(contains("seed-roles")) + .stderr(contains("--rerun-repeatable")); + + // `--rerun-repeatable` makes the note moot, so it is suppressed. + migrant() + .current_dir(dir.path()) + .args(["redo", "--rerun-repeatable"]) + .assert() + .success() + .stderr(contains("does not revert repeatable").not()); +} + +// REPEAT-4: `apply --down` never reverts a repeatable migration. +#[test] +fn apply_down_leaves_repeatable_migrations_recorded() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "create-roles", + "create table roles (name text);", + "drop table roles;", + ); + new_repeatable_migration( + dir.path(), + "seed-roles", + "insert into roles (name) values ('admin');", + ); + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .success(); + + // The repeatable migration was applied last, but the down run targets the + // versioned one and leaves the repeatable row alone. + migrant() + .current_dir(dir.path()) + .args(["apply", "-d", "--step", "100"]) + .assert() + .success(); + + let db = rusqlite::Connection::open(dir.path().join("db.db")).expect("open db"); + let remaining: Vec = db + .prepare("select tag from __migrant_migrations order by id") + .expect("prepare") + .query_map([], |r| r.get(0)) + .expect("query") + .collect::>() + .expect("rows"); + assert_eq!(1, remaining.len(), "only the repeatable row remains"); + assert!(remaining[0].ends_with("_seed-roles")); +} + +// REPEAT-4/REPEAT-7: a repeatable migration that also defines a down direction +// is rejected rather than silently ignored. +#[test] +fn repeatable_migration_with_a_down_file_is_rejected() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + // `new` (not `new --repeatable`) writes a down.sql; adding the directive to + // the up file then makes the pair invalid. + new_migration( + dir.path(), + "seed-roles", + "-- migrant:repeatable\ninsert into roles (name) values ('admin');", + "delete from roles;", + ); + + migrant() + .current_dir(dir.path()) + .arg("apply") + .assert() + .failure() + .stderr(contains("seed-roles")) + .stderr(contains("down")); +} + // REGRESSION (migrant_lib same-second ordering fix): migrations that share the // exact same timestamp second must have a deterministic definition order. // @@ -873,10 +1279,10 @@ fn apply_down_default_reverts_one() { .stdout(predicates::str::is_match(r"\[ \] \d{14}_two").expect("valid regex")); } -// CLIMIG-4: `--allow-unknown-tags` / `--allow-out-of-order` are accepted by -// both `apply` and `redo`. The full unknown-tag/out-of-order scenarios are -// covered in the library's own test suite; here we only prove the CLI wires -// the flags through without rejecting them. +// CLIMIG-4/DRIFT-6: `--allow-unknown-tags` / `--allow-out-of-order` / +// `--allow-checksum-mismatch` are accepted by both `apply` and `redo`. The full +// scenarios are covered in the library's own test suite; here we only prove the +// CLI wires the flags through without rejecting them. #[test] fn allow_flags_are_accepted_by_apply_and_redo() { let dir = sqlite_project(); @@ -894,13 +1300,23 @@ fn allow_flags_are_accepted_by_apply_and_redo() { migrant() .current_dir(dir.path()) - .args(["apply", "--allow-unknown-tags", "--allow-out-of-order"]) + .args([ + "apply", + "--allow-unknown-tags", + "--allow-out-of-order", + "--allow-checksum-mismatch", + ]) .assert() .success(); migrant() .current_dir(dir.path()) - .args(["redo", "--allow-unknown-tags", "--allow-out-of-order"]) + .args([ + "redo", + "--allow-unknown-tags", + "--allow-out-of-order", + "--allow-checksum-mismatch", + ]) .assert() .success(); } From de9abc5559f6cf0788ac0d42126334bf156587d7 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Mon, 17 Aug 2026 22:02:16 -0400 Subject: [PATCH 2/2] isolate `kitchen_sink` from the repo dev database The test ran in the repo root against `db/migrant.db`, which persists between runs. A change to the bookkeeping schema then breaks the next run against that stale file, which CI never reproduces because it starts from a fresh checkout. It now copies the repo's `Migrant.toml` and `migrations/` into a tempdir and runs there, like the other CLI tests, so every run starts from an empty database and nothing is written to the working tree. --- CONTRIBUTING.md | 10 ++++++++-- tests/migrant.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 70f5ac6..5ac8668 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,12 +20,18 @@ The `postgres` feature needs `libpq-dev` at build time on linux. ## Running Tests -The CLI integration tests use the repo's `Migrant.toml` (sqlite) and `migrations/` directory: - ```bash cargo test --features sqlite,integration_tests ``` +The CLI integration tests copy the repo's `Migrant.toml` (sqlite) and `migrations/` directory +into a tempdir and run there, so each run starts from an empty database and nothing is written +to your working tree. There is no dev database to set up or reset. + +`db/migrant.db` is only created if you run `migrant` against the repo yourself, and is +gitignored. It is a scratch playground: delete it any time, and `migrant setup` recreates it. +Nothing in the test suite reads it, so a stale one cannot break a run. + Library tests live in `migrant_lib/` (see its CONTRIBUTING for postgres/mysql setup). diff --git a/tests/migrant.rs b/tests/migrant.rs index 9d4da09..b479976 100644 --- a/tests/migrant.rs +++ b/tests/migrant.rs @@ -17,8 +17,50 @@ fn migrant() -> Command { Command::cargo_bin("migrant").expect("binary built") } +/// A tempdir holding a copy of the repo's own `Migrant.toml` and `migrations/` +/// directory, so the project committed at the repo root is exercised without +/// running against it in place. +/// +/// Running in place would leave a `db/migrant.db` behind that outlives the test. +/// A later change to the bookkeeping schema then breaks the *next* run against +/// that stale file, which CI never reproduces because it always starts from a +/// fresh checkout. Copying keeps the fixture and makes every run start clean. +fn repo_project() -> tempfile::TempDir { + let repo = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); + let dir = tempfile::tempdir().expect("create tempdir"); + std::fs::copy(repo.join("Migrant.toml"), dir.path().join("Migrant.toml")) + .expect("copy Migrant.toml"); + copy_dir(&repo.join("migrations"), &dir.path().join("migrations")); + dir +} + +/// Recursively copy the contents of `from` into `to`, creating `to` if needed. +fn copy_dir(from: &std::path::Path, to: &std::path::Path) { + std::fs::create_dir_all(to).expect("create dir"); + for entry in std::fs::read_dir(from).expect("read dir") { + let entry = entry.expect("dir entry"); + let target = to.join(entry.file_name()); + if entry.file_type().expect("file type").is_dir() { + copy_dir(&entry.path(), &target); + } else { + std::fs::copy(entry.path(), &target).expect("copy file"); + } + } +} + #[test] fn kitchen_sink() { + // A copy of the repo's own project, so this starts from an empty database + // every run (see `repo_project`). + let project = repo_project(); + // Shadows the module-level helper so every invocation below runs inside the + // copied project rather than the repo root. + let migrant = || { + let mut cmd = Command::cargo_bin("migrant").expect("binary built"); + cmd.current_dir(project.path()); + cmd + }; + // make sure we're setup and back to no applied migrations. `--step` with // a count comfortably larger than the number of migrations reverts // everything and stops early once nothing remains.