From 2775e6d5673f85f312bf937b0f1de04a665ea0c7 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 26 Aug 2026 21:58:21 +0200 Subject: [PATCH] docs: the two sidecar-only paths are production-unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing out step 10(a). The remaining `None` call sites in persist_rendered looked like an open gap; they are not reachable in production. Both `get_thumbnail` and the path variant of `generate_all_sizes_background` are called only from the `ThumbnailPort` impl, and nothing holds a `dyn ThumbnailPort` — which the existing note in get_cached_thumbnail already recorded and a grep confirms. Live renders go through get_thumbnail_from_blob and generate_all_sizes_background_from_blob, both of which carry a DedupService and dual-write. So threading a DedupService through them would be work with no runtime effect. Recorded at each call site instead, with the condition that matters: gaining a real caller means taking a DedupService first, or the gap persist_rendered exists to close reopens — sidecar-only output the import can never see, so the tail never empties and the deletion gate never opens. Marks 10(a) done in the plan with that caveat stated rather than implied. --- docs/plan/derived-blobs.md | 12 ++++++---- .../services/thumbnail_service.rs | 24 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/docs/plan/derived-blobs.md b/docs/plan/derived-blobs.md index db386840..6e460d74 100644 --- a/docs/plan/derived-blobs.md +++ b/docs/plan/derived-blobs.md @@ -1294,10 +1294,14 @@ hardcoded SQL). New sources bolt on independently. 10. **Import jobs + the dual-read fallback** — see the migration section. Revised ordering as of 2026-08-26: - a. **Consolidate onto one `persist_thumbnail`** (dual-write). The - blocker: today only one of four render paths writes the derived - row, so the import can never converge. See *Prerequisite: one - persist function*. + a. **Consolidate onto one `persist_rendered`** (dual-write) — **done + 2026-08-26**. Was the blocker: only one of four render paths wrote + the derived row, so the import could never converge. Every live + render now dual-writes. Two paths still pass `None` and stay + sidecar-only, which is safe *only* because both are reachable + solely through the `ThumbnailPort` impl and nothing holds a + `dyn ThumbnailPort` — if either gains a real caller it must take a + `DedupService` first. See *Prerequisite: one persist function*. b. **`thumb_derived_import`** (shipped) and **`thumb_attached_import`** (shipped) — two jobs, not one, because the keying differs and that difference is the security boundary. A third, `transcode_import`, diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index 6c2a9270..a22e48a9 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -372,9 +372,18 @@ impl ThumbnailService { tracing::info!("🎨 Generating thumbnail: {} {:?}", file_id_owned, size); match self.generate_thumbnail(&original_owned, size, format).await { Ok(bytes) => { - // `None`: renders from an on-disk original and holds - // no DedupService, so sidecar-only. Visible here - // rather than absent. + // `None` — sidecar-only, and that is acceptable here + // ONLY because this path is production-unreachable: + // its sole caller is the `ThumbnailPort` impl, and + // nothing holds a `dyn ThumbnailPort` (checked). Live + // renders go through `get_thumbnail_from_blob`, which + // dual-writes. + // + // If this ever gains a real caller it must take a + // `DedupService` first, or it reopens the gap + // `persist_rendered` exists to close: sidecar-only + // output the import can never see, so the tail never + // empties. self.persist_rendered(&blob_hash_owned, size, format, &bytes, None) .await; bytes @@ -1295,9 +1304,12 @@ impl ThumbnailService { // Save each size to disk and populate moka — both keyed by // blob_hash, so the two tiers agree and a copy shares them. for (size, bytes) in thumbnails { - // `None`: this variant renders from a path and holds no - // DedupService — `generate_all_sizes_background_from_blob` is - // the one that does. Sidecar-only, visibly so. + // `None` — sidecar-only, acceptable for the same reason as + // `get_thumbnail`: the path variant is reached only through + // the unused `ThumbnailPort` impl. The live upload path is + // `generate_all_sizes_background_from_blob`, which carries a + // `DedupService` and dual-writes. Give this one a real caller + // and it needs one too. self.persist_rendered(&blob_hash, size, ThumbnailFormat::Webp, &bytes, None) .await; {