fix(storage): file_exists misreported every file as missing

`SELECT 1 FROM storage.files WHERE id = $1` decoded as i64. PostgreSQL
types a bare `1` as int4, so the decode always failed — and since
`.ok().flatten()` turns a decode error into the same None as "no row",
file_exists reported false for every file. thumb_attached_import
therefore classified every sidecar as an orphan and imported nothing.

Caught by thumb_import_check.sh on its first run: the derived import
restored its rows, the attached one restored none.

Now `SELECT EXISTS(...)`, which yields a real bool and always returns
exactly one row, so absence means absence. A query error still degrades
to false — the safe direction, leaving the file on disk as a reported
orphan rather than importing it against a row that may not exist.

The failure mode is the point, and it is the third of this shape in two
days: an error converted into an innocuous-looking outcome. So the check
script now dumps a job's findings when an assertion fails. The jobs
already recorded exactly why they skipped each file — the
attached_sidecar_orphan findings naming the cause were sitting in the run
while the script reported only "did not restore the row", which is
indistinguishable from the job never having run.
This commit is contained in:
Edouard Vanbelle
2026-08-26 10:03:10 +02:00
parent 4ae1531286
commit 395296a7e7
2 changed files with 40 additions and 6 deletions
@@ -130,14 +130,23 @@ impl ThumbAttachedImport {
/// Does the file still exist? Checked explicitly rather than letting the /// Does the file still exist? Checked explicitly rather than letting the
/// foreign key reject the insert, so an orphaned sidecar is *counted* as /// foreign key reject the insert, so an orphaned sidecar is *counted* as
/// an orphan instead of surfacing as an opaque constraint error. /// an orphan instead of surfacing as an opaque constraint error.
/// `SELECT EXISTS(...)`, deliberately, rather than `SELECT 1 … LIMIT 1`.
///
/// PostgreSQL types a bare `1` as `int4`, so decoding it as `i64` fails —
/// and because a decode error is indistinguishable from "no row" once
/// swallowed, every sidecar would be misreported as an orphan and nothing
/// would import. `EXISTS` yields a real `bool` and always returns exactly
/// one row, so absence means absence.
///
/// A query error still degrades to `false`, which is the safe direction:
/// the file is reported as an orphan and left on disk for the operator,
/// rather than imported against a row that may not exist.
async fn file_exists(&self, file_id: Uuid) -> bool { async fn file_exists(&self, file_id: Uuid) -> bool {
sqlx::query_scalar::<_, i64>("SELECT 1 FROM storage.files WHERE id = $1") sqlx::query_scalar::<_, bool>("SELECT EXISTS(SELECT 1 FROM storage.files WHERE id = $1)")
.bind(file_id) .bind(file_id)
.fetch_optional(self.pool.as_ref()) .fetch_one(self.pool.as_ref())
.await .await
.ok() .unwrap_or(false)
.flatten()
.is_some()
} }
} }
+26 -1
View File
@@ -50,7 +50,32 @@ COMPOSE_FILE="$REPO_ROOT/tests/common/docker-compose.test.yml"
source "$SCRIPT_DIR/test.env" source "$SCRIPT_DIR/test.env"
log() { echo "[thumb-import] $*"; } log() { echo "[thumb-import] $*"; }
fail() { echo $'\e[31m'"[thumb-import] FAIL: $*"$'\e[0m' >&2; exit 1; }
# Dump a job's findings before dying. Without this, an import that ran but
# imported nothing looks identical to one that never ran — and the jobs
# record precisely why they skipped a file (orphan, unreadable, store
# failed). The first failure of this script was a misreported orphan, and
# the finding naming it was sitting in the run the whole time.
dump_findings() {
local job="$1" run_id findings
run_id=$(curl -sf -H "$AUTH" "$base_url/api/admin/jobs/$job/runs?limit=1" 2>/dev/null \
| jq -r 'if type == "array" then .[0].id else ((.runs // .items // [])[0].id) end // empty')
[[ -z "$run_id" ]] && { echo " ($job: no run found)" >&2; return; }
findings=$(curl -sf -H "$AUTH" \
"$base_url/api/admin/jobs/$job/runs/$run_id/findings?limit=20" 2>/dev/null || echo '[]')
echo " $job findings:" >&2
echo "$findings" | jq -r \
'if type == "array" then .[] else (.findings // .items // [])[] end
| " \(.kind // .finding_kind // "?") \(.details // {} | tostring)"' 2>/dev/null >&2 \
|| echo " (unparseable)" >&2
}
fail() {
echo $'\e[31m'"[thumb-import] FAIL: $*"$'\e[0m' >&2
dump_findings thumb_derived_import
dump_findings thumb_attached_import
exit 1
}
# psql inside the compose container — no host psql dependency, matching # psql inside the compose container — no host psql dependency, matching
# how spawn-db.sh probes readiness. # how spawn-db.sh probes readiness.