Bilder blockieren nicht mehr die halbe Seite

Warum eine angeklickte Artikelseite nur zur Haelfte lud: Die Bild-Route wird je
Tabellenzeile einmal aufgerufen, und ein Browser haelt je Host nur rund sechs
Verbindungen offen. Was diese Route aufhaelt, legt die ganze Oberflaeche lahm.
Sie hielt gleich dreifach auf:

1. Fehlte die lokale Kopie, holte ``images.ensure`` das Bild MITTEN in der
   Anfrage von der fremden Adresse - bis zu 15 Sekunden, je Zeile, und bei
   jedem Aufruf erneut, weil ein Fehlschlag nirgends vermerkt wurde. Artikel
   aus einem Backup-Import haben genau diese Ausgangslage. Gemessen: 2,01 s
   je Aufruf, dreimal hintereinander. Jetzt: 0,01 s, das Nachholen laeuft nach
   der Antwort im Hintergrund und ein toter Link wird eine Stunde gesperrt.

2. Ein Vorschaubild ist 34x34 Pixel gross - ausgeliefert wurde das Original.
   Gemessen: 2530 KB je Briefmarke, bei 40 Zeilen 99 MB fuer eine Liste. Neu
   erzeugt die Bild-Route (Pillow) eine Vorschau und legt sie daneben ab;
   ``?thumb=1`` liefert sie aus. Gemessen: 0,6 KB.

3. If-None-Match wurde ignoriert. Nach den 5 Minuten Cache-Frist lud der
   Browser jedes Bild komplett neu - das erklaert, warum es "alle 20 Minuten"
   wieder losging. Jetzt 304 ohne Daten.

Dazu im Web: laufende Bildabrufe werden beim Seitenwechsel abgebrochen (sonst
steht die neue Seite hinter Bildern Schlange, die niemand mehr sieht), und wo
``image_version`` bekannt ist, entfaellt die Anfrage fuer bildlose Artikel ganz.

Und der Verbindungsvorrat der Datenbank: 5 (+10) gegen 40 Arbeits-Threads von
FastAPI. Ab der 16. gleichzeitigen Anfrage wartete eine Route stillschweigend
30 Sekunden - von aussen ein haengender Server. Jetzt 10 (+20) mit 10 Sekunden
Frist: lieber ein Fehler als eine halbe Minute Stille.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Scarriffle
2026-08-18 21:14:43 +02:00
parent 4e2dd3e3eb
commit 8d4954610b
13 changed files with 416 additions and 72 deletions

View File

@@ -9,14 +9,25 @@ settings = get_settings()
# SQLite (used in tests) needs a special connect arg; Postgres does not. # SQLite (used in tests) needs a special connect arg; Postgres does not.
connect_args = {} connect_args = {}
pool_args: dict[str, object] = {}
if settings.database_url.startswith("sqlite"): if settings.database_url.startswith("sqlite"):
connect_args = {"check_same_thread": False} connect_args = {"check_same_thread": False}
else:
# FastAPI faehrt Routen ohne ``async`` in einem Arbeitsvorrat von 40
# Threads. Der Standard-Vorrat von SQLAlchemy fasst aber nur 5 (+10)
# Verbindungen ab der 16. gleichzeitigen Anfrage wartet eine Route
# stillschweigend bis zu 30 Sekunden auf eine freie Verbindung. Von aussen
# sieht das aus wie ein haengender Server. Deshalb mehr Verbindungen und
# eine kurze Frist: Ist wirklich keine frei, soll es KRACHEN statt zaeh zu
# werden ein Fehler ist auffindbar, eine halbe Minute Stille nicht.
pool_args = {"pool_size": 10, "max_overflow": 20, "pool_timeout": 10}
engine = create_engine( engine = create_engine(
settings.database_url, settings.database_url,
connect_args=connect_args, connect_args=connect_args,
pool_pre_ping=True, pool_pre_ping=True,
future=True, future=True,
**pool_args,
) )
SessionLocal = sessionmaker(bind=engine, autoflush=False, autocommit=False, future=True) SessionLocal = sessionmaker(bind=engine, autoflush=False, autocommit=False, future=True)

View File

@@ -209,6 +209,9 @@ def _ensure_schema() -> None:
"ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_base VARCHAR(16)", "ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_base VARCHAR(16)",
"ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_count DOUBLE PRECISION", "ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_count DOUBLE PRECISION",
"ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_amount DOUBLE PRECISION", "ALTER TABLE products ADD COLUMN IF NOT EXISTS secondary_amount DOUBLE PRECISION",
# Vorschaubild (Briefmarkengröße) neben dem Original. Bestehende Bilder
# bekommen es beim ersten Abruf; deshalb nullbar und ohne Umzug.
"ALTER TABLE product_images ADD COLUMN IF NOT EXISTS thumb BYTEA",
] ]
with engine.begin() as conn: with engine.begin() as conn:
# Zuerst die Lagerort-ID auf den Code umstellen (einmalig, idempotent), # Zuerst die Lagerort-ID auf den Code umstellen (einmalig, idempotent),

View File

@@ -468,6 +468,11 @@ class ProductImage(Base):
``source_url`` merkt sich, woher das Bild kam daran ist erkennbar, ob eine ``source_url`` merkt sich, woher das Bild kam daran ist erkennbar, ob eine
geänderte ``Product.image_url`` ein neues Bild bedeutet. geänderte ``Product.image_url`` ein neues Bild bedeutet.
``thumb`` ist dasselbe Bild auf Briefmarkengröße. Listen zeigen 34x34 Pixel
je Zeile ohne diese Spalte lud eine Liste mit 40 Einträgen 40 Fotos in
voller Auflösung, also Dutzende Megabyte für ein paar Briefmarken. Nullbar,
weil ältere Zeilen noch keine haben; sie wird beim ersten Abruf nachgezogen.
""" """
__tablename__ = "product_images" __tablename__ = "product_images"
@@ -477,6 +482,7 @@ class ProductImage(Base):
) )
content_type: Mapped[str] = mapped_column(String(64), nullable=False) content_type: Mapped[str] = mapped_column(String(64), nullable=False)
data: Mapped[bytes] = mapped_column(LargeBinary, nullable=False) data: Mapped[bytes] = mapped_column(LargeBinary, nullable=False)
thumb: Mapped[bytes | None] = mapped_column(LargeBinary, nullable=True)
source_url: Mapped[str | None] = mapped_column(String(1024), nullable=True) source_url: Mapped[str | None] = mapped_column(String(1024), nullable=True)
updated_at: Mapped[datetime] = mapped_column( updated_at: Mapped[datetime] = mapped_column(
DateTime(timezone=True), default=_now, onupdate=_now DateTime(timezone=True), default=_now, onupdate=_now

View File

@@ -1,12 +1,21 @@
import difflib import difflib
import re import re
from fastapi import APIRouter, BackgroundTasks, Depends, File, HTTPException, UploadFile, status from fastapi import (
APIRouter,
BackgroundTasks,
Depends,
File,
HTTPException,
Request,
UploadFile,
status,
)
from fastapi.responses import Response from fastapi.responses import Response
from sqlalchemy.orm import Session, joinedload, selectinload from sqlalchemy.orm import Session, joinedload, selectinload
from ..crud import product_to_out, product_tracking, products_to_out_bulk from ..crud import product_to_out, product_tracking, products_to_out_bulk
from ..database import SessionLocal, get_db from ..database import get_db
from ..deps import get_current_user, require_admin from ..deps import get_current_user, require_admin
from ..models import ( from ..models import (
Barcode, Barcode,
@@ -340,6 +349,9 @@ def off_vergleich(
@router.get("/{product_id}/image") @router.get("/{product_id}/image")
def get_product_image( def get_product_image(
product_id: int, product_id: int,
request: Request,
background_tasks: BackgroundTasks,
thumb: bool = False,
db: Session = Depends(get_db), db: Session = Depends(get_db),
_: User = Depends(get_current_user), _: User = Depends(get_current_user),
) -> Response: ) -> Response:
@@ -347,22 +359,38 @@ def get_product_image(
Anders als das Logo verlangt diese Route eine Anmeldung: Aus den Bildern Anders als das Logo verlangt diese Route eine Anmeldung: Aus den Bildern
liesse sich sonst ohne Konto ablesen, was im Vorrat liegt. liesse sich sonst ohne Konto ablesen, was im Vorrat liegt.
Diese Route wird je Tabellenzeile einmal aufgerufen. Sie muss deshalb
schnell sein und darf unter keinen Umstaenden warten siehe Modulkopf von
``services/images.py``. Drei Zusagen:
* ``thumb=true`` liefert das Vorschaubild (~3 KB) statt des Originals.
* Eine unveraenderte Kopie beantwortet sie mit 304, ohne Daten zu senden.
* Fehlt die lokale Kopie, wird sie NACH der Antwort im Hintergrund geholt.
""" """
product = db.get(Product, product_id) product = db.get(Product, product_id)
if product is None: if product is None:
raise HTTPException(status.HTTP_404_NOT_FOUND, "Produkt nicht gefunden") raise HTTPException(status.HTTP_404_NOT_FOUND, "Produkt nicht gefunden")
bild = images.ensure(db, product) bild = db.get(ProductImage, product_id)
if bild is None: if bild is None:
if product.image_url and images.nachholen_faellig(product_id):
background_tasks.add_task(images.nachholen, product_id, product.image_url)
raise HTTPException(status.HTTP_404_NOT_FOUND, "Kein Bild vorhanden") raise HTTPException(status.HTTP_404_NOT_FOUND, "Kein Bild vorhanden")
return Response(
content=bild.data, # Vorschau und Original brauchen VERSCHIEDENE Marken sonst liefert der
media_type=bild.content_type, # Zwischenspeicher des Browsers die Briefmarke fuer das grosse Bild aus.
headers={ marke = f'"bild-{product_id}-{int(bild.updated_at.timestamp())}{"-v" if thumb else ""}"'
"Cache-Control": "private, max-age=300", kopf = {"Cache-Control": "private, max-age=300", "ETag": marke}
"ETag": f'"bild-{product_id}-{int(bild.updated_at.timestamp())}"', if request.headers.get("if-none-match") == marke:
}, return Response(status_code=status.HTTP_304_NOT_MODIFIED, headers=kopf)
)
daten, typ = bild.data, bild.content_type
if thumb:
klein = bild.thumb if bild.thumb is not None else images.vorschau_nachziehen(db, bild)
if klein is not None:
daten, typ = klein, images.THUMB_TYPE
return Response(content=daten, media_type=typ, headers=kopf)
@router.put("/{product_id}/image", response_model=ProductOut) @router.put("/{product_id}/image", response_model=ProductOut)
@@ -396,14 +424,17 @@ async def upload_product_image(
f"Das Bild ist zu groß ({len(data) // 1024} KB). " f"Das Bild ist zu groß ({len(data) // 1024} KB). "
f"Erlaubt sind höchstens {images.MAX_BYTES // 1024} KB.", f"Erlaubt sind höchstens {images.MAX_BYTES // 1024} KB.",
) )
klein = images.vorschau(data, file.content_type)
bild = db.get(ProductImage, product.id) bild = db.get(ProductImage, product.id)
if bild is None: if bild is None:
bild = ProductImage( bild = ProductImage(
product_id=product.id, content_type=file.content_type, data=data, source_url=None product_id=product.id, content_type=file.content_type, data=data,
thumb=klein, source_url=None,
) )
db.add(bild) db.add(bild)
else: else:
bild.content_type, bild.data, bild.source_url = file.content_type, data, None bild.content_type, bild.data, bild.source_url = file.content_type, data, None
bild.thumb = klein
db.commit() db.commit()
db.refresh(product) db.refresh(product)
return product_to_out(db, product) return product_to_out(db, product)
@@ -422,19 +453,6 @@ def delete_product_image(
db.commit() db.commit()
def _store_product_image_bg(product_id: int, url: str) -> None:
"""Bild nach dem Anlegen im Hintergrund holen (eigene Session), damit das
Anlegen nicht auf den (langsamen) Netzwerk-Abruf wartet."""
db = SessionLocal()
try:
product = db.get(Product, product_id)
if product is not None:
images.store(db, product, url)
db.commit()
finally:
db.close()
def _pruefe_zweiteinheit(base_unit, basis, anzahl, menge) -> None: def _pruefe_zweiteinheit(base_unit, basis, anzahl, menge) -> None:
"""Die Zweiteinheit muss vollstaendig und auf eine ANDERE Art zeigen. """Die Zweiteinheit muss vollstaendig und auf eine ANDERE Art zeigen.
@@ -520,7 +538,7 @@ def create_product(
# Bild erst NACH dem Anlegen im Hintergrund holen der Netzwerk-Abruf # Bild erst NACH dem Anlegen im Hintergrund holen der Netzwerk-Abruf
# soll das Anlegen nicht mehrere Sekunden blockieren. Schlägt er fehl, # soll das Anlegen nicht mehrere Sekunden blockieren. Schlägt er fehl,
# bleibt der Artikel trotzdem angelegt (nur ohne Bild). # bleibt der Artikel trotzdem angelegt (nur ohne Bild).
background_tasks.add_task(_store_product_image_bg, product.id, product.image_url) background_tasks.add_task(images.nachholen, product.id, product.image_url)
return product_to_out(db, product) return product_to_out(db, product)

View File

@@ -1,4 +1,4 @@
"""Artikelbilder holen und lokal vorhalten. """Artikelbilder holen, verkleinern und lokal vorhalten.
Open Food Facts liefert nur eine Bild-*Adresse*. Würde die Oberfläche direkt Open Food Facts liefert nur eine Bild-*Adresse*. Würde die Oberfläche direkt
dorthin verlinken, hinge jede Artikelseite an einem fremden Dienst: Das Bild dorthin verlinken, hinge jede Artikelseite an einem fremden Dienst: Das Bild
@@ -9,11 +9,23 @@ eigenen Datenbank ausgeliefert.
Die Bilder liegen in einer eigenen Tabelle und nicht als Spalte an ``products``: Die Bilder liegen in einer eigenen Tabelle und nicht als Spalte an ``products``:
Sonst zöge jede Artikelliste die Blobs mit. Sonst zöge jede Artikelliste die Blobs mit.
**Der Abruf gehört NIE in eine laufende Anfrage.** Er hing früher in ``ensure``
mitten in ``GET /products/{id}/image`` und diese Route wird je Tabellenzeile
einmal aufgerufen. Ein toter Bildlink kostete damit 15 Sekunden pro Zeile, bei
jedem Aufruf aufs Neue; die ~6 Verbindungen, die ein Browser je Host offen hält,
waren belegt, und alle übrigen Abfragen der Seite standen dahinter Schlange. Die
Seite lud dann „nur zur Hälfte". Nachgeholt wird deshalb nur noch im
Hintergrund, und ein Fehlschlag wird gemerkt (siehe ``nachholen_faellig``).
""" """
from __future__ import annotations from __future__ import annotations
import io
import time
import httpx import httpx
from sqlalchemy import update
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from ..models import Product, ProductImage from ..models import Product, ProductImage
@@ -28,6 +40,79 @@ ALLOWED_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"}
TIMEOUT_SECONDS = 15.0 TIMEOUT_SECONDS = 15.0
# Vorschaubilder erscheinen mit 34 CSS-Pixeln Kantenlaenge. 96 deckt auch einen
# dreifach aufloesenden Bildschirm ab und bleibt bei ~3 KB je Bild.
THUMB_PIXEL = 96
THUMB_TYPE = "image/jpeg"
# Ein toter Bildlink darf nicht bei jedem Seitenaufruf erneut ins Netz gehen.
# Der Merker liegt im Arbeitsspeicher: Er darf ruhig beim Neustart verfallen
# dann wird eben einmal erneut versucht, was genau richtig ist, wenn der Dienst
# zwischenzeitlich wieder da ist.
SPERRE_SEKUNDEN = 3600.0
_gesperrt: dict[int, float] = {}
def sperre_zuruecksetzen() -> None:
"""Nur fuer Tests der Merker ist sonst modulweit und ueberlebt sie."""
_gesperrt.clear()
def nachholen_faellig(product_id: int) -> bool:
"""Darf fuer diesen Artikel (wieder) ins Netz gegriffen werden?"""
bis = _gesperrt.get(product_id)
return bis is None or time.monotonic() >= bis
def vorschau(daten: bytes, content_type: str) -> bytes | None:
"""Kleines Vorschaubild erzeugen. ``None``, wenn es nicht geht.
Ohne das laedt eine Liste mit 40 Zeilen bis zu 40 Handy-Fotos in voller
Aufloesung Dutzende Megabyte, um 34x34 Pixel je Zeile darzustellen.
Pillow wird erst hier importiert und jeder Fehler geschluckt: Ein Bild, das
sich nicht verkleinern laesst, darf die Seite nicht mitreissen. Der Aufrufer
liefert dann eben das Original aus langsam, aber richtig.
"""
try:
from PIL import Image, ImageOps
except ImportError: # pragma: no cover
return None
try:
with Image.open(io.BytesIO(daten)) as roh:
# draft() verkleinert JPEG schon beim Dekodieren aus 2 MB werden
# so Millisekunden statt eines vollen Bildaufbaus im Speicher.
roh.draft("RGB", (THUMB_PIXEL, THUMB_PIXEL))
# Handy-Fotos tragen ihre Ausrichtung in den EXIF-Daten. Beim neuen
# Kodieren gehen die verloren ohne diesen Schritt laege jedes
# zweite Vorschaubild quer.
klein = ImageOps.exif_transpose(roh).convert("RGB")
klein.thumbnail((THUMB_PIXEL, THUMB_PIXEL))
puffer = io.BytesIO()
klein.save(puffer, format="JPEG", quality=80, optimize=True)
except Exception: # noqa: BLE001 s.o.
return None
return puffer.getvalue()
def vorschau_nachziehen(db: Session, bild: ProductImage) -> bytes | None:
"""Fehlende Vorschau einmalig erzeugen und ablegen.
Fuer Bilder, die vor dieser Funktion gespeichert wurden. ``updated_at``
bleibt bewusst stehen: Das Bild hat sich nicht geaendert, und der
Zeitstempel ist die Marke, an der Browser und App ihre Kopie erkennen.
"""
neu = vorschau(bild.data, bild.content_type)
if neu is None:
return None
db.execute(
update(ProductImage)
.where(ProductImage.product_id == bild.product_id)
.values(thumb=neu, updated_at=bild.updated_at)
)
db.commit()
return neu
def fetch(url: str) -> tuple[bytes, str] | None: def fetch(url: str) -> tuple[bytes, str] | None:
"""Bild herunterladen. Gibt (Daten, Inhaltstyp) zurück oder None. """Bild herunterladen. Gibt (Daten, Inhaltstyp) zurück oder None.
@@ -68,27 +153,35 @@ def store(db: Session, product: Product, url: str) -> ProductImage | None:
bild = db.get(ProductImage, product.id) bild = db.get(ProductImage, product.id)
if bild is None: if bild is None:
bild = ProductImage( bild = ProductImage(
product_id=product.id, content_type=typ, data=daten, source_url=url product_id=product.id, content_type=typ, data=daten,
thumb=vorschau(daten, typ), source_url=url,
) )
db.add(bild) db.add(bild)
else: else:
bild.content_type, bild.data, bild.source_url = typ, daten, url bild.content_type, bild.data, bild.source_url = typ, daten, url
bild.thumb = vorschau(daten, typ)
return bild return bild
def ensure(db: Session, product: Product) -> ProductImage | None: def nachholen(product_id: int, url: str) -> None:
"""Lokale Kopie zurückgeben und bei Bedarf einmalig nachholen. """Bild nach der Antwort im Hintergrund holen mit eigener Sitzung.
So bekommen auch Artikel ein lokales Bild, die vor dieser Funktion angelegt Laeuft als Hintergrundaufgabe: beim Anlegen eines Artikels und beim ersten
wurden ohne Wanderung über alle Datensätze. Bezahlt wird das mit einer Abruf eines Bildes, dessen lokale Kopie fehlt. Schlaegt der Abruf fehl, wird
einmaligen Verzögerung beim ersten Aufruf. der Artikel fuer eine Weile gesperrt sonst klopft jede Tabellenzeile bei
jedem Seitenaufruf erneut bei einem Dienst an, der nicht antwortet.
""" """
bild = db.get(ProductImage, product.id) from ..database import SessionLocal
if bild is not None:
return bild db = SessionLocal()
if not product.image_url: try:
return None product = db.get(Product, product_id)
bild = store(db, product, product.image_url) if product is None:
if bild is not None: return
db.commit() if store(db, product, url) is None:
return bild _gesperrt[product_id] = time.monotonic() + SPERRE_SEKUNDEN
else:
_gesperrt.pop(product_id, None)
db.commit()
finally:
db.close()

View File

@@ -9,5 +9,8 @@ bcrypt==4.2.1
python-multipart==0.0.20 python-multipart==0.0.20
httpx==0.28.1 httpx==0.28.1
python-dateutil==2.9.0.post0 python-dateutil==2.9.0.post0
# Nur fuer Vorschaubilder (services/images.py). Fehlt Pillow, liefert die
# Bild-Route weiterhin das Original aus - langsam, aber funktionsfaehig.
pillow==11.1.0
pypdf==5.1.0 pypdf==5.1.0
pytest==8.3.4 pytest==8.3.4

View File

@@ -0,0 +1,167 @@
"""Die Bild-Route ist die einzige, die je Tabellenzeile einmal aufgerufen wird.
Was hier schiefgeht, vervielfacht sich also mit der Länge der Liste und weil
ein Browser nur ~6 Verbindungen je Host offen hält, warten alle übrigen
Abfragen der Seite dahinter. Genau daran lag das halb geladene Bild der
Artikelseite. Deshalb halten diese Tests drei Zusagen fest:
1. Die Auslieferung geht **nie** ins Netz.
2. Ein unverändertes Bild wird **nicht erneut** übertragen.
3. Ein Vorschaubild ist ein **kleines** Bild, nicht das Original.
"""
import io
import pytest
from fastapi import BackgroundTasks, HTTPException
from app.models import BaseUnit, Product, ProductImage
from app.routers.products import get_product_image
from app.services import images
class FakeRequest:
"""Nur die Kopfzeilen mehr braucht die Route nicht."""
def __init__(self, **kopf):
self.headers = {k.lower().replace("_", "-"): v for k, v in kopf.items()}
def _artikel(db, url=None):
p = Product(name="Pesto", base_unit=BaseUnit.gram, image_url=url)
db.add(p)
db.commit()
db.refresh(p)
return p
def _jpeg(breite=1600, hoehe=1200) -> bytes:
"""Ein Bild in Handy-Größe kein Platzhalter, sonst misst der Test nichts."""
Image = pytest.importorskip("PIL.Image")
bild = Image.new("RGB", (breite, hoehe))
# Rauschen, damit JPEG nicht auf ein paar Byte zusammenfällt.
bild.putdata([(x * 7 % 256, x * 13 % 256, x * 29 % 256) for x in range(breite * hoehe)])
puffer = io.BytesIO()
bild.save(puffer, format="JPEG", quality=90)
return puffer.getvalue()
def _hole(db, artikel, **kwargs):
return get_product_image(
artikel.id,
request=kwargs.pop("request", FakeRequest()),
background_tasks=kwargs.pop("background_tasks", BackgroundTasks()),
thumb=kwargs.pop("thumb", False),
db=db,
_=None,
)
def test_auslieferung_geht_nie_ins_netz(db, monkeypatch):
"""Der eigentliche Fehler: ``ensure`` holte das Bild MITTEN in der Anfrage.
Bei einem toten Link kostete das bis zu 15 Sekunden je Tabellenzeile, und
bei jedem Aufruf aufs Neue, weil ein Fehlschlag nirgends vermerkt wurde.
"""
monkeypatch.setattr(
images, "fetch", lambda url: pytest.fail("Die Auslieferung darf nicht ins Netz")
)
artikel = _artikel(db, "https://example.org/pesto.jpg")
with pytest.raises(HTTPException) as fehler:
_hole(db, artikel)
assert fehler.value.status_code == 404
def test_fehlende_kopie_wird_im_hintergrund_nachgeholt(db, monkeypatch):
"""Nachgeholt wird weiterhin nur eben nach der Antwort."""
monkeypatch.setattr(images, "fetch", lambda url: (b"x", "image/png"))
artikel = _artikel(db, "https://example.org/pesto.jpg")
aufgaben = BackgroundTasks()
with pytest.raises(HTTPException):
_hole(db, artikel, background_tasks=aufgaben)
assert [t.func for t in aufgaben.tasks] == [images.nachholen]
def test_ohne_bildadresse_gibt_es_nichts_nachzuholen(db, monkeypatch):
monkeypatch.setattr(
images, "fetch", lambda url: pytest.fail("Es gibt keine Adresse zum Holen")
)
aufgaben = BackgroundTasks()
with pytest.raises(HTTPException):
_hole(db, _artikel(db), background_tasks=aufgaben)
assert aufgaben.tasks == []
def test_toter_link_wird_nicht_bei_jedem_aufruf_erneut_versucht(db, monkeypatch):
"""Ohne Sperre klopft jede Tabellenzeile bei jedem Seitenaufruf erneut an."""
images.sperre_zuruecksetzen()
monkeypatch.setattr(images, "fetch", lambda url: None) # Abruf schlägt fehl
artikel = _artikel(db, "https://example.org/weg.jpg")
erste = BackgroundTasks()
with pytest.raises(HTTPException):
_hole(db, artikel, background_tasks=erste)
assert len(erste.tasks) == 1
# Die Hintergrundaufgabe oeffnet sonst eine eigene Sitzung hier soll es
# die des Fixtures sein, und schliessen darf sie sie nicht.
monkeypatch.setattr(db, "close", lambda: None)
monkeypatch.setattr("app.database.SessionLocal", lambda: db)
images.nachholen(artikel.id, artikel.image_url) # …und scheitert
zweite = BackgroundTasks()
with pytest.raises(HTTPException):
_hole(db, artikel, background_tasks=zweite)
assert zweite.tasks == []
def test_unveraendertes_bild_wird_nicht_erneut_uebertragen(db):
"""Ohne 304 lädt der Browser nach Ablauf der Frist jedes Bild komplett neu."""
artikel = _artikel(db)
db.add(ProductImage(product_id=artikel.id, content_type="image/jpeg", data=_jpeg()))
db.commit()
erste = _hole(db, artikel)
marke = erste.headers["etag"]
assert len(erste.body) > 0
zweite = _hole(db, artikel, request=FakeRequest(if_none_match=marke))
assert zweite.status_code == 304
assert zweite.body == b""
def test_vorschaubild_ist_ein_bruchteil_des_originals(db):
"""34x34 Pixel auf dem Schirm dürfen nicht 2 MB auf der Leitung sein."""
original = _jpeg()
artikel = _artikel(db)
db.add(ProductImage(product_id=artikel.id, content_type="image/jpeg", data=original))
db.commit()
gross = _hole(db, artikel)
klein = _hole(db, artikel, thumb=True)
assert len(gross.body) == len(original)
assert len(klein.body) < len(original) / 20
# Verschiedene Inhalte brauchen verschiedene Marken, sonst liefert der
# Zwischenspeicher des Browsers das eine fuer das andere aus.
assert klein.headers["etag"] != gross.headers["etag"]
def test_vorschau_bleibt_erhalten_und_ruehrt_den_zeitstempel_nicht_an(db):
"""Sie wird beim ersten Abruf nachgezogen aber das Bild ist nicht neu."""
artikel = _artikel(db)
bild = ProductImage(product_id=artikel.id, content_type="image/jpeg", data=_jpeg())
db.add(bild)
db.commit()
db.refresh(bild)
vorher = bild.updated_at
_hole(db, artikel, thumb=True)
db.expire_all()
frisch = db.get(ProductImage, artikel.id)
assert frisch.thumb is not None
assert frisch.updated_at == vorher

View File

@@ -29,9 +29,11 @@ def test_image_version_spiegelt_bildzeitstempel(db):
assert product_to_out(db, p).image_version == int(img.updated_at.timestamp()) assert product_to_out(db, p).image_version == int(img.updated_at.timestamp())
def test_ohne_bildadresse_gibt_es_nichts_zu_holen(db, monkeypatch): def _im_hintergrund(db, monkeypatch, artikel):
monkeypatch.setattr(images, "fetch", lambda url: (_ for _ in ()).throw(AssertionError)) """``nachholen`` laeuft mit eigener Sitzung - im Test mit der des Fixtures."""
assert images.ensure(db, _artikel(db)) is None monkeypatch.setattr(db, "close", lambda: None)
monkeypatch.setattr("app.database.SessionLocal", lambda: db)
images.nachholen(artikel.id, artikel.image_url)
def test_bild_wird_einmal_geholt_und_danach_lokal_geliefert(db, monkeypatch): def test_bild_wird_einmal_geholt_und_danach_lokal_geliefert(db, monkeypatch):
@@ -42,22 +44,25 @@ def test_bild_wird_einmal_geholt_und_danach_lokal_geliefert(db, monkeypatch):
return EIN_PIXEL, "image/png" return EIN_PIXEL, "image/png"
monkeypatch.setattr(images, "fetch", gefaelscht) monkeypatch.setattr(images, "fetch", gefaelscht)
images.sperre_zuruecksetzen()
artikel = _artikel(db, "https://example.org/pesto.png") artikel = _artikel(db, "https://example.org/pesto.png")
erstes = images.ensure(db, artikel) _im_hintergrund(db, monkeypatch, artikel)
erstes = db.get(ProductImage, artikel.id)
assert erstes is not None assert erstes is not None
assert erstes.data == EIN_PIXEL assert erstes.data == EIN_PIXEL
assert erstes.content_type == "image/png" assert erstes.content_type == "image/png"
# Zweiter Aufruf darf nicht erneut ins Netz gehen - genau das ist der Zweck. # Ab jetzt liegt die Kopie lokal - die Bild-Route greift gar nicht mehr zu
images.ensure(db, artikel) # ``nachholen``, siehe test_bild_auslieferung.py.
assert aufrufe == ["https://example.org/pesto.png"] assert aufrufe == ["https://example.org/pesto.png"]
def test_ein_nicht_erreichbares_bild_bleibt_folgenlos(db, monkeypatch): def test_ein_nicht_erreichbares_bild_bleibt_folgenlos(db, monkeypatch):
monkeypatch.setattr(images, "fetch", lambda url: None) monkeypatch.setattr(images, "fetch", lambda url: None)
images.sperre_zuruecksetzen()
artikel = _artikel(db, "https://example.org/weg.png") artikel = _artikel(db, "https://example.org/weg.png")
assert images.ensure(db, artikel) is None _im_hintergrund(db, monkeypatch, artikel)
assert db.get(ProductImage, artikel.id) is None assert db.get(ProductImage, artikel.id) is None

View File

@@ -134,8 +134,12 @@ actor APIClient {
} }
/// Aktuelles Artikelfoto laden (mit Anmeldung). Gibt nil bei 404 zurück. /// Aktuelles Artikelfoto laden (mit Anmeldung). Gibt nil bei 404 zurück.
func productImage(id: Int) async throws -> Data? { ///
let request = try makeRequest("/products/\(id)/image") /// `thumb: true` holt die Briefmarken-Fassung (wenige KB statt mehrerer MB).
/// Für Listen ist das Pflicht, nicht Kür: Ein Vorschaubild ist 34x34 Punkte
/// groß, und `preload` läuft über die ganze Artikelliste.
func productImage(id: Int, thumb: Bool = false) async throws -> Data? {
let request = try makeRequest("/products/\(id)/image" + (thumb ? "?thumb=1" : ""))
let (data, response) = try await URLSession.shared.data(for: request) let (data, response) = try await URLSession.shared.data(for: request)
if let http = response as? HTTPURLResponse, http.statusCode == 404 { return nil } if let http = response as? HTTPURLResponse, http.statusCode == 404 { return nil }
try check(response, data: data) try check(response, data: data)

View File

@@ -79,7 +79,7 @@ final class ProductImageCache {
for p in products { for p in products {
guard let sv = p.imageVersion else { markEmpty(p.id); continue } guard let sv = p.imageVersion else { markEmpty(p.id); continue }
if isFresh(p.id, sv) { continue } if isFresh(p.id, sv) { continue }
if let data = try? await APIClient.shared.productImage(id: p.id), if let data = try? await APIClient.shared.productImage(id: p.id, thumb: true),
let ui = UIImage(data: data) { let ui = UIImage(data: data) {
store(ui, data: data, version: sv, for: p.id) store(ui, data: data, version: sv, for: p.id)
} else { } else {
@@ -119,7 +119,7 @@ struct ProductThumb: View {
return return
} }
if ProductImageCache.shared.isKnownEmpty(productId) { return } if ProductImageCache.shared.isKnownEmpty(productId) { return }
guard let data = try? await APIClient.shared.productImage(id: productId), guard let data = try? await APIClient.shared.productImage(id: productId, thumb: true),
let ui = UIImage(data: data) else { let ui = UIImage(data: data) else {
ProductImageCache.shared.markEmpty(productId) ProductImageCache.shared.markEmpty(productId)
return return

View File

@@ -105,11 +105,18 @@ function zeitstempel() {
* *
* Gibt null zurück, wenn es kein Bild gibt; das ist der Normalfall und kein * Gibt null zurück, wenn es kein Bild gibt; das ist der Normalfall und kein
* Fehler. Wer die URL nicht mehr braucht, gibt sie mit URL.revokeObjectURL frei. * Fehler. Wer die URL nicht mehr braucht, gibt sie mit URL.revokeObjectURL frei.
*
* `signal` bricht den Abruf ab. Das ist bei Bildern kein Feinschliff, sondern
* nötig: Ein Browser hält je Host nur rund sechs Verbindungen offen. Eine Liste
* mit vierzig Vorschaubildern belegt sie alle und wer währenddessen eine Zeile
* anklickt, dessen Artikelseite steht dahinter Schlange, obwohl die Bilder
* niemanden mehr interessieren.
*/ */
export async function authorizedObjectUrl(path) { export async function authorizedObjectUrl(path, signal) {
const token = getToken(); const token = getToken();
const resp = await fetch(`${API_BASE}${path}`, { const resp = await fetch(`${API_BASE}${path}`, {
headers: token ? { Authorization: `Bearer ${token}` } : {}, headers: token ? { Authorization: `Bearer ${token}` } : {},
signal,
}); });
if (!resp.ok) return null; if (!resp.ok) return null;
return URL.createObjectURL(await resp.blob()); return URL.createObjectURL(await resp.blob());

View File

@@ -6,20 +6,33 @@ import Lightbox from "./Lightbox";
/** /**
* Lädt das Artikelbild aus der eigenen Datenbank. * Lädt das Artikelbild aus der eigenen Datenbank.
* *
* Die Objekt-URL wird beim Verlassen wieder freigegeben, sonst hält der Browser
* jedes angesehene Bild bis zum Neuladen der Seite im Speicher.
*
* ``onLoaded`` meldet, ob ein Bild vorliegt so kann die Artikelseite z.B. den * ``onLoaded`` meldet, ob ein Bild vorliegt so kann die Artikelseite z.B. den
* „Entfernen"-Knopf ausblenden, wenn es nichts zu entfernen gibt. * „Entfernen"-Knopf ausblenden, wenn es nichts zu entfernen gibt.
*
* Drei Dinge sind hier wichtiger, als sie aussehen jedes einzelne hat schon
* dafür gesorgt, dass eine angeklickte Artikelseite nur halb geladen ankam:
*
* 1. **Abbrechen beim Verlassen.** Ein Browser hält je Host rund sechs
* Verbindungen offen. Wer eine Liste mit vierzig Vorschaubildern verlässt,
* dessen neue Seite steht sonst hinter vierzig Bildern Schlange, die niemand
* mehr sieht.
* 2. **Vorschaubilder statt Originalen.** In der Tabelle sind es 34x34 Pixel;
* das Original ist ein Handy-Foto von mehreren Megabyte.
* 3. **Gar nicht erst fragen.** ``version === null`` heisst „dieser Artikel hat
* nachweislich kein Bild" (aus ``image_version``). Dann bleibt die Anfrage
* weg statt in einem 404 zu enden.
*/ */
// Modulweiter Cache: Produkt-ID+Version → Objekt-URL (oder null = „kein Bild"). // Modulweiter Cache: "Produkt-ID:Art:Version" → Objekt-URL (oder null = „kein
// So werden Thumbnails beim erneuten Öffnen einer Liste nicht jedes Mal neu // Bild"). So werden Thumbnails beim erneuten Öffnen einer Liste nicht jedes Mal
// geholt. Die Objekt-URLs bleiben absichtlich bestehen (werden nicht widerrufen), // neu geholt. Die Objekt-URLs bleiben absichtlich bestehen (werden nicht
// bis eine neue Version dieselbe ID ablöst. // widerrufen), bis eine neue Version dieselbe ID ablöst.
const bildCache = new Map(); const bildCache = new Map();
function useBildUrl(productId, version = 0, onLoaded) { function useBildUrl(productId, version, klein = false, onLoaded = undefined) {
const key = `${productId}:${version}`; // null = es gibt keins. undefined/0 = wir wissen es nicht und fragen nach.
const bekanntOhneBild = version === null;
const art = klein ? "v" : "g";
const key = `${productId}:${art}:${version || 0}`;
const [url, setUrl] = useState(() => (bildCache.has(key) ? bildCache.get(key) : null)); const [url, setUrl] = useState(() => (bildCache.has(key) ? bildCache.get(key) : null));
// Über einen Ref, damit ein bei jedem Render neu erzeugtes onLoaded den Effekt // Über einen Ref, damit ein bei jedem Render neu erzeugtes onLoaded den Effekt
// nicht erneut auslöst. // nicht erneut auslöst.
@@ -28,6 +41,11 @@ function useBildUrl(productId, version = 0, onLoaded) {
useEffect(() => { useEffect(() => {
if (!productId) return undefined; if (!productId) return undefined;
if (bekanntOhneBild) {
onLoadedRef.current?.(false);
setUrl(null);
return undefined;
}
if (bildCache.has(key)) { if (bildCache.has(key)) {
const cached = bildCache.get(key); const cached = bildCache.get(key);
onLoadedRef.current?.(Boolean(cached)); onLoadedRef.current?.(Boolean(cached));
@@ -35,17 +53,22 @@ function useBildUrl(productId, version = 0, onLoaded) {
return undefined; return undefined;
} }
let abgebrochen = false; let abgebrochen = false;
const steuerung = new AbortController();
// ``version`` haengt einen Wert an die Adresse, damit der Browser nach dem // ``version`` haengt einen Wert an die Adresse, damit der Browser nach dem
// Speichern nicht seine zwischengespeicherte Fassung ausliefert. // Speichern nicht seine zwischengespeicherte Fassung ausliefert.
authorizedObjectUrl(`/products/${productId}/image${version ? `?v=${version}` : ""}`) const frage = [klein ? "thumb=1" : "", version ? `v=${version}` : ""].filter(Boolean);
authorizedObjectUrl(
`/products/${productId}/image${frage.length ? `?${frage.join("&")}` : ""}`,
steuerung.signal,
)
.then((neu) => { .then((neu) => {
if (abgebrochen) { if (abgebrochen) {
if (neu) URL.revokeObjectURL(neu); if (neu) URL.revokeObjectURL(neu);
return; return;
} }
// Alte Version derselben ID freigeben, dann neu cachen. // Alte Version derselben ID und Art freigeben, dann neu cachen.
const alt = `${productId}:`; const alt = `${productId}:${art}:`;
for (const k of bildCache.keys()) { for (const k of bildCache.keys()) {
if (k !== key && k.startsWith(alt)) { if (k !== key && k.startsWith(alt)) {
const u = bildCache.get(k); const u = bildCache.get(k);
@@ -62,8 +85,9 @@ function useBildUrl(productId, version = 0, onLoaded) {
}); });
// Objekt-URL NICHT widerrufen sie bleibt im Cache für den nächsten Mount. // Objekt-URL NICHT widerrufen sie bleibt im Cache für den nächsten Mount.
return () => { abgebrochen = true; }; // Der Abruf selbst schon: siehe Punkt 1 oben.
}, [productId, version, key]); return () => { abgebrochen = true; steuerung.abort(); };
}, [productId, version, key, art, klein, bekanntOhneBild]);
return url; return url;
} }
@@ -75,7 +99,7 @@ function useBildUrl(productId, version = 0, onLoaded) {
* Artikel, der nie ein Bild bekommen wird, wäre nur Lärm. * Artikel, der nie ein Bild bekommen wird, wäre nur Lärm.
*/ */
export default function ProduktBild({ productId, alt, className = "", version = 0, onLoaded }) { export default function ProduktBild({ productId, alt, className = "", version = 0, onLoaded }) {
const url = useBildUrl(productId, version, onLoaded); const url = useBildUrl(productId, version, false, onLoaded);
const [zoom, setZoom] = useState(false); const [zoom, setZoom] = useState(false);
if (!url) return null; if (!url) return null;
return ( return (
@@ -92,9 +116,12 @@ export default function ProduktBild({ productId, alt, className = "", version =
* *
* Hier gibt es sehr wohl einen Platzhalter: In einer Tabellenspalte müssen alle * Hier gibt es sehr wohl einen Platzhalter: In einer Tabellenspalte müssen alle
* Zeilen gleich hoch bleiben, sonst springt die Liste. * Zeilen gleich hoch bleiben, sonst springt die Liste.
*
* ``version`` ist das ``image_version`` des Artikels, wenn die Liste es kennt:
* ``null`` spart die Anfrage ganz, eine Zahl macht die Adresse eindeutig.
*/ */
export function ProduktThumb({ productId, alt }) { export function ProduktThumb({ productId, alt, version }) {
const url = useBildUrl(productId); const url = useBildUrl(productId, version, true);
if (!url) { if (!url) {
return <span className="thumb thumb-fallback"><Icon name="box" size={16} /></span>; return <span className="thumb thumb-fallback"><Icon name="box" size={16} /></span>;
} }

View File

@@ -75,7 +75,7 @@ export default function Products({ fixedType = null }) {
const columns = [ const columns = [
{ key: "thumb", header: "", label: "Bild", fixed: true, width: 56, { key: "thumb", header: "", label: "Bild", fixed: true, width: 56,
render: (p) => <ProduktThumb productId={p.id} alt={p.name} /> }, render: (p) => <ProduktThumb productId={p.id} version={p.image_version} alt={p.name} /> },
{ key: "name", header: "Name", grow: true, min: 200, { key: "name", header: "Name", grow: true, min: 200,
filterText: (p) => p.name, sortValue: (p) => p.name, filterText: (p) => p.name, sortValue: (p) => p.name,
render: (p) => ( render: (p) => (