From 288eca41f51a64e6b9eec1898fbd67bbb1e68d94 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Mon, 11 Dec 2023 14:41:12 +0500 Subject: [PATCH] core: do not stop sync if a conflict occurs --- packages/core/src/api/sync/collector.ts | 3 + packages/core/src/api/sync/conflicts.ts | 36 ------- packages/core/src/api/sync/index.ts | 10 -- packages/core/src/api/sync/merger.ts | 121 ++++++++++++------------ packages/core/src/database/index.ts | 6 +- 5 files changed, 67 insertions(+), 109 deletions(-) delete mode 100644 packages/core/src/api/sync/conflicts.ts diff --git a/packages/core/src/api/sync/collector.ts b/packages/core/src/api/sync/collector.ts index 343e0b624..97ab5b533 100644 --- a/packages/core/src/api/sync/collector.ts +++ b/packages/core/src/api/sync/collector.ts @@ -90,6 +90,9 @@ function filterSyncableItems(items: MaybeDeletedItem[]): { 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); diff --git a/packages/core/src/api/sync/conflicts.ts b/packages/core/src/api/sync/conflicts.ts deleted file mode 100644 index 8fe8892de..000000000 --- a/packages/core/src/api/sync/conflicts.ts +++ /dev/null @@ -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 . -*/ - -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; diff --git a/packages/core/src/api/sync/index.ts b/packages/core/src/api/sync/index.ts index 81cba6e1f..d099eaa1c 100644 --- a/packages/core/src/api/sync/index.ts +++ b/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) { diff --git a/packages/core/src/api/sync/merger.ts b/packages/core/src/api/sync/merger.ts index 91b625d4d..2f2990195 100644 --- a/packages/core/src/api/sync/merger.ts +++ b/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, - remoteItem: MaybeDeletedItem, - 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, localItem: MaybeDeletedItem | 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"; + } +} diff --git a/packages/core/src/database/index.ts b/packages/core/src/database/index.ts index fa99d50e4..4617d0ab1 100644 --- a/packages/core/src/database/index.ts +++ b/packages/core/src/database/index.ts @@ -157,7 +157,6 @@ type BooleanFields = ValueOf<{ const BooleanProperties: Set = new Set([ "compressed", - "conflicted", "deleted", "disabled", "favorite", @@ -171,6 +170,9 @@ const BooleanProperties: Set = new Set([ ]); const DataMappers: Partial 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; } }