Merge PR #2272: #2266: a presets.json that can't be written counts as unsaved

docs/whats-new.md: entry added under the existing 2026-10-18 heading
This commit is contained in:
aiongg
2026-10-21 14:52:05 +00:00
3 changed files with 50 additions and 5 deletions
+20 -5
View File
@@ -84,6 +84,9 @@ pub struct Library {
/// Why the last append of queued ops failed; `None` once a save succeeds. While set, changes
/// exist only in memory ([`Session::unsaved`]).
pub unsaved_error: Option<String>,
/// Why the last write of presets.json failed; `None` once it is written. While set, the preset
/// changes exist only in memory and count as unsaved ([`Session::unsaved`], issue #2266).
presets_error: Option<String>,
/// When the frame loop may retry a failed append ([`Session::persist_if_dirty`] backs off).
retry_at: Option<web_time::Instant>,
/// What forgetting untouched Local records did when the library opened.
@@ -577,6 +580,7 @@ impl Session {
report,
last_error: None,
unsaved_error: None,
presets_error: None,
retry_at: None,
forgot_local: None,
presets_saved,
@@ -651,10 +655,17 @@ impl Session {
let Some(lib) = self.library.as_mut() else { return Ok(()) };
if state.hash == lib.presets_saved.hash {
lib.presets_saved = state;
lib.presets_error = None;
} else if let Err(e) = lib.files.write_atomic("presets.json", presets.as_bytes()) {
// the state stays stale: the next save tries again
// the state stays stale: retried by the next command and the frame loop (backing off);
// meanwhile the presets count as unsaved, so the UI warns and a backup refuses
log::error!("library: presets: {e}");
lib.presets_error = Some(format!("presets: {e}"));
lib.retry_at = Some(web_time::Instant::now() + RETRY_BACKOFF);
} else {
if lib.presets_error.take().is_some() {
log::info!("library: presets written to disk");
}
lib.presets_saved = state;
}
Ok(())
@@ -664,7 +675,8 @@ impl Session {
/// changed outside [`Session::execute`]).
pub fn persist_if_dirty(&mut self) {
let backing_off = self.library.as_ref().and_then(|l| l.retry_at).is_some_and(|t| web_time::Instant::now() < t);
if self.library.is_some() && !self.pending_log.is_empty() && !backing_off {
let presets_unsaved = self.library.as_ref().is_some_and(|l| l.presets_error.is_some());
if self.library.is_some() && (!self.pending_log.is_empty() || presets_unsaved) && !backing_off {
let _ = self.persist();
} else if let Some(lib) = self.library.as_mut() {
poll_compaction(lib);
@@ -672,11 +684,14 @@ impl Session {
}
/// Changes applied in memory but not yet written to disk because saving failed: the number of
/// queued ops and why (`None` when everything is saved, or no library is open). Retried by
/// every command and by [`Session::persist_if_dirty`].
/// queued ops (plus one for presets.json) and why (`None` when everything is saved, or no library
/// is open). Retried by every command and by [`Session::persist_if_dirty`].
pub fn unsaved(&self) -> Option<(usize, &str)> {
let lib = self.library.as_ref()?;
lib.unsaved_error.as_deref().filter(|_| !self.pending_log.is_empty()).map(|e| (self.pending_log.len(), e))
let ops = lib.unsaved_error.as_deref().filter(|_| !self.pending_log.is_empty());
let presets = lib.presets_error.as_deref();
let n = ops.map_or(0, |_| self.pending_log.len()) + usize::from(presets.is_some());
ops.or(presets).map(|why| (n, why))
}
/// Flush everything and write a snapshot (on quit). Ends an open interaction first.
+25
View File
@@ -191,3 +191,28 @@ fn close_fails_when_nothing_can_be_saved() {
s.close_library().unwrap();
assert_eq!(s.unsaved(), None);
}
/// Issue #2266: a presets.json that can't be written isn't only logged: the preset counts as
/// unsaved (the top bar warns, the web backup refuses) until a later save writes it.
#[test]
fn a_preset_that_cannot_be_written_counts_as_unsaved_until_it_is() {
let files = Flaky::default();
let mut s = Session::new();
let stores = LibraryStores { dir: "flaky".into(), catalog: Box::new(MemStore::new()), files: Box::new(files.clone()), on_disk: false };
s.open_library_in(stores, true).unwrap();
assert!(s.unsaved().is_none());
files.set_failing(true);
s.execute("preset.create", &json!({"name": "Mine"})).unwrap();
let (n, why) = s.unsaved().expect("the preset is unsaved");
assert_eq!(n, 1);
assert!(why.contains("presets") && why.contains("disk full"), "{why}");
let stored = |f: &Flaky| f.files.files.lock().unwrap().get("presets.json").cloned().unwrap_or_default();
assert!(!String::from_utf8_lossy(&stored(&files)).contains("Mine"));
// still failing: still unsaved
s.persist().unwrap();
assert!(s.unsaved().is_some());
files.set_failing(false);
s.persist().unwrap();
assert!(s.unsaved().is_none(), "written once the disk works again");
assert!(String::from_utf8_lossy(&stored(&files)).contains("Mine"));
}
+5
View File
@@ -8,6 +8,11 @@
## 2026-10-18
### A preset that couldn't be saved is no longer lost silently
- When presets.json can't be written (a full disk, or the browser's storage quota), the new or changed preset now
counts as unsaved, like other changes: the top bar warns, saving is retried, and quitting asks first (issue #2266).
Before, the failure was only logged, and the preset could be lost without any warning.
### Portrait crop ratios stay portrait
- A crop ratio no longer flips to the photo's orientation (issue #2253): after X turns the crop portrait, picking
another ratio (4 × 5, 2 × 3…) keeps it portrait, as in Lightroom, and a Custom ratio taller than wide gives a