diff --git a/backend/app/database.py b/backend/app/database.py index 0d0438f..ea1d815 100644 --- a/backend/app/database.py +++ b/backend/app/database.py @@ -9,14 +9,25 @@ settings = get_settings() # SQLite (used in tests) needs a special connect arg; Postgres does not. connect_args = {} +pool_args: dict[str, object] = {} if settings.database_url.startswith("sqlite"): 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( settings.database_url, connect_args=connect_args, pool_pre_ping=True, future=True, + **pool_args, ) SessionLocal = sessionmaker(bind=engine, autoflush=False, autocommit=False, future=True) diff --git a/backend/app/main.py b/backend/app/main.py index 7aa64ac..50a46de 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -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_count 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: # Zuerst die Lagerort-ID auf den Code umstellen (einmalig, idempotent), diff --git a/backend/app/models.py b/backend/app/models.py index 247cf4b..1e6d5d3 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -468,6 +468,11 @@ class ProductImage(Base): ``source_url`` merkt sich, woher das Bild kam – daran ist erkennbar, ob eine 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" @@ -477,6 +482,7 @@ class ProductImage(Base): ) content_type: Mapped[str] = mapped_column(String(64), 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) updated_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), default=_now, onupdate=_now diff --git a/backend/app/routers/products.py b/backend/app/routers/products.py index a0d01c5..41896ef 100644 --- a/backend/app/routers/products.py +++ b/backend/app/routers/products.py @@ -1,12 +1,21 @@ import difflib 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 sqlalchemy.orm import Session, joinedload, selectinload 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 ..models import ( Barcode, @@ -340,6 +349,9 @@ def off_vergleich( @router.get("/{product_id}/image") def get_product_image( product_id: int, + request: Request, + background_tasks: BackgroundTasks, + thumb: bool = False, db: Session = Depends(get_db), _: User = Depends(get_current_user), ) -> Response: @@ -347,22 +359,38 @@ def get_product_image( Anders als das Logo verlangt diese Route eine Anmeldung: Aus den Bildern 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) if product is None: 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 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") - return Response( - content=bild.data, - media_type=bild.content_type, - headers={ - "Cache-Control": "private, max-age=300", - "ETag": f'"bild-{product_id}-{int(bild.updated_at.timestamp())}"', - }, - ) + + # Vorschau und Original brauchen VERSCHIEDENE Marken – sonst liefert der + # Zwischenspeicher des Browsers die Briefmarke fuer das grosse Bild aus. + marke = f'"bild-{product_id}-{int(bild.updated_at.timestamp())}{"-v" if thumb else ""}"' + kopf = {"Cache-Control": "private, max-age=300", "ETag": marke} + 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) @@ -396,14 +424,17 @@ async def upload_product_image( f"Das Bild ist zu groß ({len(data) // 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) if bild is None: 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) else: bild.content_type, bild.data, bild.source_url = file.content_type, data, None + bild.thumb = klein db.commit() db.refresh(product) return product_to_out(db, product) @@ -422,19 +453,6 @@ def delete_product_image( 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: """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 # soll das Anlegen nicht mehrere Sekunden blockieren. Schlägt er fehl, # 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) diff --git a/backend/app/services/images.py b/backend/app/services/images.py index 0ee1628..97a4fd1 100644 --- a/backend/app/services/images.py +++ b/backend/app/services/images.py @@ -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 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``: 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 +import io +import time + import httpx +from sqlalchemy import update from sqlalchemy.orm import Session from ..models import Product, ProductImage @@ -28,6 +40,79 @@ ALLOWED_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"} 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: """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) if bild is None: 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) else: bild.content_type, bild.data, bild.source_url = typ, daten, url + bild.thumb = vorschau(daten, typ) return bild -def ensure(db: Session, product: Product) -> ProductImage | None: - """Lokale Kopie zurückgeben und bei Bedarf einmalig nachholen. +def nachholen(product_id: int, url: str) -> None: + """Bild nach der Antwort im Hintergrund holen – mit eigener Sitzung. - So bekommen auch Artikel ein lokales Bild, die vor dieser Funktion angelegt - wurden – ohne Wanderung über alle Datensätze. Bezahlt wird das mit einer - einmaligen Verzögerung beim ersten Aufruf. + Laeuft als Hintergrundaufgabe: beim Anlegen eines Artikels und beim ersten + Abruf eines Bildes, dessen lokale Kopie fehlt. Schlaegt der Abruf fehl, wird + 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) - if bild is not None: - return bild - if not product.image_url: - return None - bild = store(db, product, product.image_url) - if bild is not None: - db.commit() - return bild + from ..database import SessionLocal + + db = SessionLocal() + try: + product = db.get(Product, product_id) + if product is None: + return + if store(db, product, url) is None: + _gesperrt[product_id] = time.monotonic() + SPERRE_SEKUNDEN + else: + _gesperrt.pop(product_id, None) + db.commit() + finally: + db.close() diff --git a/backend/requirements.txt b/backend/requirements.txt index 793b892..66e2575 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -9,5 +9,8 @@ bcrypt==4.2.1 python-multipart==0.0.20 httpx==0.28.1 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 pytest==8.3.4 diff --git a/backend/tests/test_bild_auslieferung.py b/backend/tests/test_bild_auslieferung.py new file mode 100644 index 0000000..cf83195 --- /dev/null +++ b/backend/tests/test_bild_auslieferung.py @@ -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 diff --git a/backend/tests/test_product_images.py b/backend/tests/test_product_images.py index 623c7c6..75591ff 100644 --- a/backend/tests/test_product_images.py +++ b/backend/tests/test_product_images.py @@ -29,9 +29,11 @@ def test_image_version_spiegelt_bildzeitstempel(db): assert product_to_out(db, p).image_version == int(img.updated_at.timestamp()) -def test_ohne_bildadresse_gibt_es_nichts_zu_holen(db, monkeypatch): - monkeypatch.setattr(images, "fetch", lambda url: (_ for _ in ()).throw(AssertionError)) - assert images.ensure(db, _artikel(db)) is None +def _im_hintergrund(db, monkeypatch, artikel): + """``nachholen`` laeuft mit eigener Sitzung - im Test mit der des Fixtures.""" + 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): @@ -42,22 +44,25 @@ def test_bild_wird_einmal_geholt_und_danach_lokal_geliefert(db, monkeypatch): return EIN_PIXEL, "image/png" monkeypatch.setattr(images, "fetch", gefaelscht) + images.sperre_zuruecksetzen() 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.data == EIN_PIXEL assert erstes.content_type == "image/png" - # Zweiter Aufruf darf nicht erneut ins Netz gehen - genau das ist der Zweck. - images.ensure(db, artikel) + # Ab jetzt liegt die Kopie lokal - die Bild-Route greift gar nicht mehr zu + # ``nachholen``, siehe test_bild_auslieferung.py. assert aufrufe == ["https://example.org/pesto.png"] def test_ein_nicht_erreichbares_bild_bleibt_folgenlos(db, monkeypatch): monkeypatch.setattr(images, "fetch", lambda url: None) + images.sperre_zuruecksetzen() 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 diff --git a/ios/Sources/APIClient.swift b/ios/Sources/APIClient.swift index 80f9f04..3502e3c 100644 --- a/ios/Sources/APIClient.swift +++ b/ios/Sources/APIClient.swift @@ -134,8 +134,12 @@ actor APIClient { } /// 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) if let http = response as? HTTPURLResponse, http.statusCode == 404 { return nil } try check(response, data: data) diff --git a/ios/Sources/ProductPhotoView.swift b/ios/Sources/ProductPhotoView.swift index 432d4b6..1081d38 100644 --- a/ios/Sources/ProductPhotoView.swift +++ b/ios/Sources/ProductPhotoView.swift @@ -79,7 +79,7 @@ final class ProductImageCache { for p in products { guard let sv = p.imageVersion else { markEmpty(p.id); 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) { store(ui, data: data, version: sv, for: p.id) } else { @@ -119,7 +119,7 @@ struct ProductThumb: View { 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 { ProductImageCache.shared.markEmpty(productId) return diff --git a/web/src/api.js b/web/src/api.js index aa67940..f1a9c95 100644 --- a/web/src/api.js +++ b/web/src/api.js @@ -105,11 +105,18 @@ function zeitstempel() { * * 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. + * + * `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 resp = await fetch(`${API_BASE}${path}`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, + signal, }); if (!resp.ok) return null; return URL.createObjectURL(await resp.blob()); diff --git a/web/src/components/ProduktBild.jsx b/web/src/components/ProduktBild.jsx index 2373816..2ab6bf1 100644 --- a/web/src/components/ProduktBild.jsx +++ b/web/src/components/ProduktBild.jsx @@ -6,20 +6,33 @@ import Lightbox from "./Lightbox"; /** * 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 * „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"). -// So werden Thumbnails beim erneuten Öffnen einer Liste nicht jedes Mal neu -// geholt. Die Objekt-URLs bleiben absichtlich bestehen (werden nicht widerrufen), -// bis eine neue Version dieselbe ID ablöst. +// Modulweiter Cache: "Produkt-ID:Art:Version" → Objekt-URL (oder null = „kein +// Bild"). So werden Thumbnails beim erneuten Öffnen einer Liste nicht jedes Mal +// neu geholt. Die Objekt-URLs bleiben absichtlich bestehen (werden nicht +// widerrufen), bis eine neue Version dieselbe ID ablöst. const bildCache = new Map(); -function useBildUrl(productId, version = 0, onLoaded) { - const key = `${productId}:${version}`; +function useBildUrl(productId, version, klein = false, onLoaded = undefined) { + // 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)); // Über einen Ref, damit ein bei jedem Render neu erzeugtes onLoaded den Effekt // nicht erneut auslöst. @@ -28,6 +41,11 @@ function useBildUrl(productId, version = 0, onLoaded) { useEffect(() => { if (!productId) return undefined; + if (bekanntOhneBild) { + onLoadedRef.current?.(false); + setUrl(null); + return undefined; + } if (bildCache.has(key)) { const cached = bildCache.get(key); onLoadedRef.current?.(Boolean(cached)); @@ -35,17 +53,22 @@ function useBildUrl(productId, version = 0, onLoaded) { return undefined; } let abgebrochen = false; + const steuerung = new AbortController(); // ``version`` haengt einen Wert an die Adresse, damit der Browser nach dem // 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) => { if (abgebrochen) { if (neu) URL.revokeObjectURL(neu); return; } - // Alte Version derselben ID freigeben, dann neu cachen. - const alt = `${productId}:`; + // Alte Version derselben ID und Art freigeben, dann neu cachen. + const alt = `${productId}:${art}:`; for (const k of bildCache.keys()) { if (k !== key && k.startsWith(alt)) { 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. - return () => { abgebrochen = true; }; - }, [productId, version, key]); + // Der Abruf selbst schon: siehe Punkt 1 oben. + return () => { abgebrochen = true; steuerung.abort(); }; + }, [productId, version, key, art, klein, bekanntOhneBild]); return url; } @@ -75,7 +99,7 @@ function useBildUrl(productId, version = 0, onLoaded) { * Artikel, der nie ein Bild bekommen wird, wäre nur Lärm. */ 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); if (!url) return null; 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 * 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 }) { - const url = useBildUrl(productId); +export function ProduktThumb({ productId, alt, version }) { + const url = useBildUrl(productId, version, true); if (!url) { return ; } diff --git a/web/src/pages/Products.jsx b/web/src/pages/Products.jsx index a004675..23cc047 100644 --- a/web/src/pages/Products.jsx +++ b/web/src/pages/Products.jsx @@ -75,7 +75,7 @@ export default function Products({ fixedType = null }) { const columns = [ { key: "thumb", header: "", label: "Bild", fixed: true, width: 56, - render: (p) => }, + render: (p) => }, { key: "name", header: "Name", grow: true, min: 200, filterText: (p) => p.name, sortValue: (p) => p.name, render: (p) => (