From 33bbe139192be67d96eee39ade6f3a243c11d9a2 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 21:57:38 -0700 Subject: [PATCH 1/7] ADFA-5220: Correct the version table to a single row, not an append-only log DocumentationDatabaseVersion holds exactly one row -- the format version the database *is*, not a history of what it has been. The comments and the doc bullet described an append-only log, which was my reading of the ticket's INSERT-based update example and is wrong. resolveMajorVersion keeps ORDER BY rowid DESC, now stated as a defence rather than a model: a file that breaks the one-row contract still reads deterministically, and a downgrade still reads as a downgrade where MAX(major) would report the highest version ever declared. The two tests that asserted last-row-wins across several rows collapse into one that says what that ordering is actually for. The writer side is OfflineDocumentationTools#29. Co-Authored-By: Claude Opus 5 --- .../utils/DatabaseVersionResolverTest.kt | 16 +++++----------- .../androidide/utils/DatabaseVersionResolver.kt | 8 +++++--- docs/documentation-database.md | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt index 1761c0e4e5..5e46a197e2 100644 --- a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt +++ b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt @@ -86,18 +86,12 @@ class DatabaseVersionResolverTest { assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } - // The table is an append-only log, so the row inserted last is the current version... + // The table is meant to hold one row. A database that breaks that has to still read + // deterministically -- the row written last -- rather than whichever one SQLite returns first, + // and a downgrade has to read as a downgrade where MAX(major) would report the highest version + // the file ever declared. @Test - fun majorVersionIsTheLastRowInserted() { - createVersionTable() - insertVersion(2, 0, 0) - insertVersion(3, 1, 4) - assertEquals(3, DatabaseVersionResolver.resolveMajorVersion(db)) - } - - // ...including when that row is a downgrade, which MAX(major) would read as still current. - @Test - fun majorVersionFollowsADowngrade() { + fun majorVersionIsTheRowWrittenLast_whenADatabaseCarriesSeveral() { createVersionTable() insertVersion(3, 0, 0) insertVersion(2, 0, 0) diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index 225ffd6a39..b6344f08ea 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -27,9 +27,11 @@ object DatabaseVersionResolver { WHERE type = 'table' AND name = 'DocumentationDatabaseVersion' """ - // The table is an append-only log -- ADFA-5220 records each change as another INSERT -- so the - // current version is the row inserted last, not the highest one ever recorded: rebuilding from - // an older content set is a downgrade and has to read as one. + // The table holds exactly one row: the version the database *is*, not a history of what it has + // been (ADFA-5220), and the pipeline replaces that row rather than appending. So the ORDER BY + // here is a defence, not a model -- if a database ever turns up carrying several rows, this + // reads the one written last instead of whichever SQLite happens to return, and a downgrade + // still reads as a downgrade where MAX(major) would report the highest version ever recorded. private const val QUERY_MAJOR_VERSION = """ SELECT major FROM DocumentationDatabaseVersion diff --git a/docs/documentation-database.md b/docs/documentation-database.md index 3055c955f0..a8cf16f432 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -62,7 +62,7 @@ CREATE TABLE Tooltips ( ### Supporting tables -- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. Append-only: each change is another `INSERT`, so the **row inserted last** is the current version, not the highest one ever recorded — a rebuild from an older content set is a downgrade and has to read as one (`DatabaseVersionResolver.resolveMajorVersion`, which returns null for a database predating the table). `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. +- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**, holding the version the file *is* rather than a history of what it has been: `populate_db.py` replaces that row rather than appending, and collapses a file that somehow accumulated several back to one. Nothing in the DDL enforces the rule, so both readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) order by `rowid DESC` as a defence — a file that breaks the contract still reads deterministically, and a rebuild from an older content set reads as the downgrade it is instead of the highest version ever declared. `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. - **`CompressionDictionary(id, data)`** — single-row table (`id INTEGER PRIMARY KEY CHECK (id = 1)`) holding the raw Brotli dictionary every ADFA-5153-migrated `compression = 'brotli'` `Content` row is compressed against. Trained once, from a representative sample across the whole `Content` table, by `OfflineDocumentationTools`' `migrate_content_to_dictionary_brotli.py` / `populate_db.py` (never retrained after that — a dictionary-compressed row is only decodable against the exact dictionary it was compressed with, so replacing it would silently orphan every already-migrated row). Shipping the dictionary inside `documentation.db` itself, rather than as a separate bundled asset, keeps it version-locked to the content compressed against it. `WebServer` loads it lazily -- not merely from starting the server or swapping databases, but on the first content fetch that needs it after `database` changes, and only when `DocumentationDatabaseVersion` declares `MAJOR >= 2` (see above) -- and caches it from then on, reloading again only on the next database change (a swap can bring in a database with a different dictionary or none, so it can't stay cached across one). Per row, it tries decoding with the dictionary attached first via brotli4j's `attachDictionary`, falling back to a plain decode on failure — needed both for a database predating this migration (no `CompressionDictionary` table at all) and for plugin-contributed rows within an otherwise-migrated database (see `PluginDocumentationManager` below). - **`Templates(id, name, content)`** — Pebble template source, keyed by id (and by `name` for well-known templates like `bookshelf`). Referenced by `Content.templateId`. - **`Bookshelf(contentID, bookCategoryID, title, description)`** / **`BookCategories(id, category, description)`** — the Dynamic Bookshelf: one row per "book" (PDF or similar), linked to its Tier 3 page via `contentID` -> `Content.id`. Two DB triggers keep `Bookshelf` in sync when a PDF row is inserted/deleted from `Content`; `title`/`description` don't come from those triggers and must be set by hand. Non-PDF books need a separate ingestion path (plugin-provided, e.g. via `PluginDocumentationManager`). From 0e505a0a21750d115f712566292d0d724195fd64 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 11:47:57 -0700 Subject: [PATCH 2/7] ADFA-5220: Report a version table that holds more than one row The reader tolerates several rows on purpose -- ordering by rowid keeps the answer deterministic -- but it did so silently, so a database built by something that appended instead of replacing looked identical to a correct one. The count now rides along with the version in the same query, and more than one row is logged with the major actually used. The doc bullet stated the one-row rule twice over nine lines; it now says it once. Verified: the query returns (last major, total count) against sqlite directly, for one row, several rows, and none. The instrumented assertion for the single-row path is added but not executed -- no device is attached at the moment. Co-Authored-By: Claude Opus 5 --- .../utils/DatabaseVersionResolverTest.kt | 9 +++++++++ .../utils/DatabaseVersionResolver.kt | 19 +++++++++++++++++-- docs/documentation-database.md | 2 +- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt index 5e46a197e2..7900c0edb9 100644 --- a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt +++ b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt @@ -98,6 +98,15 @@ class DatabaseVersionResolverTest { assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } + // The count query rides along with the version, so a one-row file must still read correctly -- + // the case that matters most, and the one a malformed-file check could most easily break. + @Test + fun majorVersionIsReadFromASingleRowUnchanged() { + createVersionTable() + insertVersion(4, 1, 2) + assertEquals(4, DatabaseVersionResolver.resolveMajorVersion(db)) + } + @Test fun returnsWholedbRow_whenPresent() { createTable() diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index b6344f08ea..290b41fde6 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -32,8 +32,11 @@ object DatabaseVersionResolver { // here is a defence, not a model -- if a database ever turns up carrying several rows, this // reads the one written last instead of whichever SQLite happens to return, and a downgrade // still reads as a downgrade where MAX(major) would report the highest version ever recorded. + // The row count comes back with the version so a file that breaks the one-row contract can be + // reported rather than silently papered over: the ORDER BY makes the answer deterministic, but a + // database carrying several rows is malformed and whoever produced it should hear about it. private const val QUERY_MAJOR_VERSION = """ - SELECT major + SELECT major, (SELECT COUNT(*) FROM DocumentationDatabaseVersion) FROM DocumentationDatabaseVersion ORDER BY rowid DESC LIMIT 1 @@ -97,7 +100,19 @@ object DatabaseVersionResolver { return null } return db.rawQuery(QUERY_MAJOR_VERSION, arrayOf()).use { cursor -> - if (cursor.moveToFirst() && !cursor.isNull(0)) cursor.getInt(0) else null + if (!cursor.moveToFirst() || cursor.isNull(0)) { + return@use null + } + val rows = cursor.getInt(1) + if (rows > 1) { + Log.w( + TAG, + "DocumentationDatabaseVersion holds $rows rows; it is meant to hold one. " + + "Using the row written last (major ${cursor.getInt(0)}); the database was built by " + + "something that appended instead of replacing.", + ) + } + cursor.getInt(0) } } diff --git a/docs/documentation-database.md b/docs/documentation-database.md index a8cf16f432..e35a918716 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -62,7 +62,7 @@ CREATE TABLE Tooltips ( ### Supporting tables -- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**, holding the version the file *is* rather than a history of what it has been: `populate_db.py` replaces that row rather than appending, and collapses a file that somehow accumulated several back to one. Nothing in the DDL enforces the rule, so both readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) order by `rowid DESC` as a defence — a file that breaks the contract still reads deterministically, and a rebuild from an older content set reads as the downgrade it is instead of the highest version ever declared. `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. +- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**: the version the file *is*, which `populate_db.py` replaces rather than appends. Nothing in the DDL enforces that, so the readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) take the row with the highest `rowid` and log a warning if there is more than one; `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. - **`CompressionDictionary(id, data)`** — single-row table (`id INTEGER PRIMARY KEY CHECK (id = 1)`) holding the raw Brotli dictionary every ADFA-5153-migrated `compression = 'brotli'` `Content` row is compressed against. Trained once, from a representative sample across the whole `Content` table, by `OfflineDocumentationTools`' `migrate_content_to_dictionary_brotli.py` / `populate_db.py` (never retrained after that — a dictionary-compressed row is only decodable against the exact dictionary it was compressed with, so replacing it would silently orphan every already-migrated row). Shipping the dictionary inside `documentation.db` itself, rather than as a separate bundled asset, keeps it version-locked to the content compressed against it. `WebServer` loads it lazily -- not merely from starting the server or swapping databases, but on the first content fetch that needs it after `database` changes, and only when `DocumentationDatabaseVersion` declares `MAJOR >= 2` (see above) -- and caches it from then on, reloading again only on the next database change (a swap can bring in a database with a different dictionary or none, so it can't stay cached across one). Per row, it tries decoding with the dictionary attached first via brotli4j's `attachDictionary`, falling back to a plain decode on failure — needed both for a database predating this migration (no `CompressionDictionary` table at all) and for plugin-contributed rows within an otherwise-migrated database (see `PluginDocumentationManager` below). - **`Templates(id, name, content)`** — Pebble template source, keyed by id (and by `name` for well-known templates like `bookshelf`). Referenced by `Content.templateId`. - **`Bookshelf(contentID, bookCategoryID, title, description)`** / **`BookCategories(id, category, description)`** — the Dynamic Bookshelf: one row per "book" (PDF or similar), linked to its Tier 3 page via `contentID` -> `Content.id`. Two DB triggers keep `Bookshelf` in sync when a PDF row is inserted/deleted from `Content`; `title`/`description` don't come from those triggers and must be set by hand. Non-PDF books need a separate ingestion path (plugin-provided, e.g. via `PluginDocumentationManager`). From 278a141676ded313260e40ec46ddd333144a4046 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 16:18:34 -0700 Subject: [PATCH 3/7] ADFA-5220: Restore the test that pins the ordering, and warn on the worst case Review of this PR found that merging the two ordering tests deleted the only one that distinguished "highest rowid" from "lowest major": every remaining expectation happened to be the minimum major present, so MIN(major) would have passed the whole suite while the test named for the row written last proved nothing. Both directions are back -- the last row higher, and the last row a downgrade. The NULL check ran before the count was read, so a file that is both multi-row and ends in a NULL major returned null with nothing logged: the most malformed state there is, reported exactly like a database that has no version table. The count is read first now, and there is a test, which needs a table created without the shipped DDL's NOT NULL -- fitting, since this reader exists to defend against files another producer wrote. WebServerTest's cursor stub never answered getInt(1), so a relaxed mock returned a row count of 0 -- a state the production code has just excluded by getting a row back at all. It returns 1 now, so those tests exercise something reachable. The doc claimed docdb-studio logs a warning for a multi-row file. It does not; only this reader does. It also lost the reason highest-rowid beats MAX(major), which is the fact that stops someone simplifying the query later. Both fixed. Verified on device this time, not just compiled: 12 instrumented tests pass on a Galaxy Note 20 Ultra, and the warning appears three times in logcat -- once per multi-row case, the NULL-major one included. Co-Authored-By: Claude Opus 5 --- .../localWebServer/WebServerTest.kt | 4 ++ .../utils/DatabaseVersionResolverTest.kt | 41 ++++++++++++++----- .../utils/DatabaseVersionResolver.kt | 33 ++++++++------- docs/documentation-database.md | 2 +- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index e68b2e05e4..3fc9dd5072 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -77,6 +77,10 @@ class WebServerTest { every { moveToFirst() } returns true every { isNull(0) } returns false every { getInt(0) } returns major + // The row count the query carries. Left unstubbed, a relaxed mock answers 0 -- a + // state the production code has just excluded by getting a row back at all, so + // these tests would be exercising something that cannot happen. + every { getInt(1) } returns 1 } } } diff --git a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt index 7900c0edb9..c5d3d63cf6 100644 --- a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt +++ b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt @@ -86,25 +86,44 @@ class DatabaseVersionResolverTest { assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } - // The table is meant to hold one row. A database that breaks that has to still read - // deterministically -- the row written last -- rather than whichever one SQLite returns first, - // and a downgrade has to read as a downgrade where MAX(major) would report the highest version - // the file ever declared. + // Both directions, deliberately. Merging these into the downgrade case alone left a suite that + // MIN(major) would also have passed -- every expectation happened to be the lowest major present + // -- so nothing pinned the ordering the whole design rests on. @Test - fun majorVersionIsTheRowWrittenLast_whenADatabaseCarriesSeveral() { + fun majorVersionIsTheRowWrittenLast_whenTheLastRowIsHigher() { + createVersionTable() + insertVersion(2, 0, 0) + insertVersion(3, 1, 4) + assertEquals(3, DatabaseVersionResolver.resolveMajorVersion(db)) + } + + // ...and when it is lower, which MAX(major) would get wrong: a rebuild from an older content set + // is a downgrade and has to read as one. + @Test + fun majorVersionIsTheRowWrittenLast_whenTheLastRowIsADowngrade() { createVersionTable() insertVersion(3, 0, 0) insertVersion(2, 0, 0) assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } - // The count query rides along with the version, so a one-row file must still read correctly -- - // the case that matters most, and the one a malformed-file check could most easily break. + // Malformed twice over: several rows, and the last one has no major. The shipped DDL forbids that + // -- which is the point, since this reader defends against files another producer wrote -- so the + // table is created here without the NOT NULL. The count is read before the NULL check, so a file + // like this still warns instead of being reported as having no version table at all. @Test - fun majorVersionIsReadFromASingleRowUnchanged() { - createVersionTable() - insertVersion(4, 1, 2) - assertEquals(4, DatabaseVersionResolver.resolveMajorVersion(db)) + fun majorVersionIsNull_whenTheLastRowHasNoMajor() { + db.execSQL( + "CREATE TABLE DocumentationDatabaseVersion (" + + "major INT, minor INT, patch INT, who TEXT, comment TEXT, changeTime TIMESTAMP)", + ) + insertVersion(2, 0, 0) + db.execSQL( + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + + "VALUES (NULL, 0, 0, 'test', 'test')", + ) + + assertNull(DatabaseVersionResolver.resolveMajorVersion(db)) } @Test diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index 290b41fde6..e435416682 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -27,14 +27,10 @@ object DatabaseVersionResolver { WHERE type = 'table' AND name = 'DocumentationDatabaseVersion' """ - // The table holds exactly one row: the version the database *is*, not a history of what it has - // been (ADFA-5220), and the pipeline replaces that row rather than appending. So the ORDER BY - // here is a defence, not a model -- if a database ever turns up carrying several rows, this - // reads the one written last instead of whichever SQLite happens to return, and a downgrade - // still reads as a downgrade where MAX(major) would report the highest version ever recorded. - // The row count comes back with the version so a file that breaks the one-row contract can be - // reported rather than silently papered over: the ORDER BY makes the answer deterministic, but a - // database carrying several rows is malformed and whoever produced it should hear about it. + // One row by contract (ADFA-5220); the ORDER BY is the defence for a file that breaks it, and + // MAX(major) is the tempting wrong answer -- a rebuild from an older content set has to read as + // the downgrade it is. The count rides along so the breach can be reported rather than papered + // over. private const val QUERY_MAJOR_VERSION = """ SELECT major, (SELECT COUNT(*) FROM DocumentationDatabaseVersion) FROM DocumentationDatabaseVersion @@ -86,8 +82,13 @@ object DatabaseVersionResolver { /** * The MAJOR version [db] declares in `DocumentationDatabaseVersion` (ADFA-5220), or null when - * that table is absent or empty -- which is how every database built before it existed - * identifies itself. + * that table is absent, empty, or holds a NULL major -- the first of which is how every database + * built before it existed identifies itself. + * + * The table is contractually a single row. A file carrying several is accepted rather than + * rejected -- the row with the highest `rowid` wins, so the answer stays deterministic and a + * downgrade still reads as one -- and logs a warning, since this reader cannot repair the file + * and refusing to serve documentation over it would be a worse outcome than serving it. * * Deliberately does *not* catch exceptions, unlike [resolveDatabaseVersion]: callers cache the * answer for the lifetime of a database (see `WebServer.loadCompressionDictionary`), so a @@ -100,19 +101,21 @@ object DatabaseVersionResolver { return null } return db.rawQuery(QUERY_MAJOR_VERSION, arrayOf()).use { cursor -> - if (!cursor.moveToFirst() || cursor.isNull(0)) { + if (!cursor.moveToFirst()) { return@use null } + // Counted before the NULL check, not after: a file that is both multi-row *and* ends in a + // NULL major would otherwise return null with nothing logged -- the most malformed case + // there is, reported as if the table simply did not exist. val rows = cursor.getInt(1) if (rows > 1) { Log.w( TAG, - "DocumentationDatabaseVersion holds $rows rows; it is meant to hold one. " + - "Using the row written last (major ${cursor.getInt(0)}); the database was built by " + - "something that appended instead of replacing.", + "DocumentationDatabaseVersion holds $rows rows; it is meant to hold one. Using the row " + + "written last; the database was built by something that appended instead of replacing.", ) } - cursor.getInt(0) + if (cursor.isNull(0)) null else cursor.getInt(0) } } diff --git a/docs/documentation-database.md b/docs/documentation-database.md index e35a918716..d502ce6d49 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -62,7 +62,7 @@ CREATE TABLE Tooltips ( ### Supporting tables -- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**: the version the file *is*, which `populate_db.py` replaces rather than appends. Nothing in the DDL enforces that, so the readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) take the row with the highest `rowid` and log a warning if there is more than one; `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. +- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**: the version the file *is*, which `populate_db.py` replaces rather than appends. Nothing in the DDL enforces that, so both readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) take the row with the highest `rowid` rather than the highest `major` — a rebuild from an older content set is a downgrade and has to read as one, which `MAX(major)` would get wrong. The app's reader additionally logs a warning when it finds more than one row; docdb-studio's does not. `populate_db.py` collapses a file that somehow accumulated several back to one, and `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. - **`CompressionDictionary(id, data)`** — single-row table (`id INTEGER PRIMARY KEY CHECK (id = 1)`) holding the raw Brotli dictionary every ADFA-5153-migrated `compression = 'brotli'` `Content` row is compressed against. Trained once, from a representative sample across the whole `Content` table, by `OfflineDocumentationTools`' `migrate_content_to_dictionary_brotli.py` / `populate_db.py` (never retrained after that — a dictionary-compressed row is only decodable against the exact dictionary it was compressed with, so replacing it would silently orphan every already-migrated row). Shipping the dictionary inside `documentation.db` itself, rather than as a separate bundled asset, keeps it version-locked to the content compressed against it. `WebServer` loads it lazily -- not merely from starting the server or swapping databases, but on the first content fetch that needs it after `database` changes, and only when `DocumentationDatabaseVersion` declares `MAJOR >= 2` (see above) -- and caches it from then on, reloading again only on the next database change (a swap can bring in a database with a different dictionary or none, so it can't stay cached across one). Per row, it tries decoding with the dictionary attached first via brotli4j's `attachDictionary`, falling back to a plain decode on failure — needed both for a database predating this migration (no `CompressionDictionary` table at all) and for plugin-contributed rows within an otherwise-migrated database (see `PluginDocumentationManager` below). - **`Templates(id, name, content)`** — Pebble template source, keyed by id (and by `name` for well-known templates like `bookshelf`). Referenced by `Content.templateId`. - **`Bookshelf(contentID, bookCategoryID, title, description)`** / **`BookCategories(id, category, description)`** — the Dynamic Bookshelf: one row per "book" (PDF or similar), linked to its Tier 3 page via `contentID` -> `Content.id`. Two DB triggers keep `Bookshelf` in sync when a PDF row is inserted/deleted from `Content`; `title`/`description` don't come from those triggers and must be set by hand. Non-PDF books need a separate ingestion path (plugin-provided, e.g. via `PluginDocumentationManager`). From 3096bd99025725bfc833d76e0b7feadc902645b2 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 17:04:32 -0700 Subject: [PATCH 4/7] ADFA-5220: Log through SLF4J, and bind the fixture's values This file was one of two in common/utils still using android.util.Log where ten siblings use SLF4J, so the whole file moves rather than just the new warning -- a file mixing both would be worse than either. The duplicate-row warning takes a {} placeholder with rows as an argument. The malformed-table fixture built its INSERT by concatenating literals; the values are bound now, like every other insert in this test. Verified on device: 12 instrumented tests pass and the warning still reaches logcat three times through the SLF4J binding. Co-Authored-By: Claude Opus 5 --- .../utils/DatabaseVersionResolverTest.kt | 4 ++-- .../utils/DatabaseVersionResolver.kt | 21 ++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt index c5d3d63cf6..0ee2f4f96d 100644 --- a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt +++ b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt @@ -119,8 +119,8 @@ class DatabaseVersionResolverTest { ) insertVersion(2, 0, 0) db.execSQL( - "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) " + - "VALUES (NULL, 0, 0, 'test', 'test')", + "INSERT INTO DocumentationDatabaseVersion (major, minor, patch, who, comment) VALUES (?, ?, ?, ?, ?)", + arrayOf(null, 0, 0, "test", "test"), ) assertNull(DatabaseVersionResolver.resolveMajorVersion(db)) diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index e435416682..2aba6c2a25 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -1,12 +1,12 @@ package com.itsaky.androidide.utils import android.database.sqlite.SQLiteDatabase -import android.util.Log +import org.slf4j.LoggerFactory object DatabaseVersionResolver { const val VERSION_UNKNOWN = "Version Unknown" - private const val TAG = "DatabaseVersionResolver" + private val log = LoggerFactory.getLogger(DatabaseVersionResolver::class.java) private const val QUERY_WHOLEDB = """ SELECT changeTime, who @@ -64,18 +64,15 @@ object DatabaseVersionResolver { who = c.getString(2), documentationSet = c.getString(1), ) - Log.e( - TAG, - "Missing 'wholedb' record in LastChange table; falling back to $result", - ) + log.error("Missing 'wholedb' record in LastChange table; falling back to {}", result) return result } } - Log.e(TAG, "No versioning information available") + log.error("No versioning information available") VERSION_UNKNOWN } catch (e: Exception) { - Log.e(TAG, "No versioning information available", e) + log.error("No versioning information available", e) VERSION_UNKNOWN } } @@ -109,10 +106,10 @@ object DatabaseVersionResolver { // there is, reported as if the table simply did not exist. val rows = cursor.getInt(1) if (rows > 1) { - Log.w( - TAG, - "DocumentationDatabaseVersion holds $rows rows; it is meant to hold one. Using the row " + - "written last; the database was built by something that appended instead of replacing.", + log.warn( + "DocumentationDatabaseVersion holds {} rows; it is meant to hold one. Using the row written " + + "last; the database was built by something that appended instead of replacing.", + rows, ) } if (cursor.isNull(0)) null else cursor.getInt(0) From b69e1d24d5a8934d45445aef9b1e654c02539bcb Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 26 Aug 2026 17:45:01 -0700 Subject: [PATCH 5/7] ADFA-5220: Order the version row by change time, and cover the reader Review follow-ups on the single-version-row reader. ORDER BY rowid DESC picked "the row written last" only by accident: rowid is not insertion order, and SQLite is free to reuse the rowid of a deleted row. The table carries a changeTime column that records exactly what the comment claims to want, so order by that and let rowid break ties. On a downgrade -- major 3 written, then 2 -- the old ordering could hand back 3 and attach the shared dictionary to content that is plain brotli. A NULL major now logs. It still reads as "no declared version", because that is the answer the caller is built to handle, but it and a database predating the table are no longer indistinguishable in the log: one is an old file behaving correctly, the other is a malformed one silently losing dictionary decoding. formatVersion returned "" when changeTime, set and who were all blank, which callers stored and displayed as a stamp. Return VERSION_UNKNOWN. The existing tests are in common/src/androidTest, which no workflow runs -- CI assembles :app:assembleV8DebugAndroidTest and runs two named app classes. The branch logic now has JVM tests that execute. Four of them pin behaviour that was already correct but unproven; the ordering test fails against the previous query. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M4sTwYg47aK8VB9kRKZicU --- .../utils/DatabaseVersionResolver.kt | 22 +++- .../DatabaseVersionResolverBranchTest.kt | 104 ++++++++++++++++++ 2 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 common/src/test/java/com/itsaky/androidide/utils/DatabaseVersionResolverBranchTest.kt diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index 2aba6c2a25..466aba5723 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -34,7 +34,7 @@ object DatabaseVersionResolver { private const val QUERY_MAJOR_VERSION = """ SELECT major, (SELECT COUNT(*) FROM DocumentationDatabaseVersion) FROM DocumentationDatabaseVersion - ORDER BY rowid DESC + ORDER BY changeTime DESC, rowid DESC LIMIT 1 """ @@ -112,7 +112,20 @@ object DatabaseVersionResolver { rows, ) } - if (cursor.isNull(0)) null else cursor.getInt(0) + if (cursor.isNull(0)) { + // Logged, because the caller cannot tell this apart from the answer it gets for a + // database predating the table: both are null, and WebServer reports "version none" and + // skips the dictionary either way. For a real pre-ADFA-5220 file that is correct; for + // this one it silently disables dictionary decoding on content that needs it, which is + // the worse of the two contract breaches this reader defends against. + log.warn( + "DocumentationDatabaseVersion's newest row has a NULL major; treating the database as " + + "declaring no version, which disables dictionary decoding.", + ) + null + } else { + cursor.getInt(0) + } } } @@ -125,6 +138,9 @@ object DatabaseVersionResolver { if (!changeTime.isNullOrBlank()) parts += changeTime if (!documentationSet.isNullOrBlank()) parts += "($documentationSet)" if (!who.isNullOrBlank()) parts += who - return parts.joinToString(separator = " ") + // ifEmpty: a row whose changeTime, set and who are all null or blank produced "", which callers + // then stored and logged as a stamp ("Database last change: ."). Nothing usable is the same + // answer as no row at all. + return parts.joinToString(separator = " ").ifEmpty { VERSION_UNKNOWN } } } diff --git a/common/src/test/java/com/itsaky/androidide/utils/DatabaseVersionResolverBranchTest.kt b/common/src/test/java/com/itsaky/androidide/utils/DatabaseVersionResolverBranchTest.kt new file mode 100644 index 0000000000..f2b584a3d2 --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/utils/DatabaseVersionResolverBranchTest.kt @@ -0,0 +1,104 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import com.google.common.truth.Truth.assertThat +import io.mockk.every +import io.mockk.mockk +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Test + +/** + * The branch logic of [DatabaseVersionResolver], as JVM tests that actually run. + * + * The existing coverage lives in `common/src/androidTest`, which no workflow executes -- CI only + * assembles `:app:assembleV8DebugAndroidTest` and runs two named app classes on Test Lab -- so the + * `rows > 1` warning and the NULL-major path had no evidence behind them beyond a manual logcat + * read. These pin the decisions the resolver makes about a malformed table; the SQL ordering itself + * still belongs in the instrumented file, against a real SQLite. + */ +class DatabaseVersionResolverBranchTest { + @After + fun tearDown() { + unmockkAll() + } + + private fun database( + major: Int?, + rows: Int, + tableExists: Boolean = true, + ): SQLiteDatabase { + val existsCursor = mockk(relaxed = true) { every { moveToFirst() } returns tableExists } + val versionCursor = + mockk(relaxed = true) { + every { moveToFirst() } returns true + every { isNull(0) } returns (major == null) + every { getInt(0) } returns (major ?: 0) + every { getInt(1) } returns rows + } + return mockk(relaxed = true) { + every { rawQuery(match { it.contains("sqlite_master") }, any()) } returns existsCursor + every { + rawQuery( + match { it.contains("DocumentationDatabaseVersion") && !it.contains("sqlite_master") }, + any(), + ) + } returns versionCursor + } + } + + @Test + fun `the newest row wins, and several rows are still answered`() { + assertThat(DatabaseVersionResolver.resolveMajorVersion(database(major = 2, rows = 3))).isEqualTo(2) + } + + // A NULL major is indistinguishable to the caller from "no version table": both are null, and + // WebServer reports "version none" and skips the dictionary either way. For a genuinely old + // database that is right; for this one it disables dictionary decoding on content that needs it. + @Test + fun `a NULL major reads as no declared version`() { + assertThat(DatabaseVersionResolver.resolveMajorVersion(database(major = null, rows = 1))).isNull() + } + + @Test + fun `a NULL major in a multi-row table still reads as no declared version`() { + assertThat(DatabaseVersionResolver.resolveMajorVersion(database(major = null, rows = 4))).isNull() + } + + @Test + fun `an absent table reads as no declared version`() { + assertThat( + DatabaseVersionResolver.resolveMajorVersion(database(major = 2, rows = 1, tableExists = false)), + ).isNull() + } + + // The ordering rule is a cross-repo contract -- docdb-studio reads the same table the same way -- + // so the column it orders by is worth pinning even from this side. + @Test + fun `the newest row is chosen by change time, not by rowid alone`() { + val db = database(major = 2, rows = 2) + DatabaseVersionResolver.resolveMajorVersion(db) + + io.mockk.verify { + db.rawQuery(match { it.contains("ORDER BY changeTime DESC") && it.contains("rowid DESC") }, any()) + } + } +} From 38ce331cf96ead6fe05be935eda472cae11146b5 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:49:10 +0000 Subject: [PATCH 6/7] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`tas?= =?UTF-8?q?k/ADFA-5220-single-version-row`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @davidschachterADFA. The following files were modified: * `common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt` These files were ignored: * `app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt` * `common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt` * `common/src/test/java/com/itsaky/androidide/utils/DatabaseVersionResolverBranchTest.kt` These file types are not supported: * `docs/documentation-database.md` --- .../utils/DatabaseVersionResolver.kt | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index 466aba5723..2d6250ebff 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -45,6 +45,11 @@ object DatabaseVersionResolver { LIMIT 1 """ + /** + * Resolves the database version from the available change history. + * + * @return The formatted database version, or `VERSION_UNKNOWN` when version information is unavailable or an error occurs. + */ fun resolveDatabaseVersion(db: SQLiteDatabase): String { return try { db.rawQuery(QUERY_WHOLEDB, arrayOf()).use { c -> @@ -78,19 +83,9 @@ object DatabaseVersionResolver { } /** - * The MAJOR version [db] declares in `DocumentationDatabaseVersion` (ADFA-5220), or null when - * that table is absent, empty, or holds a NULL major -- the first of which is how every database - * built before it existed identifies itself. - * - * The table is contractually a single row. A file carrying several is accepted rather than - * rejected -- the row with the highest `rowid` wins, so the answer stays deterministic and a - * downgrade still reads as one -- and logs a warning, since this reader cannot repair the file - * and refusing to serve documentation over it would be a worse outcome than serving it. + * Resolves the database's declared major version. * - * Deliberately does *not* catch exceptions, unlike [resolveDatabaseVersion]: callers cache the - * answer for the lifetime of a database (see `WebServer.loadCompressionDictionary`), so a - * transient `SQLiteException` has to stay distinguishable from a definitive "no version table", - * or one hiccup would pin the database at unversioned until it is swapped. + * @return The newest declared major version, or `null` when the version table is absent, empty, or its newest row has a `NULL` major value. */ fun resolveMajorVersion(db: SQLiteDatabase): Int? { val tableExists = db.rawQuery(QUERY_VERSION_TABLE_EXISTS, arrayOf()).use { it.moveToFirst() } @@ -129,6 +124,14 @@ object DatabaseVersionResolver { } } + /** + * Formats database change metadata into a readable version string. + * + * @param changeTime The recorded change timestamp. + * @param who The person or process associated with the change. + * @param documentationSet The documentation set associated with the change. + * @return The combined version details, or [VERSION_UNKNOWN] when no details are available. + */ private fun formatVersion( changeTime: String?, who: String?, From 2c4a5b08cec21c12dd84a8eb4c384306b1566505 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 15:48:26 +0000 Subject: [PATCH 7/7] ADFA-5220: Address review: KDoc selection rule, doc wording, test helper dedupe - Restore resolveMajorVersion's detailed KDoc (the docstring bot had replaced it with a generic summary) and correct the selection rule it states: the greatest changeTime wins, rowid only breaks ties -- matching the query. - Fix the same stale "highest rowid" wording in docs/documentation-database.md. - WebServerTest: sendRawGetRequestAndAwaitClose now delegates to sendRawGetRequest instead of duplicating the socket setup, and the shared helper documents that plaintext HTTP is intentional -- WebServer is a loopback-only plaintext server, tested as shipped. --- .../localWebServer/WebServerTest.kt | 22 ++++++++----------- .../utils/DatabaseVersionResolver.kt | 15 +++++++++++-- docs/documentation-database.md | 2 +- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index 5986d2841b..8c387cfb9d 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -419,7 +419,10 @@ class WebServerTest { } } - // Same as sendRawGetRequestAndAwaitClose, but hands back what the server actually wrote. + // Sends a bare GET over a raw socket and hands back everything the server wrote, reading + // until the server closes the connection (every response sends "Connection: close"). + // Plaintext HTTP is intentional and stays on this machine: WebServer is a loopback-only + // plaintext server, and these tests exercise it as shipped. private fun sendRawGetRequest( port: Int, path: String, @@ -434,22 +437,15 @@ class WebServerTest { socket.getInputStream().readBytes().toString(Charsets.ISO_8859_1) } - // Blocks until the server closes the connection (every response sends "Connection: close"), - // so by the time this returns the server has fully finished processing this one request -- - // making repeated calls a reliable way to serialize several full request/response cycles. + // Discards the response; because sendRawGetRequest reads until the server closes the + // connection, by the time this returns the server has fully finished processing this one + // request -- making repeated calls a reliable way to serialize several full request/response + // cycles. private fun sendRawGetRequestAndAwaitClose( port: Int, path: String, ) { - Socket().use { socket -> - socket.connect(InetSocketAddress("localhost", port), 2_000) - socket.soTimeout = 2_000 - socket.getOutputStream().apply { - write("GET $path HTTP/1.1\r\n\r\n".toByteArray(Charsets.ISO_8859_1)) - flush() - } - socket.getInputStream().readBytes() - } + sendRawGetRequest(port, path) } // Polls by attempting an actual TCP connect rather than sleeping a fixed diff --git a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt index 2d6250ebff..fa88f63a7d 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/DatabaseVersionResolver.kt @@ -83,9 +83,20 @@ object DatabaseVersionResolver { } /** - * Resolves the database's declared major version. + * The MAJOR version [db] declares in `DocumentationDatabaseVersion` (ADFA-5220), or null when + * that table is absent, empty, or holds a NULL major -- the first of which is how every database + * built before it existed identifies itself. * - * @return The newest declared major version, or `null` when the version table is absent, empty, or its newest row has a `NULL` major value. + * The table is contractually a single row. A file carrying several is accepted rather than + * rejected -- the row with the greatest `changeTime` wins, `rowid` breaking ties, so the answer + * stays deterministic and a downgrade still reads as one -- and logs a warning, since this + * reader cannot repair the file and refusing to serve documentation over it would be a worse + * outcome than serving it. + * + * Deliberately does *not* catch exceptions, unlike [resolveDatabaseVersion]: callers cache the + * answer for the lifetime of a database (see `WebServer.loadCompressionDictionary`), so a + * transient `SQLiteException` has to stay distinguishable from a definitive "no version table", + * or one hiccup would pin the database at unversioned until it is swapped. */ fun resolveMajorVersion(db: SQLiteDatabase): Int? { val tableExists = db.rawQuery(QUERY_VERSION_TABLE_EXISTS, arrayOf()).use { it.moveToFirst() } diff --git a/docs/documentation-database.md b/docs/documentation-database.md index 640bf530d5..89f39a0d8b 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -62,7 +62,7 @@ CREATE TABLE Tooltips ( ### Supporting tables -- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**: the version the file *is*, which `populate_db.py` replaces rather than appends. Nothing in the DDL enforces that, so both readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) take the row with the highest `rowid` rather than the highest `major` — a rebuild from an older content set is a downgrade and has to read as one, which `MAX(major)` would get wrong. The app's reader additionally logs a warning when it finds more than one row; docdb-studio's does not. `populate_db.py` collapses a file that somehow accumulated several back to one, and `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. +- **`DocumentationDatabaseVersion(major, minor, patch, who, comment, changeTime)`** — the database's own semver (ADFA-5220), replacing the heuristics that used to infer the format from which tables happened to exist. **Exactly one row**: the version the file *is*, which `populate_db.py` replaces rather than appends. Nothing in the DDL enforces that, so both readers (`DatabaseVersionResolver.resolveMajorVersion` here, `database_major_version` in docdb-studio) take the newest row rather than the one with the highest `major` (the app orders by `changeTime DESC, rowid DESC`, so the greatest `changeTime` wins and `rowid` only breaks ties) — a rebuild from an older content set is a downgrade and has to read as one, which `MAX(major)` would get wrong. The app's reader additionally logs a warning when it finds more than one row; docdb-studio's does not. `populate_db.py` collapses a file that somehow accumulated several back to one, and `resolveMajorVersion` returns null for a database predating the table. `MAJOR >= 2` is what tells the app its brotli `Content` rows are dictionary-compressed; below that, `WebServer` neither reads nor attaches `CompressionDictionary`. Gating on the declared version rather than on the table's presence matters in both directions: a database can carry the dictionary table while its content is still plain brotli (every row would then pay a failed dictionary decode before its plain one, on every request), and a migrated database that lost the table fails loudly instead of quietly. - **`CompressionDictionary(id, data)`** — single-row table (`id INTEGER PRIMARY KEY CHECK (id = 1)`) holding the raw Brotli dictionary every ADFA-5153-migrated `compression = 'brotli'` `Content` row is compressed against. Trained once, from a representative sample across the whole `Content` table, by `OfflineDocumentationTools`' `migrate_content_to_dictionary_brotli.py` / `populate_db.py` (never retrained after that — a dictionary-compressed row is only decodable against the exact dictionary it was compressed with, so replacing it would silently orphan every already-migrated row). Shipping the dictionary inside `documentation.db` itself, rather than as a separate bundled asset, keeps it version-locked to the content compressed against it. `WebServer` loads it lazily -- not merely from starting the server or swapping databases, but on the first content fetch that needs it after `database` changes, and only when `DocumentationDatabaseVersion` declares `MAJOR >= 2` (see above) -- and caches it from then on, reloading again only on the next database change (a swap can bring in a database with a different dictionary or none, so it can't stay cached across one). Per row, it tries decoding with the dictionary attached first via brotli4j's `attachDictionary`, falling back to a plain decode on failure — needed both for a database predating this migration (no `CompressionDictionary` table at all) and for plugin-contributed rows within an otherwise-migrated database (see `PluginDocumentationManager` below). - **`Templates(id, name, content)`** — Pebble template source, keyed by id (and by `name` for well-known templates like `bookshelf`). Referenced by `Content.templateId`. - **`Bookshelf(contentID, bookCategoryID, title, description)`** / **`BookCategories(id, category, description)`** — the Dynamic Bookshelf: one row per "book" (PDF or similar), linked to its Tier 3 page via `contentID` -> `Content.id`. Two DB triggers keep `Bookshelf` in sync when a PDF row is inserted/deleted from `Content`; `title`/`description` don't come from those triggers and must be set by hand. Non-PDF books need a separate ingestion path (plugin-provided, e.g. via `PluginDocumentationManager`).