From 4645915c0a836fd2128ea4573911e29eed949370 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sat, 18 Jul 2026 22:03:19 -0400 Subject: [PATCH] release 1.0.0-rc.2 - settings builders and `Migrator` now use owned `self` chains - `SettingsFileInitializer::with_*_options` take the builder by value - `MigrationStatus` fields are private; use `tag()`/`applied()` accessors - `DbKind` and `ForceMode` are now `#[non_exhaustive]` - add `--no-sync` to `apply`/`redo` to disable the advisory lock - keep `Config` on `&mut self` --- CHANGELOG.md | 11 +++ Cargo.lock | 4 +- Cargo.toml | 4 +- README.md | 4 +- docs/src/cli.md | 10 ++- docs/src/concurrency.md | 3 + migrant_lib/CHANGELOG.md | 17 ++++ migrant_lib/Cargo.toml | 2 +- migrant_lib/README.md | 8 +- migrant_lib/src/config/builders.rs | 110 ++++++++++++++++++++++--- migrant_lib/src/config/init.rs | 25 +++--- migrant_lib/src/lib.rs | 1 + migrant_lib/src/migrator.rs | 38 +++++++-- migrant_lib/src/ops.rs | 35 +++++++- migrant_lib/tests/reload_memory.rs | 4 +- migrant_lib/tests/server_dbs.rs | 8 +- migrant_lib/tests/sqlite.rs | 4 +- spec/advisory-locking.md | 8 ++ spec/config-file-and-env-resolution.md | 6 ++ spec/error-handling-api.md | 11 +++ spec/migrator-api.md | 15 ++++ spec/settings-builders.md | 16 ++++ src/cli.rs | 17 +++- src/main.rs | 27 +++--- src/tui.rs | 8 +- tests/migrant.rs | 57 +++++++++++++ 26 files changed, 380 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b6a1ee..470415c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [1.0.0-rc.2] +### Added +- `apply` and `redo` accept `--no-sync` to disable the cross-process advisory lock, for when + migration runs are serialized by an external mechanism. On `redo` it applies to both the down + and up runs + +### Changed +- Update `migrant_lib` to 1.0.0-rc.2: owned-`self` builder chains for the settings builders and + `Migrator`, `#[non_exhaustive]` `DbKind`/`ForceMode`, and `MigrationStatus` field accessors. + See the migrant_lib changelog. These are source-compatible for existing chained call sites + ## [1.0.0-rc.1] ### Added - `--force` takes an optional mode: bare `--force` (or `--force=accept-failures`) records a diff --git a/Cargo.lock b/Cargo.lock index 48051b3..1583b70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1575,7 +1575,7 @@ dependencies = [ [[package]] name = "migrant" -version = "1.0.0-rc.1" +version = "1.0.0-rc.2" dependencies = [ "assert_cmd", "clap", @@ -1592,7 +1592,7 @@ dependencies = [ [[package]] name = "migrant_lib" -version = "1.0.0-rc.1" +version = "1.0.0-rc.2" dependencies = [ "chrono", "log", diff --git a/Cargo.toml b/Cargo.toml index 4573d73..95822bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "migrant" -version = "1.0.0-rc.1" +version = "1.0.0-rc.2" edition.workspace = true rust-version.workspace = true authors.workspace = true @@ -26,7 +26,7 @@ authors = ["James Kominick "] license = "MIT" [workspace.dependencies] -migrant_lib = { path = "migrant_lib", version = "1.0.0-rc.1" } +migrant_lib = { path = "migrant_lib", version = "1.0.0-rc.2" } # shared between the cli and the library rusqlite = "0.40" diff --git a/README.md b/README.md index 50f6fec..a8aa09d 100644 --- a/README.md +++ b/README.md @@ -75,9 +75,9 @@ When run interactively (without `--no-confirm`), `setup` will be run automatical `migrant list` - Display all available .sql files and mark those applied. -`migrant apply [--down, --all, --force, --fake]` - Apply the next available migration[s]. +`migrant apply [--down, --all, --force, --fake, --no-sync]` - Apply the next available migration[s]. -`migrant redo [--all, --force]` - Re-apply the latest migration (down then up). +`migrant redo [--all, --force, --no-sync]` - Re-apply the latest migration (down then up). `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 a0d984b..4fd43ad 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -36,17 +36,21 @@ run from anywhere inside the project; migrant searches upward for the config. `migrant list` : List available migrations and mark those applied. -`migrant apply [--down] [--all] [--force[=]] [--fake]` +`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[=]]` +`migrant redo [--all] [--force[=]] [--no-sync]` : Shortcut for the latest `down` then `up`. Useful while iterating on a migration - you are still writing. + you are still writing. `--no-sync` disables the advisory lock for both the + down and up runs. ## Inspect and connect diff --git a/docs/src/concurrency.md b/docs/src/concurrency.md index 8830764..f3e95a8 100644 --- a/docs/src/concurrency.md +++ b/docs/src/concurrency.md @@ -48,4 +48,7 @@ Migrator::with_config(&config) .apply()?; ``` +The `migrant` CLI exposes the same toggle as `--no-sync` on `apply` and `redo`, +for callers that serialize migrations externally; see [The CLI](cli.md). + See [Using migrant_lib](library.md) for the rest of the `Migrator` API. diff --git a/migrant_lib/CHANGELOG.md b/migrant_lib/CHANGELOG.md index 427b298..d64a720 100644 --- a/migrant_lib/CHANGELOG.md +++ b/migrant_lib/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## [1.0.0-rc.2] +Breaking pre-1.0 release, continuing the API cleanup from rc.1. + +### Changed +- Settings builders (`SqliteSettingsBuilder`/`PostgresSettingsBuilder`/`MySqlSettingsBuilder`) and + `SettingsFileInitializer` now use owned `self` builder chains: their setters take and return + `Self` instead of `&mut Self`, unifying them with the migration builders. `build(&self)` is + unchanged. `SettingsFileInitializer::with_sqlite_options`/`with_postgres_options`/ + `with_mysql_options` now take the builder by value instead of by reference +- `Migrator` setters (`direction`/`force`/`fake`/`all`/`show_output`/`synchronized`) now take and + return owned `self`. `Migrator::with_config` and `Migrator::apply` are unchanged +- `MigrationStatus` fields `tag`/`applied` are now private; read them via the `tag()` and + `applied()` accessor methods +- `DbKind` and `ForceMode` are now `#[non_exhaustive]` (like `Error`), so a downstream `match` + must include a wildcard arm. This allows future backends and force modes to be added without a + breaking change + ## [1.0.0-rc.1] Breaking pre-1.0 release. See the "Changed"/"Removed" sections for migration notes. diff --git a/migrant_lib/Cargo.toml b/migrant_lib/Cargo.toml index cc84f16..70e945b 100644 --- a/migrant_lib/Cargo.toml +++ b/migrant_lib/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "migrant_lib" -version = "1.0.0-rc.1" +version = "1.0.0-rc.2" # explicit (not workspace-inherited) so `cargo readme` can parse the manifest edition = "2021" rust-version = "1.88" diff --git a/migrant_lib/README.md b/migrant_lib/README.md index d720174..2fa0deb 100644 --- a/migrant_lib/README.md +++ b/migrant_lib/README.md @@ -30,7 +30,7 @@ include `rusqlite` and enable the `bundled` feature in your project. -### Usage +## Usage - Migrations can be defined as files, string literals, or functions. - File migrations can be either read from files at runtime or embedded in your executable at compile time @@ -76,7 +76,7 @@ config.use_migrations(&[ ``` -### In-memory sqlite databases +## In-memory sqlite databases With the `sqlite` feature, the special database path `:memory:` selects an in-memory sqlite database. The underlying connection is established once and @@ -93,7 +93,7 @@ config.setup()?; ``` -### CLI Compatibility +## CLI Compatibility Migration management identical to the [`migrant`](https://github.com/jaemk/migrant) CLI tool can also be embedded. This method only supports file-based migrations (so `FileMigration`s or `EmbeddedMigration`s using `include_str!`) @@ -110,7 +110,7 @@ example for a working sample where the `migrant` CLI tool can be used during dev and migration file contents are embedded in the application. -### Development +## Development See [CONTRIBUTING](https://github.com/jaemk/migrant/blob/main/migrant_lib/CONTRIBUTING.md) diff --git a/migrant_lib/src/config/builders.rs b/migrant_lib/src/config/builders.rs index 071d64a..ce0d161 100644 --- a/migrant_lib/src/config/builders.rs +++ b/migrant_lib/src/config/builders.rs @@ -38,7 +38,7 @@ impl SqliteSettingsBuilder { /// `Config::init_in(...).with_sqlite_options(...)`, a relative path is also accepted -- /// it is written into the generated settings file and resolved relative to that /// settings file's directory when the config is later loaded. - pub fn database_path>(&mut self, p: T) -> Result<&mut Self> { + pub fn database_path>(mut self, p: T) -> Result { self.database_path = Some(path_to_string(p.as_ref())?); Ok(self) } @@ -48,7 +48,7 @@ impl SqliteSettingsBuilder { /// The database connection is established once and then kept alive /// (shared by all clones of the built `Config`) so migrations and /// application queries all see the same database. - pub fn memory(&mut self) -> &mut Self { + pub fn memory(mut self) -> Self { self.database_path = Some(SQLITE_MEMORY_PATH.to_string()); self } @@ -58,7 +58,7 @@ impl SqliteSettingsBuilder { /// This can be an absolute or relative path. An absolute path should be preferred. /// If a relative path is provided, the path will be assumed relative to either the /// settings file's directory if a settings file exists, or the current directory. - pub fn migration_location>(&mut self, p: T) -> Result<&mut Self> { + pub fn migration_location>(mut self, p: T) -> Result { self.migration_location = Some(path_to_string(p.as_ref())?); Ok(self) } @@ -132,37 +132,37 @@ impl ServerSettingsBuilder { macro_rules! server_builder_methods { () => { /// **Required** -- Set the database name. - pub fn database_name(&mut self, name: &str) -> &mut Self { + pub fn database_name(mut self, name: &str) -> Self { self.inner.database_name = Some(name.into()); self } /// **Required** -- Set the database user. - pub fn database_user(&mut self, user: &str) -> &mut Self { + pub fn database_user(mut self, user: &str) -> Self { self.inner.database_user = Some(user.into()); self } /// **Required** -- Set the database password. - pub fn database_password(&mut self, pass: &str) -> &mut Self { + pub fn database_password(mut self, pass: &str) -> Self { self.inner.database_password = Some(pass.into()); self } /// Set the database host. - pub fn database_host(&mut self, host: &str) -> &mut Self { + pub fn database_host(mut self, host: &str) -> Self { self.inner.database_host = Some(host.into()); self } /// Set the database port. - pub fn database_port(&mut self, port: u16) -> &mut Self { + pub fn database_port(mut self, port: u16) -> Self { self.inner.database_port = Some(port.to_string()); self } /// Set a collection of database connection parameters. - pub fn database_params(&mut self, params: &[(&str, &str)]) -> &mut Self { + pub fn database_params(mut self, params: &[(&str, &str)]) -> Self { self.inner.set_params(params); self } @@ -172,7 +172,7 @@ macro_rules! server_builder_methods { /// This can be an absolute or relative path. An absolute path should be preferred. /// If a relative path is provided, the path will be assumed relative to either the /// settings file's directory if a settings file exists, or the current directory. - pub fn migration_location>(&mut self, p: T) -> Result<&mut Self> { + pub fn migration_location>(mut self, p: T) -> Result { self.inner.migration_location = Some(path_to_string(p.as_ref())?); Ok(self) } @@ -194,7 +194,7 @@ impl PostgresSettingsBuilder { server_builder_methods!(); /// Set a custom ssl cert file - pub fn ssl_cert_file>(&mut self, file: P) -> &mut Self { + pub fn ssl_cert_file>(mut self, file: P) -> Self { self.inner.ssl_cert_file = Some(file.as_ref().to_path_buf()); self } @@ -224,3 +224,91 @@ impl MySqlSettingsBuilder { Ok(Settings::new(DbSettings::MySql(self.inner.build()?))) } } + +#[cfg(test)] +mod tests { + use super::*; + + // The owned (`self -> Self` / `self -> Result`) setters must chain + // without an intermediate `mut` binding and carry every value through. + + #[test] + fn sqlite_owned_setters_chain_and_build() { + let settings = SqliteSettingsBuilder::empty() + .database_path("/abs/path/to/my.db") + .unwrap() + .migration_location("/abs/migrations") + .unwrap() + .build() + .unwrap(); + match settings.inner { + DbSettings::Sqlite(s) => { + assert_eq!(s.database_path, "/abs/path/to/my.db"); + assert_eq!(s.migration_location.as_deref(), Some("/abs/migrations")); + } + other => panic!("expected sqlite settings, got {:?}", other), + } + } + + #[test] + fn sqlite_memory_owned_setter_chains() { + // `memory()` consumes and returns owned self, chainable into `build()`. + let builder = SqliteSettingsBuilder::empty().memory(); + assert_eq!(builder.database_path.as_deref(), Some(SQLITE_MEMORY_PATH)); + let settings = builder.build().unwrap(); + match settings.inner { + DbSettings::Sqlite(s) => assert_eq!(s.database_path, SQLITE_MEMORY_PATH), + other => panic!("expected sqlite settings, got {:?}", other), + } + } + + #[test] + fn postgres_owned_server_setters_chain_and_build() { + let settings = PostgresSettingsBuilder::empty() + .database_name("mydb") + .database_user("me") + .database_password("secret") + .database_host("db.example.com") + .database_port(4444) + .database_params(&[("sslmode", "require")]) + .ssl_cert_file("/certs/db.pem") + .migration_location("/abs/migrations") + .unwrap() + .build() + .unwrap(); + match settings.inner { + DbSettings::Postgres(s) => { + assert_eq!(s.database_name, "mydb"); + assert_eq!(s.database_user, "me"); + assert_eq!(s.database_password, "secret"); + assert_eq!(s.database_host.as_deref(), Some("db.example.com")); + assert_eq!(s.database_port.as_deref(), Some("4444")); + assert_eq!( + s.database_params.unwrap().get("sslmode"), + Some(&"require".to_string()) + ); + assert_eq!(s.ssl_cert_file, Some(PathBuf::from("/certs/db.pem"))); + assert_eq!(s.migration_location.as_deref(), Some("/abs/migrations")); + } + other => panic!("expected postgres settings, got {:?}", other), + } + } + + #[test] + fn mysql_owned_server_setters_chain_and_build() { + let settings = MySqlSettingsBuilder::empty() + .database_name("mydb") + .database_user("me") + .database_password("secret") + .database_port(3307) + .build() + .unwrap(); + match settings.inner { + DbSettings::MySql(s) => { + assert_eq!(s.database_name, "mydb"); + assert_eq!(s.database_port.as_deref(), Some("3307")); + } + other => panic!("expected mysql settings, got {:?}", other), + } + } +} diff --git a/migrant_lib/src/config/init.rs b/migrant_lib/src/config/init.rs index 9ac91e9..5d40845 100644 --- a/migrant_lib/src/config/init.rs +++ b/migrant_lib/src/config/init.rs @@ -173,13 +173,13 @@ impl SettingsFileInitializer { } /// Set interactive prompts, default is `true` - pub fn interactive(&mut self, b: bool) -> &mut Self { + pub fn interactive(mut self, b: bool) -> Self { self.interactive = b; self } /// Default all file values `env:` if unspecified - pub fn with_env_defaults(&mut self, b: bool) -> &mut Self { + pub fn with_env_defaults(mut self, b: bool) -> Self { self.with_env_defaults = b; self } @@ -202,8 +202,8 @@ impl SettingsFileInitializer { /// # Ok(()) /// # } /// ``` - pub fn with_sqlite_options(&mut self, options: &SqliteSettingsBuilder) -> &mut Self { - self.database_options = Some(DatabaseConfigOptions::Sqlite(options.clone())); + pub fn with_sqlite_options(mut self, options: SqliteSettingsBuilder) -> Self { + self.database_options = Some(DatabaseConfigOptions::Sqlite(options)); self } @@ -227,8 +227,8 @@ impl SettingsFileInitializer { /// # Ok(()) /// # } /// ``` - pub fn with_postgres_options(&mut self, options: &PostgresSettingsBuilder) -> &mut Self { - self.database_options = Some(DatabaseConfigOptions::Postgres(options.clone())); + pub fn with_postgres_options(mut self, options: PostgresSettingsBuilder) -> Self { + self.database_options = Some(DatabaseConfigOptions::Postgres(options)); self } @@ -252,8 +252,8 @@ impl SettingsFileInitializer { /// # Ok(()) /// # } /// ``` - pub fn with_mysql_options(&mut self, options: &MySqlSettingsBuilder) -> &mut Self { - self.database_options = Some(DatabaseConfigOptions::MySql(options.clone())); + pub fn with_mysql_options(mut self, options: MySqlSettingsBuilder) -> Self { + self.database_options = Some(DatabaseConfigOptions::MySql(options)); self } @@ -297,18 +297,15 @@ impl SettingsFileInitializer { .map_err(|_| err!(Config, "unsupported database type: {}", db_kind))?; Ok(match db_kind { DbKind::Sqlite => { - let mut options = SqliteSettingsBuilder::empty(); - options.migration_location("migrations")?; + let options = SqliteSettingsBuilder::empty().migration_location("migrations")?; DatabaseConfigOptions::Sqlite(options) } DbKind::Postgres => { - let mut options = PostgresSettingsBuilder::empty(); - options.migration_location("migrations")?; + let options = PostgresSettingsBuilder::empty().migration_location("migrations")?; DatabaseConfigOptions::Postgres(options) } DbKind::MySql => { - let mut options = MySqlSettingsBuilder::empty(); - options.migration_location("migrations")?; + let options = MySqlSettingsBuilder::empty().migration_location("migrations")?; DatabaseConfigOptions::MySql(options) } }) diff --git a/migrant_lib/src/lib.rs b/migrant_lib/src/lib.rs index 22b4f94..4800853 100644 --- a/migrant_lib/src/lib.rs +++ b/migrant_lib/src/lib.rs @@ -174,6 +174,7 @@ pub(crate) const SQLITE_MEMORY_PATH: &str = ":memory:"; /// Database type being used #[derive(Debug, Clone, Copy, Eq, PartialEq)] +#[non_exhaustive] pub enum DbKind { /// Sqlite database Sqlite, diff --git a/migrant_lib/src/migrator.rs b/migrant_lib/src/migrator.rs index f3e99bb..689ad3d 100644 --- a/migrant_lib/src/migrator.rs +++ b/migrant_lib/src/migrator.rs @@ -34,6 +34,7 @@ impl fmt::Display for Direction { /// How a run handles a migration that fails to apply. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[non_exhaustive] pub enum ForceMode { /// A failed migration aborts the run with an error (default). #[default] @@ -167,7 +168,7 @@ impl Migrator { /// Set `direction`. Default is `Up`. /// `Up` => run `up.sql`. /// `Down` => run `down.sql`. - pub fn direction(&mut self, dir: Direction) -> &mut Self { + pub fn direction(mut self, dir: Direction) -> Self { self.direction = dir; self } @@ -180,26 +181,26 @@ impl Migrator { /// run. `ForceMode::SkipFailures` continues without recording it: the /// migration is skipped for the remainder of this run and retried on the /// next run, so it must be safe to re-attempt after a partial application. - pub fn force(&mut self, force: ForceMode) -> &mut Self { + pub fn force(mut self, force: ForceMode) -> Self { self.force = force; self } /// Set `fake` to fake application of migrations. /// Applied migrations table will be updated as if migrations were actually run. - pub fn fake(&mut self, fake: bool) -> &mut Self { + pub fn fake(mut self, fake: bool) -> Self { self.fake = fake; self } /// Set `all` to run all remaining available migrations in the given `direction` - pub fn all(&mut self, all: bool) -> &mut Self { + pub fn all(mut self, all: bool) -> Self { self.all = all; self } /// Toggle migration application output. Default is `true` - pub fn show_output(&mut self, show_output: bool) -> &mut Self { + pub fn show_output(mut self, show_output: bool) -> Self { self.show_output = show_output; self } @@ -216,7 +217,7 @@ impl Migrator { /// against, so this setting has no effect there. /// /// Disable it only when an outer mechanism already serializes migrations. - pub fn synchronized(&mut self, synchronized: bool) -> &mut Self { + pub fn synchronized(mut self, synchronized: bool) -> Self { self.synchronized = synchronized; self } @@ -527,6 +528,31 @@ mod tests { strs.iter().map(|s| (*s).to_owned()).collect() } + #[test] + fn owned_setters_chain_and_apply_each_value() { + // The setters take owned `self` and return owned `Self`, so a full + // configuration chains from `with_config` through to a value without an + // intermediate `mut` binding, and every setter must carry its value. + let settings = crate::config::Settings::configure_sqlite() + .memory() + .build() + .unwrap(); + let config = Config::with_settings(settings); + let migrator = Migrator::with_config(&config) + .direction(Direction::Down) + .force(ForceMode::AcceptFailures) + .fake(true) + .all(true) + .show_output(false) + .synchronized(false); + assert_eq!(migrator.direction, Direction::Down); + assert_eq!(migrator.force, ForceMode::AcceptFailures); + assert!(migrator.fake); + assert!(migrator.all); + assert!(!migrator.show_output); + assert!(!migrator.synchronized); + } + #[test] fn up_picks_first_unapplied_in_definition_order() { let avail = available(&["a", "b", "c"]); diff --git a/migrant_lib/src/ops.rs b/migrant_lib/src/ops.rs index 29b7a55..6541b6b 100644 --- a/migrant_lib/src/ops.rs +++ b/migrant_lib/src/ops.rs @@ -123,9 +123,21 @@ pub(crate) fn search_for_migrations(mig_root: &Path) -> Result &str { + &self.tag + } + + /// Whether the migration is currently applied + pub fn applied(&self) -> bool { + self.applied + } } /// Return the status of all migrations being managed: either those explicitly @@ -452,6 +464,25 @@ mod tests { assert_eq!(found, root.join(CONFIG_FILE)); } + #[test] + fn migration_status_accessors_read_private_fields() { + // `tag`/`applied` are private; the public accessors are the only outside + // read path and must reflect the constructed values. + let status = MigrationStatus { + tag: "20200101000000_first".to_string(), + applied: true, + }; + assert_eq!(status.tag(), "20200101000000_first"); + assert!(status.applied()); + + let unapplied = MigrationStatus { + tag: "20200102000000_second".to_string(), + applied: false, + }; + assert_eq!(unapplied.tag(), "20200102000000_second"); + assert!(!unapplied.applied()); + } + #[test] fn migration_search_finds_and_sorts() { let dir = tempfile::tempdir().unwrap(); diff --git a/migrant_lib/tests/reload_memory.rs b/migrant_lib/tests/reload_memory.rs index e1572d9..5cb6eed 100644 --- a/migrant_lib/tests/reload_memory.rs +++ b/migrant_lib/tests/reload_memory.rs @@ -41,8 +41,8 @@ fn applied_tags(config: &Config) -> Vec { migrant_lib::migration_statuses(config) .unwrap() .into_iter() - .filter(|m| m.applied) - .map(|m| m.tag) + .filter(|m| m.applied()) + .map(|m| m.tag().to_string()) .collect() } diff --git a/migrant_lib/tests/server_dbs.rs b/migrant_lib/tests/server_dbs.rs index 7b60e61..c2787df 100644 --- a/migrant_lib/tests/server_dbs.rs +++ b/migrant_lib/tests/server_dbs.rs @@ -61,7 +61,7 @@ fn apply_and_unapply(settings: &Settings) { let config = config.reload().unwrap(); let statuses = migrant_lib::migration_statuses(&config).unwrap(); assert_eq!(2, statuses.len()); - assert!(statuses.iter().all(|m| m.applied)); + assert!(statuses.iter().all(|m| m.applied())); Migrator::with_config(&config) .direction(Direction::Down) @@ -72,7 +72,7 @@ fn apply_and_unapply(settings: &Settings) { let config = config.reload().unwrap(); let statuses = migrant_lib::migration_statuses(&config).unwrap(); - assert!(statuses.iter().all(|m| !m.applied)); + assert!(statuses.iter().all(|m| !m.applied())); } /// Drop the migration table so the next run starts from a clean database. @@ -211,7 +211,7 @@ fn assert_failed_migration_rolls_back(conn_str: &str, settings: &Settings) { let config = config.reload().unwrap(); let statuses = migrant_lib::migration_statuses(&config).unwrap(); assert!( - statuses.iter().all(|m| !m.applied), + statuses.iter().all(|m| !m.applied()), "the tag must not be recorded when the migration fails" ); @@ -267,7 +267,7 @@ fn assert_force_continues_holding_lock(conn_str: &str, settings: &Settings) { let config = config.reload().unwrap(); let statuses = migrant_lib::migration_statuses(&config).unwrap(); assert!( - statuses.iter().all(|m| m.applied), + statuses.iter().all(|m| m.applied()), "force records every migration as applied, including the failed one" ); diff --git a/migrant_lib/tests/sqlite.rs b/migrant_lib/tests/sqlite.rs index 832a25f..eb09d33 100644 --- a/migrant_lib/tests/sqlite.rs +++ b/migrant_lib/tests/sqlite.rs @@ -49,8 +49,8 @@ fn applied_tags(config: &Config) -> Vec { migrant_lib::migration_statuses(config) .unwrap() .into_iter() - .filter(|m| m.applied) - .map(|m| m.tag) + .filter(|m| m.applied()) + .map(|m| m.tag().to_string()) .collect() } diff --git a/spec/advisory-locking.md b/spec/advisory-locking.md index 4efbc8d..ddb482d 100644 --- a/spec/advisory-locking.md +++ b/spec/advisory-locking.md @@ -45,6 +45,14 @@ new, unlocked session. The applied-state re-read at the start of a run does not re-read the settings file or swap the connection (see MIGRATOR-1), so the run cannot silently migrate over a different connection than the one it locked. +## LOCK-7 + +The `migrant` CLI exposes a `--no-sync` flag on the `apply` and `redo` +subcommands that disables the cross-process advisory lock by calling +`Migrator::synchronized(false)`, for use when migrations are already +serialized by an external mechanism. On `redo` it applies to both the `down` +and `up` runs. + Coverage: `migrant_lib/src/drivers/pg.rs`, `mysql.rs` (`advisory_lock`, gated on the test connection strings and run via `test.sh`); both drivers' `advisory_lock` tests cover exclusivity and lock survival across an in-transaction error; diff --git a/spec/config-file-and-env-resolution.md b/spec/config-file-and-env-resolution.md index e909903..84ade1e 100644 --- a/spec/config-file-and-env-resolution.md +++ b/spec/config-file-and-env-resolution.md @@ -28,5 +28,11 @@ responsibility. A relative `database_path` resolves relative to the config file's directory. +## CONFIG-5 + +`database_type` parses into `DbKind`, which is `#[non_exhaustive]`, so new +database backends can be added without a breaking change; downstream `match` +expressions on `DbKind` must include a wildcard arm. + Coverage: unit tests in `migrant_lib/src/config/settings.rs`; `tests/migrant.rs` (init --default-from-env). diff --git a/spec/error-handling-api.md b/spec/error-handling-api.md index ce92899..ce41f93 100644 --- a/spec/error-handling-api.md +++ b/spec/error-handling-api.md @@ -16,5 +16,16 @@ with nothing pending returns an empty `Report` (see [migrator-api.md](migrator-a `#[non_exhaustive]` enum: `is_config`, `is_migration`, `is_migration_not_found`, `is_shell_command`, `is_tag_error`, `is_invalid_db_kind`, `is_feature_required`. +## ERRORH-3 + +`DbKind` and `ForceMode` are also `#[non_exhaustive]`, so new database backends +or force modes can be added without a breaking change; downstream `match` +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`. + Coverage: unit tests in `migrant_lib/src/errors.rs`, `tags.rs`; exercised throughout the integration tests. diff --git a/spec/migrator-api.md b/spec/migrator-api.md index e6f80b3..84447af 100644 --- a/spec/migrator-api.md +++ b/spec/migrator-api.md @@ -50,5 +50,20 @@ database advisory lock, and each migration is applied in a transaction with its row. See [advisory-locking.md](advisory-locking.md) and [transactional-migrations.md](transactional-migrations.md). +## MIGRATOR-6 + +The `Migrator` setters `direction`, `force`, `fake`, `all`, `show_output`, and +`synchronized` take and return an owned `self`, not `&mut self`, so calls chain by +value: + +```rust +Migrator::with_config(&config) + .direction(Direction::Up) + .all(true) + .apply()?; +``` + +`with_config(&Config)` and `apply(&self)` are unchanged. + 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/settings-builders.md b/spec/settings-builders.md index 1912a67..ef8576d 100644 --- a/spec/settings-builders.md +++ b/spec/settings-builders.md @@ -25,5 +25,21 @@ name/user/password/host/port and `database_params` options. Generated connection strings percent-encode credentials and parameters so special characters in passwords and params are safe. +## SETTIN-5 + +The fluent setters on `SqliteSettingsBuilder`, `PostgresSettingsBuilder`, and +`MySqlSettingsBuilder` (e.g. `database_path(self) -> Result`, `memory(self) -> Self`, +`database_name(self) -> Self`) take and return an owned `Self`, not `&mut self`, so calls +chain by value: + +```rust +Settings::configure_sqlite() + .database_path("/abs/path/to/my.db")? + .migration_location("migrations")? + .build()?; +``` + +`build(&self)` still takes `&self` and does not consume the builder. + Coverage: `migrant_lib/tests/server_dbs.rs`, `sqlite.rs`; unit tests in `migrant_lib/src/config/builders.rs`. `ssl_cert_file` has no dedicated test. diff --git a/src/cli.rs b/src/cli.rs index 88232e4..597e4f3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -19,6 +19,17 @@ fn force_arg() -> Arg { ) } +/// `--no-sync`: disable the cross-process advisory lock (postgres/mysql). +fn no_sync_arg() -> Arg { + Arg::new("no-sync") + .long("no-sync") + .action(ArgAction::SetTrue) + .help( + "Disable the cross-process advisory lock (postgres/mysql). Use only when \ + migration runs are serialized by an external mechanism", + ) +} + pub fn build_cli() -> Command { Command::new("migrant") .version(env!("CARGO_PKG_VERSION")) @@ -119,7 +130,8 @@ pub fn build_cli() -> Command { .long("fake") .action(ArgAction::SetTrue) .help("Updates the migration table without running the migration"), - ), + ) + .arg(no_sync_arg()), ) .subcommand( Command::new("redo") @@ -131,7 +143,8 @@ pub fn build_cli() -> Command { .action(ArgAction::SetTrue) .help("Re-applies (down, then up) all applied migrations instead of only the latest"), ) - .arg(force_arg()), + .arg(force_arg()) + .arg(no_sync_arg()), ) .subcommand( Command::new("new") diff --git a/src/main.rs b/src/main.rs index 4803236..214aa60 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,21 +37,18 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .get_one::("location") .map(PathBuf::from) .unwrap_or_else(|| dir.to_owned()); - let mut initializer = Config::init_in(&dir); - initializer.interactive(!init_matches.get_flag("no-confirm")); - initializer.with_env_defaults(from_env); + let mut initializer = Config::init_in(&dir) + .interactive(!init_matches.get_flag("no-confirm")) + .with_env_defaults(from_env); if let Some(db_kind) = init_matches.get_one::("type") { - match db_kind.parse::()? { - DbKind::Sqlite => { - initializer.with_sqlite_options(&SqliteSettingsBuilder::empty()); - } + initializer = match db_kind.parse::()? { + DbKind::Sqlite => initializer.with_sqlite_options(SqliteSettingsBuilder::empty()), DbKind::Postgres => { - initializer.with_postgres_options(&PostgresSettingsBuilder::empty()); - } - DbKind::MySql => { - initializer.with_mysql_options(&MySqlSettingsBuilder::empty()); + initializer.with_postgres_options(PostgresSettingsBuilder::empty()) } - } + DbKind::MySql => initializer.with_mysql_options(MySqlSettingsBuilder::empty()), + _ => initializer, // DbKind is #[non_exhaustive] + }; } initializer.initialize()?; return Ok(()); @@ -88,6 +85,7 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { DbKind::Postgres | DbKind::MySql => { println!("{}", config.connect_string()?); } + _ => println!("{}", config.connect_string()?), // DbKind is #[non_exhaustive] }, Some(("list", _)) => { // load applied migrations from the database @@ -110,6 +108,7 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { let force = force_mode(matches)?; let fake = matches.get_flag("fake"); let all = matches.get_flag("all"); + let no_sync = matches.get_flag("no-sync"); let direction = if matches.get_flag("down") { Direction::Down } else { @@ -121,6 +120,7 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .force(force) .fake(fake) .all(all) + .synchronized(!no_sync) .apply()?; let config = config.reload()?; @@ -132,11 +132,13 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { let force = force_mode(matches)?; let all = matches.get_flag("all"); + let no_sync = matches.get_flag("no-sync"); Migrator::with_config(&config) .direction(Direction::Down) .force(force) .all(all) + .synchronized(!no_sync) .apply()?; let config = config.reload()?; migrant_lib::list(&config)?; @@ -145,6 +147,7 @@ fn run(dir: &Path, matches: &clap::ArgMatches) -> Result<()> { .direction(Direction::Up) .force(force) .all(all) + .synchronized(!no_sync) .apply()?; let config = config.reload()?; migrant_lib::list(&config)?; diff --git a/src/tui.rs b/src/tui.rs index 10049e8..4916752 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -158,7 +158,7 @@ impl App { ]) .areas(frame.area()); - let applied = self.statuses.iter().filter(|m| m.applied).count(); + let applied = self.statuses.iter().filter(|m| m.applied()).count(); let header = Paragraph::new(Line::from(vec![ Span::styled("migrant", Style::new().add_modifier(Modifier::BOLD)), Span::raw(format!( @@ -175,14 +175,14 @@ impl App { .statuses .iter() .map(|mig| { - let (mark, style) = if mig.applied { + let (mark, style) = if mig.applied() { ("✓", Style::new().fg(Color::Green)) } else { (" ", Style::new().fg(Color::DarkGray)) }; ListItem::new(Line::from(vec![ Span::styled(format!("[{}] ", mark), style), - Span::raw(mig.tag.clone()), + Span::raw(mig.tag().to_string()), ])) }) .collect::>(); @@ -236,7 +236,7 @@ mod tests { } fn applied_count(app: &App) -> usize { - app.statuses.iter().filter(|m| m.applied).count() + app.statuses.iter().filter(|m| m.applied()).count() } fn buffer_text(app: &mut App) -> String { diff --git a/tests/migrant.rs b/tests/migrant.rs index 4a4cbfc..ef16525 100644 --- a/tests/migrant.rs +++ b/tests/migrant.rs @@ -283,3 +283,60 @@ fn force_modes_through_the_cli() { .success() .stdout(predicates::str::is_match(r"\[✓\] \d{14}_a-bad").expect("valid regex")); } + +// CLIMIG: `apply --all --no-sync` is accepted and applies migrations +// normally. On sqlite the advisory lock is a no-op, so this proves the flag +// is wired end-to-end (accepted + migrations applied). +#[test] +fn apply_no_sync_applies_migrations() { + let dir = sqlite_project(); + migrant() + .current_dir(dir.path()) + .arg("setup") + .assert() + .success(); + new_migration( + dir.path(), + "first", + "create table no_sync_a (x integer);", + "drop table no_sync_a;", + ); + new_migration( + dir.path(), + "second", + "create table no_sync_b (x integer);", + "drop table no_sync_b;", + ); + + migrant() + .current_dir(dir.path()) + .args(["apply", "--all", "--no-sync"]) + .assert() + .success() + .stdout(contains("Applying[Up]:")); + + migrant() + .current_dir(dir.path()) + .arg("list") + .assert() + .success() + .stdout(predicates::str::is_match(r"\[✓\] \d{14}_first").expect("valid regex")) + .stdout(predicates::str::is_match(r"\[✓\] \d{14}_second").expect("valid regex")); + + // `redo --no-sync` applies the flag to both the down and up runs. + migrant() + .current_dir(dir.path()) + .args(["redo", "--all", "--no-sync"]) + .assert() + .success() + .stdout(contains("Applying[Down]:")) + .stdout(contains("Applying[Up]:")); + + migrant() + .current_dir(dir.path()) + .arg("list") + .assert() + .success() + .stdout(predicates::str::is_match(r"\[✓\] \d{14}_first").expect("valid regex")) + .stdout(predicates::str::is_match(r"\[✓\] \d{14}_second").expect("valid regex")); +}