From f136d59aa92cbf5a03769ecdc7aa904323a469fd Mon Sep 17 00:00:00 2001 From: nok Date: Tue, 18 Oct 2022 16:27:03 +0200 Subject: [PATCH] repair existing broken events in offline dbs we have some cases of technically illegal events that got imported into some offline DBs. they were serialized in a lossy way and can be detected by checking for zeroed out start- and end times. this migration deletes the event lists from the offline db that contain such events, they will then be re-downloaded when the calendar is populated in the cache postlogin action. we're leaving events belonging to these lists in the cache, they will be overwritten while redownloading because of the use of INSERT OR REPLACE and the way we have set up our primary keys. #4660 --- src/api/worker/offline/OfflineStorage.ts | 4 +- .../worker/offline/OfflineStorageMigrator.ts | 27 +++++-- .../worker/offline/migrations/repair-v1.ts | 17 +++++ test/tests/Suite.ts | 1 + .../offline/OfflineStorageMigrationsTest.ts | 73 ++++++++++++++++++- 5 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 src/api/worker/offline/migrations/repair-v1.ts diff --git a/src/api/worker/offline/OfflineStorage.ts b/src/api/worker/offline/OfflineStorage.ts index 1858087f1ef..ea782dd7600 100644 --- a/src/api/worker/offline/OfflineStorage.ts +++ b/src/api/worker/offline/OfflineStorage.ts @@ -51,7 +51,7 @@ export const customTypeDecoders: Array = (() => { return tags })() -export type Apps = keyof typeof modelInfos +export type Apps = keyof typeof modelInfos | "repair" type AppMetadataEntries = { // Yes this is cursed, give me a break @@ -262,7 +262,7 @@ AND NOT(${firstIdBigger("elementId", upper)})` return Object.fromEntries(stored.map(([key, value]) => [key, cborg.decode(value)])) as OfflineDbMeta } - async setStoredModelVersion(model: keyof typeof modelInfos, version: number) { + async setStoredModelVersion(model: Apps, version: number) { return this.putMetadata(`${model}-version`, version) } diff --git a/src/api/worker/offline/OfflineStorageMigrator.ts b/src/api/worker/offline/OfflineStorageMigrator.ts index 7cdbf019a8b..3ab2ae640bf 100644 --- a/src/api/worker/offline/OfflineStorageMigrator.ts +++ b/src/api/worker/offline/OfflineStorageMigrator.ts @@ -1,4 +1,4 @@ -import {Apps, OfflineStorage} from "./OfflineStorage.js" +import {Apps, OfflineDbMeta, OfflineStorage} from "./OfflineStorage.js" import {ModelInfos} from "../../common/EntityFunctions.js" import {typedKeys} from "@tutao/tutanota-utils" import {ProgrammingError} from "../../common/error/ProgrammingError.js" @@ -7,6 +7,7 @@ import {sys76} from "./migrations/sys-v76.js" import {tutanota54} from "./migrations/tutanota-v54.js" import {sys79} from "./migrations/sys-v79.js" import {sys80} from "./migrations/sys-v80.js" +import {repair1} from "./migrations/repair-v1" export interface OfflineMigration { readonly app: Apps @@ -22,6 +23,7 @@ export const OFFLINE_STORAGE_MIGRATIONS: ReadonlyArray = [ sys79, sys80, tutanota54, + repair1, ] /** @@ -50,15 +52,12 @@ export class OfflineStorageMigrator { // Populate model versions if they haven't been written already for (const app of typedKeys(this.modelInfos)) { - const key = `${app}-version` as const - const storedVersion = meta[key] - if (storedVersion == null) { - let version = this.modelInfos[app].version - meta[key] = version - await storage.setStoredModelVersion(app, version) - } + await this.prepopulateVersionIfNecessary(app, this.modelInfos[app].version, meta, storage) } + // always run all repair migrations if none were run on the db + await this.prepopulateVersionIfNecessary("repair", 0, meta, storage) + // Run the migrations for (const {app, version, migrate} of this.migrations) { const storedVersion = meta[`${app}-version`]! @@ -80,4 +79,16 @@ export class OfflineStorageMigrator { } } } + + /** + * update the metadata table to initialize the row of the app with the given model version + */ + private async prepopulateVersionIfNecessary(app: Apps, version: number, meta: Partial, storage: OfflineStorage) { + const key = `${app}-version` as const + const storedVersion = meta[key] + if (storedVersion == null) { + meta[key] = version + await storage.setStoredModelVersion(app, version) + } + } } \ No newline at end of file diff --git a/src/api/worker/offline/migrations/repair-v1.ts b/src/api/worker/offline/migrations/repair-v1.ts new file mode 100644 index 00000000000..095d658f645 --- /dev/null +++ b/src/api/worker/offline/migrations/repair-v1.ts @@ -0,0 +1,17 @@ +import {OfflineMigration} from "../OfflineStorageMigrator.js" +import {OfflineStorage} from "../OfflineStorage.js" +import {CalendarEvent, CalendarEventTypeRef} from "../../../entities/tutanota/TypeRefs" +import {getElementId, getListId} from "../../../common/utils/EntityUtils" + +export const repair1: OfflineMigration = { + app: "repair", + version: 1, + async migrate(storage: OfflineStorage) { + const allCalendarEvents: Array = await storage.getListElementsOfType(CalendarEventTypeRef) + const possiblyBrokenEvents = allCalendarEvents.filter(calendarEvent => calendarEvent.startTime.getTime() === 0 || calendarEvent.endTime.getTime() === 0) + const brokenListIDs = new Set(possiblyBrokenEvents.map(getListId)) + for (const listID of brokenListIDs) { + await storage.deleteRange(CalendarEventTypeRef, listID) + } + } +} \ No newline at end of file diff --git a/test/tests/Suite.ts b/test/tests/Suite.ts index e8cb10e4c52..a7cc8015988 100644 --- a/test/tests/Suite.ts +++ b/test/tests/Suite.ts @@ -145,6 +145,7 @@ async function setupSuite() { await import("./desktop/db/OfflineDbFacadeTest.js") await import ("./desktop/credentials/DesktopCredentialsEncryptionTest.js") await import("./api/worker/offline/OfflineStorageMigratorTest.js") + await import("./api/worker/offline/OfflineStorageMigrationsTest.js") await import("./api/worker/offline/OfflineStorageTest.js") } diff --git a/test/tests/api/worker/offline/OfflineStorageMigrationsTest.ts b/test/tests/api/worker/offline/OfflineStorageMigrationsTest.ts index 95497784b43..76c8cb3e893 100644 --- a/test/tests/api/worker/offline/OfflineStorageMigrationsTest.ts +++ b/test/tests/api/worker/offline/OfflineStorageMigrationsTest.ts @@ -1,9 +1,13 @@ import o from "ospec" import {OfflineStorage} from "../../../../../src/api/worker/offline/OfflineStorage.js" -import {object, when} from "testdouble" +import {object, when, matchers} from "testdouble" import {GiftCard, GiftCardTypeRef} from "../../../../../src/api/entities/sys/TypeRefs.js" import {booleanToNumberValue, migrateAllListElements, renameAttribute} from "../../../../../src/api/worker/offline/StandardMigrations.js" import {verify} from "@tutao/tutanota-test-utils" +import {CalendarEvent, CalendarEventTypeRef, createCalendarEvent} from "../../../../../src/api/entities/tutanota/TypeRefs" +import {repair1} from "../../../../../src/api/worker/offline/migrations/repair-v1" + +const {anything} = matchers o.spec("OfflineStorageMigrations", function () { o.spec("migrateAllListElements", function () { @@ -54,4 +58,71 @@ o.spec("OfflineStorageMigrations", function () { .deepEquals({attr: '0', ignoreMe: "doing nothing"}) }) }) + o.spec("repairMigrations", function () { + o("calendar only contains events after 1970, therefor no migrations is needed", async function () { + let storageMock: OfflineStorage = object() + const normalEvent: CalendarEvent = createCalendarEvent({ + startTime: new Date("1971"), + endTime: new Date("1990"), + _id: ["validListID", "someEventID"] + }) + when(storageMock.getListElementsOfType( + CalendarEventTypeRef + )).thenResolve([normalEvent]) + await repair1.migrate(storageMock) + + verify(storageMock.deleteRange( + anything(), + anything() + ), {times: 0}) + }) + o("calendar contains edge case event with 1970 as start date and should be migrated", async function () { + let storageMock: OfflineStorage = object() + const invalidEvent: CalendarEvent = createCalendarEvent({ + startTime: new Date("1970"), + endTime: new Date("1990"), + _id: ["invalidListID", "someEventID"] + }) + when(storageMock.getListElementsOfType( + CalendarEventTypeRef + )).thenResolve([invalidEvent]) + await repair1.migrate(storageMock) + + verify(storageMock.deleteRange( + CalendarEventTypeRef, + "invalidListID" + ), {times: 1}) + }) + o("two listIDs, one with valid events the other one with invalid events -> only one list should be deleted", async function () { + let storageMock: OfflineStorage = object() + const normalEvent: CalendarEvent = createCalendarEvent({ + startTime: new Date("1971"), + endTime: new Date("1990"), + _id: ["invalidListID", "someEventID"] + }) + const invalidEvent: CalendarEvent = createCalendarEvent({ + startTime: new Date("1970"), + endTime: new Date("1970"), + _id: ["invalidListID", "otherEventID"] + }) + const normalEvent2: CalendarEvent = createCalendarEvent({ + startTime: new Date("2000"), + endTime: new Date("2022"), + _id: ["validListID", "EventID"] + }) + when(storageMock.getListElementsOfType( + CalendarEventTypeRef + )).thenResolve([normalEvent, invalidEvent, normalEvent2]) + await repair1.migrate(storageMock) + + verify(storageMock.deleteRange( + CalendarEventTypeRef, + "invalidListID" + ), {times: 1}) + verify(storageMock.deleteRange( + CalendarEventTypeRef, + "validListID" + ), {times: 0}) + }) + }) })