fix(thumbnails): surface backfill results + tighten validity check
Two coupled problems, operator-flagged 2026-06-01: "missing thumbnails
but triggering backfill found nothing."
1. **Backfill UI was a black box.** `POST /api/thumbnails/backfill`
returned just `{celery_task_id}` and the admin card said
"Enqueued." with no counts. There was no way to tell whether the
scan found 0 candidates, 5000 candidates, or whether the worker
even picked up the task. "Found nothing" was indistinguishable
from a broken queue.
Fix: refactor the scan into a sync helper (`_run_backfill_scan`)
shared by the Celery task and the API endpoint. The API now runs
the scan in an executor and returns `{scanned, enqueued, ok,
regenerated}`. The actual thumbnail generation work still goes
to the thumbnail Celery queue per row via
`generate_thumbnail.delay()` — the scan itself is fast
(SELECT id+thumbnail_path + a file.stat() per row).
2. **`_thumb_is_valid` accepted header-only corrupt files.** The
magic-byte check passed for any 8-byte file starting with a JPEG
or PNG header, including empty/truncated/zero-pad files that
browsers render as broken. Backfill counted these as `ok` and
never regenerated.
Fix: also require file size ≥ MIN_THUMB_BYTES (256). Real
thumbnails are minimum ~2KB even on solid-color sources; header-
only corrupt files top out around 12 bytes. 256 is well above
the corrupt floor and well below any legitimate thumbnail.
Plus the admin card now shows the per-run counts instead of
"Enqueued.":
Scanned 5,432 · enqueued 3 (2 regenerated) · 5,429 ok
This commit is contained in:
@@ -1,5 +1,7 @@
|
|||||||
"""Thumbnail admin API: backfill trigger."""
|
"""Thumbnail admin API: backfill trigger."""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
|
||||||
from quart import Blueprint, jsonify
|
from quart import Blueprint, jsonify
|
||||||
|
|
||||||
thumbnails_bp = Blueprint("thumbnails", __name__, url_prefix="/api/thumbnails")
|
thumbnails_bp = Blueprint("thumbnails", __name__, url_prefix="/api/thumbnails")
|
||||||
@@ -7,7 +9,20 @@ thumbnails_bp = Blueprint("thumbnails", __name__, url_prefix="/api/thumbnails")
|
|||||||
|
|
||||||
@thumbnails_bp.route("/backfill", methods=["POST"])
|
@thumbnails_bp.route("/backfill", methods=["POST"])
|
||||||
async def trigger_backfill():
|
async def trigger_backfill():
|
||||||
from ..tasks.thumbnail import backfill_thumbnails
|
"""Run the backfill scan synchronously, return the counts. The actual
|
||||||
|
thumbnail generation work is still off-loaded to the thumbnail Celery
|
||||||
|
queue via `generate_thumbnail.delay()` per missing row — so this
|
||||||
|
handler is fast even on a 100k-image library (a scan is just SELECT
|
||||||
|
id, thumbnail_path + a file.stat() per row, no heavy work).
|
||||||
|
|
||||||
r = backfill_thumbnails.delay()
|
Operator-flagged 2026-06-01: the previous fire-and-forget shape
|
||||||
return jsonify({"celery_task_id": r.id}), 202
|
returned `{celery_task_id}` only, so the admin UI had no idea whether
|
||||||
|
backfill found 0 or 5000 candidates — \"found nothing\" was
|
||||||
|
indistinguishable from \"the worker isn't picking up the task.\""""
|
||||||
|
from ..tasks.thumbnail import _run_backfill_scan
|
||||||
|
|
||||||
|
# Sync scan inside an executor so we don't block the event loop.
|
||||||
|
counts = await asyncio.get_running_loop().run_in_executor(
|
||||||
|
None, _run_backfill_scan,
|
||||||
|
)
|
||||||
|
return jsonify(counts), 200
|
||||||
|
|||||||
@@ -20,14 +20,32 @@ IMAGES_ROOT = Path("/images")
|
|||||||
THUMB_MAGIC_JPEG = b"\xff\xd8\xff"
|
THUMB_MAGIC_JPEG = b"\xff\xd8\xff"
|
||||||
THUMB_MAGIC_PNG = b"\x89PNG\r\n\x1a\n"
|
THUMB_MAGIC_PNG = b"\x89PNG\r\n\x1a\n"
|
||||||
|
|
||||||
|
# Minimum file size for a thumbnail to count as valid. Anything smaller
|
||||||
|
# is almost certainly truncated/corrupt — a legitimate 400×400 JPEG@85
|
||||||
|
# bottoms out around 2KB even on a solid-color image; 400×400 PNG starts
|
||||||
|
# around 1KB. 256 bytes is well below any real thumbnail and well above
|
||||||
|
# header-only corrupt files (~8-12 bytes). Operator-flagged 2026-06-01:
|
||||||
|
# header-only corrupt files were silently passing the magic-byte check
|
||||||
|
# and backfill counted them as "ok" — so broken-image tiles in the UI
|
||||||
|
# never got regenerated even after running backfill.
|
||||||
|
MIN_THUMB_BYTES = 256
|
||||||
|
|
||||||
|
|
||||||
def _thumb_is_valid(path: Path) -> bool:
|
def _thumb_is_valid(path: Path) -> bool:
|
||||||
"""Return True iff `path` exists and starts with a JPEG or PNG magic header.
|
"""Return True iff `path` exists, starts with a JPEG or PNG magic
|
||||||
|
header, AND is at least MIN_THUMB_BYTES on disk.
|
||||||
|
|
||||||
The on-disk thumbnail format is set by services/thumbnailer.py — JPEG for
|
The on-disk thumbnail format is set by services/thumbnailer.py — JPEG
|
||||||
opaque sources, PNG for alpha sources. Anything else (missing file, OSError,
|
for opaque sources, PNG for alpha sources. Anything else (missing
|
||||||
truncated, wrong magic) is invalid.
|
file, OSError, truncated below the size floor, wrong magic) is
|
||||||
|
invalid and gets re-enqueued.
|
||||||
"""
|
"""
|
||||||
|
try:
|
||||||
|
size = path.stat().st_size
|
||||||
|
except OSError:
|
||||||
|
return False
|
||||||
|
if size < MIN_THUMB_BYTES:
|
||||||
|
return False
|
||||||
try:
|
try:
|
||||||
with path.open("rb") as f:
|
with path.open("rb") as f:
|
||||||
head = f.read(12)
|
head = f.read(12)
|
||||||
@@ -42,6 +60,59 @@ def _thumb_is_valid(path: Path) -> bool:
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _run_backfill_scan() -> dict:
|
||||||
|
"""Synchronous scan logic shared by the Celery task and the API
|
||||||
|
endpoint. Returns {enqueued, ok, regenerated, scanned}.
|
||||||
|
|
||||||
|
Operator-flagged 2026-06-01: the original task was fire-and-forget,
|
||||||
|
so the admin UI couldn't show what backfill actually found —
|
||||||
|
operator saw \"Enqueued.\" with no counts and assumed nothing was
|
||||||
|
happening. Now the API runs this synchronously and returns the
|
||||||
|
real numbers; the periodic Celery task wraps it too."""
|
||||||
|
from sqlalchemy import select, update
|
||||||
|
|
||||||
|
SessionLocal = _sync_session_factory()
|
||||||
|
enqueued = 0
|
||||||
|
ok = 0
|
||||||
|
regenerated = 0
|
||||||
|
scanned = 0
|
||||||
|
last_id = 0
|
||||||
|
with SessionLocal() as session:
|
||||||
|
while True:
|
||||||
|
rows = session.execute(
|
||||||
|
select(ImageRecord.id, ImageRecord.thumbnail_path)
|
||||||
|
.where(ImageRecord.id > last_id)
|
||||||
|
.order_by(ImageRecord.id.asc())
|
||||||
|
.limit(500)
|
||||||
|
).all()
|
||||||
|
if not rows:
|
||||||
|
break
|
||||||
|
scanned += len(rows)
|
||||||
|
for image_id, thumb_path in rows:
|
||||||
|
if thumb_path is None:
|
||||||
|
generate_thumbnail.delay(image_id)
|
||||||
|
enqueued += 1
|
||||||
|
elif _thumb_is_valid(Path(thumb_path)):
|
||||||
|
ok += 1
|
||||||
|
else:
|
||||||
|
session.execute(
|
||||||
|
update(ImageRecord)
|
||||||
|
.where(ImageRecord.id == image_id)
|
||||||
|
.values(thumbnail_path=None)
|
||||||
|
)
|
||||||
|
generate_thumbnail.delay(image_id)
|
||||||
|
enqueued += 1
|
||||||
|
regenerated += 1
|
||||||
|
session.commit()
|
||||||
|
last_id = rows[-1][0]
|
||||||
|
return {
|
||||||
|
"scanned": scanned,
|
||||||
|
"enqueued": enqueued,
|
||||||
|
"ok": ok,
|
||||||
|
"regenerated": regenerated,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@celery.task(
|
@celery.task(
|
||||||
name="backend.app.tasks.thumbnail.generate_thumbnail",
|
name="backend.app.tasks.thumbnail.generate_thumbnail",
|
||||||
bind=True,
|
bind=True,
|
||||||
@@ -84,48 +155,15 @@ def backfill_thumbnails(self) -> dict:
|
|||||||
"""Scan ImageRecord and enqueue generate_thumbnail for rows whose
|
"""Scan ImageRecord and enqueue generate_thumbnail for rows whose
|
||||||
thumbnail is missing, gone from disk, or has wrong magic bytes.
|
thumbnail is missing, gone from disk, or has wrong magic bytes.
|
||||||
|
|
||||||
Keyset paginates by id ASC, page size 500. NULLs out thumbnail_path for
|
Keyset paginates by id ASC, page size 500. NULLs out thumbnail_path
|
||||||
rows that point at a missing or corrupt file before enqueueing — keeps
|
for rows that point at a missing or corrupt file before enqueueing —
|
||||||
the DB self-consistent on partial runs and makes re-runs safe.
|
keeps the DB self-consistent on partial runs and makes re-runs safe.
|
||||||
|
|
||||||
Returns {"enqueued": N, "ok": M, "regenerated": K} where:
|
Returns {scanned, enqueued, ok, regenerated} where:
|
||||||
- enqueued = total generate_thumbnail.delay() calls
|
- scanned = total rows examined
|
||||||
- ok = rows whose existing thumbnail file is valid (skipped)
|
- enqueued = total generate_thumbnail.delay() calls
|
||||||
- regenerated = subset of enqueued that had a non-NULL thumbnail_path
|
- ok = rows whose existing thumbnail file is valid (skipped)
|
||||||
cleared (i.e. missing + corrupt)
|
- regenerated = subset of enqueued that had a non-NULL
|
||||||
|
thumbnail_path cleared (i.e. missing + corrupt)
|
||||||
"""
|
"""
|
||||||
from sqlalchemy import select, update
|
return _run_backfill_scan()
|
||||||
|
|
||||||
SessionLocal = _sync_session_factory()
|
|
||||||
enqueued = 0
|
|
||||||
ok = 0
|
|
||||||
regenerated = 0
|
|
||||||
last_id = 0
|
|
||||||
with SessionLocal() as session:
|
|
||||||
while True:
|
|
||||||
rows = session.execute(
|
|
||||||
select(ImageRecord.id, ImageRecord.thumbnail_path)
|
|
||||||
.where(ImageRecord.id > last_id)
|
|
||||||
.order_by(ImageRecord.id.asc())
|
|
||||||
.limit(500)
|
|
||||||
).all()
|
|
||||||
if not rows:
|
|
||||||
break
|
|
||||||
for image_id, thumb_path in rows:
|
|
||||||
if thumb_path is None:
|
|
||||||
generate_thumbnail.delay(image_id)
|
|
||||||
enqueued += 1
|
|
||||||
elif _thumb_is_valid(Path(thumb_path)):
|
|
||||||
ok += 1
|
|
||||||
else:
|
|
||||||
session.execute(
|
|
||||||
update(ImageRecord)
|
|
||||||
.where(ImageRecord.id == image_id)
|
|
||||||
.values(thumbnail_path=None)
|
|
||||||
)
|
|
||||||
generate_thumbnail.delay(image_id)
|
|
||||||
enqueued += 1
|
|
||||||
regenerated += 1
|
|
||||||
session.commit()
|
|
||||||
last_id = rows[-1][0]
|
|
||||||
return {"enqueued": enqueued, "ok": ok, "regenerated": regenerated}
|
|
||||||
|
|||||||
@@ -10,7 +10,14 @@
|
|||||||
<v-btn color="primary" rounded="pill" :loading="busy" @click="run">
|
<v-btn color="primary" rounded="pill" :loading="busy" @click="run">
|
||||||
<v-icon start>mdi-image-refresh</v-icon> Run backfill now
|
<v-icon start>mdi-image-refresh</v-icon> Run backfill now
|
||||||
</v-btn>
|
</v-btn>
|
||||||
<span v-if="done" class="ml-3 text-caption">Enqueued.</span>
|
<span v-if="result" class="ml-3 text-caption">
|
||||||
|
Scanned <strong>{{ result.scanned }}</strong> · enqueued
|
||||||
|
<strong>{{ result.enqueued }}</strong>
|
||||||
|
<span v-if="result.regenerated > 0">
|
||||||
|
({{ result.regenerated }} regenerated)
|
||||||
|
</span>
|
||||||
|
· {{ result.ok }} ok
|
||||||
|
</span>
|
||||||
<QueueStatusBar queue="thumbnail" queue-label="Thumbnail" />
|
<QueueStatusBar queue="thumbnail" queue-label="Thumbnail" />
|
||||||
</v-card-text>
|
</v-card-text>
|
||||||
</v-card>
|
</v-card>
|
||||||
@@ -23,10 +30,11 @@ import { useThumbnailsStore } from '../../stores/thumbnails.js'
|
|||||||
import QueueStatusBar from './QueueStatusBar.vue'
|
import QueueStatusBar from './QueueStatusBar.vue'
|
||||||
const store = useThumbnailsStore()
|
const store = useThumbnailsStore()
|
||||||
const busy = ref(false)
|
const busy = ref(false)
|
||||||
const done = ref(false)
|
const result = ref(null)
|
||||||
async function run () {
|
async function run () {
|
||||||
busy.value = true
|
busy.value = true
|
||||||
try { await store.triggerBackfill(); done.value = true }
|
result.value = null
|
||||||
|
try { result.value = await store.triggerBackfill() }
|
||||||
catch (e) { toast({ text: e.message, type: 'error' }) }
|
catch (e) { toast({ text: e.message, type: 'error' }) }
|
||||||
finally { busy.value = false }
|
finally { busy.value = false }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,8 +4,11 @@ import { useApi } from '../composables/useApi.js'
|
|||||||
export const useThumbnailsStore = defineStore('thumbnails', () => {
|
export const useThumbnailsStore = defineStore('thumbnails', () => {
|
||||||
const api = useApi()
|
const api = useApi()
|
||||||
|
|
||||||
|
// Returns { scanned, enqueued, ok, regenerated } — the API now runs
|
||||||
|
// the scan synchronously so the operator gets immediate feedback
|
||||||
|
// instead of just a Celery task id with no visible outcome.
|
||||||
async function triggerBackfill () {
|
async function triggerBackfill () {
|
||||||
await api.post('/api/thumbnails/backfill')
|
return await api.post('/api/thumbnails/backfill')
|
||||||
}
|
}
|
||||||
|
|
||||||
return { triggerBackfill }
|
return { triggerBackfill }
|
||||||
|
|||||||
@@ -13,8 +13,16 @@ def eager():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_trigger_thumbnail_backfill(client):
|
async def test_trigger_thumbnail_backfill_returns_counts(client):
|
||||||
|
"""The API runs the scan synchronously now and returns
|
||||||
|
{scanned, enqueued, ok, regenerated} — operator-flagged 2026-06-01:
|
||||||
|
the previous fire-and-forget shape returned only a celery_task_id,
|
||||||
|
so the admin UI couldn't show whether backfill found 0 or 5000
|
||||||
|
candidates. \"Found nothing\" was indistinguishable from \"the
|
||||||
|
worker isn't picking up the task.\""""
|
||||||
r = await client.post("/api/thumbnails/backfill")
|
r = await client.post("/api/thumbnails/backfill")
|
||||||
assert r.status_code == 202
|
assert r.status_code == 200
|
||||||
body = await r.get_json()
|
body = await r.get_json()
|
||||||
assert "celery_task_id" in body
|
assert set(body) == {"scanned", "enqueued", "ok", "regenerated"}
|
||||||
|
for key in ("scanned", "enqueued", "ok", "regenerated"):
|
||||||
|
assert isinstance(body[key], int)
|
||||||
|
|||||||
@@ -12,13 +12,16 @@ pytestmark = pytest.mark.integration
|
|||||||
|
|
||||||
def test_thumb_is_valid_jpeg(tmp_path):
|
def test_thumb_is_valid_jpeg(tmp_path):
|
||||||
p = tmp_path / "good.jpg"
|
p = tmp_path / "good.jpg"
|
||||||
p.write_bytes(b"\xff\xd8\xff\xe0" + b"\x00" * 100)
|
# Real thumbnails are at least ~2KB; size check (MIN_THUMB_BYTES=256)
|
||||||
|
# requires the file body be plausible. 300 bytes here clears the
|
||||||
|
# floor with margin.
|
||||||
|
p.write_bytes(b"\xff\xd8\xff\xe0" + b"\x00" * 300)
|
||||||
assert _thumb_is_valid(p) is True
|
assert _thumb_is_valid(p) is True
|
||||||
|
|
||||||
|
|
||||||
def test_thumb_is_valid_png(tmp_path):
|
def test_thumb_is_valid_png(tmp_path):
|
||||||
p = tmp_path / "good.png"
|
p = tmp_path / "good.png"
|
||||||
p.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 100)
|
p.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 300)
|
||||||
assert _thumb_is_valid(p) is True
|
assert _thumb_is_valid(p) is True
|
||||||
|
|
||||||
|
|
||||||
@@ -38,6 +41,16 @@ def test_thumb_is_valid_missing_file(tmp_path):
|
|||||||
assert _thumb_is_valid(tmp_path / "nope") is False
|
assert _thumb_is_valid(tmp_path / "nope") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_thumb_is_valid_header_only_below_min_size(tmp_path):
|
||||||
|
"""Operator-flagged 2026-06-01: header-only corrupt files were
|
||||||
|
silently passing the magic-byte check and backfill counted them as
|
||||||
|
`ok`, so the UI's broken-image tiles never got regenerated. Files
|
||||||
|
smaller than MIN_THUMB_BYTES are now invalid even with valid magic."""
|
||||||
|
p = tmp_path / "header_only.jpg"
|
||||||
|
p.write_bytes(b"\xff\xd8\xff\xe0" + b"\x00" * 50) # 54 bytes total
|
||||||
|
assert _thumb_is_valid(p) is False
|
||||||
|
|
||||||
|
|
||||||
# --- backfill_thumbnails planner tests ------------------------------------
|
# --- backfill_thumbnails planner tests ------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@@ -80,13 +93,14 @@ def _rec(db_sync, path, *, sha, thumb_path=None, mime="image/jpeg"):
|
|||||||
|
|
||||||
def _write_jpeg(p: Path) -> Path:
|
def _write_jpeg(p: Path) -> Path:
|
||||||
p.parent.mkdir(parents=True, exist_ok=True)
|
p.parent.mkdir(parents=True, exist_ok=True)
|
||||||
p.write_bytes(b"\xff\xd8\xff\xe0" + b"\x00" * 100)
|
# ≥ MIN_THUMB_BYTES (256) so the size floor doesn't reject it.
|
||||||
|
p.write_bytes(b"\xff\xd8\xff\xe0" + b"\x00" * 300)
|
||||||
return p
|
return p
|
||||||
|
|
||||||
|
|
||||||
def _write_png(p: Path) -> Path:
|
def _write_png(p: Path) -> Path:
|
||||||
p.parent.mkdir(parents=True, exist_ok=True)
|
p.parent.mkdir(parents=True, exist_ok=True)
|
||||||
p.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 100)
|
p.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 300)
|
||||||
return p
|
return p
|
||||||
|
|
||||||
|
|
||||||
@@ -238,5 +252,5 @@ def test_backfill_mixed_aggregate(db_sync, tmp_path, monkeypatch):
|
|||||||
)
|
)
|
||||||
|
|
||||||
result = m.backfill_thumbnails()
|
result = m.backfill_thumbnails()
|
||||||
assert result == {"enqueued": 3, "ok": 2, "regenerated": 2}
|
assert result == {"scanned": 5, "enqueued": 3, "ok": 2, "regenerated": 2}
|
||||||
assert sorted(delayed) == sorted([r_null.id, r_missing.id, r_bad.id])
|
assert sorted(delayed) == sorted([r_null.id, r_missing.id, r_bad.id])
|
||||||
|
|||||||
Reference in New Issue
Block a user