cli: add --target-version CLI flags for migrate run/revert (#2538)

* cli: add --target-version CLI flags for migrate run/revert

* cli: fix broken test

* cli: test harness for `sqlx migrate` along with --target-version tests

* cli: Fail if version supplied to run/revert is too old/new

After some discussion with my coworkers, we thought about the behavior a bit more:

The behavior is now that for a run, if the provided version is too old, the CLI
will return with failure rather than being a no-op. This gives feedback to the
operator instead of being quiet.

It is still valid to up/downgrade to the latest version, this will still be a no-op
to allow for idempotency.
This commit is contained in:
Ameer Ghani 2023-07-31 15:49:53 -04:00 committed by GitHub
parent febf9ed775
commit 84f21e99ef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 438 additions and 11 deletions

View File

@ -90,6 +90,28 @@ jobs:
--manifest-path sqlx-core/Cargo.toml
--features json,_rt-${{ matrix.runtime }},_tls-${{ matrix.tls }}
cli-test:
name: CLI Unit Test
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
- uses: Swatinem/rust-cache@v1
with:
key: ${{ runner.os }}-test
- uses: actions-rs/cargo@v1
with:
command: test
args: >
--manifest-path sqlx-cli/Cargo.toml
cli:
name: CLI Binaries
runs-on: ${{ matrix.os }}

53
Cargo.lock generated
View File

@ -130,6 +130,21 @@ version = "0.7.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6"
[[package]]
name = "assert_cmd"
version = "2.0.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "86d6b683edf8d1119fe420a94f8a7e389239666aa72e65495d91c00462510151"
dependencies = [
"anstyle",
"bstr",
"doc-comment",
"predicates 3.0.3",
"predicates-core",
"predicates-tree",
"wait-timeout",
]
[[package]]
name = "async-attributes"
version = "1.1.2"
@ -495,6 +510,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5ffdb39cb703212f3c11973452c2861b972f757b021158f3516ba10f2fa8b2c1"
dependencies = [
"memchr",
"once_cell",
"regex-automata",
"serde",
]
@ -1096,6 +1113,12 @@ dependencies = [
"winapi",
]
[[package]]
name = "doc-comment"
version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10"
[[package]]
name = "dotenvy"
version = "0.15.6"
@ -1931,7 +1954,7 @@ dependencies = [
"fragile",
"lazy_static",
"mockall_derive",
"predicates",
"predicates 2.1.5",
"predicates-tree",
]
@ -2326,6 +2349,18 @@ dependencies = [
"regex",
]
[[package]]
name = "predicates"
version = "3.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "09963355b9f467184c04017ced4a2ba2d75cbcb4e7462690d388233253d4b1a9"
dependencies = [
"anstyle",
"difflib",
"itertools",
"predicates-core",
]
[[package]]
name = "predicates-core"
version = "1.0.6"
@ -2563,6 +2598,12 @@ dependencies = [
"regex-syntax",
]
[[package]]
name = "regex-automata"
version = "0.1.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132"
[[package]]
name = "regex-syntax"
version = "0.6.27"
@ -3072,6 +3113,7 @@ name = "sqlx-cli"
version = "0.7.1"
dependencies = [
"anyhow",
"assert_cmd",
"async-trait",
"backoff",
"cargo_metadata",
@ -3979,6 +4021,15 @@ version = "0.9.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
[[package]]
name = "wait-timeout"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9f200f5b12eb75f8c1ed65abd4b2db8a6e1b138a20de009dacee265a2498f3f6"
dependencies = [
"libc",
]
[[package]]
name = "waker-fn"
version = "1.1.0"

View File

@ -66,3 +66,6 @@ sqlite = ["sqlx/sqlite"]
openssl-vendored = ["openssl/vendored"]
completions = ["dep:clap_complete"]
[dev-dependencies]
assert_cmd = "2.0.11"

View File

@ -50,7 +50,7 @@ pub async fn reset(
pub async fn setup(migration_source: &str, connect_opts: &ConnectOpts) -> anyhow::Result<()> {
create(connect_opts).await?;
migrate::run(migration_source, connect_opts, false, false).await
migrate::run(migration_source, connect_opts, false, false, None).await
}
fn ask_to_continue(connect_opts: &ConnectOpts) -> bool {

View File

@ -35,13 +35,33 @@ pub async fn run(opt: Opt) -> Result<()> {
dry_run,
ignore_missing,
connect_opts,
} => migrate::run(&source, &connect_opts, dry_run, *ignore_missing).await?,
target_version,
} => {
migrate::run(
&source,
&connect_opts,
dry_run,
*ignore_missing,
target_version,
)
.await?
}
MigrateCommand::Revert {
source,
dry_run,
ignore_missing,
connect_opts,
} => migrate::revert(&source, &connect_opts, dry_run, *ignore_missing).await?,
target_version,
} => {
migrate::revert(
&source,
&connect_opts,
dry_run,
*ignore_missing,
target_version,
)
.await?
}
MigrateCommand::Info {
source,
connect_opts,

View File

@ -267,8 +267,15 @@ pub async fn run(
connect_opts: &ConnectOpts,
dry_run: bool,
ignore_missing: bool,
target_version: Option<i64>,
) -> anyhow::Result<()> {
let migrator = Migrator::new(Path::new(migration_source)).await?;
if let Some(target_version) = target_version {
if !migrator.iter().any(|m| target_version == m.version) {
bail!(MigrateError::VersionNotPresent(target_version));
}
}
let mut conn = crate::connect(connect_opts).await?;
conn.ensure_migrations_table().await?;
@ -281,6 +288,17 @@ pub async fn run(
let applied_migrations = conn.list_applied_migrations().await?;
validate_applied_migrations(&applied_migrations, &migrator, ignore_missing)?;
let latest_version = applied_migrations
.iter()
.max_by(|x, y| x.version.cmp(&y.version))
.and_then(|migration| Some(migration.version))
.unwrap_or(0);
if let Some(target_version) = target_version {
if target_version < latest_version {
bail!(MigrateError::VersionTooOld(target_version, latest_version));
}
}
let applied_migrations: HashMap<_, _> = applied_migrations
.into_iter()
.map(|m| (m.version, m))
@ -299,12 +317,23 @@ pub async fn run(
}
}
None => {
let elapsed = if dry_run {
let skip = match target_version {
Some(target_version) if migration.version > target_version => true,
_ => false,
};
let elapsed = if dry_run || skip {
Duration::new(0, 0)
} else {
conn.apply(migration).await?
};
let text = if dry_run { "Can apply" } else { "Applied" };
let text = if skip {
"Skipped"
} else if dry_run {
"Can apply"
} else {
"Applied"
};
println!(
"{} {}/{} {} {}",
@ -333,8 +362,15 @@ pub async fn revert(
connect_opts: &ConnectOpts,
dry_run: bool,
ignore_missing: bool,
target_version: Option<i64>,
) -> anyhow::Result<()> {
let migrator = Migrator::new(Path::new(migration_source)).await?;
if let Some(target_version) = target_version {
if target_version != 0 && !migrator.iter().any(|m| target_version == m.version) {
bail!(MigrateError::VersionNotPresent(target_version));
}
}
let mut conn = crate::connect(&connect_opts).await?;
conn.ensure_migrations_table().await?;
@ -347,6 +383,17 @@ pub async fn revert(
let applied_migrations = conn.list_applied_migrations().await?;
validate_applied_migrations(&applied_migrations, &migrator, ignore_missing)?;
let latest_version = applied_migrations
.iter()
.max_by(|x, y| x.version.cmp(&y.version))
.and_then(|migration| Some(migration.version))
.unwrap_or(0);
if let Some(target_version) = target_version {
if target_version > latest_version {
bail!(MigrateError::VersionTooNew(target_version, latest_version));
}
}
let applied_migrations: HashMap<_, _> = applied_migrations
.into_iter()
.map(|m| (m.version, m))
@ -361,12 +408,22 @@ pub async fn revert(
}
if applied_migrations.contains_key(&migration.version) {
let elapsed = if dry_run {
let skip = match target_version {
Some(target_version) if migration.version <= target_version => true,
_ => false,
};
let elapsed = if dry_run || skip {
Duration::new(0, 0)
} else {
conn.revert(migration).await?
};
let text = if dry_run { "Can apply" } else { "Applied" };
let text = if skip {
"Skipped"
} else if dry_run {
"Can apply"
} else {
"Applied"
};
println!(
"{} {}/{} {} {}",
@ -378,8 +435,12 @@ pub async fn revert(
);
is_applied = true;
// Only a single migration will be reverted at a time, so we break
break;
// Only a single migration will be reverted at a time if no target
// version is supplied, so we break.
if let None = target_version {
break;
}
}
}
if !is_applied {

View File

@ -159,6 +159,11 @@ pub enum MigrateCommand {
#[clap(flatten)]
connect_opts: ConnectOpts,
/// Apply migrations up to the specified version. If unspecified, apply all
/// pending migrations. If already at the target version, then no-op.
#[clap(long)]
target_version: Option<i64>,
},
/// Revert the latest migration with a down file.
@ -175,6 +180,12 @@ pub enum MigrateCommand {
#[clap(flatten)]
connect_opts: ConnectOpts,
/// Revert migrations down to the specified version. If unspecified, revert
/// only the last migration. Set to 0 to revert all migrations. If already
/// at the target version, then no-op.
#[clap(long)]
target_version: Option<i64>,
},
/// List all available migrations.

View File

@ -361,7 +361,7 @@ mod tests {
let sample_metadata = std::fs::read_to_string(sample_metadata_path)?;
let metadata: Metadata = sample_metadata.parse()?;
let action = minimal_project_recompile_action(&metadata)?;
let action = minimal_project_recompile_action(&metadata);
assert_eq!(
action,
ProjectRecompileAction {

View File

@ -0,0 +1,93 @@
use assert_cmd::{assert::Assert, Command};
use sqlx::{migrate::Migrate, Connection, SqliteConnection};
use std::{
env::temp_dir,
fs::remove_file,
path::{Path, PathBuf},
};
pub struct TestDatabase {
file_path: PathBuf,
migrations: String,
}
impl TestDatabase {
pub fn new(name: &str, migrations: &str) -> Self {
let migrations_path = Path::new("tests").join(migrations);
let file_path = Path::new(&temp_dir()).join(format!("test-{}.db", name));
let ret = Self {
file_path,
migrations: String::from(migrations_path.to_str().unwrap()),
};
Command::cargo_bin("cargo-sqlx")
.unwrap()
.args([
"sqlx",
"database",
"create",
"--database-url",
&ret.connection_string(),
])
.assert()
.success();
ret
}
pub fn connection_string(&self) -> String {
format!("sqlite://{}", self.file_path.display())
}
pub fn run_migration(&self, revert: bool, version: Option<i64>, dry_run: bool) -> Assert {
let ver = match version {
Some(v) => v.to_string(),
None => String::from(""),
};
Command::cargo_bin("cargo-sqlx")
.unwrap()
.args(
[
vec![
"sqlx",
"migrate",
match revert {
true => "revert",
false => "run",
},
"--database-url",
&self.connection_string(),
"--source",
&self.migrations,
],
match version {
Some(_) => vec!["--target-version", &ver],
None => vec![],
},
match dry_run {
true => vec!["--dry-run"],
false => vec![],
},
]
.concat(),
)
.assert()
}
pub async fn applied_migrations(&self) -> Vec<i64> {
let mut conn = SqliteConnection::connect(&self.connection_string())
.await
.unwrap();
conn.list_applied_migrations()
.await
.unwrap()
.iter()
.map(|m| m.version)
.collect()
}
}
impl Drop for TestDatabase {
fn drop(&mut self) {
remove_file(&self.file_path).unwrap();
}
}

147
sqlx-cli/tests/migrate.rs Normal file
View File

@ -0,0 +1,147 @@
mod common;
use common::TestDatabase;
#[tokio::test]
async fn run_reversible_migrations() {
let all_migrations: Vec<i64> = vec![
20230101000000,
20230201000000,
20230301000000,
20230401000000,
20230501000000,
];
// Without --target-version specified.k
{
let db = TestDatabase::new("migrate_run_reversible_latest", "migrations_reversible");
db.run_migration(false, None, false).success();
assert_eq!(db.applied_migrations().await, all_migrations);
}
// With --target-version specified.
{
let db = TestDatabase::new(
"migrate_run_reversible_latest_explicit",
"migrations_reversible",
);
// Move to latest, explicitly specified.
db.run_migration(false, Some(20230501000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations);
// Move to latest when we're already at the latest.
db.run_migration(false, Some(20230501000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations);
// Upgrade to an old version.
db.run_migration(false, Some(20230301000000), false)
.failure();
assert_eq!(db.applied_migrations().await, all_migrations);
}
// With --target-version, incrementally upgrade.
{
let db = TestDatabase::new(
"migrate_run_reversible_incremental",
"migrations_reversible",
);
// First version
db.run_migration(false, Some(20230101000000), false)
.success();
assert_eq!(db.applied_migrations().await, vec![20230101000000]);
// Dry run upgrade to latest.
db.run_migration(false, None, true).success();
assert_eq!(db.applied_migrations().await, vec![20230101000000]);
// Dry run upgrade + 2
db.run_migration(false, Some(20230301000000), true)
.success();
assert_eq!(db.applied_migrations().await, vec![20230101000000]);
// Upgrade to non-existent version.
db.run_migration(false, Some(20230901000000999), false)
.failure();
assert_eq!(db.applied_migrations().await, vec![20230101000000]);
// Upgrade + 1
db.run_migration(false, Some(20230201000000), false)
.success();
assert_eq!(
db.applied_migrations().await,
vec![20230101000000, 20230201000000]
);
// Upgrade + 2
db.run_migration(false, Some(20230401000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations[..4]);
}
}
#[tokio::test]
async fn revert_migrations() {
let all_migrations: Vec<i64> = vec![
20230101000000,
20230201000000,
20230301000000,
20230401000000,
20230501000000,
];
// Without --target-version
{
let db = TestDatabase::new("migrate_revert_incremental", "migrations_reversible");
db.run_migration(false, None, false).success();
// Dry-run
db.run_migration(true, None, true).success();
assert_eq!(db.applied_migrations().await, all_migrations);
// Downgrade one
db.run_migration(true, None, false).success();
assert_eq!(db.applied_migrations().await, all_migrations[..4]);
// Downgrade one
db.run_migration(true, None, false).success();
assert_eq!(db.applied_migrations().await, all_migrations[..3]);
}
// With --target-version
{
let db = TestDatabase::new("migrate_revert_incremental", "migrations_reversible");
db.run_migration(false, None, false).success();
// Dry-run downgrade to version 3.
db.run_migration(true, Some(20230301000000), true).success();
assert_eq!(db.applied_migrations().await, all_migrations);
// Downgrade to version 3.
db.run_migration(true, Some(20230301000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations[..3]);
// Try downgrading to the same version.
db.run_migration(true, Some(20230301000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations[..3]);
// Try downgrading to a newer version.
db.run_migration(true, Some(20230401000000), false)
.failure();
assert_eq!(db.applied_migrations().await, all_migrations[..3]);
// Try downgrading to a non-existent version.
db.run_migration(true, Some(9999), false).failure();
assert_eq!(db.applied_migrations().await, all_migrations[..3]);
// Ensure we can still upgrade
db.run_migration(false, Some(20230401000000), false)
.success();
assert_eq!(db.applied_migrations().await, all_migrations[..4]);
// Downgrade to zero.
db.run_migration(true, Some(0), false).success();
assert_eq!(db.applied_migrations().await, vec![] as Vec<i64>);
}
}

View File

@ -0,0 +1 @@
DROP TABLE test1;

View File

@ -0,0 +1 @@
CREATE TABLE test1(x INTEGER PRIMARY KEY);

View File

@ -0,0 +1 @@
DROP TABLE test2;

View File

@ -0,0 +1 @@
CREATE TABLE test2(x INTEGER PRIMARY KEY);

View File

@ -0,0 +1 @@
DROP TABLE test3;

View File

@ -0,0 +1 @@
CREATE TABLE test3(x INTEGER PRIMARY KEY);

View File

@ -0,0 +1 @@
DROP TABLE test4;

View File

@ -0,0 +1 @@
CREATE TABLE test4(x INTEGER PRIMARY KEY);

View File

@ -0,0 +1 @@
DROP TABLE test5;

View File

@ -0,0 +1 @@
CREATE TABLE test5(x INTEGER PRIMARY KEY);

View File

@ -15,6 +15,15 @@ pub enum MigrateError {
#[error("migration {0} was previously applied but has been modified")]
VersionMismatch(i64),
#[error("migration {0} is not present in the migration source")]
VersionNotPresent(i64),
#[error("migration {0} is older than the latest applied migration {1}")]
VersionTooOld(i64, i64),
#[error("migration {0} is newer than the latest applied migration {1}")]
VersionTooNew(i64, i64),
#[error("cannot mix reversible migrations with simple migrations. All migrations should be reversible or simple migrations")]
InvalidMixReversibleAndSimple,