fix(thumbs): the cache aged the booth it cached, and two more surfaces
Two corrections to the thumbnail work, the first of them a live bug shipped an
hour ago and caught by design-dev before its worst form landed.
⚠ GENERATING A THUMBNAIL RESET THE BOOTH'S EXPIRY CLOCK. `_newest_mtime`
excludes `.lock` sidecars because machinery is not the operator doing something;
the thumbnail cache is machinery too, and it is written by the SERVER on a mere
view. Excluding the cache's CONTENTS turned out not to be enough — creating
`.thumbs/` touches the BOOTH DIRECTORY's own mtime, which is exactly what
_newest_mtime seeds from. The booth's stamp is now restored across the mkdir,
which cannot hide real activity because any file an agent adds is counted by its
own mtime in the same walk.
The failure this prevents is not small. Once the Desk's preview strip pulls a
thumbnail per booth, ONE INDEX LOAD would have pushed every booth's expiry out
and the TTL would never have fired again — nothing would ever sweep. It was
already live for the gallery, one booth at a time.
TWO MORE SURFACES, because the fix only helped where it was wired:
Desk preview strip four small images per booth on the page he opens FIRST.
design-dev measured 28 originals / 24.1 MB on a 12-booth
copy; live has 28. The heaviest surface in the service,
heavier than the gallery it previews.
flag tray _marks.html rendered originals as tray thumbnails.
The review stage stays on the original, because that is the full-size review.
754 green plus the new guards.
This commit is contained in:
+16
-2
@@ -144,7 +144,7 @@ def set_blurred(booth: Path, rel: str, on: bool) -> set[str]:
|
||||
# The link-board logic lives in booth/links.py (stdlib only) so the `booth` CLI
|
||||
# can use it without pulling FastAPI in. Re-exported here because call sites and
|
||||
# tests already reference these names through app.
|
||||
from booth.thumbs import ensure_thumb
|
||||
from booth.thumbs import THUMB_DIR, ensure_thumb
|
||||
from booth.asks import ( # noqa: E402
|
||||
ANSWER_SUFFIX,
|
||||
ASK_SUFFIX,
|
||||
@@ -248,6 +248,15 @@ def _newest_mtime(path: Path) -> float:
|
||||
for p in path.rglob("*"):
|
||||
if p.name.startswith(".") and p.name.endswith(".lock"):
|
||||
continue
|
||||
# ⚠ AND THE THUMBNAIL CACHE, for the same reason as the locks: it is
|
||||
# MACHINERY, not the operator doing something, and it is written by the
|
||||
# SERVER on a mere view. Counting it made looking at a booth age it —
|
||||
# and once the Desk's preview strip pulls a thumbnail per booth, one
|
||||
# index load would push EVERY booth's expiry out and the TTL would
|
||||
# never fire again. `.viewed` counting is different and deliberate:
|
||||
# that is a record of a person looking, which U4 says is activity.
|
||||
if THUMB_DIR in p.relative_to(path).parts:
|
||||
continue
|
||||
try:
|
||||
m = p.stat().st_mtime
|
||||
except FileNotFoundError:
|
||||
@@ -674,7 +683,12 @@ def list_booths(data_dir: Path, ttl_seconds: float, now: float | None = None) ->
|
||||
"viewed_at": _viewed_at(child),
|
||||
# The first four images in item order, as the originals shown
|
||||
# small. Blurred ones stay blurred, the cover's rule.
|
||||
"preview": [(it.url, it.blurred) for it in items if it.kind == "image"][:4],
|
||||
# THE THUMBNAIL, not the original: this strip is four small
|
||||
# images per booth on the page he opens FIRST, so on 28 booths
|
||||
# it was the heaviest surface in the service — heavier than the
|
||||
# gallery it previews. `thumb` already carries `?thumb=1`.
|
||||
"preview": [(it.thumb or it.url, it.blurred)
|
||||
for it in items if it.kind == "image"][:4],
|
||||
}
|
||||
)
|
||||
# Newest first, NAME as the tie-break. Sorting on mtime alone left equal-mtime
|
||||
|
||||
@@ -145,7 +145,7 @@
|
||||
<div class="tray">
|
||||
{% for it in tray %}
|
||||
<a class="tray-item{% if it.blurred %} is-blurred{% endif %}" href="view?f={{ it.url }}" title="{{ it.name }}">
|
||||
{%- if it.kind == 'image' %}<img loading="lazy" src="{{ it.url }}" alt="">{% else %}<span class="tray-kind">{{ it.kind }}</span>{% endif -%}
|
||||
{%- if it.kind == 'image' %}<img loading="lazy" decoding="async" src="{{ it.thumb or it.url }}" alt="">{% else %}<span class="tray-kind">{{ it.kind }}</span>{% endif -%}
|
||||
<span class="tray-ord">#{{ "%0*d"|format(ord_width, it.ordinal) }}</span></a>
|
||||
{% endfor %}
|
||||
</div>
|
||||
|
||||
@@ -82,7 +82,24 @@ def ensure_thumb(booth: Path, rel: str) -> Path | None:
|
||||
im.thumbnail((THUMB_MAX, THUMB_MAX))
|
||||
if im.mode not in ("RGB", "RGBA"):
|
||||
im = im.convert("RGBA" if "A" in im.getbands() else "RGB")
|
||||
# ⚠ CREATING THE CACHE DIR TOUCHES THE BOOTH DIRECTORY'S OWN
|
||||
# MTIME, and `_newest_mtime` seeds from exactly that — so merely
|
||||
# LOOKING at a booth aged it, and once the Desk pulls a thumbnail
|
||||
# per booth, one index load would push every expiry out and the TTL
|
||||
# would never fire again. Excluding the cache's CONTENTS is not
|
||||
# enough; the directory entry is the leak.
|
||||
#
|
||||
# Restoring the booth's mtime cannot hide real activity: any file an
|
||||
# agent adds is counted by its OWN mtime in the same walk, and the
|
||||
# directory stamp is only the seed.
|
||||
made = not out.parent.exists()
|
||||
booth_stat = booth.stat() if made else None
|
||||
out.parent.mkdir(parents=True, exist_ok=True)
|
||||
if booth_stat is not None:
|
||||
try:
|
||||
os.utime(booth, ns=(booth_stat.st_atime_ns, booth_stat.st_mtime_ns))
|
||||
except OSError:
|
||||
pass
|
||||
# Atomic, like every other sidecar this service writes: a reader
|
||||
# polling the cache never sees a half-encoded image.
|
||||
tmp = out.with_name(out.name + f".{os.getpid()}.tmp")
|
||||
|
||||
@@ -1096,3 +1096,42 @@ def test_a_booth_name_cannot_reach_a_js_string_context(client):
|
||||
|
||||
assert 'data-confirm="wipe"' in html, "the name travels as data, where escaping is escaping"
|
||||
assert ">'+xssCanary7+'<" in html, "and still renders as the name it is"
|
||||
|
||||
|
||||
def test_generating_a_thumbnail_does_not_age_a_booth(tmp_path):
|
||||
"""⚠ A VIEW-DRIVEN WRITE MUST NOT RESET THE EXPIRY CLOCK, and the thumbnail
|
||||
cache is the first thing in this repo that writes without the operator
|
||||
doing anything.
|
||||
|
||||
`.viewed` counts as activity ON PURPOSE — U4's "viewing is activity" — but
|
||||
that is a DELIBERATE look. A derived cache is machinery, exactly like the
|
||||
`.lock` sidecars already excluded here, and it is written by the SERVER.
|
||||
|
||||
The failure this prevents is not small. Once the Desk's preview strip pulls
|
||||
a thumbnail for every booth, loading the index would touch every booth's
|
||||
cache and push every expiry out — the TTL would never fire again and
|
||||
nothing would ever sweep. Caught by design-dev before the strip landed;
|
||||
the bug was already live for the gallery.
|
||||
|
||||
Defeating change: dropping the THUMB_DIR arm of the exclusion."""
|
||||
import os
|
||||
import time
|
||||
|
||||
from booth.app import _newest_mtime
|
||||
from booth.thumbs import ensure_thumb
|
||||
|
||||
pytest.importorskip("PIL.Image")
|
||||
from PIL import Image
|
||||
|
||||
b = tmp_path / "g"
|
||||
b.mkdir()
|
||||
Image.new("RGB", (1024, 1024), (9, 9, 9)).save(b / "a.png")
|
||||
old = time.time() - 86400 * 3
|
||||
for p in b.rglob("*"):
|
||||
os.utime(p, (old, old))
|
||||
os.utime(b, (old, old))
|
||||
|
||||
before = _newest_mtime(b)
|
||||
assert ensure_thumb(b, "a.png") is not None, "nothing was generated to test"
|
||||
assert _newest_mtime(b) == pytest.approx(before, abs=2), \
|
||||
"generating a thumbnail reset the booth's expiry clock"
|
||||
|
||||
@@ -161,3 +161,19 @@ def test_the_filmstrip_and_tray_use_thumbnails_but_the_stage_does_not(tmp_path):
|
||||
assert 'src="b.png?thumb=1"' in html, "the filmstrip still pulls full images"
|
||||
assert 'src="b.png"' not in html.replace('src="b.png?thumb=1"', ""), \
|
||||
"a full-size frame survived in the strip"
|
||||
|
||||
|
||||
def test_the_desk_preview_strip_uses_thumbnails(tmp_path):
|
||||
"""The heaviest surface in the service, on the page he opens FIRST: four
|
||||
small images per booth, across every booth. design-dev measured 28
|
||||
originals / 24.1 MB on a 12-booth copy; live has 28 booths.
|
||||
|
||||
Defeating change: `it.url` in the preview tuple."""
|
||||
for name in ("one", "two"):
|
||||
b = tmp_path / name
|
||||
for n in ("a.png", "b.png"):
|
||||
_img(b / n, 1024, 1024)
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
html = c.get("/").text
|
||||
assert 'src="/b/one/a.png?thumb=1"' in html, "the Desk strip still pulls full images"
|
||||
assert 'src="/b/one/a.png"' not in html
|
||||
|
||||
Reference in New Issue
Block a user