From 4168d67cfa7aef5437d4a8457ce85a7b74f09632 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 10:36:26 +0200 Subject: [PATCH] Do not push a catalog over one we could not read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sync_catalog` is a read-modify-write over a file another device also writes: take theirs, merge, push the union. It was shaped if let Ok(bytes) = backend.get(&RemoteId::Path(target), None).await { which folds *every* failure into "there is no remote catalog" and carries straight on to the upload. On a placeholder library the snapshot in `.darkroom-derived/` is dehydrated like anything else, so the read failed every time and each sync pushed our catalog over theirs unmerged — taking the other device's collections and their members with it. The same shape as the sidecar bug, and the same fix: a read that fails for anything other than `NotFound` stops the upload and says why. An unreadable or unopenable snapshot stops it too — "will not parse" is not "is not there". This is what `NotFound` and `NotMaterialised` being separate errors is *for*: one means ours is the whole truth, the other means do not dare. Shard downloads go through the same fetch-on-demand read. They logged and skipped before, which on a library the client keeps dehydrated is every shard, every pass, and a peer's thumbnails and faces silently never arriving. And `put` over a placeholder now replaces it rather than refusing. Refusing was over-cautious of me: derived state lives inside the library folder, so a folder the client had dehydrated could never be written to again. An unconditional write replaces the whole file, so there is nothing in the stub to keep — content first, then the placeholder, since in a synced tree an absence is a deletion that propagates. `IfMatch` still refuses, because a stub's validator describes the stub; `IfAbsent` fails, because the file is there and only its content is not. --- core/dr-sync-folder/src/lib.rs | 51 +++++++++++++++++---- core/dr-sync-folder/src/tests.rs | 51 +++++++++++++++++++-- docs/storage.md | 30 ++++++++++-- docs/traceability.md | 6 +-- ui/dr-ui/src/derived_sync.rs | 78 +++++++++++++++++++++++++++++--- 5 files changed, 190 insertions(+), 26 deletions(-) diff --git a/core/dr-sync-folder/src/lib.rs b/core/dr-sync-folder/src/lib.rs index f69675f..69f0d22 100644 --- a/core/dr-sync-folder/src/lib.rs +++ b/core/dr-sync-folder/src/lib.rs @@ -588,15 +588,36 @@ impl RemoteBackend for FolderBackend { body: Vec, precond: Option, ) -> Result { - let (local, materialised) = self.locate(path)?; - if !materialised { - // Writing `a.drsc` while `a.drsc.nextcloud` sits beside it creates - // two files for one document and hands the sync client a conflict - // to resolve — in favour of whichever it sees last. The caller - // must materialise it and merge, which is what the typed error is - // for. - return Err(RemoteError::NotMaterialised(local.display().to_string())); - } + let (found, materialised) = self.locate(path)?; + // Where the content belongs, which is not where a placeholder for it + // sits — suffix-mode VFS gives the two different names. + let local = self.resolve(path)?; + + // A stub is still this file, so what to do about it depends entirely + // on what the caller is promising. + let replaces = if materialised { + None + } else { + match &precond { + // Nothing here can satisfy it: the validator on a placeholder + // describes the placeholder. The caller fetches the content + // and tries again, which is what the typed error asks for. + Some(Precondition::IfMatch(_)) => { + return Err(RemoteError::NotMaterialised(found.display().to_string())) + } + // Something *is* there — the file exists, only its content is + // elsewhere — so a create-if-absent must fail. + Some(Precondition::IfAbsent) => return Err(RemoteError::PreconditionFailed), + // An unconditional write replaces the whole file, so there is + // nothing in the stub worth reading and no reason to download + // it first. Refusing here instead was a mistake: derived state + // lives in the library folder and the client dehydrates it + // like anything else, so a refusal meant sync could never + // write to a folder it had been away from. + None => Some(found), + } + }; + blocking(move || { let what = local.display().to_string(); if let Some(parent) = local.parent() { @@ -658,6 +679,18 @@ impl RemoteBackend for FolderBackend { return Err(map_io(e, &what)); } + // The stub goes only once the content is safely in place. The + // other order risks leaving neither, and in a synced tree an + // absence is a deletion the client would propagate. + if let Some(stub) = replaces { + if let Err(e) = std::fs::remove_file(&stub) { + // The content landed, so the write succeeded; a leftover + // placeholder beside it is untidy rather than harmful, and + // the client reconciles the pair on its next pass. + log::warn!("removing placeholder {}: {e}", stub.display()); + } + } + let meta = std::fs::metadata(&local).map_err(|e| map_io(e, &what))?; Ok(validator_of(&meta)) }) diff --git a/core/dr-sync-folder/src/tests.rs b/core/dr-sync-folder/src/tests.rs index e45aa54..606cdf1 100644 --- a/core/dr-sync-folder/src/tests.rs +++ b/core/dr-sync-folder/src/tests.rs @@ -630,19 +630,60 @@ async fn reading_a_placeholder_is_distinguishable_from_a_missing_file() { } #[tokio::test] -async fn writing_over_a_placeholder_is_refused() { - // Writing `a.drsc` beside `a.drsc.stub` makes two files for one document - // and hands the sync client a conflict it resolves arbitrarily. +async fn an_unconditional_write_replaces_a_placeholder() { + // Derived state — shards, the catalog snapshot — lives in the library + // folder, so the client dehydrates it like anything else. Refusing here + // meant sync could never write to a folder it had been away from. The + // whole file is being replaced, so there is nothing in the stub to keep. let t = Tmp::new("vfs-write"); t.file("a.drsc.stub", &[0u8]); let b = with_stubs(&t); + b.put(&RemotePath::new("a.drsc"), b"".to_vec(), None) + .await + .unwrap(); + + assert_eq!(std::fs::read(t.0.join("a.drsc")).unwrap(), b""); + // And exactly one file for one document: a leftover stub beside it is a + // conflict the client would resolve in favour of whichever it saw last. + assert!(!t.0.join("a.drsc.stub").exists(), "placeholder left behind"); + assert_eq!( + names(&b.list(&RemotePath::root(), None).await.unwrap()), + vec!["a.drsc"] + ); +} + +#[tokio::test] +async fn a_conditional_write_over_a_placeholder_asks_for_the_content_first() { + // `IfMatch` guards a read-modify-write. A stub's validator describes the + // placeholder, not the document, so nothing here can satisfy it — and + // quietly writing anyway is how the other device's edits are lost. + let t = Tmp::new("vfs-write-cond"); + t.file("a.drsc.stub", &[0u8]); + let b = with_stubs(&t); + let e = b - .put(&RemotePath::new("a.drsc"), b"".to_vec(), None) + .put( + &RemotePath::new("a.drsc"), + b"".to_vec(), + Some(Precondition::IfMatch(Validator::new("whatever"))), + ) .await .unwrap_err(); assert!(matches!(e, RemoteError::NotMaterialised(_)), "{e:?}"); - assert!(!t.0.join("a.drsc").exists(), "no rival file created"); + assert!(!t.0.join("a.drsc").exists(), "nothing written"); + + // And a create-if-absent fails, because the file *is* there — only its + // content is elsewhere. + let e = b + .put( + &RemotePath::new("a.drsc"), + b"".to_vec(), + Some(Precondition::IfAbsent), + ) + .await + .unwrap_err(); + assert!(matches!(e, RemoteError::PreconditionFailed), "{e:?}"); } #[tokio::test] diff --git a/docs/storage.md b/docs/storage.md index a3e8342..7fd1f8a 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -418,7 +418,9 @@ free of any client's protocol. | `RemoteEntry::materialised` | `false` on a stub; the catalog maps it to `Availability::Offline` | | `RemoteEntry::size` | `0` on a stub, meaning *unknown* — see below | | `get` on a stub | `RemoteError::NotMaterialised`, **never** `NotFound` and never the stub's one byte | -| `put` over a stub | refused; writing a rival file hands the client a conflict to resolve arbitrarily | +| `put` over a stub, unconditional | **replaces it** — the whole file is being written, so there is nothing in the stub to keep, and the placeholder is removed after the content lands | +| `put` over a stub, `IfMatch` | `NotMaterialised` — a stub's validator describes the placeholder, so nothing here can satisfy the guard; the caller fetches and retries | +| `put` over a stub, `IfAbsent` | `PreconditionFailed` — the file *is* there, only its content is elsewhere | | `move_to` a stub | moves the stub and keeps it a stub — culling without downloading is ordinary | | `delete` a stub | deletes it; a photograph is deleted whether or not its bytes are here | | `capabilities().materialisation` | `OnDemand` with a client, `Placeholders` without, `Always` on a plain folder | @@ -480,7 +482,29 @@ keeps. A pass over all 100, borrowing and releasing as it goes: The peak is the working set, not the library, and the release is selective. -### 6.4 Release means dehydrate, never delete +### 6.4 Derived state is dehydrated too + +Shards and the catalog snapshot live in `.darkroom-derived/` **inside the +library folder**, so a sync client dehydrates them exactly as it dehydrates a +photograph. Unlike a photograph, none of them can be skipped: a shard that will +not open is a peer's thumbnails never merging, and a catalog snapshot that will +not open is their collections. + +`derived_sync::read_derived` fetches on demand rather than giving up. More +important is what happens when it *cannot*: + +The catalog sync is a read-modify-write over a file another device also writes. +It was shaped `if let Ok(bytes) = backend.get(..)`, which folded every failure +into "there is no remote catalog" and carried straight on to the upload — so a +dehydrated snapshot meant pushing ours over theirs unmerged, taking their +collections and members with it. The same shape as the sidecar bug in §6, and +the same fix: a read that fails for any reason other than `NotFound` **stops the +upload**. + +That is why `NotFound` and `NotMaterialised` had to be separate errors. One +means "yours is the whole truth, write it"; the other means "do not dare". + +### 6.5 Release means dehydrate, never delete The single most dangerous thing in this feature. A synced folder is not a cache: deleting a materialised file inside it propagates the deletion to the @@ -492,7 +516,7 @@ This is also why the originals cache (`dr_catalog::cache`) cannot simply be pointed at a VFS library: `Cache::release` deletes bytes, which is right for a copy under `originals/` and catastrophic in place. -### 6.5 Which photographs stay downloaded +### 6.6 Which photographs stay downloaded The user's half of the bargain: a pass borrows for a moment, but *some* of the library should stay local — the trip you are about to take, the shoot you are diff --git a/docs/traceability.md b/docs/traceability.md index b61aef8..5ce459d 100644 --- a/docs/traceability.md +++ b/docs/traceability.md @@ -10,7 +10,7 @@ Denominators are parsed from [`requirements.md`](requirements.md) at run time, n | Metric | Value | |---|---| | Source files scanned | 287 | -| TRACES tags found | 840 | +| TRACES tags found | 842 | | Requirements defined | 179 | | Requirements covered | 107 | | **Coverage** | **59.8%** (107/179) | @@ -96,12 +96,12 @@ _None._ | FR-NC-6 | [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1) | | FR-NC-6a | [`core/dr-catalog/src/cache.rs:1`](../core/dr-catalog/src/cache.rs#L1), [`core/dr-catalog/src/cache.rs:225`](../core/dr-catalog/src/cache.rs#L225), [`core/dr-catalog/src/schema.rs:1014`](../core/dr-catalog/src/schema.rs#L1014), [`core/dr-catalog/src/schema.rs:613`](../core/dr-catalog/src/schema.rs#L613), [`core/dr-sync-folder/src/borrow.rs:1`](../core/dr-sync-folder/src/borrow.rs#L1), [`core/dr-types/src/selector.rs:1`](../core/dr-types/src/selector.rs#L1), [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/collections_ui.rs:3174`](../ui/dr-ui/src/collections_ui.rs#L3174), [`ui/dr-ui/src/collections_ui.rs:621`](../ui/dr-ui/src/collections_ui.rs#L621), [`ui/dr-ui/src/collections_ui.rs:683`](../ui/dr-ui/src/collections_ui.rs#L683), [`ui/dr-ui/src/lib.rs:1807`](../ui/dr-ui/src/lib.rs#L1807), [`ui/dr-ui/src/lib.rs:2698`](../ui/dr-ui/src/lib.rs#L2698), [`ui/dr-ui/src/library.rs:1449`](../ui/dr-ui/src/library.rs#L1449), [`ui/dr-ui/src/library.rs:1472`](../ui/dr-ui/src/library.rs#L1472), [`ui/dr-ui/src/library.rs:1747`](../ui/dr-ui/src/library.rs#L1747), [`ui/dr-ui/src/library_ui.rs:1065`](../ui/dr-ui/src/library_ui.rs#L1065), [`ui/dr-ui/src/library_ui.rs:1138`](../ui/dr-ui/src/library_ui.rs#L1138), [`ui/dr-ui/src/library_ui.rs:1239`](../ui/dr-ui/src/library_ui.rs#L1239), [`ui/dr-ui/src/library_ui.rs:1294`](../ui/dr-ui/src/library_ui.rs#L1294), [`ui/dr-ui/src/library_ui.rs:1429`](../ui/dr-ui/src/library_ui.rs#L1429), [`ui/dr-ui/src/library_ui.rs:1547`](../ui/dr-ui/src/library_ui.rs#L1547), [`ui/dr-ui/src/library_ui.rs:2074`](../ui/dr-ui/src/library_ui.rs#L2074), [`ui/dr-ui/src/library_ui.rs:273`](../ui/dr-ui/src/library_ui.rs#L273), [`ui/dr-ui/src/library_ui.rs:284`](../ui/dr-ui/src/library_ui.rs#L284), [`ui/dr-ui/src/library_ui.rs:292`](../ui/dr-ui/src/library_ui.rs#L292), [`ui/dr-ui/src/library_ui.rs:304`](../ui/dr-ui/src/library_ui.rs#L304), [`ui/dr-ui/src/library_ui.rs:313`](../ui/dr-ui/src/library_ui.rs#L313), [`ui/dr-ui/src/library_ui.rs:405`](../ui/dr-ui/src/library_ui.rs#L405), [`ui/dr-ui/src/library_ui.rs:415`](../ui/dr-ui/src/library_ui.rs#L415), [`ui/dr-ui/src/library_ui.rs:457`](../ui/dr-ui/src/library_ui.rs#L457), [`ui/dr-ui/src/library_ui.rs:520`](../ui/dr-ui/src/library_ui.rs#L520), [`ui/dr-ui/src/library_ui.rs:5318`](../ui/dr-ui/src/library_ui.rs#L5318), [`ui/dr-ui/src/library_ui.rs:5336`](../ui/dr-ui/src/library_ui.rs#L5336), [`ui/dr-ui/src/library_ui.rs:5348`](../ui/dr-ui/src/library_ui.rs#L5348), [`ui/dr-ui/src/library_ui.rs:551`](../ui/dr-ui/src/library_ui.rs#L551), [`ui/dr-ui/src/library_ui.rs:563`](../ui/dr-ui/src/library_ui.rs#L563), [`ui/dr-ui/src/settings_store.rs:1`](../ui/dr-ui/src/settings_store.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1), [`ui/dr-ui/ui/app.slint:2335`](../ui/dr-ui/ui/app.slint#L2335), [`ui/dr-ui/ui/app.slint:434`](../ui/dr-ui/ui/app.slint#L434), [`ui/dr-ui/ui/collections.slint:249`](../ui/dr-ui/ui/collections.slint#L249), [`ui/dr-ui/ui/collections.slint:369`](../ui/dr-ui/ui/collections.slint#L369), [`ui/dr-ui/ui/collections.slint:52`](../ui/dr-ui/ui/collections.slint#L52), [`ui/dr-ui/ui/collections.slint:682`](../ui/dr-ui/ui/collections.slint#L682), [`ui/dr-ui/ui/collections.slint:84`](../ui/dr-ui/ui/collections.slint#L84), [`ui/dr-ui/ui/icons.slint:260`](../ui/dr-ui/ui/icons.slint#L260), [`ui/dr-ui/ui/library.slint:980`](../ui/dr-ui/ui/library.slint#L980) | | FR-NC-6b | [`ui/dr-ui/src/library_ui.rs:1294`](../ui/dr-ui/src/library_ui.rs#L1294) | -| FR-NC-6c | [`core/dr-catalog/src/cache.rs:225`](../core/dr-catalog/src/cache.rs#L225), [`core/dr-sync-folder/src/borrow.rs:1`](../core/dr-sync-folder/src/borrow.rs#L1), [`core/dr-sync-folder/src/lib.rs:197`](../core/dr-sync-folder/src/lib.rs#L197), [`core/dr-sync-folder/src/lib.rs:73`](../core/dr-sync-folder/src/lib.rs#L73), [`core/dr-sync-folder/src/lib.rs:770`](../core/dr-sync-folder/src/lib.rs#L770), [`core/dr-sync-folder/src/lib.rs:809`](../core/dr-sync-folder/src/lib.rs#L809), [`core/dr-sync-folder/src/vfs.rs:1`](../core/dr-sync-folder/src/vfs.rs#L1), [`core/dr-sync-nextcloud/src/desktop_client.rs:167`](../core/dr-sync-nextcloud/src/desktop_client.rs#L167), [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-sync/src/capability.rs:41`](../core/dr-sync/src/capability.rs#L41), [`core/dr-sync/src/error.rs:39`](../core/dr-sync/src/error.rs#L39), [`core/dr-sync/src/lib.rs:158`](../core/dr-sync/src/lib.rs#L158), [`core/dr-sync/src/types.rs:101`](../core/dr-sync/src/types.rs#L101), [`core/dr-types/src/lib.rs:119`](../core/dr-types/src/lib.rs#L119), [`core/dr-types/src/lib.rs:201`](../core/dr-types/src/lib.rs#L201), [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1), [`ui/dr-ui/src/collections_ui.rs:3174`](../ui/dr-ui/src/collections_ui.rs#L3174), [`ui/dr-ui/src/collections_ui.rs:621`](../ui/dr-ui/src/collections_ui.rs#L621), [`ui/dr-ui/src/collections_ui.rs:683`](../ui/dr-ui/src/collections_ui.rs#L683), [`ui/dr-ui/src/library.rs:1569`](../ui/dr-ui/src/library.rs#L1569), [`ui/dr-ui/src/library.rs:1659`](../ui/dr-ui/src/library.rs#L1659), [`ui/dr-ui/src/library.rs:3138`](../ui/dr-ui/src/library.rs#L3138), [`ui/dr-ui/src/library.rs:3476`](../ui/dr-ui/src/library.rs#L3476), [`ui/dr-ui/src/library.rs:948`](../ui/dr-ui/src/library.rs#L948), [`ui/dr-ui/src/library_ui.rs:1065`](../ui/dr-ui/src/library_ui.rs#L1065), [`ui/dr-ui/src/library_ui.rs:1138`](../ui/dr-ui/src/library_ui.rs#L1138), [`ui/dr-ui/src/library_ui.rs:1337`](../ui/dr-ui/src/library_ui.rs#L1337), [`ui/dr-ui/src/remote.rs:40`](../ui/dr-ui/src/remote.rs#L40), [`ui/dr-ui/ui/collections.slint:249`](../ui/dr-ui/ui/collections.slint#L249), [`ui/dr-ui/ui/collections.slint:682`](../ui/dr-ui/ui/collections.slint#L682), [`ui/dr-ui/ui/icons.slint:260`](../ui/dr-ui/ui/icons.slint#L260) | +| FR-NC-6c | [`core/dr-catalog/src/cache.rs:225`](../core/dr-catalog/src/cache.rs#L225), [`core/dr-sync-folder/src/borrow.rs:1`](../core/dr-sync-folder/src/borrow.rs#L1), [`core/dr-sync-folder/src/lib.rs:197`](../core/dr-sync-folder/src/lib.rs#L197), [`core/dr-sync-folder/src/lib.rs:73`](../core/dr-sync-folder/src/lib.rs#L73), [`core/dr-sync-folder/src/lib.rs:803`](../core/dr-sync-folder/src/lib.rs#L803), [`core/dr-sync-folder/src/lib.rs:842`](../core/dr-sync-folder/src/lib.rs#L842), [`core/dr-sync-folder/src/vfs.rs:1`](../core/dr-sync-folder/src/vfs.rs#L1), [`core/dr-sync-nextcloud/src/desktop_client.rs:167`](../core/dr-sync-nextcloud/src/desktop_client.rs#L167), [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-sync/src/capability.rs:41`](../core/dr-sync/src/capability.rs#L41), [`core/dr-sync/src/error.rs:39`](../core/dr-sync/src/error.rs#L39), [`core/dr-sync/src/lib.rs:158`](../core/dr-sync/src/lib.rs#L158), [`core/dr-sync/src/types.rs:101`](../core/dr-sync/src/types.rs#L101), [`core/dr-types/src/lib.rs:119`](../core/dr-types/src/lib.rs#L119), [`core/dr-types/src/lib.rs:201`](../core/dr-types/src/lib.rs#L201), [`ui/dr-ui/src/activity.rs:1`](../ui/dr-ui/src/activity.rs#L1), [`ui/dr-ui/src/collections_ui.rs:3174`](../ui/dr-ui/src/collections_ui.rs#L3174), [`ui/dr-ui/src/collections_ui.rs:621`](../ui/dr-ui/src/collections_ui.rs#L621), [`ui/dr-ui/src/collections_ui.rs:683`](../ui/dr-ui/src/collections_ui.rs#L683), [`ui/dr-ui/src/derived_sync.rs:555`](../ui/dr-ui/src/derived_sync.rs#L555), [`ui/dr-ui/src/derived_sync.rs:627`](../ui/dr-ui/src/derived_sync.rs#L627), [`ui/dr-ui/src/library.rs:1569`](../ui/dr-ui/src/library.rs#L1569), [`ui/dr-ui/src/library.rs:1659`](../ui/dr-ui/src/library.rs#L1659), [`ui/dr-ui/src/library.rs:3138`](../ui/dr-ui/src/library.rs#L3138), [`ui/dr-ui/src/library.rs:3476`](../ui/dr-ui/src/library.rs#L3476), [`ui/dr-ui/src/library.rs:948`](../ui/dr-ui/src/library.rs#L948), [`ui/dr-ui/src/library_ui.rs:1065`](../ui/dr-ui/src/library_ui.rs#L1065), [`ui/dr-ui/src/library_ui.rs:1138`](../ui/dr-ui/src/library_ui.rs#L1138), [`ui/dr-ui/src/library_ui.rs:1337`](../ui/dr-ui/src/library_ui.rs#L1337), [`ui/dr-ui/src/remote.rs:40`](../ui/dr-ui/src/remote.rs#L40), [`ui/dr-ui/ui/collections.slint:249`](../ui/dr-ui/ui/collections.slint#L249), [`ui/dr-ui/ui/collections.slint:682`](../ui/dr-ui/ui/collections.slint#L682), [`ui/dr-ui/ui/icons.slint:260`](../ui/dr-ui/ui/icons.slint#L260) | | FR-NC-7 | [`core/dr-catalog/src/face_shard.rs:1`](../core/dr-catalog/src/face_shard.rs#L1), [`core/dr-sync-nextcloud/src/lib.rs:104`](../core/dr-sync-nextcloud/src/lib.rs#L104), [`ui/dr-ui/src/derived_sync.rs:1`](../ui/dr-ui/src/derived_sync.rs#L1), [`ui/dr-ui/src/library.rs:3356`](../ui/dr-ui/src/library.rs#L3356), [`ui/dr-ui/src/library_ui.rs:3627`](../ui/dr-ui/src/library_ui.rs#L3627), [`ui/dr-ui/ui/settings.slint:372`](../ui/dr-ui/ui/settings.slint#L372) | | FR-NC-7a | [`core/dr-ingest/src/layout.rs:1`](../core/dr-ingest/src/layout.rs#L1), [`core/dr-sync/src/upload.rs:1`](../core/dr-sync/src/upload.rs#L1), [`core/dr-sync/src/upload.rs:40`](../core/dr-sync/src/upload.rs#L40), [`core/dr-types/src/settings.rs:116`](../core/dr-types/src/settings.rs#L116), [`ui/dr-ui/src/import.rs:1`](../ui/dr-ui/src/import.rs#L1), [`ui/dr-ui/src/import.rs:97`](../ui/dr-ui/src/import.rs#L97), [`ui/dr-ui/src/import_ui.rs:1`](../ui/dr-ui/src/import_ui.rs#L1), [`ui/dr-ui/src/lib.rs:1123`](../ui/dr-ui/src/lib.rs#L1123), [`ui/dr-ui/ui/import.slint:5`](../ui/dr-ui/ui/import.slint#L5) | | FR-NC-7b | [`core/dr-ingest/src/lib.rs:733`](../core/dr-ingest/src/lib.rs#L733), [`core/dr-sync/src/upload.rs:1`](../core/dr-sync/src/upload.rs#L1), [`ui/dr-ui/src/import.rs:122`](../ui/dr-ui/src/import.rs#L122), [`ui/dr-ui/src/import.rs:336`](../ui/dr-ui/src/import.rs#L336), [`ui/dr-ui/src/import.rs:584`](../ui/dr-ui/src/import.rs#L584), [`ui/dr-ui/src/import.rs:97`](../ui/dr-ui/src/import.rs#L97), [`ui/dr-ui/src/import_ui.rs:1`](../ui/dr-ui/src/import_ui.rs#L1), [`ui/dr-ui/src/lib.rs:1123`](../ui/dr-ui/src/lib.rs#L1123) | | FR-NC-8 | [`core/dr-pipeline/src/sidecar.rs:118`](../core/dr-pipeline/src/sidecar.rs#L118), [`core/dr-pipeline/src/sidecar.rs:92`](../core/dr-pipeline/src/sidecar.rs#L92), [`ui/dr-ui/src/lib.rs:1777`](../ui/dr-ui/src/lib.rs#L1777), [`ui/dr-ui/src/library.rs:459`](../ui/dr-ui/src/library.rs#L459), [`ui/dr-ui/src/library_ui.rs:472`](../ui/dr-ui/src/library_ui.rs#L472) | -| FR-NC-9 | [`core/dr-catalog/src/merge.rs:1`](../core/dr-catalog/src/merge.rs#L1), [`core/dr-catalog/src/schema.rs:556`](../core/dr-catalog/src/schema.rs#L556), [`core/dr-catalog/src/sync.rs:1`](../core/dr-catalog/src/sync.rs#L1), [`core/dr-pipeline/src/sidecar.rs:156`](../core/dr-pipeline/src/sidecar.rs#L156), [`core/dr-pipeline/src/sidecar.rs:183`](../core/dr-pipeline/src/sidecar.rs#L183), [`core/dr-pipeline/src/sidecar.rs:2033`](../core/dr-pipeline/src/sidecar.rs#L2033), [`core/dr-pipeline/src/sidecar.rs:352`](../core/dr-pipeline/src/sidecar.rs#L352), [`core/dr-pipeline/src/sidecar.rs:450`](../core/dr-pipeline/src/sidecar.rs#L450), [`core/dr-pipeline/src/spot.rs:245`](../core/dr-pipeline/src/spot.rs#L245), [`core/dr-pipeline/tests/spot_sidecar.rs:1`](../core/dr-pipeline/tests/spot_sidecar.rs#L1), [`ui/dr-ui/src/library.rs:844`](../ui/dr-ui/src/library.rs#L844), [`ui/dr-ui/src/library.rs:998`](../ui/dr-ui/src/library.rs#L998) | +| FR-NC-9 | [`core/dr-catalog/src/merge.rs:1`](../core/dr-catalog/src/merge.rs#L1), [`core/dr-catalog/src/schema.rs:556`](../core/dr-catalog/src/schema.rs#L556), [`core/dr-catalog/src/sync.rs:1`](../core/dr-catalog/src/sync.rs#L1), [`core/dr-pipeline/src/sidecar.rs:156`](../core/dr-pipeline/src/sidecar.rs#L156), [`core/dr-pipeline/src/sidecar.rs:183`](../core/dr-pipeline/src/sidecar.rs#L183), [`core/dr-pipeline/src/sidecar.rs:2033`](../core/dr-pipeline/src/sidecar.rs#L2033), [`core/dr-pipeline/src/sidecar.rs:352`](../core/dr-pipeline/src/sidecar.rs#L352), [`core/dr-pipeline/src/sidecar.rs:450`](../core/dr-pipeline/src/sidecar.rs#L450), [`core/dr-pipeline/src/spot.rs:245`](../core/dr-pipeline/src/spot.rs#L245), [`core/dr-pipeline/tests/spot_sidecar.rs:1`](../core/dr-pipeline/tests/spot_sidecar.rs#L1), [`ui/dr-ui/src/derived_sync.rs:555`](../ui/dr-ui/src/derived_sync.rs#L555), [`ui/dr-ui/src/library.rs:844`](../ui/dr-ui/src/library.rs#L844), [`ui/dr-ui/src/library.rs:998`](../ui/dr-ui/src/library.rs#L998) | | FR-PLAT-AND-1 | [`core/dr-types/src/lib.rs:53`](../core/dr-types/src/lib.rs#L53), [`platform/dr-plat/src/volumes.rs:62`](../platform/dr-plat/src/volumes.rs#L62) | | FR-PLAT-AND-3 | [`core/dr-catalog/src/jobs.rs:1`](../core/dr-catalog/src/jobs.rs#L1) | | FR-PLAT-LIN-1 | [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`platform/dr-plat/src/storage.rs:344`](../platform/dr-plat/src/storage.rs#L344), [`ui/dr-ui/src/lib.rs:866`](../ui/dr-ui/src/lib.rs#L866), [`ui/dr-ui/src/settings_store.rs:1`](../ui/dr-ui/src/settings_store.rs#L1) | diff --git a/ui/dr-ui/src/derived_sync.rs b/ui/dr-ui/src/derived_sync.rs index b5597f6..a41c202 100644 --- a/ui/dr-ui/src/derived_sync.rs +++ b/ui/dr-ui/src/derived_sync.rs @@ -34,7 +34,7 @@ use std::path::{Path, PathBuf}; -use dr_sync::{Connection, RemoteBackend, RemoteId, RemotePath}; +use dr_sync::{Connection, RemoteBackend, RemoteError, RemoteId, RemotePath}; use dr_thumbs::ThumbStore; @@ -281,7 +281,10 @@ async fn sync_shards( } let source = RemotePath::new(format!("{}/{name}", base.as_str())); - let bytes = match backend.get(&RemoteId::Path(source), None).await { + // Fetched where it is only a placeholder: a shard that will not open + // is a peer's thumbnails never merging, and on a library the client + // keeps dehydrated that would be every shard, every pass, silently. + let bytes = match read_derived(backend, &source).await { Ok(b) => b, Err(e) => { log::warn!("downloading {name}: {e}"); @@ -464,7 +467,9 @@ async fn sync_face_shards( ))); let source = RemotePath::new(format!("{}/{name}", face_base.as_str())); - let bytes = match backend.get(&RemoteId::Path(source), None).await { + // Fetched where it is only a placeholder, for the reason the thumbnail + // shards are: otherwise a peer's faces never arrive and nothing says so. + let bytes = match read_derived(backend, &source).await { Ok(b) => b, Err(e) => { log::warn!("downloading face shard {name}: {e}"); @@ -547,7 +552,30 @@ async fn sync_catalog( // Merging before uploading means our upload carries the union rather than // only our own half, so a third device syncing next gets everything in one // fetch. - if let Ok(bytes) = backend.get(&RemoteId::Path(target.clone()), None).await { + // TRACES: FR-NC-9 | FR-NC-6c + // A read that fails for any reason other than "there is not one yet" must + // stop the upload below. This is a read-modify-write over a file another + // device also writes, so skipping the read does not merely lose an + // optimisation — it turns the write into a clobber, and the other device's + // collections and their members go with it. + // + // The shape was previously `if let Ok(bytes) = ...`, which swallowed every + // failure into "no remote catalog" and carried straight on to the upload. + let theirs = match read_derived(backend, &target).await { + Ok(bytes) => Some(bytes), + // Genuinely the first sync of this library. Nothing to merge, and + // ours is the whole truth. + Err(RemoteError::NotFound(_)) => None, + Err(e) => { + log::warn!( + "not pushing the catalog: the copy on the server could not be read ({e}); \ + uploading over it would discard whatever another device put there" + ); + return Ok(()); + } + }; + + if let Some(bytes) = theirs { let downloaded = scratch.join("catalog-remote.sqlite"); if std::fs::write(&downloaded, &bytes).is_ok() { match dr_catalog::Catalog::open(catalog_path) { @@ -557,9 +585,19 @@ async fn sync_catalog( report.collections_gained = merge.inserted + merge.updated; report.members_gained = merge.members_added; } - Err(e) => log::warn!("merging remote catalog: {e}"), + // Unreadable is not the same as absent: it may be a newer + // format, or a torn upload. Ours must not go over it. + Err(e) => { + log::warn!("not pushing the catalog: merging the server's copy: {e}"); + let _ = std::fs::remove_file(&downloaded); + return Ok(()); + } }, - Err(e) => log::warn!("opening catalog to merge: {e}"), + Err(e) => { + log::warn!("not pushing the catalog: opening ours to merge: {e}"); + let _ = std::fs::remove_file(&downloaded); + return Ok(()); + } } let _ = std::fs::remove_file(&downloaded); } @@ -586,6 +624,34 @@ async fn sync_catalog( Ok(()) } +/// TRACES: FR-NC-6c +/// Read a derived file, fetching its content first if only a placeholder is +/// here. +/// +/// Derived state lives *inside the library folder*, so on a placeholder +/// library a sync client dehydrates a shard or a catalog snapshot exactly as +/// it dehydrates a photograph. Unlike a photograph, these are ours, and none of +/// them can be skipped: a shard that will not open is face data that never +/// merges, and a catalog snapshot that will not open is the other device's +/// collections. +/// +/// So this fetches rather than giving up — and where it cannot, it says so +/// with the error rather than an empty result, because the callers below treat +/// "nothing there" as licence to write their own copy (ARCH §9.0a). +async fn read_derived( + backend: &dyn RemoteBackend, + path: &RemotePath, +) -> Result, RemoteError> { + let id = RemoteId::Path(path.clone()); + match backend.get(&id, None).await { + Err(RemoteError::NotMaterialised(_)) => { + backend.materialise(&id).await?; + backend.get(&id, None).await + } + other => other, + } +} + fn shard_name(client: &str, id: u32) -> String { format!("shard-{client}-{id:04}.sqlite") }