From d202b4b5ca2b01bb305a03cdf978c7cf3e6b0e38 Mon Sep 17 00:00:00 2001
From: Edouard Vanbelle <opensource@edouard.vanbelle.fr>
Date: Thu, 27 Aug 2026 13:18:51 +0200
Subject: [PATCH] fix(storage): the derived import conflated variant with
 directory
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

76590160 changed `variant_of` to return `{size}.{ext}`, but that value
was also being used as the on-disk DIRECTORY. Reads became
`.thumbnails/preview.webp/{hash}.webp`, which does not exist, so every
sidecar counted as unreadable and thumb_derived_import restored nothing.

Caught by thumb_import_check.sh on the run after the migration — the
harness earning its keep twice now, since this is the second defect it
has caught that no unit test could.

They are genuinely two strings and are now named as such: `dir_name` for
the path, `variant` for the row key. The cursor keeps using the
directory, so a run paused before the migration resumes at the same
position rather than restarting.

Also stops podman's compose-provider banner from burying the script's
output. Filtered rather than discarded, so genuine psql errors still
surface — swallowing those would turn a broken query into a silently
wrong assertion. Suppressing it at the source needs
`[engine] compose_warning_logs = false` in containers.conf, which is
per-developer config and cannot be relied on in CI.
---
 .../services/thumb_derived_import_service.rs  | 24 ++++++++++++-------
 tests/api/thumb_import_check.sh               | 11 ++++++++-
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/infrastructure/services/thumb_derived_import_service.rs b/src/infrastructure/services/thumb_derived_import_service.rs
index 4a189f7a..570305ed 100644
--- a/src/infrastructure/services/thumb_derived_import_service.rs
+++ b/src/infrastructure/services/thumb_derived_import_service.rs
@@ -159,11 +159,14 @@ impl RecoverableJobHandler for ThumbDerivedImport {
         let mut already = 0u64;
         let mut failed = 0u64;
         let mut since_checkpoint = 0usize;
-        // 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.
+        // Two different strings, and conflating them is a real trap: the
+        // DIRECTORY is `{size}` on disk, while the VARIANT is
+        // `{size}.{ext}` since migration `20261022000000`. Using the variant
+        // as a path yields `.thumbnails/preview.webp/…`, which does not
+        // exist, so every file reads as unreadable and nothing imports.
+        //
+        // Sidecars here are always `{hash}.webp` — the name filter requires
+        // that extension — so the variant is unconditionally the WebP one.
         let variant_of = |s: ThumbnailSize| {
             format!(
                 "{}.{}",
@@ -173,8 +176,11 @@ impl RecoverableJobHandler for ThumbDerivedImport {
         };
 
         for size in ThumbnailSize::all() {
-            let dir_name = variant_of(*size);
+            let dir_name = size.dir_name(); // on-disk directory
+            let variant = variant_of(*size); // content_derived_blobs.variant
             for name in Self::sidecar_names(&self.thumbnails_root, *size).await {
+                // Cursor position uses the DIRECTORY, so a run paused before
+                // this change resumes at the same place.
                 let position = format!("{dir_name}/{name}");
 
                 // Resume: everything at or before the cursor is done.
@@ -206,13 +212,13 @@ impl RecoverableJobHandler for ThumbDerivedImport {
                 // reason this job is safe to trigger repeatedly.
                 if self
                     .dedup
-                    .find_derived_blob(hash, "thumbnail", &dir_name)
+                    .find_derived_blob(hash, "thumbnail", &variant)
                     .await
                     .is_some()
                 {
                     already += 1;
                 } else {
-                    let path = self.thumbnails_root.join(&dir_name).join(&name);
+                    let path = self.thumbnails_root.join(dir_name).join(&name);
                     match fs::read(&path).await {
                         Ok(data) => {
                             match self
@@ -220,7 +226,7 @@ impl RecoverableJobHandler for ThumbDerivedImport {
                                 .store_derived_blob(
                                     hash,
                                     "thumbnail",
-                                    &dir_name,
+                                    &variant,
                                     "image/webp",
                                     Bytes::from(data),
                                 )
diff --git a/tests/api/thumb_import_check.sh b/tests/api/thumb_import_check.sh
index f7978972..763bfd22 100755
--- a/tests/api/thumb_import_check.sh
+++ b/tests/api/thumb_import_check.sh
@@ -80,8 +80,17 @@ fail() {
 # psql inside the compose container — no host psql dependency, matching
 # how spawn-db.sh probes readiness.
 sql() {
+  # Podman's docker-compose shim prints a provider banner to stderr on every
+  # invocation, which buries this script's own output. Filtered rather than
+  # discarded (`2>/dev/null`) so genuine psql errors still surface — losing
+  # those would turn a broken query into a silently wrong assertion.
+  #
+  # Suppressing it at the source needs `[engine] compose_warning_logs = false`
+  # in containers.conf, which is per-developer config and cannot be relied on
+  # in CI.
   docker compose -f "$COMPOSE_FILE" exec -T postgres-test \
-    psql -U oxicloud_test -d oxicloud_test -tAqc "$1"
+    psql -U oxicloud_test -d oxicloud_test -tAqc "$1" \
+    2> >(grep -v 'Executing external compose provider' >&2)
 }
 
 TOKEN=$(curl -sf -X POST "$base_url/api/auth/login" \