diff --git a/crates/automation/src/a11y.rs b/crates/automation/src/a11y.rs index 1f0c9cb8a5..8fb1e0fd52 100644 --- a/crates/automation/src/a11y.rs +++ b/crates/automation/src/a11y.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use sha2::{Digest, Sha256}; use std::collections::BTreeSet; -use crate::{Args, Automation, Result, ToolError, child, failed, unused, write_atomic}; +use crate::{Args, Automation, Result, ToolError, child, failed, write_atomic}; mod documents; mod form_tools; diff --git a/crates/automation/src/a11y/documents.rs b/crates/automation/src/a11y/documents.rs index 5ae3d4fe46..ceb520e9d4 100644 --- a/crates/automation/src/a11y/documents.rs +++ b/crates/automation/src/a11y/documents.rs @@ -1,4 +1,4 @@ -use super::{Args, Automation, Result, ToolError, Value, child, failed, json, unused, write_atomic}; +use super::{Args, Automation, Result, ToolError, Value, child, failed, json, write_atomic}; impl Automation { pub(crate) fn doc_export_office(&self, a: &Args) -> Result { @@ -148,18 +148,7 @@ impl Automation { .and_then(|bytes| run_on(&action, &name, std::sync::Arc::new(bytes), |_, _| {})); out.push(match result { Ok(r) => { - let target = child(&folder, &name); - let target = if target.exists() { - let stem = - std::path::Path::new(&name).file_stem().map(|s| s.to_string_lossy().into_owned()).unwrap_or_else(|| "document".into()); - unused(&folder, &stem) - } else { - target - }; - if target.exists() { - return Err(failed(format!("{}: no unused output name available", target.display()))); - } - write_atomic(&target, &r.bytes)?; + let target = crate::batch::publish(&child(&folder, &name), &r.bytes)?; json!({ "path": p, "output": target.to_string_lossy(), "log": r.log }) } Err(e) => json!({ "path": p, "error": e }), diff --git a/crates/automation/src/batch.rs b/crates/automation/src/batch.rs index f650e565b7..14a98f23e2 100644 --- a/crates/automation/src/batch.rs +++ b/crates/automation/src/batch.rs @@ -281,7 +281,7 @@ impl Automation { } /// Publish atomically without replacing any existing name, even if another writer wins a race. -fn publish(target: &Path, bytes: &[u8]) -> Result { +pub(crate) fn publish(target: &Path, bytes: &[u8]) -> Result { let dir = target.parent().ok_or_else(|| failed("output has no parent"))?; std::fs::create_dir_all(dir).map_err(failed)?; let mut temp = tempfile::NamedTempFile::new_in(dir).map_err(failed)?; diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 28cbbfdca4..594a129d8b 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -70,7 +70,7 @@ use pages::{DEFAULT_DPI, MAX_DPI, encode_png, rendered_content}; use pages::{FIND_FIRST_BATCH, FIND_MAX_WARNING_BYTES, FIND_MAX_WARNINGS, extract_parallel}; #[cfg(test)] pub(crate) use paths::MAX_INPUT; -use paths::{child, lexical, unused, write_atomic}; +use paths::{child, lexical, write_atomic}; pub(crate) use paths::{read_input, read_input_bytes}; #[cfg(test)] use paths::{write_atomic_synced, write_atomic_with}; diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index f3ddc8e80d..9a042320ee 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -8368,6 +8368,22 @@ fn action_run_keeps_outputs_with_duplicate_input_basenames() { assert_eq!(std::fs::read(first_path).unwrap(), first_bytes, "an earlier output must not be replaced"); } +#[cfg(unix)] +#[test] +fn action_run_never_replaces_an_entry_that_exists_check_misses() { + let dir = workdir("action-noclobber"); + std::fs::create_dir_all(dir.join("results")).unwrap(); + std::fs::write(dir.join("report.pdf"), fixture(1)).unwrap(); + // A dangling link is invisible to `exists()` but is still an occupied name. + std::os::unix::fs::symlink(dir.join("nowhere.pdf"), dir.join("results/report.pdf")).unwrap(); + let mut a = auto(&dir); + let args = json!({ "steps": [{ "step": "set_title", "arg": "Processed" }], "paths": ["report.pdf"], "folder": "results" }); + let result = ok(&mut a, "action_run", args); + let output = result["files"][0]["output"].as_str().unwrap(); + assert!(output.ends_with("report (2).pdf"), "{output}"); + assert!(dir.join("results/report.pdf").symlink_metadata().unwrap().file_type().is_symlink(), "the existing entry must stay"); +} + #[test] fn pdfa_through_tools() { let dir = workdir("pdfa");