diff --git a/.github/workflows/build-kotlin-docs-local.yaml b/.github/workflows/build-kotlin-docs-local.yaml new file mode 100644 index 00000000..789df468 --- /dev/null +++ b/.github/workflows/build-kotlin-docs-local.yaml @@ -0,0 +1,440 @@ +name: Build Kotlin Docs (Local) + +# Local-filesystem counterpart of build-kotlin-docs.yaml: same five steps +# (find_missing_assets -> populate_db -> insert_optimized_media -> +# build-stdlib-json-docs -> sync_kdoc_json_to_db), same ADFA-4737 blacklist, +# but reads its documentation.db/webHelpImages.zip inputs from paths on the +# runner's own disk (db_path / images_zip_path) instead of Google Drive, and +# writes its outputs (the updated database, the missing-assets report) back +# to disk (output_dir / db_path) instead of uploading them to Drive. No GCP +# Workload Identity Federation, Drive API, or associated secrets are used +# anywhere in this file. +# +# Since a GitHub-hosted runner is a fresh, disposable VM with no access to +# anyone's actual local disk, db_path/images_zip_path/output_dir only make +# sense here against a self-hosted runner, or when this workflow is run +# locally (e.g. via https://github.com/nektos/act) with those host paths +# bind-mounted into the job's container at the paths you pass as inputs. +# run-build-kotlin-docs-with-act.sh at the repo root drives exactly that and +# is the supported way to run this file locally. +# +# CAUTION when invoking act by hand rather than through that script: act does +# NOT apply workflow_dispatch input defaults. Every "default:" below applies +# on real GitHub and is simply absent under act, so an input you don't pass +# arrives empty. That matters most for dry_run, whose default is true: with +# it unset, "${{ !inputs.dry_run }}" evaluates to true and the final step +# writes the rebuilt database back over db_path. Always pass +# --input dry_run=true/false explicitly (the script always does). +# +# KNOWN LIMITATION: populate_db.py requires Writerside's own image export +# ("webHelpImages.zip"), which JetBrains only produces via IntelliJ IDEA's +# Writerside plugin build/export action - there is no headless/CLI way to +# generate it (see ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/README.md, +# "Inputs you need before starting"). So this workflow expects that export +# to already exist on disk at images_zip_path rather than generating it +# itself. Use skip_website_docs to bypass this entirely and only refresh the +# kotlin-stdlib/-reflect/-test JSON content. +# +# Optional secret (Slack notifications are skipped with a warning if unset) - +# same as build-kotlin-docs.yaml: +# SLACK_WEBHOOK_URL - Incoming Webhook URL for the "Notify Slack" steps +# below ("Grabbing baton" on start, "...Dropping +# baton" on finish - org shorthand for lock +# acquire/release, since this workflow mutates a +# single shared local file, db_path). + +permissions: + contents: read + +# This workflow overwrites a single shared local file (db_path) - never let +# two runs race to write it at the same time. +concurrency: + group: build-kotlin-docs-local + cancel-in-progress: false + +on: + workflow_dispatch: + inputs: + kotlin_web_site_ref: + description: >- + Branch/tag/commit of JetBrains/kotlin-web-site to check out for the + "docs" tree (topics/, images/, kr.tree, v.list). Leave empty to use + the repo's default branch. + required: false + default: '' + kotlin_ref: + description: >- + Branch/tag/commit of JetBrains/kotlin to check out for the + kotlin-stdlib-docs build. Leave empty to use the repo's default + branch. Pin this to a real release tag for a reproducible build. + required: false + default: '' + db_path: + description: >- + Path on this runner's disk to the input documentation.db. Read + directly (no download/unzip) and, unless dry_run is true, written + back to this same path when the run finishes. + required: true + images_zip_path: + description: >- + Path on this runner's disk to Writerside's webHelpImages.zip + export matching kotlin_web_site_ref (see KNOWN LIMITATION above). + Required unless skip_website_docs is true. + required: false + default: '' + output_dir: + description: >- + Directory on this runner's disk to write outputs into: the + missing-assets QA report and a run-numbered copy of the built + database (documentation-db-.db). Created if it + doesn't already exist. + required: false + default: 'build-kotlin-docs-output' + kotlin_libs_version: + description: >- + Version of the published kotlin-stdlib/-reflect/-test artifacts to + document (Gradle -PdeployVersion). kotlin_big extracts the real + binaries at this version rather than requiring a local build of the + whole kotlin repo, which is what the checkout's own + defaultSnapshotVersion would otherwise demand. Keep this in step with + kotlin_ref. Set empty to fall back to that snapshot default, which + only resolves if you have built the kotlin repo yourself. + required: false + default: '2.4.10' + kotlin_libs_repo: + description: >- + Maven repository to resolve those artifacts from (Gradle + -PkotlinLibsRepo). kotlin_big already declares mavenCentral(), so a + released kotlin_libs_version needs nothing here; set it to point at a + private or snapshot repository instead. + required: false + default: '' + skip_website_docs: + description: 'Skip the kotlin-web-site steps and only refresh kotlin-stdlib/-reflect/-test JSON content.' + required: false + default: false + type: boolean + skip_stdlib_docs: + description: >- + Skip the kotlin-stdlib/-reflect/-test steps (cloning JetBrains/kotlin, + the Dokka JSON build, and the sync into the database) and only refresh + the kotlin-web-site content. The mirror image of skip_website_docs - + setting both leaves nothing for the run to do and is rejected. + required: false + default: false + type: boolean + dry_run: + description: >- + If true, build and verify everything but do NOT write the result + back to db_path - the input file on disk is left untouched. Set to + false only once you trust a given ref/path combination (see this + workflow's testing notes). + required: false + default: true + type: boolean + +jobs: + build-kotlin-docs: + runs-on: ubuntu-latest + timeout-minutes: 180 + env: + # The repo's uv convention, as one string so every step below reads the + # same: `$UV_RUN python3 + + + + +{# + Recursive block renderer. Macros only see the variables passed to them, so + every block that can nest other blocks (blockquote, note/tip/warning, list + items, table cells, tabs) passes its children back through renderBlock(). + Macros defined in a template are directly visible to themselves and to each + other within that same template, so no self-import is needed for recursion. +#} +{% macro renderBlock(b) %} +{% if b.type == "heading" %} +{{ b.html|raw }} + +{% elseif b.type == "paragraph" %} +

{{ b.html|raw }}

+ +{% elseif b.type == "code" %} +
{{ b.code }}
+ +{% elseif b.type == "blockquote" %} +
+{% if b.attrs.title %}

{{ b.attrs.title }}

{% endif %} +{% for child in b.blocks %}{{ renderBlock(child) }} +{% endfor %}
+ +{% elseif b.type == "note" or b.type == "tip" or b.type == "warning" %} +
+{% if b.attrs.title %}

{{ b.attrs.title }}

{% endif %} +{% for child in b.blocks %}{{ renderBlock(child) }} +{% endfor %}
+ +{% elseif b.type == "list" %} +{% if b.ordered %}
    {% else %}
{% else %}{% endif %} + +{% elseif b.type == "table" %} + +{% if b.headers is not empty %} +{% for h in b.headers %}{% endfor %} +{% endif %} + +{% for row in b.rows %}{% for cell in row %}{% endfor %} +{% endfor %} +
{{ h|raw }}
{{ cell|raw }}
+ +{# + No "image" branch, deliberately. md_to_json.py never emits a standalone + image block - CommonMark only produces "image" as an inline token, so + `![alt](foo.png)` always lands as an inside the enclosing block's + own "html" string (see md_to_json.py's schema comment, and its block-level + fallback). A branch here rendering an image block's src was unreachable. + + Do not add one back without also widening insert_optimized_media.py's + IMAGE_REF_RE and rewrite_pages' substitutions: both are anchored on an + image reference appearing as an HTML src="..." attribute, which a + bare-field image block would not satisfy - so post-optimization renames + would silently stop being applied to it. +#} +{% elseif b.type == "hr" %} +
+ +{% elseif b.type == "tabs" and b.tabs is not empty %} +{# + Tab switching + the group-key syncing (e.g. picking "Groovy" in one + Kotlin/Groovy/Maven tabs block switches every other tabs block sharing the + same data-group on the page, matching Writerside's data-sync-tabs + behavior) is implemented in assets/tabs.js. md_to_json.py's + _finalize_container always gives every "tabs" block a non-empty "tabs" + list (synthesizing one from code-block languages, or dropping the wrapper + entirely, when the source had no children) - the "b.tabs is not + empty" guard here is just a defensive backstop against any other producer + of this JSON schema making the same mistake, not something this pipeline + itself still needs. +#} +
+
+ {% for tab in b.tabs %}{% set tabKey = tab.attrs["group-key"]|default(tab.title)|default(loop.index) %} + {% endfor %}
+ {% for tab in b.tabs %}{% set tabKey = tab.attrs["group-key"]|default(tab.title)|default(loop.index) %}
+ {% for child in tab.blocks %}{{ renderBlock(child) }} + {% endfor %}
+ {% endfor %} +
+ +{% elseif b.type == "html" %} +{{ b.html|raw }} + +{% elseif b.type == "tab" %} +{# A lone not wrapped in (e.g. seen in eap.json's HTML-table + compatibility layout); render its children rather than dropping them. #} +{% for child in b.blocks %}{{ renderBlock(child) }} +{% endfor %} + +{% else %} +{% if b.html %}{{ b.html|raw }}{% endif %} + +{% endif %} +{% endmacro %} diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_renumber_misnumbered_fragments.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_renumber_misnumbered_fragments.py new file mode 100644 index 00000000..dba7f948 --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_renumber_misnumbered_fragments.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python3 +"""Tests for renumber_misnumbered_fragments.py (ADFA-5171). + +Run directly: python3 test_renumber_misnumbered_fragments.py +""" +import sqlite3 +import tempfile +import unittest +from pathlib import Path + +from populate_db import CHUNK_SIZE +from renumber_misnumbered_fragments import repair + +SCHEMA_SQL = """ +CREATE TABLE Languages (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE); +CREATE TABLE ContentTypes (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE, compression TEXT NOT NULL); +CREATE TABLE Content ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + path TEXT NOT NULL, + languageID INTEGER NOT NULL, + content BLOB NOT NULL, + contentTypeID INTEGER NOT NULL, + templateId INTEGER, + UNIQUE(path) +); +""" + + +def chunk_bytes(n: int, fill: bytes) -> bytes: + return (fill * (n // len(fill) + 1))[:n] + + +class RenumberMisnumberedFragmentsTest(unittest.TestCase): + def setUp(self): + fd, path = tempfile.mkstemp(suffix=".db") + Path(path).unlink(missing_ok=True) + self.db_path = Path(path) + self.conn = sqlite3.connect(self.db_path) + self.conn.executescript(SCHEMA_SQL) + self.conn.execute("INSERT INTO Languages (value) VALUES ('en-US')") + self.conn.execute("INSERT INTO ContentTypes (value, compression) VALUES ('image/gif', 'none')") + self.conn.commit() + + def tearDown(self): + self.conn.close() + self.db_path.unlink(missing_ok=True) + + def insert(self, path: str, content: bytes): + self.conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID) VALUES (?, 1, ?, 1)", + (path, content), + ) + + def all_paths(self) -> set: + return {row[0] for row in self.conn.execute("SELECT path FROM Content")} + + def content_at(self, path: str) -> bytes: + return self.conn.execute("SELECT content FROM Content WHERE path = ?", (path,)).fetchone()[0] + + def test_renumbers_chain_starting_at_minus_2(self): + base = "a/devsite/media/size-range.gif" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-2", chunk_bytes(CHUNK_SIZE, b"B")) + self.insert(f"{base}-3", chunk_bytes(CHUNK_SIZE, b"C")) + self.insert(f"{base}-4", chunk_bytes(CHUNK_SIZE, b"D")) + self.insert(f"{base}-5", b"E" * 100) + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 1) + self.assertEqual(stats["fragments_moved"], 4) + self.assertEqual(stats["chains_gapped"], 0) + self.assertEqual( + self.all_paths(), + {base, f"{base}-1", f"{base}-2", f"{base}-3", f"{base}-4"}, + ) + self.assertEqual(self.content_at(f"{base}-1"), chunk_bytes(CHUNK_SIZE, b"B")) + self.assertEqual(self.content_at(f"{base}-2"), chunk_bytes(CHUNK_SIZE, b"C")) + self.assertEqual(self.content_at(f"{base}-3"), chunk_bytes(CHUNK_SIZE, b"D")) + self.assertEqual(self.content_at(f"{base}-4"), b"E" * 100) + + def test_renumbers_zero_based_chain(self): + """A chain numbered from -0 shifts *up*, where renaming in ascending + order would land on a slot still occupied and trip UNIQUE(path) - + rolling back every other repair in the same pass. It is as broken as a + -2 chain: WebServer.kt probes "-1", finds it, and serves the chain with + "-0" silently dropped.""" + base = "a/devsite/media/zero-based.gif" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-0", chunk_bytes(CHUNK_SIZE, b"B")) + self.insert(f"{base}-1", chunk_bytes(CHUNK_SIZE, b"C")) + self.insert(f"{base}-2", b"D" * 100) + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 1) + self.assertEqual(stats["chains_gapped"], 0) + self.assertEqual(self.all_paths(), {base, f"{base}-1", f"{base}-2", f"{base}-3"}) + # order preserved: -0 -> -1, -1 -> -2, -2 -> -3 + self.assertEqual(self.content_at(f"{base}-1"), chunk_bytes(CHUNK_SIZE, b"B")) + self.assertEqual(self.content_at(f"{base}-2"), chunk_bytes(CHUNK_SIZE, b"C")) + self.assertEqual(self.content_at(f"{base}-3"), b"D" * 100) + + def test_zero_based_chain_does_not_block_other_repairs(self): + """One chain tripping UNIQUE(path) used to roll back the whole run.""" + zero_based = "a/devsite/media/zero.gif" + self.insert(zero_based, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{zero_based}-0", b"B" * 100) + two_based = "a/devsite/media/two.gif" + self.insert(two_based, chunk_bytes(CHUNK_SIZE, b"C")) + self.insert(f"{two_based}-2", b"D" * 100) + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 2) + self.assertEqual(self.all_paths(), + {zero_based, f"{zero_based}-1", two_based, f"{two_based}-1"}) + + def test_single_orphaned_continuation(self): + base = "j/html/api/index-all.html" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-2", b"tail" * 10) + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 1) + self.assertEqual(stats["fragments_moved"], 1) + self.assertEqual(self.all_paths(), {base, f"{base}-1"}) + self.assertEqual(self.content_at(f"{base}-1"), b"tail" * 10) + + def test_correctly_numbered_chain_untouched(self): + base = "k/html/already-fine.html" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-1", chunk_bytes(CHUNK_SIZE, b"B")) + self.insert(f"{base}-2", b"tail") + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 0) + self.assertEqual(stats["fragments_moved"], 0) + self.assertEqual(self.all_paths(), {base, f"{base}-1", f"{base}-2"}) + + def test_idempotent_second_run(self): + base = "a/devsite/media/size-range.gif" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-2", b"tail") + self.conn.commit() + + repair(self.conn) + self.conn.commit() + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 0) + self.assertEqual(stats["fragments_moved"], 0) + + def test_exact_size_file_with_no_continuation_left_alone(self): + path = "k/html/exactly-one-mb.bin" + self.insert(path, chunk_bytes(CHUNK_SIZE, b"A")) + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 0) + self.assertEqual(stats["chains_gapped"], 0) + self.assertEqual(self.all_paths(), {path}) + + def test_chain_with_real_gap_reported_and_left_untouched(self): + base = "k/html/actually-missing-a-chunk.html" + self.insert(base, chunk_bytes(CHUNK_SIZE, b"A")) + self.insert(f"{base}-2", chunk_bytes(CHUNK_SIZE, b"B")) + self.insert(f"{base}-4", b"tail") # -3 is genuinely missing + self.conn.commit() + + stats = repair(self.conn) + self.conn.commit() + + self.assertEqual(stats["chains_renumbered"], 0) + self.assertEqual(stats["chains_gapped"], 1) + self.assertEqual(self.all_paths(), {base, f"{base}-2", f"{base}-4"}) + + +if __name__ == "__main__": + unittest.main() diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_find_missing_assets.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_find_missing_assets.py new file mode 100644 index 00000000..33c79c32 --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_find_missing_assets.py @@ -0,0 +1,73 @@ +"""Regression test for find_missing_assets.py's exit-code behavior (PR #24 +review). Runs the script as a subprocess against the real md_to_json.py +(merged from ADFA-5039); a non-UTF-8 .md file gives convert_file a genuine +reason to raise, matching the pattern md_to_json.py's own test suite uses +for its equivalent main()-exit-code tests. +""" +import subprocess +import sys +from pathlib import Path + +import find_missing_assets as fma + + +def _write_minimal_docs_root(tmp_path, *, with_failure=False): + docs_root = tmp_path / "docs" + (docs_root / "topics").mkdir(parents=True) + (docs_root / "topics" / "good.md").write_text("# Good\n\nHello.\n", encoding="utf-8") + if with_failure: + # Not valid UTF-8 - convert_file's read_text(encoding="utf-8") raises. + (docs_root / "topics" / "bad.md").write_bytes(b"\xff\xfe not utf-8") + return docs_root + + +def _run(*args): + script = Path(__file__).resolve().parent.parent / "find_missing_assets.py" + return subprocess.run([sys.executable, str(script), *map(str, args)], capture_output=True, text=True) + + +def test_exits_zero_and_reports_zero_failures_when_nothing_fails(tmp_path): + docs_root = _write_minimal_docs_root(tmp_path) + report = tmp_path / "report.md" + result = _run(docs_root, report) + assert result.returncode == 0 + assert "0 file(s) failed to scan" in report.read_text(encoding="utf-8") + + +def test_exits_nonzero_when_a_file_fails_to_scan(tmp_path): + """A per-file scan failure used to be printed to stderr and otherwise + ignored - the report still claimed a clean summary and the process + still exited 0, so a totally broken corpus was indistinguishable from a + clean one (this is the pre-flight gate run before populate_db.py). The + bad file is scanned by two independent passes (the main conversion loop + and find_include_warnings' own scan), so it counts twice.""" + docs_root = _write_minimal_docs_root(tmp_path, with_failure=True) + report = tmp_path / "report.md" + result = _run(docs_root, report) + assert result.returncode == 1 + text = report.read_text(encoding="utf-8") + assert "2 file(s) failed to scan" in text + assert "incomplete" in text.lower() + + +def test_allow_failures_exits_zero_despite_failure(tmp_path): + docs_root = _write_minimal_docs_root(tmp_path, with_failure=True) + report = tmp_path / "report.md" + result = _run(docs_root, report, "--allow-failures") + assert result.returncode == 0 + + +def test_find_include_warnings_reports_failure_instead_of_raising(tmp_path): + """find_include_warnings had its own unguarded read_text(encoding="utf-8") + outside the main loop's try/except - a non-UTF-8 file crashed the whole + process with an uncaught traceback, bypassing --allow-failures entirely + rather than being counted as a scan failure like every other file-read + in this script.""" + topics_dir = tmp_path / "topics" + topics_dir.mkdir() + (topics_dir / "good.md").write_text("no includes here\n", encoding="utf-8") + (topics_dir / "bad.md").write_bytes(b"\xff\xfe not utf-8") + + warnings, failed = fma.find_include_warnings(topics_dir) + assert warnings == [] + assert failed == 1 diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_insert_optimized_media.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_insert_optimized_media.py new file mode 100644 index 00000000..07c71200 --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_insert_optimized_media.py @@ -0,0 +1,159 @@ +"""Regression tests for insert_optimized_media.py's destructive paths. + +Every test here covers a way this script could delete a row it shouldn't - +the LIKE-wildcard over-match in delete_content, and the "no pages to check +against" case that would otherwise wipe the whole image corpus. +""" +import shutil +import sqlite3 + +import pytest + +from insert_optimized_media import ( + IMAGES_URL_PREFIX, + collect_referenced_media, + delete_content, + delete_unreferenced_media, +) +from content_chunking import CHUNK_SIZE +from optimize_media import Logger +from populate_db import DictionaryCompressor + +SCHEMA = """ +CREATE TABLE Languages (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE); +CREATE TABLE ContentTypes (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE, + compression TEXT NOT NULL); +CREATE TABLE Content ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + path TEXT NOT NULL, + languageID INTEGER NOT NULL, + content BLOB NOT NULL, + contentTypeID INTEGER NOT NULL, + templateId INTEGER NOT NULL DEFAULT 0, + UNIQUE(path) +); +""" + +PAGE_TYPE_ID = 12 +# Content is dictionary-compressed from schema 2.0.0 on (ADFA-5153), and +# insert_optimized_media reads pages back through that dictionary, so these +# fixtures have to speak it too. +DICTIONARY = bytes(range(256)) * 64 +needs_brotli_cli = pytest.mark.skipif(shutil.which("brotli") is None, reason="brotli CLI not installed") + +pytestmark = needs_brotli_cli + + +@pytest.fixture +def compressor(): + instance = DictionaryCompressor(DICTIONARY) + yield instance + instance.close() + + +@pytest.fixture +def conn(): + connection = sqlite3.connect(":memory:") + connection.executescript(SCHEMA) + connection.execute("INSERT INTO Languages (id, value) VALUES (1, 'en-US')") + connection.execute("INSERT INTO ContentTypes (id, value, compression) VALUES (?, 'text/html', 'brotli')", + (PAGE_TYPE_ID,)) + yield connection + connection.close() + + +# A base row only owns continuation fragments when it is exactly CHUNK_SIZE +# bytes - that is what tells the server (and every tool here) the row was +# split rather than merely sharing a name with an unrelated page. Fixtures +# that chain fragments off a 1-byte base describe a shape that cannot occur, +# and would pass whether or not the code honoured that rule. +CHUNKED_BASE = b"x" * CHUNK_SIZE + + +def add_row(conn, path, blob=b"x", template_id=0): + conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID, templateId) VALUES (?, 1, ?, ?, ?)", + (path, blob, PAGE_TYPE_ID, template_id), + ) + + +def paths(conn): + return {row[0] for row in conn.execute("SELECT path FROM Content")} + + +def page_blob(compressor, *image_names): + """A stored page blob referencing each image the way md_to_json bakes it + in: an HTML src="..." attribute inside JSON, so the filename is followed + by an escaped quote.""" + srcs = "".join(f'' for name in image_names) + return compressor.compress(f'{{"blocks":[{{"html":"{srcs}"}}]}}'.encode("utf-8")) + + +class TestDeleteContent: + def test_removes_the_row_and_its_chunk_fragments(self, conn): + add_row(conn, "k/html/images/big.png", CHUNKED_BASE) + add_row(conn, "k/html/images/big.png-1") + add_row(conn, "k/html/images/big.png-2") + add_row(conn, "k/html/images/other.png") + + delete_content(conn, "k/html/images/big.png") + + assert paths(conn) == {"k/html/images/other.png"} + + def test_underscore_is_not_treated_as_a_wildcard(self, conn): + # "_" in a LIKE pattern matches any single character, so an unescaped + # "k/html/_nav.html-%" would also match "k/html/Xnav.html-1" and take + # an unrelated page's chunk fragment with it. + add_row(conn, "k/html/_nav.html", CHUNKED_BASE) + add_row(conn, "k/html/_nav.html-1") + add_row(conn, "k/html/Xnav.html", CHUNKED_BASE) + add_row(conn, "k/html/Xnav.html-1") + + delete_content(conn, "k/html/_nav.html") + + assert paths(conn) == {"k/html/Xnav.html", "k/html/Xnav.html-1"} + + def test_percent_is_not_treated_as_a_wildcard(self, conn): + add_row(conn, "k/html/images/100%.png", CHUNKED_BASE) + add_row(conn, "k/html/images/100%.png-1") + add_row(conn, "k/html/images/100-other.png-1") + + delete_content(conn, "k/html/images/100%.png") + + assert paths(conn) == {"k/html/images/100-other.png-1"} + + +class TestDeleteUnreferencedMedia: + def test_removes_only_images_no_page_references(self, conn, compressor): + add_row(conn, "k/html/page.html", page_blob(compressor, "kept.png"), template_id=2) + add_row(conn, "k/html/images/kept.png") + add_row(conn, "k/html/images/orphan.png", CHUNKED_BASE) + add_row(conn, "k/html/images/orphan.png-1") + + removed = delete_unreferenced_media(conn, PAGE_TYPE_ID, Logger(None), compressor) + + assert removed == 1 + assert paths(conn) == {"k/html/page.html", "k/html/images/kept.png"} + + def test_refuses_to_run_when_no_page_references_any_image(self, conn, compressor): + # populate_db.py hasn't written its pages yet (or was skipped): the + # reference scan comes back empty and every stored image looks like + # garbage. Deleting the whole corpus is never what was meant. + add_row(conn, "k/html/images/a.png") + add_row(conn, "k/html/images/b.png") + + with pytest.raises(RuntimeError, match="no page references any image"): + delete_unreferenced_media(conn, PAGE_TYPE_ID, Logger(None), compressor) + + assert paths(conn) == {"k/html/images/a.png", "k/html/images/b.png"} + + def test_empty_database_is_not_an_error(self, conn, compressor): + assert delete_unreferenced_media(conn, PAGE_TYPE_ID, Logger(None), compressor) == 0 + + def test_untemplated_rows_are_not_scanned_for_references(self, conn, compressor): + # templateId 0 marks a raw asset, not a page; only page/nav rows carry + # the JSON that image references live in. + add_row(conn, "k/html/page.html", page_blob(compressor, "kept.png"), template_id=2) + add_row(conn, "k/html/images/kept.png") + + assert collect_referenced_media(conn, PAGE_TYPE_ID, compressor) == {"kept.png"} diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_review_findings.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_review_findings.py new file mode 100644 index 00000000..f6bc0d9c --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/tests/test_review_findings.py @@ -0,0 +1,1205 @@ +"""Regression tests for the PR #24 review findings (F01-F15). + +Each test constructs the specific input the reviewer identified as untested - +the shapes nothing else in the suite feeds these functions. Every one of them +fails against the code as it stood before the corresponding fix, which is the +only reason they are worth having: the two criticals in particular were silent, +exit-0 data loss that truthful-looking statistics actively concealed. +""" +import json +import sqlite3 +import sys + +import brotli +import pytest +from PIL import Image, ImageDraw + +import optimize_media as om +from build_nav import load_page_index +from insert_optimized_media import reassemble_content +from migrate_content_to_dictionary_brotli import is_chunked_base, load_base_rows, write_item +from populate_db import CHUNK_SIZE +from renumber_misnumbered_fragments import find_chains, find_fragment_paths + +SCHEMA_SQL = """ +CREATE TABLE Languages (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE); +CREATE TABLE ContentTypes (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE, + compression TEXT NOT NULL); +CREATE TABLE Content ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + path TEXT NOT NULL, + languageID INTEGER NOT NULL, + content BLOB NOT NULL, + contentTypeID INTEGER NOT NULL, + templateId INTEGER, + UNIQUE(path) +); +""" + + +def _new_stats(): + return {"raster": 0, "svg": 0, "svg_rasterized": 0, "copied": 0, "errors": 0, + "original_bytes": 0, "optimized_bytes": 0} + + +@pytest.fixture +def conn(): + connection = sqlite3.connect(":memory:") + connection.executescript(SCHEMA_SQL) + connection.execute("INSERT INTO Languages (value) VALUES ('en-US')") + connection.execute("INSERT INTO ContentTypes (value, compression) VALUES ('text/html', 'brotli')") + yield connection + connection.close() + + +def _insert(conn, path, blob): + conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID, templateId) VALUES (?, 1, ?, 1, 0)", + (path, blob), + ) + + +# --- F01: two sources colliding on a rewritten extension --------------------- + +def test_sources_differing_only_by_extension_both_survive(tmp_path): + """logo.png + logo.jpg both become logo.webp, and the loser used to be + silently gone - with errors 0 and both pages repointed at the survivor.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (40, 30), (200, 30, 30)).save(src / "logo.png") + Image.new("RGB", (40, 30), (30, 30, 200)).save(src / "logo.jpg") + + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + renamed = om.optimize_directory(src, out, cfg=cfg, pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()).renamed + + assert len(list(out.iterdir())) == 2, "one source was clobbered by the other" + # Both renames are reported, so a caller rewriting stored URLs follows them. + assert set(renamed) == {"logo.png", "logo.jpg"} + assert len(set(renamed.values())) == 2, "both sources still map to one output" + + +def test_three_way_extension_collision_all_survive(tmp_path): + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + for name, colour in (("logo.png", (1, 1, 1)), ("logo.jpg", (2, 2, 2)), ("logo.gif", (3, 3, 3))): + Image.new("RGB", (20, 20), colour).save(src / name) + + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + om.optimize_directory(src, out, cfg=cfg, pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()) + + assert len(list(out.iterdir())) == 3 + + +def test_non_colliding_names_keep_their_own_stems(tmp_path): + """The de-confliction must not rename anything that didn't collide.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (5, 5, 5)).save(src / "alpha.png") + Image.new("RGB", (20, 20), (6, 6, 6)).save(src / "beta.png") + + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()) + + assert sorted(p.name for p in out.iterdir()) == ["alpha.png", "beta.png"] + + +# --- F07: optimizing a directory into itself --------------------------------- + +def test_optimizing_into_the_input_directory_is_refused(tmp_path): + """Used to destroy the originals in place; the only error raised was a + copy2 SameFileError on the first non-image file, long after the damage.""" + Image.new("RGB", (400, 300), (10, 200, 10)).save(tmp_path / "logo.png") + before = (tmp_path / "logo.png").read_bytes() + + with pytest.raises(ValueError, match="input directory"): + om.optimize_directory(tmp_path, tmp_path, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert (tmp_path / "logo.png").read_bytes() == before + + +# --- F02: an unrelated "X-1" page alongside "X" ------------------------------- + +def test_independent_page_named_like_a_fragment_is_not_a_continuation(conn): + _insert(conn, "k/html/guide.html", brotli.compress(b"the base page, comfortably under one chunk")) + _insert(conn, "k/html/guide.html-1", brotli.compress(b"a wholly unrelated page")) + + scanned = {row[0] for row in load_base_rows(conn)} + assert "k/html/guide.html-1" in scanned, "victim was invisible to the migration entirely" + + +def test_write_item_does_not_delete_an_unrelated_lookalike_page(conn): + victim = brotli.compress(b"a wholly unrelated page") + _insert(conn, "k/html/guide.html", brotli.compress(b"the base page")) + _insert(conn, "k/html/guide.html-1", victim) + + write_item(conn, "k/html/guide.html", 1, 1, 0, brotli.compress(b"rewritten base page")) + + row = conn.execute("SELECT content FROM Content WHERE path = 'k/html/guide.html-1'").fetchone() + assert row is not None, "unrelated page deleted as a surplus fragment" + assert row[0] == victim, "unrelated page overwritten" + + +def test_genuinely_chunked_base_still_owns_its_continuations(conn): + """The length gate must not break real chunking - including an ADFA-5171 + chain numbered from -2, which has no -1 at all.""" + _insert(conn, "k/html/big.html", b"x" * CHUNK_SIZE) + _insert(conn, "k/html/big.html-2", b"y" * 10) + + scanned = {row[0] for row in load_base_rows(conn)} + assert "k/html/big.html-2" not in scanned, "real continuation treated as its own page" + assert is_chunked_base({"k/html/big.html": CHUNK_SIZE}, "k/html/big.html") + assert not is_chunked_base({"k/html/guide.html": 42}, "k/html/guide.html") + + +# --- F06: reassembly of a chain numbered from -2 ------------------------------ + +def test_reassemble_content_handles_a_chain_numbered_from_two(conn): + """Probing "-1" first returned a truncated stream for the exact shape + renumber_misnumbered_fragments.py exists to repair.""" + tail = b"z" * 20 + _insert(conn, "k/html/images/big.png", b"a" * CHUNK_SIZE) + _insert(conn, "k/html/images/big.png-2", tail) + + assembled = reassemble_content(conn, "k/html/images/big.png", b"a" * CHUNK_SIZE) + assert assembled == b"a" * CHUNK_SIZE + tail + + +# --- F08: a chain with an interior gap --------------------------------------- + +def test_chain_with_interior_gap_is_reported_as_gapped(conn): + """p-1, p-2, p-4 starts at 1, so it used to short-circuit as healthy and be + reported as "0 chain(s) had a real gap" on a truncated page.""" + _insert(conn, "k/html/page.html", b"a" * CHUNK_SIZE) + for n in (1, 2, 4): + _insert(conn, f"k/html/page.html-{n}", b"a" * (CHUNK_SIZE if n != 4 else 10)) + + misnumbered, gapped = find_chains(conn, find_fragment_paths(conn)) + + assert [path for path, _f in gapped] == ["k/html/page.html"] + assert misnumbered == [] + + +def test_contiguous_chain_from_one_is_left_alone(conn): + _insert(conn, "k/html/page.html", b"a" * CHUNK_SIZE) + _insert(conn, "k/html/page.html-1", b"a" * 10) + + misnumbered, gapped = find_chains(conn, find_fragment_paths(conn)) + assert misnumbered == [] and gapped == [] + + +# --- F03: build_nav reading its own nav.json --------------------------------- + +def test_load_page_index_skips_the_generated_nav_json(tmp_path): + """The documented invocation passes output_dir as the scan dir, so a second + run read its own nav.json - a top-level array - and died on list.get.""" + (tmp_path / "page.json").write_text(json.dumps({"id": "k/html/a", "title": "A"}), encoding="utf-8") + (tmp_path / "nav.json").write_text(json.dumps([{"id": "k/html/a", "children": []}]), encoding="utf-8") + + stem_to_id, id_to_title = load_page_index(tmp_path) + + assert stem_to_id == {"a": "k/html/a"} + assert id_to_title == {"k/html/a": "A"} + + +# ============================================================================= +# Second review round (F01-F15). Several of these are regressions introduced by +# the fixes above - the chunking protocol in particular was re-derived at four +# call sites, each wrong differently, which is why it now lives in one module. +# ============================================================================= + +from content_chunking import owned_fragment_paths, served_fragment_paths # noqa: E402 + + +# --- F13: a short fragment terminates the chain, as WebServer.kt does -------- + +def test_reassembly_stops_at_the_short_fragment(conn): + """A gapped chain (p-1 full, p-2 short, p-4 orphaned) is served as + p + p-1 + p-2. Concatenating the whole discovered chain instead produces + bytes the server never had - which rewrite_pages would re-compress and + store.""" + full = b"a" * CHUNK_SIZE + _insert(conn, "p", full) + _insert(conn, "p-1", full) + _insert(conn, "p-2", b"short") + _insert(conn, "p-4", b"orphaned tail") + + assert served_fragment_paths(conn, "p") == ["p-1", "p-2"] + assert reassemble_content(conn, "p", full) == full + full + b"short" + + +def test_ownership_still_includes_the_orphaned_tail(conn): + """Deleting or replacing a base must take everything named after it, or + the tail is orphaned - a different question from what gets served.""" + full = b"a" * CHUNK_SIZE + _insert(conn, "p", full) + _insert(conn, "p-1", b"short") + _insert(conn, "p-4", b"orphaned tail") + + assert owned_fragment_paths(conn, "p") == ["p-1", "p-4"] + + +# --- F08: a real chain hidden behind an unrelated lookalike base ------------- + +def test_misnumbered_chain_behind_a_lookalike_base_is_found(conn): + """An ordinary page at "guide.html" must not make a genuinely chunked, + misnumbered page at "guide.html-1" look like its fragment - that hid the + chain from the tool written to repair it, reporting a clean run.""" + _insert(conn, "k/html/guide.html", b"an ordinary small page") + _insert(conn, "k/html/guide.html-1", b"a" * CHUNK_SIZE) + _insert(conn, "k/html/guide.html-1-2", b"tail") + + fragments = find_fragment_paths(conn) + misnumbered, _gapped = find_chains(conn, fragments) + + assert "k/html/guide.html-1" not in fragments + assert [path for path, _f in misnumbered] == ["k/html/guide.html-1"] + + +# --- F04/F05: de-confliction keyed on the predicted output, flat namespace --- + +def test_same_basename_in_different_directories_deconflicts(tmp_path): + """insert_optimized_media addresses images by bare basename, so + sub-a/logo.png and sub-b/logo.jpg both land on logo.webp under --webp + even though they are in different source directories.""" + src, out = tmp_path / "in", tmp_path / "out" + (src / "sub-a").mkdir(parents=True) + (src / "sub-b").mkdir(parents=True) + out.mkdir() + Image.new("RGB", (20, 20), (200, 0, 0)).save(src / "sub-a" / "logo.png") + Image.new("RGB", (20, 20), (0, 0, 200)).save(src / "sub-b" / "logo.jpg") + + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + basenames = [p.name for p in out.rglob("*") if p.is_file()] + assert len(basenames) == 2 + assert len(set(basenames)) == 2, "collide once flattened to bare basenames" + + +def test_no_rename_when_the_extension_cannot_change(tmp_path): + """Without --webp neither encoder rewrites an extension, so logo.png and + logo.jpg cannot collide - renaming anyway put a bogus entry in `renamed`, + which rewrote every stored URL and churned the row for nothing.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (1, 1, 1)).save(src / "logo.png") + Image.new("RGB", (20, 20), (2, 2, 2)).save(src / "logo.jpg") + (src / "notes.txt").write_text("x") + (src / "notes.md").write_text("y") + + renamed = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()).renamed + + assert renamed == {} + assert sorted(p.name for p in out.iterdir()) == ["logo.jpg", "logo.png", "notes.md", "notes.txt"] + + +# --- F10: a broken symlink is one file's error, not the run's --------------- + +def test_broken_symlink_does_not_abort_the_run(tmp_path): + """`not p.is_dir()` keeps dangling symlinks, and process_file used to stat + outside its try - so one of them escaped as an unhandled FileNotFoundError + past insert_optimized_media's ValueError-only catch.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (9, 9, 9)).save(src / "real.png") + (src / "dangling.png").symlink_to(src / "nonexistent.png") + + stats = _new_stats() + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=stats) + + assert stats["errors"] == 1 + assert sorted(p.name for p in out.iterdir()) == ["real.png"] + + +# ============================================================================= +# Third review round. Both of these are defects in the fixes above: the shared +# chunking module implemented half of its own stated terminator rule, and the +# dangling-nav-link fix was applied to the database path but not the static one. +# ============================================================================= + +from build_nav import build_node, drop_unreachable_ids, render_node # noqa: E402 + + +# --- reassembly must stop at a gap, not just at a short fragment ------------- + +def test_reassembly_stops_at_a_gap_in_the_numbering(conn): + """The server probes consecutive suffixes and stops at the first miss. + Jumping the hole appends a tail it never reaches, which insert_optimized_media + would then store back or die decompressing.""" + full = b"a" * CHUNK_SIZE + _insert(conn, "p", full) + _insert(conn, "p-1", full) + _insert(conn, "p-2", full) + _insert(conn, "p-4", b"tail the server never reaches") + + assert served_fragment_paths(conn, "p") == ["p-1", "p-2"] + assert reassemble_content(conn, "p", full) == full + full + full + # ownership is unchanged: a delete still has to take the orphaned tail + assert owned_fragment_paths(conn, "p") == ["p-1", "p-2", "p-4"] + + +def test_adfa_5171_chain_from_two_is_still_read_whole(conn): + """Contiguity is enforced from wherever the chain starts, not from 1 - the + repair and migration tooling has to be able to read a -2 chain whole.""" + full = b"a" * CHUNK_SIZE + _insert(conn, "p", full) + _insert(conn, "p-2", full) + _insert(conn, "p-3", b"end") + + assert served_fragment_paths(conn, "p") == ["p-2", "p-3"] + + +def test_gap_inside_a_misnumbered_chain_still_terminates(conn): + full = b"a" * CHUNK_SIZE + _insert(conn, "p", full) + _insert(conn, "p-2", full) + _insert(conn, "p-5", b"tail") + + assert served_fragment_paths(conn, "p") == ["p-2"] + + +# --- the static nav path must agree with the database one ------------------- + +def test_render_node_emits_a_group_title_when_the_page_is_missing(): + """nav.peb and render_node both branch on the id, so a synthesized id for + an unconverted *.topic renders as a live to a URL that 404s.""" + node = {"title": "Overview", "id": None, "hidden": False, + "noLinkColor": "#999999", "children": []} + html = render_node(node) + + assert 'class="nav-group-title"' in html + assert "' + '' + '' + '' + ) + warnings = [] + nav = [build_node(tree, {"real": "k/html/real"}, {}, warnings, "#999999", id_prefix="k/html/")] + + cleared = drop_unreachable_ids(nav, {"k/html/real"}) + + assert cleared == ["k/html/api-references"] + overview, real = nav[0]["children"] + assert overview["id"] is None and "') + warnings = [] + node = build_node(el, {}, {}, warnings, "#999999", id_prefix="k/html/") + + assert node["id"] == "k/html/api-references" + assert node["noLinkColor"] == "#999999" + + +# ============================================================================= +# Fourth review round (F26-F34). +# ============================================================================= +import shutil # noqa: E402 +import subprocess # noqa: E402 +from pathlib import Path # noqa: E402 + +import insert_optimized_media as iom # noqa: E402 +from insert_optimized_media import ( # noqa: E402 + delete_unreferenced_media, + list_stored_media, + rewrite_pages, +) +from find_missing_assets import INCLUDE_RE, outside_fences # noqa: E402 +from md_to_json import Converter, build_tree, make_markdown_it # noqa: E402 +from populate_db import DictionaryCompressor, pages_linking_to # noqa: E402 + +needs_brotli_cli = pytest.mark.skipif(shutil.which("brotli") is None, reason="brotli CLI not installed") + + +def _animated_gif(path, frames=3, size=(40, 40), loop=None): + """An animated GIF, with a NETSCAPE loop block only if `loop` is given - + Pillow signals "plays once" by omitting the key entirely. + + Each frame draws a rectangle in a different place: frames that are + byte-identical get collapsed on save, which quietly produces a + single-frame "animation" that proves nothing.""" + images = [] + for i in range(frames): + frame = Image.new("RGB", size, (0, 0, 0)) + ImageDraw.Draw(frame).rectangle([i * 5, i * 5, i * 5 + 10, i * 5 + 10], fill=(255, i * 80, 0)) + images.append(frame.convert("P")) + kwargs = {"save_all": True, "append_images": images[1:], "duration": 80} + if loop is not None: + kwargs["loop"] = loop + images[0].save(path, **kwargs) + return path + + +# --- F26: an animated GIF must not be planned as, or replaced by, a .webp ---- + +def test_animated_gif_survives_a_webp_run(tmp_path): + """--webp cannot re-encode an animated GIF (optimize_raster resizes it as + a GIF instead), so predicting a ".webp" output for it left the animation + stored under a name nothing referenced - and rewrite_pages repointed every + page at a .webp that was never written.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + _animated_gif(src / "spin.gif") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert result.renamed == {}, "nothing was renamed, so no page URL should be rewritten" + assert [p.name for p in out.iterdir()] == ["spin.gif"] + with Image.open(out / "spin.gif") as written: + assert getattr(written, "n_frames", 1) == 3 + + +def test_still_gif_is_still_converted_to_webp(tmp_path): + """The animated-GIF exemption is exactly that: a single-frame GIF keeps + converting, so the fix doesn't quietly opt every GIF out of --webp.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("P", (40, 40), 7).save(src / "static.gif") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert [p.name for p in out.iterdir()] == ["static.webp"] + assert result.renamed == {"static.gif": "static.webp"} + + +def test_duplicate_basenames_keep_the_first_without_repointing_it(tmp_path): + """Two sources with the *same* basename flatten onto one stored image. + De-conflicting them (a/logo.png -> logo.png, b/logo.png -> logo-2.png) + invented a rename for a file that was never written under the new name; + keeping the first and skipping the rest matches what the insert loop + downstream does with a duplicate.""" + src, out = tmp_path / "in", tmp_path / "out" + (src / "a").mkdir(parents=True) + (src / "b").mkdir(parents=True) + out.mkdir() + Image.new("RGB", (20, 20), (9, 9, 9)).save(src / "a" / "logo.png") + Image.new("RGB", (20, 20), (8, 8, 8)).save(src / "b" / "logo.png") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + written = [p for p in out.rglob("*") if p.is_file()] + assert [p.name for p in written] == ["logo.png"] + assert result.renamed == {}, "the skipped duplicate must not repoint the surviving name" + + +# --- F27: the insert loop follows what was written, not what is lying about -- + +def test_written_lists_only_this_run_s_outputs(tmp_path): + """A work directory is a documented positional, so it can hold files from + an earlier run whose sources are since gone. Those must not be reported as + written - the insert loop reads this list, and an rglob would resurrect + them.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (1, 2, 3)).save(src / "current.png") + (out / "deleted-last-week.png").write_bytes(b"stale") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert [p.name for p in result.written] == ["current.png"] + assert (out / "deleted-last-week.png").exists(), "left alone on disk, just not re-inserted" + + +# --- F31: a play-once GIF must not come back looping forever ----------------- + +def test_resize_preserves_a_play_once_gif(tmp_path): + """info.get("loop", 0) read "plays once" (key absent) and wrote "loops + forever" (0), adding a NETSCAPE block the source never had.""" + src = _animated_gif(tmp_path / "once.gif", size=(300, 300)) + with Image.open(src) as img: + assert "loop" not in img.info + resized = om.resize_animated_gif(img, tmp_path / "out.gif", max_width=100) + with Image.open(resized) as written: + assert "loop" not in written.info + assert written.n_frames == 3 + + +def test_resize_preserves_an_explicit_loop_count(tmp_path): + src = _animated_gif(tmp_path / "thrice.gif", size=(300, 300), loop=3) + with Image.open(src) as img: + resized = om.resize_animated_gif(img, tmp_path / "out.gif", max_width=100) + with Image.open(resized) as written: + assert written.info.get("loop") == 3 + + +# --- F29: pages converted before a failure still link to the failed page ----- + +def test_pages_linking_to_finds_the_pre_failure_pages(): + """Only pages converted before the failed stem was dropped carry the + resolved href; ones converted after keep the raw ".md" and are + already styled broken, so they must not be re-converted.""" + pages = [ + {"id": "before", "blocks": [{"type": "paragraph", "html": 'x'}]}, + {"id": "after", "blocks": [{"type": "paragraph", "html": 'x'}]}, + {"id": "nested", "blocks": [{"type": "blockquote", "blocks": [ + {"type": "paragraph", "html": 'x'}]}]}, + {"id": "unrelated", "blocks": [{"type": "paragraph", "html": 'x'}]}, + ] + + assert pages_linking_to(pages, ["gone"]) == [0, 2] + assert pages_linking_to(pages, []) == [] + assert pages_linking_to(pages, ["never-existed"]) == [] + + +# --- F28: page.peb depends on images never being a block type of their own --- + +def test_markdown_images_stay_inline_never_a_block(): + """templates/page.peb has no "image" branch, and IMAGE_REF_RE / rewrite_pages + are anchored on src="..." - all three rest on this. + + Goes through convert_nodes rather than convert_file because convert_file + wants a path on disk, and the block schema is what's under test.""" + converter = Converter(make_markdown_it(), {}, {}, {"a.png": "a.png"}, image_url_prefix="/k/html/images/") + tokens = converter.md.parse("Text with ![alt](a.png) inline.\n\n![alt](a.png)\n") + blocks = converter.convert_nodes(build_tree(tokens)) + + assert all(b["type"] != "image" for b in blocks), "a standalone image block would render as nothing" + assert any('src="/k/html/images/a.png"' in b.get("html", "") for b in blocks) + + +# --- F30/F34: reading pages and media back exactly once ---------------------- + +PAGE_TYPE_ID = 1 # the conn fixture's only ContentType +DICTIONARY = bytes(range(256)) * 64 + + +@pytest.fixture +def compressor(): + instance = DictionaryCompressor(DICTIONARY) + yield instance + instance.close() + + +def _page_blob(compressor, *image_names): + """A page row's stored bytes: the JSON md_to_json would produce, with each + image referenced the only way it ever is - as an HTML src attribute.""" + blocks = [{"type": "paragraph", "html": f''} + for name in image_names] + text = json.dumps({"id": "p", "blocks": blocks}) + return compressor.compress(text.encode("utf-8")) + + +def _add(conn, path, blob=b"x", template_id=0): + conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID, templateId) VALUES (?, 1, ?, ?, ?)", + (path, blob, PAGE_TYPE_ID, template_id), + ) + + +@needs_brotli_cli +def test_pages_are_decompressed_once_per_run(conn, compressor): + """rewrite_pages and collect_referenced_media selected the same rows and + decompressed each of them independently. Every decompression is a `brotli` + subprocess (the shared-dictionary path has no Python binding), so the + second pass cost one process spawn per page - 268 of them, 2.3s, on the + real corpus, for a set of references the first pass had already seen.""" + # A real rename, so the old code took both passes: four decompressions to + # rewrite, then four more to collect what the rewritten pages reference. + for i in range(4): + _add(conn, f"k/html/page{i}.html", _page_blob(compressor, "logo.png"), template_id=2) + _add(conn, "k/html/images/logo.webp") + _add(conn, "k/html/images/orphan.png") + + calls = [] + real_decompress = compressor.decompress + compressor.decompress = lambda data: (calls.append(1), real_decompress(data))[1] + + rewritten = rewrite_pages(conn, {"logo.png": "logo.webp"}, 1, PAGE_TYPE_ID, om.Logger(None), [], compressor) + removed = delete_unreferenced_media(conn, PAGE_TYPE_ID, om.Logger(None), compressor, + referenced=rewritten.referenced) + + assert rewritten.changed == 4 + assert rewritten.referenced == {"logo.webp"}, "collected post-substitution, as the pages now read" + assert removed == 1, "orphan.png is referenced by nothing" + assert len(calls) == 4, f"one decompression per page row, got {len(calls)}" + + +@needs_brotli_cli +def test_an_empty_rename_map_rewrites_nothing(conn, compressor): + """Dropping rewrite_pages' "nothing to rename, return now" shortcut (so its + single pass can also collect references) must not turn an empty rename_map + into a regex that matches everywhere: re.compile("") matches at every + position, which would have rewritten every page to no effect.""" + _add(conn, "k/html/page.html", _page_blob(compressor, "a.png"), template_id=2) + before = conn.execute("SELECT content FROM Content WHERE path = 'k/html/page.html'").fetchone()[0] + + rewritten = rewrite_pages(conn, {}, 1, PAGE_TYPE_ID, om.Logger(None), [], compressor) + + assert rewritten.changed == 0 + assert conn.execute("SELECT content FROM Content WHERE path = 'k/html/page.html'").fetchone()[0] == before + + +def test_an_image_named_like_a_fragment_is_still_listed(conn): + """An image genuinely called "diagram.png-1", stored beside an ordinary + (not CHUNK_SIZE) "diagram.png", read as that row's continuation and + vanished from the listing - so delete_unreferenced_media could neither see + it nor remove it. The base row's length is what settles it.""" + _add(conn, "k/html/images/diagram.png", b"small") + _add(conn, "k/html/images/diagram.png-1", b"a separate file that just looks like a fragment") + + assert set(list_stored_media(conn)) == {"diagram.png", "diagram.png-1"} + + +def test_a_real_continuation_is_still_collapsed(conn): + """The other direction: a genuinely chunked image is one entry, not two.""" + _add(conn, "k/html/images/big.png", b"x" * CHUNK_SIZE) + _add(conn, "k/html/images/big.png-1", b"tail") + + assert set(list_stored_media(conn)) == {"big.png"} + + +# --- F32: no ~250MB backup for a run that has nothing to repair -------------- + +def _repair_db(tmp_path, rows): + path = tmp_path / "documentation.db" + conn = sqlite3.connect(path) + conn.executescript(SCHEMA_SQL) + conn.execute("INSERT INTO Languages (value) VALUES ('en-US')") + conn.execute("INSERT INTO ContentTypes (value, compression) VALUES ('text/html', 'brotli')") + for row_path, blob in rows: + conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID, templateId) VALUES (?, 1, ?, 1, 0)", + (row_path, blob), + ) + conn.commit() + conn.close() + return path + + +def _run_renumber_main(monkeypatch, db_path): + """Runs the script's main() against db_path, reporting whether it took a + backup.""" + import renumber_misnumbered_fragments as rmf + backups = [] + monkeypatch.setattr(rmf, "backup_database", lambda p: (backups.append(p), Path(f"{p}.bak"))[1]) + monkeypatch.setattr(sys, "argv", ["renumber_misnumbered_fragments.py", str(db_path)]) + rmf.main() + return backups + + +def test_a_clean_database_is_not_backed_up(tmp_path, monkeypatch, capsys): + """This script is documented as idempotent, so re-running it against + production is the normal case and finds nothing to do. Backing up first + wrote another full VACUUM INTO copy of a ~250MB file every single time.""" + db_path = _repair_db(tmp_path, [("k/html/page.html", b"small"), + ("k/html/big.html", b"x" * CHUNK_SIZE), + ("k/html/big.html-1", b"tail")]) + + assert _run_renumber_main(monkeypatch, db_path) == [] + assert "Renumbered 0 chain(s)" in capsys.readouterr().out + + +def test_a_database_needing_repair_is_backed_up(tmp_path, monkeypatch): + """The other direction: skipping the backup must not extend to the run + that actually rewrites paths.""" + db_path = _repair_db(tmp_path, [("k/html/big.html", b"x" * CHUNK_SIZE), + ("k/html/big.html-2", b"tail")]) + + assert _run_renumber_main(monkeypatch, db_path) == [db_path] + + conn = sqlite3.connect(db_path) + paths = {row[0] for row in conn.execute("SELECT path FROM Content")} + conn.close() + assert paths == {"k/html/big.html", "k/html/big.html-1"} + + +# --- F35: a flag given no value should say so, not die inside bash ----------- + +REPO_ROOT = Path(__file__).resolve().parents[4] + + +@pytest.mark.parametrize("script, flag", [ + ("run-build-kotlin-docs-with-act.sh", "--db-path"), + ("Dokka-plugin-kdoc2json/scripts/kotlin/build-stdlib-json-docs.sh", "--kotlin-libs-version"), +]) +def test_a_flag_without_a_value_is_reported(script, flag): + """Both scripts read $2 unguarded under `set -u`, so a trailing flag exited + with bash's own "$2: unbound variable" instead of the usage message that + exists for exactly this mistake.""" + path = REPO_ROOT / script + if not path.exists(): # pragma: no cover - only when run outside the repo + pytest.skip(f"{script} not found") + proc = subprocess.run(["bash", str(path), flag], capture_output=True, text=True) + + assert proc.returncode == 1 + combined = proc.stdout + proc.stderr + assert f"{flag} needs a value" in combined + assert "unbound variable" not in combined + + +# --- an shown inside a code sample is not a broken reference ------- + +@pytest.mark.parametrize("source, expected", [ + ("before\n~~~\n\n~~~\nafter\n", []), + ("x\n````\n\n```\n\n````\ny\n", []), + ("text\n\n", ["real.md"]), + ("```\n\n```\n\n", ["real.md"]), +]) +def test_includes_inside_any_fence_are_not_scanned(source, expected): + """The old pattern was "```.*?```": a ~~~-fenced sample was scanned as real + source, and a longer ```` fence wrapping a ``` sample closed early and + exposed the rest of the block. Both warned about files nobody meant to + ship. md_to_json.fenced_spans - which extract_title already relies on - + knows both fence characters and the "at least as long" close rule.""" + assert INCLUDE_RE.findall(outside_fences(source, "topics/x.md")) == expected + + +# ============================================================================= +# Self-review of the fourth round's own fixes. +# ============================================================================= + +def _apng(path, frames=3, size=(40, 40)): + """An animated PNG. Pillow reports is_animated for these exactly as it + does for a GIF, and optimize_raster copies any animated non-GIF through + unchanged - so the output keeps the .png.""" + images = [] + for i in range(frames): + frame = Image.new("RGB", size, (0, 0, 0)) + ImageDraw.Draw(frame).rectangle([i * 5, i * 5, i * 5 + 10, i * 5 + 10], fill=(255, i * 80, 0)) + images.append(frame) + images[0].save(path, save_all=True, append_images=images[1:], duration=80) + return path + + +# --- the duplicate-basename skip must not swallow a case-differing pair ----- + +def test_names_differing_only_by_case_are_both_kept(tmp_path): + """populate_db indexes image basenames exactly ("Logo.png" and "logo.png" + are two addressable images, and a page may reference either), so skipping + one here as a "duplicate" left that page's reference resolving to a row + nothing inserts. They are a collision, which de-confliction handles, not a + duplicate.""" + src, out = tmp_path / "in", tmp_path / "out" + (src / "a").mkdir(parents=True) + (src / "b").mkdir(parents=True) + out.mkdir() + Image.new("RGB", (20, 20), (9, 9, 9)).save(src / "a" / "Logo.png") + Image.new("RGB", (20, 20), (8, 8, 8)).save(src / "b" / "logo.png") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + written = sorted(p.name for p in result.written) + assert len(written) == 2, "neither source may be dropped - both are referenceable" + assert len(set(n.lower() for n in written)) == 2, "and they must not collide once flattened" + # The de-conflicted one is reported, so stored URLs follow it. Asserted as + # a property rather than against the "{stem}-{ext}" literal: the naming + # scheme is not what this test is about, and pinning it here would make a + # rename of that convention look like a case-handling regression. + assert list(result.renamed) == ["b/logo.png"], "only the second, de-conflicted source moved" + new_name = Path(result.renamed["b/logo.png"]).name + assert new_name in written and new_name.lower() != "logo.png" + + +def test_identical_basenames_are_still_skipped(tmp_path): + """The other direction: an exact duplicate is still one image, because + populate_db drops it too.""" + src, out = tmp_path / "in", tmp_path / "out" + (src / "a").mkdir(parents=True) + (src / "b").mkdir(parents=True) + out.mkdir() + Image.new("RGB", (20, 20), (9, 9, 9)).save(src / "a" / "logo.png") + Image.new("RGB", (20, 20), (8, 8, 8)).save(src / "b" / "logo.png") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert [p.name for p in result.written] == ["logo.png"] + assert result.renamed == {} + + +# --- the animated exemption covers every animated raster, not just GIF ------ + +def test_animated_png_is_not_predicted_as_webp(tmp_path): + """optimize_raster copies an animated non-GIF through untouched, so an + APNG under --webp writes anim.png. Predicting anim.webp claimed a name + nothing writes, and de-conflicted a genuine .webp producer against that + phantom - a rename, and a rewrite of every stored URL, for a collision + that never existed.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + _apng(src / "anim.png") + Image.new("RGB", (20, 20), (4, 4, 4)).save(src / "anim.jpg") + + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + result = om.optimize_directory(src, out, cfg=cfg, pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()) + + assert sorted(p.name for p in result.written) == ["anim.png", "anim.webp"] + # anim.jpg -> anim.webp is a real conversion; nothing else moved. + assert result.renamed == {"anim.jpg": "anim.webp"} + with Image.open(out / "anim.png") as written: + assert written.n_frames == 3, "the animation survived" + + +@pytest.mark.parametrize("suffix, animated, expected", [ + (".gif", True, {"x.gif"}), + (".gif", False, {"x.webp"}), + (".gif", None, set()), # undetermined: optimize_raster fails too, so nothing is written + (".png", None, set()), + (".png", True, {"x.png"}), + (".png", False, {"x.webp"}), + (".tiff", True, {"x.tiff"}), + (".jpg", True, {"x.jpg"}), # an MPO stored as .jpg: Pillow reports it animated + (".jpg", False, {"x.webp"}), + (".jpg", None, set()), + (".webp", True, {"x.webp"}), # copied through, under the name a conversion would give + (".webp", False, {"x.webp"}), + (".webp", None, set()), +]) +def test_possible_output_names_resolves_every_animated_raster(suffix, animated, expected): + """The prediction is a pure function of (suffix, cfg, animated) - no I/O, + so it can be called once per de-confliction attempt without re-opening + the source each time. + + One rule for every raster: under --webp it keeps its own extension + exactly when it is animated, because that is when optimize_raster copies + it through instead of re-encoding. The curated set this replaced kept + getting the membership wrong - first .gif only, then a four-entry set that + omitted .webp and .jpg.""" + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + assert om.possible_output_names("x", suffix, cfg, animated) == expected + + +# --- an unterminated fence must not silently switch the scan off ------------ + +def test_unterminated_fence_is_reported(capsys): + """fenced_spans runs an unclosed fence to EOF, which is CommonMark-correct + but means one stray ``` line stops the scan for the rest of the + file. The old backtick-only regex needed a closing fence to match, so it + kept scanning - losing that has to be said out loud, not inferred from a + suddenly-short report.""" + text = 'a\n```\n\n\n\n' + + assert INCLUDE_RE.findall(outside_fences(text, "topics/x.md")) == [] + assert "unterminated code fence" in capsys.readouterr().err + + +def test_a_closing_fence_on_the_last_line_is_not_reported(capsys): + """The warning has to be precise: a file that simply ends with a closed + code block reaches EOF too, and warning about it would train operators to + ignore the message.""" + outside_fences("a\n```\nx\n```\n", "topics/x.md") + + assert capsys.readouterr().err == "" + + +# --- repair() takes both halves of a scan, or neither ---------------------- + +def test_repair_refuses_half_a_scan(tmp_path): + """The two lists come from one find_chains call. Answering a half-supplied + pair with a fresh scan silently discarded the caller's snapshot, so + main()'s backup decision could describe a different repair than the one + that ran.""" + import renumber_misnumbered_fragments as rmf + conn = sqlite3.connect(":memory:") + conn.executescript(SCHEMA_SQL) + try: + with pytest.raises(TypeError, match="both misnumbered and gapped"): + rmf.repair(conn, []) + with pytest.raises(TypeError, match="both misnumbered and gapped"): + rmf.repair(conn, gapped=[]) + assert rmf.repair(conn)["chains_renumbered"] == 0 # neither: scans + assert rmf.repair(conn, [], [])["chains_renumbered"] == 0 # both: uses them + finally: + conn.close() + + +# --- what process_file writes has to be what the planner claimed ------------ + +def test_a_write_the_planner_did_not_predict_is_refused(tmp_path, monkeypatch): + """possible_output_names predicts, in a second place, what process_file -> + optimize_raster -> encode_raster will do, and that prediction has drifted + twice. Nothing connected the two halves, so both drifts were silent: a + name claimed but never written de-conflicts an unrelated image and + rewrites its stored URLs, and a name written but never claimed is free to + overwrite another source's output.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (1, 1, 1)).save(src / "photo.png") + + # Stand in for a future encoder change the planner doesn't know about. + real_process_file = om.process_file + def drifting_process_file(source, dst, **kwargs): + written = real_process_file(source, dst, **kwargs) + return written.with_suffix(".avif") if written else written + monkeypatch.setattr(om, "process_file", drifting_process_file) + + # ValueError, not RuntimeError: insert_optimized_media wraps this call in + # `except ValueError` to turn it into a clean "error: ..." line, and a + # RuntimeError sailed past that as a raw traceback. + with pytest.raises(ValueError, match="possible_output_names did not predict"): + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()) + + +def test_an_unprobeable_source_claims_no_names(tmp_path, capsys): + """A raster whose animation can't be determined is one optimize_raster is + about to fail on for the same reason, so it writes nothing and must hold + nothing. Claiming both names instead de-conflicted a perfectly good + unrelated image against an output that never appears - renaming it, and + rewriting every stored URL that pointed at it.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + (src / "broken.png").write_bytes(b"not a png at all") + Image.new("RGB", (20, 20), (2, 2, 2)).save(src / "broken.jpg") + + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + stats = _new_stats() + result = om.optimize_directory(src, out, cfg=cfg, pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=stats) + + # broken.jpg keeps its own stem: nothing real ever claimed broken.webp. + assert [p.name for p in result.written] == ["broken.webp"] + assert result.renamed == {"broken.jpg": "broken.webp"} + assert stats["errors"] == 1, "the unreadable file is still counted as one file's error" + # And the probe said why, rather than swallowing it. + assert "could not determine whether" in capsys.readouterr().out + + +def test_the_probe_reports_its_own_failure(tmp_path, capsys): + """_is_animated_raster now runs for every PNG and TIFF in the tree, not + just the handful of GIFs, so a systematic failure would shift the whole + de-confliction pass. Swallowing it left no way to find out why.""" + bad = tmp_path / "corrupt.png" + bad.write_bytes(b"still not a png") + + assert om._is_animated_raster(bad, om.Logger(sys.stdout)) is None + assert "could not determine whether" in capsys.readouterr().out + # Silent without a logger, so a standalone caller isn't forced to have one. + assert om._is_animated_raster(bad) is None + assert capsys.readouterr().out == "" + + +# --- the written-vs-claimed check must not fire on ordinary inputs ---------- + +@pytest.mark.parametrize("name, expected_webp, expected_plain", [ + ("Diagram.SVG", "Diagram.SVG", "Diagram.SVG"), # small enough not to rasterize + ("Diagram.Svg", "Diagram.Svg", "Diagram.Svg"), + ("Photo.PNG", "Photo.webp", "Photo.PNG"), + ("Photo.JPG", "Photo.webp", "Photo.JPG"), + ("Notes.TXT", "Notes.TXT", "Notes.TXT"), # passthrough copy +]) +@pytest.mark.parametrize("webp", [True, False]) +def test_an_uppercase_extension_is_not_mistaken_for_drift(tmp_path, name, expected_webp, expected_plain, webp): + """possible_output_names' SVG branch returned the lowercase SVG_EXTENSION + literal while optimize_svg writes `dst`, which carries the source's own + spelling - so "Diagram.SVG" was claimed as "Diagram.svg". Harmless while + it only fed the casefolded `claimed` map; once the written-vs-claimed + check existed it aborted the whole run over one ordinary file.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + if name.lower().endswith(".svg"): + (src / name).write_text('' + '') + elif name.lower().endswith(".txt"): + (src / name).write_text("passthrough") + else: + Image.new("RGB", (20, 20), (3, 3, 3)).save(src / name) + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": webp}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + # The name matters, not just the count: the bug was a claim spelled + # ".svg" for a file written as ".SVG", and asserting only that one file + # came out would pass just as happily if the output had been lowercased. + assert [p.name for p in result.written] == [expected_webp if webp else expected_plain] + + +def test_the_svg_branch_claims_the_source_s_own_spelling(): + """Every other branch echoes `suffix`; this one hardcoded the constant.""" + cfg = dict(om.BUILTIN_DEFAULTS) | {"webp": True} + assert om.possible_output_names("d", ".SVG", cfg) == {"d.SVG", "d.webp"} + assert om.possible_output_names("d", ".svg", cfg) == {"d.svg", "d.webp"} + + +@pytest.mark.parametrize("webp", [True, False]) +def test_an_undetermined_probe_skips_one_file_and_no_more(tmp_path, monkeypatch, capsys, webp): + """optimize_raster takes the planner's answer rather than re-deriving it, + so a source whose animation could not be determined is refused there, + counted as one file's error, and never reaches the written-vs-claimed + check. The run continues - this module's rule is that a single file's + problem is reported and the rest of the tree still processes. + + The --webp False half is the case the old exemption got wrong. The + planner claims the source's own name there regardless of `animated` (that + branch returns before consulting it), so "the probe failed" and "the + planner claimed nothing" came apart - and the exemption, keyed on the + former, waved genuine drift through as a warning.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (5, 5, 5)).save(src / "photo.png") + # A passthrough file as the control, not another raster: the stub answers + # every probe, and rasters are all probed now, so a .jpg control would be + # skipped too and the assertion below would fail for a reason that has + # nothing to do with what this test protects. + (src / "other.txt").write_text("passthrough") + monkeypatch.setattr(om, "_is_animated_raster", lambda source, logger=None: None) + + stats = _new_stats() + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": webp}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=stats) + + assert stats["errors"] == 1, "the undetermined file, and only it" + assert [p.name for p in result.written] == ["other.txt"] + assert "could not determine whether this file is animated" in capsys.readouterr().out + + +def test_the_drift_message_names_the_claim_as_the_planner_wrote_it(tmp_path, monkeypatch): + """Built from `claimed`'s keys, the message reported a casefolded + "diagram.svg" for a source that claimed "Diagram.SVG" - pointing whoever + reads it at a case mismatch that is not the problem.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + (src / "Diagram.SVG").write_text('' + '') + real_process_file = om.process_file + def drifting_process_file(source, dst, **kwargs): + written = real_process_file(source, dst, **kwargs) + return written.with_suffix(".avif") if written else written + monkeypatch.setattr(om, "process_file", drifting_process_file) + + with pytest.raises(ValueError, match=r"it claimed \['Diagram\.SVG', 'Diagram\.png'\]"): + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS), pngquant_path=om.find_pngquant(), + logger=om.Logger(sys.stdout), stats=_new_stats()) + + +# --- every animated raster, not just the GIF the tests happened to cover ---- + +def _multi_frame(path, fmt=None, frames=4, size=(40, 40)): + """A multi-frame image in whatever format `path`/`fmt` implies. Frames + differ so the encoder cannot collapse them into one.""" + images = [] + for i in range(frames): + frame = Image.new("RGB", size, (0, 0, 0)) + ImageDraw.Draw(frame).rectangle([i * 5, i * 5, i * 5 + 10, i * 5 + 10], fill=(255, i * 60, 0)) + images.append(frame) + kwargs = {"save_all": True, "append_images": images[1:]} + if fmt: + kwargs["format"] = fmt + images[0].save(path, **kwargs) + return path + + +@pytest.mark.parametrize("name, fmt", [ + ("anim.webp", None), # animated WEBP - flattened to one frame for a whole commit + ("anim.png", None), # APNG + ("stereo.jpg", "MPO"), # a phone stereo/burst capture; Pillow reports it animated +]) +@pytest.mark.parametrize("webp", [True, False]) +def test_every_animated_raster_keeps_its_frames(tmp_path, name, fmt, webp): + """optimize_raster copies an animated non-GIF through untouched, so the + animation survives and the file keeps its own extension. That branch was + unreachable for .webp and .jpg while the planner probed only a curated + set of extensions and passed a hard-coded False for the rest - and the + written-vs-claimed check could not see it, because the *name* was exactly + what was predicted. Only the frames were gone. + + Run both ways: possible_output_names takes a different branch without + --webp, and optimize_raster still reads `animated` there to choose + copy-through over encoding, so the two halves can disagree on that path + too - and it is the path where the undetermined case was mishandled.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + _multi_frame(src / name, fmt=fmt) + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": webp}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + assert [p.name for p in result.written] == [name], "an animated raster keeps its own extension" + assert result.renamed == {}, "so nothing repoints the stored URLs" + with Image.open(out / name) as written: + assert written.n_frames == 4, "and every frame survives" + + +def test_a_static_file_of_the_same_types_still_converts(tmp_path): + """The exemption is animation, not extension: single-frame sources of the + same types still convert, so this does not quietly opt whole formats out + of --webp.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (20, 20), (1, 1, 1)).save(src / "still.png") + Image.new("RGB", (20, 20), (2, 2, 2)).save(src / "still.jpg") + + result = om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), + stats=_new_stats()) + + # Both claim still.webp, so one is de-conflicted - the point here is that + # each of them converted, not which one kept the plain name. + assert len(result.written) == 2 + assert all(p.suffix == ".webp" for p in result.written) + assert len(result.renamed) == 2, "both changed extension, so both repoint" + + +def test_a_copied_through_animation_is_not_reported_as_optimized(tmp_path, capsys): + """The copy-through branch writes the source byte-for-byte: no resize, no + re-encode. Reporting it as "Optimized" and counting it in stats["raster"] + hid that an animated file ships at its original dimensions, past + --max-width - and the log was the only place that could have said so.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + _multi_frame(src / "big.webp", size=(900, 900)) + assert om.BUILTIN_DEFAULTS["max_width"] < 900, "the fixture has to exceed the cap" + + stats = _new_stats() + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), stats=stats) + + with Image.open(out / "big.webp") as written: + assert written.size == (900, 900), "unresized - which is the thing worth saying out loud" + assert stats["copied"] == 1 and stats["raster"] == 0, "counted as copied, not as an optimization" + assert "Copied unresized (animated)" in capsys.readouterr().out + + +def test_a_resized_still_image_is_still_reported_as_optimized(tmp_path, capsys): + """The other direction: the honest label must not spread to files that + really were optimized.""" + src, out = tmp_path / "in", tmp_path / "out" + src.mkdir(), out.mkdir() + Image.new("RGB", (900, 900), (3, 3, 3)).save(src / "big.png") + + stats = _new_stats() + om.optimize_directory(src, out, cfg=dict(om.BUILTIN_DEFAULTS) | {"webp": True}, + pngquant_path=om.find_pngquant(), logger=om.Logger(sys.stdout), stats=stats) + + out_text = capsys.readouterr().out + assert stats["raster"] == 1 and stats["copied"] == 0 + assert "Optimized" in out_text and "Copied unresized" not in out_text diff --git a/ProcessDocs/ProcessKotlinDocs/run_e2e_pipeline_test.sh b/ProcessDocs/ProcessKotlinDocs/run_e2e_pipeline_test.sh new file mode 100755 index 00000000..bbbcf8df --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/run_e2e_pipeline_test.sh @@ -0,0 +1,242 @@ +#!/usr/bin/env bash +# End-to-end test of the Kotlin website JSON/DB pipeline (ADFA-4737): convert +# docs + prune blacklist into the database, re-optimize/reinsert media, +# generate fresh kotlin-stdlib/-reflect/-test JSON docs via a freshly-built +# kdoc-to-json plugin, then sync them into the database. Operates on a +# scratch copy of documentation.db so the real database is never touched. +# Re-run freely; each run recopies the source db from scratch. +# +# Before running: fill in every value below for your machine. +# The script refuses to start if any are left unfilled or don't exist on disk. +set -euo pipefail + +if ! command -v uv >/dev/null 2>&1; then + echo "error: uv is required - see https://docs.astral.sh/uv/getting-started/installation/" >&2 + exit 1 +fi + +# --- Repo-relative paths - auto-detected, no edits needed --------------- +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +PROCESS_DIR="$REPO_ROOT/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON" +SYNC_SCRIPT="$REPO_ROOT/scripts/sync_kotlin_stdlib_docs/sync_kdoc_json_to_db.py" +UV_RUN=(uv run --with-requirements "$REPO_ROOT/requirements.txt") + +# config.json, templates/, and assets/ are staged directly in $PROCESS_DIR +# (this repo) rather than pulled from anyone's local machine - populate_db.py +# looks these up next to its own script location: +# config.json - theming config (broken-ext-link-color, menu-no-link-color) +# templates/*.peb - page.peb / nav.peb, upserted into the Templates table +# assets/* - docs.css / tabs.js / sidebar.js, inserted at assets/ +CONFIG_JSON="$PROCESS_DIR/config.json" + +# --- Machine-specific paths - fill these in -------------------------------- + +# DOCS_ROOT: the "docs" subdirectory of a Writerside checkout of the official +# Kotlin website. Get it from https://github.com/JetBrains/kotlin-web-site - +# clone that repo and point this at "/kotlin-web-site/docs" (the +# directory directly containing kr.tree, topics/, images/, v.list). +DOCS_ROOT="" + +# IMAGES_ZIP: Writerside's own image export for that same docs project (e.g. +# "webHelpImages.zip"). Produced by running IntelliJ IDEA's Writerside plugin +# build/export action against DOCS_ROOT's parent Writerside project; the zip +# is written next to kr.tree once that build finishes. +IMAGES_ZIP="" + +# STDLIB_DOCS_DIR: the "libraries/tools/kotlin-stdlib-docs" directory inside +# a full clone of https://github.com/JetBrains/kotlin (not the kotlin repo +# root itself - this exact subdirectory). Step 4 below builds a fresh copy +# of the kdoc-to-json plugin from this repo's Dokka-plugin-kdoc2json/ and +# runs it against this checkout to produce kotlin-stdlib/-reflect/-test JSON +# docs (common + jvm source sets only) - no separate manual doc-generation +# step needed. +STDLIB_DOCS_DIR="" + +# SOURCE_DB: the runtime "documentation.db" SQLite database this project's +# offline documentation app/server reads from (see docdb-studio/ and +# check-tools/ in this repo for tooling that operates on the same file). Must +# already have its schema populated (Languages, ContentTypes, Templates +# tables) - point this at your own working copy. +SOURCE_DB="" + +TEST_DB="$(dirname "$SOURCE_DB")/documentation.test.db" + +JPEG_QUALITY=85 +WEBP_QUALITY=90 + +# Which published kotlin-stdlib/-reflect/-test artifacts step 4/5 documents. +# kotlin_big (inside kotlin-stdlib-docs) extracts the real binaries from a +# Maven repo; left unset it looks for "/build/repo" at the +# checkout's own defaultSnapshotVersion, i.e. artifacts that only exist if you +# have built the whole kotlin repo locally. Naming a released version instead +# resolves them straight from Maven Central. Keep it in step with the ref your +# STDLIB_DOCS_DIR checkout is on; set empty to use the checkout's own default. +KOTLIN_LIBS_VERSION=2.4.10 +# Maven repo to resolve them from. Empty is fine for a released +# KOTLIN_LIBS_VERSION (kotlin_big already declares mavenCentral()); set this +# only to point at a private or snapshot repository. +KOTLIN_LIBS_REPO="" + +# Full toc-title path (top-level -> ... -> target), joined with "\/" per +# populate_db.py's --blacklisted-element-titles convention. These are the +# concrete cases named in ADFA-4737; add more "path" entries here to prune +# additional sections. Re-derive these from your own DOCS_ROOT/kr.tree if the +# site's navigation structure has changed since this was written. +BLACKLIST=( + 'Development\/Web development' + 'Interoperability\/Swift/Objective-C and C interop' + 'Interoperability\/JavaScript interop' +) + +# --- Fail fast on unfilled placeholders or missing paths ------------------- +require_path() { + local name="$1" value="$2" + if [[ "$value" == "<"*">" ]]; then + echo "error: $name is still a placeholder ('$value') - edit this script and fill in your local path." >&2 + exit 1 + fi + if [[ ! -e "$value" ]]; then + echo "error: $name points to '$value', which does not exist." >&2 + exit 1 + fi +} +require_path DOCS_ROOT "$DOCS_ROOT" +require_path IMAGES_ZIP "$IMAGES_ZIP" +require_path STDLIB_DOCS_DIR "$STDLIB_DOCS_DIR" +require_path SOURCE_DB "$SOURCE_DB" + +WORKDIR="$(mktemp -d /tmp/adfa4737-e2e.XXXXXX)" +cleanup() { rm -rf "$WORKDIR"; } +trap cleanup EXIT + +echo "== Copying $SOURCE_DB -> $TEST_DB ==" +rm -f "$TEST_DB" +cp "$SOURCE_DB" "$TEST_DB" + +echo +echo "== Step 1/5: find_missing_assets.py (source QA report) ==" +REPORT_PATH="$WORKDIR/missing-assets-report.md" +"${UV_RUN[@]}" "$PROCESS_DIR/find_missing_assets.py" "$DOCS_ROOT" "$REPORT_PATH" +echo "Report written to $REPORT_PATH" + +echo +echo "== Step 2/5: populate_db.py (convert docs, prune blacklist, insert into test db) ==" +( cd "$PROCESS_DIR" && "${UV_RUN[@]}" populate_db.py "$DOCS_ROOT" "$CONFIG_JSON" "$IMAGES_ZIP" "$TEST_DB" \ + --blacklisted-element-titles "${BLACKLIST[@]}" ) + +echo +echo "== Step 3/5: insert_optimized_media.py (re-optimize + reinsert k/html/images/*) ==" +# --webp requires an "image/webp" ContentTypes row, which this database +# doesn't ship with (see insert_optimized_media.py's own module docstring) - +# add it (idempotent) before running. +sqlite3 "$TEST_DB" "INSERT OR IGNORE INTO ContentTypes (value, compression) VALUES ('image/webp', 'brotli');" + +# insert_optimized_media.py addresses images by bare filename, matching +# populate_db.py's own flat k/html/images/ convention - so its input +# media_dir needs to be a directory of files with those same basenames. +# The images actually inserted above came from IMAGES_ZIP, so extract that +# same zip here rather than pointing at DOCS_ROOT/images (the raw, unoptimized +# Writerside source tree - a different, much larger set of files). +MEDIA_DIR="$WORKDIR/media" +mkdir -p "$MEDIA_DIR" +unzip -q "$IMAGES_ZIP" -d "$MEDIA_DIR" + +"${UV_RUN[@]}" "$PROCESS_DIR/insert_optimized_media.py" "$MEDIA_DIR" "$TEST_DB" \ + --jpeg-quality "$JPEG_QUALITY" --webp --webp-quality "$WEBP_QUALITY" --verbose + +echo +echo "== Step 4/5: build-stdlib-json-docs.sh (fresh plugin build -> kotlin-stdlib/-reflect/-test JSON) ==" +# STDLIB_DOCS_DIR is .../kotlin/libraries/tools/kotlin-stdlib-docs; the +# kotlin repo root (needed to locate gradle/libs.versions.toml and to +# resolve kotlin_root inside the injected build.gradle.kts) is exactly three +# levels up, matching that build.gradle.kts's own "../../../" convention. +KOTLIN_ROOT="$(cd "$STDLIB_DOCS_DIR/../../.." && pwd)" +STDLIB_ARGS=() +[ -n "$KOTLIN_LIBS_VERSION" ] && STDLIB_ARGS+=(--kotlin-libs-version "$KOTLIN_LIBS_VERSION") +[ -n "$KOTLIN_LIBS_REPO" ] && STDLIB_ARGS+=(--kotlin-libs-repo "$KOTLIN_LIBS_REPO") +STDLIB_ALL_LIBS="$("$REPO_ROOT/Dokka-plugin-kdoc2json/scripts/kotlin/build-stdlib-json-docs.sh" \ + ${STDLIB_ARGS[@]+"${STDLIB_ARGS[@]}"} "$KOTLIN_ROOT" "$WORKDIR/stdlib-json")" +echo "Generated JSON docs at $STDLIB_ALL_LIBS" + +echo +echo "== Step 5/5: sync_kdoc_json_to_db.py (overwrite kotlin-stdlib/-reflect/-test content) ==" +"${UV_RUN[@]}" "$SYNC_SCRIPT" "$STDLIB_ALL_LIBS" --db "$TEST_DB" + +echo +echo "== Summary ==" +"${UV_RUN[@]}" python3 - "$TEST_DB" <<'PYEOF' +import sqlite3 +import sys + +db_path = sys.argv[1] +conn = sqlite3.connect(db_path) + + +def count(where, params=()): + return conn.execute(f"SELECT count(*) FROM Content WHERE {where}", params).fetchone()[0] + + +print(f"Database: {db_path}") +print(f" k/html/* rows: {count('path LIKE ?', ('k/html/%',))}") +print(f" k/html/images/* rows: {count('path LIKE ?', ('k/html/images/%',))}") +print(f" k/html/images/*.webp rows: {count('path LIKE ?', ('k/html/images/%.webp%',))}") +print(f" k/kotlin-stdlib/* rows: {count('path LIKE ? OR path = ?', ('k/kotlin-stdlib/%', 'k/kotlin-stdlib'))}") +print(f" k/kotlin-reflect/* rows: {count('path LIKE ? OR path = ?', ('k/kotlin-reflect/%', 'k/kotlin-reflect'))}") +print(f" k/kotlin-test/* rows: {count('path LIKE ? OR path = ?', ('k/kotlin-test/%', 'k/kotlin-test'))}") + +conn.close() +PYEOF + +echo +echo "== Blacklist pruning verification ==" +# Recomputes, from the same kr.tree and BLACKLIST used above, exactly which +# topic stems populate_db.py's own prune_blacklisted_elements() decided to +# exclude - then confirms none of those pages made it into the database. +# Reusing that real pruning logic (rather than guessing at path patterns) +# means this check stays correct if BLACKLIST or kr.tree's structure change. +"${UV_RUN[@]}" python3 - "$PROCESS_DIR" "$DOCS_ROOT" "$TEST_DB" "${BLACKLIST[@]}" <<'PYEOF' +import sqlite3 +import sys +import xml.etree.ElementTree as ET +from pathlib import Path + +process_dir, docs_root, db_path, *blacklist_raw = sys.argv[1:] +sys.path.insert(0, process_dir) +import populate_db # noqa: E402 + +root = ET.parse(Path(docs_root) / "kr.tree").getroot() +blacklisted_paths = {populate_db.parse_blacklist_path(raw) for raw in blacklist_raw} +blacklisted_stems, unmatched_paths = populate_db.prune_blacklisted_elements(root, blacklisted_paths) + +conn = sqlite3.connect(db_path) +leftover = [] +for stem in sorted(blacklisted_stems): + path = f"k/html/{stem}.html" + if conn.execute("SELECT 1 FROM Content WHERE path = ?", (path,)).fetchone(): + leftover.append(path) +conn.close() + +print(f"Blacklisted toc-element path(s) checked: {len(blacklisted_paths)}") +for path in sorted(blacklisted_paths): + status = "unmatched (no such element in kr.tree)" if path in unmatched_paths else "matched" + print(f" {' > '.join(path)}: {status}") +print(f"Topic page(s) expected removed: {len(blacklisted_stems)}") + +if unmatched_paths: + print(f"FAIL: {len(unmatched_paths)} blacklist path(s) never matched a - " + "check BLACKLIST against this DOCS_ROOT's kr.tree.") + sys.exit(1) +if leftover: + print(f"FAIL: {len(leftover)} blacklisted page(s) still present in the database:") + for path in leftover: + print(f" {path}") + sys.exit(1) + +print(f"PASS: all {len(blacklisted_stems)} blacklisted topic page(s) confirmed absent from {db_path}.") +PYEOF + +echo +echo "Done. Backups (populate_db.py, insert_optimized_media.py, and sync_kdoc_json_to_db.py" +echo "each make their own) live alongside $TEST_DB as documentation.test.db.backup-* and" +echo "documentation.test.db.bak.*" +echo "The real database at $SOURCE_DB was never opened for writing." diff --git a/docdb-studio/docdb_studio.py b/docdb-studio/docdb_studio.py index 3a9991d9..670795c5 100644 --- a/docdb-studio/docdb_studio.py +++ b/docdb-studio/docdb_studio.py @@ -1348,7 +1348,7 @@ def _find_brotli_cli() -> str: return path -def get_compression_dictionary(db_path: Path) -> bytes | None: +def get_compression_dictionary(db_path: Path, *, strict: bool = False) -> bytes | None: """Returns db_path's CompressionDictionary bytes (see ADFA-5153), or None if it doesn't have one yet. @@ -1357,7 +1357,15 @@ def get_compression_dictionary(db_path: Path) -> bytes | None: writes the same file; caching that as None would downgrade the whole session to plain Brotli, so imports would write plain rows into a dictionary database and reads of existing rows would fail. On any such error this returns None for this - one call and retries on the next.""" + one call and retries on the next. + + `strict=True` raises instead of returning None on that indeterminate case. + Callers that are about to *write* must pass it: a None they cannot tell apart + from "no dictionary" makes them store a plain-Brotli row in a dictionary + database, which nothing detects afterwards - the row simply fails to decode + later. Read paths can afford the lenient answer, because decoding a + dictionary row without the dictionary raises loudly rather than returning + wrong bytes.""" if db_path in _dictionary_cache: return _dictionary_cache[db_path] dictionary_data: bytes | None = None @@ -1373,6 +1381,12 @@ def get_compression_dictionary(db_path: Path) -> bytes | None: if data_row is not None: dictionary_data = data_row[0] except sqlite3.OperationalError as exc: + if strict: + raise RuntimeError( + f"could not determine whether {db_path} has a shared compression dictionary ({exc}); " + "refusing to guess, because writing a plain-Brotli row into a dictionary database " + "produces content that cannot be decoded later" + ) from exc print(f"warning: could not read {db_path}'s compression dictionary ({exc}); " f"not caching that, will retry", file=sys.stderr) return None @@ -1399,7 +1413,12 @@ def compress_for_storage(data: bytes, compression: str, db_path: Path) -> bytes: that migration. Anything else passes through unchanged.""" if compression != "brotli": return data - dictionary_data = get_compression_dictionary(db_path) + # strict: a lock-induced None here would be read as "no dictionary" and + # silently store a plain row in a dictionary database (see ADFA-5153). + # Reachable from import_content_files itself, whose phase-1 orphan DELETE + # holds a write transaction on one connection while this opens another + # against the same file. + dictionary_data = get_compression_dictionary(db_path, strict=True) if dictionary_data is None: return brotli.compress(data) dict_path = _dictionary_temp_path(db_path, dictionary_data) @@ -1442,9 +1461,27 @@ def decompress_brotli(data: bytes, db_path: Path) -> bytes: if dictionary_data is None: return brotli.decompress(data) dict_path = _dictionary_temp_path(db_path, dictionary_data) + try: + # BrotliCliMissing subclasses brotli.error, not OSError, so resolving the + # CLI inside the argv list below let it escape the except: a *plain* row + # in a dictionary database became unreadable without the CLI, even though + # brotli.decompress handles it fine and did before. Those are exactly the + # rows the fallback at the end exists for - plugin-contributed content and + # partly-migrated databases. + # + # Only that case falls through, though. If plain decoding also fails the + # row really is dictionary-compressed and the CLI really is required, so + # re-raise and let the call sites print where to get it - telling someone + # to install brotli is right there and useless on a plain row. + brotli_cli = _find_brotli_cli() + except BrotliCliMissing as cli_missing: + try: + return brotli.decompress(data) + except brotli.error: + raise cli_missing from None try: result = subprocess.run( - [_find_brotli_cli(), "-d", "-D", str(dict_path), "-c"], + [brotli_cli, "-d", "-D", str(dict_path), "-c"], input=data, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False, ) except OSError as exc: @@ -1628,6 +1665,21 @@ def _report(phase: str, current: int, total: int) -> None: if progress_callback is not None and total > 0: progress_callback(phase, current, total) + # Resolve the shared compression dictionary *before* opening the write + # connection below, and let the result be cached for the whole import. + # + # compress_for_storage needs a definitive answer - a plain-Brotli row + # written into a dictionary database can never be decoded again, and a + # lock-induced None is indistinguishable from "no dictionary". But asking + # for it lazily inside the loop meant opening a second connection while + # phase 1's orphan DELETE still held a write transaction on the first, so + # the import could fail on a lock it had inflicted on itself, rolling back + # phase 1 and dying mid-way. Asking here means the calls below are pure + # cache lookups, and a lock at this point is a genuine external one, hit + # before anything has been deleted. + if any(item.compression == "brotli" for item in plan): + get_compression_dictionary(db_path, strict=True) + with sqlite3.connect(db_path) as conn: # Phase 1: bulk-delete orphans by id. Reports progress per batch so a # large orphan list still shows the bar advancing. diff --git a/requirements.txt b/requirements.txt index c5fc3aa0..9d7c018c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,4 +5,6 @@ brotli Pillow openpyxl>=3.1.0 tqdm-loggable>=0.1.0 +scour +cairosvg markdown-it-py>=2.0 diff --git a/run-build-kotlin-docs-with-act.sh b/run-build-kotlin-docs-with-act.sh new file mode 100755 index 00000000..7a2db8fc --- /dev/null +++ b/run-build-kotlin-docs-with-act.sh @@ -0,0 +1,246 @@ +#!/usr/bin/env bash +# Runs the "Build Kotlin Docs (Local)" GitHub Actions workflow +# (.github/workflows/build-kotlin-docs-local.yaml) locally via act +# (https://github.com/nektos/act). +# +# This drives the *local* workflow, not build-kotlin-docs.yaml. The Drive +# workflow authenticates to Google Cloud with Workload Identity Federation, +# and WIF validates the OIDC token's issuer against GitHub's own token +# endpoint for a specific repo and run - act cannot mint a token GCP will +# accept, so that workflow can never get past its auth step locally no matter +# what secrets you supply. build-kotlin-docs-local.yaml exists precisely to +# be runnable here: it reads its documentation.db / webHelpImages.zip from +# disk and writes its outputs back to disk, and is otherwise step-for-step +# identical to the Drive workflow (same find_missing_assets -> populate_db -> +# insert_optimized_media -> build-stdlib-json-docs -> sync_kdoc_json_to_db, +# same ADFA-4737 blacklist, same verification). +# +# Requires: +# - act (https://github.com/nektos/act#installation) on PATH +# - a running Docker daemon (act executes each step inside a container) +# +# Secrets: none are required. SLACK_WEBHOOK_URL is the only secret this +# workflow reads, and it is optional - the two "Notify Slack" steps print a +# skip notice and continue when it is unset. Export it if you want to see +# them actually fire. Both notifications are ungated - "build complete" runs on +# if: always(), so it fires on a dry run and on a failed build too, and reports +# which of those happened. GitHub never exposes a stored secret's value +# through any API or CLI, so if you do want the real webhook you have to +# supply your own copy of the value. +# +# Inputs are host paths, bind-mounted into the job container at fixed +# locations and passed to the workflow as those in-container paths (a +# GitHub-hosted runner has no access to your disk, so the workflow only ever +# sees the mounted paths). Note this means the host paths must live somewhere +# your container runtime is allowed to share - under $HOME is safe for both +# colima and Docker Desktop; /tmp on macOS often is not. +# +# Usage: +# ./run-build-kotlin-docs-with-act.sh --db-path PATH [options] [-- ] +# +# Options: +# --db-path PATH Host path to the input documentation.db (required). +# With --live this file is overwritten in place. +# --images-zip-path PATH Host path to Writerside's webHelpImages.zip. +# Required unless --skip-website-docs. +# --output-dir PATH Host directory for outputs - the missing-assets +# report and a run-numbered copy of the built +# database. Created if absent. +# (default: ./build-kotlin-docs-output) +# --live dry_run=false: write the rebuilt database back +# over --db-path when the run finishes. +# Default is dry_run=true. +# --skip-website-docs skip_website_docs=true (default: false) +# --skip-stdlib-docs skip_stdlib_docs=true (default: false). Skips +# cloning JetBrains/kotlin and the Dokka JSON +# build - by far the slowest part of a run, and +# the half you don't need when iterating on the +# kotlin-web-site content. +# --kotlin-web-site-ref REF kotlin_web_site_ref input (default: '') +# --kotlin-ref REF kotlin_ref input (default: '') +# --kotlin-libs-version V Version of the published kotlin-stdlib/-reflect/ +# -test artifacts to document. Defaults to the +# workflow's own default; pass '' to fall back to +# the kotlin checkout's snapshot version, which +# only resolves if you built the kotlin repo. +# --kotlin-libs-repo URL Maven repo to resolve them from (default: '', +# i.e. mavenCentral, which is enough for a +# released --kotlin-libs-version). +# +# Every workflow input is passed explicitly on every run, including the ones +# whose YAML "default:" would cover them. act does not apply +# workflow_dispatch input defaults - an input you don't pass arrives empty - +# and for dry_run that inverts the intended behaviour: "${{ !inputs.dry_run }}" +# on an empty value is true, so the step that writes the database back over +# --db-path would run. Passing all of them keeps a local run's semantics +# identical to a real dispatch. +# +# On Apple Silicon act warns about container architecture; append +# `-- --container-architecture linux/arm64` if you want to silence it (the +# default works). +set -euo pipefail + +# $2 is read unguarded otherwise, so a trailing flag dies with bash's own +# "$2: unbound variable" under `set -u` rather than a usable message. +need_value() { [ $# -ge 2 ] || { echo "error: $1 needs a value" >&2; exit 1; }; } + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORKFLOW="$REPO_ROOT/.github/workflows/build-kotlin-docs-local.yaml" + +# Where the host paths below get bind-mounted inside the job container, and +# therefore what the workflow itself is told its inputs are. +CONTAINER_DB_PATH="/mnt/act-inputs/documentation.db" +CONTAINER_IMAGES_ZIP_PATH="/mnt/act-inputs/webHelpImages.zip" +CONTAINER_OUTPUT_DIR="/mnt/act-output" + +DB_PATH="" +IMAGES_ZIP_PATH="" +OUTPUT_DIR="$REPO_ROOT/build-kotlin-docs-output" +KOTLIN_WEB_SITE_REF="" +KOTLIN_REF="" +# Mirrors build-kotlin-docs-local.yaml's own default. Restated here because act +# does not apply workflow_dispatch defaults (see the note above); passing the +# input unconditionally is what keeps a local run equivalent to a real one. +KOTLIN_LIBS_VERSION="2.4.10" +KOTLIN_LIBS_REPO="" +SKIP_WEBSITE_DOCS="false" +SKIP_STDLIB_DOCS="false" +DRY_RUN="true" + +EXTRA_ACT_ARGS=() + +while [ $# -gt 0 ]; do + case "$1" in + --db-path) need_value "$@"; DB_PATH="$2"; shift 2 ;; + --images-zip-path) need_value "$@"; IMAGES_ZIP_PATH="$2"; shift 2 ;; + --output-dir) need_value "$@"; OUTPUT_DIR="$2"; shift 2 ;; + --live) DRY_RUN="false"; shift ;; + --skip-website-docs) SKIP_WEBSITE_DOCS="true"; shift ;; + --skip-stdlib-docs) SKIP_STDLIB_DOCS="true"; shift ;; + --kotlin-web-site-ref) need_value "$@"; KOTLIN_WEB_SITE_REF="$2"; shift 2 ;; + --kotlin-ref) need_value "$@"; KOTLIN_REF="$2"; shift 2 ;; + --kotlin-libs-version) need_value "$@"; KOTLIN_LIBS_VERSION="$2"; shift 2 ;; + --kotlin-libs-repo) need_value "$@"; KOTLIN_LIBS_REPO="$2"; shift 2 ;; + --) shift; EXTRA_ACT_ARGS+=("$@"); break ;; + *) echo "error: unrecognized argument '$1'" >&2; exit 1 ;; + esac +done + +if [ "$SKIP_WEBSITE_DOCS" = "true" ] && [ "$SKIP_STDLIB_DOCS" = "true" ]; then + echo "error: --skip-website-docs and --skip-stdlib-docs together skip every step that" >&2 + echo "error: changes the database, leaving nothing for the run to do." >&2 + exit 1 +fi + +if [ -z "$DB_PATH" ]; then + echo "error: --db-path is required (host path to the documentation.db to build against)" >&2 + exit 1 +fi +if [ ! -f "$DB_PATH" ]; then + echo "error: --db-path '$DB_PATH' does not exist or is not a file" >&2 + exit 1 +fi +DB_PATH="$(cd "$(dirname "$DB_PATH")" && pwd)/$(basename "$DB_PATH")" + +if [ "$SKIP_WEBSITE_DOCS" != "true" ]; then + if [ -z "$IMAGES_ZIP_PATH" ]; then + echo "error: --images-zip-path is required unless --skip-website-docs is passed." >&2 + echo "error: Writerside's webHelpImages.zip is only produced by IntelliJ IDEA's" >&2 + echo "error: Writerside plugin - there is no headless way to generate it. See the" >&2 + echo "error: KNOWN LIMITATION note at the top of $WORKFLOW." >&2 + exit 1 + fi + if [ ! -f "$IMAGES_ZIP_PATH" ]; then + echo "error: --images-zip-path '$IMAGES_ZIP_PATH' does not exist or is not a file" >&2 + exit 1 + fi + IMAGES_ZIP_PATH="$(cd "$(dirname "$IMAGES_ZIP_PATH")" && pwd)/$(basename "$IMAGES_ZIP_PATH")" +fi + +mkdir -p "$OUTPUT_DIR" +OUTPUT_DIR="$(cd "$OUTPUT_DIR" && pwd)" + +# One -v per input. Mounting the files individually (rather than their parent +# directories) keeps the container's view to exactly what the run needs, and +# lets --db-path and --images-zip-path live in unrelated places on the host. +# +# act takes --container-options as one string and splits it with shell-style +# quoting rules, so each mount spec is emitted double-quoted: an unquoted +# join would break the moment a host path contained a space. +CONTAINER_OPTIONS="" +add_mount() { CONTAINER_OPTIONS+=" -v \"$1:$2\""; } +add_mount "$DB_PATH" "$CONTAINER_DB_PATH" +add_mount "$OUTPUT_DIR" "$CONTAINER_OUTPUT_DIR" +WORKFLOW_IMAGES_ZIP_PATH="" +if [ "$SKIP_WEBSITE_DOCS" != "true" ]; then + add_mount "$IMAGES_ZIP_PATH" "$CONTAINER_IMAGES_ZIP_PATH" + WORKFLOW_IMAGES_ZIP_PATH="$CONTAINER_IMAGES_ZIP_PATH" +fi + +# Last, after every argument check above, and immediately before the only +# thing that needs it. Anything earlier shadows a real complaint about the +# arguments with "act is required" on the machines that don't have act - +# which is most of them, including CI. Moving it off line 1 fixed that for +# the parse errors; it has to sit below the semantic checks too, or a missing +# --db-path or a --skip-website-docs/--skip-stdlib-docs pair still gets +# answered with the wrong message. +if ! command -v act >/dev/null 2>&1; then + echo "error: act is required - see https://github.com/nektos/act#installation" >&2 + exit 1 +fi + +if [ "$DRY_RUN" = "true" ]; then + echo "note: dry_run=true - '$DB_PATH' will NOT be modified; the built database is" >&2 + echo "note: written to '$OUTPUT_DIR' only. Both Slack notifications still fire if" >&2 + echo "note: SLACK_WEBHOOK_URL is set - the baton has to be dropped whatever the" >&2 + echo "note: outcome - and 'build complete' will say the database was unchanged." >&2 +else + echo "WARNING: --live - '$DB_PATH' will be OVERWRITTEN in place when the run finishes." >&2 +fi + +# The workflow reads SLACK_WEBHOOK_URL and tolerates it being unset, so pass +# it through when it's in the environment and stay silent when it isn't. +# +# SECRET_ARGS and EXTRA_ACT_ARGS are expanded below as +# ${arr[@]+"${arr[@]}"} rather than plain "${arr[@]}": macOS still ships bash +# 3.2, where `set -u` treats an empty array's "${arr[@]}" as an unbound +# variable and aborts. Both arrays are empty on a normal run. +SECRET_ARGS=() +if [ -n "${SLACK_WEBHOOK_URL:-}" ]; then + SECRETS_FILE="$(mktemp)" + trap 'rm -f "$SECRETS_FILE"' EXIT + printf 'SLACK_WEBHOOK_URL=%s\n' "$SLACK_WEBHOOK_URL" > "$SECRETS_FILE" + SECRET_ARGS=(--secret-file "$SECRETS_FILE") +fi + +echo "== Running $WORKFLOW via act ==" +echo " db_path $DB_PATH -> $CONTAINER_DB_PATH" +echo " images_zip_path ${IMAGES_ZIP_PATH:-(skipped)}${IMAGES_ZIP_PATH:+ -> $CONTAINER_IMAGES_ZIP_PATH}" +echo " output_dir $OUTPUT_DIR -> $CONTAINER_OUTPUT_DIR" +echo " dry_run=$DRY_RUN skip_website_docs=$SKIP_WEBSITE_DOCS skip_stdlib_docs=$SKIP_STDLIB_DOCS" +if [ "$SKIP_STDLIB_DOCS" != "true" ]; then + echo " stdlib artifacts ${KOTLIN_LIBS_VERSION:-(kotlin checkout default)} from ${KOTLIN_LIBS_REPO:-(mavenCentral)}" +fi + +# --container-daemon-socket - : act otherwise bind-mounts the host's Docker +# socket into the job container so steps can run Docker themselves. Nothing in +# this workflow does, and the mount outright fails on runtimes whose socket +# isn't a plain bind-mountable file - under colima it aborts the run with +# "error while creating mount source path ...: operation not supported". +act workflow_dispatch \ + -W "$WORKFLOW" \ + -P ubuntu-latest=catthehacker/ubuntu:act-latest \ + --container-daemon-socket - \ + --container-options "$CONTAINER_OPTIONS" \ + --input db_path="$CONTAINER_DB_PATH" \ + --input images_zip_path="$WORKFLOW_IMAGES_ZIP_PATH" \ + --input output_dir="$CONTAINER_OUTPUT_DIR" \ + --input kotlin_web_site_ref="$KOTLIN_WEB_SITE_REF" \ + --input kotlin_ref="$KOTLIN_REF" \ + --input kotlin_libs_version="$KOTLIN_LIBS_VERSION" \ + --input kotlin_libs_repo="$KOTLIN_LIBS_REPO" \ + --input skip_website_docs="$SKIP_WEBSITE_DOCS" \ + --input skip_stdlib_docs="$SKIP_STDLIB_DOCS" \ + --input dry_run="$DRY_RUN" \ + ${SECRET_ARGS[@]+"${SECRET_ARGS[@]}"} \ + ${EXTRA_ACT_ARGS[@]+"${EXTRA_ACT_ARGS[@]}"} diff --git a/scripts/sync_kotlin_stdlib_docs/sync_kdoc_json_to_db.py b/scripts/sync_kotlin_stdlib_docs/sync_kdoc_json_to_db.py index d1585de6..9b3b27b4 100755 --- a/scripts/sync_kotlin_stdlib_docs/sync_kdoc_json_to_db.py +++ b/scripts/sync_kotlin_stdlib_docs/sync_kdoc_json_to_db.py @@ -11,15 +11,30 @@ - If that file exists, re-compress it (matching the row's existing ContentTypes.compression) and overwrite the row's `content` blob only -- `path`, `languageID`, `contentTypeID`, and `templateId` are left untouched. - - If it doesn't exist, delete the row. - -Any TooltipButtons row whose `uri` (ignoring a trailing "#fragment") matches one -of the deleted Content paths is now a dead link. Its entire parent Tooltips + A result too large for a single row is split into "-1", "-2", + ... continuation rows instead (see CHUNK_SIZE), the same fragmentation + populate_db.py writes and WebServer.kt reads back; that case replaces the + base row rather than updating it in place, so its `id` changes, but the + four columns above still carry over unchanged. + - If it doesn't exist, delete the row (and any continuation rows it owns). + +Existing "-N" continuation rows are not treated as pages of their own: +they belong to their base row and are rewritten or removed along with it. + +Any TooltipButtons row whose `uri` (ignoring a "?query" and/or "#fragment", +matching docdb-studio's own URI normalizer) matches one of the deleted Content +paths is now a dead link. Its entire parent Tooltips record -- along with all of that tooltip's other TooltipButtons rows, dead or not -- is deleted too, since TooltipButtons has no ON DELETE CASCADE and a dangling tooltipId would otherwise be left behind. -A timestamped backup of the database is made before anything is modified. +A timestamped backup of the database is made before anything is modified - taken +after the prechecks, so a run that refuses to proceed doesn't leave one behind. +The database is VACUUMed at the end to reclaim the space freed by the rewrite. + +Pages present in the plugin output with no existing Content row are reported but +not inserted: this script only ever updates or deletes rows it found in the +database. """ import argparse import atexit @@ -34,6 +49,25 @@ import brotli +# The Content chunking protocol lives in exactly one place, so this script and +# the ProcessKotlinWebsiteJSON tools cannot drift apart on it again - four +# divergent re-derivations of the same rules is what produced ADFA-5171's +# undetected chains and an unrelated page being deleted as a "surplus fragment". +# This script is otherwise standalone (stdlib + brotli), hence the explicit path +# rather than a package import. +# append, not insert(0): position 0 would search that directory ahead of the +# standard library, so any module added there whose name collides with a stdlib +# one (types.py, json.py, io.py are all plausible) would be imported in its +# place, here and in everything imported transitively. This script needs the +# directory reachable, not preferred. +sys.path.append(str(Path(__file__).resolve().parents[2] + / "ProcessDocs" / "ProcessKotlinDocs" / "ProcessKotlinWebsiteJSON")) +from content_chunking import ( # noqa: E402 - must follow the sys.path line above + CHUNK_SIZE, + is_continuation_path, + owned_fragment_paths, +) + PREFIXES = ["k/kotlin-stdlib", "k/kotlin-reflect", "k/kotlin-test"] # Refuse to run if this fraction or more of the matched rows resolve to no source @@ -43,12 +77,95 @@ def backup_database(db_path): + """Writes a timestamped backup beside db_path. Uses SQLite's own VACUUM + INTO rather than a file copy: it takes a read transaction for the + duration, so the result is always an internally consistent database even + if something else is mid-write (a plain copy of a live, or WAL-mode, + database can be torn). Same approach populate_db.py uses.""" timestamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") backup_path = f"{db_path}.bak.{timestamp}" - shutil.copy2(db_path, backup_path) + conn = sqlite3.connect(db_path) + try: + conn.execute("VACUUM INTO ?", (backup_path,)) + finally: + conn.close() return backup_path +def is_fragment_path(path, lengths): + """True when path is a "-" chunk continuation row of a genuinely + chunked base - i.e. one whose own content is exactly CHUNK_SIZE bytes. + + Takes {path: length}, not just the set of paths: "the base exists" is not + sufficient, because two unrelated pages may legitimately be named "X" and + "X-1", and treating the second as a fragment makes it invisible to this + sync (never updated, never deleted, never reported).""" + return is_continuation_path(lengths, path) + + +def fragment_paths(conn, path): + """Every continuation row owned by `path`, ordered by suffix. + + Delegates to content_chunking.owned_fragment_paths, which short-circuits + on the base row's length before going near the "-%" LIKE. That + matters for cost as much as correctness here: this runs once per updated + row (tens of thousands for kotlin-stdlib), SQLite's default LIKE is + case-insensitive so UNIQUE(path) cannot serve it, and every one of those + calls used to scan the whole Content table - for a lookup that can only + return rows when the base is exactly CHUNK_SIZE bytes, which these pages + essentially never are.""" + return owned_fragment_paths(conn, path) + + +def delete_content_with_fragments(cur, content_id, path): + """Deletes a Content row along with any chunk continuation rows it owns. + + Ownership is resolved before the base row goes: it is the base's own + length that decides whether it owns continuations at all, and that is + unreadable once it has been deleted.""" + owned = fragment_paths(cur, path) + cur.execute("DELETE FROM Content WHERE id = ?", (content_id,)) + for fragment_path in owned: + cur.execute("DELETE FROM Content WHERE path = ?", (fragment_path,)) + + +def write_content(cur, content_id, path, blob, language_id, content_type_id, template_id, chunked_log): + """Replaces the content stored at `path` with `blob`, honouring the + CHUNK_SIZE fragmentation contract. + + The common case (blob fits in one row) is an in-place UPDATE, which keeps + the row's id stable; any stale fragments left over from a previous, + larger version of the page are removed. An oversized blob can't be stored + that way at all - the server would only ever serve the first row - so the + row is replaced by a fresh base row plus "-1", "-2", ... + continuations, each carrying the original row's languageID/contentTypeID/ + templateId. Appends (path, total size, chunk count) to chunked_log for + anything that needed more than one row.""" + stale = fragment_paths(cur, path) + + if len(blob) <= CHUNK_SIZE: + cur.execute("UPDATE Content SET content = ? WHERE id = ?", (blob, content_id)) + for fragment_path in stale: + cur.execute("DELETE FROM Content WHERE path = ?", (fragment_path,)) + return + + cur.execute("DELETE FROM Content WHERE id = ?", (content_id,)) + for fragment_path in stale: + cur.execute("DELETE FROM Content WHERE path = ?", (fragment_path,)) + + insert = ("INSERT INTO Content (path, languageID, content, contentTypeID, templateId) " + "VALUES (?, ?, ?, ?, ?)") + cur.execute(insert, (path, language_id, blob[:CHUNK_SIZE], content_type_id, template_id)) + fragment_number = 1 + offset = CHUNK_SIZE + while offset < len(blob): + cur.execute(insert, (f"{path}-{fragment_number}", language_id, blob[offset:offset + CHUNK_SIZE], + content_type_id, template_id)) + offset += CHUNK_SIZE + fragment_number += 1 + chunked_log.append((path, len(blob), fragment_number)) # fragment_number == total chunk count here + + def relative_target_path(content_path): """'k/kotlin-stdlib/kotlin.text/index.html' -> 'kotlin-stdlib/kotlin.text/index.json' 'k/kotlin-stdlib/package-list' -> 'kotlin-stdlib/package-list' (no extension to swap)""" @@ -111,6 +228,49 @@ def compress_for(compression, raw_bytes, path, compressor=None): raise ValueError(f"Unknown compression '{compression}' needed for {path}") +def content_path_for_source(plugin_output_root, source_file): + """Inverse of relative_target_path: the Content.path a Dokka output file + would populate. '/kotlin-stdlib/kotlin.text/index.json' -> + 'k/kotlin-stdlib/kotlin.text/index.html'.""" + rel = os.path.relpath(source_file, plugin_output_root) + rel = rel.replace(os.sep, "/") + if rel.endswith(".json"): + rel = rel[: -len(".json")] + ".html" + return "k/" + rel + + +def unmatched_source_pages(plugin_output_root, known_paths): + """Dokka output files that map to no existing Content row, sorted. + + This script can only UPDATE or DELETE: its loop iterates rows read from the + database, so a page Dokka newly emits has nothing to match and is silently + dropped. Bumping --kotlin-libs-version to a release that adds stdlib API is + exactly that case, and the refreshed pages link to those missing pages, so + every such link 404s in the app. Reporting them is the minimum; actually + inserting them needs contentTypeID/templateId decisions this script has no + basis to make on its own.""" + found = [] + for dirpath, _dirnames, filenames in os.walk(plugin_output_root): + for name in filenames: + if not (name.endswith(".json") or name == "package-list"): + continue + candidate = content_path_for_source(plugin_output_root, os.path.join(dirpath, name)) + if candidate not in known_paths: + found.append(candidate) + return sorted(found) + + +def _content_path_for_uri(uri): + """The Content.path a TooltipButtons.uri addresses: everything before the + first '?' or '#'. Mirrors docdb-studio's _uri_path_for_content_lookup.""" + u = uri or "" + if "?" in u: + u = u.split("?", 1)[0] + if "#" in u: + u = u.split("#", 1)[0] + return u + + def cleanup_orphaned_tooltips(cur, deleted_paths, dry_run): """Delete any Tooltips (and all their TooltipButtons) that reference a now-deleted Content path via a TooltipButtons.uri. Returns (tooltips_removed, @@ -129,8 +289,14 @@ def cleanup_orphaned_tooltips(cur, deleted_paths, dry_run): f"SELECT tooltipId, uri FROM TooltipButtons WHERE {where_clause}", params ).fetchall() + # Strip ?query as well as #fragment, matching this repo's canonical + # normalizer (docdb-studio's _uri_path_for_content_lookup). Splitting on + # "#" alone left a "...html?v=2" button pointing at a row this run just + # deleted - exactly the dangling state this cleanup exists to prevent, and + # one docdb-studio's "Validate URIs" audit then flags. orphaned_tooltip_ids = sorted( - {tooltip_id for tooltip_id, uri in candidate_buttons if uri.split("#", 1)[0] in deleted_path_set} + {tooltip_id for tooltip_id, uri in candidate_buttons + if _content_path_for_uri(uri) in deleted_path_set} ) if not orphaned_tooltip_ids: return 0, 0 @@ -170,9 +336,6 @@ def main(): if args.dry_run: print("Dry run: no backup will be made and no changes will be written.") - else: - backup_path = backup_database(args.db) - print(f"Backed up database to: {backup_path}") conn = sqlite3.connect(args.db) cur = conn.cursor() @@ -184,16 +347,31 @@ def main(): for prefix in PREFIXES: params.extend([prefix, prefix + "/%"]) - rows = cur.execute( - f"SELECT id, path, contentTypeID FROM Content WHERE {where_clause}", params + all_rows = cur.execute( + f"SELECT id, path, contentTypeID, languageID, templateId, LENGTH(content) " + f"FROM Content WHERE {where_clause}", params ).fetchall() - print(f"Found {len(rows)} existing Content record(s) under {PREFIXES}.") + # A chunked page is stored as a base row plus "-1", "-2", ... + # continuation rows (see CHUNK_SIZE). Those fragments are part of their + # base row's content, not pages in their own right - handled wholesale by + # write_content below - so drop them from the work list. Left in, each + # would be looked up as its own source file, never found (there's no + # "index.html-1" in the plugin output), and counted as a deletion. + all_paths = {row[1] for row in all_rows} + # Keyed by length, not just presence: only a base row of exactly + # CHUNK_SIZE bytes actually owns "-" continuations. + lengths = {row[1]: row[5] for row in all_rows} + rows = [row[:5] for row in all_rows if not is_fragment_path(row[1], lengths)] + fragments = len(all_rows) - len(rows) + + print(f"Found {len(rows)} existing Content record(s) under {PREFIXES}" + f"{f' (plus {fragments} chunk continuation row(s))' if fragments else ''}.") updated = 0 deleted = 0 deleted_paths = [] - unknown_types = set() + chunked_log = [] dictionary_data = load_compression_dictionary(conn) compressor = DictionaryBrotli(dictionary_data) if dictionary_data else None @@ -206,8 +384,8 @@ def main(): # Resolve every source file before touching anything, so a wholesale miss # aborts instead of deleting the rows one at a time (see MAX_DELETE_FRACTION). - missing = [path for _id, path, _type in rows - if not os.path.isfile(os.path.join(args.plugin_output_root, relative_target_path(path)))] + missing = [row[1] for row in rows + if not os.path.isfile(os.path.join(args.plugin_output_root, relative_target_path(row[1])))] if rows and len(missing) >= max(1, int(len(rows) * MAX_DELETE_FRACTION)): print( f"error: {len(missing)} of {len(rows)} matched Content rows resolve to no file under " @@ -219,9 +397,31 @@ def main(): conn.close() sys.exit(1) + # Reported, not inserted - see unmatched_source_pages. Printed before the + # transaction so it shows up even on a dry run. + # all_paths, not {row[0] ...}: row[0] is the integer Content.id, so the + # membership test could never match a path string and every source page + # was reported unmatched - tens of thousands of false warnings burying the + # one signal this check exists to surface. + unmatched = unmatched_source_pages(args.plugin_output_root, all_paths) + if unmatched: + print( + f"warning: {len(unmatched)} page(s) in {args.plugin_output_root!r} have no Content row and " + f"will NOT be inserted; links to them will 404. Examples: {', '.join(unmatched[:3])}", + file=sys.stderr, + ) + + # Backed up only once every check that can still refuse to run has passed - + # the MAX_DELETE_FRACTION precheck above is the last of them. Taking it + # earlier meant a run that correctly aborted on a layout mismatch still left + # a full-size copy of the database behind, for nothing. + if not args.dry_run: + backup_path = backup_database(args.db) + print(f"Backed up database to: {backup_path}") + try: conn.execute("BEGIN") - for content_id, path, content_type_id in rows: + for content_id, path, content_type_id, language_id, template_id in rows: rel_target = relative_target_path(path) source_file = os.path.join(args.plugin_output_root, rel_target) @@ -229,34 +429,44 @@ def main(): with open(source_file, "rb") as f: raw_bytes = f.read() + # An unresolvable contentTypeID means this row's declared + # type isn't in ContentTypes at all, so there's no way to + # know whether the server will try to decompress what gets + # written here. Guessing "uncompressed" and committing anyway + # is how a row ends up serving bytes that contradict its own + # declared type - fail instead. compression = compression_by_type.get(content_type_id) if compression is None: - unknown_types.add(content_type_id) - compression = "none" + raise RuntimeError( + f"{path} has contentTypeID {content_type_id}, which has no row in ContentTypes; " + "cannot tell how its content should be compressed. Fix the database's ContentTypes " + "table (or this row's contentTypeID) and re-run." + ) new_blob = compress_for(compression, raw_bytes, path, compressor) if args.dry_run: - print(f" [UPDATE] {path} <- {rel_target}") + chunks = -(-len(new_blob) // CHUNK_SIZE) or 1 + print(f" [UPDATE] {path} <- {rel_target}" + f"{f' ({len(new_blob):,} bytes -> {chunks} chunks)' if chunks > 1 else ''}") else: - cur.execute("UPDATE Content SET content = ? WHERE id = ?", (new_blob, content_id)) + write_content(cur, content_id, path, new_blob, language_id, content_type_id, template_id, + chunked_log) updated += 1 else: if args.dry_run: print(f" [DELETE] {path} (no matching {rel_target})") else: - cur.execute("DELETE FROM Content WHERE id = ?", (content_id,)) + delete_content_with_fragments(cur, content_id, path) deleted += 1 deleted_paths.append(path) tooltips_removed, buttons_removed = cleanup_orphaned_tooltips(cur, deleted_paths, args.dry_run) - if unknown_types: - print( - f"WARNING: contentTypeID(s) {sorted(unknown_types)} not found in ContentTypes; " - "treated as uncompressed.", - file=sys.stderr, - ) + if chunked_log: + print(f"Chunked {len(chunked_log)} file(s) over {CHUNK_SIZE:,} bytes:") + for path, total_size, chunk_count in chunked_log: + print(f" {path}: {total_size:,} bytes -> {chunk_count} chunks") if args.dry_run: conn.rollback() @@ -277,6 +487,22 @@ def main(): finally: conn.close() + # SQLite only ever moves freed pages onto its internal freelist; the file + # itself never shrinks. This script rewrites every k/kotlin-stdlib* blob and + # deletes Content, Tooltips and TooltipButtons rows, so a run that replaces + # large pages with smaller ones leaves the difference as dead space. It is + # step 5/5 of the pipeline, so nothing downstream reclaims it and the bloat + # ships in the on-device database. Same trailing VACUUM as populate_db.py, + # insert_optimized_media.py and renumber_misnumbered_fragments.py, on its + # own connection because SQLite refuses to VACUUM inside a transaction. + if not args.dry_run: + print("Vacuuming database to reclaim freed space...") + vacuum_conn = sqlite3.connect(args.db) + try: + vacuum_conn.execute("VACUUM") + finally: + vacuum_conn.close() + if __name__ == "__main__": main() diff --git a/scripts/sync_kotlin_stdlib_docs/test_sync_kdoc_json_to_db.py b/scripts/sync_kotlin_stdlib_docs/test_sync_kdoc_json_to_db.py new file mode 100644 index 00000000..1cc16adc --- /dev/null +++ b/scripts/sync_kotlin_stdlib_docs/test_sync_kdoc_json_to_db.py @@ -0,0 +1,331 @@ +"""Regression tests for sync_kdoc_json_to_db.py. + +Covers the ways this script could write content the server cannot read back: +ignoring the CHUNK_SIZE fragmentation contract, over-matching when deleting +continuation rows, and guessing at a compression policy it can't determine. +Dictionary compression itself (ADFA-5153) is exercised end-to-end here too, +since a plain-Brotli row in a dictionary database is unreadable. +""" +import shutil +import sqlite3 +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from sync_kdoc_json_to_db import ( # noqa: E402 + CHUNK_SIZE, + DictionaryBrotli, + backup_database, + compress_for, + delete_content_with_fragments, + fragment_paths, + is_fragment_path, + load_compression_dictionary, + content_path_for_source, + relative_target_path, + unmatched_source_pages, + write_content, +) + +SCHEMA = """ +CREATE TABLE ContentTypes (id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT NOT NULL UNIQUE, + compression TEXT NOT NULL); +CREATE TABLE Content ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + path TEXT NOT NULL, + languageID INTEGER NOT NULL, + content BLOB NOT NULL, + contentTypeID INTEGER NOT NULL, + templateId INTEGER NOT NULL DEFAULT 0, + UNIQUE(path) +); +""" + +HTML_TYPE_ID = 12 +needs_brotli_cli = pytest.mark.skipif(shutil.which("brotli") is None, reason="brotli CLI not installed") + + +@pytest.fixture +def conn(): + connection = sqlite3.connect(":memory:") + connection.executescript(SCHEMA) + connection.execute("INSERT INTO ContentTypes (id, value, compression) VALUES (?, 'text/html', 'brotli')", + (HTML_TYPE_ID,)) + yield connection + connection.close() + + +# Only a base row of exactly CHUNK_SIZE bytes owns "-" continuations +# - that length is what distinguishes a split page from two unrelated pages +# that happen to share a name prefix. Fixtures chaining fragments off a 3-byte +# base describe a shape that cannot occur and would pass either way. +CHUNKED_BASE = b"x" * CHUNK_SIZE + + +def add_row(conn, path, blob=b"old", template_id=7, language_id=1): + return conn.execute( + "INSERT INTO Content (path, languageID, content, contentTypeID, templateId) VALUES (?, ?, ?, ?, ?)", + (path, language_id, blob, HTML_TYPE_ID, template_id), + ).lastrowid + + +def rows(conn): + return dict(conn.execute("SELECT path, content FROM Content")) + + +class TestRelativeTargetPath: + def test_html_becomes_json(self): + assert relative_target_path("k/kotlin-stdlib/kotlin.text/index.html") == "kotlin-stdlib/kotlin.text/index.json" + + def test_extensionless_path_is_unchanged(self): + assert relative_target_path("k/kotlin-stdlib/package-list") == "kotlin-stdlib/package-list" + + +class TestIsFragmentPath: + def test_recognises_a_continuation_row(self): + assert is_fragment_path("k/kotlin-stdlib/x.html-1", + {"k/kotlin-stdlib/x.html": CHUNK_SIZE, "k/kotlin-stdlib/x.html-1": 3}) + + def test_a_base_row_is_not_a_fragment(self): + assert not is_fragment_path("k/kotlin-stdlib/x.html", {"k/kotlin-stdlib/x.html": CHUNK_SIZE}) + + def test_trailing_digits_without_a_base_are_not_a_fragment(self): + assert not is_fragment_path("k/kotlin-stdlib/part-2", {"k/kotlin-stdlib/part-2": 3}) + + def test_non_numeric_suffix_is_not_a_fragment(self): + assert not is_fragment_path("k/kotlin-stdlib/all-types", + {"k/kotlin-stdlib/all": CHUNK_SIZE, "k/kotlin-stdlib/all-types": 3}) + + +class TestFragmentPaths: + def test_finds_the_chain_in_order(self, conn): + add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + for n in (2, 1, 3): + add_row(conn, f"k/kotlin-stdlib/x.html-{n}") + assert fragment_paths(conn, "k/kotlin-stdlib/x.html") == [ + "k/kotlin-stdlib/x.html-1", "k/kotlin-stdlib/x.html-2", "k/kotlin-stdlib/x.html-3", + ] + + def test_finds_a_chain_that_starts_at_two(self, conn): + # ADFA-5171: probing "-1" first and stopping at the gap would miss + # these entirely and leave them behind as orphans. + add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/x.html-2") + add_row(conn, "k/kotlin-stdlib/x.html-3") + assert fragment_paths(conn, "k/kotlin-stdlib/x.html") == [ + "k/kotlin-stdlib/x.html-2", "k/kotlin-stdlib/x.html-3", + ] + + def test_underscore_in_a_path_is_not_treated_as_a_wildcard(self, conn): + add_row(conn, "k/kotlin-stdlib/a_b.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/a_b.html-1") + add_row(conn, "k/kotlin-stdlib/aXb.html-1") + assert fragment_paths(conn, "k/kotlin-stdlib/a_b.html") == ["k/kotlin-stdlib/a_b.html-1"] + + def test_lookalike_suffixes_are_excluded(self, conn): + add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/x.html-notanumber") + assert fragment_paths(conn, "k/kotlin-stdlib/x.html") == [] + + +class TestWriteContent: + def test_small_blob_updates_in_place_and_keeps_the_row_id(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + write_content(conn, row_id, "k/kotlin-stdlib/x.html", b"new", 1, HTML_TYPE_ID, 7, []) + assert rows(conn) == {"k/kotlin-stdlib/x.html": b"new"} + assert conn.execute("SELECT id FROM Content").fetchone()[0] == row_id + + def test_small_blob_clears_stale_fragments_from_a_previous_larger_version(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/x.html-1") + add_row(conn, "k/kotlin-stdlib/x.html-2") + write_content(conn, row_id, "k/kotlin-stdlib/x.html", b"new", 1, HTML_TYPE_ID, 7, []) + assert set(rows(conn)) == {"k/kotlin-stdlib/x.html"} + + def test_oversized_blob_is_split_across_continuation_rows(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/big.html") + blob = b"z" * (CHUNK_SIZE * 2 + 17) + chunked_log = [] + write_content(conn, row_id, "k/kotlin-stdlib/big.html", blob, 1, HTML_TYPE_ID, 7, chunked_log) + + stored = rows(conn) + assert set(stored) == {"k/kotlin-stdlib/big.html", "k/kotlin-stdlib/big.html-1", "k/kotlin-stdlib/big.html-2"} + # The server detects fragmentation by the base row being exactly + # CHUNK_SIZE, then reads on until a short row. + assert len(stored["k/kotlin-stdlib/big.html"]) == CHUNK_SIZE + assert len(stored["k/kotlin-stdlib/big.html-1"]) == CHUNK_SIZE + assert len(stored["k/kotlin-stdlib/big.html-2"]) == 17 + assert (stored["k/kotlin-stdlib/big.html"] + stored["k/kotlin-stdlib/big.html-1"] + + stored["k/kotlin-stdlib/big.html-2"]) == blob + assert chunked_log == [("k/kotlin-stdlib/big.html", len(blob), 3)] + + def test_fragments_inherit_the_base_row_columns(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/big.html", template_id=9) + write_content(conn, row_id, "k/kotlin-stdlib/big.html", b"z" * (CHUNK_SIZE + 1), 1, HTML_TYPE_ID, 9, []) + for language_id, content_type_id, template_id in conn.execute( + "SELECT languageID, contentTypeID, templateId FROM Content" + ): + assert (language_id, content_type_id, template_id) == (1, HTML_TYPE_ID, 9) + + def test_exactly_chunk_size_stays_a_single_row(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + write_content(conn, row_id, "k/kotlin-stdlib/x.html", b"z" * CHUNK_SIZE, 1, HTML_TYPE_ID, 7, []) + assert set(rows(conn)) == {"k/kotlin-stdlib/x.html"} + + +class TestDeleteContentWithFragments: + def test_removes_the_base_row_and_its_fragments_only(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/x.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/x.html-1") + add_row(conn, "k/kotlin-stdlib/y.html") + delete_content_with_fragments(conn, row_id, "k/kotlin-stdlib/x.html") + assert set(rows(conn)) == {"k/kotlin-stdlib/y.html"} + + def test_does_not_delete_lookalike_rows(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/a_b.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/a_b.html-1") + add_row(conn, "k/kotlin-stdlib/aXb.html-1") + delete_content_with_fragments(conn, row_id, "k/kotlin-stdlib/a_b.html") + assert set(rows(conn)) == {"k/kotlin-stdlib/aXb.html-1"} + + +class TestCompressFor: + def test_unknown_compression_is_an_error(self): + with pytest.raises(ValueError, match="Unknown compression"): + compress_for("lzma", b"data", "k/kotlin-stdlib/x.html") + + def test_none_passes_bytes_through(self): + assert compress_for("none", b"data", "p") == b"data" + + def test_brotli_without_a_dictionary_is_plain_brotli(self): + import brotli + assert brotli.decompress(compress_for("brotli", b"data" * 50, "p")) == b"data" * 50 + + +class TestLoadCompressionDictionary: + def test_returns_none_when_the_table_predates_schema_2(self, conn): + assert load_compression_dictionary(conn) is None + + def test_returns_none_for_an_empty_dictionary_table(self, conn): + conn.execute("CREATE TABLE CompressionDictionary (id INTEGER PRIMARY KEY CHECK (id = 1), data BLOB NOT NULL)") + assert load_compression_dictionary(conn) is None + + def test_returns_the_stored_bytes(self, conn): + conn.execute("CREATE TABLE CompressionDictionary (id INTEGER PRIMARY KEY CHECK (id = 1), data BLOB NOT NULL)") + conn.execute("INSERT INTO CompressionDictionary (id, data) VALUES (1, ?)", (b"dictionary-bytes",)) + assert load_compression_dictionary(conn) == b"dictionary-bytes" + + +@needs_brotli_cli +class TestDictionaryBrotli: + """DictionaryBrotli only compresses - this script never reads content back - + so these decode through the `brotli` CLI directly, the same way the server + ultimately does.""" + + DICTIONARY = bytes(range(256)) * 64 + + @staticmethod + def cli_decompress(blob, dictionary, tmp_path): + dict_path = tmp_path / "dictionary.bin" + dict_path.write_bytes(dictionary) + import subprocess + result = subprocess.run( + [shutil.which("brotli"), "-d", "-D", str(dict_path), "-c"], + input=blob, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False, + ) + assert result.returncode == 0, result.stderr.decode(errors="replace") + return result.stdout + + def test_round_trips_through_the_dictionary(self, tmp_path): + payload = b'{"id":"k/kotlin-stdlib/x","blocks":[]}' * 20 + blob = DictionaryBrotli(self.DICTIONARY).compress(payload) + assert self.cli_decompress(blob, self.DICTIONARY, tmp_path) == payload + + def test_dictionary_output_is_not_readable_as_plain_brotli(self): + # The whole reason this matters: the two encodings are not + # interchangeable, so writing plain Brotli into a dictionary database + # produces rows the server cannot decode. + import brotli + blob = DictionaryBrotli(self.DICTIONARY).compress(b"kotlin stdlib documentation payload" * 40) + with pytest.raises(Exception): + brotli.decompress(blob) + + def test_compress_for_uses_the_dictionary_when_given_one(self, tmp_path): + compressor = DictionaryBrotli(self.DICTIONARY) + payload = b"payload" * 100 + blob = compress_for("brotli", payload, "p", compressor) + assert self.cli_decompress(blob, self.DICTIONARY, tmp_path) == payload + + +class TestBackupDatabase: + def test_backup_is_a_readable_database_not_a_file_copy(self, tmp_path): + db_path = tmp_path / "documentation.db" + setup = sqlite3.connect(db_path) + setup.executescript(SCHEMA) + setup.execute("INSERT INTO ContentTypes (value, compression) VALUES ('text/html', 'brotli')") + setup.commit() + setup.close() + + backup_path = backup_database(str(db_path)) + + assert Path(backup_path).is_file() + restored = sqlite3.connect(backup_path) + try: + assert restored.execute("PRAGMA integrity_check").fetchone()[0] == "ok" + assert restored.execute("SELECT value FROM ContentTypes").fetchone()[0] == "text/html" + finally: + restored.close() + + +class TestUnmatchedSourcePages: + """F01: this was handed `{row[0] for row in all_rows}` - a set of integer + Content ids - so the `candidate in known_paths` test could never match and + every source page was reported unmatched, burying the one genuinely new + page under tens of thousands of false warnings.""" + + def _tree(self, tmp_path): + (tmp_path / "kotlin-stdlib" / "kotlin.text").mkdir(parents=True) + for name in ("index.json", "brand-new.json"): + (tmp_path / "kotlin-stdlib" / "kotlin.text" / name).write_text("{}") + return tmp_path + + def test_reports_only_pages_with_no_content_row(self, tmp_path): + root = self._tree(tmp_path) + known = {"k/kotlin-stdlib/kotlin.text/index.html"} + assert unmatched_source_pages(str(root), known) == ["k/kotlin-stdlib/kotlin.text/brand-new.html"] + + def test_row_ids_would_have_matched_nothing(self, tmp_path): + """Guards the actual regression: given ids instead of paths, every page + comes back unmatched.""" + root = self._tree(tmp_path) + assert len(unmatched_source_pages(str(root), {1, 2, 3})) == 2 + + def test_maps_a_source_file_back_to_its_content_path(self, tmp_path): + root = self._tree(tmp_path) + source = str(root / "kotlin-stdlib" / "kotlin.text" / "index.json") + assert content_path_for_source(str(root), source) == "k/kotlin-stdlib/kotlin.text/index.html" + + +class TestFragmentOwnershipRules: + """F02/F08: a "-" row belongs to its base only when that base is + exactly CHUNK_SIZE bytes. Without the gate an unrelated page is treated as + a fragment - invisible to the sync, and deleted along with the base.""" + + def test_short_base_owns_no_fragments(self, conn): + add_row(conn, "k/kotlin-stdlib/guide.html", b"a small page") + add_row(conn, "k/kotlin-stdlib/guide.html-1", b"an unrelated page") + assert fragment_paths(conn, "k/kotlin-stdlib/guide.html") == [] + + def test_unrelated_lookalike_survives_a_delete(self, conn): + row_id = add_row(conn, "k/kotlin-stdlib/guide.html", b"a small page") + add_row(conn, "k/kotlin-stdlib/guide.html-1", b"an unrelated page") + delete_content_with_fragments(conn, row_id, "k/kotlin-stdlib/guide.html") + assert set(rows(conn)) == {"k/kotlin-stdlib/guide.html-1"} + + def test_chunked_base_still_owns_its_chain(self, conn): + add_row(conn, "k/kotlin-stdlib/big.html", CHUNKED_BASE) + add_row(conn, "k/kotlin-stdlib/big.html-1", b"tail") + assert fragment_paths(conn, "k/kotlin-stdlib/big.html") == ["k/kotlin-stdlib/big.html-1"]