feat: thumbnails for uploaded images, at two widths and no others
A question's stem image is two to four megabytes of scanned radiograph, and a
media grid is forty of those pulled at full size to draw forty postage stamps.
`?w=256` and `?w=640` now serve a WebP copy instead, made on the first ask and
kept beside the original under `thumbs/{width}/{key}` — same bucket, so nothing
new has to be configured for them to be backed up or thrown away.
Three rules, all about not making this a way to spend the server's afternoon.
Those two widths and no others: any other `?w=` is refused with a 400, because
an endpoint that resizes to whatever the query string asks for is a CPU sink
anybody can point at. Never enlarged: a 180px image asked for at 640 is served
as it is, since scaling up invents detail and charges bytes for it. And best
effort throughout — a PDF, an SVG, a truncated upload or a file that is not the
image its name claims all serve their original rather than failing, because a
preview must never take down the page that wanted it.
Authorisation is unchanged and still runs first: a thumbnail of a file you may
not read is a file you may not read. They stay `private, no-store` like
everything else here — they are behind authentication, so there is nothing for
a shared cache to do with them, and the win is the byte count.
EXIF rotation is read before anything measures the image. Every phone stores a
portrait photograph sideways with a flag; a thumbnail made without reading it
is a sideways thumbnail.
Pillow rather than sharp, which is Node. It is not pinned in requirements: the
pin invalidates the pip layer, and that layer no longer builds because
litellm==1.28.13 has been withdrawn from PyPI. Re-pinning litellm is a
deliberate upgrade of the AI layer, not something to slip into this. Noted in
the TODO.
Also: the article hover-card excerpt was printing `[[288|eczema]]` at readers.
The generic markdown-link rule does not know our own cross-reference syntax, so
it left the brackets and the id behind.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
f94bdddaf5
commit
cedab6e91e
12 changed files with 343 additions and 26 deletions
|
|
@ -401,6 +401,12 @@ def _plain_excerpt(article: Article, limit: int = 260) -> str:
|
||||||
if not source.strip() and article.sections:
|
if not source.strip() and article.sections:
|
||||||
source = (article.sections[0] or {}).get("content") or ""
|
source = (article.sections[0] or {}).get("content") or ""
|
||||||
text = re.sub(r"!\[[^\]]*\]\([^)]*\)", " ", source) # images
|
text = re.sub(r"!\[[^\]]*\]\([^)]*\)", " ", source) # images
|
||||||
|
# Our own cross-reference syntax, before the plain-link rule below gets at
|
||||||
|
# it: `[[288|eczema]]` and `[[Eczema|eczema]]` both mean a word, and the
|
||||||
|
# generic rule left "[[288 eczema]]" on the card. Which half is the word
|
||||||
|
# depends on which half is a number.
|
||||||
|
text = re.sub(r"\[\[(?:(\d+)\|([^\]]+)|([^\]|]+?)(?:\|[a-z0-9-]+)?)\]\]",
|
||||||
|
lambda m: (m.group(2) or m.group(3) or "").strip(), text)
|
||||||
text = re.sub(r"\[([^\]]*)\]\([^)]*\)", r"\1", text) # links keep their words
|
text = re.sub(r"\[([^\]]*)\]\([^)]*\)", r"\1", text) # links keep their words
|
||||||
text = re.sub(r"^\s{0,3}#{1,6}\s*", "", text, flags=re.M) # headings
|
text = re.sub(r"^\s{0,3}#{1,6}\s*", "", text, flags=re.M) # headings
|
||||||
text = re.sub(r"[*_`>|]|^\s*[-+]\s", " ", text, flags=re.M)
|
text = re.sub(r"[*_`>|]|^\s*[-+]\s", " ", text, flags=re.M)
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@ from fastapi.responses import FileResponse, Response
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app.database import get_db
|
from app.database import get_db
|
||||||
from app.services import storage_service
|
from app.services import storage_service, thumbnails
|
||||||
from app.utils.auth import get_current_user
|
from app.utils.auth import get_current_user
|
||||||
from app.utils.upload_access import (
|
from app.utils.upload_access import (
|
||||||
LEGACY_LMS_PREFIXES, local_upload_path, upload_file, references,
|
LEGACY_LMS_PREFIXES, local_upload_path, upload_file, references,
|
||||||
|
|
@ -17,16 +17,32 @@ router = APIRouter()
|
||||||
|
|
||||||
|
|
||||||
@router.api_route("/uploads/{path:path}", methods=["GET", "HEAD"])
|
@router.api_route("/uploads/{path:path}", methods=["GET", "HEAD"])
|
||||||
def read_upload(path: str, request: Request, attempt_id: int | None = None, db: Session = Depends(get_db)):
|
def read_upload(path: str, request: Request, attempt_id: int | None = None,
|
||||||
|
w: int | None = None, db: Session = Depends(get_db)):
|
||||||
|
"""An upload, optionally at one of two smaller widths.
|
||||||
|
|
||||||
|
`?w=256` and `?w=640` are the only sizes there are, and anything else is
|
||||||
|
refused rather than honoured — an endpoint that resizes to whatever the
|
||||||
|
query string asks for is a CPU sink anybody can point at. The authorisation
|
||||||
|
below is unchanged and runs first: a thumbnail of a file you may not read
|
||||||
|
is a file you may not read.
|
||||||
|
|
||||||
|
These stay `private, no-store` like everything else here. They are behind
|
||||||
|
authentication, so there is nothing for a shared cache to do with them; the
|
||||||
|
win is that the bytes are a tenth the size.
|
||||||
|
"""
|
||||||
headers = {"Cache-Control": "private, no-store", "Vary": "Cookie, Authorization", "X-Content-Type-Options": "nosniff"}
|
headers = {"Cache-Control": "private, no-store", "Vary": "Cookie, Authorization", "X-Content-Type-Options": "nosniff"}
|
||||||
|
if w is not None and w not in thumbnails.WIDTHS:
|
||||||
|
raise HTTPException(400, f"Width must be one of {', '.join(map(str, thumbnails.WIDTHS))}",
|
||||||
|
headers=headers)
|
||||||
try:
|
try:
|
||||||
return _read_upload(path, request, attempt_id, db, headers)
|
return _read_upload(path, request, attempt_id, db, headers, w)
|
||||||
except HTTPException as exc:
|
except HTTPException as exc:
|
||||||
exc.headers = {**(exc.headers or {}), **headers}
|
exc.headers = {**(exc.headers or {}), **headers}
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
|
||||||
def _read_upload(path, request, attempt_id, db, headers):
|
def _read_upload(path, request, attempt_id, db, headers, width=None):
|
||||||
try:
|
try:
|
||||||
path = local_upload_path(path)
|
path = local_upload_path(path)
|
||||||
target = upload_file(path)
|
target = upload_file(path)
|
||||||
|
|
@ -47,6 +63,18 @@ def _read_upload(path, request, attempt_id, db, headers):
|
||||||
if target.suffix.lower() == ".svg":
|
if target.suffix.lower() == ".svg":
|
||||||
headers["Content-Security-Policy"] = "sandbox; default-src 'none'; style-src 'unsafe-inline'"
|
headers["Content-Security-Policy"] = "sandbox; default-src 'none'; style-src 'unsafe-inline'"
|
||||||
|
|
||||||
|
# Asked for small, and small exists: the derivative is made on the first
|
||||||
|
# ask and stored beside the original. `None` back means there is nothing
|
||||||
|
# smaller worth serving — not an image, or already narrower than asked —
|
||||||
|
# so the original goes out, which is what the caller wanted anyway.
|
||||||
|
if width:
|
||||||
|
small = thumbnails.get(path, width)
|
||||||
|
if small is not None:
|
||||||
|
if request.method == "HEAD":
|
||||||
|
return Response(status_code=200, media_type="image/webp",
|
||||||
|
headers={**headers, "Content-Length": str(len(small))})
|
||||||
|
return Response(content=small, media_type="image/webp", headers=headers)
|
||||||
|
|
||||||
# Serving must go through the storage service, or object storage would be
|
# Serving must go through the storage service, or object storage would be
|
||||||
# write-only: the bytes would be in the bucket and still read from disk.
|
# write-only: the bytes would be in the bucket and still read from disk.
|
||||||
# The path is already authorised and confined to the upload root above.
|
# The path is already authorised and confined to the upload root above.
|
||||||
|
|
|
||||||
115
backend/app/services/thumbnails.py
Normal file
115
backend/app/services/thumbnails.py
Normal file
|
|
@ -0,0 +1,115 @@
|
||||||
|
"""Smaller copies of an uploaded image, made once and kept.
|
||||||
|
|
||||||
|
A question's stem image is 2–4 MB of scanned radiograph and a grid of them is
|
||||||
|
twenty of those, so a browse page was pulling forty megabytes to draw forty
|
||||||
|
postage stamps. This makes a 256px and a 640px WebP of each and serves those
|
||||||
|
where a small one will do.
|
||||||
|
|
||||||
|
Three rules, all of them about not turning this into a way to spend the
|
||||||
|
server's afternoon:
|
||||||
|
|
||||||
|
* **Allow-listed widths only.** 256 and 640, and any other `?w=` is refused
|
||||||
|
rather than honoured. An endpoint that resizes to whatever the query string
|
||||||
|
asks for is a CPU sink anybody can point at.
|
||||||
|
* **Never enlarged.** A 180px image asked for at 640 is served as it is.
|
||||||
|
Scaling up invents detail and costs bytes to do it.
|
||||||
|
* **Best effort.** A derivative that cannot be made is not an error — the
|
||||||
|
caller falls back to the original. A failed preview must never fail an
|
||||||
|
upload, and a corrupt file must never take a page down with it.
|
||||||
|
|
||||||
|
Derivatives live in the same bucket under `thumbs/{width}/{key}`, so
|
||||||
|
credentials, lifecycle and backup are unchanged and nothing new has to be
|
||||||
|
configured for them to be backed up or thrown away.
|
||||||
|
|
||||||
|
Pillow rather than sharp: sharp is Node, this backend is Python, and Pillow is
|
||||||
|
already here and does the same three things.
|
||||||
|
"""
|
||||||
|
import io
|
||||||
|
import logging
|
||||||
|
import mimetypes
|
||||||
|
|
||||||
|
from app.services import storage_service
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
#: The only widths that exist. See the module docstring.
|
||||||
|
WIDTHS = (256, 640)
|
||||||
|
|
||||||
|
#: What is worth resizing. A PDF or an SVG is not: one is not an image and the
|
||||||
|
#: other is already small and resolution-independent.
|
||||||
|
RESIZABLE = {"image/jpeg", "image/png", "image/webp", "image/gif", "image/bmp", "image/tiff"}
|
||||||
|
|
||||||
|
QUALITY = 82
|
||||||
|
|
||||||
|
|
||||||
|
def thumb_key(key: str, width: int) -> str:
|
||||||
|
return f"thumbs/{width}/{key}"
|
||||||
|
|
||||||
|
|
||||||
|
def is_resizable(key: str) -> bool:
|
||||||
|
return (mimetypes.guess_type(key)[0] or "") in RESIZABLE
|
||||||
|
|
||||||
|
|
||||||
|
def render(data: bytes, width: int) -> bytes | None:
|
||||||
|
"""One derivative, or None if it should not or cannot be made."""
|
||||||
|
try:
|
||||||
|
from PIL import Image, ImageOps
|
||||||
|
except ImportError: # pragma: no cover - Pillow is a hard dependency here
|
||||||
|
logger.warning("Pillow is not installed; serving originals")
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
image = Image.open(io.BytesIO(data))
|
||||||
|
# Before anything measures it: a photograph from a phone is stored
|
||||||
|
# sideways with a rotation flag, and a thumbnail made without reading
|
||||||
|
# that flag is a sideways thumbnail of a portrait image.
|
||||||
|
image = ImageOps.exif_transpose(image)
|
||||||
|
if image.width <= width:
|
||||||
|
return None
|
||||||
|
height = max(1, round(image.height * width / image.width))
|
||||||
|
image = image.convert("RGBA" if image.mode in ("RGBA", "LA", "P") else "RGB")
|
||||||
|
image = image.resize((width, height), Image.LANCZOS)
|
||||||
|
out = io.BytesIO()
|
||||||
|
image.save(out, format="WEBP", quality=QUALITY, method=4)
|
||||||
|
return out.getvalue()
|
||||||
|
except Exception:
|
||||||
|
# A file that is not the image its name claims, a truncated upload, a
|
||||||
|
# decompression bomb Pillow refuses to open. None of those is a reason
|
||||||
|
# to fail the request that asked for a preview.
|
||||||
|
logger.info("Could not make a %spx thumbnail", width, exc_info=True)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def get(key: str, width: int) -> bytes | None:
|
||||||
|
"""The derivative, making it on first ask. None means "serve the original".
|
||||||
|
|
||||||
|
Not a cache that can be turned off: the second reader of a page gets the
|
||||||
|
stored copy, and the first pays for it once.
|
||||||
|
"""
|
||||||
|
if width not in WIDTHS or not is_resizable(key):
|
||||||
|
return None
|
||||||
|
derived = thumb_key(key, width)
|
||||||
|
existing = storage_service.load(derived)
|
||||||
|
if existing:
|
||||||
|
return existing
|
||||||
|
source = storage_service.load(key)
|
||||||
|
if not source:
|
||||||
|
return None
|
||||||
|
made = render(source, width)
|
||||||
|
if not made:
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
storage_service.save(derived, made, "image/webp")
|
||||||
|
except Exception:
|
||||||
|
# Serve it anyway. Storing is an optimisation for the next reader; this
|
||||||
|
# one already has the bytes in hand.
|
||||||
|
logger.info("Could not store the %spx thumbnail for %s", width, key, exc_info=True)
|
||||||
|
return made
|
||||||
|
|
||||||
|
|
||||||
|
def forget(key: str) -> None:
|
||||||
|
"""Drop every derivative of a key, for when the original changes or goes."""
|
||||||
|
for width in WIDTHS:
|
||||||
|
try:
|
||||||
|
storage_service.delete(thumb_key(key, width))
|
||||||
|
except Exception:
|
||||||
|
logger.info("Could not remove the %spx thumbnail for %s", width, key, exc_info=True)
|
||||||
|
|
@ -280,13 +280,20 @@ class ArticleReadingTests(unittest.TestCase):
|
||||||
|
|
||||||
def test_excerpt_reads_as_prose_not_markup(self):
|
def test_excerpt_reads_as_prose_not_markup(self):
|
||||||
self.make([self.section('a', 'A')], slug='marked-up', summary='',
|
self.make([self.section('a', 'A')], slug='marked-up', summary='',
|
||||||
content='## Heading\n\n See [the workup](/articles/workup) **now**.')
|
content='## Heading\n\n See [the workup](/articles/workup) **now**, '
|
||||||
|
'stratified by [[288|eczema]] and [[Croup|croup]].')
|
||||||
self.client.post('/articles/1/publish', json={'published': True})
|
self.client.post('/articles/1/publish', json={'published': True})
|
||||||
excerpt = self.client.get('/articles/preview/marked-up').json()['excerpt']
|
excerpt = self.client.get('/articles/preview/marked-up').json()['excerpt']
|
||||||
self.assertNotIn('!', excerpt)
|
self.assertNotIn('!', excerpt)
|
||||||
self.assertNotIn('##', excerpt)
|
self.assertNotIn('##', excerpt)
|
||||||
self.assertNotIn('/uploads/', excerpt)
|
self.assertNotIn('/uploads/', excerpt)
|
||||||
self.assertIn('the workup', excerpt) # a link keeps its words
|
self.assertIn('the workup', excerpt) # a link keeps its words
|
||||||
|
# And so does a cross-reference. The generic link rule does not know
|
||||||
|
# this syntax, so it used to leave "[[288 eczema]]" on the card.
|
||||||
|
self.assertNotIn('[[', excerpt)
|
||||||
|
self.assertNotIn('288', excerpt)
|
||||||
|
self.assertIn('eczema', excerpt)
|
||||||
|
self.assertIn('Croup', excerpt)
|
||||||
|
|
||||||
def test_all_three_views_round_trip_through_a_save(self):
|
def test_all_three_views_round_trip_through_a_save(self):
|
||||||
payload = {"title": "Nested topic", "slug": "three-views", "summary": "S", "content": "I",
|
payload = {"title": "Nested topic", "slug": "three-views", "summary": "S", "content": "I",
|
||||||
|
|
|
||||||
112
backend/tests/test_thumbnails.py
Normal file
112
backend/tests/test_thumbnails.py
Normal file
|
|
@ -0,0 +1,112 @@
|
||||||
|
"""Smaller copies of an uploaded image: which widths exist, and what refuses.
|
||||||
|
|
||||||
|
Disposable temp directory for the upload root; Pillow does the real work, so
|
||||||
|
these are real images in and real WebP out rather than mocks agreeing with
|
||||||
|
themselves.
|
||||||
|
"""
|
||||||
|
import io
|
||||||
|
import os
|
||||||
|
import tempfile
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
os.environ["DATABASE_URL"] = "sqlite:///:memory:"
|
||||||
|
|
||||||
|
from PIL import Image
|
||||||
|
|
||||||
|
from app.config import settings
|
||||||
|
from app.services import storage_service, thumbnails
|
||||||
|
|
||||||
|
|
||||||
|
def png(width, height, colour=(200, 30, 40)):
|
||||||
|
buffer = io.BytesIO()
|
||||||
|
Image.new("RGB", (width, height), colour).save(buffer, format="PNG")
|
||||||
|
return buffer.getvalue()
|
||||||
|
|
||||||
|
|
||||||
|
class ThumbnailTests(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
self.root = tempfile.TemporaryDirectory()
|
||||||
|
self.previous = (settings.UPLOAD_DIR, settings.STORAGE_BACKEND)
|
||||||
|
settings.UPLOAD_DIR = self.root.name
|
||||||
|
# Pinned local, so the test is about resizing rather than about which
|
||||||
|
# bucket this container happens to be pointed at.
|
||||||
|
settings.STORAGE_BACKEND = "local"
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
settings.UPLOAD_DIR, settings.STORAGE_BACKEND = self.previous
|
||||||
|
self.root.cleanup()
|
||||||
|
|
||||||
|
def put(self, key, data):
|
||||||
|
storage_service.save(key, data)
|
||||||
|
return key
|
||||||
|
|
||||||
|
def test_a_wide_image_comes_back_narrower_and_as_webp(self):
|
||||||
|
key = self.put("questions/big.png", png(1600, 900))
|
||||||
|
small = thumbnails.get(key, 256)
|
||||||
|
self.assertIsNotNone(small)
|
||||||
|
image = Image.open(io.BytesIO(small))
|
||||||
|
self.assertEqual(image.width, 256)
|
||||||
|
self.assertEqual(image.format, "WEBP")
|
||||||
|
# The aspect ratio is kept rather than the height being guessed.
|
||||||
|
self.assertEqual(image.height, round(900 * 256 / 1600))
|
||||||
|
self.assertLess(len(small), len(storage_service.load(key)))
|
||||||
|
|
||||||
|
def test_it_is_made_once_and_kept_beside_the_original(self):
|
||||||
|
key = self.put("questions/kept.png", png(1200, 600))
|
||||||
|
first = thumbnails.get(key, 640)
|
||||||
|
self.assertIsNotNone(first)
|
||||||
|
stored = storage_service.load(thumbnails.thumb_key(key, 640))
|
||||||
|
self.assertEqual(stored, first)
|
||||||
|
# Removing the original must not change what a stored derivative
|
||||||
|
# returns: the second reader is served from the copy, not from a resize.
|
||||||
|
storage_service.delete(key)
|
||||||
|
self.assertEqual(thumbnails.get(key, 640), first)
|
||||||
|
|
||||||
|
def test_a_small_image_is_never_enlarged(self):
|
||||||
|
key = self.put("questions/small.png", png(180, 120))
|
||||||
|
# None means "there is nothing smaller worth serving", and the caller
|
||||||
|
# sends the original. Scaling up invents detail and costs bytes to do it.
|
||||||
|
self.assertIsNone(thumbnails.get(key, 256))
|
||||||
|
self.assertIsNone(thumbnails.get(key, 640))
|
||||||
|
|
||||||
|
def test_only_the_two_widths_exist(self):
|
||||||
|
key = self.put("questions/widths.png", png(1600, 900))
|
||||||
|
self.assertEqual(thumbnails.WIDTHS, (256, 640))
|
||||||
|
for width in (1, 255, 512, 641, 4000):
|
||||||
|
self.assertIsNone(thumbnails.get(key, width))
|
||||||
|
|
||||||
|
def test_what_is_not_an_image_is_left_alone(self):
|
||||||
|
self.assertFalse(thumbnails.is_resizable("documents/handbook.pdf"))
|
||||||
|
self.assertFalse(thumbnails.is_resizable("figures/diagram.svg"))
|
||||||
|
key = self.put("documents/handbook.pdf", b"%PDF-1.4 not really")
|
||||||
|
self.assertIsNone(thumbnails.get(key, 256))
|
||||||
|
|
||||||
|
def test_a_file_that_is_not_the_image_it_claims_is_not_an_error(self):
|
||||||
|
key = self.put("questions/broken.png", b"this is not a PNG")
|
||||||
|
self.assertIsNone(thumbnails.get(key, 256))
|
||||||
|
|
||||||
|
def test_forgetting_removes_every_derivative(self):
|
||||||
|
key = self.put("questions/gone.png", png(1600, 900))
|
||||||
|
for width in thumbnails.WIDTHS:
|
||||||
|
self.assertIsNotNone(thumbnails.get(key, width))
|
||||||
|
thumbnails.forget(key)
|
||||||
|
for width in thumbnails.WIDTHS:
|
||||||
|
self.assertIsNone(storage_service.load(thumbnails.thumb_key(key, width)))
|
||||||
|
|
||||||
|
def test_a_sideways_photograph_is_turned_the_right_way_up(self):
|
||||||
|
# Orientation 6 is "rotate 90° clockwise to view", which is how every
|
||||||
|
# phone stores a portrait photograph. A thumbnail made without reading
|
||||||
|
# it is a sideways thumbnail.
|
||||||
|
buffer = io.BytesIO()
|
||||||
|
image = Image.new("RGB", (1200, 600), (10, 20, 30))
|
||||||
|
exif = image.getexif()
|
||||||
|
exif[274] = 6
|
||||||
|
image.save(buffer, format="JPEG", exif=exif)
|
||||||
|
key = self.put("questions/sideways.jpg", buffer.getvalue())
|
||||||
|
small = Image.open(io.BytesIO(thumbnails.get(key, 256)))
|
||||||
|
self.assertEqual(small.width, 256)
|
||||||
|
self.assertGreater(small.height, small.width)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
26
docs/TODO.md
26
docs/TODO.md
|
|
@ -251,16 +251,22 @@ Captured so nothing is lost while the article writing runs.
|
||||||
whether a code is valid before the account is made.
|
whether a code is valid before the account is made.
|
||||||
- [x] **Settings, properly** — done 2026-09-11: People and AI models rebuilt natively, AdminPage deleted.
|
- [x] **Settings, properly** — done 2026-09-11: People and AI models rebuilt natively, AdminPage deleted.
|
||||||
- [x] **Comments backend removed** — done 2026-09-11: router, model and table dropped.
|
- [x] **Comments backend removed** — done 2026-09-11: router, model and table dropped.
|
||||||
- [ ] **Image thumbnails.** The ped-ai design, read 2026-09-11: allow-listed
|
- [x] **Image thumbnails** — done 2026-09-12. `?w=256` and `?w=640` on
|
||||||
widths only (256 and 640 — any other `?w=` is refused, so the endpoint
|
`/uploads/...`, any other width refused with a 400 rather than honoured.
|
||||||
cannot be turned into a resize-on-demand CPU sink), EXIF rotate, resize
|
EXIF rotate, never enlarged, WebP q82, derivatives under `thumbs/{width}/{key}`
|
||||||
without enlarging, WebP q82, derivatives stored in the same bucket under
|
in the same bucket so credentials, lifecycle and backup are unchanged, and
|
||||||
a `thumbs/{id}/{width}` prefix so credentials, lifecycle and backup are
|
best effort throughout — a file that will not decode serves its original
|
||||||
unchanged, generated best-effort so a failed preview never fails the
|
rather than failing. Pillow, not sharp. No Caddy caching: these are behind
|
||||||
upload. **Pillow, not sharp** — sharp is Node and this backend is Python;
|
authentication and stay `private, no-store`; the win is the byte count.
|
||||||
Pillow 12.3 is already installed and does the same three things. **No
|
Asked for by the media grid, the figure strip's compact thumbnails, the
|
||||||
Caddy caching**: these are behind auth, ped-ai serves its own
|
figure manager and the quiz editor.
|
||||||
`private, no-store` for that reason, and the win is the 256px WebP.
|
**Loose end:** Pillow is not pinned in `backend/requirements.txt`. It
|
||||||
|
arrives transitively via PyMuPDF and is 12.3.0 in the image today. Adding
|
||||||
|
the pin invalidates the pip layer, and rebuilding that layer now fails
|
||||||
|
outright because `litellm==1.28.13` has been withdrawn from PyPI. Pinning
|
||||||
|
Pillow therefore means re-pinning litellm first, which is a deliberate
|
||||||
|
upgrade of the AI layer and not a side effect to slip into a thumbnail
|
||||||
|
change.
|
||||||
- [ ] ~~Image thumbnails and caching~~ — you mentioned a tool from the ped-ai
|
- [ ] ~~Image thumbnails and caching~~ — you mentioned a tool from the ped-ai
|
||||||
work that generates thumbnails and caches through Caddy so images load
|
work that generates thumbnails and caches through Caddy so images load
|
||||||
fast, click to open full size, same bucket, and no straightforward
|
fast, click to open full size, same bucket, and no straightforward
|
||||||
|
|
|
||||||
|
|
@ -109,7 +109,7 @@ export default function FigureManager({ questionId }) {
|
||||||
<ul className="fm-list">
|
<ul className="fm-list">
|
||||||
{inRole.map((figure, index) => (
|
{inRole.map((figure, index) => (
|
||||||
<li key={figure.id} className="fm-item">
|
<li key={figure.id} className="fm-item">
|
||||||
<img src={uploadUrl(figure.path)} alt={figure.caption || figure.label}
|
<img src={uploadUrl(figure.path, undefined, 256)} alt={figure.caption || figure.label}
|
||||||
onError={e => { e.target.style.visibility = 'hidden' }} />
|
onError={e => { e.target.style.visibility = 'hidden' }} />
|
||||||
|
|
||||||
<div className="fm-meta">
|
<div className="fm-meta">
|
||||||
|
|
|
||||||
|
|
@ -40,7 +40,8 @@ export default function FigureStrip({ figures, attemptId, size = 'full', label }
|
||||||
aria-label={`Open ${[figure.label, figure.caption].filter(Boolean).join(': ')
|
aria-label={`Open ${[figure.label, figure.caption].filter(Boolean).join(': ')
|
||||||
|| `figure ${index + 1}`}`}
|
|| `figure ${index + 1}`}`}
|
||||||
onClick={() => setOpen(index)}>
|
onClick={() => setOpen(index)}>
|
||||||
<img src={uploadUrl(figure.path, attemptId)} alt={figure.caption || figure.label || ''}
|
<img src={uploadUrl(figure.path, attemptId, size === 'compact' ? 256 : 640)}
|
||||||
|
alt={figure.caption || figure.label || ''}
|
||||||
loading="lazy" onError={e => { e.currentTarget.style.visibility = 'hidden' }} />
|
loading="lazy" onError={e => { e.currentTarget.style.visibility = 'hidden' }} />
|
||||||
<span className="fs-cap">
|
<span className="fs-cap">
|
||||||
{/* Only what somebody wrote. An unlabelled figure says
|
{/* Only what somebody wrote. An unlabelled figure says
|
||||||
|
|
|
||||||
|
|
@ -8,7 +8,9 @@ import { uploadUrl } from '../utils/uploads'
|
||||||
* the element; everything else about the tile stays identical.
|
* the element; everything else about the tile stays identical.
|
||||||
*/
|
*/
|
||||||
export default function MediaTile({ item, className = '', controls = true }) {
|
export default function MediaTile({ item, className = '', controls = true }) {
|
||||||
const src = uploadUrl(item.path)
|
// A tile, not a plate. The server sends the original back when there is
|
||||||
|
// nothing smaller worth sending, so this is safe for every kind of file.
|
||||||
|
const src = uploadUrl(item.path, undefined, 256)
|
||||||
if (item.kind === 'audio') {
|
if (item.kind === 'audio') {
|
||||||
return <audio className={className} src={src} controls={controls} preload="none" />
|
return <audio className={className} src={src} controls={controls} preload="none" />
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,7 @@ function ImagePreview({ label, path }) {
|
||||||
</div>
|
</div>
|
||||||
<div style={{ border: '1px solid var(--border)', borderRadius: 10, padding: 8, background: 'var(--card-bg)' }}>
|
<div style={{ border: '1px solid var(--border)', borderRadius: 10, padding: 8, background: 'var(--card-bg)' }}>
|
||||||
<img
|
<img
|
||||||
src={uploadUrl(path)}
|
src={uploadUrl(path, undefined, 256)}
|
||||||
alt={label}
|
alt={label}
|
||||||
style={{ display: 'block', maxWidth: '100%', maxHeight: 260, objectFit: 'contain', margin: '0 auto', borderRadius: 8 }}
|
style={{ display: 'block', maxWidth: '100%', maxHeight: 260, objectFit: 'contain', margin: '0 auto', borderRadius: 8 }}
|
||||||
/>
|
/>
|
||||||
|
|
|
||||||
|
|
@ -1,15 +1,25 @@
|
||||||
|
// The only widths the server will make. Asking for anything else is refused
|
||||||
|
// rather than honoured, so this list and the server's must agree — they are the
|
||||||
|
// same two numbers on purpose, and adding a third means adding it there first.
|
||||||
|
export const THUMB_WIDTHS = [256, 640]
|
||||||
|
|
||||||
// Keep the editor's relative/absolute URL behavior; attempt context is local only.
|
// Keep the editor's relative/absolute URL behavior; attempt context is local only.
|
||||||
export function uploadUrl(path, attemptId) {
|
export function uploadUrl(path, attemptId, width) {
|
||||||
if (!path) return ''
|
if (!path) return ''
|
||||||
const value = /^(https?:)?\/\//i.test(path) || path.startsWith('/uploads/') ? path : `/uploads/${path}`
|
const value = /^(https?:)?\/\//i.test(path) || path.startsWith('/uploads/') ? path : `/uploads/${path}`
|
||||||
let url
|
let url
|
||||||
try { url = new URL(value, window.location.origin) }
|
try { url = new URL(value, window.location.origin) }
|
||||||
catch { return value } // Legacy invalid URLs must not crash the question/player.
|
catch { return value } // Legacy invalid URLs must not crash the question/player.
|
||||||
if (attemptId != null && url.origin === window.location.origin && url.pathname.startsWith('/uploads/')) {
|
const ours = url.origin === window.location.origin && url.pathname.startsWith('/uploads/')
|
||||||
url.searchParams.set('attempt_id', attemptId)
|
if (!ours) return value
|
||||||
return `${url.pathname}${url.search}${url.hash}`
|
// A width is a request for a smaller copy, not a promise of one: the server
|
||||||
}
|
// sends the original back whenever there is nothing smaller worth sending —
|
||||||
return value
|
// a PDF, an SVG, an image already narrower than asked for.
|
||||||
|
const wanted = THUMB_WIDTHS.includes(width) ? width : null
|
||||||
|
if (attemptId == null && !wanted) return value
|
||||||
|
if (attemptId != null) url.searchParams.set('attempt_id', attemptId)
|
||||||
|
if (wanted) url.searchParams.set('w', wanted)
|
||||||
|
return `${url.pathname}${url.search}${url.hash}`
|
||||||
}
|
}
|
||||||
|
|
||||||
export function markdownImageUrl(src, attemptId) {
|
export function markdownImageUrl(src, attemptId) {
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import { uploadUrl, markdownImageUrl } from './uploads'
|
import { uploadUrl, markdownImageUrl } from './uploads'
|
||||||
|
|
||||||
it('leaves malformed legacy image URLs to normal browser image-error handling', () => {
|
it('leaves malformed legacy image URLs to normal browser image-error handling', () => {
|
||||||
|
|
@ -22,3 +22,33 @@ it('uses the existing upload format and adds attempt context only to same-origin
|
||||||
expect(markdownImageUrl('https://external.example/test.png', 12)).toBe('https://external.example/test.png')
|
expect(markdownImageUrl('https://external.example/test.png', 12)).toBe('https://external.example/test.png')
|
||||||
expect(uploadUrl(null, 12)).toBe('')
|
expect(uploadUrl(null, 12)).toBe('')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('asking for a smaller copy', () => {
|
||||||
|
it('adds the width when it is one the server will make', () => {
|
||||||
|
expect(uploadUrl('questions/stem.png', undefined, 256))
|
||||||
|
.toBe('/uploads/questions/stem.png?w=256')
|
||||||
|
expect(uploadUrl('questions/stem.png', undefined, 640))
|
||||||
|
.toBe('/uploads/questions/stem.png?w=640')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('ignores a width the server would refuse', () => {
|
||||||
|
// Not silently rounded to the nearest allowed size: a page that asks for
|
||||||
|
// 512 wants 512, and the honest answer is the original rather than a
|
||||||
|
// different size it did not ask for.
|
||||||
|
for (const width of [1, 100, 512, 1024, undefined, null, '256']) {
|
||||||
|
expect(uploadUrl('questions/stem.png', undefined, width))
|
||||||
|
.toBe('/uploads/questions/stem.png')
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
it('carries the attempt and the width together', () => {
|
||||||
|
const url = uploadUrl('questions/stem.png', 42, 256)
|
||||||
|
expect(url).toContain('attempt_id=42')
|
||||||
|
expect(url).toContain('w=256')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('leaves somebody else\'s image alone', () => {
|
||||||
|
expect(uploadUrl('https://elsewhere.example/x.png', undefined, 256))
|
||||||
|
.toBe('https://elsewhere.example/x.png')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue