Merge pull request #2668 from storytold/land/2260

Saves to a missing folder write the file and say which folder was created (#2260)

apps/cadcraft-cli/src/output.rs

@@ -122,10 +122,21 @@ pub(crate) fn export_window(s: &Session, framing: Framing, before: Option<&[(u64
 /// Write `bytes` to `out` through the engine's atomic writer (an exclusively created sibling file
 /// renamed over `out`), so a failed or interrupted write leaves the previous file untouched. An
 /// existing `out` is resolved first: the rename would otherwise replace a symlink with a file and
-/// leave the file it points to (`--in-place` through a link) unchanged.
-pub(crate) fn write_atomic(out: &Path, bytes: &[u8]) -> Result<(), OutputError> {
+/// leave the file it points to (`--in-place` through a link) unchanged. Folders missing above `out`
+/// are created, and the topmost one returned for the caller to report.
+pub(crate) fn write_atomic(out: &Path, bytes: &[u8]) -> Result<Option<std::path::PathBuf>, OutputError> {
     let target = std::fs::canonicalize(out).unwrap_or_else(|_| out.to_path_buf());
-    cadcraft_engine::atomic_write::write_atomic(&target, bytes).map_err(|source| OutputError::Write { path: out.to_path_buf(), source })
+    let folder = cadcraft_engine::atomic_write::missing_folder(&target);
+    cadcraft_engine::atomic_write::write_atomic_making_folders(&target, bytes)
+        .map_err(|source| OutputError::Write { path: out.to_path_buf(), source })?;
+    Ok(folder)
+}
+
+/// Say that `out` went into a folder that didn't exist, so a mistyped path is noticed.
+pub(crate) fn report_new_folder(folder: Option<std::path::PathBuf>, io: &mut Streams) {
+    if let Some(folder) = folder {
+        io.errln(format_args!("created the folder {}, which did not exist", folder.display()));
+    }
 }
 
 pub(crate) fn export(s: &Session, out: &Path, io: &mut Streams) -> Result {
@@ -135,7 +146,8 @@ pub(crate) fn export(s: &Session, out: &Path, io: &mut Streams) -> Result {
 pub(crate) fn export_framed(s: &Session, out: &Path, window: Option<Bounds2>, io: &mut Streams) -> Result {
     let d = s.doc()?;
     let bytes = cadcraft_io::write_framed(d, out, window)?;
-    write_atomic(out, &bytes)?;
+    let folder = write_atomic(out, &bytes)?;
+    report_new_folder(folder, io);
     io.errln(format_args!("wrote {} ({} bytes)", out.display(), bytes.len()));
     if cadcraft_engine::cmd::file::drops_unknown(out) {
         let lost = cadcraft_engine::cmd::file::not_saved(d);

apps/cadcraft-cli/src/sample.rs

@@ -1,6 +1,6 @@
 use super::args::{SampleArgs, SampleName};
 use super::error::Result;
-use super::output::{Streams, write_atomic};
+use super::output::{Streams, report_new_folder, write_atomic};
 
 pub(crate) fn sample(a: &SampleArgs, io: &mut Streams) -> Result {
     let d = match a.name {
@@ -8,7 +8,8 @@ pub(crate) fn sample(a: &SampleArgs, io: &mut Streams) -> Result {
         SampleName::Floorplan => cadcraft_engine::sample::floor_plan(),
     };
     let bytes = cadcraft_io::write(&d, &a.output)?;
-    write_atomic(&a.output, &bytes)?;
+    let folder = write_atomic(&a.output, &bytes)?;
+    report_new_folder(folder, io);
     io.errln(format_args!("wrote {} ({} bytes)", a.output.display(), bytes.len()));
     Ok(())
 }

crates/engine/src/atomic_write.rs

@@ -42,6 +42,27 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> {
     write_atomic_with(path, permissions, |file| file.write_all(bytes))
 }
 
+/// The topmost folder missing above `path`: what [`write_atomic_making_folders`] will create.
+/// `None` when the file's folder exists, or when `path` itself exists (a dangling symlink's target
+/// folder is not ours to make).
+pub fn missing_folder(path: &Path) -> Option<PathBuf> {
+    if std::fs::symlink_metadata(path).is_ok() {
+        return None;
+    }
+    path.ancestors().skip(1).filter(|a| !a.as_os_str().is_empty()).take_while(|a| std::fs::symlink_metadata(a).is_err()).last().map(Path::to_path_buf)
+}
+
+/// [`write_atomic`] to a path the user named, first creating the folders missing above it: a
+/// mistyped folder costs less than a drawing that could not be written. The caller reports
+/// [`missing_folder`], asked before this call, so the new folder is never a surprise.
+pub fn write_atomic_making_folders(path: &Path, bytes: &[u8]) -> io::Result<()> {
+    if let Some(folder) = missing_folder(path) {
+        let dir = path.parent().unwrap_or(&folder);
+        std::fs::create_dir_all(dir).map_err(|e| io::Error::new(e.kind(), format!("the folder {} could not be created: {e}", dir.display())))?;
+    }
+    write_atomic(path, bytes)
+}
+
 /// [`write_atomic`] filling the temporary file through `fill` (streamed rather than one buffer),
 /// with `permissions` given to it from its creation (`None`: the usual creation mode). A symlink
 /// at `path` is replaced by the new file, never written through.
@@ -159,6 +180,30 @@ fn create_exclusive(path: &Path, permissions: Option<&Permissions>) -> io::Resul
 mod tests {
     use super::*;
 
+    #[test]
+    fn creates_missing_parent_directories() {
+        let dir = std::env::temp_dir().join(format!("cadcraft-aw-mkdir-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed)));
+        let dest = dir.join("new_folder").join("deeper").join("out.bin");
+        std::fs::create_dir_all(&dir).unwrap();
+        assert_eq!(missing_folder(&dest), Some(dir.join("new_folder")));
+        assert!(write_atomic(&dest, b"created").is_err(), "the plain writer never makes folders");
+        write_atomic_making_folders(&dest, b"created").unwrap();
+        assert_eq!(std::fs::read(&dest).unwrap(), b"created");
+        assert_eq!(missing_folder(&dest), None);
+        std::fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn a_folder_that_cannot_be_made_is_named_in_the_error() {
+        let dir = std::env::temp_dir().join(format!("cadcraft-aw-nomk-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed)));
+        std::fs::create_dir_all(&dir).unwrap();
+        let blocker = dir.join("a_file");
+        std::fs::write(&blocker, b"x").unwrap();
+        let e = write_atomic_making_folders(&blocker.join("sub").join("out.bin"), b"new").unwrap_err();
+        assert!(e.to_string().contains("could not be created"), "{e}");
+        std::fs::remove_dir_all(&dir).ok();
+    }
+
     #[test]
     fn replaces_contents_atomically() {
         let dir = std::env::temp_dir().join(format!("cadcraft-aw-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed)));
@@ -259,6 +304,8 @@ mod tests {
         let link = dir.join("link.dxf");
         symlink("missing/real.dxf", &link).unwrap();
         assert!(write_atomic(&link, b"new").is_err());
+        assert!(write_atomic_making_folders(&link, b"new").is_err());
+        assert!(!dir.join("missing").exists(), "a link's unavailable target folder must not be created");
         assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink());
         std::fs::remove_dir_all(dir).unwrap();
     }

crates/engine/src/cmd/blocks/wblock.rs

@@ -8,7 +8,7 @@ use cadcraft_geom::{Mat3, Vec2, Vec3};
 use serde_json::{Value, json};
 
 use super::super::curves::KW;
-use super::super::file::{drops_unknown, not_saved, not_saved_message, report_not_saved};
+use super::super::file::{drops_unknown, new_folder, not_saved, not_saved_message, report_new_folder, report_not_saved};
 use super::super::machines::{SelOutcome, SelectPhase};
 use super::super::modify::erase_kept;
 use super::super::{CommandSpec, bad, point_param, str_param, targets};
@@ -43,11 +43,13 @@ enum Source<'a> {
 fn run_wblock(s: &mut Session, p: &Value) -> Result<Value> {
     let path = str_param(p, "path").ok_or_else(|| bad("wblock", "`path` is required"))?.to_string();
     let base = point_param(p, "base");
+    let folder = new_folder(&path);
     let (entities, bytes, lost) = match str_param(p, "name") {
         Some(name) => write(s, "wblock", &path, Source::Block(name, base))?,
         None => write(s, "wblock", &path, Source::Objects(&targets(s, p)?, base.unwrap_or(Vec2::ZERO)))?,
     };
     let mut r = json!({ "path": path, "entities": entities, "bytes": bytes });
+    report_new_folder(s, &mut r, folder);
     report_not_saved(s, &mut r, &lost);
     Ok(r)
 }
@@ -58,6 +60,7 @@ fn run_dash_wblock(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let path = str_param(p, "path").ok_or_else(|| bad("-wblock", "`path` is required"))?.to_string();
     let base = point_param(p, "base");
+    let folder = new_folder(&path);
     let (entities, bytes, lost) = match str_param(p, "name") {
         Some(name) => write(s, "-wblock", &path, Source::Block(name, base))?,
         None => {
@@ -72,6 +75,7 @@ fn run_dash_wblock(s: &mut Session, p: &Value) -> Result<Value> {
         }
     };
     let mut r = json!({ "path": path, "entities": entities, "bytes": bytes });
+    report_new_folder(s, &mut r, folder);
     report_not_saved(s, &mut r, &lost);
     Ok(r)
 }
@@ -101,7 +105,7 @@ fn write(s: &Session, cmd: &str, path: &str, what: Source) -> Result<(usize, usi
     let bytes = (hooks.write)(&d, path).map_err(|e| bad(cmd, e))?;
     // Through a temporary file, as SAVE writes, so a failed write keeps the file it replaces.
     #[cfg(not(target_arch = "wasm32"))]
-    crate::atomic_write::write_atomic(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?;
+    crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?;
     let lost = if drops_unknown(path) { not_saved(&d) } else { BTreeMap::new() };
     Ok((d.model.len(), bytes.len(), lost))
 }

crates/engine/src/cmd/exchange.rs

@@ -82,7 +82,7 @@ fn write_file(cmd: &str, path: &str, bytes: &[u8]) -> Result<()> {
     {
         // Exclusively create a fresh temporary file and rename it into place so a planted
         // `<path>.cadcraft-tmp` symlink cannot redirect the export write (CWE-59).
-        crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))
+        crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))
     }
     #[cfg(target_arch = "wasm32")]
     {
@@ -119,6 +119,7 @@ fn run_export(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let file = format!("export.{format}");
     let bytes = (hooks.write)(s.doc()?, &file).map_err(|e| bad("export", e))?;
+    let folder = path.and_then(super::file::new_folder);
     let mut r = match path {
         Some(path) => {
             write_file("export", path, &bytes)?;
@@ -126,6 +127,7 @@ fn run_export(s: &mut Session, p: &Value) -> Result<Value> {
         }
         None => json!({ "data": base64_encode(&bytes), "format": format, "bytes": bytes.len() }),
     };
+    super::file::report_new_folder(s, &mut r, folder);
     super::file::report_loss(s, &mut r, &file);
     Ok(r)
 }

crates/engine/src/cmd/file.rs

@@ -446,7 +446,7 @@ fn store(path: &str, bytes: &[u8]) -> Result<()> {
         }
         // Exclusively create a fresh temporary file and rename it over `path`, so a planted
         // `<path>.cadcraft-tmp` symlink cannot redirect the write onto another file (CWE-59).
-        crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad("save", format!("{path}: {e}")))
+        crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad("save", format!("{path}: {e}")))
     }
     #[cfg(target_arch = "wasm32")]
     {
@@ -851,6 +851,28 @@ fn report_sat_text_dropped(s: &mut Session, r: &mut Value) {
     }
 }
 
+/// The folder a write to `path` has to create, asked before writing so it can be reported after.
+/// The web has no folders.
+pub(super) fn new_folder(path: &str) -> Option<String> {
+    #[cfg(not(target_arch = "wasm32"))]
+    return crate::atomic_write::missing_folder(std::path::Path::new(path)).map(|f| f.display().to_string());
+    #[cfg(target_arch = "wasm32")]
+    {
+        let _ = path;
+        None
+    }
+}
+
+/// Say on the command line that the file went into a folder that didn't exist, so a mistyped path
+/// is noticed, and name the folder in the result's `createdFolder`.
+pub(super) fn report_new_folder(s: &mut Session, r: &mut Value, folder: Option<String>) {
+    let Some(folder) = folder else { return };
+    s.echo(format!("Created the folder {folder}, which did not exist. Check the path if you did not mean to."));
+    if let Some(o) = r.as_object_mut() {
+        o.insert("createdFolder".into(), json!(folder));
+    }
+}
+
 /// Say on the command line what the file couldn't keep, and list it in the result's `notSaved`.
 pub(super) fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::collections::BTreeMap<String, usize>) {
     let Some(msg) = not_saved_message(lost) else { return };
@@ -867,7 +889,9 @@ fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> R
     let path = str_param(p, "path").map(str::to_string).or_else(|| s.state().ok().and_then(|st| st.path.clone())).unwrap_or_default();
     let version = asked.unwrap_or_else(|| drawing_version(s, &path));
     let lost = s.doc().map(|d| not_saved_as(d, version)).unwrap_or_default();
+    let folder = new_folder(&path);
     let mut r = run(s, p)?;
+    report_new_folder(s, &mut r, folder);
     if r.get("path").and_then(Value::as_str).is_some_and(drops_unknown) {
         report_not_saved(s, &mut r, &lost);
     } else if version >= Version::R2013 && r.get("path").and_then(Value::as_str).is_some_and(is_dxf_file) {

crates/engine/src/cmd/layout.rs

@@ -840,8 +840,11 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result<Value> {
                 let path = _path;
                 // Exclusively create a fresh temporary file and rename it into place so a planted
                 // `<path>.cadcraft-tmp` symlink cannot redirect the plot write (CWE-59).
-                crate::atomic_write::write_atomic(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?;
-                Ok(json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages }))
+                let folder = super::file::new_folder(path);
+                crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?;
+                let mut r = json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages });
+                super::file::report_new_folder(s, &mut r, folder);
+                Ok(r)
             }
             #[cfg(target_arch = "wasm32")]
             {

crates/engine/src/cmd/layout/saveas.rs

@@ -104,8 +104,10 @@ pub(super) fn run_saveas(s: &mut Session, p: &Value) -> Result<Value> {
     let t = template_of(d, &name)?;
     let hooks = io().ok_or_else(|| bad("layout.saveas", "file formats are not available in this build"))?;
     let bytes = (hooks.write)(&t, &path).map_err(|e| bad("layout.saveas", e))?;
+    let folder = super::super::file::new_folder(&path);
     store(&path, &bytes)?;
     let mut r = json!({ "name": name, "path": path, "bytes": bytes.len() });
+    super::super::file::report_new_folder(s, &mut r, folder);
     let lost = not_saved(&t);
     if drops_unknown(&path)
         && let Some(msg) = not_saved_message(&lost)
@@ -124,7 +126,7 @@ fn store(path: &str, bytes: &[u8]) -> Result<()> {
     {
         // Exclusively create a fresh temporary file and rename it into place so a planted
         // `<path>.cadcraft-tmp` symlink cannot redirect the write (CWE-59).
-        crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad("layout.saveas", format!("{path}: {e}")))
+        crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad("layout.saveas", format!("{path}: {e}")))
     }
     #[cfg(target_arch = "wasm32")]
     {

crates/engine/tests/save_creates_missing_folders.rs

@@ -0,0 +1,62 @@
+//! A save to a folder that doesn't exist writes the file anyway and says what it created, so a
+//! mistyped path costs a stray folder, not the drawing (#2215).
+
+use cadcraft_engine::Session;
+use cadcraft_engine::cmd::file::{IoHooks, set_io};
+use cadcraft_engine::doc::Drawing;
+use serde_json::json;
+use std::path::PathBuf;
+
+fn scratch(name: &str) -> PathBuf {
+    let dir = std::env::temp_dir().join(format!("cadcraft-new-folder-{}-{name}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    dir
+}
+
+fn session() -> Session {
+    set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0\nEOF\n".to_vec()), plot: None });
+    let mut s = Session::empty();
+    s.open_drawing(Drawing::new_imperial(), "a.dxf", None);
+    s
+}
+
+#[test]
+fn saveas_and_export_write_the_file_and_report_the_new_folder() {
+    let dir = scratch("cmds");
+    let mut s = session();
+    for (cmd, name) in [("saveas", "one"), ("export", "two")] {
+        let top = dir.join(name);
+        let path = top.join("deeper").join("a.dxf");
+        let r = s.execute(cmd, &json!({ "path": path.to_string_lossy() })).unwrap();
+        assert!(path.is_file(), "{cmd} wrote no file");
+        assert_eq!(r["createdFolder"], json!(top.to_string_lossy()), "{cmd}");
+        let said = top.display().to_string();
+        assert!(s.log.iter().any(|l| l.contains(&said) && l.starts_with("Created the folder")), "{cmd}: {:?}", s.log);
+    }
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+#[test]
+fn a_save_into_an_existing_folder_reports_nothing() {
+    let dir = scratch("quiet");
+    let mut s = session();
+    let path = dir.join("a.dxf");
+    let r = s.execute("saveas", &json!({ "path": path.to_string_lossy() })).unwrap();
+    assert!(r.get("createdFolder").is_none());
+    assert!(!s.log.iter().any(|l| l.starts_with("Created the folder")), "{:?}", s.log);
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+#[test]
+fn a_folder_that_cannot_be_made_fails_and_says_so() {
+    let dir = scratch("blocked");
+    let blocker = dir.join("a_file");
+    std::fs::write(&blocker, b"x").unwrap();
+    let mut s = session();
+    let path = blocker.join("sub").join("a.dxf");
+    let e = s.execute("saveas", &json!({ "path": path.to_string_lossy() })).unwrap_err().to_string();
+    assert!(e.contains("could not be created"), "{e}");
+    assert!(!s.log.iter().any(|l| l.starts_with("Created the folder")), "{:?}", s.log);
+    let _ = std::fs::remove_dir_all(&dir);
+}

crates/mcp/src/backend.rs

@@ -415,10 +415,17 @@ impl Backend for Headless {
                 let (w, h) = (view.width, view.height);
                 let png =
                     cadcraft_render::raster::render_png(&list, &view, &cadcraft_render::raster::RasterOptions::default()).ok_or("render failed")?;
+                let mut r =
+                    json!({"pngBase64": cadcraft_engine::cmd::file::base64_encode(&png), "width": w, "height": h, "truncated": list.truncated});
                 if let Some(path) = str_p("path") {
-                    std::fs::write(&path, &png).map_err(|e| format!("{path}: {e}"))?;
+                    let file = std::path::Path::new(&path);
+                    let folder = cadcraft_engine::atomic_write::missing_folder(file);
+                    cadcraft_engine::atomic_write::write_atomic_making_folders(file, &png).map_err(|e| format!("{path}: {e}"))?;
+                    if let Some(folder) = folder {
+                        r["createdFolder"] = json!(folder.display().to_string());
+                    }
                 }
-                Ok(json!({"pngBase64": cadcraft_engine::cmd::file::base64_encode(&png), "width": w, "height": h, "truncated": list.truncated}))
+                Ok(r)
             }
             "app.open" => {
                 let path = str_p("path").ok_or("missing path")?;

crates/ui-egui/src/control.rs

@@ -489,10 +489,16 @@ pub fn handle(app: &mut CadApp, ctx: &egui::Context, req: &ControlRequest) -> Ou
             };
             #[cfg(not(target_arch = "wasm32"))]
             if let Some(path) = s("path") {
-                if let Err(e) = std::fs::write(path, &png) {
+                let file = std::path::Path::new(&path);
+                let folder = cadcraft_engine::atomic_write::missing_folder(file);
+                if let Err(e) = cadcraft_engine::atomic_write::write_atomic_making_folders(file, &png) {
                     return err(e);
                 }
-                return ok(render_reply(&png, w, h, Some(path), list.truncated));
+                let mut reply = render_reply(&png, w, h, Some(path), list.truncated);
+                if let Some(folder) = folder {
+                    reply["createdFolder"] = json!(folder.display().to_string());
+                }
+                return ok(reply);
             }
             ok(render_reply(&png, w, h, None, list.truncated))
         }