From 86d0d655836d4c2117e168b8116b3e275235d380 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Thu, 27 Aug 2026 07:21:35 +0200 Subject: [PATCH] feat(storage): derived variant encodes the output format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `content_derived_blobs.variant` held the size alone, so one source could hold exactly one artifact per size regardless of codec. That surfaced when the read order flipped in 10c: a JPEG request matched the WebP row and would have been served the wrong codec — hidden previously because the .jpg sidecar won first. The flip had to be gated to WebP, which meant JPEG clients could never leave the sidecar, which meant the sidecar could never be deleted. It blocks transcodes harder: those are multi-format by nature, so two output codecs of one source collide on the primary key without a format term. The axis goes inside the string rather than into a fourth PK column, per the column's own rule — "new axes go inside this string, never into new columns". Shape is {size}.{ext}: preview.webp, icon.jpg, later 720p.webp. The backfill is deterministic, not a guess: store_derived_blob has only ever written "image/webp" for thumbnails. content_type is checked anyway rather than assumed — a row that fails the assumption is left alone and counted in a warning, because the read path then simply misses it and falls back to the sidecar, whereas guessing a codec would serve wrong bytes. Idempotent via NOT LIKE '%.%', so a re-apply cannot produce preview.webp.webp; verified on a scratch PG by applying it twice. One helper builds the string, because it is a primary-key component: a writer and reader that disagree do not fail loudly, they just never find each other's rows and the derived tier silently looks empty. It lives on the service's ThumbnailSize, not the port's — they are distinct types, which the compiler pointed out after I put it on the wrong one. The WebP gate on the read path is now removed: each codec has its own row, so JPEG can finally reach the derived tier — the prerequisite for deleting the sidecar for those clients. file_attached_blobs keeps a bare size: store_external_thumbnail re-encodes everything to JPEG, so it is single-format by construction and a format term would cost a migration for nothing. --- ...2000000_derived_variant_encodes_format.sql | 56 +++++++++++++++++++ .../services/thumb_derived_import_service.rs | 13 ++++- .../services/thumbnail_service.rs | 36 ++++++++---- 3 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 migrations/20261022000000_derived_variant_encodes_format.sql diff --git a/migrations/20261022000000_derived_variant_encodes_format.sql b/migrations/20261022000000_derived_variant_encodes_format.sql new file mode 100644 index 00000000..f3efcd16 --- /dev/null +++ b/migrations/20261022000000_derived_variant_encodes_format.sql @@ -0,0 +1,56 @@ +-- Put the output format inside `variant`, where the plan says new axes go. +-- +-- `content_derived_blobs.variant` held the size alone (`icon` | `preview` | +-- `large`), so a size could hold exactly ONE stored artifact regardless of +-- codec. That surfaced when the read order flipped (step 10c): a JPEG request +-- matched the WebP row and would have been served the wrong codec, which the +-- old ordering hid because the `.jpg` sidecar won first. The flip had to be +-- gated to WebP, which in turn means JPEG clients can never leave the sidecar +-- — so the sidecar can never be deleted. +-- +-- It blocks transcodes harder still: those are multi-format by nature, so +-- without a format term two output codecs of one source collide on the +-- primary key. +-- +-- Per the column's own comment — "new axes go inside this string, never into +-- new columns" — the axis goes in the string rather than into a fourth PK +-- column. The PK stays `(source_hash, kind, variant)`. +-- +-- Shape: `{size}.{ext}` — `preview.webp`, `icon.jpg`, and later `720p.webp` +-- for transcodes. +-- +-- The backfill is deterministic rather than a guess: `store_derived_blob` has +-- only ever been called with `"image/webp"` for thumbnails, so every existing +-- thumbnail row is WebP. `content_type` is checked anyway rather than assumed +-- — if that assumption is ever wrong, the row is left alone for a human to +-- look at instead of being silently mislabelled. + +UPDATE storage.content_derived_blobs + SET variant = variant || '.webp' + WHERE kind = 'thumbnail' + AND content_type = 'image/webp' + -- Idempotent: skip anything already carrying a format suffix, so a + -- re-applied migration cannot produce `preview.webp.webp`. + AND variant NOT LIKE '%.%'; + +-- Anything left without a format suffix did not match the WebP assumption. +-- Surfaced as a warning rather than coerced: the read path will simply miss +-- those rows and fall back to the sidecar, which is safe, whereas guessing a +-- codec would serve the wrong bytes. +DO $$ +DECLARE + v_unsuffixed INT; +BEGIN + SELECT COUNT(*) INTO v_unsuffixed + FROM storage.content_derived_blobs + WHERE kind = 'thumbnail' AND variant NOT LIKE '%.%'; + + IF v_unsuffixed > 0 THEN + RAISE WARNING + 'derived_variant_encodes_format: % thumbnail row(s) have no format suffix (content_type was not image/webp). They will be ignored by the read path and re-derived on demand; inspect before deleting the sidecars.', + v_unsuffixed; + END IF; +END $$; + +COMMENT ON COLUMN storage.content_derived_blobs.variant IS + 'Opaque discriminator carrying every axis but the source and the kind: size AND output format, as {size}.{ext} (preview.webp | icon.jpg | 720p.webp). New axes go inside this string, never into new columns. A format term is required — without one, two codecs of the same source collide on the primary key, and the read path cannot tell which codec a row holds.'; diff --git a/src/infrastructure/services/thumb_derived_import_service.rs b/src/infrastructure/services/thumb_derived_import_service.rs index cf0d95dd..4a189f7a 100644 --- a/src/infrastructure/services/thumb_derived_import_service.rs +++ b/src/infrastructure/services/thumb_derived_import_service.rs @@ -159,7 +159,18 @@ impl RecoverableJobHandler for ThumbDerivedImport { let mut already = 0u64; let mut failed = 0u64; let mut since_checkpoint = 0usize; - let variant_of = |s: ThumbnailSize| s.dir_name().to_string(); + // Sidecars under `.thumbnails/` are `{hash}.webp` — the filter that + // built this list requires the extension — so the imported rows are + // WebP, and the variant must say so since migration + // `20261022000000`. Writing the bare size here would produce rows the + // read path can never match. + let variant_of = |s: ThumbnailSize| { + format!( + "{}.{}", + s.dir_name(), + crate::application::ports::thumbnail_ports::ThumbnailFormat::Webp.ext() + ) + }; for size in ThumbnailSize::all() { let dir_name = variant_of(*size); diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index 6794f794..bb596d80 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -59,6 +59,22 @@ impl ThumbnailSize { } } + /// The `content_derived_blobs.variant` value for this size and format. + /// + /// One place builds the string, because it is a primary-key component: a + /// writer and a reader that disagree do not fail loudly, they simply + /// never find each other's rows — the read falls back to the sidecar and + /// the derived tier silently looks empty. + /// + /// The format term is what lets one source hold both codecs at a size. + /// Without it a JPEG request matched the WebP row and would be served the + /// wrong codec, which is why the step-10c read flip had to be gated to + /// WebP and why JPEG clients could never leave the sidecar. See migration + /// `20261022000000`. + pub fn derived_variant(&self, format: ThumbnailFormat) -> String { + format!("{}.{}", self.dir_name(), format.ext()) + } + /// Get all thumbnail sizes pub fn all() -> &'static [ThumbnailSize] { &[ @@ -310,7 +326,7 @@ impl ThumbnailService { .store_derived_blob( blob_hash, "thumbnail", - size.dir_name(), + &size.derived_variant(format), format.mime(), bytes.clone(), ) @@ -806,17 +822,15 @@ impl ThumbnailService { // draining, most content has a sidecar and no row, and terminating // here would return "no thumbnail" for all of it. // - // **WebP only.** `store_derived_blob` writes `image/webp` and keys - // `variant` on the size alone, with no format term, so a JPEG request - // would match the WebP row and be served the wrong codec — a - // regression the old ordering hid, because the `.jpg` sidecar won - // first. Until `variant` encodes format, JPEG clients stay on the - // sidecar, and the sidecar therefore cannot be deleted for them. See - // docs/plan/derived-blobs.md. - if format == ThumbnailFormat::Webp - && let Some(dedup) = dedup + // All formats, since migration `20261022000000` put the output format + // inside `variant`. Before that, `variant` was the size alone, so a + // JPEG request matched the WebP row and would have been served the + // wrong codec — the flip had to be gated to WebP, which meant JPEG + // clients could never leave the sidecar and the sidecar could never + // be deleted. Now each codec has its own row. + if let Some(dedup) = dedup && let Some(derived) = dedup - .find_derived_blob(hash, "thumbnail", size.dir_name()) + .find_derived_blob(hash, "thumbnail", &size.derived_variant(format)) .await && let Some(bytes) = Self::read_blob_to_bytes(dedup, &derived.blob_hash, file_id, size).await