core: migrate deleted items as well

packages/core/__tests__/migrations.test.ts

@@ -21,7 +21,7 @@ import { test, expect, describe } from "vitest";
 import { migrateItem, migrateKV, migrateVaultKey } from "../src/migrations";
 import { databaseTest } from "./utils";
 import { getId, makeId } from "../src/utils/id";
-import { LegacySettingsItem } from "../src/types";
+import { DeletedItem, LegacySettingsItem } from "../src/types";
 import { KEYS } from "../src/database/kv";
 
 describe.concurrent("[5.2] replace date edited with date modified", () => {
@@ -728,3 +728,18 @@ test("[5.9] migrate vaultKey", () =>
     expect((await db.vaults.default())?.key).toStrictEqual(key);
     expect((await db.vaults.default())?.key).toStrictEqual(key);
   }));
+
+test("[5.9] remove deleteReason from deleted items", () =>
+  databaseTest().then(async (db) => {
+    const item: DeletedItem = {
+      dateModified: Date.now(),
+      deleted: true,
+      id: "hello",
+      remote: false,
+      synced: true,
+      deleteReason: true as any
+    };
+    await migrateItem(item, 5.9, 6.0, "all", db, "backup");
+
+    expect(item.deleteReason).toBeUndefined();
+  }));

packages/core/src/api/sync/index.ts

@@ -38,12 +38,18 @@ import { migrateItem, migrateVaultKey } from "../../migrations";
 import { SerializedKey } from "@notesnook/crypto";
 import {
   Attachment,
+  isDeleted,
+  isTrashItem,
   Item,
   MaybeDeletedItem,
   Note,
   Notebook
 } from "../../types";
-import { SYNC_COLLECTIONS_MAP, SyncTransferItem } from "./types";
+import {
+  SYNC_COLLECTIONS_MAP,
+  SyncableItemType,
+  SyncTransferItem
+} from "./types";
 import { DownloadableFile } from "../../database/fs";
 import { SyncDevices } from "./devices";
 import { DefaultColors } from "../../collections/colors";
@@ -306,7 +312,12 @@ class Sync {
     for (let i = 0; i < decrypted.length; ++i) {
       const decryptedItem = decrypted[i];
       const version = chunk.items[i].v;
-      const item = await deserializeItem(decryptedItem, version, this.db);
+      const item = await deserializeItem(
+        decryptedItem,
+        itemType,
+        version,
+        this.db
+      );
       if (item) deserialized.push(item);
     }
 
@@ -427,49 +438,50 @@ function promiseTimeout(ms: number, promise: Promise<unknown>) {
 
 async function deserializeItem(
   decryptedItem: string,
+  type: SyncableItemType,
   version: number,
   database: Database
 ): Promise<MaybeDeletedItem<Item> | undefined> {
-  const item = JSON.parse(decryptedItem);
+  const item = JSON.parse(decryptedItem) as MaybeDeletedItem<Item>;
   item.remote = true;
   item.synced = true;
 
-  if (!item.cipher) {
-    let migrationResult = await migrateItem(
-      item,
+  let migrationResult = await migrateItem(
+    item,
+    version,
+    CURRENT_DATABASE_VERSION,
+    isDeleted(item) ? type : item.type,
+    database,
+    "sync"
+  );
+  if (migrationResult === "skip") return;
+
+  // since items in trash can have their own set of migrations,
+  // we have to run the migration again to account for that.
+  if (isTrashItem(item)) {
+    migrationResult = await migrateItem(
+      item as unknown as Note | Notebook,
       version,
       CURRENT_DATABASE_VERSION,
-      item.type,
+      item.itemType,
       database,
       "sync"
     );
     if (migrationResult === "skip") return;
-
-    // since items in trash can have their own set of migrations,
-    // we have to run the migration again to account for that.
-    if (item.type === "trash" && item.itemType) {
-      migrationResult = await migrateItem(
-        item as unknown as Note | Notebook,
-        version,
-        CURRENT_DATABASE_VERSION,
-        item.itemType,
-        database,
-        "sync"
-      );
-      if (migrationResult === "skip") return;
-    }
-
-    const itemType =
-      // colors are naively of type "tag" instead of "color" so we have to fix that.
-      item.type === "tag" && DefaultColors[item.title.toLowerCase()]
-        ? "color"
-        : item.type === "trash" && "itemType" in item && item.itemType
-        ? item.itemType
-        : item.type;
-
-    if (!itemType || itemType === "topic" || itemType === "settings") return;
-
-    if (migrationResult) item.synced = false;
   }
+
+  const itemType = isDeleted(item)
+    ? type
+    : // colors are naively of type "tag" instead of "color" so we have to fix that.
+    item.type === "tag" && DefaultColors[item.title.toLowerCase()]
+    ? "color"
+    : item.type === "trash" && "itemType" in item && item.itemType
+    ? item.itemType
+    : item.type;
+
+  if (!itemType || itemType === "topic" || itemType === "settings") return;
+
+  if (migrationResult) item.synced = false;
+
   return item;
 }

packages/core/src/database/migrator.ts

@@ -108,18 +108,12 @@ class Migrator {
       if (Array.isArray(item)) continue;
       if (!item) continue;
 
-      // check if item is permanently deleted or just a soft delete
-      if (isDeleted(item)) {
-        toAdd.push(item);
-        continue;
-      }
-
       const itemId = item.id;
       let migrated = await migrateItem(
         item,
         version,
         CURRENT_DATABASE_VERSION,
-        item.type || type,
+        isDeleted(item) ? "all" : item.type,
         db,
         "local"
       );
@@ -137,9 +131,10 @@ class Migrator {
       }
 
       if (migrated === true) {
-        if (item.type === "settings") {
+        if (!isDeleted(item) && item.type === "settings") {
           // we are removing the old settings.
-          await db.storage().remove("settings");
+          if (process.env.NODE_ENV === "test")
+            await db.storage().remove("settings");
         } else toAdd.push(item);
 
         // if id changed after migration, we need to delete the old one.
@@ -151,7 +146,6 @@ class Migrator {
 
     if (toAdd.length > 0) {
       await table.put(toAdd as any);
-      // await collection.setItems(toAdd);
       sendMigrationProgressEvent(
         db.eventManager,
         type,

packages/core/src/migrations.ts

@@ -30,7 +30,9 @@ import {
   Item,
   ItemMap,
   ItemType,
-  ToolbarConfigPlatforms
+  MaybeDeletedItem,
+  ToolbarConfigPlatforms,
+  isDeleted
 } from "./types";
 import { isCipher } from "./database/crypto";
 import { IndexedCollection } from "./database/indexed-collection";
@@ -43,7 +45,7 @@ type MigrationItemType = ItemType | "notehistory" | "content" | "all";
 type MigrationItemMap = ItemMap & {
   notehistory: HistorySession;
   content: ContentItem;
-  all: Item;
+  all: MaybeDeletedItem<Item>;
 };
 type Migration = {
   version: number;
@@ -430,7 +432,7 @@ const migrations: Migration[] = [
 ];
 
 export async function migrateItem<TItemType extends MigrationItemType>(
-  item: MigrationItemMap[TItemType],
+  item: MaybeDeletedItem<MigrationItemMap[TItemType]>,
   itemVersion: number,
   databaseVersion: number,
   type: TItemType,
@@ -453,15 +455,26 @@ export async function migrateItem<TItemType extends MigrationItemType>(
     const migration = migrations[migrationStartIndex];
     if (migration.version === databaseVersion) break;
 
-    if (
-      migration.items.all &&
-      (await migration.items.all(item, database, migrationType))
-    )
+    let result =
+      !!migration.items.all &&
+      (await migration.items.all(item, database, migrationType));
+    if (result === "skip") return "skip";
+    if (result) {
+      if (
+        !isDeleted(item) &&
+        item.type &&
+        item.type !== "trash" &&
+        item.type !== type
+      )
+        type = item.type as TItemType;
       count++;
+    }
+
+    if (isDeleted(item)) continue;
 
     const itemMigrator = migration.items[type];
     if (!itemMigrator) continue;
-    const result = await itemMigrator(item, database, migrationType);
+    result = await itemMigrator(item, database, migrationType);
     if (result === "skip") return "skip";
     if (result) {
       if (item.type && item.type !== "trash" && item.type !== type)

packages/core/src/types.ts

@@ -469,6 +469,11 @@ export interface DeletedItem {
   dateModified: number;
   remote?: boolean;
   synced?: boolean;
+
+  /**
+   * @deprecated only kept here for migration purposes
+   */
+  deleteReason?: never;
 }
 
 export type MaybeDeletedItem<T> = T | DeletedItem;