From c9c5c43aa11326d3523cc56d07f0df3c33dd6bb2 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Mon, 17 Aug 2026 12:32:20 +0200 Subject: [PATCH] Carry the library across when its home moves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit moved durable data out of Android's cache directory, and on its own that would have been an upgrade that quietly discarded work. The app looks in the new location, finds nothing, and rescans a library of tens of thousands of images over the network — while the old copy, including every offline rating and edit that had not yet synced, sits in a directory the system is free to delete. So the account's directory is moved once at startup, before anything opens a store. A rename rather than a copy: both are inside the app's own data on one filesystem, so it is atomic and cannot half-finish. An existing destination wins and the move is skipped — that covers a second run and a fresh install, and in neither case may this overwrite live data. A failed move is logged, not fatal. The cost is a rescan, which is recoverable; refusing to start is not. Two tests, against ordinary directories rather than the platform's idea of a cache: one puts an unsynced sidecar in the old location and asserts it is readable in the new one afterwards, the other pins that live data is never overwritten. Co-Authored-By: Claude Opus 5 --- ui/dr-ui/src/lib.rs | 4 ++ ui/dr-ui/src/library.rs | 116 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index f8b8b07..73c6ac4 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -846,6 +846,10 @@ pub fn run(paths: Vec) -> Result<()> { let session = controller.model.borrow().session().cloned(); if let Some(session) = session { log::info!("resuming library for {}", session.describe()); + // Before anything opens a store: an upgrade must not + // abandon a catalog, its thumbnails, or the offline + // ratings and edits waiting beside them. + library::migrate_legacy_cache_data(&session.server, &session.user_id); library_ui::open( &window, library.clone(), diff --git a/ui/dr-ui/src/library.rs b/ui/dr-ui/src/library.rs index b4fe230..173db79 100644 --- a/ui/dr-ui/src/library.rs +++ b/ui/dr-ui/src/library.rs @@ -810,6 +810,72 @@ pub fn catalog_path(server: &str, user_id: &str) -> PathBuf { .join("catalog.sqlite") } +/// TRACES: FR-NC-10 | NFR-R1 +/// Move an account's data out of the cache directory it used to live in. +/// +/// Called once at startup, before anything opens a store. The durable +/// location changed when `catalog_path` stopped falling through to +/// `temp_dir()` on Android, and without this the app would find no catalog, +/// rescan a library of tens of thousands of images over the network, and +/// re-fetch every thumbnail — while the old copy sat in a directory the +/// system was free to delete. +/// +/// Worse than the cost: `sidecars/` and `outbox/` hold work that exists +/// nowhere else. Abandoning them would discard offline ratings and edits that +/// had not yet synced, silently, as an upgrade. +/// +/// A rename, not a copy: both directories are inside the app's own data on +/// one filesystem, so it is atomic and cannot half-finish. If the destination +/// already exists this does nothing — the migration has run, or this is a +/// fresh install, and in neither case may it overwrite live data. +pub fn migrate_legacy_cache_data(server: &str, user_id: &str) { + // Only meaningful where the old fallback and the new one differ, which is + // exactly the platform that had the problem. On a desktop with XDG set, + // both resolve to the same place and this returns immediately. + let legacy_base = std::env::temp_dir(); + let Some(current) = catalog_path(server, user_id) + .parent() + .map(|p| p.to_path_buf()) + else { + return; + }; + let Some(account) = current.file_name() else { + return; + }; + let legacy = legacy_base.join("darkroom").join(account); + + move_account_dir(&legacy, ¤t); +} + +/// The move itself, separated so it can be tested against ordinary +/// directories rather than the platform's idea of a cache. +fn move_account_dir(legacy: &std::path::Path, current: &std::path::Path) { + if legacy == current || !legacy.is_dir() || current.exists() { + return; + } + if let Some(parent) = current.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + log::warn!("preparing {}: {e}", parent.display()); + return; + } + } + match std::fs::rename(legacy, current) { + Ok(()) => log::info!( + "moved library data out of the cache: {} -> {}", + legacy.display(), + current.display() + ), + // Reported rather than fatal: a failed move leaves the old copy where + // it was and costs a rescan, which is recoverable. Stopping the app + // over it would not be. + Err(e) => log::warn!( + "could not move {} to {}: {e}", + legacy.display(), + current.display() + ), + } +} + /// Run a scan on a worker thread, writing results into the catalog. /// /// Returns the receiver the UI drains. The worker owns its own tokio runtime @@ -2741,6 +2807,56 @@ mod tests { assert_ne!(a, c); } + #[test] + fn a_legacy_cache_directory_is_moved_rather_than_abandoned() { + // The upgrade hazard: `sidecars/` and `outbox/` hold work that exists + // nowhere else, so leaving them behind in a directory the system may + // empty would discard unsynced ratings and edits as a side effect of + // installing a new build. + let root = std::env::temp_dir().join(format!("dr-migrate-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let legacy = root.join("darkroom").join("cloud-example-duncan"); + std::fs::create_dir_all(legacy.join("sidecars")).unwrap(); + std::fs::write(legacy.join("catalog.sqlite"), b"catalog").unwrap(); + std::fs::write(legacy.join("sidecars").join("a.drsc"), b"an unsynced edit").unwrap(); + + let current = root.join("new").join("cloud-example-duncan"); + move_account_dir(&legacy, ¤t); + + assert!(!legacy.exists(), "the old copy must not be left behind"); + assert_eq!( + std::fs::read(current.join("catalog.sqlite")).unwrap(), + b"catalog" + ); + assert_eq!( + std::fs::read(current.join("sidecars").join("a.drsc")).unwrap(), + b"an unsynced edit", + "an unsynced edit must survive the move" + ); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn a_migration_never_overwrites_live_data() { + // Running twice, or a fresh install that already has a catalog. The + // destination wins: it is the one the application is using. + let root = std::env::temp_dir().join(format!("dr-migrate2-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let legacy = root.join("old").join("acct"); + let current = root.join("new").join("acct"); + std::fs::create_dir_all(&legacy).unwrap(); + std::fs::create_dir_all(¤t).unwrap(); + std::fs::write(legacy.join("catalog.sqlite"), b"stale").unwrap(); + std::fs::write(current.join("catalog.sqlite"), b"live").unwrap(); + + move_account_dir(&legacy, ¤t); + assert_eq!( + std::fs::read(current.join("catalog.sqlite")).unwrap(), + b"live" + ); + let _ = std::fs::remove_dir_all(&root); + } + #[test] fn durable_data_never_lands_in_a_cache_directory() { // The fault this guards against is silent and total: on Android the