core: do not stop sync if a conflict occurs

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

@@ -90,6 +90,9 @@ function filterSyncableItems(items: MaybeDeletedItem<Item>[]): {
   const ids = [];
   const syncableItems = [];
   for (const item of items) {
+    // do not sync conflicted note or content
+    if ("conflicted" in item && item.conflicted) continue;
+
     delete item.synced;
 
     ids.push(item.id);

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

@@ -1,36 +0,0 @@
-/*
-This file is part of the Notesnook project (https://notesnook.com/)
-
-Copyright (C) 2023 Streetwriters (Private) Limited
-
-This program is free software: you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or
-(at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-import Database from "..";
-
-class Conflicts {
-  constructor(private readonly db: Database) {}
-
-  async check() {
-    return (await this.db.notes.conflicted.count()) > 0;
-  }
-
-  throw() {
-    throw new Error(
-      "Merge conflicts detected. Please resolve all conflicts to continue syncing."
-      //  { cause: "MERGE_CONFLICT" }
-    );
-  }
-}
-export default Conflicts;

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

@@ -30,7 +30,6 @@ import TokenManager from "../token-manager";
 import Collector from "./collector";
 import * as signalr from "@microsoft/signalr";
 import Merger from "./merger";
-import Conflicts from "./conflicts";
 import { AutoSync } from "./auto-sync";
 import { MessagePackHubProtocol } from "@microsoft/signalr-protocol-msgpack";
 import { logger } from "../../logger";
@@ -97,7 +96,6 @@ export default class SyncManager {
 }
 
 class Sync {
-  conflicts = new Conflicts(this.db);
   collector = new Collector(this.db);
   merger = new Merger(this.db);
   autoSync = new AutoSync(this.db, 1000);
@@ -180,10 +178,6 @@ class Sync {
   async init(isForceSync?: boolean) {
     await this.checkConnection();
 
-    if (await this.conflicts.check()) {
-      this.conflicts.throw();
-    }
-
     if (isForceSync) {
       await this.devices.unregister();
       await this.devices.register();
@@ -241,10 +235,6 @@ class Sync {
     }
 
     this.connection.off("SendItems");
-
-    if (await this.conflicts.check()) {
-      this.conflicts.throw();
-    }
   }
 
   async send(deviceId: string, isForceSync?: boolean) {

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

@@ -22,6 +22,7 @@ import { isHTMLEqual } from "../../utils/html-diff";
 import Database from "..";
 import { ContentItem, Item, MaybeDeletedItem, isDeleted } from "../../types";
 
+const THRESHOLD = process.env.NODE_ENV === "test" ? 6 * 1000 : 60 * 1000;
 class Merger {
   logger = logger.scope("Merger");
   constructor(private readonly db: Database) {}
@@ -30,43 +31,6 @@ class Merger {
   //   return type in SYNC_COLLECTIONS_MAP;
   // }
 
-  isConflicted(
-    localItem: MaybeDeletedItem<Item>,
-    remoteItem: MaybeDeletedItem<Item>,
-    conflictThreshold: number
-  ) {
-    const isResolved =
-      "dateResolved" in localItem &&
-      localItem.dateResolved === remoteItem.dateModified;
-    const isModified =
-      // the local item is modified if it was changed/modified after the last
-      // sync i.e. it wasn't synced yet.
-      // However, in case a sync is interrupted the local item's date modified
-      // will be ahead of last sync. In that case, we also have to check if the
-      // synced flag is false (it is only false if a user makes edits on the
-      // local device).
-      localItem.dateModified > remoteItem.dateModified && !localItem.synced;
-    if (isModified && !isResolved) {
-      // If time difference between local item's edits & remote item's edits
-      // is less than threshold, we shouldn't trigger a merge conflict; instead
-      // we will keep the most recently changed item.
-      const timeDiff =
-        Math.max(remoteItem.dateModified, localItem.dateModified) -
-        Math.min(remoteItem.dateModified, localItem.dateModified);
-
-      if (timeDiff < conflictThreshold) {
-        if (remoteItem.dateModified > localItem.dateModified) {
-          return "merge";
-        }
-        return;
-      }
-
-      return "conflict";
-    } else if (!isResolved) {
-      return "merge";
-    }
-  }
-
   mergeItemSync(
     remoteItem: MaybeDeletedItem<Item>,
     localItem: MaybeDeletedItem<Item> | undefined,
@@ -103,36 +67,37 @@ class Merger {
   ) {
     if (localItem && "localOnly" in localItem && localItem.localOnly) return;
 
-    const THRESHOLD = process.env.NODE_ENV === "test" ? 6 * 1000 : 60 * 1000;
-    const conflicted =
-      localItem && this.isConflicted(localItem, remoteItem, THRESHOLD);
-    if (!localItem || conflicted === "merge") {
-      return remoteItem;
-    } else if (conflicted === "conflict") {
-      if (
-        isDeleted(localItem) ||
-        isDeleted(remoteItem) ||
-        remoteItem.type !== "tiptap" ||
-        localItem.type !== "tiptap" ||
-        localItem.locked ||
-        remoteItem.locked ||
-        !localItem.data ||
-        !remoteItem.data ||
-        isHTMLEqual(localItem.data, remoteItem.data)
-      ) {
-        if (remoteItem.dateModified > localItem.dateModified) return remoteItem;
-        return;
-      }
+    if (
+      !localItem ||
+      isDeleted(localItem) ||
+      isDeleted(remoteItem) ||
+      remoteItem.type !== "tiptap" ||
+      localItem.type !== "tiptap" ||
+      localItem.locked ||
+      remoteItem.locked ||
+      !localItem.data ||
+      !remoteItem.data
+    ) {
+      if (!localItem || remoteItem.dateModified > localItem.dateModified)
+        return remoteItem;
+      return;
+    } else {
+      // it's possible that the local item already has a conflict so
+      // we can just replace the conflicted content
+      const conflicted = localItem.conflicted
+        ? "conflict"
+        : isContentConflicted(localItem, remoteItem, THRESHOLD);
+
+      if (conflicted === "merge") return remoteItem;
+      else if (!conflicted) return;
 
       // otherwise we trigger the conflicts
       await this.db.notes.add({
         id: localItem.noteId,
         conflicted: true
       });
-      return {
-        ...localItem,
-        conflicted: remoteItem
-      } as ContentItem;
+      localItem.conflicted = remoteItem;
+      return localItem;
     }
   }
 
@@ -171,3 +136,37 @@ class Merger {
   }
 }
 export default Merger;
+
+function isContentConflicted(
+  localItem: ContentItem,
+  remoteItem: ContentItem,
+  conflictThreshold: number
+) {
+  const isResolved = localItem.dateResolved === remoteItem.dateModified;
+  const isEdited =
+    // the local item is edited if it was changed/edited after the remote
+    // note and it also wasn't synced yet.
+    localItem.dateEdited > remoteItem.dateEdited && !localItem.synced;
+  if (isEdited && !isResolved) {
+    // If time difference between local item's edits & remote item's edits
+    // is less than threshold, we shouldn't trigger a merge conflict; instead
+    // we will keep the most recently changed item.
+    const timeDiff =
+      Math.max(remoteItem.dateEdited, localItem.dateEdited) -
+      Math.min(remoteItem.dateEdited, localItem.dateEdited);
+
+    if (
+      timeDiff < conflictThreshold ||
+      isHTMLEqual(localItem.data, remoteItem.data)
+    ) {
+      if (remoteItem.dateModified > localItem.dateModified) {
+        return "merge";
+      }
+      return;
+    }
+
+    return "conflict";
+  } else if (!isResolved) {
+    return "merge";
+  }
+}

packages/core/src/database/index.ts

@@ -157,7 +157,6 @@ type BooleanFields = ValueOf<{
 
 const BooleanProperties: Set<BooleanFields> = new Set([
   "compressed",
-  "conflicted",
   "deleted",
   "disabled",
   "favorite",
@@ -171,6 +170,9 @@ const BooleanProperties: Set<BooleanFields> = new Set([
 ]);
 
 const DataMappers: Partial<Record<ItemType, (row: any) => void>> = {
+  note: (row) => {
+    row.conflicted = row.conflicted === 1;
+  },
   reminder: (row) => {
     if (row.selectedDays) row.selectedDays = JSON.parse(row.selectedDays);
   },
@@ -263,7 +265,7 @@ export class SqliteBooleanPlugin implements KyselyPlugin {
 
       for (const key in row) {
         if (BooleanProperties.has(key as BooleanFields)) {
-          row[key] = row[key] === 1 ? true : false;
+          row[key] = row[key] === 1;
         }
       }