From aa4ef00334b8d180eb75cc25a8a4352ab2e85db7 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Thu, 23 May 2024 22:00:19 +0000 Subject: [PATCH] feat: rebuild SQLite when migrations occur See [#436]. [#436]: https://github.com/digidem/comapeo-core/issues/436 --- src/constants.js | 3 ++ src/lib/drizzle-helpers.js | 48 +++++++++++++++++++++++++++++ src/mapeo-project.js | 60 ++++++++++++++++++++++++++----------- test-e2e/migration.js | 8 +++++ test/lib/drizzle-helpers.js | 30 +++++++++++++++++++ 5 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 src/lib/drizzle-helpers.js create mode 100644 test/lib/drizzle-helpers.js diff --git a/src/constants.js b/src/constants.js index a95af9df3..d93929c79 100644 --- a/src/constants.js +++ b/src/constants.js @@ -32,3 +32,6 @@ export const NAMESPACE_SCHEMAS = /** @type {const} */ ({ }) export const SUPPORTED_CONFIG_VERSION = 1 + +// WARNING: This value is persisted. Be careful when changing it. +export const DRIZZLE_MIGRATIONS_TABLE = '__drizzle_migrations' diff --git a/src/lib/drizzle-helpers.js b/src/lib/drizzle-helpers.js new file mode 100644 index 000000000..781bec238 --- /dev/null +++ b/src/lib/drizzle-helpers.js @@ -0,0 +1,48 @@ +import { sql } from 'drizzle-orm' +import { assert } from '../utils.js' +/** @import { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3' */ + +/** + * @param {unknown} queryResult + * @returns {number} + */ +const getNumberResult = (queryResult) => { + assert( + queryResult && + typeof queryResult === 'object' && + 'result' in queryResult && + typeof queryResult.result === 'number', + 'expected query to return proper result' + ) + return queryResult.result +} + +/** + * Get the number of rows in a table using `SELECT COUNT(*)`. + * Returns 0 if the table doesn't exist. + * + * @param {BetterSQLite3Database} db + * @param {string} tableName + * @returns {number} + */ +export const tableCountIfExists = (db, tableName) => + db.transaction((tx) => { + const existsQuery = sql` + SELECT EXISTS ( + SELECT 1 + FROM sqlite_master + WHERE type IS 'table' + AND name IS ${tableName} + ) AS result + ` + const existsResult = tx.get(existsQuery) + const exists = getNumberResult(existsResult) + if (!exists) return 0 + + const countQuery = sql` + SELECT COUNT(*) AS result + FROM ${sql.identifier(tableName)} + ` + const countResult = tx.get(countQuery) + return getNumberResult(countResult) + }) diff --git a/src/mapeo-project.js b/src/mapeo-project.js index 8bac0de79..b560d30d4 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -1,4 +1,5 @@ import path from 'path' +import * as fs from 'node:fs' import Database from 'better-sqlite3' import { decodeBlockPrefix, decode, parseVersionId } from '@comapeo/schema' import { drizzle } from 'drizzle-orm/better-sqlite3' @@ -6,7 +7,11 @@ import { migrate } from 'drizzle-orm/better-sqlite3/migrator' import { discoveryKey } from 'hypercore-crypto' import { TypedEmitter } from 'tiny-typed-emitter' -import { NAMESPACES, NAMESPACE_SCHEMAS } from './constants.js' +import { + NAMESPACES, + NAMESPACE_SCHEMAS, + DRIZZLE_MIGRATIONS_TABLE, +} from './constants.js' import { CoreManager } from './core-manager/index.js' import { DataStore } from './datastore/index.js' import { DataType, kCreateWithDocId } from './datatype/index.js' @@ -44,6 +49,7 @@ import { projectKeyToPublicId, valueOf, } from './utils.js' +import { tableCountIfExists } from './lib/drizzle-helpers.js' import { omit } from './lib/omit.js' import { MemberApi } from './member-api.js' import { SyncApi, kHandleDiscoveryKey } from './sync/sync-api.js' @@ -139,11 +145,42 @@ export class MapeoProject extends TypedEmitter { this.#isArchiveDevice = isArchiveDevice ///////// 1. Setup database + this.#sqlite = new Database(dbPath) const db = drizzle(this.#sqlite) - migrate(db, { migrationsFolder: projectMigrationsFolder }) + const migrationsBefore = tableCountIfExists(db, DRIZZLE_MIGRATIONS_TABLE) + migrate(db, { + migrationsFolder: projectMigrationsFolder, + migrationsTable: DRIZZLE_MIGRATIONS_TABLE, + }) + const migrationsAfter = tableCountIfExists(db, DRIZZLE_MIGRATIONS_TABLE) + const didMigrateDatabase = migrationsAfter !== migrationsBefore + + const indexedTables = [ + observationTable, + trackTable, + presetTable, + fieldTable, + coreOwnershipTable, + roleTable, + deviceInfoTable, + iconTable, + translationTable, + remoteDetectionAlertTable, + ] + + ///////// 2. Wipe data if we need to re-index + + if (didMigrateDatabase) { + fs.rmSync(INDEXER_STORAGE_FOLDER_NAME, { + force: true, + recursive: true, + maxRetries: 10, + }) + for (const table of indexedTables) db.delete(table).run() + } - ///////// 2. Setup random-access-storage functions + ///////// 3. Setup random-access-storage functions /** @type {ConstructorParameters[0]['storage']} */ const coreManagerStorage = (name) => @@ -153,7 +190,7 @@ export class MapeoProject extends TypedEmitter { const indexerStorage = (name) => coreStorage(path.join(INDEXER_STORAGE_FOLDER_NAME, name)) - ///////// 3. Create instances + ///////// 4. Create instances this.#coreManager = new CoreManager({ projectSecretKey, @@ -166,18 +203,7 @@ export class MapeoProject extends TypedEmitter { }) this.#indexWriter = new IndexWriter({ - tables: [ - observationTable, - trackTable, - presetTable, - fieldTable, - coreOwnershipTable, - roleTable, - deviceInfoTable, - iconTable, - translationTable, - remoteDetectionAlertTable, - ], + tables: indexedTables, sqlite: this.#sqlite, getWinner, mapDoc: (doc, version) => { @@ -362,7 +388,7 @@ export class MapeoProject extends TypedEmitter { dataType: this.#dataTypes.translation, }) - ///////// 4. Replicate local peers automatically + ///////// 5. Replicate local peers automatically // Replicate already connected local peers for (const peer of localPeers.peers) { diff --git a/test-e2e/migration.js b/test-e2e/migration.js index e62406c67..f138ed4d8 100644 --- a/test-e2e/migration.js +++ b/test-e2e/migration.js @@ -14,6 +14,14 @@ const projectMigrationsFolder = new URL('../drizzle/project', import.meta.url) const clientMigrationsFolder = new URL('../drizzle/client', import.meta.url) .pathname +test('migrations pick up values that were previously not understood', async () => { + // TODO(evanhahn) Write this test + // Receive an observation with a new field, `foo` + // Get the bytes, add it to the core, see that it's in SQLite without a `foo` column + // Do a migration where `foo` is added + // Reload the project and see that `foo` is now there +}) + test('migration of localDeviceInfo table', async (t) => { const comapeoCorePreMigrationUrl = await import.meta.resolve?.( '@comapeo/core2.0.1' diff --git a/test/lib/drizzle-helpers.js b/test/lib/drizzle-helpers.js new file mode 100644 index 000000000..1244292b8 --- /dev/null +++ b/test/lib/drizzle-helpers.js @@ -0,0 +1,30 @@ +import Database from 'better-sqlite3' +import { drizzle } from 'drizzle-orm/better-sqlite3' +import assert from 'node:assert/strict' +import test, { describe } from 'node:test' +import { tableCountIfExists } from '../../src/lib/drizzle-helpers.js' + +describe('table count if exists', () => { + const db = new Database(':memory:') + + db.exec('CREATE TABLE empty (ignored)') + + db.exec('CREATE TABLE filled (n INT)') + db.exec('INSERT INTO filled (n) VALUES (9)') + db.exec('INSERT INTO filled (n) VALUES (8)') + db.exec('INSERT INTO filled (n) VALUES (7)') + + const driz = drizzle(db) + + test("when table doesn't exist", () => { + assert.equal(tableCountIfExists(driz, 'doesnt_exist'), 0) + }) + + test('when table is empty', () => { + assert.equal(tableCountIfExists(driz, 'empty'), 0) + }) + + test('when table has rows', () => { + assert.equal(tableCountIfExists(driz, 'filled'), 3) + }) +})