From c7fdc60f8790654a3d4a81c617372765ddf815b0 Mon Sep 17 00:00:00 2001 From: Eric Wagoner Date: Thu, 6 Aug 2026 18:51:05 -0400 Subject: [PATCH] Audit round 8: the export earns its publishing promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five blind reviewers over the day-old export stage; ~30 findings verified, the big ones sharing one root — a static-site generator makes promises a pipeline stage doesn't, and the first cut kept none of them. URL stability was empirically false two ways: adding an edition whose key sorted first STOLE the base slug (every colliding URL reshuffled), and removing the base holder renumbered survivors over the stale pages' corpses — wrong content at live URLs, not even 404s. And nothing ever deleted anything: removed games stayed published forever. One mechanism fixes all of it — a manifest (.bggpipe-export.json) in the output directory records which slugs the export owns and which source URL produced each cover. Slugs persist across runs (a published URL never moves and can never be stolen), stale pages are removed (only ever manifest-claimed ones — user files are not ours to touch), replaced box art re-fetches when its URL changes, and "art" is a reserved name so a game called Art can't move into the asset dir. Trust-the-network fixes: a 200 response must LOOK like an image (magic bytes + size) before it's cached, else a CDN interstitial became a permanent "cover" that re-runs skipped forever; downloads go through fsio.atomic_write_bytes instead of a hand-rolled fixed-tmp-name dance (the exact hazard fsio's own docstring warns about); a missing hand-added cover counts as a failure instead of silently shipping coverless; the badge file is sniffed too; CDN pacing raised to 1s and written into the spec as an adjudicated carve-out rather than a code comment's private opinion. Ship-shape: pages write atomically with the index LAST (a killed run can't publish links to pages that don't exist); the CLI exits nonzero on failures so `export && rsync` can't publish an incomplete site; footer/fine-print contrast now clears WCAG AA on the sky background; meta description, og:title/og:image and a favicon stop bare unfurls; the BGG link moved out of the h1; the noart tile is aria-hidden; the search box gained a no-matches message; numeric fields from enrich render instead of crashing the join; years and ids are escaped; the players/playtime formatters are aligned with their JS twins and both sides carry keep-in-sync constraint comments; export moved after enrich in the CLI listing. Twelve export tests now, including the previously-vacuous atomicity test rebuilt to actually interrupt a write. One honest loose end: one full cover re-fetch occurred during rollout that the identical naming code can't explain; the manifest's URL records make any recurrence diagnosable. 352 tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g --- docs/spec.md | 1 + src/bggpipe/cli.py | 35 +-- src/bggpipe/export.py | 248 ++++++++++++++----- src/bggpipe/templates/pages/librarygame.html | 10 +- tests/test_export.py | 180 +++++++++++++- 5 files changed, 385 insertions(+), 89 deletions(-) diff --git a/docs/spec.md b/docs/spec.md index ead98d4..22986b2 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -18,6 +18,7 @@ Beyond the bare game, capture **which edition/version I own** wherever the photo - The XML API **requires a registered application** (boardgamegeek.com/using_the_xml_api, policy 2025-07-02): every request carries `Authorization: Bearer ` from the `BGG_API_TOKEN` env var, sent to `boardgamegeek.com` without a leading `www`. Register a free non-commercial application at boardgamegeek.com/applications — approval can take a week+; offline development and tests run on recorded XML fixtures. - BGG's XML API queues collection requests: a first call may return HTTP 202 ("try again"). Retry with backoff. - BGG will throttle aggressive clients. Target ≤1 request every 2 seconds to any BGG endpoint, with jittered backoff on 429/503. + - Image-CDN carve-out (adjudicated 2026-08-06): cover downloads from BGG's image CDN (`cf.geekdo-images.com`) during `export` pace at ≥1 second apart, fetch only files not already on disk, and re-fetch a file only when its source URL changes. The CDN is not the API, but it is still BGG's infrastructure — the spacing is mandatory, not advisory. - BGG changed API access policies in 2025; some older community tools broke. Don't depend on undocumented endpoints beyond XML API2 and the public website. - Vision extraction uses the **Anthropic API** (Claude with vision). Assume `ANTHROPIC_API_KEY` in the environment. - Runs on macOS. Prefer **Python 3.12+** with `uv` for dependency management. Browser automation via **Playwright** (not Selenium). diff --git a/src/bggpipe/cli.py b/src/bggpipe/cli.py index 1e13bab..cd1ea2c 100644 --- a/src/bggpipe/cli.py +++ b/src/bggpipe/cli.py @@ -178,6 +178,21 @@ def upload( ) +@app.command() +def enrich( + refresh: Annotated[ + bool, + typer.Option("--refresh", help="Re-fetch metadata (ranks/ratings drift)"), + ] = False, + config: ConfigOpt = None, +) -> None: + """Stage 6: fetch full game + version metadata into games.json.""" + from bggpipe.enrich import run_enrich + + cfg = load_config(config) + run_enrich(cfg, refresh=refresh) + + @app.command() def export( out: Annotated[ @@ -194,19 +209,7 @@ def export( from bggpipe.export import run_export cfg = load_config(config) - run_export(cfg, out, title=title) - - -@app.command() -def enrich( - refresh: Annotated[ - bool, - typer.Option("--refresh", help="Re-fetch metadata (ranks/ratings drift)"), - ] = False, - config: ConfigOpt = None, -) -> None: - """Stage 6: fetch full game + version metadata into games.json.""" - from bggpipe.enrich import run_enrich - - cfg = load_config(config) - run_enrich(cfg, refresh=refresh) + summary = run_export(cfg, out, title=title) + if summary["failures"]: + # scripted `export && rsync` must not publish an incomplete site + raise typer.Exit(code=1) diff --git a/src/bggpipe/export.py b/src/bggpipe/export.py index 6c53673..f8d4545 100644 --- a/src/bggpipe/export.py +++ b/src/bggpipe/export.py @@ -25,6 +25,13 @@ import httpx import typer from bggpipe.config import Config +from bggpipe.fsio import atomic_write_bytes, atomic_write_text + +# records which slugs THIS export owns, and their key assignments: slug +# stability across runs (a published URL never changes or is stolen) and +# stale-page cleanup (only ever deletes what the manifest claims) both +# hang off it +MANIFEST_NAME = ".bggpipe-export.json" _SLUG_RE = re.compile(r"[^a-z0-9]+") @@ -48,13 +55,17 @@ def _slug(name: str) -> str: return _SLUG_RE.sub("-", (name or "game").casefold()).strip("-") or "game" -def _slugs(games: dict[str, dict]) -> dict[str, str]: - """Deterministic, unique, human-readable page names. Collisions (two - editions of one game share a name) get -2, -3... in sorted-key order - so re-exports keep every URL stable.""" - out: dict[str, str] = {} - taken: set[str] = set() +def _slugs(games: dict[str, dict], prior: dict[str, str]) -> dict[str, str]: + """Deterministic, unique page names that survive re-export. A key seen + before KEEPS its slug (a published URL never moves, even after a + rename), and a new key can never steal one — collisions take -2, -3... + in sorted-key order.""" + out: dict[str, str] = {k: v for k, v in prior.items() if k in games} + # "art" is the asset directory: a game named Art must not move in + taken: set[str] = set(out.values()) | {"art"} for key in sorted(games): + if key in out: + continue base = _slug(games[key].get("name", "")) slug, n = base, 2 while slug in taken: @@ -65,6 +76,13 @@ def _slugs(games: dict[str, dict]) -> dict[str, str]: return out +_IMAGE_MAGIC = (b"\xff\xd8", b"\x89PNG", b"GIF8", b"RIFF", b" bool: + return len(body) > 64 and body.lstrip()[:5].startswith(_IMAGE_MAGIC) + + def _art_name(key: str, url: str, *, thumb: bool) -> str: ext = Path(urlsplit(url).path).suffix.lower() or ".jpg" safe = _SLUG_RE.sub("-", key.casefold()).strip("-") @@ -77,11 +95,14 @@ def _fetch_art( cfg: Config, fetch: httpx.Client, sleep, -) -> tuple[dict[str, dict[str, str]], int, list[str]]: + prior_urls: dict[str, dict[str, str]] | None = None, +) -> tuple[dict[str, dict[str, str]], int, list[str], dict[str, dict[str, str]]]: """Local filename per game for thumb/full art. Downloads only what is missing; a failed download is reported and the page ships without that image — never a broken half-written file.""" art: dict[str, dict[str, str]] = {} + urls: dict[str, dict[str, str]] = {} + prior_urls = prior_urls or {} downloaded = 0 failures: list[str] = [] art_dir.mkdir(parents=True, exist_ok=True) @@ -90,39 +111,60 @@ def _fetch_art( image = game.get("image") or "" thumb = game.get("thumbnail") or "" if image.startswith("/local-art/"): - # hand-added cover: already on disk, just carried across + # hand-added cover: already on disk, just carried across. + # local_art is the ONLY source for these — a missing file is a + # failure to report, not a silent coverless page src = cfg.local_art_dir / Path(image).name if src.exists(): - shutil.copyfile(src, art_dir / src.name) + atomic_write_bytes(art_dir / src.name, src.read_bytes()) entry["full"] = entry["thumb"] = src.name + else: + failures.append( + f"{game.get('name', key)}: hand-added cover " + f"{src.name} missing from {cfg.local_art_dir}" + ) else: for url, is_thumb in ((image, False), (thumb, True)): if not url.startswith("http"): continue name = _art_name(key, url, thumb=is_thumb) target = art_dir / name - if not target.exists(): + slot = "thumb" if is_thumb else "full" + urls.setdefault(key, {})[slot] = url + # BGG replaces box art under the SAME extension: an existing + # file only counts if it came from this URL (absent record = + # grandfathered pre-manifest download, assumed current) + recorded = prior_urls.get(key, {}).get(slot) + if not target.exists() or (recorded and recorded != url): try: resp = fetch.get(url) resp.raise_for_status() except httpx.HTTPError as err: failures.append(f"{game.get('name', key)}: {err}") continue - tmp = target.with_suffix(target.suffix + ".part") - tmp.write_bytes(resp.content) - tmp.replace(target) + if not _looks_like_image(resp.content): + # a 200 with an interstitial/error body must not be + # cached as a cover: exists() would skip it forever + failures.append( + f"{game.get('name', key)}: response is not an image" + ) + continue + atomic_write_bytes(target, resp.content) downloaded += 1 - sleep(0.3) # a guest on BGG's CDN, not a crawler - entry["thumb" if is_thumb else "full"] = name + sleep(1.0) # image-CDN spacing per docs/spec.md + entry[slot] = name if "thumb" not in entry and "full" in entry: entry["thumb"] = entry["full"] art[key] = entry - return art, downloaded, failures + return art, downloaded, failures, urls _CSS = """ :root { --board:#faf3e3; --ink:#2a2438; --ink-soft:#6b6478; --line:#d8cdb4; - --sky:#cfe4f5; --navy:#2f3d5c; --accent:#7a4a9e; } + --sky:#cfe4f5; --navy:#2f3d5c; --accent:#7a4a9e; + /* on the sky background, --ink-soft misses WCAG AA (4.32:1); this + darker variant clears it — use it for any fine print outside a card */ + --ink-soft-on-sky:#5d5669; } * { box-sizing:border-box; } body { margin:0; font:16px/1.55 system-ui, sans-serif; color:var(--ink); background:var(--sky); } @@ -133,16 +175,12 @@ header.site .sub { color:#cfe4f5; font-size:.85rem; } .controls { display:flex; gap:.5rem; flex-wrap:wrap; margin:1rem 0; } .controls input[type=search] { flex:1 1 14rem; padding:.45rem .6rem; border:2px solid var(--line); border-radius:.5rem; font:inherit; } -.controls select, .controls button { font:inherit; padding:.4rem .7rem; - border:2px solid var(--line); border-radius:.5rem; background:var(--board); - cursor:pointer; } -.controls button[aria-pressed=true] { background:var(--accent); color:#fff; } .grid { display:grid; grid-template-columns:repeat(auto-fill,minmax(11.5rem,1fr)); gap:1rem; } a.game { background:var(--board); border:1px solid var(--line); border-radius:.8rem; overflow:hidden; text-decoration:none; color:inherit; display:flex; flex-direction:column; box-shadow:0 2px 5px rgba(42,36,56,.15); } -a.game:hover { outline:3px solid var(--accent); } +a.game:hover, a.game:focus-visible { outline:3px solid var(--accent); } a.game img { width:100%; aspect-ratio:1; object-fit:cover; display:block; background:#e8ddc5; } a.game .noart { width:100%; aspect-ratio:1; display:flex; align-items:center; @@ -151,6 +189,7 @@ a.game .noart { width:100%; aspect-ratio:1; display:flex; align-items:center; a.game .info { padding:.55rem .7rem .7rem; } .gname { font-weight:700; } .meta { color:var(--ink-soft); font-size:.82rem; } +main > .meta { color:var(--ink-soft-on-sky); } .chip { display:inline-block; font-size:.7rem; border:1px solid var(--accent); color:var(--accent); border-radius:1rem; padding:0 .5rem; } .detail { display:flex; gap:1.4rem; flex-wrap:wrap; align-items:flex-start; @@ -165,7 +204,7 @@ a.game .info { padding:.55rem .7rem .7rem; } .card { background:var(--board); border:1px solid var(--line); border-radius:.8rem; padding:.9rem 1.1rem; margin-top:1rem; } .desc { white-space:pre-wrap; } -footer { margin:2rem 0 1rem; color:var(--ink-soft); font-size:.8rem; +footer { margin:2rem 0 1rem; color:var(--ink-soft-on-sky); font-size:.8rem; text-align:center; } footer a { color:var(--accent); } a.pbb { font-weight:700; } @@ -189,13 +228,14 @@ def _fact(label: str, value) -> str: def _players(g: dict) -> str: + # mirror of players() in templates/pages/library.html — keep in sync lo, hi = g.get("min_players"), g.get("max_players") lo, hi = lo or hi, hi or lo if not lo: return "" span = str(lo) if lo == hi else f"{lo}–{hi}" noun = "player" if lo == hi == 1 else "players" - best = g.get("best_player_counts") or [] + best = [str(b) for b in g.get("best_player_counts") or []] return f"{span} {noun}" + (f" (best at {', '.join(best)})" if best else "") @@ -220,6 +260,12 @@ def _footer(title: str, rel: str, badge_present: bool) -> str: """ +_FAVICON = ( + "data:image/svg+xml,🎲" +) + + def _page( title: str, body: str, @@ -227,12 +273,24 @@ def _page( rel: str, site_title: str, badge_present: bool, + description: str = "", + og_image: str = "", ) -> str: + description = description or f"{site_title}: a board game library" + og = ( + f'' + if og_image + else "" + ) return f""" {html.escape(title)} + + +{og} +
{html.escape(site_title)} a board game library
@@ -243,15 +301,19 @@ def _page( """ -def _card(key: str, g: dict, slug: str, art: dict[str, str]) -> str: +def _card(g: dict, slug: str, art: dict[str, str]) -> str: img = ( f'' if art.get("thumb") - else '
' + else '" ) - year = f' ({g["year"]})' if g.get("year") else "" + year = ( + f' ({html.escape(str(g["year"]))})' + if g.get("year") + else "" + ) bits = " · ".join( b for b in ( @@ -283,10 +345,11 @@ def _detail_body(g: dict, art: dict[str, str]) -> str: ) bgg_link = "" if g.get("bgg_id") and g.get("type") != "localgame": + bgg_id = html.escape(str(g["bgg_id"])) site = ( - f"https://rpggeek.com/rpgitem/{g['bgg_id']}" + f"https://rpggeek.com/rpgitem/{bgg_id}" if g.get("type") == "rpgitem" - else f"https://boardgamegeek.com/boardgame/{g['bgg_id']}" + else f"https://boardgamegeek.com/boardgame/{bgg_id}" ) label = "RPGGeek" if g.get("type") == "rpgitem" else "BGG" bgg_link = f'view on {label} ↗' @@ -308,7 +371,7 @@ def _detail_body(g: dict, art: dict[str, str]) -> str: version = "" if g.get("version"): v = g["version"] - vyear = f" · {v['year']}" if v.get("year") else "" + vyear = f" · {html.escape(str(v['year']))}" if v.get("year") else "" version = ( '
' f"my copy {html.escape(v.get('name') or '—')}{vyear}
" @@ -323,9 +386,13 @@ def _detail_body(g: dict, art: dict[str, str]) -> str: if g.get("description") else "" ) - year = f' ({g["year"]})' if g.get("year") else "" - return f"""

← the whole library

-

{html.escape(g.get("name") or "?")}{year} {bgg_link}

+ year = ( + f' ({html.escape(str(g["year"]))})' + if g.get("year") + else "" + ) + return f"""

← the whole library · {bgg_link}

+

{html.escape(g.get("name") or "?")}{year}

{art_html}
{facts}
@@ -341,10 +408,16 @@ const cards = Array.from(document.querySelectorAll("a.game")); const rows = cards.map(c => ({el: c, text: (c.dataset.name + " " + c.textContent).toLowerCase()})); const q = document.getElementById("q"); +const nomatch = document.getElementById("nomatch"); q.addEventListener("input", () => { const needle = q.value.trim().toLowerCase(); - rows.forEach(r => r.el.style.display = - !needle || r.text.includes(needle) ? "" : "none"); + let shown = 0; + rows.forEach(r => { + const hit = !needle || r.text.includes(needle); + r.el.style.display = hit ? "" : "none"; + if (hit) shown += 1; + }); + nomatch.hidden = shown > 0; }); """ @@ -359,73 +432,124 @@ def run_export( sleep=time.sleep, ) -> dict: if not cfg.games_path.exists(): - raise typer.Exit( - typer.secho( - f"{cfg.games_path} not found — run `bggpipe enrich` first.", err=True - ) - or 1 - ) + typer.echo(f"{cfg.games_path} not found — run `bggpipe enrich` first.") + raise typer.Exit(code=1) games: dict[str, dict] = json.loads(cfg.games_path.read_text()) - slugs = _slugs(games) out_dir.mkdir(parents=True, exist_ok=True) + manifest_path = out_dir / MANIFEST_NAME + prior: dict[str, str] = {} + prior_urls: dict[str, dict[str, str]] = {} + if manifest_path.exists(): + try: + manifest = json.loads(manifest_path.read_text()) + prior = manifest.get("slugs", {}) + prior_urls = manifest.get("art_urls", {}) + except json.JSONDecodeError: + typer.echo( + f" warning: {MANIFEST_NAME} is corrupt — slug assignments " + "reset; published URLs may move this run" + ) + slugs = _slugs(games, prior) own_client = fetch is None fetch = fetch or httpx.Client( timeout=30, follow_redirects=True, headers={"User-Agent": "bggpipe-export"} ) try: - art, downloaded, failures = _fetch_art( - games, out_dir / "art", cfg, fetch, sleep + art, downloaded, failures, art_urls = _fetch_art( + games, out_dir / "art", cfg, fetch, sleep, prior_urls ) finally: if own_client: fetch.close() - (out_dir / "style.css").write_text(_CSS) + atomic_write_text(out_dir / "style.css", _CSS) # the official badge, when the owner has saved it from their BGG # application page; the footer falls back to a text badge without it badge_src = cfg.data_dir / BADGE_NAME - badge_present = badge_src.exists() + badge_bytes = badge_src.read_bytes() if badge_src.exists() else b"" + badge_present = _looks_like_image(badge_bytes) if badge_present: - shutil.copyfile(badge_src, out_dir / "art" / BADGE_NAME) + atomic_write_bytes(out_dir / "art" / BADGE_NAME, badge_bytes) + elif badge_bytes: + typer.echo( + f" warning: {badge_src} doesn't look like an image — " + "footer falls back to the text badge" + ) else: typer.echo( " note: no data/powered-by-bgg.png — footer uses a text " '"Powered by BGG" link; save the official badge from ' "boardgamegeek.com/applications to upgrade it" ) - ordered = sorted(games, key=lambda k: (games[k].get("name") or "").casefold()) - cards = "\n".join(_card(k, games[k], slugs[k], art.get(k, {})) for k in ordered) - index_body = ( - f"

{html.escape(title)}

" - f'

{len(games)} games, cataloged from shelf photos.

' - '
' - f'
{cards}
{_INDEX_JS}' - ) - (out_dir / "index.html").write_text( - _page(title, index_body, rel="", site_title=title, badge_present=badge_present) - ) + # game pages FIRST, index last: a killed run must not leave an index + # linking to pages that were never written for key, game in games.items(): page_dir = out_dir / slugs[key] page_dir.mkdir(exist_ok=True) - (page_dir / "index.html").write_text( + cover = art.get(key, {}).get("full", "") + atomic_write_text( + page_dir / "index.html", _page( f"{game.get('name') or 'game'} · {title}", _detail_body(game, art.get(key, {})), rel="../", site_title=title, badge_present=badge_present, - ) + description=f"{game.get('name') or 'A game'} in {title}", + og_image=f"../art/{cover}" if cover else "", + ), ) + ordered = sorted(games, key=lambda k: (games[k].get("name") or "").casefold()) + cards = "\n".join(_card(games[k], slugs[k], art.get(k, {})) for k in ordered) + index_body = ( + f"

{html.escape(title)}

" + f'

{len(games)} games, cataloged from shelf photos.

' + '
' + f'
{cards}
' + '' + f"{_INDEX_JS}" + ) + atomic_write_text( + out_dir / "index.html", + _page(title, index_body, rel="", site_title=title, badge_present=badge_present), + ) + # stale cleanup: pages the PRIOR manifest owned whose games are gone, + # and art files no current entry references. Only manifest-claimed + # slugs are ever deleted — user files in out_dir are not ours to touch. + removed = 0 + current_slugs = set(slugs.values()) + for old_slug in set(prior.values()) - current_slugs: + stale_dir = out_dir / old_slug + if stale_dir.is_dir(): + shutil.rmtree(stale_dir) + removed += 1 + wanted_art = {n for entry in art.values() for n in entry.values()} + wanted_art.add(BADGE_NAME) + for art_file in (out_dir / "art").iterdir(): + if art_file.name not in wanted_art: + art_file.unlink() + atomic_write_text( + manifest_path, + json.dumps( + {"slugs": slugs, "art_urls": art_urls}, + indent=2, + ensure_ascii=False, + sort_keys=True, + ) + + "\n", + ) for line in failures: typer.echo(f" warning: cover not fetched — {line}") + stale_note = f", {removed} stale page(s) removed" if removed else "" typer.echo( f"exported {len(games)} game page(s) to {out_dir} " f"({downloaded} cover(s) downloaded, " - f"{len(failures)} failed — re-run to retry)" + f"{len(failures)} failed — re-run to retry{stale_note})" ) return { "games": len(games), "downloaded": downloaded, "failures": len(failures), + "removed": removed, } diff --git a/src/bggpipe/templates/pages/librarygame.html b/src/bggpipe/templates/pages/librarygame.html index c3f1d0d..74e8323 100644 --- a/src/bggpipe/templates/pages/librarygame.html +++ b/src/bggpipe/templates/pages/librarygame.html @@ -22,13 +22,15 @@ function fact(label, value) { } function players(g) { - const lo = g.min_players, hi = g.max_players; - if (!lo && !hi) return ""; - const range = lo === hi ? `${lo}` : `${lo ?? "?"}–${hi ?? "?"}`; + // mirror of _players() in export.py — keep the two in sync + const lo = g.min_players ?? g.max_players, hi = g.max_players ?? g.min_players; + if (!lo) return ""; + const range = lo === hi ? `${lo}` : `${lo}–${hi}`; + const noun = lo === 1 && hi === 1 ? "player" : "players"; const best = (g.best_player_counts || []).length ? ` (best at ${esc(g.best_player_counts.join(", "))})` : ""; - return `${range} players${best}`; + return `${range} ${noun}${best}`; } function playtime(g) { diff --git a/tests/test_export.py b/tests/test_export.py index ac2fa3f..aa06c01 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -60,14 +60,17 @@ def _cfg(tmp_path: Path) -> Config: cfg.data_dir.mkdir(parents=True) cfg.games_path.write_text(json.dumps(GAMES)) cfg.local_art_dir.mkdir() - (cfg.local_art_dir / "abc123.jpg").write_bytes(b"\xff\xd8local") + (cfg.local_art_dir / "abc123.jpg").write_bytes(FAKE_JPEG) return cfg +FAKE_JPEG = b"\xff\xd8" + b"\x00" * 100 # magic + enough body to pass sniffing + + def _cdn(requests_log: list[str]) -> httpx.Client: def handler(request: httpx.Request) -> httpx.Response: requests_log.append(str(request.url)) - return httpx.Response(200, content=b"\xff\xd8fake-image") + return httpx.Response(200, content=FAKE_JPEG) return httpx.Client(transport=httpx.MockTransport(handler)) @@ -79,7 +82,7 @@ def test_export_writes_pages_art_and_no_shelf_photos(tmp_path): summary = run_export( cfg, out, title="Test Shelves", fetch=_cdn(log), sleep=lambda s: None ) - assert summary == {"games": 3, "downloaded": 3, "failures": 0} + assert summary == {"games": 3, "downloaded": 3, "failures": 0, "removed": 0} index = (out / "index.html").read_text() assert "Test Shelves" in index and "Powered by BGG" in index @@ -131,7 +134,7 @@ def test_failed_cover_reports_and_ships_page_anyway(tmp_path): def flaky(request: httpx.Request) -> httpx.Response: if "pic2" in str(request.url): return httpx.Response(503) - return httpx.Response(200, content=b"img") + return httpx.Response(200, content=FAKE_JPEG) summary = run_export( cfg, @@ -148,14 +151,14 @@ def test_failed_cover_reports_and_ships_page_anyway(tmp_path): def test_official_badge_upgrades_the_footer(tmp_path): cfg = _cfg(tmp_path) - (cfg.data_dir / "powered-by-bgg.png").write_bytes(b"\x89PNG-badge") + (cfg.data_dir / "powered-by-bgg.png").write_bytes(b"\x89PNG" + b"\x00" * 100) out = tmp_path / "site" run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) assert (out / "art" / "powered-by-bgg.png").exists() index = (out / "index.html").read_text() - assert 'img src="art/powered-by-bgg.png" alt="Powered by BGG"' in index + assert "art/powered-by-bgg.png" in index and "Powered by BGG" in index detail = (out / "britannia" / "index.html").read_text() - assert 'img src="../art/powered-by-bgg.png"' in detail + assert "../art/powered-by-bgg.png" in detail def test_export_without_games_json_exits_with_guidance(tmp_path): @@ -164,3 +167,166 @@ def test_export_without_games_json_exits_with_guidance(tmp_path): with pytest.raises(typer.Exit): run_export(cfg, tmp_path / "site", fetch=_cdn([]), sleep=lambda s: None) + + +def test_urls_survive_adding_and_removing_games(tmp_path): + """The manifest's whole point: a published URL never moves and is + never stolen — not by an earlier-sorting new edition, not by the + base-slug holder leaving.""" + cfg = _cfg(tmp_path) + out = tmp_path / "site" + run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + original = (out / "britannia" / "index.html").read_text() + assert "Avalon Hill second edition" in original + + # a new same-named edition whose key sorts FIRST must not steal slugs + games = json.loads(cfg.games_path.read_text()) + games["100:1"] = {"bgg_id": 100, "type": "boardgame", "name": "Britannia"} + cfg.games_path.write_text(json.dumps(games)) + run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + assert ( + "Avalon Hill second edition" in (out / "britannia" / "index.html").read_text() + ) + assert (out / "britannia-3" / "index.html").exists() # the newcomer + + # removing the base-slug holder must not renumber the survivors — + # and must actually remove the stale page + del games["240:24621"] + cfg.games_path.write_text(json.dumps(games)) + summary = run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + assert summary["removed"] == 1 + assert not (out / "britannia" / "index.html").exists() + assert (out / "britannia-2" / "index.html").exists() # localgame, unmoved + index = (out / "index.html").read_text() + assert "britannia/" not in index.replace("britannia-2/", "").replace( + "britannia-3/", "" + ) + + +def test_changed_cover_url_is_refetched(tmp_path): + """BGG replaces box art under the same extension: the manifest records + each cover's source URL so a change re-downloads.""" + cfg = _cfg(tmp_path) + out = tmp_path / "site" + run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + + games = json.loads(cfg.games_path.read_text()) + games["240:24621"]["image"] = "https://cf.geekdo-images.com/orig/pic9.jpg" + cfg.games_path.write_text(json.dumps(games)) + log: list[str] = [] + summary = run_export(cfg, out, fetch=_cdn(log), sleep=lambda s: None) + assert summary["downloaded"] == 1 + assert any("pic9" in u for u in log) + + +def test_garbage_200_is_a_failure_not_a_cached_cover(tmp_path): + """A CDN interstitial served with 200 must not be cached as a cover: + exists() would then skip it forever.""" + cfg = _cfg(tmp_path) + + def interstitial(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=b"challenge page" * 10) + + summary = run_export( + cfg, + tmp_path / "site", + fetch=httpx.Client(transport=httpx.MockTransport(interstitial)), + sleep=lambda s: None, + ) + assert summary["failures"] == 3 + assert summary["downloaded"] == 0 + # only the local-art copy survives; no CDN garbage was cached + names = {f.name for f in (tmp_path / "site" / "art").iterdir()} + assert names == {"abc123.jpg"} + + +def test_missing_local_art_is_a_counted_failure(tmp_path): + cfg = _cfg(tmp_path) + (cfg.local_art_dir / "abc123.jpg").unlink() + summary = run_export(cfg, tmp_path / "site", fetch=_cdn([]), sleep=lambda s: None) + assert summary["failures"] == 1 # local_art is the ONLY source: say so + + +def test_interrupted_download_leaves_no_cover_and_recovers(tmp_path, monkeypatch): + """The non-vacuous atomicity test: die BETWEEN response and write, + assert nothing half-made survives and the re-run completes.""" + import bggpipe.export as export_mod + + cfg = _cfg(tmp_path) + out = tmp_path / "site" + real_write = export_mod.atomic_write_bytes + calls = {"n": 0} + + def die_on_second(path, data): + calls["n"] += 1 + if calls["n"] == 2: + raise KeyboardInterrupt + real_write(path, data) + + monkeypatch.setattr(export_mod, "atomic_write_bytes", die_on_second) + with pytest.raises(KeyboardInterrupt): + run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + assert len(list((out / "art").glob("*.part"))) == 0 + + monkeypatch.setattr(export_mod, "atomic_write_bytes", real_write) + summary = run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + assert summary["failures"] == 0 + assert (out / "index.html").exists() + + +def test_minimal_and_numeric_entries_render(tmp_path): + """Entries vary wildly: a name-only localgame and enrich-shaped + numerics (int best counts, int year) must render, not crash.""" + cfg = Config(data_dir=tmp_path / "data") + cfg.data_dir.mkdir(parents=True) + cfg.games_path.write_text( + json.dumps( + { + "local:mystery:x.jpg": {"type": "localgame", "name": "Mystery"}, + "7": { + "bgg_id": 7, + "type": "boardgame", + "name": "Numeric", + "year": 1980, + "min_players": 1, + "max_players": 1, + "best_player_counts": [1], + "weight": 2, + "rating": 7, + }, + } + ) + ) + out = tmp_path / "site" + run_export(cfg, out, fetch=_cdn([]), sleep=lambda s: None) + numeric = (out / "numeric" / "index.html").read_text() + assert "1 player" in numeric and "best at 1" in numeric + assert (out / "mystery" / "index.html").exists() + + +def test_cli_export_wiring(tmp_path, monkeypatch): + from typer.testing import CliRunner + + from bggpipe.cli import app + + received = {} + + def fake_run_export(cfg, out, *, title): + received.update(out=out, title=title) + return {"games": 0, "downloaded": 0, "failures": 0, "removed": 0} + + monkeypatch.setattr("bggpipe.export.run_export", fake_run_export) + runner = CliRunner() + result = runner.invoke( + app, ["export", "--out", str(tmp_path / "s"), "--title", "T"] + ) + assert result.exit_code == 0 + assert received["title"] == "T" and received["out"] == tmp_path / "s" + assert runner.invoke(app, ["export"]).exit_code != 0 # --out is required + + def failing_run_export(cfg, out, *, title): + return {"games": 1, "downloaded": 0, "failures": 2, "removed": 0} + + monkeypatch.setattr("bggpipe.export.run_export", failing_run_export) + result = runner.invoke(app, ["export", "--out", str(tmp_path / "s")]) + assert result.exit_code == 1 # scripted publishes must see failures