diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py index ce4b28e3..c7a8ebe0 100644 --- a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py @@ -200,6 +200,76 @@ # worth it). DEFAULT_DICT_SIZE = 256 * 1024 +# ADFA-5220. MAJOR is a compatibility contract, not a build number: 2 means the +# brotli Content rows are compressed against CompressionDictionary, which is +# exactly what the app gates on (DatabaseVersionResolver's +# MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY). Bump MAJOR only when a reader that +# understands the previous number would get this format wrong; MINOR/PATCH for +# additions and fixes it can ignore. +# +# This has to be written by whichever step establishes the format, and that is +# this script, because it is where load_or_create_dictionary mints the +# dictionary. A database with dictionary-compressed content and no version row +# reads as unversioned, so the app declines to use the dictionary and every +# brotli row then fails to decode -- a working-looking database that serves +# nothing. +# +# The table holds exactly one row: the version this file is, not a log of what it +# has been. declare_database_version therefore replaces rather than appends. +DATABASE_FORMAT_VERSION = (2, 0, 0) + +# Verbatim from ADFA-5220, including the comments: this is the schema the app +# reads and the shipped database already carries, so it is copied rather than +# paraphrased. changeTime is left to its default. +VERSION_TABLE_SQL = """ +CREATE TABLE IF NOT EXISTS DocumentationDatabaseVersion ( + -- From https://semver.org/ + -- + -- Given a version number MAJOR.MINOR.PATCH, increment the: + -- MAJOR version when you make incompatible API changes + -- MINOR version when you add functionality in a backward compatible manner + -- PATCH version when you make backward compatible bug fixes + major INT NOT NULL, + minor INT NOT NULL, + patch INT NOT NULL, + who TEXT NOT NULL, -- Who made the change? + comment TEXT NOT NULL, -- What changed? + changeTime TIMESTAMP DEFAULT CURRENT_TIMESTAMP -- Don't provide this. The default is fine. +); +""" + + +def declare_database_version( + conn, + who: str = "populate_db.py", + comment: str = "Content rows compressed against CompressionDictionary", +) -> bool: + """Records DATABASE_FORMAT_VERSION in DocumentationDatabaseVersion, creating + the table if this database predates it. Returns whether the row changed. + + **The table holds exactly one row**: the version this file *is*, not a history + of what it has been. So this replaces rather than appends, and a database that + somehow holds several rows -- an older tool, a hand-edit -- is collapsed back + to one. Anything wanting the history has git and the LastChange table. + + The replacement is unconditional in either direction, including a version + lower than the file already declares: rebuilding from an older pipeline + genuinely produces an older format, and the row has to say what the file + contains now rather than the highest it ever contained. + """ + conn.execute(VERSION_TABLE_SQL) + declared = conn.execute("SELECT major, minor, patch FROM DocumentationDatabaseVersion").fetchall() + if len(declared) == 1 and tuple(declared[0]) == DATABASE_FORMAT_VERSION: + return False + + major, minor, patch = DATABASE_FORMAT_VERSION + conn.execute("DELETE FROM DocumentationDatabaseVersion") + conn.execute( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) VALUES (?, ?, ?, ?, ?)", + (major, minor, patch, who, comment), + ) + return True + def find_pngquant() -> str: """Locates the pngquant executable on PATH. Raises if it's missing, @@ -752,6 +822,14 @@ def main(): nav_json_bytes = json.dumps({"tree": nav_tree}, separators=(",", ":"), ensure_ascii=False).encode("utf-8") dictionary_data = load_or_create_dictionary(conn, page_json_bytes + [nav_json_bytes]) + # Declared next to the dictionary because the dictionary is what the + # version means (ADFA-5220): the two have to land in the same + # transaction, or a reader can see one without the other. + version = ".".join(str(part) for part in DATABASE_FORMAT_VERSION) + if declare_database_version(conn): + print(f"Declared documentation database version {version}", file=sys.stderr) + else: + print(f"Documentation database already declares version {version}", file=sys.stderr) with DictionaryCompressor(dictionary_data) as compressor: for page, json_bytes in zip(pages, page_json_bytes): path = f"{page['id']}.html" diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py new file mode 100644 index 00000000..774c69d8 --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py @@ -0,0 +1,91 @@ +#!/usr/bin/env python3 +"""Tests for populate_db's database-version declaration (ADFA-5220). + +Run directly: python3 test_database_version.py +""" +import sqlite3 +import tempfile +import unittest +from pathlib import Path + +from populate_db import ( + DATABASE_FORMAT_VERSION, + VERSION_TABLE_SQL, + declare_database_version, +) + + +class DeclareDatabaseVersionTest(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) + + def tearDown(self): + self.conn.close() + self.db_path.unlink(missing_ok=True) + + def declared(self) -> list: + return self.conn.execute( + "SELECT major, minor, patch, who FROM DocumentationDatabaseVersion ORDER BY rowid" + ).fetchall() + + def test_creates_the_table_and_declares_the_version(self): + """A database built before the table exists gets one, populated.""" + self.assertTrue(declare_database_version(self.conn)) + major, minor, patch = DATABASE_FORMAT_VERSION + self.assertEqual(self.declared(), [(major, minor, patch, "populate_db.py")]) + + def test_a_second_run_changes_nothing(self): + """Re-declaring the same version is a no-op, so a rebuild does not + rewrite a row that already says the right thing.""" + declare_database_version(self.conn) + self.assertFalse(declare_database_version(self.conn)) + self.assertEqual(len(self.declared()), 1) + + def test_an_older_declaration_is_replaced_not_appended(self): + """The table holds the version the file *is*, so the old row goes.""" + declare_database_version(self.conn, who="someone", comment="older format") + self.conn.execute( + "UPDATE DocumentationDatabaseVersion SET major = 1, minor = 4, patch = 0" + ) + self.assertTrue(declare_database_version(self.conn)) + self.assertEqual([row[:3] for row in self.declared()], [DATABASE_FORMAT_VERSION]) + + def test_a_downgrade_replaces_a_higher_declaration(self): + """Rebuilding from an older pipeline really does produce an older format. + The row has to say what the file contains now, not the highest version it + ever contained.""" + self.conn.execute(VERSION_TABLE_SQL) + self.conn.execute( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + "VALUES (9, 0, 0, 'future', 'a format this pipeline does not produce')" + ) + self.assertTrue(declare_database_version(self.conn)) + self.assertEqual([row[:3] for row in self.declared()], [DATABASE_FORMAT_VERSION]) + + def test_several_rows_are_collapsed_to_one(self): + """A database that picked up extra rows -- an older tool that appended, a + hand-edit -- is repaired rather than read around, so "the version" can + never be ambiguous.""" + self.conn.execute(VERSION_TABLE_SQL) + for major in (1, 2, 3): + self.conn.execute( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + "VALUES (?, 0, 0, 'older tool', 'appended')", + (major,), + ) + self.assertTrue(declare_database_version(self.conn)) + self.assertEqual([row[:3] for row in self.declared()], [DATABASE_FORMAT_VERSION]) + + def test_the_declared_version_is_what_the_app_gates_on(self): + """CoGo's DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY + is 2; a database this script builds carries dictionary-compressed rows, so + declaring anything lower would make the app decline to use the dictionary + and fail every brotli row.""" + self.assertGreaterEqual(DATABASE_FORMAT_VERSION[0], 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/docdb-studio/README.md b/docdb-studio/README.md index d8c654dd..0061b988 100644 --- a/docdb-studio/README.md +++ b/docdb-studio/README.md @@ -81,6 +81,14 @@ These same commands work on macOS, Linux, and Windows (in PowerShell, Command Pr ## Installing the `brotli` command-line tool +## Database versions it will open + +Databases declare a format version in `DocumentationDatabaseVersion` (ADFA-5220). docdb-studio understands MAJOR **2** and **refuses to open anything higher**, showing what it found and what it expected instead of the browser. + +That refusal is not caution for its own sake: a higher MAJOR means the format changed in a way this build would read incorrectly, and because this tool *writes*, a misreading gets saved back into the file. Update docdb-studio when you meet one. + +An older version, or none at all, opens normally. Those formats are strictly simpler -- no shared compression dictionary -- and are read correctly by the same fallback that handles plain-Brotli rows. + One dependency `uv sync` cannot install for you. Databases built since ADFA-5153 compress their `Content` rows against a shared dictionary stored inside the database itself, and no Python library exposes a custom dictionary, so docdb-studio runs the `brotli` **command-line program** to read and write those rows. If it is missing, content rows come up blank and docdb-studio prints an explanatory error in the terminal you launched it from — so a blank preview plus that message means "install this tool", not "this page is empty". A database with no `CompressionDictionary` table needs nothing extra. > **The `brotli` in `uv sync` is not this.** The Python package named `brotli` is already installed for you, and it is a *different thing* from the `brotli` program. Installing the Python package again will not help; you need the program, which puts a `brotli` (or `brotli.exe`) command on your `PATH`. diff --git a/docdb-studio/SCHEMA.md b/docdb-studio/SCHEMA.md index e10c2845..e9c9cc10 100644 --- a/docdb-studio/SCHEMA.md +++ b/docdb-studio/SCHEMA.md @@ -54,3 +54,16 @@ CREATE TABLE LastChange ( changeTime TIMESTAMP DEFAULT CURRENT_TIMESTAMP, who TEXT ); +CREATE TABLE DocumentationDatabaseVersion ( + major INT NOT NULL, + minor INT NOT NULL, + patch INT NOT NULL, + who TEXT NOT NULL, + comment TEXT NOT NULL, + changeTime TIMESTAMP DEFAULT CURRENT_TIMESTAMP +); + +`DocumentationDatabaseVersion` holds **exactly one row**: the format version the +file *is*, not a history of what it has been. Nothing in the DDL enforces that, +so `populate_db.py` replaces the row rather than appending, and collapses a file +that somehow accumulated several back to one. diff --git a/docdb-studio/docdb_studio.py b/docdb-studio/docdb_studio.py index 3a9991d9..a62c2c8f 100644 --- a/docdb-studio/docdb_studio.py +++ b/docdb-studio/docdb_studio.py @@ -164,6 +164,50 @@ def _search_params(term: str) -> tuple[str, ...]: return (prefix, prefix, prefix, prefix, contains, contains) +# ADFA-5220: the MAJOR version this build knows how to read. MAJOR is a +# compatibility contract, so a database declaring a higher one may store Content +# in a way this code would misread -- and this tool *writes*, so misreading means +# writing damage back. Refuse it instead. +# +# A lower or absent version is fine and is not refused: those formats are +# strictly simpler (no shared dictionary, plain Brotli) and decompress_brotli's +# fallback already reads them. +SUPPORTED_DATABASE_MAJOR_VERSION = 2 + + +def database_major_version(db_path: Path) -> int | None: + """The MAJOR version `db_path` declares (ADFA-5220), or None when it declares + none -- which is how every database built before that table identifies itself. + + The table holds exactly one row -- the version the file is, not a history of + what it has been -- so the ORDER BY below is a defence, not a model: if a + database ever turns up with several rows, this reads the one written last + instead of whichever SQLite happens to return. Same rule as the app's + DatabaseVersionResolver, and populate_db repairs the file when it sees this. + """ + with sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) as conn: + table = conn.execute( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'DocumentationDatabaseVersion'" + ).fetchone() + if table is None: + return None + row = conn.execute( + "SELECT major FROM DocumentationDatabaseVersion ORDER BY rowid DESC LIMIT 1" + ).fetchone() + return row[0] if row is not None and row[0] is not None else None + + +def unsupported_version_error(db_path: Path, major: int) -> str: + """The message shown instead of the UI when a database is too new to edit.""" + return ( + f"{db_path} declares documentation database version {major}, and this build of " + f"docdb-studio understands version {SUPPORTED_DATABASE_MAJOR_VERSION}.\n\n" + "A higher MAJOR means the format changed in a way this build would read incorrectly, and " + "editing through it could write that misreading back into the file. Update docdb-studio, " + "or open a database built for this version." + ) + + def get_total_count(db_path: Path, search_term: str | None = None) -> int: sql = COUNT_BASE.rstrip() + (SEARCH_WHERE if search_term else "") params = _search_params(search_term) if search_term else () @@ -4930,7 +4974,13 @@ def update_bulk_delete_button() -> None: error = f"File not found: {args.database}" else: try: - total_count = get_total_count(args.database) + # Checked before anything else touches the database: a format this + # build does not understand must not be read *or* written (ADFA-5220). + declared_major = database_major_version(args.database) + if declared_major is not None and declared_major > SUPPORTED_DATABASE_MAJOR_VERSION: + error = unsupported_version_error(args.database, declared_major) + else: + total_count = get_total_count(args.database) except sqlite3.Error as e: error = str(e) diff --git a/docdb-studio/tests/test_compression_dictionary.py b/docdb-studio/tests/test_compression_dictionary.py index 2ed41d42..4a8e68fc 100644 --- a/docdb-studio/tests/test_compression_dictionary.py +++ b/docdb-studio/tests/test_compression_dictionary.py @@ -280,3 +280,86 @@ def test_missing_brotli_cli_is_reported_not_swallowed(capsys) -> None: finally: docdb_studio.shutil.which = real_which db.unlink(missing_ok=True) + + +# ---------- ADFA-5220: refuse a database this build cannot read ---------- + + +def _set_version(db: Path, major: int, minor: int = 0, patch: int = 0) -> None: + with sqlite3.connect(db) as conn: + conn.execute( + """ + CREATE TABLE IF NOT EXISTS DocumentationDatabaseVersion ( + major INT NOT NULL, minor INT NOT NULL, patch INT NOT NULL, + who TEXT NOT NULL, comment TEXT NOT NULL, + changeTime TIMESTAMP DEFAULT CURRENT_TIMESTAMP) + """ + ) + conn.execute("DELETE FROM DocumentationDatabaseVersion") + conn.execute( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + "VALUES (?, ?, ?, 'test', 'test')", + (major, minor, patch), + ) + conn.commit() + + +def _append_stray_version(db: Path, major: int) -> None: + """Adds a row without removing the existing one -- what the contract forbids, + so the reader can be tested against a file that broke it.""" + with sqlite3.connect(db) as conn: + conn.execute( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + "VALUES (?, 0, 0, 'stray', 'appended')", + (major,), + ) + conn.commit() + + +def test_a_database_with_no_version_table_reads_as_unversioned() -> None: + db = _make_db() + try: + assert docdb_studio.database_major_version(db) is None + finally: + db.unlink(missing_ok=True) + + +def test_a_stray_extra_row_cannot_make_the_answer_arbitrary() -> None: + """The table is supposed to hold one row. If a file breaks that, the version + read has to still be deterministic -- the row written last -- rather than + whichever one SQLite happens to return first.""" + db = _make_db() + try: + _set_version(db, 3) + _append_stray_version(db, 2) + assert docdb_studio.database_major_version(db) == 2 + finally: + db.unlink(missing_ok=True) + + +def test_a_supported_or_older_version_is_not_refused() -> None: + """Older formats are strictly simpler -- no shared dictionary -- and + decompress_brotli's plain fallback already reads them.""" + db = _make_db() + try: + for major in (1, docdb_studio.SUPPORTED_DATABASE_MAJOR_VERSION): + _set_version(db, major) + assert docdb_studio.database_major_version(db) <= docdb_studio.SUPPORTED_DATABASE_MAJOR_VERSION + finally: + db.unlink(missing_ok=True) + + +def test_a_newer_major_produces_an_actionable_refusal() -> None: + db = _make_db() + try: + newer = docdb_studio.SUPPORTED_DATABASE_MAJOR_VERSION + 1 + _set_version(db, newer) + assert docdb_studio.database_major_version(db) == newer + + message = docdb_studio.unsupported_version_error(db, newer) + assert str(newer) in message + assert str(docdb_studio.SUPPORTED_DATABASE_MAJOR_VERSION) in message + # says what to do, not just what is wrong + assert "Update docdb-studio" in message + finally: + db.unlink(missing_ok=True)