Merge: answer Android's memory warnings, and stop reporting a lost root as an empty library
FR-PLAT-AND-5 in full, FR-PLAT-AND-2 in part -- the recovery is built and live for Nextcloud roots, the SAF cause it names does not exist yet. FR-PLAT-AND-4 and FR-PLAT-AND-6 are not here, both blocked behind the same gap: assemble-apk.sh compiles no Java, so the APK cannot carry a Service or a FileProvider. The container has JDK 17 and build-tools 36; the build step is what is missing. Verified: fmt, clippy --workspace --all-targets -D warnings, and 1043 tests across dr-catalog, dr-sync, dr-sync-folder, dr-sync-nextcloud, dr-plat and dr-ui. The aarch64 target was checked before the branch was finished but not after; no device was available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -65,7 +65,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.
|
||||
///
|
||||
|
||||
@@ -432,7 +432,7 @@ fn bump_generation(conn: &Connection, root: RootId, now: i64) -> Result<i64, Cat
|
||||
Ok(next)
|
||||
}
|
||||
|
||||
/// TRACES: FR-CAT-9
|
||||
/// TRACES: FR-CAT-9 | FR-PLAT-AND-2
|
||||
/// Mark every image under a root as unreachable.
|
||||
///
|
||||
/// The other half of FR-CAT-9's distinction: a source *proven absent* may leave
|
||||
@@ -445,14 +445,38 @@ fn bump_generation(conn: &Connection, root: RootId, now: i64) -> Result<i64, Cat
|
||||
/// the root now claims something about the files that is no longer known to be
|
||||
/// true, and only reading the directories again can settle it. Pruning would
|
||||
/// skip them all and leave a plugged-in library showing as offline forever.
|
||||
fn mark_root_offline(conn: &Connection, root: RootId) -> 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
|
||||
|
||||
@@ -1026,6 +1026,44 @@ impl AdjustPass {
|
||||
h
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5 | NFR-RES-1
|
||||
/// Give back every allocation this pass is holding only to be fast again.
|
||||
///
|
||||
/// What goes, and why each is safe to lose:
|
||||
///
|
||||
/// - **The compiled pipelines**, here and in the detail stage. A pure
|
||||
/// lookup keyed by structure hash with a compile-on-miss behind it, and
|
||||
/// unbounded until now — nothing ever removed an entry, so a session
|
||||
/// that visited enough distinct edit structures accumulated shader
|
||||
/// objects for the life of the process.
|
||||
/// - **The detail intermediates**, which are viewport-sized `Rgba16Float`
|
||||
/// and, as `detail.rs` says of them, grow but never shrink.
|
||||
/// - **The two output textures.** Dropping these does not take the picture
|
||||
/// off the screen: whatever was handed to the compositor holds its own
|
||||
/// reference to the `wgpu::Texture`, so releasing ours only means the
|
||||
/// *next* render allocates rather than reuses. `ensure_target` already
|
||||
/// treats an empty slot as "allocate", because that is the state it
|
||||
/// starts in.
|
||||
///
|
||||
/// **`colour_key` must be cleared with them, and this is the part that
|
||||
/// would bite.** The key is the promise that slot 0 of the detail pool
|
||||
/// still holds the fused colour result, and it is what lets a sharpening
|
||||
/// slider skip the colour chain (FR-DEV-3d). Freeing the pool while the
|
||||
/// promise stood would make the next detail-only render sample a
|
||||
/// just-allocated texture with nothing in it — a silently wrong frame, not
|
||||
/// a failure, and one that would only appear on a device under memory
|
||||
/// pressure.
|
||||
///
|
||||
/// What deliberately stays: the demosaiced source is not this pass's to
|
||||
/// drop, the film tables are set once by a caller that will not be asked
|
||||
/// again, and the bind group layouts are bytes rather than megabytes.
|
||||
pub fn release_caches(&mut self) {
|
||||
self.cache.clear();
|
||||
self.detail.release_caches();
|
||||
self.targets = [None, None];
|
||||
self.colour_key = None;
|
||||
}
|
||||
|
||||
/// How many distinct pipelines are compiled. Exposed for tests asserting
|
||||
/// that slider movement does not recompile.
|
||||
pub fn cached_pipelines(&self) -> usize {
|
||||
|
||||
@@ -157,6 +157,24 @@ impl Intermediates {
|
||||
self.allocations += 1;
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5
|
||||
/// Drop the pool, leaving it as [`Intermediates::new`] left it.
|
||||
///
|
||||
/// The size is reset along with the slots, not merely because it is tidy:
|
||||
/// [`Self::ensure`] only refills when the count is short *or* the size
|
||||
/// differs, so a pool cleared while still claiming its old dimensions is
|
||||
/// indistinguishable from one that never held anything — which is fine
|
||||
/// here, and would stop being fine the moment `ensure` grew a fast path
|
||||
/// that trusted the stored size. `allocations` deliberately keeps
|
||||
/// counting: it exists so a test can see textures being made, and a
|
||||
/// counter reset on eviction would hide a reallocation storm rather than
|
||||
/// report one.
|
||||
fn release(&mut self) {
|
||||
self.slots.clear();
|
||||
self.width = 0;
|
||||
self.height = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/// Runs the detail stage.
|
||||
@@ -526,6 +544,19 @@ impl DetailRunner {
|
||||
self.cache.len()
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5
|
||||
/// Give back everything this stage is only holding to be fast.
|
||||
///
|
||||
/// Both pools and the pipeline cache. Nothing here is state: a pool slot
|
||||
/// is re-created by the next [`Intermediates::ensure`] and a pipeline by
|
||||
/// the next compile-on-miss, so the only cost of this call is the work of
|
||||
/// doing both again.
|
||||
pub(crate) fn release_caches(&mut self) {
|
||||
self.cache.clear();
|
||||
self.pool.release();
|
||||
self.reduced.release();
|
||||
}
|
||||
|
||||
/// How many intermediate textures have been allocated since this pass was
|
||||
/// created. For tests — see [`crate::MaskPass::allocations`] for the
|
||||
/// regression this shape of counter exists to catch.
|
||||
|
||||
@@ -413,3 +413,56 @@ fn an_empty_chain_falls_through_to_the_ordinary_render() {
|
||||
assert_eq!(pass.detail_dispatches(), 0);
|
||||
assert_eq!(pass.detail_allocations(), 0);
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-5
|
||||
#[test]
|
||||
fn eviction_gives_the_pools_back_without_changing_a_pixel() {
|
||||
// The half of memory-pressure eviction that cannot be checked by looking
|
||||
// at a counter. `release_caches` frees the detail pool, and slot 0 of that
|
||||
// pool is where the fused colour result lives between frames — so the
|
||||
// render after an eviction has to notice that the promise recorded in
|
||||
// `colour_key` no longer holds and run the colour chain again.
|
||||
//
|
||||
// Leave the key standing and this test does not error: it draws. It draws
|
||||
// whatever a freshly-allocated texture happens to contain, which is the
|
||||
// failure worth building a test around, because on a device it would
|
||||
// appear only under memory pressure and only as a wrong-looking photograph.
|
||||
let Some(ctx) = ctx() else { return };
|
||||
const SIZE: u32 = 48;
|
||||
let source = step_edge(&ctx, SIZE);
|
||||
let mut pass = AdjustPass::new(&ctx);
|
||||
let mut graph = EditGraph::with_detail_probe();
|
||||
graph.set_param(PROBE, RADIUS, 0.05);
|
||||
|
||||
let before = render(&ctx, &mut pass, &graph, &source, SIZE);
|
||||
assert!(
|
||||
pass.cached_pipelines() > 0,
|
||||
"the colour pass compiled something"
|
||||
);
|
||||
assert!(
|
||||
pass.cached_detail_pipelines() > 0,
|
||||
"so did the detail stage"
|
||||
);
|
||||
let allocations = pass.detail_allocations();
|
||||
assert!(allocations > 0, "and the pool holds textures");
|
||||
|
||||
pass.release_caches();
|
||||
assert_eq!(pass.cached_pipelines(), 0);
|
||||
assert_eq!(pass.cached_detail_pipelines(), 0);
|
||||
|
||||
// The same edit at the same size. Nothing about the picture changed, so
|
||||
// nothing about the pixels may change either — only what it cost.
|
||||
let after = render(&ctx, &mut pass, &graph, &source, SIZE);
|
||||
assert_eq!(before.len(), after.len());
|
||||
for (i, (a, b)) in before.iter().zip(&after).enumerate() {
|
||||
assert!(
|
||||
a.abs_diff(*b) <= 1,
|
||||
"byte {i}: {a} before eviction, {b} after — the colour chain did \
|
||||
not re-run, so this frame is reading an empty intermediate"
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
pass.detail_allocations() > allocations,
|
||||
"the pool was rebuilt, which is the evidence it was really given back"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<PathBuf>) -> Result<Self, RemoteError> {
|
||||
Self::with_vfs(root, Arc::new(NoVfs))
|
||||
}
|
||||
@@ -232,7 +239,15 @@ impl FolderBackend {
|
||||
pub fn with_vfs(root: impl Into<PathBuf>, vfs: Arc<dyn Vfs>) -> Result<Self, RemoteError> {
|
||||
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()
|
||||
)));
|
||||
|
||||
@@ -48,13 +48,22 @@ fn names(entries: &[RemoteEntry]) -> Vec<String> {
|
||||
|
||||
// --- 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 --------------------------------------------------------------
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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<usize>,
|
||||
probes: RefCell<usize>,
|
||||
/// 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<String, Deny>,
|
||||
}
|
||||
|
||||
/// 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<Vec<RemoteEntry>, 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<Validator, RemoteError> {
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user