Make sure the offline storage version is consistent with migrations

buildSrc/DevBuild.js

@@ -24,12 +24,11 @@ const projectRoot = path.resolve(path.join(buildSrc, ".."))
  * @param host
  * @param desktop
  * @param clean
- * @param ignoreMigrations
  * @param networkDebugging
  * @param app {"mail"|"calendar"}
  * @returns {Promise<void>}
  */
-export async function runDevBuild({ stage, host, desktop, clean, ignoreMigrations, networkDebugging, app }) {
+export async function runDevBuild({ stage, host, desktop, clean, networkDebugging, app }) {
 	const isCalendarBuild = app === "calendar"
 	const tsConfig = isCalendarBuild ? "tsconfig-calendar-app.json" : "tsconfig.json"
 	const buildDir = isCalendarBuild ? "build-calendar-app" : "build"

make.js

@@ -13,7 +13,6 @@ await program
 	.option("--desktop-build-only", "Assemble desktop client without starting")
 	.option("-v, --verbose", "activate verbose logging in desktop client")
 	.option("-s, --serve", "Start a local server to serve the website")
-	.option("--ignore-migrations", "Dont check offline database migrations.")
 	.option("--network-debugging", "activate network debugging, sending attributeNames and attributeIds in the json request/response payloads", false)
 	.option("-D, --dev-tools", "Start the desktop client with DevTools open")
 	.action(async (stage, host, options) => {
@@ -27,7 +26,7 @@ await program
 			host = "https://app.local.tuta.com:9000"
 		}
 
-		const { clean, watch, serve, startDesktop, desktopBuildOnly, ignoreMigrations, app, networkDebugging, devTools } = options
+		const { clean, watch, serve, startDesktop, desktopBuildOnly, app, networkDebugging, devTools } = options
 
 		if (serve) {
 			console.error("--serve is currently disabled, point any server to ./build directory instead or build desktop")
@@ -41,7 +40,6 @@ await program
 				watch,
 				serve,
 				desktop: startDesktop || desktopBuildOnly,
-				ignoreMigrations,
 				networkDebugging,
 				app,
 			})

src/common/api/worker/offline/OfflineStorageMigrator.ts

@@ -1,11 +1,12 @@
 import { OfflineDbMeta, OfflineStorage } from "./OfflineStorage.js"
-import { assertNotNull } from "@tutao/tutanota-utils"
+import { assertNotNull, last } from "@tutao/tutanota-utils"
 import { SqlCipherFacade } from "../../../native/common/generatedipc/SqlCipherFacade.js"
 import { OutOfSyncError } from "../../common/error/OutOfSyncError.js"
 import { offline5 } from "./migrations/offline-v5"
 import { offline6 } from "./migrations/offline-v6"
 import { offline7 } from "./migrations/offline-v7"
 import { offline8 } from "./migrations/offline-v8"
+import { ProgrammingError } from "../../common/error/ProgrammingError"
 
 export interface OfflineMigration {
 	readonly version: number
@@ -42,6 +43,8 @@ export class OfflineStorageMigrator {
 	constructor(private readonly migrations: ReadonlyArray<OfflineMigration>) {}
 
 	async migrate(storage: OfflineStorage, sqlCipherFacade: SqlCipherFacade) {
+		assertLastMigrationConsistentVersion(this.migrations)
+
 		const meta = await storage.dumpMetadata()
 
 		// We did not write down the "offline" version from the beginning, so we need to figure out if we need to run the migration for the db structure or
@@ -62,39 +65,45 @@ export class OfflineStorageMigrator {
 			throw new OutOfSyncError(`offline database has newer schema than client`)
 		}
 
-		await this.runMigrations(meta, storage, sqlCipherFacade)
+		// note: we are passing populatedMeta to have up-to-date version
+		await this.runMigrations(populatedMeta, storage, sqlCipherFacade)
 	}
 
-	private async runMigrations(meta: Partial<OfflineDbMeta>, storage: OfflineStorage, sqlCipherFacade: SqlCipherFacade) {
+	private async runMigrations(meta: Pick<OfflineDbMeta, "offline-version">, storage: OfflineStorage, sqlCipherFacade: SqlCipherFacade) {
+		let currentOfflineVersion = meta[`offline-version`]
 		for (const { version, migrate } of this.migrations) {
-			const storedOfflineVersion = meta[`offline-version`]!
-			if (storedOfflineVersion < version) {
-				console.log(`running offline db migration from ${storedOfflineVersion} to ${version}`)
+			if (currentOfflineVersion < version) {
+				console.log(`running offline db migration from ${currentOfflineVersion} to ${version}`)
 				await migrate(storage, sqlCipherFacade)
-				console.log("migration finished")
+				console.log(`migration finished to ${currentOfflineVersion}`)
 				await storage.setCurrentOfflineSchemaVersion(version)
+				currentOfflineVersion = version
 			}
 		}
 	}
 
-	private async populateModelVersions(meta: Readonly<Partial<OfflineDbMeta>>, storage: OfflineStorage): Promise<Partial<OfflineDbMeta>> {
+	private async populateModelVersions(meta: Readonly<Partial<OfflineDbMeta>>, storage: OfflineStorage): Promise<Pick<OfflineDbMeta, "offline-version">> {
 		// copy metadata because it's going to be mutated
 		const newMeta = { ...meta }
-		await this.prepopulateVersionIfAbsent(CURRENT_OFFLINE_VERSION, newMeta, storage)
-		return newMeta
+		return await this.prepopulateVersionIfAbsent(CURRENT_OFFLINE_VERSION, newMeta, storage)
 	}
 
 	/**
 	 * update the metadata table to initialize the row of the app with the given schema version
 	 *
 	 * NB: mutates meta
 	 */
-	private async prepopulateVersionIfAbsent(version: number, meta: Partial<OfflineDbMeta>, storage: OfflineStorage) {
+	private async prepopulateVersionIfAbsent(
+		version: number,
+		meta: Partial<OfflineDbMeta>,
+		storage: OfflineStorage,
+	): Promise<Pick<OfflineDbMeta, "offline-version">> {
 		const storedVersion = meta["offline-version"]
 		if (storedVersion == null) {
 			meta["offline-version"] = version
 			await storage.setCurrentOfflineSchemaVersion(version)
 		}
+		return meta as { "offline-version": typeof version }
 	}
 
 	/**
@@ -109,3 +118,12 @@ export class OfflineStorageMigrator {
 		return assertNotNull(meta[`offline-version`]) > CURRENT_OFFLINE_VERSION
 	}
 }
+
+export function assertLastMigrationConsistentVersion(migrations: ReadonlyArray<OfflineMigration>): void {
+	const lastMigration = last(migrations)
+	if (lastMigration != null && lastMigration.version !== CURRENT_OFFLINE_VERSION) {
+		throw new ProgrammingError(
+			`Inconsistent offline migration state: expected latest version to be ${CURRENT_OFFLINE_VERSION} based on CURRENT_OFFLINE_VERSION but the last migration version is ${lastMigration.version}`,
+		)
+	}
+}

test/tests/api/worker/offline/OfflineStorageMigratorTest.ts

@@ -1,5 +1,6 @@
 import o from "@tutao/otest"
 import {
+	assertLastMigrationConsistentVersion,
 	CURRENT_OFFLINE_VERSION,
 	OFFLINE_STORAGE_MIGRATIONS,
 	OfflineMigration,
@@ -8,37 +9,11 @@ import {
 import { OfflineStorage } from "../../../../../src/common/api/worker/offline/OfflineStorage.js"
 import { func, instance, matchers, object, when } from "testdouble"
 import { verify } from "@tutao/tutanota-test-utils"
-import { ModelInfos } from "../../../../../src/common/api/common/EntityFunctions.js"
+import { ProgrammingError } from "../../../../../src/common/api/common/error/ProgrammingError.js"
 import { SqlCipherFacade } from "../../../../../src/common/native/common/generatedipc/SqlCipherFacade.js"
 import { maxBy } from "@tutao/tutanota-utils"
 
 o.spec("OfflineStorageMigrator", function () {
-	const modelInfos: ModelInfos = {
-		base: {
-			version: 1,
-		},
-		sys: {
-			version: 1,
-		},
-		tutanota: {
-			version: 42,
-		},
-		storage: {
-			version: 1,
-		},
-		accounting: {
-			version: 1,
-		},
-		gossip: {
-			version: 1,
-		},
-		monitor: {
-			version: 1,
-		},
-		usage: {
-			version: 1,
-		},
-	}
 	let migrations: OfflineMigration[]
 	let migrator: OfflineStorageMigrator
 	let storage: OfflineStorage
@@ -51,34 +26,59 @@ o.spec("OfflineStorageMigrator", function () {
 		sqlCipherFacade = object()
 	})
 
-	o("when there's an empty database the current model versions are written", async function () {
+	o.test("when there's an empty database the current model versions are written and migrations are not run", async function () {
 		when(storage.dumpMetadata()).thenResolve({})
+		const migration: OfflineMigration = {
+			version: CURRENT_OFFLINE_VERSION,
+			migrate: func() as OfflineMigration["migrate"],
+		}
+		migrations.push(migration)
 
 		await migrator.migrate(storage, sqlCipherFacade)
 		verify(storage.setCurrentOfflineSchemaVersion(CURRENT_OFFLINE_VERSION))
+		verify(migration.migrate(matchers.anything(), matchers.anything()), { times: 0 })
 	})
 
-	o("when the model version is written it is not overwritten", async function () {
+	o.test("when the model version is written it is not overwritten", async function () {
 		when(storage.dumpMetadata()).thenResolve({ "offline-version": 5 })
 
 		await migrator.migrate(storage, sqlCipherFacade)
 
 		verify(storage.setCurrentOfflineSchemaVersion(matchers.anything()), { times: 0 })
 	})
 
-	o("when migration exists and the version is incompatible the migration is run", async function () {
+	o.test("when migration exists and the version is incompatible the migration is run", async function () {
 		// stored is older than current so we actually "migrate" something
-		when(storage.dumpMetadata()).thenResolve({ "offline-version": 4 }, { "offline-version": 5 })
+		when(storage.dumpMetadata()).thenResolve({ "offline-version": 4 }, { "offline-version": CURRENT_OFFLINE_VERSION })
 		const migration: OfflineMigration = {
-			version: 5,
+			version: CURRENT_OFFLINE_VERSION,
 			migrate: func() as OfflineMigration["migrate"],
 		}
 		migrations.push(migration)
 
 		await migrator.migrate(storage, sqlCipherFacade)
 
 		verify(migration.migrate(storage, sqlCipherFacade))
-		verify(storage.setCurrentOfflineSchemaVersion(5))
+		verify(storage.setCurrentOfflineSchemaVersion(CURRENT_OFFLINE_VERSION))
+	})
+
+	o.test("when the last migration has inconsistent version it throws", async function () {
+		// stored is older than current so we actually "migrate" something
+		when(storage.dumpMetadata()).thenResolve({ "offline-version": 4 }, { "offline-version": CURRENT_OFFLINE_VERSION })
+		const migration: OfflineMigration = {
+			version: CURRENT_OFFLINE_VERSION - 1,
+			migrate: func() as OfflineMigration["migrate"],
+		}
+		migrations.push(migration)
+
+		await o.check(() => migrator.migrate(storage, sqlCipherFacade)).asyncThrows(ProgrammingError)
+
+		verify(migration.migrate(matchers.anything(), matchers.anything()), { times: 0 })
+		verify(storage.setCurrentOfflineSchemaVersion(matchers.anything()), { times: 0 })
+	})
+
+	o.test("real migration list: consistent with the latest version", async function () {
+		assertLastMigrationConsistentVersion(OFFLINE_STORAGE_MIGRATIONS)
 	})
 
 	o("ensure CURRENT_OFFLINE_VERSION matches the greatest registered migration", async function () {