From 75a3070400d6ce7da875cedb9b26936e0a127873 Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:51:05 -0600 Subject: [PATCH 1/3] Native: offline backup-hold command for restores (#973) A restore releases the restored backup hold while the cell is still offline, before any Server process starts. PHP does this with a one-off `artisan external-payloads:backup-hold release --owner=`. Native only had #953's admin endpoints, so the release failed closed. `durable-workflow-server backup-hold acquire|renew|release|status [--owner ] [--ttl ]` is PHP's command, offline: - It reads only the DB settings and starts no listener or workers. - It opens only an initialized native database. SQLite is inspected read-only first; PostgreSQL and MySQL pass their read-only schema check before the writable pool opens. A missing, empty or foreign database is refused with native_schema_not_initialized and left as it was. - It runs the same store methods as the endpoints (idempotent, recorded owner, database clock) and prints the same v1 report. On refusal it prints the reason or validation message and exits non-zero. MySQL's schema check now refuses an unmarked database as native_schema_not_initialized, as PostgreSQL's does, rather than failing on the missing marker table as storage_unavailable. Tests run on SQLite, PostgreSQL and MySQL: - the binary releases a restored lapsed hold with no Server running; - only the recorded owner can release, and a second release is idempotent; - PHP's validation messages and argument refusals; - acquire, renew and release by the next backup; - a database with no native schema is refused and left without one. Co-Authored-By: Claude Opus 5.5 --- docs/contracts/external-payload-storage.md | 16 +++ docs/self-hosted-backup-and-restore.md | 6 +- rust/src/main.rs | 39 ++++++ rust/src/runtime/backup_hold.rs | 100 ++++++++++++++++ rust/src/runtime/mod.rs | 2 +- rust/src/runtime/mysql.rs | 5 + rust/src/runtime/sqlite.rs | 29 +++++ .../external_payloads/backup_hold.rs | 111 ++++++++++++++++++ 8 files changed, 305 insertions(+), 3 deletions(-) diff --git a/docs/contracts/external-payload-storage.md b/docs/contracts/external-payload-storage.md index e23a674df..479744a11 100644 --- a/docs/contracts/external-payload-storage.md +++ b/docs/contracts/external-payload-storage.md @@ -261,6 +261,22 @@ curl -fsS "$SERVER/api/system/external-payload-backup-hold?owner=$BACKUP_ID" \ -H "Authorization: Bearer $ADMIN_TOKEN" -H 'X-Durable-Workflow-Control-Plane-Version: 2' ``` +With no Server process running, for example while a restore is still +offline, the native image runs the same command against the database. It +reads only the database settings (`DB_CONNECTION` and its connection +variables), opens only an already initialized native database, prints the same +JSON report, and starts no listener or workers: + +```sh +docker run --rm --env-file server.env "$SERVER_IMAGE" \ + backup-hold release --owner="$BACKUP_ID" +# Also: acquire|renew --owner=... [--ttl=900], and status [--owner=...]. +``` + +On refusal it prints the reason (`backup_hold_not_owned`, +`backup_hold_owned_elsewhere`, `native_schema_not_initialized`, or the +validation message) and exits non-zero. + Each answers the command's JSON report. Where a command exits unsuccessfully, the endpoint answers 409: `backup_hold_owned_elsewhere` when another backup holds the hold, and `backup_hold_not_owned` when the owner's hold expired, was diff --git a/docs/self-hosted-backup-and-restore.md b/docs/self-hosted-backup-and-restore.md index 06bb4db99..a27433b55 100644 --- a/docs/self-hosted-backup-and-restore.md +++ b/docs/self-hosted-backup-and-restore.md @@ -4,8 +4,10 @@ This procedure applies to the PHP 2.x Compose stack, `docker-compose.php.yml`. The published Compose file runs the native server from 3.0.0. On native 3.x Server, take and release the backup hold through its admin -endpoints instead of `php artisan external-payloads:backup-hold`. The owner -UUID, TTL limits and status checks are the same; see +endpoints instead of `php artisan external-payloads:backup-hold`. To release a +restored hold before any Server process starts, run the image's offline +`backup-hold release --owner=` command against the restored database. The +owner UUID, TTL limits and status checks are the same; see [coordinating online backups](contracts/external-payload-storage.md#coordinating-online-backups). This procedure applies to the published single-node Compose stack with MySQL, diff --git a/rust/src/main.rs b/rust/src/main.rs index a246f7af3..8b4498d97 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -245,6 +245,42 @@ fn main() -> Result<(), Box> { .block_on(run()) } +/// `backup-hold acquire|renew|release|status [--owner ] [--ttl ]`: +/// PHP's `external-payloads:backup-hold`, offline (Server #973). It opens +/// only an initialized native database from the DB settings, prints the v1 +/// report, and on refusal prints the reason and exits non-zero. +async fn backup_hold_command() -> Result<(), Box> { + use durable_workflow_server::runtime::backup_hold; + let args: Vec = env::args().skip(2).collect(); + let backend = env::var("DB_CONNECTION").unwrap_or_default(); + let result = match backup_hold::command(&args) { + Err(error) => Err(error), + Ok(command) => match backend.as_str() { + "sqlite" => { + let path = env::var("DB_DATABASE").map_err(|_| "DB_DATABASE is required")?; + backup_hold::console_sqlite(&path, &command).await + } + "pgsql" => backup_hold::console_postgres(postgres_options()?, &command).await, + "mysql" | "mariadb" => backup_hold::console_mysql(mysql_options()?, &command).await, + _ => { + return Err( + "backup-hold requires DB_CONNECTION=sqlite, pgsql, mysql or mariadb".into(), + ); + } + }, + }; + match result { + Ok(report) => { + println!("{report}"); + Ok(()) + } + Err(error) => { + eprintln!("{}", backup_hold::console_failure(&error)); + Err("backup hold command refused".into()) + } + } +} + /// A release build carries the version of its `native-` tag, built in as /// `DW_SERVER_VERSION`, and starts without the opt-in. A development build /// (no release version) still requires `DW_RUST_EXPERIMENTAL=1`, so an @@ -257,6 +293,9 @@ async fn run() -> Result<(), Box> { if !release_build() && env::var("DW_RUST_EXPERIMENTAL").as_deref() != Ok("1") { return Err("Rust development build requires DW_RUST_EXPERIMENTAL=1; release builds do not".into()); } + if env::args().nth(1).as_deref() == Some("backup-hold") { + return backup_hold_command().await; + } if let Some(command) = env::args().nth(1) { if env::args().count() != 2 || !matches!( diff --git a/rust/src/runtime/backup_hold.rs b/rust/src/runtime/backup_hold.rs index 794ca2014..3ac07c53a 100644 --- a/rust/src/runtime/backup_hold.rs +++ b/rust/src/runtime/backup_hold.rs @@ -187,6 +187,89 @@ fn sql_time(at: DateTime) -> String { at.format("%Y-%m-%d %H:%M:%S").to_string() } +/// One offline `backup-hold` invocation, as PHP's +/// `external-payloads:backup-hold {action} --owner= --ttl=900` takes it +/// (Server #973): a restore releases the restored hold before any Server +/// process starts. +#[derive(Debug, PartialEq, Eq)] +pub struct Command { + /// `None` is `status`. + action: Option, + owner: Option, + ttl: i64, +} + +/// Parse the arguments after `backup-hold`: an action (`acquire`, `renew`, +/// `release` or `status`), then `--owner` and `--ttl`, each as `--name=value` +/// or `--name value` and at most once. The TTL must be an integer whatever +/// the action, as PHP's command checks it first. +pub fn command(args: &[String]) -> Result { + let invalid = || super::refuse(StatusCode::BAD_REQUEST, "backup_hold_command_invalid"); + let mut args = args.iter(); + let action = match args.next().map(String::as_str) { + Some("acquire") => Some(Action::Acquire), + Some("renew") => Some(Action::Renew), + Some("release") => Some(Action::Release), + Some("status") => None, + _ => return Err(super::refuse(StatusCode::BAD_REQUEST, "backup_hold_action_invalid")), + }; + let (mut owner, mut ttl) = (None, None); + while let Some(arg) = args.next() { + let (name, value) = match arg.split_once('=') { + Some((name, value)) => (name, value.to_owned()), + None => (arg.as_str(), args.next().ok_or_else(invalid)?.clone()), + }; + let slot = match name { + "--owner" => &mut owner, + "--ttl" => &mut ttl, + _ => return Err(invalid()), + }; + if slot.replace(value).is_some() { + return Err(invalid()); + } + } + let ttl = match ttl { + None => DEFAULT_TTL_SECONDS, + Some(text) => super::laravel_validation::integer(&json!(text)) + .ok_or_else(|| super::refuse(StatusCode::BAD_REQUEST, "backup_hold_ttl_invalid"))?, + }; + Ok(Command { action, owner, ttl }) +} + +/// `backup-hold` against a native SQLite database that already exists. +pub async fn console_sqlite(database: &str, command: &Command) -> Result { + let store = super::store::Store::new(super::sqlite::open_initialized(database).await?); + store.run_backup_hold_command(command).await +} + +/// `backup-hold` against an initialized native PostgreSQL database; an empty +/// or foreign database is refused before the writable pool opens. +pub async fn console_postgres(options: sqlx::postgres::PgConnectOptions, command: &Command) -> Result { + super::postgres::PostgresStorage::check(options.clone()).await?; + let store = super::store::Store::new(super::postgres::PostgresStorage::open(options).await?.into_pool()); + store.run_backup_hold_command(command).await +} + +/// `backup-hold` against an initialized native MySQL or MariaDB database. +pub async fn console_mysql(options: sqlx::mysql::MySqlConnectOptions, command: &Command) -> Result { + super::mysql::MySqlStorage::check(options.clone()).await?; + let store = super::store::Store::new(super::mysql::MySqlStorage::open(options).await?.into_pool()); + store.run_backup_hold_command(command).await +} + +/// The line the command prints on failure: the refusal reason, or the +/// validation messages of an invalid owner or TTL. +pub fn console_failure(error: &super::RuntimeError) -> String { + match error { + super::RuntimeError::Refused { reason, .. } => (*reason).to_owned(), + super::RuntimeError::Protocol { response, .. } => response["message"] + .as_str() + .unwrap_or("backup_hold_request_invalid") + .to_owned(), + _ => "storage_unavailable".to_owned(), + } +} + store_methods! { /// The hold row and the database's clock, without a lock: the preflight /// PHP's `assertReclamationAllowed` makes before a prune stages its @@ -246,6 +329,23 @@ store_methods! { Ok(next.report()) } + /// The offline command's report, from the same methods #953's endpoints + /// call. The pool closes whatever the outcome. + async fn run_backup_hold_command(&self, command: &Command) -> Result { + let result = match command.action { + None => match status_owner(command.owner.as_deref()) { + Ok(owner) => self.backup_hold_status(owner.as_deref()).await, + Err(error) => Err(error), + }, + Some(action) => match request(&json!({ "owner": command.owner, "ttl": command.ttl }), action) { + Ok(request) => self.change_backup_hold(action, &request).await, + Err(error) => Err(error), + }, + }; + self.close().await; + result + } + /// `status`: the hold as it stands, refused when a scoped owner no longer /// holds it. pub(crate) async fn backup_hold_status(&self, owner: Option<&str>) -> Result { diff --git a/rust/src/runtime/mod.rs b/rust/src/runtime/mod.rs index 669799cf1..39d1681e9 100644 --- a/rust/src/runtime/mod.rs +++ b/rust/src/runtime/mod.rs @@ -78,7 +78,7 @@ mod memo_limits; mod history_meter; mod standalone_activities; mod archive; -pub(crate) mod backup_hold; +pub mod backup_hold; mod command_payloads; mod completion_rules; mod deployments; diff --git a/rust/src/runtime/mysql.rs b/rust/src/runtime/mysql.rs index 34cd8a13f..791eb3d25 100644 --- a/rust/src/runtime/mysql.rs +++ b/rust/src/runtime/mysql.rs @@ -817,6 +817,11 @@ impl MySqlStorage { configure(&mut connection).await?; let flavor = Flavor::detect(&mut connection).await?; let mut tx = connection.begin_with("START TRANSACTION READ ONLY").await?; + // As on PostgreSQL: a database without the native marker is not + // initialized, rather than unreadable. + if !scope(&mut tx).await?.iter().any(|table| table == "dw_server_schema") { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } verify(&mut tx, flavor).await?; tx.rollback().await?; Ok::<_, super::RuntimeError>(()) diff --git a/rust/src/runtime/sqlite.rs b/rust/src/runtime/sqlite.rs index 19387878d..9ba7806c8 100644 --- a/rust/src/runtime/sqlite.rs +++ b/rust/src/runtime/sqlite.rs @@ -26,6 +26,35 @@ pub(super) async fn open(database: &str) -> Result { unreachable!("the final startup attempt always returns") } +/// Open a database that is already native, for an offline console command: +/// a missing, empty or foreign file is refused before any write, so the +/// command never bootstraps a schema. +pub(super) async fn open_initialized(database: &str) -> Result { + if database.is_empty() || database.contains('?') || database.contains(':') { + return Err(refuse( + StatusCode::BAD_REQUEST, + "invalid_development_configuration", + )); + } + let path = Path::new(database); + if !path.exists() { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } + let mut preflight = SqliteConnection::connect_with( + &SqliteConnectOptions::new() + .filename(path) + .read_only(true) + .foreign_keys(false), + ) + .await?; + let initialized = inspect(&mut preflight).await; + preflight.close().await?; + if !initialized? { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } + open(database).await +} + async fn open_once(database: &str) -> Result { if database.is_empty() || database.contains('?') || database.contains(':') { return Err(refuse( diff --git a/rust/tests/execution/external_payloads/backup_hold.rs b/rust/tests/execution/external_payloads/backup_hold.rs index 691b9370e..6e93dee26 100644 --- a/rust/tests/execution/external_payloads/backup_hold.rs +++ b/rust/tests/execution/external_payloads/backup_hold.rs @@ -215,3 +215,114 @@ async fn a_restored_lapsed_hold_is_released_by_its_recorded_owner() { let (code, body) = change(&router, "release", json!({"owner": OTHER})).await; assert_eq!((code, &body["reason"]), (StatusCode::CONFLICT, &json!("backup_hold_not_owned")), "{body}"); } + +/// The offline `backup-hold` command (Server #973), run as its own process +/// with only the DB settings: (succeeded, printed report, first stderr +/// line: the refusal reason; main's own error line follows it). +async fn console(database: &TestDatabase, args: &[&str]) -> (bool, Value, String) { + let mut command = std::process::Command::new(env!("CARGO_BIN_EXE_durable-workflow-server")); + command.env_clear().env("DW_RUST_EXPERIMENTAL", "1").arg("backup-hold").args(args); + match database { + TestDatabase::Sqlite(dir) => command.env("DB_CONNECTION", "sqlite").env("DB_DATABASE", dir.path().join("runtime.sqlite")), + TestDatabase::Postgres { url, .. } => command.env("DB_CONNECTION", "pgsql").env("DB_URL", url), + TestDatabase::MySql { url, .. } => command.env("DB_CONNECTION", "mysql").env("DB_URL", url), + }; + // Waiting on the child blocks, so it runs on the blocking pool. + let output = tokio::task::spawn_blocking(move || command.output().unwrap()).await.unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + (output.status.success(), serde_json::from_str(stdout.trim()).unwrap_or(Value::Null), first_line(&output.stderr)) +} + +fn first_line(stderr: &[u8]) -> String { + String::from_utf8_lossy(stderr).lines().next().unwrap_or_default().to_owned() +} + +#[tokio::test] +async fn the_offline_command_releases_a_restored_hold_before_any_server_starts() { + let temp = tempfile::tempdir().unwrap(); + let (database, runtime, router) = setup(ExternalPayloadConfig::default(), &temp.path().join("objects")).await; + let (code, body) = change(&router, "acquire", json!({"owner": OWNER, "ttl": 3600})).await; + assert_eq!(code, StatusCode::OK, "{body}"); + database.execute("UPDATE runtime_external_payload_backup_hold SET acquired_at='2000-01-01 00:00:00',expires_at='2000-01-01 01:00:00'").await; + // The cell is offline: no Server process holds the database. + drop(router); + runtime.close().await; + + let (ok, body, stderr) = console(&database, &["status"]).await; + assert!(ok, "{stderr}"); + assert_eq!(body["schema"], "durable-workflow.external-payload-backup-hold.v1"); + assert_eq!([&body["active"], &body["owner"]], [&json!(false), &json!(OWNER)]); + // A scoped status is refused once its owner no longer holds the hold. + assert_eq!(console(&database, &["status", "--owner", OWNER]).await.2, "backup_hold_not_owned"); + + // Only the recorded owner releases it, in either option form, and a + // second release reports the same released hold. + assert_eq!(console(&database, &["release", "--owner", OTHER]).await.2, "backup_hold_not_owned"); + for args in [["release", "--owner", OWNER].as_slice(), [&*format!("--owner={OWNER}")].as_slice()] { + let args: Vec<&str> = if args.len() == 1 { vec!["release", args[0]] } else { args.to_vec() }; + let (ok, body, stderr) = console(&database, &args).await; + assert!(ok, "{stderr}"); + assert_eq!([&body["active"], &body["owner"], &body["acquired_at"], &body["expires_at"]], + [&json!(false), &json!(OWNER), &json!("2000-01-01 00:00:00"), &Value::Null]); + } + + // PHP's validation, before the database is touched. + for (args, refusal) in [ + (vec!["release"], "The owner field is required."), + (vec!["release", "--owner", "not-a-uuid"], "The owner field must be a valid UUID."), + (vec!["acquire", "--owner", OTHER, "--ttl", "3601"], "The ttl field must not be greater than 3600."), + (vec!["status", "--ttl", "soon"], "backup_hold_ttl_invalid"), + (vec!["inspect"], "backup_hold_action_invalid"), + (vec![], "backup_hold_action_invalid"), + (vec!["release", "--owner", OWNER, "--owner", OTHER], "backup_hold_command_invalid"), + (vec!["release", "--owner"], "backup_hold_command_invalid"), + (vec!["release", "--force"], "backup_hold_command_invalid"), + ] { + let (ok, body, stderr) = console(&database, &args).await; + assert!(!ok && body.is_null(), "{args:?} succeeded: {body}"); + assert_eq!(stderr, refusal, "{args:?}"); + } + + // The released hold goes to the next backup, which renews and releases + // it; the database's clock decides every expiry. + let (ok, body, stderr) = console(&database, &["acquire", "--owner", OTHER, "--ttl", "60"]).await; + assert!(ok, "{stderr}"); + assert_eq!([&body["active"], &body["owner"]], [&json!(true), &json!(OTHER)]); + assert_eq!(seconds(&body["acquired_at"], &body["expires_at"]), 60); + let (ok, body, stderr) = console(&database, &["renew", "--owner", OTHER, "--ttl=120"]).await; + assert!(ok, "{stderr}"); + assert_eq!(seconds(&body["database_time"], &body["expires_at"]), 120); + assert!(console(&database, &["status", "--owner", OTHER]).await.0); + assert!(console(&database, &["release", "--owner", OTHER]).await.0); + let (ok, body, _) = console(&database, &["status"]).await; + assert!(ok); + assert_eq!([&body["active"], &body["owner"]], [&json!(false), &json!(OTHER)]); +} + +#[tokio::test] +async fn the_offline_command_never_initializes_a_database() { + // A database with no native schema is refused, and left without one. + let fresh = TestDatabase::new().await; + for args in [["status"].as_slice(), &["release", "--owner", OWNER], &["acquire", "--owner", OWNER]] { + let (ok, _, stderr) = console(&fresh, args).await; + assert!(!ok); + assert_eq!(stderr, "native_schema_not_initialized", "{args:?}"); + } + match &fresh { + TestDatabase::Sqlite(dir) => assert!(!dir.path().join("runtime.sqlite").exists(), "the command created a database"), + _ => assert_eq!(table_counts(&fresh).await, Vec::<(String, i64)>::new(), "the command created tables"), + } + + // An existing SQLite file that is not a native database is refused + // unchanged. + let temp = tempfile::tempdir().unwrap(); + let empty = temp.path().join("empty.sqlite"); + std::fs::write(&empty, b"").unwrap(); + let mut command = std::process::Command::new(env!("CARGO_BIN_EXE_durable-workflow-server")); + command.env_clear().env("DW_RUST_EXPERIMENTAL", "1").env("DB_CONNECTION", "sqlite").env("DB_DATABASE", &empty) + .args(["backup-hold", "release", "--owner", OWNER]); + let output = tokio::task::spawn_blocking(move || command.output().unwrap()).await.unwrap(); + assert!(!output.status.success()); + assert_eq!(first_line(&output.stderr), "native_schema_not_initialized"); + assert_eq!(std::fs::metadata(&empty).unwrap().len(), 0, "the command wrote to a non-native database"); +} From 3780f8be0cced19da2ebbc186ad08037df230dbb Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:00:43 -0600 Subject: [PATCH 2/3] backup-hold: open without any bootstrap path Security found the console's check-then-open sequence open to a race: each path ran a read-only check, then the ordinary open, which can bootstrap. If restore tooling emptied or replaced the database in between, the command could initialize it. Each backend now has open_existing, used only by the console. None of them has a bootstrap path: - SQLite never creates the file or switches it to WAL. The pool's own connection must find the marker and the exact schema. - PostgreSQL takes the bootstrap advisory lock on the pool's connection, then requires the marker and the exact schema. - MySQL requires the marker table and the exact schema on the pool's connection. Anything else is refused as it is. The separate check calls are gone, and so is the earlier MySQL check change. Tests, all three backends: - A fresh database is refused and left without a schema. - Counterfactual: the server's own open initializes the same database, and the command then serves it. - A database that loses dw_query_cache or its marker is refused and not repaired. - Negative control: restoring the bootstrapping open fails the test on SQLite and PostgreSQL. Co-Authored-By: Claude Opus 5.5 --- rust/src/runtime/backup_hold.rs | 11 ++--- rust/src/runtime/mysql.rs | 31 ++++++++++--- rust/src/runtime/postgres.rs | 31 +++++++++++++ rust/src/runtime/sqlite.rs | 45 ++++++++++++------- .../external_payloads/backup_hold.rs | 24 ++++++++++ 5 files changed, 114 insertions(+), 28 deletions(-) diff --git a/rust/src/runtime/backup_hold.rs b/rust/src/runtime/backup_hold.rs index 3ac07c53a..ecbee77c1 100644 --- a/rust/src/runtime/backup_hold.rs +++ b/rust/src/runtime/backup_hold.rs @@ -238,22 +238,19 @@ pub fn command(args: &[String]) -> Result { /// `backup-hold` against a native SQLite database that already exists. pub async fn console_sqlite(database: &str, command: &Command) -> Result { - let store = super::store::Store::new(super::sqlite::open_initialized(database).await?); + let store = super::store::Store::new(super::sqlite::open_existing(database).await?); store.run_backup_hold_command(command).await } -/// `backup-hold` against an initialized native PostgreSQL database; an empty -/// or foreign database is refused before the writable pool opens. +/// `backup-hold` against an initialized native PostgreSQL database. pub async fn console_postgres(options: sqlx::postgres::PgConnectOptions, command: &Command) -> Result { - super::postgres::PostgresStorage::check(options.clone()).await?; - let store = super::store::Store::new(super::postgres::PostgresStorage::open(options).await?.into_pool()); + let store = super::store::Store::new(super::postgres::PostgresStorage::open_existing(options).await?); store.run_backup_hold_command(command).await } /// `backup-hold` against an initialized native MySQL or MariaDB database. pub async fn console_mysql(options: sqlx::mysql::MySqlConnectOptions, command: &Command) -> Result { - super::mysql::MySqlStorage::check(options.clone()).await?; - let store = super::store::Store::new(super::mysql::MySqlStorage::open(options).await?.into_pool()); + let store = super::store::Store::new(super::mysql::MySqlStorage::open_existing(options).await?); store.run_backup_hold_command(command).await } diff --git a/rust/src/runtime/mysql.rs b/rust/src/runtime/mysql.rs index 791eb3d25..aa983e4e7 100644 --- a/rust/src/runtime/mysql.rs +++ b/rust/src/runtime/mysql.rs @@ -811,17 +811,38 @@ impl MySqlStorage { let pool = runtime_pool_options().connect_with(connect_options).await?; Ok(Self { pool }) } + /// Open an initialized native database for an offline console command + /// (Server #973). There is no bootstrap path: the pool's own connection + /// must find the native marker and the exact schema, or the database is + /// refused as it is. + pub(super) async fn open_existing(connect_options: MySqlConnectOptions) -> Result { + let pool = runtime_pool_options() + .max_connections(1) + .connect_with(options(connect_options)) + .await?; + let result = async { + let mut connection = pool.acquire().await?; + configure(&mut connection).await?; + let flavor = Flavor::detect(&mut connection).await?; + if !scope(&mut connection).await?.iter().any(|table| table == "dw_server_schema") { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } + verify(&mut connection, flavor).await?; + Ok::<_, super::RuntimeError>(()) + } + .await; + if let Err(error) = result { + pool.close().await; + return Err(error); + } + Ok(pool) + } pub async fn check(connect_options: MySqlConnectOptions) -> Result<()> { let mut connection = MySqlConnection::connect_with(&options(connect_options)).await?; let result = async { configure(&mut connection).await?; let flavor = Flavor::detect(&mut connection).await?; let mut tx = connection.begin_with("START TRANSACTION READ ONLY").await?; - // As on PostgreSQL: a database without the native marker is not - // initialized, rather than unreadable. - if !scope(&mut tx).await?.iter().any(|table| table == "dw_server_schema") { - return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); - } verify(&mut tx, flavor).await?; tx.rollback().await?; Ok::<_, super::RuntimeError>(()) diff --git a/rust/src/runtime/postgres.rs b/rust/src/runtime/postgres.rs index 3ff5ef5b8..a179a38bf 100644 --- a/rust/src/runtime/postgres.rs +++ b/rust/src/runtime/postgres.rs @@ -390,6 +390,37 @@ impl PostgresStorage { Ok(Self { pool }) } + /// Open an initialized native database for an offline console command + /// (Server #973). There is no bootstrap path: under the bootstrap + /// advisory lock, the pool's own connection must find the native marker + /// and the exact schema, or the database is refused as it is. + pub(super) async fn open_existing(connect_options: PgConnectOptions) -> Result { + let pool = PgPoolOptions::new() + .max_connections(1) + .acquire_timeout(Duration::from_secs(5)) + .connect_with(options(connect_options)) + .await?; + let result = async { + let mut tx = pool.begin().await?; + sqlx::query("SELECT pg_advisory_xact_lock(4924501473315676162)") + .execute(&mut *tx) + .await?; + verify_scope(&mut tx).await?; + if !marked(&mut tx).await? { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } + verify(&mut tx).await?; + tx.commit().await?; + Ok::<_, super::RuntimeError>(()) + } + .await; + if let Err(error) = result { + pool.close().await; + return Err(error); + } + Ok(pool) + } + /// Upgrade an earlier native schema version to this one, in one /// transaction under the bootstrap advisory lock. Returns the version /// found; a current database is only verified. diff --git a/rust/src/runtime/sqlite.rs b/rust/src/runtime/sqlite.rs index 9ba7806c8..92759c758 100644 --- a/rust/src/runtime/sqlite.rs +++ b/rust/src/runtime/sqlite.rs @@ -26,10 +26,11 @@ pub(super) async fn open(database: &str) -> Result { unreachable!("the final startup attempt always returns") } -/// Open a database that is already native, for an offline console command: -/// a missing, empty or foreign file is refused before any write, so the -/// command never bootstraps a schema. -pub(super) async fn open_initialized(database: &str) -> Result { +/// Open an initialized native database for an offline console command +/// (Server #973). There is no bootstrap path: the file is never created or +/// switched to WAL, and the pool's own connection must find the native marker +/// and the exact schema, or the database is refused as it is. +pub(super) async fn open_existing(database: &str) -> Result { if database.is_empty() || database.contains('?') || database.contains(':') { return Err(refuse( StatusCode::BAD_REQUEST, @@ -40,19 +41,31 @@ pub(super) async fn open_initialized(database: &str) -> Result { if !path.exists() { return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); } - let mut preflight = SqliteConnection::connect_with( - &SqliteConnectOptions::new() - .filename(path) - .read_only(true) - .foreign_keys(false), - ) - .await?; - let initialized = inspect(&mut preflight).await; - preflight.close().await?; - if !initialized? { - return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .acquire_timeout(Duration::from_secs(5)) + .connect_with( + SqliteConnectOptions::new() + .filename(path) + .create_if_missing(false) + .synchronous(SqliteSynchronous::Full) + .foreign_keys(true) + .busy_timeout(Duration::from_secs(5)), + ) + .await?; + let result = async { + let mut connection = pool.acquire().await?; + if !inspect(&mut connection).await? { + return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); + } + Ok::<_, super::RuntimeError>(()) + } + .await; + if let Err(error) = result { + pool.close().await; + return Err(error); } - open(database).await + Ok(pool) } async fn open_once(database: &str) -> Result { diff --git a/rust/tests/execution/external_payloads/backup_hold.rs b/rust/tests/execution/external_payloads/backup_hold.rs index 6e93dee26..9c70e13c2 100644 --- a/rust/tests/execution/external_payloads/backup_hold.rs +++ b/rust/tests/execution/external_payloads/backup_hold.rs @@ -312,6 +312,17 @@ async fn the_offline_command_never_initializes_a_database() { TestDatabase::Sqlite(dir) => assert!(!dir.path().join("runtime.sqlite").exists(), "the command created a database"), _ => assert_eq!(table_counts(&fresh).await, Vec::<(String, i64)>::new(), "the command created tables"), } + // Counterfactual: the server's own open initializes that same database, + // and the command then serves it. + fresh.open().await.unwrap().close().await; + let (ok, body, stderr) = console(&fresh, &["status"]).await; + assert!(ok, "{stderr}"); + assert_eq!(body["owner"], Value::Null); + + // A database that changes under the command, losing its marker or part of + // its schema, is refused and never repaired: there is no bootstrap path. + database_refuses_without_repair(&fresh, "DROP TABLE dw_query_cache", "dw_query_cache").await; + database_refuses_without_repair(&fresh, "DROP TABLE dw_server_schema", "dw_server_schema").await; // An existing SQLite file that is not a native database is refused // unchanged. @@ -326,3 +337,16 @@ async fn the_offline_command_never_initializes_a_database() { assert_eq!(first_line(&output.stderr), "native_schema_not_initialized"); assert_eq!(std::fs::metadata(&empty).unwrap().len(), 0, "the command wrote to a non-native database"); } + +/// Change `database` with `sql`, then check the command refuses it and leaves +/// `table` missing. +async fn database_refuses_without_repair(database: &TestDatabase, sql: &'static str, table: &str) { + database.execute(sql).await; + for args in [["status"].as_slice(), &["release", "--owner", OWNER]] { + let (ok, body, stderr) = console(database, args).await; + assert!(!ok && body.is_null(), "{sql}: {args:?} succeeded: {body}"); + assert!(!stderr.is_empty(), "{sql}: no refusal reason"); + } + let tables: Vec = table_counts(database).await.into_iter().map(|(name, _)| name).collect(); + assert!(!tables.iter().any(|name| name == table), "{sql}: the command recreated {table}"); +} From 4cd133d0135b84cd36d23877332f9daf61306fe0 Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:09:10 -0600 Subject: [PATCH 3/3] backup-hold: only release and a read-only inspect offline General, Cloud and Security settled the offline interface on least authority, matching Cloud's recorded contract for cloud#591: - `backup-hold release --owner ` (or `--owner=`): #953's release, by the recorded owner, idempotent, a lapsed restored hold included. - `backup-hold inspect`: the hold as stored, from plain SELECTs of row id=1 in one read-only snapshot. No lock, no row creation, no write. An absent row reports active false and owner null. Acquire, renew and status are gone from the console; live backups use the endpoints. Any other argument is refused before the database opens. inspect also opens read-only. SQLite opens the file read-only, and PostgreSQL checks the schema in a READ ONLY transaction without the bootstrap lock. MySQL's check was already one read-only transaction and now is for release too. Tests, all three backends: - inspect on a schema without the hold row reports no hold and creates none; - inspect leaves a restored row byte-identical; - release, both forms and repeated; - every other command or option is refused. Negative control: pointing inspect at the locking status path fails the test (SQLite refuses the write; PostgreSQL creates the row). Co-Authored-By: Claude Opus 5.5 --- docs/contracts/external-payload-storage.md | 23 ++-- docs/self-hosted-backup-and-restore.md | 3 +- rust/src/main.rs | 8 +- rust/src/runtime/backup_hold.rs | 103 +++++++++--------- rust/src/runtime/mysql.rs | 8 +- rust/src/runtime/postgres.rs | 18 ++- rust/src/runtime/sqlite.rs | 6 +- .../external_payloads/backup_hold.rs | 93 +++++++++------- 8 files changed, 145 insertions(+), 117 deletions(-) diff --git a/docs/contracts/external-payload-storage.md b/docs/contracts/external-payload-storage.md index 479744a11..f1071be7e 100644 --- a/docs/contracts/external-payload-storage.md +++ b/docs/contracts/external-payload-storage.md @@ -261,21 +261,24 @@ curl -fsS "$SERVER/api/system/external-payload-backup-hold?owner=$BACKUP_ID" \ -H "Authorization: Bearer $ADMIN_TOKEN" -H 'X-Durable-Workflow-Control-Plane-Version: 2' ``` -With no Server process running, for example while a restore is still -offline, the native image runs the same command against the database. It -reads only the database settings (`DB_CONNECTION` and its connection -variables), opens only an already initialized native database, prints the same -JSON report, and starts no listener or workers: +A restore that is still offline, with no Server process running, uses the +native image's two console commands instead. They read only the database +settings (`DB_CONNECTION` and its connection variables), open only an already +initialized native database, never create or change its schema, print the +same JSON report, and start no listener or workers: ```sh -docker run --rm --env-file server.env "$SERVER_IMAGE" \ +# The hold as stored, read-only: no lock, no row created, no write. +docker run --rm --env-file db.env "$SERVER_IMAGE" backup-hold inspect +# Release the restored hold by its recorded owner; repeating it is harmless. +docker run --rm --env-file db.env "$SERVER_IMAGE" \ backup-hold release --owner="$BACKUP_ID" -# Also: acquire|renew --owner=... [--ttl=900], and status [--owner=...]. ``` -On refusal it prints the reason (`backup_hold_not_owned`, -`backup_hold_owned_elsewhere`, `native_schema_not_initialized`, or the -validation message) and exits non-zero. +On refusal they print the reason (`backup_hold_not_owned`, +`native_schema_not_initialized`, or the owner validation message) and exit +non-zero. Acquire, renew and owner-scoped status stay on the endpoints, while +the Server runs. Each answers the command's JSON report. Where a command exits unsuccessfully, the endpoint answers 409: `backup_hold_owned_elsewhere` when another backup diff --git a/docs/self-hosted-backup-and-restore.md b/docs/self-hosted-backup-and-restore.md index a27433b55..37c236228 100644 --- a/docs/self-hosted-backup-and-restore.md +++ b/docs/self-hosted-backup-and-restore.md @@ -6,7 +6,8 @@ The published Compose file runs the native server from 3.0.0. On native 3.x Server, take and release the backup hold through its admin endpoints instead of `php artisan external-payloads:backup-hold`. To release a restored hold before any Server process starts, run the image's offline -`backup-hold release --owner=` command against the restored database. The +`backup-hold release --owner=` command against the restored database +(`backup-hold inspect` reads the hold without changing anything). The owner UUID, TTL limits and status checks are the same; see [coordinating online backups](contracts/external-payload-storage.md#coordinating-online-backups). diff --git a/rust/src/main.rs b/rust/src/main.rs index cff5d4108..ebc9f44d5 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -245,10 +245,10 @@ fn main() -> Result<(), Box> { .block_on(run()) } -/// `backup-hold acquire|renew|release|status [--owner ] [--ttl ]`: -/// PHP's `external-payloads:backup-hold`, offline (Server #973). It opens -/// only an initialized native database from the DB settings, prints the v1 -/// report, and on refusal prints the reason and exits non-zero. +/// `backup-hold release --owner ` and `backup-hold inspect`: what a +/// restore runs before any Server process starts (Server #973). It opens only +/// an initialized native database from the DB settings, `inspect` read-only, +/// prints the v1 report, and on refusal prints the reason and exits non-zero. async fn backup_hold_command() -> Result<(), Box> { use durable_workflow_server::runtime::backup_hold; let args: Vec = env::args().skip(2).collect(); diff --git a/rust/src/runtime/backup_hold.rs b/rust/src/runtime/backup_hold.rs index ecbee77c1..8bdd8cbbc 100644 --- a/rust/src/runtime/backup_hold.rs +++ b/rust/src/runtime/backup_hold.rs @@ -187,64 +187,53 @@ fn sql_time(at: DateTime) -> String { at.format("%Y-%m-%d %H:%M:%S").to_string() } -/// One offline `backup-hold` invocation, as PHP's -/// `external-payloads:backup-hold {action} --owner= --ttl=900` takes it -/// (Server #973): a restore releases the restored hold before any Server -/// process starts. +/// One offline `backup-hold` invocation (Server #973): only what a restore +/// needs before any Server process starts. Live backups acquire, renew and +/// check their hold through #953's endpoints. #[derive(Debug, PartialEq, Eq)] -pub struct Command { - /// `None` is `status`. - action: Option, - owner: Option, - ttl: i64, +pub enum Command { + /// `release --owner `: #953's release, by the hold's recorded + /// owner, idempotent, a lapsed restored hold included. + Release { owner: String }, + /// `inspect`: the hold as stored, read without a lock or a write. + Inspect, +} + +impl Command { + /// Whether the command writes; `inspect` opens the database read-only. + fn writes(&self) -> bool { + matches!(self, Self::Release { .. }) + } } -/// Parse the arguments after `backup-hold`: an action (`acquire`, `renew`, -/// `release` or `status`), then `--owner` and `--ttl`, each as `--name=value` -/// or `--name value` and at most once. The TTL must be an integer whatever -/// the action, as PHP's command checks it first. +/// Parse the arguments after `backup-hold`: `release --owner ` (or +/// `--owner=`), or `inspect` alone. pub fn command(args: &[String]) -> Result { let invalid = || super::refuse(StatusCode::BAD_REQUEST, "backup_hold_command_invalid"); - let mut args = args.iter(); - let action = match args.next().map(String::as_str) { - Some("acquire") => Some(Action::Acquire), - Some("renew") => Some(Action::Renew), - Some("release") => Some(Action::Release), - Some("status") => None, - _ => return Err(super::refuse(StatusCode::BAD_REQUEST, "backup_hold_action_invalid")), - }; - let (mut owner, mut ttl) = (None, None); - while let Some(arg) = args.next() { - let (name, value) = match arg.split_once('=') { - Some((name, value)) => (name, value.to_owned()), - None => (arg.as_str(), args.next().ok_or_else(invalid)?.clone()), - }; - let slot = match name { - "--owner" => &mut owner, - "--ttl" => &mut ttl, - _ => return Err(invalid()), - }; - if slot.replace(value).is_some() { - return Err(invalid()); + match args { + [action, rest @ ..] if action == "release" => { + let owner = match rest { + [option] => option.strip_prefix("--owner=").map(str::to_owned), + [flag, value] if flag == "--owner" => Some(value.clone()), + _ => None, + }; + owner.map(|owner| Command::Release { owner }).ok_or_else(invalid) } + [action] if action == "inspect" => Ok(Command::Inspect), + [action, ..] if action == "inspect" => Err(invalid()), + _ => Err(super::refuse(StatusCode::BAD_REQUEST, "backup_hold_action_invalid")), } - let ttl = match ttl { - None => DEFAULT_TTL_SECONDS, - Some(text) => super::laravel_validation::integer(&json!(text)) - .ok_or_else(|| super::refuse(StatusCode::BAD_REQUEST, "backup_hold_ttl_invalid"))?, - }; - Ok(Command { action, owner, ttl }) } /// `backup-hold` against a native SQLite database that already exists. pub async fn console_sqlite(database: &str, command: &Command) -> Result { - let store = super::store::Store::new(super::sqlite::open_existing(database).await?); + let store = super::store::Store::new(super::sqlite::open_existing(database, command.writes()).await?); store.run_backup_hold_command(command).await } /// `backup-hold` against an initialized native PostgreSQL database. pub async fn console_postgres(options: sqlx::postgres::PgConnectOptions, command: &Command) -> Result { - let store = super::store::Store::new(super::postgres::PostgresStorage::open_existing(options).await?); + let store = super::store::Store::new(super::postgres::PostgresStorage::open_existing(options, command.writes()).await?); store.run_backup_hold_command(command).await } @@ -326,16 +315,13 @@ store_methods! { Ok(next.report()) } - /// The offline command's report, from the same methods #953's endpoints - /// call. The pool closes whatever the outcome. + /// The offline command's report. Release is #953's own method; the pool + /// closes whatever the outcome. async fn run_backup_hold_command(&self, command: &Command) -> Result { - let result = match command.action { - None => match status_owner(command.owner.as_deref()) { - Ok(owner) => self.backup_hold_status(owner.as_deref()).await, - Err(error) => Err(error), - }, - Some(action) => match request(&json!({ "owner": command.owner, "ttl": command.ttl }), action) { - Ok(request) => self.change_backup_hold(action, &request).await, + let result = match command { + Command::Inspect => self.inspect_backup_hold().await, + Command::Release { owner } => match request(&json!({ "owner": owner }), Action::Release) { + Ok(request) => self.change_backup_hold(Action::Release, &request).await, Err(error) => Err(error), }, }; @@ -343,6 +329,21 @@ store_methods! { result } + /// The hold as stored, from plain reads in one read-only snapshot: no + /// lock, no row creation, no write. A missing row is no hold. + async fn inspect_backup_hold(&self) -> Result { + let mut tx = self.begin_read().await?; + let hold = match Self::read_backup_hold(&mut tx).await? { + Some(hold) => hold, + None => { + let now = Self::query(DB::BACKUP_HOLD_CLOCK).fetch_one(&mut *tx).await?; + Hold { owner: None, acquired_at: None, expires_at: None, now: DB::instant(&now, "database_time")? } + } + }; + tx.rollback().await?; + Ok(hold.report()) + } + /// `status`: the hold as it stands, refused when a scoped owner no longer /// holds it. pub(crate) async fn backup_hold_status(&self, owner: Option<&str>) -> Result { diff --git a/rust/src/runtime/mysql.rs b/rust/src/runtime/mysql.rs index aa983e4e7..25193b22e 100644 --- a/rust/src/runtime/mysql.rs +++ b/rust/src/runtime/mysql.rs @@ -814,7 +814,7 @@ impl MySqlStorage { /// Open an initialized native database for an offline console command /// (Server #973). There is no bootstrap path: the pool's own connection /// must find the native marker and the exact schema, or the database is - /// refused as it is. + /// refused as it is. The check only reads, in one read-only transaction. pub(super) async fn open_existing(connect_options: MySqlConnectOptions) -> Result { let pool = runtime_pool_options() .max_connections(1) @@ -824,10 +824,12 @@ impl MySqlStorage { let mut connection = pool.acquire().await?; configure(&mut connection).await?; let flavor = Flavor::detect(&mut connection).await?; - if !scope(&mut connection).await?.iter().any(|table| table == "dw_server_schema") { + let mut tx = connection.begin_with("START TRANSACTION READ ONLY").await?; + if !scope(&mut tx).await?.iter().any(|table| table == "dw_server_schema") { return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); } - verify(&mut connection, flavor).await?; + verify(&mut tx, flavor).await?; + tx.rollback().await?; Ok::<_, super::RuntimeError>(()) } .await; diff --git a/rust/src/runtime/postgres.rs b/rust/src/runtime/postgres.rs index a179a38bf..55b27aa93 100644 --- a/rust/src/runtime/postgres.rs +++ b/rust/src/runtime/postgres.rs @@ -393,18 +393,24 @@ impl PostgresStorage { /// Open an initialized native database for an offline console command /// (Server #973). There is no bootstrap path: under the bootstrap /// advisory lock, the pool's own connection must find the native marker - /// and the exact schema, or the database is refused as it is. - pub(super) async fn open_existing(connect_options: PgConnectOptions) -> Result { + /// and the exact schema, or the database is refused as it is. A command + /// that only reads checks in a read-only transaction, without the lock. + pub(super) async fn open_existing(connect_options: PgConnectOptions, writable: bool) -> Result { let pool = PgPoolOptions::new() .max_connections(1) .acquire_timeout(Duration::from_secs(5)) .connect_with(options(connect_options)) .await?; let result = async { - let mut tx = pool.begin().await?; - sqlx::query("SELECT pg_advisory_xact_lock(4924501473315676162)") - .execute(&mut *tx) - .await?; + let mut tx = if writable { + let mut tx = pool.begin().await?; + sqlx::query("SELECT pg_advisory_xact_lock(4924501473315676162)") + .execute(&mut *tx) + .await?; + tx + } else { + pool.begin_with("BEGIN ISOLATION LEVEL REPEATABLE READ READ ONLY").await? + }; verify_scope(&mut tx).await?; if !marked(&mut tx).await? { return Err(refuse(StatusCode::CONFLICT, "native_schema_not_initialized")); diff --git a/rust/src/runtime/sqlite.rs b/rust/src/runtime/sqlite.rs index 92759c758..3fa2ed044 100644 --- a/rust/src/runtime/sqlite.rs +++ b/rust/src/runtime/sqlite.rs @@ -29,8 +29,9 @@ pub(super) async fn open(database: &str) -> Result { /// Open an initialized native database for an offline console command /// (Server #973). There is no bootstrap path: the file is never created or /// switched to WAL, and the pool's own connection must find the native marker -/// and the exact schema, or the database is refused as it is. -pub(super) async fn open_existing(database: &str) -> Result { +/// and the exact schema, or the database is refused as it is. A command that +/// only reads opens the file read-only. +pub(super) async fn open_existing(database: &str, writable: bool) -> Result { if database.is_empty() || database.contains('?') || database.contains(':') { return Err(refuse( StatusCode::BAD_REQUEST, @@ -48,6 +49,7 @@ pub(super) async fn open_existing(database: &str) -> Result { SqliteConnectOptions::new() .filename(path) .create_if_missing(false) + .read_only(!writable) .synchronous(SqliteSynchronous::Full) .foreign_keys(true) .busy_timeout(Duration::from_secs(5)), diff --git a/rust/tests/execution/external_payloads/backup_hold.rs b/rust/tests/execution/external_payloads/backup_hold.rs index 9c70e13c2..351ec95f1 100644 --- a/rust/tests/execution/external_payloads/backup_hold.rs +++ b/rust/tests/execution/external_payloads/backup_hold.rs @@ -237,10 +237,29 @@ fn first_line(stderr: &[u8]) -> String { String::from_utf8_lossy(stderr).lines().next().unwrap_or_default().to_owned() } +/// The hold row as stored, as text, for comparing before and after a read. +async fn hold_rows(database: &TestDatabase) -> Vec<(String, String)> { + text_rows(database, + "SELECT COALESCE(owner,'')||'|'||COALESCE(acquired_at,''), COALESCE(expires_at,'') FROM runtime_external_payload_backup_hold", + "SELECT COALESCE(owner::text,'')||'|'||COALESCE(acquired_at::text,''), COALESCE(expires_at::text,'') FROM runtime_external_payload_backup_hold", + "SELECT CONCAT(COALESCE(owner,''),'|',COALESCE(CAST(acquired_at AS CHAR),'')), COALESCE(CAST(expires_at AS CHAR),'') FROM runtime_external_payload_backup_hold").await +} + #[tokio::test] async fn the_offline_command_releases_a_restored_hold_before_any_server_starts() { let temp = tempfile::tempdir().unwrap(); let (database, runtime, router) = setup(ExternalPayloadConfig::default(), &temp.path().join("objects")).await; + + // Native schemas start without the hold row. Inspect reports no hold and + // still leaves no row. + let (ok, body, stderr) = console(&database, &["inspect"]).await; + assert!(ok, "{stderr}"); + assert_eq!(body["schema"], "durable-workflow.external-payload-backup-hold.v1"); + assert_eq!([&body["active"], &body["owner"], &body["expires_at"]], [&json!(false), &Value::Null, &Value::Null]); + assert!(body["database_time"].is_string(), "{body}"); + assert_eq!(hold_rows(&database).await, Vec::<(String, String)>::new(), "inspect created the hold row"); + + // A restored snapshot carries the capturing backup's hold, long expired. let (code, body) = change(&router, "acquire", json!({"owner": OWNER, "ttl": 3600})).await; assert_eq!(code, StatusCode::OK, "{body}"); database.execute("UPDATE runtime_external_payload_backup_hold SET acquired_at='2000-01-01 00:00:00',expires_at='2000-01-01 01:00:00'").await; @@ -248,62 +267,54 @@ async fn the_offline_command_releases_a_restored_hold_before_any_server_starts() drop(router); runtime.close().await; - let (ok, body, stderr) = console(&database, &["status"]).await; + let restored = hold_rows(&database).await; + let (ok, body, stderr) = console(&database, &["inspect"]).await; assert!(ok, "{stderr}"); - assert_eq!(body["schema"], "durable-workflow.external-payload-backup-hold.v1"); - assert_eq!([&body["active"], &body["owner"]], [&json!(false), &json!(OWNER)]); - // A scoped status is refused once its owner no longer holds the hold. - assert_eq!(console(&database, &["status", "--owner", OWNER]).await.2, "backup_hold_not_owned"); + assert_eq!([&body["active"], &body["owner"], &body["acquired_at"], &body["expires_at"]], + [&json!(false), &json!(OWNER), &json!("2000-01-01 00:00:00"), &json!("2000-01-01 01:00:00")]); + assert_eq!(hold_rows(&database).await, restored, "inspect changed the hold row"); // Only the recorded owner releases it, in either option form, and a // second release reports the same released hold. assert_eq!(console(&database, &["release", "--owner", OTHER]).await.2, "backup_hold_not_owned"); - for args in [["release", "--owner", OWNER].as_slice(), [&*format!("--owner={OWNER}")].as_slice()] { - let args: Vec<&str> = if args.len() == 1 { vec!["release", args[0]] } else { args.to_vec() }; + assert_eq!(hold_rows(&database).await, restored); + let owner_option = format!("--owner={OWNER}"); + for args in [vec!["release", "--owner", OWNER], vec!["release", owner_option.as_str()]] { let (ok, body, stderr) = console(&database, &args).await; assert!(ok, "{stderr}"); assert_eq!([&body["active"], &body["owner"], &body["acquired_at"], &body["expires_at"]], [&json!(false), &json!(OWNER), &json!("2000-01-01 00:00:00"), &Value::Null]); } + let (ok, body, _) = console(&database, &["inspect"]).await; + assert!(ok); + assert_eq!([&body["owner"], &body["expires_at"]], [&json!(OWNER), &Value::Null]); - // PHP's validation, before the database is touched. + // Only release and inspect exist offline; everything else is refused + // before the database is opened. for (args, refusal) in [ - (vec!["release"], "The owner field is required."), - (vec!["release", "--owner", "not-a-uuid"], "The owner field must be a valid UUID."), - (vec!["acquire", "--owner", OTHER, "--ttl", "3601"], "The ttl field must not be greater than 3600."), - (vec!["status", "--ttl", "soon"], "backup_hold_ttl_invalid"), - (vec!["inspect"], "backup_hold_action_invalid"), - (vec![], "backup_hold_action_invalid"), - (vec!["release", "--owner", OWNER, "--owner", OTHER], "backup_hold_command_invalid"), + (vec!["release"], "backup_hold_command_invalid"), (vec!["release", "--owner"], "backup_hold_command_invalid"), + (vec!["release", "--owner", OWNER, "--owner", OTHER], "backup_hold_command_invalid"), + (vec!["release", "--owner", OWNER, "--ttl", "60"], "backup_hold_command_invalid"), (vec!["release", "--force"], "backup_hold_command_invalid"), + (vec!["release", "--owner", "not-a-uuid"], "The owner field must be a valid UUID."), + (vec!["inspect", "--owner", OWNER], "backup_hold_command_invalid"), + (vec!["acquire", "--owner", OTHER], "backup_hold_action_invalid"), + (vec!["renew", "--owner", OWNER], "backup_hold_action_invalid"), + (vec!["status"], "backup_hold_action_invalid"), + (vec![], "backup_hold_action_invalid"), ] { let (ok, body, stderr) = console(&database, &args).await; assert!(!ok && body.is_null(), "{args:?} succeeded: {body}"); assert_eq!(stderr, refusal, "{args:?}"); } - - // The released hold goes to the next backup, which renews and releases - // it; the database's clock decides every expiry. - let (ok, body, stderr) = console(&database, &["acquire", "--owner", OTHER, "--ttl", "60"]).await; - assert!(ok, "{stderr}"); - assert_eq!([&body["active"], &body["owner"]], [&json!(true), &json!(OTHER)]); - assert_eq!(seconds(&body["acquired_at"], &body["expires_at"]), 60); - let (ok, body, stderr) = console(&database, &["renew", "--owner", OTHER, "--ttl=120"]).await; - assert!(ok, "{stderr}"); - assert_eq!(seconds(&body["database_time"], &body["expires_at"]), 120); - assert!(console(&database, &["status", "--owner", OTHER]).await.0); - assert!(console(&database, &["release", "--owner", OTHER]).await.0); - let (ok, body, _) = console(&database, &["status"]).await; - assert!(ok); - assert_eq!([&body["active"], &body["owner"]], [&json!(false), &json!(OTHER)]); } #[tokio::test] async fn the_offline_command_never_initializes_a_database() { // A database with no native schema is refused, and left without one. let fresh = TestDatabase::new().await; - for args in [["status"].as_slice(), &["release", "--owner", OWNER], &["acquire", "--owner", OWNER]] { + for args in [["inspect"].as_slice(), &["release", "--owner", OWNER]] { let (ok, _, stderr) = console(&fresh, args).await; assert!(!ok); assert_eq!(stderr, "native_schema_not_initialized", "{args:?}"); @@ -315,7 +326,7 @@ async fn the_offline_command_never_initializes_a_database() { // Counterfactual: the server's own open initializes that same database, // and the command then serves it. fresh.open().await.unwrap().close().await; - let (ok, body, stderr) = console(&fresh, &["status"]).await; + let (ok, body, stderr) = console(&fresh, &["inspect"]).await; assert!(ok, "{stderr}"); assert_eq!(body["owner"], Value::Null); @@ -329,20 +340,22 @@ async fn the_offline_command_never_initializes_a_database() { let temp = tempfile::tempdir().unwrap(); let empty = temp.path().join("empty.sqlite"); std::fs::write(&empty, b"").unwrap(); - let mut command = std::process::Command::new(env!("CARGO_BIN_EXE_durable-workflow-server")); - command.env_clear().env("DW_RUST_EXPERIMENTAL", "1").env("DB_CONNECTION", "sqlite").env("DB_DATABASE", &empty) - .args(["backup-hold", "release", "--owner", OWNER]); - let output = tokio::task::spawn_blocking(move || command.output().unwrap()).await.unwrap(); - assert!(!output.status.success()); - assert_eq!(first_line(&output.stderr), "native_schema_not_initialized"); - assert_eq!(std::fs::metadata(&empty).unwrap().len(), 0, "the command wrote to a non-native database"); + for args in [["inspect"].as_slice(), &["release", "--owner", OWNER]] { + let mut command = std::process::Command::new(env!("CARGO_BIN_EXE_durable-workflow-server")); + command.env_clear().env("DW_RUST_EXPERIMENTAL", "1").env("DB_CONNECTION", "sqlite").env("DB_DATABASE", &empty) + .arg("backup-hold").args(args); + let output = tokio::task::spawn_blocking(move || command.output().unwrap()).await.unwrap(); + assert!(!output.status.success()); + assert_eq!(first_line(&output.stderr), "native_schema_not_initialized"); + assert_eq!(std::fs::metadata(&empty).unwrap().len(), 0, "the command wrote to a non-native database"); + } } /// Change `database` with `sql`, then check the command refuses it and leaves /// `table` missing. async fn database_refuses_without_repair(database: &TestDatabase, sql: &'static str, table: &str) { database.execute(sql).await; - for args in [["status"].as_slice(), &["release", "--owner", OWNER]] { + for args in [["inspect"].as_slice(), &["release", "--owner", OWNER]] { let (ok, body, stderr) = console(database, args).await; assert!(!ok && body.is_null(), "{sql}: {args:?} succeeded: {body}"); assert!(!stderr.is_empty(), "{sql}: no refusal reason");