From 51861d7c46e50b9240d34c074c696668b25c6662 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 21:45:17 -0700 Subject: [PATCH 1/2] ADFA-5220: Write the database version, and refuse a format we cannot read The version table existed and the app already gated on it, but nothing created it. Grepping this repo for DocumentationDatabaseVersion returned nothing: not populate_db.py, not the migration or re-mint scripts, not docdb-studio, not the workflows. The one row in the shipped database was inserted by hand. That matters because CoGo now reads it. DatabaseVersionResolver treats a missing table as "unversioned" and declines to attach the compression dictionary, so a freshly built database -- dictionary-compressed content, no version row -- would have the app skip the dictionary and fail every brotli row: a well-formed file that serves nothing. It is not broken today only because the currently shipped asset has no dictionary either; it breaks the moment this pipeline's dictionary work produces one. populate_db declares the version in the same transaction that mints the dictionary, because the dictionary is what the version means. A row is appended only when the declared version *changes*: the table logs what the format became, not who ran what, and a row per invocation would bury the two or three entries that matter. A last row differing in either direction is appended, including a lower version than the file already declares -- rebuilding from an older pipeline genuinely is a downgrade, and "the row inserted last wins" is the convention the app's reader implements, so recording it is what keeps the file honest. docdb-studio now reads that version before anything else touches the database and refuses a MAJOR above the one it understands, showing what it found and what it expected instead of the browser. Refusal rather than a warning because this tool writes: a format it would read incorrectly is a format it would save damage back into. Lower or absent versions open normally -- those are strictly simpler, and the plain-Brotli fallback already reads them. The version-reading rule is duplicated deliberately rather than shared: this repo and the app cannot import from each other, so both read the row inserted last by rowid, and both say so in a comment naming the other. Nine tests. Five on the declaration -- creates the table, a second run adds nothing, an older declaration is superseded, a downgrade is recorded rather than hidden, and the declared MAJOR is at least what the app gates on. Four on docdb-studio -- unversioned reads as none, the last row wins over a higher earlier one, supported and older versions are not refused, and the refusal message names both versions and says what to do. 34 pipeline tests and 178 docdb-studio tests pass. SCHEMA.md gains the table; docdb-studio's README explains which versions it will open and why it declines the rest. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit dbae587b00708069121e29e68946dfe72d338529) --- .../ProcessKotlinWebsiteJSON/populate_db.py | 77 ++++++++++++++++++ .../test_database_version.py | 78 +++++++++++++++++++ docdb-studio/README.md | 8 ++ docdb-studio/SCHEMA.md | 8 ++ docdb-studio/docdb_studio.py | 50 +++++++++++- .../tests/test_compression_dictionary.py | 69 ++++++++++++++++ 6 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py index ce4b28e3..abe15328 100644 --- a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py @@ -200,6 +200,75 @@ # 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. +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 a row was added. + + Appends only when the declared version *changes*. The table is a log of what + the format became, not of who ran what: a row per invocation would bury the + two or three entries that matter under hundreds of identical ones, and the + app reads only the last row anyway. + + A last row that differs in either direction gets an append, including a lower + version than it already declares. That is deliberate -- rebuilding from an + older pipeline genuinely is a downgrade, and "the row inserted last wins" is + the convention the app's reader implements, so recording it is what keeps the + file honest about what it now contains. + """ + conn.execute(VERSION_TABLE_SQL) + declared = conn.execute( + "SELECT major, minor, patch FROM DocumentationDatabaseVersion ORDER BY rowid DESC LIMIT 1" + ).fetchone() + if declared is not None and tuple(declared) == DATABASE_FORMAT_VERSION: + return False + + major, minor, patch = DATABASE_FORMAT_VERSION + 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 +821,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..1b6688aa --- /dev/null +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py @@ -0,0 +1,78 @@ +#!/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, 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_adds_nothing(self): + """The table logs what the format became, not who ran what -- a row per + invocation would bury the entries that matter.""" + declare_database_version(self.conn) + self.assertFalse(declare_database_version(self.conn)) + self.assertEqual(len(self.declared()), 1) + + def test_an_older_declaration_is_superseded(self): + 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()], [(1, 4, 0), DATABASE_FORMAT_VERSION]) + + def test_a_downgrade_is_recorded_rather_than_hidden(self): + """Rebuilding from an older pipeline really is a downgrade, and the app + reads the row inserted last -- so it has to be written.""" + self.conn.execute( + """ + 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) + """ + ) + 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(self.declared()[-1][:3], 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..8b9886ea 100644 --- a/docdb-studio/SCHEMA.md +++ b/docdb-studio/SCHEMA.md @@ -54,3 +54,11 @@ 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 +); diff --git a/docdb-studio/docdb_studio.py b/docdb-studio/docdb_studio.py index 3a9991d9..33985d48 100644 --- a/docdb-studio/docdb_studio.py +++ b/docdb-studio/docdb_studio.py @@ -164,6 +164,48 @@ 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. + + Reads the row inserted *last*, not the highest: the table is an append-only + log, so a rebuild from an older pipeline is a downgrade and has to read as + one. Same rule as the app's DatabaseVersionResolver. + """ + 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 +4972,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..a53b4f8c 100644 --- a/docdb-studio/tests/test_compression_dictionary.py +++ b/docdb-studio/tests/test_compression_dictionary.py @@ -280,3 +280,72 @@ 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( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + "VALUES (?, ?, ?, 'test', 'test')", + (major, minor, patch), + ) + 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_the_declared_major_is_the_row_inserted_last() -> None: + """The table is an append-only log, so a rebuild from an older pipeline is a + downgrade and has to read as one -- the same rule the app's resolver uses.""" + db = _make_db() + try: + _set_version(db, 3) + _set_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) From a332e08ddfdaf92f413c3fcb7004b5bd20011b85 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 21:55:48 -0700 Subject: [PATCH 2/2] ADFA-5220: Hold exactly one row in DocumentationDatabaseVersion The table records the format version the file *is*, not a history of what it has been, so declare_database_version replaces its row instead of appending, and collapses a database that somehow accumulated several back to one. The replacement stays unconditional in either direction, including a version lower than the file already declares: rebuilding from an older pipeline really does produce an older format, and the row has to say what the file contains now. Both readers keep their ORDER BY rowid DESC, now as a defence rather than a model -- if a file ever breaks the contract, the version read stays deterministic instead of depending on what SQLite returns first. Co-Authored-By: Claude Opus 5 (cherry picked from commit 1eca4253e9fca0fce8bf918f66becdbd9156f2be) --- .../ProcessKotlinWebsiteJSON/populate_db.py | 33 +++++++------ .../test_database_version.py | 49 ++++++++++++------- docdb-studio/SCHEMA.md | 5 ++ docdb-studio/docdb_studio.py | 8 +-- .../tests/test_compression_dictionary.py | 22 +++++++-- 5 files changed, 76 insertions(+), 41 deletions(-) diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py index abe15328..c7a8ebe0 100644 --- a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/populate_db.py @@ -213,6 +213,9 @@ # 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 @@ -242,27 +245,25 @@ def declare_database_version( comment: str = "Content rows compressed against CompressionDictionary", ) -> bool: """Records DATABASE_FORMAT_VERSION in DocumentationDatabaseVersion, creating - the table if this database predates it. Returns whether a row was added. - - Appends only when the declared version *changes*. The table is a log of what - the format became, not of who ran what: a row per invocation would bury the - two or three entries that matter under hundreds of identical ones, and the - app reads only the last row anyway. - - A last row that differs in either direction gets an append, including a lower - version than it already declares. That is deliberate -- rebuilding from an - older pipeline genuinely is a downgrade, and "the row inserted last wins" is - the convention the app's reader implements, so recording it is what keeps the - file honest about what it now contains. + 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 ORDER BY rowid DESC LIMIT 1" - ).fetchone() - if declared is not None and tuple(declared) == DATABASE_FORMAT_VERSION: + 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), diff --git a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py index 1b6688aa..774c69d8 100644 --- a/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py +++ b/ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/test_database_version.py @@ -8,7 +8,11 @@ import unittest from pathlib import Path -from populate_db import DATABASE_FORMAT_VERSION, declare_database_version +from populate_db import ( + DATABASE_FORMAT_VERSION, + VERSION_TABLE_SQL, + declare_database_version, +) class DeclareDatabaseVersionTest(unittest.TestCase): @@ -33,38 +37,47 @@ def test_creates_the_table_and_declares_the_version(self): major, minor, patch = DATABASE_FORMAT_VERSION self.assertEqual(self.declared(), [(major, minor, patch, "populate_db.py")]) - def test_a_second_run_adds_nothing(self): - """The table logs what the format became, not who ran what -- a row per - invocation would bury the entries that matter.""" + 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_superseded(self): + 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()], [(1, 4, 0), DATABASE_FORMAT_VERSION]) + self.assertEqual([row[:3] for row in self.declared()], [DATABASE_FORMAT_VERSION]) - def test_a_downgrade_is_recorded_rather_than_hidden(self): - """Rebuilding from an older pipeline really is a downgrade, and the app - reads the row inserted last -- so it has to be written.""" - self.conn.execute( - """ - 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) - """ - ) + 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(self.declared()[-1][:3], DATABASE_FORMAT_VERSION) + 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 diff --git a/docdb-studio/SCHEMA.md b/docdb-studio/SCHEMA.md index 8b9886ea..e9c9cc10 100644 --- a/docdb-studio/SCHEMA.md +++ b/docdb-studio/SCHEMA.md @@ -62,3 +62,8 @@ CREATE TABLE DocumentationDatabaseVersion ( 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 33985d48..a62c2c8f 100644 --- a/docdb-studio/docdb_studio.py +++ b/docdb-studio/docdb_studio.py @@ -179,9 +179,11 @@ 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. - Reads the row inserted *last*, not the highest: the table is an append-only - log, so a rebuild from an older pipeline is a downgrade and has to read as - one. Same rule as the app's DatabaseVersionResolver. + 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( diff --git a/docdb-studio/tests/test_compression_dictionary.py b/docdb-studio/tests/test_compression_dictionary.py index a53b4f8c..4a8e68fc 100644 --- a/docdb-studio/tests/test_compression_dictionary.py +++ b/docdb-studio/tests/test_compression_dictionary.py @@ -295,6 +295,7 @@ def _set_version(db: Path, major: int, minor: int = 0, patch: int = 0) -> None: changeTime TIMESTAMP DEFAULT CURRENT_TIMESTAMP) """ ) + conn.execute("DELETE FROM DocumentationDatabaseVersion") conn.execute( "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " "VALUES (?, ?, ?, 'test', 'test')", @@ -303,6 +304,18 @@ def _set_version(db: Path, major: int, minor: int = 0, patch: int = 0) -> None: 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: @@ -311,13 +324,14 @@ def test_a_database_with_no_version_table_reads_as_unversioned() -> None: db.unlink(missing_ok=True) -def test_the_declared_major_is_the_row_inserted_last() -> None: - """The table is an append-only log, so a rebuild from an older pipeline is a - downgrade and has to read as one -- the same rule the app's resolver uses.""" +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) - _set_version(db, 2) + _append_stray_version(db, 2) assert docdb_studio.database_major_version(db) == 2 finally: db.unlink(missing_ok=True)