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 0515bf05fb..5986d2841b 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -68,6 +68,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 1761c0e4e5..0ee2f4f96d 100644 --- a/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt +++ b/common/src/androidTest/java/com/itsaky/androidide/utils/DatabaseVersionResolverTest.kt @@ -86,24 +86,46 @@ class DatabaseVersionResolverTest { assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } - // The table is an append-only log, so the row inserted last is the current version... + // 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 majorVersionIsTheLastRowInserted() { + fun majorVersionIsTheRowWrittenLast_whenTheLastRowIsHigher() { 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. + // ...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 majorVersionFollowsADowngrade() { + fun majorVersionIsTheRowWrittenLast_whenTheLastRowIsADowngrade() { createVersionTable() insertVersion(3, 0, 0) insertVersion(2, 0, 0) assertEquals(2, DatabaseVersionResolver.resolveMajorVersion(db)) } + // 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 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 (?, ?, ?, ?, ?)", + arrayOf(null, 0, 0, "test", "test"), + ) + + assertNull(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 225ffd6a39..2d6250ebff 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 @@ -27,13 +27,14 @@ 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. + // 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 major, (SELECT COUNT(*) FROM DocumentationDatabaseVersion) FROM DocumentationDatabaseVersion - ORDER BY rowid DESC + ORDER BY changeTime DESC, rowid DESC LIMIT 1 """ @@ -44,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 -> @@ -63,31 +69,23 @@ 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 } } /** - * 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. + * 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() } @@ -95,10 +93,45 @@ 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()) { + 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.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)) { + // 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) + } } } + /** + * 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?, @@ -108,6 +141,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()) + } + } +} diff --git a/docs/documentation-database.md b/docs/documentation-database.md index 76c6b8a3ed..640bf530d5 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**: 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`).