From abf29ea392061b3ae81a5541c5afbef03c365d0e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:53:46 +0000 Subject: [PATCH 1/2] #1508: web backup refuses to start while changes are unsaved The browser backup copies the stored library files, so while a save to browser storage is failing it reported success but left out the queued changes. It now refuses with the number of unsaved changes and why (Session::unsaved), and is allowed again once they are written. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- apps/lightcraft-web/src/backup.rs | 12 ++++++++++++ apps/lightcraft-web/src/files.rs | 26 ++++++++++++++++++++++++++ apps/lightcraft-web/src/web.rs | 3 +++ docs/web.md | 4 +++- docs/whats-new.md | 8 ++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/apps/lightcraft-web/src/backup.rs b/apps/lightcraft-web/src/backup.rs index 00ec12a41..281a32296 100644 --- a/apps/lightcraft-web/src/backup.rs +++ b/apps/lightcraft-web/src/backup.rs @@ -52,6 +52,18 @@ pub fn valid_hash(h: &str) -> bool { !h.is_empty() && h.len() <= 128 && h.chars().all(|c| c.is_ascii_alphanumeric()) } +/// Why a backup can't start yet, given [`lightcraft_engine::Session::unsaved`]: changes applied in +/// the app but not yet written to browser storage (a save failed). The zip copies the stored +/// library files, so it would report success while missing them; `None` when everything is saved. +pub fn unsaved_refusal(unsaved: Option<(usize, &str)>) -> Option { + let (ops, why) = unsaved?; + let changes = if ops == 1 { "1 change".to_string() } else { format!("{ops} changes") }; + Some(format!( + "Backup not started: {changes} could not be saved to browser storage yet ({why}), so the backup would miss them. \ + Free up storage space; LightCraft retries the save with every change. Back up once nothing is left unsaved." + )) +} + /// Zip entry name for an original. pub fn original_entry(hash: &str, file_name: &str) -> String { let name: String = file_name.chars().map(|c| if c == '/' || c == '\\' || c.is_control() { '_' } else { c }).collect(); diff --git a/apps/lightcraft-web/src/files.rs b/apps/lightcraft-web/src/files.rs index d0c9f348b..4b5f6289d 100644 --- a/apps/lightcraft-web/src/files.rs +++ b/apps/lightcraft-web/src/files.rs @@ -282,6 +282,32 @@ mod tests { assert_eq!(s2.catalog.photo(id).unwrap().rating, 5, "the rating survived"); } + /// A backup copies the stored library files, so while a save is failing it would miss the + /// queued changes: it is refused with the reason until they are written (#1508). + #[test] + fn backup_is_refused_while_changes_are_unsaved() { + let files = Files::default(); + let mut s = Session::new(); + let stores = LibraryStores { dir: "browser".into(), catalog: Box::new(files.store()), files: Box::new(files.store()), on_disk: false }; + s.open_library_in(stores, true).unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "everything saved: the backup may start"); + files.set_error(Some("QuotaExceededError".into())); + let _ = s.execute("photo.rate", &json!({"rating": 5})); + let refusal = crate::backup::unsaved_refusal(s.unsaved()).expect("refused while the rating is unsaved"); + assert!(refusal.contains("could not be saved") && refusal.contains("QuotaExceeded"), "{refusal}"); + files.set_error(None); + s.persist_if_dirty(); + s.persist().unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "allowed again once the change is written"); + } + + #[test] + fn unsaved_refusal_counts_changes() { + use crate::backup::unsaved_refusal; + assert!(unsaved_refusal(Some((1, "quota"))).unwrap().contains("1 change could not")); + assert!(unsaved_refusal(Some((3, "quota"))).unwrap().contains("3 changes could not")); + } + #[test] fn library_survives_reload_through_flushes() { let mut disk = BTreeMap::new(); diff --git a/apps/lightcraft-web/src/web.rs b/apps/lightcraft-web/src/web.rs index 6a4487d09..3ce8eb66c 100644 --- a/apps/lightcraft-web/src/web.rs +++ b/apps/lightcraft-web/src/web.rs @@ -173,6 +173,9 @@ fn services(originals: Originals, backend: Option, files: Files, frozen screen_profile: None, backup_library: Some(Box::new(move |session: &mut Session| { let Some(b) = backup_backend.clone() else { return Err("nothing is stored in this browser session (?store=memory)".into()) }; + if let Some(why) = crate::backup::unsaved_refusal(session.unsaved()) { + return Err(why); + } // the photos' own names for the originals in the zip let names: std::collections::HashMap = session .catalog diff --git a/docs/web.md b/docs/web.md index b275e837d..a755bfd80 100644 --- a/docs/web.md +++ b/docs/web.md @@ -111,7 +111,9 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not grant it, a notice says the library may be evicted under storage pressure. - **File ▸ Back Up Library…** downloads a zip of the library files and every stored original (`originals//`, so an unzipped backup is browsable; entries are stored, not - compressed; at most 4 GB). **File ▸ Restore Library from Backup…** reads such a zip into a + compressed; at most 4 GB). While a save is failing it refuses to start and says how many + changes are unsaved and why: the zip copies the stored library files, so it would miss them + (issue #1508). **File ▸ Restore Library from Backup…** reads such a zip into a new library folder (`library-restored-