Share one exporter, and flush the file before saying it is written (#686)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

crates-cli/yaak-cli/src/commands/import_export.rs

@@ -3,7 +3,7 @@ use crate::context::CliContext;
 use crate::utils::workspace::resolve_workspace_id;
 use std::fs;
 use std::io::ErrorKind;
-use yaak::export::{self, ExportDataParams};
+use yaak_models::export::{self, ExportDataParams};
 use yaak::import;
 use yaak_models::util::{
     BatchUpsertResult, ImportDestination, ImportOrigin, ImportPlanAction, ImportPlanItem,
@@ -127,6 +127,46 @@ fn format_skipped(items: &[ImportPlanItem]) -> Option<String> {
     if parts.is_empty() { None } else { Some(parts.join(", ")) }
 }
 
+/// Write a file so that afterwards it is either the old one or the whole new one.
+///
+/// Writing in place would truncate the previous export before the new one existed, so an
+/// interrupted run loses both. Instead the bytes go to a neighbouring file, are flushed, and
+/// take the target's name in one step. The directory is flushed too: `sync_all` on a file
+/// promises its contents, not that the name it just gained will survive a power cut.
+fn write_durably(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> {
+    use std::io::Write;
+
+    let dir = match path.parent() {
+        Some(p) if !p.as_os_str().is_empty() => p,
+        _ => std::path::Path::new("."),
+    };
+    // Alongside the target, because a rename is only atomic within one filesystem.
+    let mut name = path.file_name().unwrap_or_default().to_os_string();
+    name.push(format!(".tmp{}", std::process::id()));
+    let tmp = dir.join(name);
+
+    let write = |tmp: &std::path::Path| -> std::io::Result<()> {
+        let mut file = std::fs::File::create(tmp)?;
+        file.write_all(bytes)?;
+        file.sync_all()
+    };
+    if let Err(e) = write(&tmp) {
+        let _ = std::fs::remove_file(&tmp);
+        return Err(e);
+    }
+    if let Err(e) = std::fs::rename(&tmp, path) {
+        let _ = std::fs::remove_file(&tmp);
+        return Err(e);
+    }
+
+    // Only Unix lets a directory be opened for this. Elsewhere the rename is as much as
+    // the platform offers, and the file itself is already flushed.
+    #[cfg(unix)]
+    std::fs::File::open(dir)?.sync_all()?;
+
+    Ok(())
+}
+
 fn export(ctx: &CliContext, args: ExportArgs) -> CommandResult<usize> {
     let workspace_ids = resolve_export_workspace_ids(ctx, args.workspace_ids, args.all)?;
     let workspace_id_refs: Vec<&str> = workspace_ids.iter().map(String::as_str).collect();
@@ -137,7 +177,10 @@ fn export(ctx: &CliContext, args: ExportArgs) -> CommandResult<usize> {
         include_private_environments: args.include_private_environments,
     })
     .map_err(|e| format!("Failed to export data: {e}"))?;
-    std::fs::write(&args.file, document)
+    // Flushed before reporting success: an export is a backup, and a backup that the
+    // command called done while it was still only in the page cache is the one case
+    // where saying nothing went wrong is worst.
+    write_durably(&args.file, document.as_bytes())
         .map_err(|e| format!("Failed to write {}: {e}", args.file.display()))?;
 
     Ok(workspace_ids.len())

crates/yaak-commands/src/data.rs

@@ -3,7 +3,7 @@
 use crate::error::Result;
 use crate::host::Host;
 use yaak::example::create_example_workspace;
-use yaak::export::{self, ExportDataParams};
+use yaak_models::export::{self, ExportDataParams};
 use yaak_models::util::BatchUpsertResult;
 use yaak_rpc_schema::*;
 use yaak_templates::format_json::format_json;

crates/yaak-models/src/export.rs

@@ -1,6 +1,12 @@
-use crate::Result;
+//! Building an export document.
-use yaak_models::query_manager::QueryManager;
+//!
-use yaak_models::util::get_workspace_export_resources;
+//! Here rather than in `yaak` because the whole of it is this crate: a connection, the
+//! resources, and JSON. Putting it where the browser can reach it too is what keeps a
+//! tab's export from being a second implementation that drifts.
+
+use crate::error::Result;
+use crate::query_manager::QueryManager;
+use crate::util::get_workspace_export_resources;
 
 pub struct ExportDataParams<'a> {
     pub query_manager: &'a QueryManager,
@@ -12,8 +18,8 @@ pub struct ExportDataParams<'a> {
 /// The export document, as JSON.
 ///
 /// Returned rather than written: where an export goes is the host's to decide, and a browser
-/// tab has no path to be handed. The desktop hands the bytes to its save dialog; a tab hands
+/// tab has no path to be handed. The desktop hands the bytes to its save dialog, the CLI
-/// them to a download. Neither needs this function to know which.
+/// writes them, a tab downloads them. None of that is this function's business.
 pub fn export_data(params: ExportDataParams<'_>) -> Result<String> {
     let db = params.query_manager.connect();
     let export_data = get_workspace_export_resources(

crates/yaak-models/src/lib.rs

@@ -13,6 +13,7 @@ pub mod client_db;
 mod connection_or_tx;
 pub mod cookies;
 pub mod error;
+pub mod export;
 pub mod migrate;
 pub mod models;
 pub mod models_ops;

crates/yaak-wasm/pkg/yaak_wasm_bg.js

@@ -697,22 +697,22 @@ export function __wbg_warn_b6f36cac66fc96a4(arg0, arg1) {
     console.warn(arg0, arg1);
 }
 export function __wbindgen_cast_0000000000000001(arg0, arg1) {
-    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 1115, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
+    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 1116, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
     const ret = makeMutClosure(arg0, arg1, wasm_bindgen_9b5275515258a0f8___convert__closures_____invoke___wasm_bindgen_9b5275515258a0f8___JsValue__core_f0fd674eaa06beef___result__Result_____wasm_bindgen_9b5275515258a0f8___JsError___true_);
     return ret;
 }
 export function __wbindgen_cast_0000000000000002(arg0, arg1) {
-    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Event")], shim_idx: 207, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Event")], shim_idx: 206, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
     const ret = makeMutClosure(arg0, arg1, wasm_bindgen_9b5275515258a0f8___convert__closures_____invoke___web_sys_e1a11cd1518a8b4d___features__gen_Event__Event______true_);
     return ret;
 }
 export function __wbindgen_cast_0000000000000003(arg0, arg1) {
-    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("IDBVersionChangeEvent")], shim_idx: 172, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
+    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("IDBVersionChangeEvent")], shim_idx: 113, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
     const ret = makeMutClosure(arg0, arg1, wasm_bindgen_9b5275515258a0f8___convert__closures_____invoke___web_sys_e1a11cd1518a8b4d___features__gen_IdbVersionChangeEvent__IdbVersionChangeEvent__core_f0fd674eaa06beef___result__Result_____wasm_bindgen_9b5275515258a0f8___JsValue___true_);
     return ret;
 }
 export function __wbindgen_cast_0000000000000004(arg0, arg1) {
-    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [], shim_idx: 209, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+    // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [], shim_idx: 208, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
     const ret = makeMutClosure(arg0, arg1, wasm_bindgen_9b5275515258a0f8___convert__closures_____invoke_______true_);
     return ret;
 }

crates/yaak-wasm/src/lib.rs

@@ -416,20 +416,20 @@ fn dispatch(
             to_json(())
         }
 
-        // The export document, built by the same `yaak-models` helper the desktop and the CLI
+        // The same `export_data` the desktop and the CLI call, not a second copy of its
-        // build it with. Nothing about what an export *is* is decided here — this host only
+        // steps. Nothing about what an export *is* is decided here; this host differs only
-        // differs in what happens to the bytes afterwards, which is the tab's business.
+        // in what happens to the bytes afterwards, which is the tab's business.
         "cmd_export_data" => {
             let req: ExportDataReq = from_js(payload)?;
-            let db = host.queries.connect();
+            to_json(
-            let export = yaak_models::util::get_workspace_export_resources(
+                yaak_models::export::export_data(yaak_models::export::ExportDataParams {
-                &db,
+                    query_manager: &host.queries,
-                EXPORT_VERSION,
+                    yaak_version: EXPORT_VERSION,
-                req.workspace_ids.iter().map(|s| s.as_str()).collect(),
+                    workspace_ids: req.workspace_ids.iter().map(|s| s.as_str()).collect(),
-                req.include_private_environments,
+                    include_private_environments: req.include_private_environments,
+                })
+                .map_err(js_error)?,
             )
-            .map_err(js_error)?;
-            to_json(serde_json::to_string_pretty(&export).map_err(js_error)?)
         }
 
         "cmd_get_workspace_meta" => {

crates/yaak/src/lib.rs

@@ -1,6 +1,5 @@
 pub mod error;
 pub mod example;
-pub mod export;
 pub mod import;
 pub mod plugin_events;
 pub mod response_body;