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