Keep offline work out of a directory Android empties
The catalog, the sidecar cache and the export outbox were all landing in the app's cache directory on Android, where the system deletes them without asking under storage pressure. `catalog_path` derived its base from `XDG_DATA_HOME` or `HOME`, and neither is set on Android, so it fell through to `temp_dir()` — which resolves there to `/data/user/0/<pkg>/cache`. Confirmed on the tablet: the app logs its catalog under `cache/darkroom/...`. What sits beside that catalog is not disposable. `sidecars/` is the commit point for every rating and edit made with no connection — the whole mechanism that makes offline culling safe — and `outbox/` holds exports the user has already been told succeeded. A day of sorting on a train, evicted by an OS housekeeping pass before it ever reached the server, is the worst failure this application can have, and it would leave no error and no trace. The fallback is now `SessionStore::data_dir()`, the persistent per-app directory the Android entry point establishes before any store opens — the same one credentials and sessions already use. Desktop is untouched: the XDG data location is still preferred, so nobody's catalog moves. Two tests. One asserts no durable path contains `/cache/` or `/tmp/`; the other pins the outbox to the catalog's parent, because three call sites derive their location that way and a change here moves all of them at once. Found while verifying that offline browsing works on the tablet with wifi disabled — which it does: the app cold-starts with no network, reports the scan failure without crashing, and serves its grid from the local store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+50
-1
@@ -782,10 +782,28 @@ pub fn catalog_path(server: &str, user_id: &str) -> PathBuf {
|
||||
.map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
|
||||
.collect();
|
||||
|
||||
// **Not the cache directory, and on Android that distinction is the whole
|
||||
// point.** Neither `XDG_DATA_HOME` nor `HOME` is set there, so this used
|
||||
// to fall through to `temp_dir()` — which Android resolves to the app's
|
||||
// *cache*, a directory the system deletes without asking under storage
|
||||
// pressure.
|
||||
//
|
||||
// What sits beside this catalog is not disposable. `sidecars/` is the
|
||||
// commit point for every rating and edit made offline (see
|
||||
// `sidecar_cache`), and `outbox/` holds exports the user has been told
|
||||
// succeeded. A day of culling on a train, evicted by the OS before it ever
|
||||
// reached the server, is the worst failure this application can have, and
|
||||
// it would be silent.
|
||||
//
|
||||
// `SessionStore::data_dir()` is the persistent per-app directory the
|
||||
// Android entry point establishes before anything opens a store. On a
|
||||
// desktop it is the XDG config directory, and the two lines below keep the
|
||||
// established XDG *data* location there rather than moving anyone's
|
||||
// catalog.
|
||||
let base = std::env::var_os("XDG_DATA_HOME")
|
||||
.map(PathBuf::from)
|
||||
.or_else(|| std::env::var_os("HOME").map(|h| PathBuf::from(h).join(".local/share")))
|
||||
.unwrap_or_else(std::env::temp_dir);
|
||||
.unwrap_or_else(dr_sync_nextcloud::session::SessionStore::data_dir);
|
||||
|
||||
base.join("darkroom")
|
||||
.join(format!("{slug}-{user_id}"))
|
||||
@@ -2723,6 +2741,37 @@ mod tests {
|
||||
assert_ne!(a, c);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn durable_data_never_lands_in_a_cache_directory() {
|
||||
// The fault this guards against is silent and total: on Android the
|
||||
// fallback used to be `temp_dir()`, which resolves to the app's cache
|
||||
// — a directory the system empties under storage pressure. Beside this
|
||||
// catalog sit `sidecars/`, the commit point for every offline rating
|
||||
// and edit, and `outbox/`, holding exports the user was told had
|
||||
// succeeded. Losing a day of culling to an OS housekeeping pass, with
|
||||
// no error and no trace, is the worst outcome this application has.
|
||||
let path = catalog_path("https://cloud.example", "duncan");
|
||||
let text = path.to_string_lossy().to_lowercase();
|
||||
assert!(
|
||||
!text.contains("/cache/") && !text.contains("/tmp/"),
|
||||
"the catalog and everything beside it must be durable, got {}",
|
||||
path.display()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_outbox_and_sidecars_sit_beside_the_catalog() {
|
||||
// Stated as a test because three separate call sites derive their
|
||||
// location by taking this path's parent, and a change here moves all
|
||||
// of them at once — including the two holding unsynced user work.
|
||||
let catalog = catalog_path("https://cloud.example", "duncan");
|
||||
let parent = catalog.parent().expect("a parent");
|
||||
assert_eq!(
|
||||
crate::export::outbox_dir("https://cloud.example", "duncan"),
|
||||
parent.join("outbox")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_path_is_filesystem_safe() {
|
||||
let p = catalog_path("https://cloud.example.com:8443/nc", "duncan");
|
||||
|
||||
Reference in New Issue
Block a user