mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-02 22:41:47 -06:00
Refuse an OCR batch whose inputs would write one output file
The ocr_recognize_files tool wrote every input's searchable copy to the same name in the output folder. Two inputs with one file name (a/scan.pdf and b/scan.pdf) therefore wrote the same output: the second replaced the first while both results reported success. Reproduced on the unchanged tree with the OCR models installed: both results pointed at out/scan.pdf, and the folder held one file. The whole batch is now refused before anything happens. The output folder is not created, nothing is read, and the OCR engines are not loaded. The error names the colliding file names and says what to do. Names are compared the way the default filesystem compares them: without regard to case on Windows and macOS, exactly elsewhere, and on Windows a trailing dot or space is not part of the name. The compared name is the one the output is written under, after the separator replacement the writer already applies. A request without collisions runs as before. Not changed here: the batch dialog in crates/ui-egui (ocr_ui.rs) writes its outputs the same way and needs its own change. Checked: the three ocr_recognize_files tests in crates/automation/tests/automation.rs and the unit test shared_output_names_follow_the_default_filesystem, run with the models installed and without them. Each pinned line was reverted in turn and the test that pins it failed; the file was restored after each step.
This commit is contained in:
@@ -601,11 +601,22 @@ impl Automation {
|
||||
for p in &paths {
|
||||
self.resolve_for(p, crate::file_access::Access::Read, true)?;
|
||||
}
|
||||
// Two inputs that would write the same output file overwrite each other while both report
|
||||
// success, so the whole batch is refused here: before the folder is created, anything is
|
||||
// read, or the OCR engines load.
|
||||
let targets: Vec<std::path::PathBuf> = paths.iter().map(|p| child(&folder, &input_file_name(p))).collect();
|
||||
let shared = shared_file_names(&targets);
|
||||
if !shared.is_empty() {
|
||||
return Err(ToolError::InvalidArgs(format!(
|
||||
"more than one input would write to the same output file, so each would replace the one before it. Colliding file names: {}. Give each input a different file name, or run them in separate batches with different folders.",
|
||||
shared.join(", ")
|
||||
)));
|
||||
}
|
||||
std::fs::create_dir_all(&folder).map_err(|e| failed(e.to_string()))?;
|
||||
let ocr = pdfcraft_engine::ocr::engine().map_err(failed)?;
|
||||
let mut out = Vec::new();
|
||||
for p in paths {
|
||||
let name = std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into());
|
||||
let name = input_file_name(p);
|
||||
let result =
|
||||
self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| std::fs::read(&src).map_err(|e| e.to_string())).and_then(|bytes| {
|
||||
pdfcraft_engine::ocr::recognize_file(&name, std::sync::Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true)
|
||||
@@ -650,6 +661,36 @@ impl Automation {
|
||||
}
|
||||
}
|
||||
|
||||
/// The file name an input is written under: its own file name, or `document.pdf` when it has none.
|
||||
fn input_file_name(path: &str) -> String {
|
||||
std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into())
|
||||
}
|
||||
|
||||
/// The output file names that two or more of `targets` share, each spelling listed once, in input
|
||||
/// order. Names are compared the way the default filesystem compares them: without regard to case
|
||||
/// on Windows and macOS and exactly elsewhere; on Windows a trailing dot or space is not part of
|
||||
/// the name (`scan.pdf.` is `scan.pdf`). The folder is the same for every target, so only the
|
||||
/// names are compared.
|
||||
fn shared_file_names(targets: &[std::path::PathBuf]) -> Vec<String> {
|
||||
let mut groups: std::collections::BTreeMap<String, (usize, Vec<String>)> = std::collections::BTreeMap::new();
|
||||
for target in targets {
|
||||
let name = target.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
|
||||
let key = if cfg!(windows) {
|
||||
name.trim_end_matches(['.', ' ']).to_lowercase()
|
||||
} else if cfg!(target_os = "macos") {
|
||||
name.to_lowercase()
|
||||
} else {
|
||||
name.clone()
|
||||
};
|
||||
let (count, spellings) = groups.entry(key).or_insert_with(|| (0, Vec::new()));
|
||||
*count += 1;
|
||||
if !spellings.contains(&name) {
|
||||
spellings.push(name);
|
||||
}
|
||||
}
|
||||
groups.into_values().filter(|(count, _)| *count > 1).flat_map(|(_, spellings)| spellings).collect()
|
||||
}
|
||||
|
||||
fn request_json(r: &pdfcraft_engine::js::Request) -> Value {
|
||||
use pdfcraft_engine::js::Request as R;
|
||||
match r {
|
||||
@@ -1042,3 +1083,21 @@ mod review_html_tests {
|
||||
assert!(!html.contains("<script>"));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod shared_output_name_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn shared_output_names_follow_the_default_filesystem() {
|
||||
let targets = |names: &[&str]| -> Vec<std::path::PathBuf> { names.iter().map(|n| std::path::Path::new("out").join(n)).collect() };
|
||||
// A spelling that repeats is one shared name, listed once.
|
||||
assert_eq!(shared_file_names(&targets(&["a.pdf", "b.pdf", "a.pdf"])), vec!["a.pdf".to_string()]);
|
||||
assert!(shared_file_names(&targets(&["a.pdf", "b.pdf"])).is_empty());
|
||||
// Case is ignored on Windows and macOS only; a trailing dot is ignored on Windows only.
|
||||
let case = shared_file_names(&targets(&["scan.pdf", "Scan.PDF"]));
|
||||
let dot = shared_file_names(&targets(&["scan.pdf", "scan.pdf."]));
|
||||
assert_eq!(case.len(), if cfg!(any(windows, target_os = "macos")) { 2 } else { 0 }, "{case:?}");
|
||||
assert_eq!(dot.len(), if cfg!(windows) { 2 } else { 0 }, "{dot:?}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5802,6 +5802,60 @@ fn ocr_recognize_files_writes_searchable_copies() {
|
||||
assert!(found.to_string().contains("\"page\""), "{found}");
|
||||
}
|
||||
|
||||
/// Two inputs that share a file name (from different folders) would both write `out/scan.pdf`:
|
||||
/// the second output replaces the first while both results report success. The batch is refused
|
||||
/// up front, naming the file, before the folder is created, anything is read or the OCR engines
|
||||
/// load, so the outcome is the same with or without the models installed.
|
||||
#[test]
|
||||
fn ocr_recognize_files_refuses_inputs_that_share_a_file_name() {
|
||||
let dir = workdir("ocr-files-same-name");
|
||||
let mut a = auto(&dir);
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/scan.pdf" }));
|
||||
let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/scan.pdf"], "folder": "out" }));
|
||||
assert!(matches!(&r, Err(ToolError::InvalidArgs(m)) if m.contains("scan.pdf")), "{r:?}");
|
||||
assert!(!dir.join("out").exists(), "nothing was started: the output folder was not created");
|
||||
}
|
||||
|
||||
/// File names are compared the way the default filesystem compares them: without regard to case
|
||||
/// on Windows and macOS (`scan.pdf` and `Scan.PDF` are one file there), and exactly elsewhere,
|
||||
/// where they are two files and the batch is not refused.
|
||||
#[test]
|
||||
fn ocr_recognize_files_compares_names_the_way_the_filesystem_does() {
|
||||
let dir = workdir("ocr-files-case");
|
||||
let mut a = auto(&dir);
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/Scan.PDF" }));
|
||||
let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/Scan.PDF"], "folder": "out" }));
|
||||
if cfg!(any(windows, target_os = "macos")) {
|
||||
assert!(matches!(&r, Err(ToolError::InvalidArgs(m)) if m.contains("Scan.PDF")), "{r:?}");
|
||||
assert!(!dir.join("out").exists(), "nothing was started: the output folder was not created");
|
||||
} else {
|
||||
assert!(!matches!(&r, Err(ToolError::InvalidArgs(_))), "{r:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Inputs with distinct names are not refused, and the batch writes one searchable copy per input
|
||||
/// under its own name. Without the models it stops at the models error, as it did before.
|
||||
#[test]
|
||||
fn ocr_recognize_files_with_distinct_names_runs_as_before() {
|
||||
let dir = workdir("ocr-files-distinct");
|
||||
let mut a = auto(&dir);
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
|
||||
ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/other.pdf" }));
|
||||
if ok(&mut a, "ocr_status", json!({}))["available"] == true {
|
||||
let r = ok(&mut a, "ocr_recognize_files", json!({ "paths": ["one/scan.pdf", "two/other.pdf"], "folder": "out" }));
|
||||
assert!(r["files"][0]["error"].is_null() && r["files"][1]["error"].is_null(), "{r}");
|
||||
assert!(dir.join("out/scan.pdf").is_file() && dir.join("out/other.pdf").is_file(), "{r}");
|
||||
} else {
|
||||
let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/other.pdf"], "folder": "out" }));
|
||||
assert!(matches!(&r, Err(ToolError::Failed(m)) if m.contains("not installed")), "{r:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn javascript_through_tools() {
|
||||
let dir = workdir("js");
|
||||
|
||||
Reference in New Issue
Block a user