Skip to content

Commit

Permalink
repair existing broken events in offline dbs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nokhub committed Oct 19, 2022
1 parent 45e8b6f commit f136d59
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/api/worker/offline/OfflineStorage.ts
Expand Up @@ -51,7 +51,7 @@ export const customTypeDecoders: Array<TypeDecoder> = (() => {
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
Expand Down Expand Up @@ -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)
}

Expand Down
27 changes: 19 additions & 8 deletions 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"
Expand All @@ -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
Expand All @@ -22,6 +23,7 @@ export const OFFLINE_STORAGE_MIGRATIONS: ReadonlyArray<OfflineMigration> = [
sys79,
sys80,
tutanota54,
repair1,
]

/**
Expand Down Expand Up @@ -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`]!
Expand All @@ -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<OfflineDbMeta>, storage: OfflineStorage) {
const key = `${app}-version` as const
const storedVersion = meta[key]
if (storedVersion == null) {
meta[key] = version
await storage.setStoredModelVersion(app, version)
}
}
}
17 changes: 17 additions & 0 deletions 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<CalendarEvent> = 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)
}
}
}
1 change: 1 addition & 0 deletions test/tests/Suite.ts
Expand Up @@ -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")
}

Expand Down
73 changes: 72 additions & 1 deletion 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 () {
Expand Down Expand Up @@ -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})
})
})
})

0 comments on commit f136d59

Please sign in to comment.