fix(save): replace files atomically

Replace native engine saves and engine/MCP exports atomically, preserving private permissions during staging and cleaning up failed writes. Add regressions for long filenames and caught writer unwinding.

Fixes #436.

Co-authored-by: aiongg <aiongtaigi@gmail.com>
Co-authored-by: skylar taylor-barrick <hi@skylar.cc>
This commit is contained in:
aiongg
2026-10-12 09:19:36 -06:00
committed by GitHub
co-authored by aiongg skylar taylor-barrick
parent 3624f48819
commit 5cdcac4ace
13 changed files with 417 additions and 19 deletions
+3 -3
View File
@@ -28,7 +28,7 @@ fn no_book() -> EngineError {
#[cfg(not(target_arch = "wasm32"))]
fn save(b: &Book) -> Result<()> {
let bytes = serde_json::to_vec_pretty(b).map_err(|e| EngineError::Other(e.to_string()))?;
std::fs::write(&b.path, bytes).map_err(|e| EngineError::Other(format!("{}: {e}", b.path)))
crate::atomic::write_file(&b.path, &bytes)
}
#[cfg(target_arch = "wasm32")]
@@ -52,7 +52,7 @@ fn load(path: &str) -> Result<designcraft_doc::Document> {
fn store(path: &str, d: &designcraft_doc::Document) -> Result<()> {
#[cfg(not(target_arch = "wasm32"))]
return std::fs::write(path, super::file::to_bytes(d)).map_err(|e| EngineError::Other(format!("{path}: {e}")));
return crate::atomic::write_file(path, &super::file::to_bytes(d));
#[cfg(target_arch = "wasm32")]
{
let _ = (path, d);
@@ -210,7 +210,7 @@ pub fn specs() -> Vec<CommandSpec> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, &bytes)?;
Ok(json!({"path": path, "bytes": bytes.len(), "pages": pages}))
}
None => Ok(json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len(), "pages": pages})),
+5 -5
View File
@@ -44,7 +44,7 @@ fn print_booklet(s: &mut Session, p: &Value) -> Result<Value> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, &r.bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, &r.bytes)?;
Ok(json!({"path": path, "bytes": r.bytes.len(), "sheets": r.pages, "warnings": r.warnings}))
}
None => Ok(json!({"base64": super::file::base64_encode(&r.bytes), "bytes": r.bytes.len(), "sheets": r.pages, "warnings": r.warnings})),
@@ -59,7 +59,7 @@ fn export_html(s: &mut Session, p: &Value) -> Result<Value> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, text.as_bytes())?;
Ok(json!({"path": path, "bytes": text.len()}))
}
None => Ok(json!({"bytes": text.len(), "text": text})),
@@ -91,7 +91,7 @@ fn export_text(s: &mut Session, p: &Value) -> Result<Value> {
match path {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, text.as_bytes())?;
Ok(json!({"path": path, "bytes": text.len()}))
}
None => Ok(json!({"bytes": text.len(), "text": text})),
@@ -160,7 +160,7 @@ fn export_epub(s: &mut Session, p: &Value) -> Result<Value> {
match p.get("path").and_then(Value::as_str) {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, &bytes).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, &bytes)?;
Ok(serde_json::json!({"path": path, "bytes": bytes.len()}))
}
None => Ok(serde_json::json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len()})),
@@ -598,7 +598,7 @@ fn export_pdf(s: &mut Session, p: &Value) -> Result<Value> {
#[cfg(not(target_arch = "wasm32"))]
fn write_file(path: &str, bytes: &[u8]) -> Result<()> {
std::fs::write(path, bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))
crate::atomic::write_file(path, bytes)
}
#[cfg(target_arch = "wasm32")]
fn write_file(path: &str, _bytes: &[u8]) -> Result<()> {
+34 -3
View File
@@ -27,7 +27,7 @@ pub fn specs() -> Vec<CommandSpec> {
let path = str_param(p, "path").ok_or_else(|| bad("file.saveACopy", "missing `path`"))?.to_string();
let bytes = to_bytes(&s.doc()?.doc);
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(&path, &bytes)?;
Ok(json!({"path": path, "bytes": bytes.len()}))
}),
cmd!(noundo "file.revert", "Revert", ["File"], None, "{} — back to the last saved version (not undoable)", can_revert, file_revert),
@@ -247,7 +247,7 @@ fn file_save(s: &mut Session, p: &Value) -> Result<Value> {
}
let bytes = to_bytes(&st.doc);
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(&path, &bytes)?;
st.path = Some(path.clone());
st.saved_revision = st.revision;
st.saved_doc = st.doc.clone();
@@ -518,7 +518,7 @@ fn snippet_export(s: &mut Session, p: &Value) -> Result<Value> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, &bytes)?;
Ok(json!({"path": path, "bytes": bytes.len(), "items": d.spreads.first().map(|s| s.items.len()).unwrap_or(0)}))
}
None => Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()})),
@@ -735,6 +735,37 @@ mod layout_place_tests {
}
}
#[cfg(test)]
mod save_tests {
use serde_json::json;
use crate::Session;
/// Saving over an existing document replaces it whole and leaves nothing beside it.
#[test]
fn save_replaces_the_existing_file_without_leftovers() {
let dir = std::env::temp_dir().join(format!("dc-save-atomic-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("doc.designcraft");
std::fs::write(&path, vec![b'x'; 1 << 20]).unwrap();
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Saved whole", "caret": false})).unwrap();
s.execute("file.save", &json!({"path": path.to_string_lossy()})).unwrap();
s.execute("file.saveACopy", &json!({"path": path.to_string_lossy()})).unwrap();
let saved = super::from_bytes(&std::fs::read(&path).unwrap()).unwrap();
assert!(saved.stories.values().any(|st| st.text == "Saved whole"));
let names: Vec<_> = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name()).collect();
assert_eq!(names, ["doc.designcraft"], "no temporary file is left");
// A folder that doesn't exist: an error naming the path.
let missing = dir.join("missing").join("doc.designcraft");
let e = s.execute("file.save", &json!({"path": missing.to_string_lossy()})).unwrap_err();
assert!(e.to_string().contains("missing"), "{e}");
let _ = std::fs::remove_dir_all(&dir);
}
}
#[cfg(test)]
mod document_fonts_tests {
use std::path::{Path, PathBuf};
+1 -1
View File
@@ -25,7 +25,7 @@ fn export_idml(s: &mut Session, p: &Value) -> Result<Value> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, &bytes)?;
Ok(json!({"path": path, "bytes": bytes.len()}))
}
None => Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()})),
+1 -1
View File
@@ -29,7 +29,7 @@ impl Library {
#[cfg(not(target_arch = "wasm32"))]
if let Some(p) = &self.path {
let bytes = serde_json::to_vec_pretty(self).map_err(|e| crate::EngineError::Other(e.to_string()))?;
std::fs::write(p, bytes).map_err(|e| crate::EngineError::Other(format!("{p}: {e}")))?;
crate::atomic::write_file(p, &bytes)?;
}
Ok(())
}
+5 -1
View File
@@ -844,7 +844,11 @@ fn save_swatches(s: &mut Session, p: &Value) -> Result<Value> {
let bytes = designcraft_color::ase::write(&chosen);
match str_param(p, "path") {
Some(path) => {
std::fs::write(path, &bytes).map_err(|e| bad("swatch.save", format!("{path}: {e}")))?;
#[cfg(not(target_arch = "wasm32"))]
let written = crate::atomic::write_atomic(std::path::Path::new(path), &bytes);
#[cfg(target_arch = "wasm32")]
let written = std::fs::write(path, &bytes);
written.map_err(|e| bad("swatch.save", format!("{path}: {e}")))?;
Ok(json!({"path": path, "bytes": bytes.len()}))
}
None => Ok(json!({"base64": super::file::base64_encode(&bytes)})),
+1 -1
View File
@@ -305,7 +305,7 @@ pub fn specs() -> Vec<CommandSpec> {
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
std::fs::write(path, text.as_bytes()).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?;
crate::atomic::write_file(path, text.as_bytes())?;
Ok(json!({"path": path, "bytes": text.len()}))
}
None => Ok(json!({"text": text, "bytes": text.len()})),