From 75fd5619ca12c3fc9d0904d1fd4410e28a921dbf Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 22:19:38 +0200 Subject: [PATCH] Refuse a scan whose root has gone, instead of reporting it empty FR-PLAT-AND-2, and a silent failure on both platforms. `dr_sync::scan` stepped over a NotFound or PermissionDenied the way it does for a child that vanished mid-walk -- correct for a child, wrong for the root, where it ended the walk, returned Ok with nothing in it, and reported a successful scan of a library that was no longer there. A lost root is now its own error. The images under it are marked Availability::Offline per FR-CAT-9 and no catalog row is deleted; `library::persist` clears the mark per file as each one is listed again, so a root that comes back needs no repair step. Partly satisfied rather than closed, and the gap is worth stating. The recovery half is real and reachable on Android today, because `map_status` turns Nextcloud's 403 and 404 into it and Nextcloud is how a phone actually gets a library in this build. The causes the requirement names -- revocation, reinstall, a removed card -- are properties of a persisted tree permission, and there is none: SAF does not exist here, `SourceRef::Document` is constructed only in test modules, and `LocalStorage` rejects the variant outright. When SAF lands it becomes a third producer of this error and nothing above it changes, which is why the discovery belongs in the connector. Co-Authored-By: Claude Opus 5 (1M context) --- core/dr-catalog/src/lib.rs | 2 +- core/dr-catalog/src/walk.rs | 64 ++++++++++++++- core/dr-sync-folder/src/lib.rs | 25 ++++-- core/dr-sync-folder/src/tests.rs | 17 +++- core/dr-sync/src/error.rs | 70 ++++++++++++++-- core/dr-sync/src/scan.rs | 134 +++++++++++++++++++++++++++++++ ui/dr-ui/src/library.rs | 106 ++++++++++++++++++++++-- ui/dr-ui/src/library_ui.rs | 98 +++++++++++++++++++--- 8 files changed, 478 insertions(+), 38 deletions(-) diff --git a/core/dr-catalog/src/lib.rs b/core/dr-catalog/src/lib.rs index c99c94c..41c124c 100644 --- a/core/dr-catalog/src/lib.rs +++ b/core/dr-catalog/src/lib.rs @@ -63,7 +63,7 @@ pub use query::{Query, Sort}; pub use rating::{Judgement, MAX_RATING}; pub use scan::{DirAction, DirState, EntryAction, ScanOutcome}; pub use trash::{TrashedImage, TRASH_DIR}; -pub use walk::{ensure_root, scan_root, RootKind, ScanProgress, ScanReport}; +pub use walk::{ensure_root, mark_root_offline, scan_root, RootKind, ScanProgress, ScanReport}; /// One row of the library grid. /// diff --git a/core/dr-catalog/src/walk.rs b/core/dr-catalog/src/walk.rs index 11246d3..7d935a1 100644 --- a/core/dr-catalog/src/walk.rs +++ b/core/dr-catalog/src/walk.rs @@ -432,7 +432,7 @@ fn bump_generation(conn: &Connection, root: RootId, now: i64) -> Result Result Result<(), CatalogError> { +/// +/// # Why the ETag goes with the mtime +/// +/// The three columns are the same fact told by three kinds of storage: a local +/// directory proves it is unchanged with its mtime and entry count, and a +/// remote one proves it with a propagating ETag (ARCH §6.6). Clearing two of +/// them and leaving the third would disarm the re-listing on exactly the +/// libraries this is most likely to be called for — a remote scan prunes on +/// the ETag alone, so a root that came back would be walked, found unchanged +/// at every level, pruned whole, and left with every row still marked offline +/// and nothing that would ever clear the mark. +/// +/// # Public, because losing a root is not only the local walk's business +/// +/// This began as the private end of [`scan_root`]'s root-failure branches, +/// which is the only route a library reached through [`Storage`] can take. +/// The application does not currently take that route at all: it opens +/// libraries through `dr-sync`'s connectors, so the discovery happens in a +/// crate that cannot see this one's internals, and the correct response is +/// identical (FR-PLAT-AND-2). Exported rather than reimplemented beside the +/// caller that found out — a second copy would be a second thing to remember +/// when the ETag rule below changes. +/// +/// [`Storage`]: dr_plat::Storage +pub fn mark_root_offline(conn: &Connection, root: RootId) -> Result<(), CatalogError> { let root_id = root.0 as i64; conn.execute( "UPDATE images SET availability = ?1 WHERE root_id = ?2 AND availability != ?1", rusqlite::params![availability_code(Availability::Offline), root_id], )?; conn.execute( - "UPDATE folders SET mtime = NULL, entry_count = NULL WHERE root_id = ?1", + "UPDATE folders SET mtime = NULL, entry_count = NULL, etag = NULL WHERE root_id = ?1", [root_id], )?; Ok(()) @@ -995,6 +1019,40 @@ mod tests { ); } + /// TRACES: FR-PLAT-AND-2 | FR-CAT-9 + #[test] + fn marking_a_root_offline_forgets_the_remote_validator_too() { + // The half of the marking that only a remote library can notice, and + // the reason it has to be here rather than beside the connector: a + // remote scan prunes on the propagating ETag alone (ARCH §6.6). Clear + // the local mtime and leave the ETag standing and a library that came + // back would be walked, found unchanged at every level, pruned whole, + // and left with every row still marked offline — with nothing that + // would ever clear the mark, because clearing it is something only a + // listing can do. + // + // Written directly because this module never writes an ETag; it is + // `ui/dr-ui/src/library.rs`'s scan that does, against the same table. + let lib = Library::new("etag-forgotten"); + lib.file("2026/IMG.CR3", b"raw"); + lib.scan(); + lib.conn() + .execute( + "UPDATE folders SET etag = 'e1' WHERE root_id = ?1", + [lib.root.0 as i64], + ) + .expect("etag"); + assert!(lib.count("SELECT COUNT(*) FROM folders WHERE etag IS NOT NULL") > 0); + + mark_root_offline(lib.conn(), lib.root).expect("mark"); + + assert_eq!( + lib.count("SELECT COUNT(*) FROM folders WHERE etag IS NOT NULL"), + 0, + "an unreachable library must be re-listed, not pruned as unchanged" + ); + } + #[test] fn a_root_that_comes_back_is_available_again() { // The other half: a drive plugged back in must return the library to diff --git a/core/dr-sync-folder/src/lib.rs b/core/dr-sync-folder/src/lib.rs index 69f0d22..a4df0ba 100644 --- a/core/dr-sync-folder/src/lib.rs +++ b/core/dr-sync-folder/src/lib.rs @@ -216,10 +216,17 @@ impl std::fmt::Debug for FolderBackend { impl FolderBackend { /// Open the folder at `root`. /// - /// The directory must exist now. It may stop existing later — a drive - /// unplugged, a mount dropped — and that surfaces per-operation as - /// [`RemoteError::Network`], which is what puts the app into offline mode - /// and leaves the catalog readable, exactly as a dead server does. + /// The directory must exist now, and not existing is + /// [`RemoteError::RootUnavailable`] — the library folder could not be + /// opened, which is the whole of what this knows. A drive unplugged + /// between sessions and a path typed wrongly at setup are the same + /// observation from here, and both are answered the same way: keep the + /// catalog, say which folder, and offer it again (FR-PLAT-AND-2). + /// + /// A mount dropped *during* a session surfaces per-operation as + /// [`RemoteError::Network`] instead, which is what puts the app into + /// offline mode and leaves the catalog readable, exactly as a dead server + /// does. pub fn new(root: impl Into) -> Result { Self::with_vfs(root, Arc::new(NoVfs)) } @@ -232,7 +239,15 @@ impl FolderBackend { pub fn with_vfs(root: impl Into, vfs: Arc) -> Result { let root = root.into(); if !root.is_dir() { - return Err(RemoteError::Configuration(format!( + // TRACES: FR-PLAT-AND-2 + // Not `Configuration`, which is where this lived while there was + // nothing better. The distinction that matters is not "was the + // account written wrongly" — which nothing here can know — but + // "can this library be opened", and a caller that knows the + // library was working yesterday can act on the second answer: + // mark what it holds as offline rather than deleting it, and ask + // for the folder again (FR-CAT-9). + return Err(RemoteError::RootUnavailable(format!( "{} is not a folder", root.display() ))); diff --git a/core/dr-sync-folder/src/tests.rs b/core/dr-sync-folder/src/tests.rs index 606cdf1..3ecea0b 100644 --- a/core/dr-sync-folder/src/tests.rs +++ b/core/dr-sync-folder/src/tests.rs @@ -48,13 +48,22 @@ fn names(entries: &[RemoteEntry]) -> Vec { // --- opening -------------------------------------------------------------- +/// TRACES: FR-PLAT-AND-2 #[test] -fn a_missing_folder_is_a_configuration_error_not_a_network_one() { - // It must not put the app into offline mode: nothing was unreachable, the - // account names somewhere that is not a folder. +fn a_missing_folder_is_an_unavailable_root_not_a_network_failure() { + // Still not offline mode — nothing was unreachable over a wire, and + // reporting it as a network failure would tell the user to wait for a + // connection that is working. + // + // `RootUnavailable` rather than `Configuration`, because the caller that + // has to act on this is the one whose library worked yesterday: an + // ejected card is indistinguishable from a mistyped path here, and only + // the first of those has a catalog full of ratings to protect. let err = FolderBackend::new("/definitely/not/here").unwrap_err(); - assert!(matches!(err, RemoteError::Configuration(_)), "{err:?}"); + assert!(matches!(err, RemoteError::RootUnavailable(_)), "{err:?}"); + assert!(err.indicates_lost_root()); assert!(!err.indicates_offline()); + assert!(err.to_string().contains("/definitely/not/here"), "{err}"); } // --- listing -------------------------------------------------------------- diff --git a/core/dr-sync/src/error.rs b/core/dr-sync/src/error.rs index 0f966bf..de57c94 100644 --- a/core/dr-sync/src/error.rs +++ b/core/dr-sync/src/error.rs @@ -61,14 +61,18 @@ pub enum RemoteError { /// connector for. /// /// **Not a network failure and not an auth failure**, which is why it is - /// its own variant. A folder library whose directory has been unmounted, - /// or an account naming a backend a cut-down build was not compiled with, - /// produces a request that never leaves the process — reporting either as - /// `Network` would put the app into offline mode and tell the user their - /// connection is down, and reporting them as `AuthFailed` would send them - /// to re-enter a credential that is fine. The message names what is wrong - /// with the configuration, because that is the only thing that will fix - /// it. + /// its own variant. An account naming a backend a cut-down build was not + /// compiled with, or a path that would leave the library folder, produces + /// a request that never leaves the process — reporting either as `Network` + /// would put the app into offline mode and tell the user their connection + /// is down, and reporting them as `AuthFailed` would send them to re-enter + /// a credential that is fine. The message names what is wrong with the + /// configuration, because that is the only thing that will fix it. + /// + /// A folder library whose directory is not there was once reported here + /// too, and is now [`RootUnavailable`](Self::RootUnavailable): it is not + /// something wrong with the configuration, it is the library being gone, + /// and only the second of those has a catalog to protect. #[error("account misconfigured: {0}")] Configuration(String), @@ -105,6 +109,43 @@ pub enum RemoteError { #[error("operation cancelled")] Cancelled, + + /// TRACES: FR-PLAT-AND-2 | FR-CAT-9 + /// The library root itself could not be opened. + /// + /// **The one failure that is about the library rather than about a file in + /// it**, and it is a separate variant because every other classification + /// of it is wrong in a way that costs the user something: + /// + /// - As [`NotFound`](Self::NotFound) it is indistinguishable from a folder + /// deleted between listing its parent and reaching it, which the walk + /// correctly steps over — so a whole library going away is reported as a + /// successful scan that found nothing. + /// - As [`PermissionDenied`](Self::PermissionDenied) it inherits a message + /// about Nextcloud share permissions and sidecar writes, which is + /// accurate for the case it was written for and nonsense for a tree + /// grant the user revoked in system settings. + /// - As [`Network`](Self::Network) it would claim the connection is down, + /// which is a promise that waiting will fix it. + /// + /// Today this is a Nextcloud root that answers 404 or 403 — deleted, or a + /// share withdrawn — or a folder library whose directory is not there. It + /// is also, exactly, the shape a revoked Android tree permission will have + /// when the Storage Access Framework connector FR-PLAT-AND-1 asks for + /// exists: the tree URI still stored, the permission behind it gone, every + /// read failing at the root and nowhere else. **That connector is not + /// built**, so no SAF grant can be lost yet; what this variant does is put + /// the recovery FR-PLAT-AND-2 requires in the one place all three causes + /// pass through, so the third needs no new handling above it. + /// + /// The response is the same for all of them and is the point of the + /// variant: mark what the catalog holds as offline, keep every row, and + /// say which library and why (FR-CAT-9). + /// + /// The string is the underlying failure, not a rewrite of it. What the + /// user is told is composed where the library's name is known. + #[error("the library folder could not be opened: {0}")] + RootUnavailable(String), } impl RemoteError { @@ -150,6 +191,19 @@ impl RemoteError { pub fn indicates_offline(&self) -> bool { matches!(self, RemoteError::Network(_)) } + + /// TRACES: FR-PLAT-AND-2 + /// Whether the *library* is gone, as opposed to the server or one file. + /// + /// Kept beside [`Self::indicates_offline`] because the two answer the same + /// shape of question and must not be confused. Both put the app into a + /// degraded mode that keeps working from the catalog, but they differ in + /// what the user is told and in what would end it: an offline library + /// comes back when the network does, and an unavailable root comes back + /// only when someone grants access again. + pub fn indicates_lost_root(&self) -> bool { + matches!(self, RemoteError::RootUnavailable(_)) + } } #[cfg(test)] diff --git a/core/dr-sync/src/scan.rs b/core/dr-sync/src/scan.rs index 6f723cd..6e4bad5 100644 --- a/core/dr-sync/src/scan.rs +++ b/core/dr-sync/src/scan.rs @@ -171,6 +171,37 @@ where let entries = match backend.list(&dir, None).await { Ok(e) => e, + + // TRACES: FR-PLAT-AND-2 | FR-CAT-9 + // The root is the one directory the walk may not step over, and + // `depth == 0` is the only place it can be — nothing is ever + // pushed at that depth but the root itself. + // + // Below, a directory that has gone is a directory that went away + // between its parent being listed and it being reached, and + // continuing is right. At the root the identical error means the + // *library* is gone, and continuing is catastrophic in a way that + // is completely silent: the walk ends, the scan succeeds having + // found nothing, and the app reports a healthy library with no new + // images while every path in the catalog now points nowhere. + // + // Refused rather than reclassified. Only these two causes are — + // a `Network` failure at the root is still a network failure, and + // must stay one or an unplugged network cable would present itself + // as a revoked permission and offline mode would never engage. + Err(RemoteError::NotFound(_)) if depth == 0 => { + return Err(RemoteError::RootUnavailable(format!( + "{root} is no longer there" + ))); + } + Err(RemoteError::PermissionDenied) if depth == 0 => { + // Deliberately not the variant's own message, which describes + // a Nextcloud share that refuses to *update* a sidecar. At the + // root nothing has been read at all. + return Err(RemoteError::RootUnavailable(format!( + "{root} can no longer be read" + ))); + } Err(RemoteError::NotFound(_)) => { // Deleted between listing its parent and reaching it. log::debug!("scan: {dir} vanished during the walk"); @@ -239,6 +270,20 @@ mod tests { caps: Capabilities, lists: RefCell, probes: RefCell, + /// Directories whose listing fails, and how. + /// + /// An absent directory is not enough to model this: the fake answers + /// an unknown path with an empty listing, which is exactly the shape + /// the walk must *not* confuse with a library that has gone away. + deny: HashMap, + } + + /// The two ways a real backend refuses a directory that is still named in + /// the catalog: it is not there, or it may not be read. + #[derive(Clone, Copy)] + enum Deny { + Missing, + Forbidden, } // The fake is single-threaded; tests never share it across threads. @@ -307,8 +352,15 @@ mod tests { }, lists: RefCell::new(0), probes: RefCell::new(0), + deny: HashMap::new(), } } + + /// Make one directory refuse to be listed. + fn denying(mut self, path: &str, how: Deny) -> Self { + self.deny.insert(path.to_string(), how); + self + } } #[async_trait] @@ -325,6 +377,11 @@ mod tests { _since: Option<&Validator>, ) -> Result, RemoteError> { *self.lists.borrow_mut() += 1; + match self.deny.get(dir.as_str()) { + Some(Deny::Missing) => return Err(RemoteError::NotFound(dir.to_string())), + Some(Deny::Forbidden) => return Err(RemoteError::PermissionDenied), + None => {} + } Ok(self.tree.get(dir.as_str()).cloned().unwrap_or_default()) } async fn dir_validator(&self, dir: &RemotePath) -> Result { @@ -404,6 +461,83 @@ mod tests { assert_eq!(r.progress.directories_listed, 3); } + /// TRACES: FR-PLAT-AND-2 | FR-CAT-9 + #[tokio::test] + async fn a_root_that_is_gone_is_a_failure_and_not_an_empty_library() { + // The silent one. A vanished directory below the root is stepped over, + // and before this the root was stepped over on the same terms — which + // ended the walk immediately, returned `Ok` with nothing in it, and + // let the app report a successful scan of a library that no longer + // exists. Nothing in that path is ever told the library went away, so + // nothing marks it offline and nothing tells the user. + let b = + FakeBackend::sample(ChangeDetection::PropagatingEtags).denying("Photos", Deny::Missing); + let e = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .expect_err("a library that is not there is not a library with no photographs"); + + assert!(e.indicates_lost_root(), "got {e:?}"); + assert!(!e.indicates_offline(), "waiting will not bring this back"); + assert!(e.to_string().contains("Photos"), "names the library: {e}"); + } + + /// TRACES: FR-PLAT-AND-2 + #[tokio::test] + async fn a_root_that_may_not_be_read_reports_the_root_and_not_the_share_advice() { + // A Nextcloud share withdrawn, a directory the process may no longer + // read — and the shape a revoked Android tree grant will have when one + // can be held at all. Reported as plain `PermissionDenied` it would + // have carried that variant's message, which is several lines about a + // Nextcloud share refusing to *update* an existing sidecar: advice for + // a case where reads work, offered to a user whose reads have stopped + // entirely. + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags) + .denying("Photos", Deny::Forbidden); + let e = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .expect_err("a root that cannot be read is a failed scan"); + + assert!(e.indicates_lost_root(), "got {e:?}"); + assert!( + !e.to_string().contains("sidecar"), + "the share-permission advice does not belong here: {e}" + ); + } + + /// TRACES: FR-PLAT-AND-2 + #[tokio::test] + async fn a_folder_that_goes_away_below_the_root_is_still_stepped_over() { + // The other side of the split, and the reason the root is keyed on + // depth rather than on the error. A subfolder deleted between its + // parent being listed and it being reached is ordinary, and failing + // the scan over it would abandon every photograph beside it. + let b = FakeBackend::sample(ChangeDetection::PropagatingEtags) + .denying("Photos/2025", Deny::Missing); + let r = scan( + &b, + &RemotePath::new("Photos"), + &FormatFilter::all(), + &HashMap::new(), + |_| {}, + ) + .await + .expect("one folder going away is not the library going away"); + + assert_eq!(r.images.len(), 1, "2026 was still walked"); + } + /// A library with a trash folder holding a soft-deleted image. fn with_trash() -> FakeBackend { let mut b = FakeBackend::sample(ChangeDetection::PropagatingEtags); diff --git a/ui/dr-ui/src/library.rs b/ui/dr-ui/src/library.rs index 8bc230c..ffd7d94 100644 --- a/ui/dr-ui/src/library.rs +++ b/ui/dr-ui/src/library.rs @@ -80,7 +80,20 @@ pub enum ScanMessage { /// amount of string matching on the far side can reliably recover it. /// Without the flag a dead connection and a bad password produce the same /// banner, which sends the user to re-enter a credential that was fine. - Failed { message: String, offline: bool }, + /// + /// `lost_root` is the same idea one step further out, and it is carried + /// separately from `offline` rather than folded into it because the two + /// end differently. An offline library comes back when the network does, + /// with nothing asked of anyone; a library whose root cannot be opened + /// comes back only when someone restores access to it — a share put back + /// on the server, a drive plugged in, and in time a document tree granted + /// again once one can be (FR-PLAT-AND-2). Both show the same grid of what + /// is stored locally, and they must not offer the same explanation. + Failed { + message: String, + offline: bool, + lost_root: bool, + }, } /// One decoded thumbnail, ready for the grid. @@ -1146,6 +1159,7 @@ pub fn spawn_scan( let _ = tx.send(ScanMessage::Failed { message: e.message, offline: e.offline, + lost_root: e.lost_root, }); } }); @@ -1160,6 +1174,7 @@ pub fn spawn_scan( struct ScanFailure { message: String, offline: bool, + lost_root: bool, } impl ScanFailure { @@ -1169,6 +1184,7 @@ impl ScanFailure { Self { message: message.to_string(), offline: false, + lost_root: false, } } } @@ -1177,11 +1193,48 @@ impl From for ScanFailure { fn from(e: dr_sync::RemoteError) -> Self { Self { offline: e.indicates_offline(), + lost_root: e.indicates_lost_root(), message: e.to_string(), } } } +/// TRACES: FR-PLAT-AND-2 | FR-CAT-9 +/// Record that a library can no longer be opened, without losing it. +/// +/// Called on the worker, before the failure crosses the channel, because this +/// is where the catalog handle is — and because the marking must be durable +/// whether or not anyone is left to draw a banner. A process killed between +/// the failure and the next launch must still come back knowing what it could +/// not reach. +/// +/// Nothing is deleted. Every rating, every edit and every row stays exactly +/// where it was; what changes is that the images now say they are offline, so +/// the grid can show them as held-not-here rather than as ordinary +/// photographs whose thumbnails happen to be failing one at a time. +/// +/// A root with no row yet is the first scan of a library that has never +/// succeeded, and there is nothing to mark — the failure alone is the whole +/// story, and the launch screen is where it is told. +fn mark_library_offline(catalog: &Catalog, root: &str) { + let conn = catalog.connection(); + let root_id: Option = conn + .query_row( + "SELECT id FROM roots WHERE label = ?1 AND kind = 'remote'", + [root], + |r| r.get(0), + ) + .ok(); + let Some(root_id) = root_id else { + log::info!("library {root} has no catalog root yet; nothing to mark offline"); + return; + }; + match dr_catalog::mark_root_offline(conn, dr_types::RootId(root_id as u64)) { + Ok(()) => log::warn!("library {root} is unreachable; its images are marked offline"), + Err(e) => log::error!("could not mark {root} offline: {e}"), + } +} + fn run_scan( tx: &Sender, conn: Connection, @@ -1199,21 +1252,50 @@ fn run_scan( let rt = crate::net_runtime::build().map_err(ScanFailure::local)?; rt.block_on(async { - let backend = crate::remote::connect(&conn).map_err(ScanFailure::local)?; + // TRACES: FR-PLAT-AND-2 | FR-CAT-9 + // Classified rather than flattened to a local failure, because the + // removed-card case never gets as far as a request: the folder + // connector checks its root when it is constructed, so a library on an + // ejected card fails here and not in the walk. Reported as an ordinary + // error it left the grid showing a healthy library of images that + // could no longer be opened, one silent thumbnail failure at a time. + let backend = match crate::remote::connect(&conn) { + Ok(b) => b, + Err(e) => { + if e.indicates_lost_root() { + mark_library_offline(&catalog, &root); + } + return Err(e.into()); + } + }; // Stored folder ETags, so an unchanged subtree is skipped whole. On a // first run this is empty and the walk is complete; on every run after // it is what keeps cost proportional to what changed (ARCH §8.4). let known = load_folder_etags(&catalog, &root); - let result = dr_sync::scan(&*backend, &RemotePath::new(&root), &filter, &known, |p| { + let scanned = dr_sync::scan(&*backend, &RemotePath::new(&root), &filter, &known, |p| { let _ = tx.send(ScanMessage::Progress { directories: p.directories_listed, pruned: p.directories_pruned, images: p.images_found, }); }) - .await?; + .await; + + // TRACES: FR-PLAT-AND-2 | FR-CAT-9 + // Written before the failure is reported, not after: the banner is a + // consequence of the catalog state and not the other way round, and a + // process that dies between the two must come back knowing. + let result = match scanned { + Ok(r) => r, + Err(e) => { + if e.indicates_lost_root() { + mark_library_offline(&catalog, &root); + } + return Err(e.into()); + } + }; persist(&catalog, &root, &result).map_err(ScanFailure::local)?; @@ -1306,13 +1388,27 @@ fn persist( .ok() }); + // TRACES: FR-PLAT-AND-2 | FR-CAT-9 + // The `availability` arm is what ends an offline library, and it does + // it one photograph at a time. 3 is `Availability::Offline` and 0 is + // `MetadataOnly`, the same code this statement inserts new rows with — + // so a row that was marked offline when the root became unreachable is + // returned to exactly the state a fresh scan would have given it, and + // a row that was never marked is not touched at all. + // + // Conditional rather than a blanket reset for the same reason + // `dr_catalog::walk` restores per file rather than per root: the only + // thing that may clear "I could not reach this" is having reached it, + // and this statement runs precisely once per file the scan listed. tx.execute( "INSERT INTO images(root_id, folder_id, source_ref, format, file_size, availability, metadata_state, added_at) VALUES (?1, ?2, ?3, ?4, ?5, 0, 1, ?6) ON CONFLICT(root_id, source_ref) DO UPDATE SET file_size = excluded.file_size, - folder_id = excluded.folder_id", + folder_id = excluded.folder_id, + availability = CASE WHEN images.availability = 3 + THEN 0 ELSE images.availability END", rusqlite::params![ root_id, folder_id, diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index ae6844d..dfef1f9 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -270,6 +270,22 @@ pub struct LibraryController { /// judgement, and carrying the old one over would report a server down /// that was never contacted. reachability: RefCell, + /// TRACES: FR-PLAT-AND-2 | FR-CAT-9 + /// Why the library folder itself could not be opened, if it could not. + /// + /// Beside [`Self::reachability`] rather than inside it, because + /// `dr_sync::Reachability` models *the server*, and it is deliberately + /// unmoved by a refusal — a forbidden file must not report the network as + /// down (see its own tests). A revoked tree grant is a refusal, so folding + /// it in would either break that rule or need an exception carved through + /// it. + /// + /// Set only by a scan that failed at the root, and cleared only by one + /// that succeeded. Both states drive the same banner as being offline + /// does, because what the user can do is the same — carry on with what is + /// stored on the device — but the sentence under it is different, and so + /// is what will end it. + root_lost: RefCell>, /// TRACES: FR-NC-6a /// Drains the pin downloader. Held so a second pin replaces the timer /// rather than leaving two draining the same finished channel. @@ -372,6 +388,7 @@ impl LibraryController { sidecar_timer: RefCell::new(None), generation: std::cell::Cell::new(0), reachability: RefCell::new(dr_sync::Reachability::new()), + root_lost: RefCell::new(None), outbox_timer: RefCell::new(None), outbox_maybe_dirty: std::cell::Cell::new(true), geometry_timer: RefCell::new(None), @@ -443,10 +460,15 @@ impl LibraryController { } } - /// TRACES: FR-CAT-9 - /// Whether the app currently believes the server is unreachable. + /// TRACES: FR-CAT-9 | FR-PLAT-AND-2 + /// Whether the library cannot be reached, for either of the two reasons. + /// + /// One answer rather than two because every caller asks it for the same + /// purpose: to decide whether starting a transfer is worth attempting. + /// A revoked grant fails that question exactly as a dead network does, and + /// a sync started against it would spend its retries proving it. pub fn is_offline(&self) -> bool { - self.reachability.borrow().is_offline() + self.reachability.borrow().is_offline() || self.root_lost.borrow().is_some() } /// Whether the grid is narrowed to locally-stored originals. @@ -941,6 +963,17 @@ fn drain_scan( { log::info!("back online"); } + // TRACES: FR-PLAT-AND-2 + // And it is the only evidence that clears a lost root, + // for the same reason: the walk began by listing the + // root, so a scan that finished is a root that opened. + // The rows it marked offline are restored one at a + // time by `library::persist`, as each file is listed + // again — this only stops the banner claiming what is + // no longer true. + if ctl.root_lost.borrow_mut().take().is_some() { + log::info!("library folder is readable again"); + } refresh_offline(&w, ctl); // An incremental rescan lists almost nothing, so @@ -995,7 +1028,11 @@ fn drain_scan( stop(&ctl.scan_timer); return; } - ScanMessage::Failed { message, offline } => { + ScanMessage::Failed { + message, + offline, + lost_root, + } => { log::warn!("scan failed: {message}"); w.set_library_scanning(false); // Recorded as a failure even where it is only the @@ -1004,7 +1041,26 @@ fn drain_scan( // stopped because of it. job.fail(message.clone()); - if offline { + if lost_root { + // TRACES: FR-PLAT-AND-2 | FR-CAT-9 + // The library folder itself could not be opened — + // a share withdrawn, an unplugged drive, and in + // time a revoked document-tree grant. The worker + // has already marked every row under this root + // offline and deleted none of them; this is the + // half the user sees. + // + // Tested first because it is also true that the + // library is unreachable, and the generic answer + // would be reached first and be less useful. + *ctl.root_lost.borrow_mut() = Some(message); + refresh_offline(&w, ctl); + // Same reason as the offline arm below: without + // this a launch that began with a revoked grant + // shows an empty grid, which is the one impression + // this whole path exists to avoid. + open_catalog_for_offline(&w, ctl, &catalog_path, &coll_ctl); + } else if offline { // Not an error state. The catalog from the last // successful scan is still on disk and still // accurate for everything already indexed, so the @@ -1585,17 +1641,35 @@ fn scope_is_pinned(catalog: &Catalog, images: &[dr_types::ImageId]) -> bool { /// which is what keeps it testable without a display server. fn refresh_offline(window: &AppWindow, ctl: &Rc) { let reach = ctl.reachability.borrow(); - let offline = reach.is_offline(); + // TRACES: FR-PLAT-AND-2 + // A lost root wins over a dead network, and does so even when both are + // true — which is the ordinary case, since the scan that discovered the + // grant was gone was also the last request the app made. Reported the + // other way round the user is told to wait for a connection that is + // working, and the thing that would actually fix it is never mentioned. + let lost = ctl.root_lost.borrow(); + let offline = reach.is_offline() || lost.is_some(); window.set_library_offline(offline); - window.set_library_offline_reason(reach.reason().unwrap_or_default().into()); + window.set_library_offline_reason(match lost.as_deref() { + Some(why) => why.into(), + None => reach.reason().unwrap_or_default().into(), + }); window.set_library_offline_since( - reach - .offline_for(std::time::Instant::now()) - .map(describe_duration) - .unwrap_or_default() - .into(), + // A duration is what a network outage has and a revoked permission + // does not: "for 4 minutes" invites waiting, and waiting is precisely + // what will not help here. + if lost.is_some() { + slint::SharedString::default() + } else { + reach + .offline_for(std::time::Instant::now()) + .map(describe_duration) + .unwrap_or_default() + .into() + }, ); + drop(lost); // A stale scan error under an offline banner reports one problem twice. if offline {