diff --git a/src/lib/Account.ts b/src/lib/Account.ts index 1c6675e241..bb2627a090 100644 --- a/src/lib/Account.ts +++ b/src/lib/Account.ts @@ -317,7 +317,8 @@ export default class Account { // set from the persisted continuation Logger.log('Fetching local bookmarks tree') this.syncProcess.setCacheTree(cacheTree) - await this.localCachingResource.setCacheTree(await this.localCachingResource.getBookmarksTree()) + // Allow Caching of the local tree + await this.localCachingResource.getBookmarksTree() } Logger.log('Starting sync process') diff --git a/src/lib/CachingTreeWrapper.ts b/src/lib/CachingTreeWrapper.ts index f787dc255c..08ef4ee668 100644 --- a/src/lib/CachingTreeWrapper.ts +++ b/src/lib/CachingTreeWrapper.ts @@ -14,21 +14,23 @@ export default class CachingTreeWrapper implements OrderFolderResource> { const tree = await this.innerTree.getBookmarksTree() - this.cacheTree.setTree(tree) + this.cacheTree.setTree(tree.copy()) return tree } async setCacheTree(tree: Folder) { - this.cacheTree.setTree(tree) + this.cacheTree.setTree(tree.copy()) } async createBookmark(bookmark:Bookmark): Promise { const id = await this.innerTree.createBookmark(bookmark) const cacheId = await this.cacheTree.createBookmark(bookmark.copy(false)) const cacheBookmark = this.cacheTree.bookmarksCache.findBookmark(cacheId) + this.cacheTree.bookmarksCache.removeFromIndex(cacheBookmark) cacheBookmark.id = id cacheBookmark.parentId = bookmark.parentId - this.cacheTree.bookmarksCache.createIndex() + cacheBookmark.createIndex() + this.cacheTree.bookmarksCache.updateIndex(cacheBookmark) return id } @@ -46,9 +48,11 @@ export default class CachingTreeWrapper implements OrderFolderResource, NewLocat ReorderAction : never -export default class Diff> { +export default class Diff< + L1 extends TItemLocation, + L2 extends TItemLocation, + A extends Action +> { private readonly actions: A[] constructor() { this.actions = [] } - clone(filter: (action:A)=>boolean = () => true): Diff { - const newDiff : Diff = new Diff + clone(filter: (action: A) => boolean = () => true): Diff { + const newDiff: Diff = new Diff() this.getActions().forEach((action: A) => { if (filter(action)) { newDiff.commit(action) @@ -96,33 +100,55 @@ export default class Diff, item2: TItem, itemTree: Folder, cache: Record): boolean { - const cacheKey = 'contains:' + Mappings.mapId(mappingsSnapshot, item2, ItemLocation.LOCAL) + ':' + Mappings.mapId(mappingsSnapshot, item2, ItemLocation.SERVER) + - '-' + Mappings.mapId(mappingsSnapshot, item1, ItemLocation.LOCAL) + ':' + Mappings.mapId(mappingsSnapshot, item1, ItemLocation.SERVER) + static containsParent( + mappingsSnapshot: MappingSnapshot, + item1: TItem, + item2: TItem, + itemTree: Folder, + cache: Record + ): boolean { + const cacheKey = + 'contains:' + + Mappings.mapId(mappingsSnapshot, item2, ItemLocation.LOCAL) + + ':' + + Mappings.mapId(mappingsSnapshot, item2, ItemLocation.SERVER) + + '-' + + Mappings.mapId(mappingsSnapshot, item1, ItemLocation.LOCAL) + + ':' + + Mappings.mapId(mappingsSnapshot, item1, ItemLocation.SERVER) if (typeof cache[cacheKey] !== 'undefined') { return cache[cacheKey] } - const item1InTree = itemTree.findItem(item1.type, Mappings.mapId(mappingsSnapshot, item1, itemTree.location)) + const item1InTree = itemTree.findItem( + item1.type, + Mappings.mapId(mappingsSnapshot, item1, itemTree.location) + ) if ( - item1.findItem(ItemType.FOLDER, - Mappings.mapParentId(mappingsSnapshot, item2, item1.location)) || - (item1InTree && item1InTree.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, item2, itemTree.location))) || - String(Mappings.mapId(mappingsSnapshot, item1, item2.location)) === String(item2.parentId) || - String(Mappings.mapParentId(mappingsSnapshot, item2, item1.location)) === String(item1.id) + item1.findItem( + ItemType.FOLDER, + Mappings.mapParentId(mappingsSnapshot, item2, item1.location) + ) || + (item1InTree && + item1InTree.findItem( + ItemType.FOLDER, + Mappings.mapParentId(mappingsSnapshot, item2, itemTree.location) + )) || + String(Mappings.mapId(mappingsSnapshot, item1, item2.location)) === + String(item2.parentId) || + String(Mappings.mapParentId(mappingsSnapshot, item2, item1.location)) === + String(item1.id) ) { cache[cacheKey] = true return true @@ -140,24 +166,66 @@ export default class Diff = {}, chain: Action[] = [] ): boolean { - const currentItemLocalId = Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.LOCAL) - const currentItemServerId = Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.SERVER) - const targetPayloadLocalId = Mappings.mapId(mappingsSnapshot, targetAction.payload, ItemLocation.LOCAL) - const targetPayloadServerId = Mappings.mapId(mappingsSnapshot, targetAction.payload, ItemLocation.SERVER) + const currentItemLocalId = Mappings.mapId( + mappingsSnapshot, + currentItem, + ItemLocation.LOCAL + ) + const currentItemServerId = Mappings.mapId( + mappingsSnapshot, + currentItem, + ItemLocation.SERVER + ) + const targetPayloadLocalId = Mappings.mapId( + mappingsSnapshot, + targetAction.payload, + ItemLocation.LOCAL + ) + const targetPayloadServerId = Mappings.mapId( + mappingsSnapshot, + targetAction.payload, + ItemLocation.SERVER + ) const cacheKey = `hasChain:${currentItemLocalId}:${currentItemServerId}-${targetPayloadLocalId}:${targetPayloadServerId}` if (typeof cache[cacheKey] !== 'undefined') { return cache[cacheKey] } - if (Diff.containsParent(mappingsSnapshot, targetAction.payload, currentItem, itemTree, cache)) { + if ( + Diff.containsParent( + mappingsSnapshot, + targetAction.payload, + currentItem, + itemTree, + cache + ) + ) { cache[cacheKey] = true return true } - const newCurrentActions = actions.filter(newTargetAction => - !chain.includes(newTargetAction) && Diff.containsParent(mappingsSnapshot, newTargetAction.payload, currentItem, itemTree, cache) + const newCurrentActions = actions.filter( + (newTargetAction) => + !chain.includes(newTargetAction) && + Diff.containsParent( + mappingsSnapshot, + newTargetAction.payload, + currentItem, + itemTree, + cache + ) ) if (newCurrentActions.length) { for (const newCurrentAction of newCurrentActions) { - if (Diff.findChain(mappingsSnapshot, actions, itemTree, newCurrentAction.payload, targetAction, cache,[...chain, newCurrentAction])) { + if ( + Diff.findChain( + mappingsSnapshot, + actions, + itemTree, + newCurrentAction.payload, + targetAction, + cache, + [...chain, newCurrentAction] + ) + ) { return true } } @@ -166,27 +234,44 @@ export default class Diff(actions: MoveAction[], tree: Folder) :MoveAction[][] { - const bookmarks = actions.filter(a => a.payload.type === ItemType.BOOKMARK) - const folderMoves = actions.filter(a => a.payload.type === ItemType.FOLDER) - const DAG = folderMoves - .reduce((DAG, action1) => { - DAG[action1.payload.id] = folderMoves.filter(action2 => { - if (action1 === action2 || String(action1.payload.id) === String(action2.payload.id)) { + static sortMoves( + actions: MoveAction[], + tree: Folder + ): MoveAction[][] { + const bookmarks = actions.filter( + (a) => a.payload.type === ItemType.BOOKMARK + ) + const folderMoves = actions.filter( + (a) => a.payload.type === ItemType.FOLDER + ) + const DAG = folderMoves.reduce((DAG, action1) => { + DAG[action1.payload.id] = folderMoves + .filter((action2) => { + if ( + action1 === action2 || + String(action1.payload.id) === String(action2.payload.id) + ) { return false } return ( - (tree.findItem(action1.payload.type, action1.payload.id) && tree.findItem(action1.payload.type, action1.payload.id).findItem(action2.payload.type, action2.payload.id)) + tree.findItem(action1.payload.type, action1.payload.id) && + tree + .findItem(action1.payload.type, action1.payload.id) + .findItem(action2.payload.type, action2.payload.id) ) }) - .map(a => a.payload.id) - return DAG - }, {}) + .map((a) => a.payload.id) + return DAG + }, {}) let batches try { - batches = batchingToposort(DAG).map(batch => batch.map(id => folderMoves.find(a => String(a.payload.id) === String(id)))) + batches = batchingToposort(DAG).map((batch) => + batch.map((id) => + folderMoves.find((a) => String(a.payload.id) === String(id)) + ) + ) } catch (e) { - console.log({DAG, tree, actions}) + console.log({ DAG, tree, actions }) throw e } batches.push(bookmarks) @@ -202,13 +287,18 @@ export default class Diff(mappingsSnapshot:MappingSnapshot, targetLocation: L3, filter: (action: A)=>boolean = () => true, skipErroneousActions = false): Diff> { - const newDiff : Diff> = new Diff + map( + mappingsSnapshot: MappingSnapshot, + targetLocation: L3, + filter: (action: A) => boolean = () => true, + skipErroneousActions = false + ): Diff> { + const newDiff: Diff> = new Diff() // Map payloads this.getActions() - .map(a => a as A) - .forEach(action => { + .map((a) => a as A) + .forEach((action) => { let newAction if (!filter(action)) { @@ -227,7 +317,10 @@ export default class Diff { - return {...item, id: mappingsSnapshot[(targetLocation === ItemLocation.LOCAL ? ItemLocation.SERVER : ItemLocation.LOCAL) + 'To' + targetLocation][item.type][item.id]} + newAction.order = action.order.map((item) => { + return { + ...item, + id: mappingsSnapshot[ + (targetLocation === ItemLocation.LOCAL + ? ItemLocation.SERVER + : ItemLocation.LOCAL) + + 'To' + + targetLocation + ][item.type][item.id], + } }) } @@ -286,24 +413,48 @@ export default class Diff { - await yieldToEventLoop() - return { - ...action, - payload: await action.payload.clone(false).toJSONAsync(), - oldItem: await action.oldItem && action.oldItem.clone(false).toJSONAsync(), - } - }, 1) + return Parallel.map( + this.getActions(), + async(action: A) => { + await yieldToEventLoop() + return { + ...action, + payload: await action.payload.clone(false).toJSONAsync(), + oldItem: + action.oldItem && await action.oldItem.clone(false).toJSONAsync(), + } + }, + 1 + ) } - inspect(depth = 0):string { - return 'Diff\n' + this.getActions().map((action: A) => { - return `\nAction: ${action.type}\nPayload: #${action.payload.id}[${action.payload.title}]${'url' in action.payload ? `(${action.payload.url})` : ''} parentId: ${action.payload.parentId} ${'index' in action ? `Index: ${action.index}\n` : ''}${'order' in action ? `Order: ${JSON.stringify(action.order, null, '\t')}` : ''}` - }).join('\n') + inspect(depth = 0): string { + return ( + 'Diff\n' + + this.getActions() + .map((action: A) => { + return `\nAction: ${action.type}\nPayload: #${action.payload.id}[${ + action.payload.title + }]${ + 'url' in action.payload ? `(${action.payload.url})` : '' + } parentId: ${action.payload.parentId} ${ + 'index' in action ? `Index: ${action.index}\n` : '' + }${ + 'order' in action + ? `Order: ${JSON.stringify(action.order, null, '\t')}` + : '' + }` + }) + .join('\n') + ) } - static fromJSON>(json) { - const diff: Diff = new Diff + static fromJSON< + L1 extends TItemLocation, + L2 extends TItemLocation, + A2 extends Action + >(json) { + const diff: Diff = new Diff() json.forEach((action: A2): void => { action.payload = hydrate(action.payload) action.oldItem = action.oldItem && hydrate(action.oldItem) @@ -311,6 +462,21 @@ export default class Diff + >(json) { + const diff: Diff = new Diff() + await Parallel.map(json, async(action: A2): Promise => { + await yieldToEventLoop() + action.payload = hydrate(action.payload) + action.oldItem = action.oldItem && hydrate(action.oldItem) + diff.commit(action) + }, 1) + return diff + } } export interface PlanStage1 { @@ -342,5 +508,5 @@ export interface PlanRevert UPDATE: Diff> MOVE: Diff> REMOVE: Diff> - REORDER: Diff> + REORDER: Diff> } diff --git a/src/lib/LocalTabs.ts b/src/lib/LocalTabs.ts index dffb5c5252..860315da1f 100644 --- a/src/lib/LocalTabs.ts +++ b/src/lib/LocalTabs.ts @@ -15,6 +15,22 @@ export default class LocalTabs implements OrderFolderResource> { let tabs = await browser.tabs.query({ windowType: 'normal' // no devtools or panels or popups @@ -46,7 +62,7 @@ export default class LocalTabs implements OrderFolderResource - browser.tabGroups.get(bookmark.parentId) + browser.tabGroups.get(this.getTabGroupIdFromFolderId(bookmark.parentId)) ) if (tabGroup) { isTabGroup = true @@ -146,9 +162,9 @@ export default class LocalTabs implements OrderFolderResource browser.tabs.group({ tabIds: [node.id], - groupId: bookmark.parentId + groupId: this.getTabGroupIdFromFolderId(bookmark.parentId) }) ) } @@ -209,7 +225,7 @@ export default class LocalTabs implements OrderFolderResource - browser.tabGroups.get(bookmark.parentId) + browser.tabGroups.get(this.getTabGroupIdFromFolderId(bookmark.parentId)) ) isTabGroup = !!tabGroup } @@ -225,7 +241,7 @@ export default class LocalTabs implements OrderFolderResource browser.tabs.group({ tabIds: [bookmark.id], - groupId: bookmark.parentId + groupId: this.getTabGroupIdFromFolderId(bookmark.parentId) }) ) } else { @@ -240,7 +256,7 @@ export default class LocalTabs implements OrderFolderResource browser.tabs.move(bookmark.id, { - windowId: bookmark.parentId, + windowId: this.getWindowIdFromFolderId(bookmark.parentId), index: -1, // last }) ) @@ -268,7 +284,7 @@ export default class LocalTabs implements OrderFolderResource): Promise { + async createFolder(folder:Folder): Promise { Logger.log('(tabs)CREATEFOLDER', folder) // If parentId is 'tabs', create a window @@ -276,14 +292,14 @@ export default class LocalTabs implements OrderFolderResource browser.windows.create() ) - return node.id + return this.getFolderIdFromWindowId(node.id) } else { // Otherwise, create a tab group try { const groupId = await this.queue.add(async() => { // Create a dummy tab in the parent window to hold the group const dummyTab = await browser.tabs.create({ - windowId: folder.parentId, + windowId: this.getWindowIdFromFolderId(folder.parentId), url: 'about:blank', active: false }) @@ -292,8 +308,8 @@ export default class LocalTabs implements OrderFolderResource browser.tabs.query({groupId: id})) + const tabs = await this.queue.add(() => browser.tabs.query({groupId: this.getTabGroupIdFromFolderId(id)})) if (tabs.length) { isTabGroup = true // Get the tab group's current index @@ -382,14 +398,14 @@ export default class LocalTabs implements OrderFolderResource - browser.tabGroups.move(folder.id, { index: currentIndex }) + browser.tabGroups.move(this.getTabGroupIdFromFolderId(folder.id), { index: currentIndex }) ) // Get the size of the folder (number of tabs in the group) const folderTabs = await this.queue.add(() => browser.tabs.query({ windowType: 'normal', - groupId: folder.id + groupId: this.getTabGroupIdFromFolderId(folder.id) }) ) @@ -431,7 +447,7 @@ export default class LocalTabs implements OrderFolderResource - browser.tabGroups.update(folder.id, { + browser.tabGroups.update(this.getTabGroupIdFromFolderId(folder.id), { title: folder.title }) ) @@ -453,7 +469,7 @@ export default class LocalTabs implements OrderFolderResource browser.windows.remove(id)) + await this.queue.add(() => browser.windows.remove(this.getWindowIdFromFolderId(id))) } catch (e) { Logger.log('Failed to remove window', e) // Don't throw error if the window doesn't exist anymore @@ -467,7 +483,7 @@ export default class LocalTabs implements OrderFolderResource browser.tabs.query({ - groupId: id + groupId: this.getTabGroupIdFromFolderId(id) }) ) diff --git a/src/lib/Mappings.ts b/src/lib/Mappings.ts index fc288d6085..de9cabc14a 100644 --- a/src/lib/Mappings.ts +++ b/src/lib/Mappings.ts @@ -71,7 +71,7 @@ export default class Mappings { } private static add(mappings, { localId, remoteId }: { localId?:string|number, remoteId?:string|number }) { - if (typeof localId === 'undefined' || typeof remoteId === 'undefined') { + if (typeof localId === 'undefined' || typeof remoteId === 'undefined' || localId === null || remoteId === null) { throw new Error('Cannot add empty mapping') } mappings.LocalToServer[localId] = remoteId diff --git a/src/lib/Scanner.ts b/src/lib/Scanner.ts index 7d8fdda4b2..15a1a969ca 100644 --- a/src/lib/Scanner.ts +++ b/src/lib/Scanner.ts @@ -1,4 +1,3 @@ -import * as Parallel from 'async-parallel' import Diff, { ActionType, CreateAction, MoveAction, RemoveAction, ReorderAction, UpdateAction } from './Diff' import { Bookmark, Folder, ItemLocation, ItemType, TItem, TItemLocation } from './Tree' import Logger from './Logger' @@ -28,7 +27,7 @@ export default class Scanner this.newTree = newTree this.mergeable = mergeable this.hashSettings = hashSettings - this.checkHashes = typeof checkHashes === 'undefined' ? true : checkHashes + this.checkHashes = typeof checkHashes === 'undefined' || checkHashes === null ? true : checkHashes this.hasCache = hasCache this.result = { CREATE: new Diff(), @@ -70,16 +69,32 @@ export default class Scanner } } - if (oldFolder.title !== newFolder.title && typeof oldFolder.parentId !== 'undefined' && typeof newFolder.parentId !== 'undefined') { + if ( + oldFolder.title !== newFolder.title && + typeof oldFolder.parentId !== 'undefined' && + typeof newFolder.parentId !== 'undefined' + ) { // folder title changed and it's not the root folder - this.result.UPDATE.commit({type: ActionType.UPDATE, payload: newFolder, oldItem: oldFolder}) + this.result.UPDATE.commit({ + type: ActionType.UPDATE, + payload: newFolder, + oldItem: oldFolder, + }) } // Generate REORDERS before diffing anything to make sure REORDERS are from top to bottom (necessary for tab sync) if (newFolder.children.length > 1) { let needReorder = false - for (let i = 0; i < Math.max(newFolder.children.length, oldFolder.children.length); i++) { - if (!oldFolder.children[i] || !newFolder.children[i] || !this.mergeable(oldFolder.children[i], newFolder.children[i])) { + for ( + let i = 0; + i < Math.max(newFolder.children.length, oldFolder.children.length); + i++ + ) { + if ( + !oldFolder.children[i] || + !newFolder.children[i] || + !this.mergeable(oldFolder.children[i], newFolder.children[i]) + ) { needReorder = true break } @@ -88,40 +103,77 @@ export default class Scanner this.result.REORDER.commit({ type: ActionType.REORDER, payload: newFolder, - order: newFolder.children.map(i => ({ type: i.type, id: i.id })), + order: newFolder.children.map((i) => ({ type: i.type, id: i.id })), }) } } // Preserved Items and removed Items + // Optimization: Use a Map for O(1) lookups + const unmatchedMap = new Map[]>() + for (const child of newFolder.children) { + const key = `${child.type}_${child.title}` // Or a better unique key based on mergeable logic + const list = unmatchedMap.get(key) || [] + list.push(child) + unmatchedMap.set(key, list) + } + const stillUnmatched = new Set(newFolder.children) + // (using map here, because 'each' doesn't provide indices) - const unmatchedChildren = newFolder.children.slice(0) - await Parallel.map(oldFolder.children, async(old, index) => { - const newItem = unmatchedChildren.find((child) => old.type === child.type && this.mergeable(old, child)) + let index = 0 + for (const old of oldFolder.children) { + const key = `${old.type}_${old.title}` + const potentialMatches = unmatchedMap.get(key) + let newItem = null + if (potentialMatches) { + const matchIndex = potentialMatches.findIndex((m) => + this.mergeable(old, m) + ) + if (matchIndex !== -1) { + newItem = potentialMatches.splice(matchIndex, 1)[0] + stillUnmatched.delete(newItem) + } + } else { + newItem = newFolder.children.find((child) => old.type === child.type && this.mergeable(old, child)) + if (newItem) stillUnmatched.delete(newItem) + } // we found an item in the new folder that matches the one in the old folder if (newItem) { await this.diffItem(old, newItem) - unmatchedChildren.splice(unmatchedChildren.indexOf(newItem), 1) - return + index++ + continue } if (newFolder.isRoot && newFolder.location === ItemLocation.LOCAL) { // We can't remove root folders locally - return + index++ + continue } - this.result.REMOVE.commit({type: ActionType.REMOVE, payload: old, index}) - }, 1) + this.result.REMOVE.commit({ + type: ActionType.REMOVE, + payload: old, + index, + }) + + index++ + } // created Items - // (using map here, because 'each' doesn't provide indices) - await Parallel.map(unmatchedChildren, async(newChild) => { + const childToIndex = new Map, number>() + newFolder.children.forEach((child, i) => childToIndex.set(child, i)) + + for (const newChild of stillUnmatched) { if (oldFolder.isRoot && oldFolder.location === ItemLocation.LOCAL) { // We can't create root folders locally - return + continue } - this.result.CREATE.commit({type: ActionType.CREATE, payload: newChild, index: newFolder.children.findIndex(child => child === newChild)}) - }, 1) + this.result.CREATE.commit({ + type: ActionType.CREATE, + payload: newChild, + index: childToIndex.get(newChild), + }) + } } async diffBookmark(oldBookmark:Bookmark, newBookmark:Bookmark):Promise { @@ -150,123 +202,283 @@ export default class Scanner async findMoves():Promise { Logger.log('Scanner: Finding moves') - let createActions - let removeActions - let reconciled = true - - // As soon as one match is found, action list is updated and search is started with the new list - // repeat until no rewrites happen anymore - while (reconciled) { - reconciled = false - let createAction: CreateAction, removeAction: RemoveAction - - // First find direct matches (avoids glitches when folders and their contents have been moved) - createActions = this.result.CREATE.getActions() - while (!reconciled && (createAction = createActions.shift())) { - // give the browser time to breathe - await Promise.resolve() - const createdItem = createAction.payload - removeActions = this.result.REMOVE.getActions() - while (!reconciled && (removeAction = removeActions.shift())) { - // give the browser time to breathe - await Promise.resolve() - const removedItem = removeAction.payload - - if (this.mergeable(removedItem, createdItem) && - (removedItem.type !== 'folder' || - (!this.hasCache && removedItem.childrenSimilarity(createdItem) > 0.8))) { - this.result.CREATE.retract(createAction) - this.result.REMOVE.retract(removeAction) - this.result.MOVE.commit({ - type: ActionType.MOVE, - payload: createdItem, - oldItem: removedItem, - index: createAction.index, - oldIndex: removeAction.index - }) - reconciled = true - // Don't use the items from the action, but the ones in the actual tree to avoid using tree parts mutated by this algorithm (see below) - await this.diffItem(removedItem, createdItem) + + let hasNewActions = true + let iterations = 0 + + while (hasNewActions) { + hasNewActions = false + + const createActions = this.result.CREATE.getActions() + const removeActions = this.result.REMOVE.getActions() + + if (createActions.length === 0 || removeActions.length === 0) break + + // Build Multi-Signal Fuzzy Index (O(N)) + const removedFuzzyMap = new Map; item: TItem }[]>() + const allCreatedItems: { rootAction: CreateAction; item: TItem }[] = [] + + const addToFuzzyIndex = (item: TItem, action: RemoveAction) => { + const keys = new Set() + // Signal 1: Full signature (Type + Title + URL) + keys.add(`${item.type}_${item.title}_${item.type === 'bookmark' ? (item as Bookmark).url : ''}`) + + // Signal 2: Title only (Handles URL changes for bookmarks or ID changes for folders) + if (item.title) keys.add(`${item.type}_title_${item.title}`) + + // Signal 3: URL only (Handles Title changes for bookmarks) + if (item instanceof Bookmark) keys.add(`bookmark_url_${item.url}`) + + keys.add(item.type) + + const element = { rootAction: action, item } // outside the loop so we can later use Set#has + for (const key of keys) { + let list = removedFuzzyMap.get(key) + if (!list) { + list = [] + removedFuzzyMap.set(key, list) } + list.push(element) + } + } + + for (const action of removeActions) { + await addToFuzzyIndex(action.payload, action) + if (action.payload instanceof Folder) { + await action.payload.traverse((child) => addToFuzzyIndex(child, action)) } } - // Then find descendant matches - createActions = this.result.CREATE.getActions() - while (!reconciled && (createAction = createActions.shift())) { - // give the browser time to breathe - await Promise.resolve() + // First pass: Try to find direct matches + + // Match directly created items against removed pool + for (const createAction of createActions) { + if (++iterations % 1000 === 0) { + await yieldToEventLoop() + } + const createdItem = createAction.payload - removeActions = this.result.REMOVE.getActions() - while (!reconciled && (removeAction = removeActions.shift())) { - // give the browser time to breathe - await Promise.resolve() - const removedItem = removeAction.payload - const oldItem = removedItem.findItemFilter( - createdItem.type, - item => this.mergeable(item, createdItem), - item => item.childrenSimilarity(createdItem) - ) - if (oldItem) { - let oldIndex - this.result.CREATE.retract(createAction) - if (oldItem === removedItem) { - this.result.REMOVE.retract(removeAction) - } else { - // We clone the item here, because we don't want to mutate all copies of this tree (item) - const removedItemClone = removedItem.copy(true) - const oldParentClone = removedItemClone.findItem(ItemType.FOLDER, oldItem.parentId) as Folder - const oldItemClone = removedItemClone.findItem(oldItem.type, oldItem.id) - oldIndex = oldParentClone.children.indexOf(oldItemClone) - oldParentClone.children.splice(oldIndex, 1) - removeAction.payload = removedItemClone - removeAction.payload.createIndex() - } - this.result.MOVE.commit({ - type: ActionType.MOVE, - payload: createdItem, - oldItem, - index: createAction.index, - oldIndex: oldIndex || removeAction.index - }) - reconciled = true - if (oldItem.type === ItemType.FOLDER) { // TODO: Is this necessary? - await this.diffItem(oldItem, createdItem) + + // Gather potential matches from all signals + const searchKeys = [ + `${createdItem.type}_${createdItem.title}_${ + createdItem.type === 'bookmark' + ? (createdItem as Bookmark).url + : '' + }`, + `${createdItem.type}_title_${createdItem.title}`, + ...(createdItem instanceof Bookmark + ? [`bookmark_url_${createdItem.url}`] + : []), + createdItem.type, + ] + + // Collect unique potential matches from all signals + const matchesSet = new Set<{ + rootAction: RemoveAction + item: TItem + }>() + for (const key of searchKeys) { + const list = removedFuzzyMap.get(key) + if (!list) continue + for (const m of list) { + if (this.mergeable(m.item, createdItem)) { + matchesSet.add(m) } - } else { - const newItem = createdItem.findItemFilter( - removedItem.type, - item => this.mergeable(removedItem, item), - item => item.childrenSimilarity(removedItem) - ) - let index - if (newItem) { - this.result.REMOVE.retract(removeAction) - if (newItem === createdItem) { - this.result.CREATE.retract(createAction) - } else { - // We clone the item here, because we don't want to mutate all copies of this tree (item) - const createdItemClone = createdItem.copy(true) - const newParentClone = createdItemClone.findItem(ItemType.FOLDER, newItem.parentId) as Folder - const newClonedItem = createdItemClone.findItem(newItem.type, newItem.id) - index = newParentClone.children.indexOf(newClonedItem) - newParentClone.children.splice(index, 1) - createAction.payload = createdItemClone - createAction.payload.createIndex() - } - this.result.MOVE.commit({ - type: ActionType.MOVE, - payload: newItem, - oldItem: removedItem, - index: index || createAction.index, - oldIndex: removeAction.index - }) - reconciled = true - if (removedItem.type === ItemType.FOLDER) { - await this.diffItem(removedItem, newItem) - } + } + if (matchesSet.size > 0) { + break + } + } + + if (matchesSet.size === 0) { + continue + } + + const matches = Array.from(matchesSet) + + // Heuristic: Prefer matches that have more descendants + // In case we have no cache: Calculate similarity and sore by it + matches.sort((a, b) => { + if (createdItem.type === 'folder' && !this.hasCache) { + const simA = a.item.childrenSimilarity(createdItem) + const simB = b.item.childrenSimilarity(createdItem) + if (simA !== simB) return simB - simA + } + return ( + b.item.countFolders() * 1000 + + b.item.count() - + (a.item.countFolders() * 1000 + a.item.count()) + ) + }) + + if (matches.length === 0) { + continue + } + + const { rootAction: removeRootAction, item: oldItem } = matches[0] + const removedRoot = removeRootAction.payload + + let oldIndex, newIndex + + // Retract or Mutate "Old" (Removed) side + if (oldItem === removedRoot) { + this.result.REMOVE.retract(removeRootAction) + } else { + const clone = (removedRoot as Folder).clone(true) + const parentClone = clone.findItem( + ItemType.FOLDER, + oldItem.parentId + ) as Folder + const itemClone = clone.findItem(oldItem.type, oldItem.id) + if (parentClone && itemClone) { + oldIndex = parentClone.children.indexOf(itemClone) + parentClone.children.splice(oldIndex, 1) + clone.createIndex() + removeRootAction.payload = clone + } + } + + // Retract Created side + this.result.CREATE.retract(createAction) + + this.result.MOVE.commit({ + type: ActionType.MOVE, + payload: createdItem, + oldItem, + index: newIndex ?? createAction.index, + oldIndex: oldIndex ?? removeRootAction.index, + }) + + await this.diffItem(oldItem, createdItem) + // After diffing we need to start from scratch + // to make sure we match the newly created actions + if (oldItem instanceof Folder) { + hasNewActions = true + break + } + } + + if (hasNewActions) { + continue + } + + // Then find subtree matches + + // We enumerate all created items + for (const action of createActions) { + allCreatedItems.push({ rootAction: action, item: action.payload }) + if (action.payload instanceof Folder) { + await action.payload.traverse((child) => { + allCreatedItems.push({ rootAction: action, item: child }) + }) + } + } + + allCreatedItems + .sort((a, b) => b.item.count() - a.item.count()) + + // Match ALL created items (roots + descendants) against removed pool + for (const createdEntry of allCreatedItems) { + if (++iterations % 1000 === 0) { + await yieldToEventLoop() + } + const { rootAction: createRootAction, item: createdItem } = createdEntry + + // Gather potential matches from all signals + const searchKeys = [ + `${createdItem.type}_${createdItem.title}_${createdItem.type === 'bookmark' ? (createdItem as Bookmark).url : ''}`, + `${createdItem.type}_title_${createdItem.title}`, + ...(createdItem instanceof Bookmark ? [`bookmark_url_${createdItem.url}`] : []), + createdItem.type + ] + + // Collect unique potential matches from all signals + const matchesSet = new Set<{ rootAction: RemoveAction; item: TItem }>() + for (const key of searchKeys) { + const list = removedFuzzyMap.get(key) + if (!list) continue + for (const m of list) { + if (this.mergeable(m.item, createdItem)) { + matchesSet.add(m) } } + if (matchesSet.size > 0) { + break + } + } + + if (matchesSet.size === 0) { + continue + } + + const matches = Array.from(matchesSet) + + // Heuristic: Prefer matches that have more descendants + // In case we have no cache: Calculate similarity and sore by it + matches.sort((a, b) => { + if (createdItem.type === 'folder' && !this.hasCache) { + const simA = a.item.childrenSimilarity(createdItem) + const simB = b.item.childrenSimilarity(createdItem) + if (simA !== simB) return simB - simA + } + return (b.item.countFolders() * 1000 + b.item.count()) - (a.item.countFolders() * 1000 + a.item.count()) + }) + + if (matches.length === 0) { + continue + } + + const { rootAction: removeRootAction, item: oldItem } = matches[0] + const removedRoot = removeRootAction.payload + const createdRoot = createRootAction.payload + + let oldIndex, newIndex + + // Retract or Mutate "Old" (Removed) side + if (oldItem === removedRoot) { + this.result.REMOVE.retract(removeRootAction) + } else { + const clone = (removedRoot as Folder).clone(true) + const parentClone = clone.findItem(ItemType.FOLDER, oldItem.parentId) as Folder + const itemClone = clone.findItem(oldItem.type, oldItem.id) + if (parentClone && itemClone) { + oldIndex = parentClone.children.indexOf(itemClone) + parentClone.children.splice(oldIndex, 1) + clone.createIndex() + removeRootAction.payload = clone + } + } + + // Retract or Mutate "New" (Created) side + if (createdItem === createdRoot) { + this.result.CREATE.retract(createRootAction) + } else { + const clone = (createdRoot as Folder).clone(true) + const parentClone = clone.findItem(ItemType.FOLDER, createdItem.parentId) as Folder + const itemClone = clone.findItem(createdItem.type, createdItem.id) + if (parentClone && itemClone) { + newIndex = parentClone.children.indexOf(itemClone) + parentClone.children.splice(newIndex, 1) + clone.createIndex() + createRootAction.payload = clone + } + } + + this.result.MOVE.commit({ + type: ActionType.MOVE, + payload: createdItem, + oldItem, + index: newIndex ?? createRootAction.index, + oldIndex: oldIndex ?? removeRootAction.index, + }) + + await this.diffItem(oldItem, createdItem) + // After diffing we need to start from scratch + // to make sure we match the newly created actions + if (oldItem instanceof Folder) { + hasNewActions = true + break } } } diff --git a/src/lib/Tree.ts b/src/lib/Tree.ts index 7907362ff4..468d3e0467 100644 --- a/src/lib/Tree.ts +++ b/src/lib/Tree.ts @@ -39,6 +39,7 @@ export class Bookmark { public location: L public isRoot = false private hashValue: Record + public index: IItemIndex constructor({ id, @@ -111,7 +112,7 @@ export class Bookmark { if (!this.hashValue) { this.hashValue = {} } - if (typeof this.hashValue[hashFn] === 'undefined') { + if (typeof this.hashValue[hashFn] === 'undefined' || this.hashValue[hashFn] === null) { const json = JSON.stringify({ title: this.title, url: this.url }) if (hashFn === 'sha256') { this.hashValue[hashFn] = await Crypto.sha256(json) @@ -194,8 +195,9 @@ export class Bookmark { return result } - createIndex(): any { - return { [this.id]: this } + createIndex(): IItemIndex { + this.index = { bookmark: {[this.id]: this}, folder: {} } + return this.index } // TODO: Make this return the correct type based on the type param @@ -222,6 +224,10 @@ export class Bookmark { return 1 } + countFolders(): number { + return 0 + } + inspect(depth = 0): string { return ( Array(depth < 0 ? 0 : depth) @@ -258,7 +264,7 @@ export class Folder { public isRoot = false public loaded = true public location: L - private index: IItemIndex + public index: IItemIndex constructor({ id, @@ -363,7 +369,7 @@ export class Folder { ): Promise { await Parallel.each( this.children, - async (item) => { + async(item) => { await fn(item, this) if (item.type === 'folder') { // give the browser time to breathe @@ -385,15 +391,10 @@ export class Folder { childrenSimilarity(otherItem: TItem): number { if (otherItem instanceof Folder) { - return ( - this.children.reduce( - (count, item) => - otherItem.children.find((i) => i.title === item.title) - ? count + 1 - : count, - 0 - ) / Math.max(this.children.length, otherItem.children.length) - ) + const myChildrenTitles = new Set(this.children.map((child) => child.title)) + const otherChildrenTitles = new Set(otherItem.children.map((child) => child.title)) + const overlappingTitles = new Set([...myChildrenTitles].filter((title) => otherChildrenTitles.has(title))) + return overlappingTitles.size / Math.max(myChildrenTitles.size, otherChildrenTitles.size) } return 0 } @@ -419,6 +420,8 @@ export class Folder { throw new Error("Trying to calculate hash of a folder that isn't loaded") } + await yieldToEventLoop() + const children = this.children.slice() if (!preserveOrder) { // only re-sort unless we sync the order of the children as well @@ -514,6 +517,9 @@ export class Folder { Object.entries(obj).forEach(([key, value]) => { if (key === 'index') return if (!(key in result)) { + if (key === 'children') { + value = value.map((child) => child.toJSON()) + } result[key] = value } }) @@ -560,24 +566,76 @@ export class Folder { createIndex(): IItemIndex { this.index = { folder: { [this.id]: this }, - bookmark: this.children - .filter((child) => child instanceof Bookmark) - .reduce((obj, child) => { - obj[child.id] = child - return obj - }, {}), + bookmark: {} } - this.children - .filter((child) => child instanceof Folder) - .map((child) => child.createIndex()) - .forEach((subIndex) => { + for (const child of this.children) { + if (child instanceof Bookmark) { + this.index.bookmark[child.id] = child + } else if (child instanceof Folder) { + const subIndex = child.createIndex() Object.assign(this.index.folder, subIndex.folder) Object.assign(this.index.bookmark, subIndex.bookmark) - }) + } + } + return this.index } + /** + * Update the index with the given item (this method should be called on the root folder) + */ + updateIndex(item: TItem) { + if (!item) { + return + } + if (!this.index) { + this.createIndex() + return + } + const itemIndex = item.index || item.createIndex() + let currentItem = this.index.folder[item.parentId] + while (currentItem) { + if (currentItem.index) { + Object.assign(currentItem.index.folder, itemIndex.folder) + Object.assign(currentItem.index.bookmark, itemIndex.bookmark) + } else { + currentItem.createIndex() + } + currentItem = this.index.folder[currentItem.parentId] + } + } + + /** + * Update the index by removing the given item and its children (this method should be called on the root folder) + */ + removeFromIndex(item: TItem) { + if (!item) return + if (!this.index) { + this.createIndex() + return + } + if (!item.index) { + item.createIndex() + } + if (item.parentId) { + let parentFolder = this.index.folder[item.parentId] + while (parentFolder && this.index.folder[parentFolder.parentId] !== parentFolder) { + if (item instanceof Bookmark) { + delete parentFolder.index.bookmark[item.id] + } else { + for (const folderId in item.index.folder) { + delete parentFolder.index.folder[folderId] + } + for (const bookmarkId in item.index.bookmark) { + delete parentFolder.index.bookmark[bookmarkId] + } + } + parentFolder = this.index.folder[parentFolder.parentId] + } + } + } + inspect(depth = 0): string { return ( Array(depth < 0 ? 0 : depth) @@ -618,13 +676,13 @@ export class Folder { ...obj, children: obj.children ? obj.children.map((child) => { - // Firefox seems to set 'url' even for folders - if ('url' in child && typeof child.url === 'string') { - return Bookmark.hydrate(child) - } else { - return Folder.hydrate(child) - } - }) + // Firefox seems to set 'url' even for folders + if ('url' in child && typeof child.url === 'string') { + return Bookmark.hydrate(child) + } else { + return Folder.hydrate(child) + } + }) : null, }) } diff --git a/src/lib/adapters/Caching.ts b/src/lib/adapters/Caching.ts index 43bd1ffaf9..63be224c9a 100644 --- a/src/lib/adapters/Caching.ts +++ b/src/lib/adapters/Caching.ts @@ -53,13 +53,13 @@ export default class CachingAdapter implements Adapter, BulkImportResource):Promise { Logger.log('CREATE', bm) - bm = bm.copy() + bm = bm.copyWithLocation(true, this.location) bm.id = ++this.highestId const foundFolder = this.bookmarksCache.findFolder(bm.parentId) if (!foundFolder) { throw new UnknownCreateTargetError() } foundFolder.children.push(bm) - this.bookmarksCache.createIndex() + this.bookmarksCache.updateIndex(bm) return bm.id } @@ -109,9 +109,11 @@ export default class CachingAdapter implements Adapter, BulkImportResource): Promise { @@ -131,7 +133,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource): Promise { @@ -142,7 +144,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource):Promise { @@ -179,15 +182,12 @@ export default class CachingAdapter implements Adapter, BulkImportResource { const child = folder.findItem(item.type, item.id) if (!child || String(child.parentId) !== String(folder.id)) { throw new UnknownFolderItemOrderError(id + ':' + JSON.stringify(item)) } - }) - let newChildren = [] - order.forEach(item => { - const child = folder.findItem(item.type, item.id) newChildren.push(child) }) const diff = difference(folder.children.map(i => i.type + ':' + i.id), order.map(i => i.type + ':' + i.id)) @@ -198,8 +198,9 @@ export default class CachingAdapter implements Adapter, BulkImportResource { const [type, id] = item.split(':') const child = folder.findItem(type, id) + if (!child) return const index = folder.children.indexOf(child) - newChildren = newChildren.slice(0, index - 1).concat([child], newChildren.slice(index - 1)) + newChildren = newChildren.slice(0, index).concat([child], newChildren.slice(index)) }) } folder.children = newChildren @@ -218,7 +219,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource):Promise> { @@ -237,7 +238,8 @@ export default class CachingAdapter implements Adapter, BulkImportResource):Promise { @@ -573,7 +580,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes parent.children = parent.children.filter( (child) => String(child.id) !== String(id) ) - this.tree.createIndex() + this.tree.removeFromIndex(oldFolder) } } @@ -691,7 +698,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes this.list && this.list.push(upstreamMark) if (this.tree) { newParentFolder.children.push(upstreamMark) - this.tree.createIndex() + this.tree.updateIndex(upstreamMark) } return bm.id @@ -742,11 +749,22 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes throw e } + const oldParentFolder = this.tree.findFolder(oldParentId) + if (!oldParentFolder) { + throw new UnknownFolderParentUpdateError() + } + const oldBm = oldParentFolder.findBookmark(newBm.id) + oldParentFolder.children = oldParentFolder.children.filter( + (item) => !(item.type === 'bookmark' && item.id === newBm.id) + ) + if (oldBm && this.tree) { + this.tree.removeFromIndex(oldBm) + } if (!newFolder.children.find(item => String(item.id) === String(newBm.id) && item.type === 'bookmark')) { newFolder.children.push(newBm) } newBm.id = upstreamId + ';' + newBm.parentId - this.tree.createIndex() + this.tree.updateIndex(newBm) return newBm.id }) diff --git a/src/lib/browser/BrowserAccountStorage.js b/src/lib/browser/BrowserAccountStorage.js index 9862979ce6..03d2cefe76 100644 --- a/src/lib/browser/BrowserAccountStorage.js +++ b/src/lib/browser/BrowserAccountStorage.js @@ -128,9 +128,9 @@ export default class BrowserAccountStorage { } async initCache() { - await BrowserAccountStorage.changeEntry( + await BrowserAccountStorage.setEntry( `bookmarks[${this.accountId}].cache`, - () => ({}) + {} ) } @@ -144,9 +144,9 @@ export default class BrowserAccountStorage { } async setCache(data) { - await BrowserAccountStorage.changeEntry( + await BrowserAccountStorage.setEntry( `bookmarks[${this.accountId}].cache`, - () => data + data.toJSON ? data.toJSON() : data ) } @@ -157,9 +157,9 @@ export default class BrowserAccountStorage { } async initMappings() { - await BrowserAccountStorage.changeEntry( + await BrowserAccountStorage.setEntry( `bookmarks[${this.accountId}].mappings`, - () => ({}) + {} ) } @@ -185,9 +185,9 @@ export default class BrowserAccountStorage { } async setMappings(data) { - await BrowserAccountStorage.changeEntry( + await BrowserAccountStorage.setEntry( `bookmarks[${this.accountId}].mappings`, - () => data + data ) } diff --git a/src/lib/browser/BrowserTree.ts b/src/lib/browser/BrowserTree.ts index e9062be500..117e5e9c15 100644 --- a/src/lib/browser/BrowserTree.ts +++ b/src/lib/browser/BrowserTree.ts @@ -366,7 +366,7 @@ export default class BrowserTree implements IResource } static async getIdPathFromLocalId(localId:string|null, path:string[] = []):Promise { - if (typeof localId === 'undefined') { + if (typeof localId === 'undefined' || localId === null) { return path } path.unshift(localId) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index bbde5702bc..223a244db1 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -35,7 +35,7 @@ import NextcloudBookmarksAdapter from '../adapters/NextcloudBookmarks' import CachingAdapter from '../adapters/Caching' import { yieldToEventLoop } from '../yieldToEventLoop' -const ACTION_CONCURRENCY = 12 +export const ACTION_CONCURRENCY = 5 export default class SyncProcess { protected mappings: Mappings @@ -54,24 +54,24 @@ export default class SyncProcess { protected serverScanResult: ScanResult = null // Stage 1 - private localPlanStage1: PlanStage1 - private serverPlanStage1: PlanStage1 + private localPlanStage1: PlanStage1 = null + private serverPlanStage1: PlanStage1 = null // Stage 2 - private localPlanStage2: PlanStage2 + private localPlanStage2: PlanStage2 = null private serverPlanStage2: PlanStage2 // Stage 3 - private planStage3Local: PlanStage3 - private planStage3Server: PlanStage3 - private localDonePlan: PlanStage3 - private serverDonePlan: PlanStage3 - private prelimLocalReorders: Diff> - private prelimServerReorders: Diff> + private planStage3Local: PlanStage3 = null + private planStage3Server: PlanStage3 = null + private localDonePlan: PlanStage3 = null + private serverDonePlan: PlanStage3 = null + private prelimLocalReorders: Diff> = null + private prelimServerReorders: Diff> = null // Stage 4 - private localReorders: Diff> - private serverReorders: Diff> + private localReorders: Diff> = null + private serverReorders: Diff> = null protected actionsDone = 0 protected actionsPlanned = 0 @@ -106,31 +106,48 @@ export default class SyncProcess { } getMembersToPersist() { - return [ - // Stage 0 - 'localScanResult', - 'serverScanResult', - - // Stage 1 - 'localPlanStage1', - 'serverPlanStage1', - - // Stage 2 - 'localPlanStage2', - 'serverPlanStage2', - - // Stage 3 - 'planStage3Local', - 'planStage3Server', - 'localDonePlan', - 'serverDonePlan', - 'prelimLocalReorders', - 'prelimServerReorders', - - // Stage 4 - 'localReorders', - 'serverReorders', - ] + const members = [] + // Stage 0 + if ( + (!this.serverPlanStage1 || !this.localPlanStage1) && + (!this.serverPlanStage2 || !this.localPlanStage2) && + (!this.planStage3Local || !this.planStage3Server) && + this.actionsPlanned === 0 + ) { + members.push('localScanResult') + members.push('serverScanResult') + } + + // Stage 1 + if ( + (!this.serverPlanStage2 || !this.localPlanStage2) && + (!this.planStage3Local || !this.planStage3Server) && this.actionsPlanned === 0 + ) { + members.push('localPlanStage1') + members.push('serverPlanStage1') + } + + // Stage 2 + if ((!this.planStage3Local || !this.planStage3Server) && this.actionsPlanned === 0) { + members.push('localPlanStage2') + members.push('serverPlanStage2') + } + + // Stage 3 + if (this.actionsDone < this.actionsPlanned) { + members.push('planStage3Local') + members.push('planStage3Server') + members.push('localDonePlan') + members.push('serverDonePlan') + members.push('prelimLocalReorders') + members.push('prelimServerReorders') + } + + // Stage 4 + members.push('localReorders') + members.push('serverReorders') + + return members } getMappingsInstance(): Mappings { @@ -158,7 +175,7 @@ export default class SyncProcess { } async updateProgress():Promise { - if (typeof this.actionsDone === 'undefined') { + if (typeof this.actionsDone === 'undefined' || this.actionsDone === null) { this.actionsDone = 0 } this.actionsDone++ @@ -177,21 +194,34 @@ export default class SyncProcess { Logger.log(`Executed ${this.actionsDone} actions from ${this.actionsPlanned} actions`) } - setProgress({actionsDone, actionsPlanned}: {actionsDone: number, actionsPlanned: number}) { - this.actionsDone = actionsDone - this.actionsPlanned = actionsPlanned - this.throttledProgressCb( - Math.min( - 1, - 0.5 + (this.actionsDone / (this.actionsPlanned + 1)) * 0.5 - ), - this.actionsDone - ).catch((er) => { - if (er instanceof CanceledError) { - return + async setProgress(json: any) { + if (json.serverTreeRoot) { + this.serverTreeRoot = Folder.hydrate(json.serverTreeRoot) + delete json.serverTreeRoot + } + if (json.localTreeRoot) { + this.localTreeRoot = Folder.hydrate(json.localTreeRoot) + delete json.localTreeRoot + } + if (json.cacheTreeRoot) { + this.cacheTreeRoot = Folder.hydrate(json.cacheTreeRoot) + delete json.cacheTreeRoot + } + for (const member of Object.keys(json)) { + if (member.toLowerCase().includes('scanresult') || member.toLowerCase().includes('plan')) { + this[member] = { + CREATE: await Diff.fromJSONAsync(json[member].CREATE), + UPDATE: await Diff.fromJSONAsync(json[member].UPDATE), + MOVE: await Diff.fromJSONAsync(json[member].MOVE), + REMOVE: await Diff.fromJSONAsync(json[member].REMOVE), + REORDER: await Diff.fromJSONAsync(json[member].REORDER), + } + } else if (member.toLowerCase().includes('reorders')) { + this[member] = await Diff.fromJSONAsync(json[member]) + } else { + this[member] = json[member] } - throw er - }) + } } setDirection(direction:TItemLocation):void { @@ -224,7 +254,7 @@ export default class SyncProcess { Logger.log({localTreeRoot: this.localTreeRoot, serverTreeRoot: this.serverTreeRoot, cacheTreeRoot: this.cacheTreeRoot}) - if (!this.localScanResult && !this.serverScanResult) { + if (!this.localScanResult && !this.serverScanResult && !this.localPlanStage1 && !this.serverPlanStage1 && !this.localPlanStage2 && !this.serverPlanStage2 && !this.planStage3Local && !this.planStage3Server) { const { localScanResult, serverScanResult } = await this.getDiffs() Logger.log({ localScanResult, serverScanResult }) this.localScanResult = localScanResult @@ -241,21 +271,14 @@ export default class SyncProcess { throw new CancelledSyncError() } - if (!this.serverPlanStage1) { + if (!this.serverPlanStage1 && !this.localPlanStage1 && !this.serverPlanStage2 && !this.localPlanStage2 && !this.planStage3Local && !this.planStage3Server) { this.serverPlanStage1 = await this.reconcileDiffs(this.localScanResult, this.serverScanResult, ItemLocation.SERVER) - } - - if (this.canceled) { - throw new CancelledSyncError() - } - - if (!this.localPlanStage1) { this.localPlanStage1 = await this.reconcileDiffs(this.serverScanResult, this.localScanResult, ItemLocation.LOCAL) } let mappingsSnapshot: MappingSnapshot - if (!this.serverPlanStage2) { + if (!this.serverPlanStage2 && !this.localPlanStage2 && !this.planStage3Local && !this.planStage3Server) { // have to get snapshot after reconciliation, because of concurrent creation reconciliation mappingsSnapshot = this.mappings.getSnapshot() Logger.log('Mapping server plan') @@ -267,15 +290,7 @@ export default class SyncProcess { REMOVE: this.serverPlanStage1.REMOVE.map(mappingsSnapshot, ItemLocation.SERVER), REORDER: this.serverPlanStage1.REORDER, } - } - if (this.canceled) { - throw new CancelledSyncError() - } - - if (!this.localPlanStage2) { - // have to get snapshot after reconciliation, because of concurrent creation reconciliation - if (!mappingsSnapshot) mappingsSnapshot = this.mappings.getSnapshot() Logger.log('Mapping local plan') this.localPlanStage2 = { @@ -293,10 +308,15 @@ export default class SyncProcess { Logger.log({localPlan: this.localPlanStage2, serverPlan: this.serverPlanStage2}) - this.applyDeletionFailsafe(ItemLocation.LOCAL, this.localTreeRoot, this.localPlanStage2.REMOVE) - this.applyAdditionFailsafe(ItemLocation.LOCAL, this.localTreeRoot, this.localPlanStage2.CREATE) - this.applyDeletionFailsafe(ItemLocation.SERVER, this.serverTreeRoot, this.serverPlanStage2.REMOVE) - this.applyAdditionFailsafe(ItemLocation.SERVER, this.serverTreeRoot, this.serverPlanStage2.CREATE) + if (this.serverPlanStage2) { + this.applyDeletionFailsafe(ItemLocation.SERVER, this.serverTreeRoot, this.serverPlanStage2.REMOVE) + this.applyAdditionFailsafe(ItemLocation.SERVER, this.serverTreeRoot, this.serverPlanStage2.CREATE) + } + + if (this.localPlanStage2) { + this.applyDeletionFailsafe(ItemLocation.LOCAL, this.localTreeRoot, this.localPlanStage2.REMOVE) + this.applyAdditionFailsafe(ItemLocation.LOCAL, this.localTreeRoot, this.localPlanStage2.CREATE) + } if (!this.localDonePlan) { this.localDonePlan = { @@ -316,18 +336,20 @@ export default class SyncProcess { } } - if (!this.prelimLocalReorders) { + if (!this.prelimLocalReorders && this.localPlanStage2) { this.prelimLocalReorders = this.localPlanStage2.REORDER this.prelimServerReorders = this.serverPlanStage2.REORDER } if (!this.actionsPlanned) { - this.actionsPlanned = Object.values(this.serverPlanStage2).reduce((acc, diff) => diff.getActions().length + acc, 0) + - Object.values(this.localPlanStage2).reduce((acc, diff) => diff.getActions().length + acc, 0) + this.actionsPlanned = Object.values(this.serverPlanStage2 || this.planStage3Server).reduce((acc, diff) => diff.getActions().length + acc, 0) + + Object.values(this.localPlanStage2 || this.planStage3Local).reduce((acc, diff) => diff.getActions().length + acc, 0) } - Logger.log('Executing server stage2 plan') - await this.executeStage2(this.server, this.serverPlanStage2, ItemLocation.SERVER, this.serverDonePlan, this.prelimServerReorders) + if (this.serverPlanStage2) { + Logger.log('Executing server stage2 plan') + await this.executeStage2(this.server, this.serverPlanStage2, ItemLocation.SERVER, this.serverDonePlan, this.prelimServerReorders) + } if (!this.planStage3Server) { if (this.canceled) { @@ -349,15 +371,19 @@ export default class SyncProcess { } } - Logger.log('Executing local stage 3 plan') - await this.executeStage3(this.server, this.planStage3Server, ItemLocation.SERVER, this.serverDonePlan) + if (this.planStage3Server) { + Logger.log('Executing server stage 3 plan') + await this.executeStage3(this.server, this.planStage3Server, ItemLocation.SERVER, this.serverDonePlan) + } if (this.canceled) { throw new CancelledSyncError() } - Logger.log('Executing local stage 2 plan') - await this.executeStage2(this.localTree, this.localPlanStage2, ItemLocation.LOCAL, this.localDonePlan, this.prelimLocalReorders) + if (this.localPlanStage2) { + Logger.log('Executing local stage 2 plan') + await this.executeStage2(this.localTree, this.localPlanStage2, ItemLocation.LOCAL, this.localDonePlan, this.prelimLocalReorders) + } if (!this.planStage3Local) { if (this.canceled) { @@ -379,8 +405,10 @@ export default class SyncProcess { } } - Logger.log('Executing local stage 3 plan') - await this.executeStage3(this.localTree, this.planStage3Local, ItemLocation.LOCAL, this.localDonePlan) + if (this.planStage3Local) { + Logger.log('Executing local stage 3 plan') + await this.executeStage3(this.localTree, this.planStage3Local, ItemLocation.LOCAL, this.localDonePlan) + } // Remove mappings only after both plans have been executed await Parallel.map(this.localDonePlan.REMOVE.getActions(), async(action) => @@ -486,7 +514,11 @@ export default class SyncProcess { // Failsafe kicks in if more than 20% is deleted or more than 1k bookmarks if ((countTotal > 5 && countDeleted / countTotal > 0.2) || countDeleted > 1000) { const failsafe = this.server.getData().failsafe - if (failsafe !== false || typeof failsafe === 'undefined') { + if ( + failsafe !== false || + typeof failsafe === 'undefined' || + failsafe === null + ) { const percentage = Math.ceil((countDeleted / countTotal) * 100) if (direction === ItemLocation.LOCAL) { throw new ClientsideDeletionFailsafeError(percentage) @@ -505,7 +537,7 @@ export default class SyncProcess { // Failsafe kicks in if more than 20% is added or more than 1k bookmarks if (countTotal > 5 && ((countAdded >= 20 && countAdded / countTotal > 0.2) || countAdded > 1000)) { const failsafe = this.server.getData().failsafe - if (failsafe !== false || typeof failsafe === 'undefined') { + if (failsafe !== false || typeof failsafe === 'undefined' || failsafe === null) { const percentage = Math.ceil((countAdded / countTotal) * 100) if (direction === ItemLocation.LOCAL) { throw new ClientsideAdditionFailsafeError(percentage) @@ -1011,7 +1043,7 @@ export default class SyncProcess { action.payload.visitCreate(resource), this.cancelPromise ]) - if (typeof id === 'undefined') { + if (typeof id === 'undefined' || id === null) { // undefined means we couldn't create the item. we're ignoring it await done() return @@ -1349,6 +1381,7 @@ export default class SyncProcess { const isUsingTabs = await this.localTree.isUsingBrowserTabs?.() await Parallel.each(reorderings.getActions(), async(action) => { + await yieldToEventLoop() Logger.log('Executing reorder action', `${action.type} Payload: #${action.payload.id}[${action.payload.title}]${'url' in action.payload ? `(${action.payload.url})` : ''} parentId: ${action.payload.parentId}`) const item = action.payload @@ -1513,25 +1546,6 @@ export default class SyncProcess { parentReorder.order = parentReorder.order.filter(item => !(item.type === oldItem.type && String(Mappings.mapId(mappingsSnapshot, oldItem, parentReorder.payload.location)) === String(item.id))) } - toJSON(): ISerializedSyncProcess { - if (!this.staticContinuation) { - this.staticContinuation = { - // Do not store these as the continuation size can get huge otherwise - localTreeRoot: null, - cacheTreeRoot: null, - serverTreeRoot: null, - } - } - const membersToPersist = this.getMembersToPersist() - return { - strategy: 'default', - ...this.staticContinuation, - ...(Object.fromEntries(Object.entries(this) - .filter(([key]) => membersToPersist.includes(key))) - ), - } - } - async toJSONAsync(): Promise { if (!this.staticContinuation) { this.staticContinuation = { @@ -1543,23 +1557,36 @@ export default class SyncProcess { } const membersToPersist = this.getMembersToPersist() return { - strategy: 'default', + strategy: 'unidirectional', ...this.staticContinuation, ...(Object.fromEntries( - await Parallel.map( - Object.entries(this) - .filter(([key]) => membersToPersist.includes(key)), - async([key, value]) => { - if ('toJSONAsync' in value) { - return [key, await value.toJSONAsync()] - } - if ('toJSON' in value) { - await yieldToEventLoop() - return [key, value.toJSON()] - } - return [key, value] - }, 1) - ) + await Parallel.map( + Object.entries(this) + .filter(([key]) => membersToPersist.includes(key)), + async([key, value]) => { + if (value && value.CREATE && value.REMOVE && value.UPDATE && value.MOVE && value.REORDER) { + // property holds a Plan + return [key, Object.fromEntries(await Parallel.map(Object.entries(value), async([key, diff]: [string, Diff>]) => { + if (diff && diff.toJSONAsync) { + return [key, await diff.toJSONAsync()] + } + if (diff && diff.toJSON) { + await yieldToEventLoop() + return [key, diff.toJSON()] + } + return [key, diff] + }))] + } + if (value && value.toJSONAsync) { + return [key, await value.toJSONAsync()] + } + if (value && value.toJSON) { + await yieldToEventLoop() + return [key, value.toJSON()] + } + return [key, value] + }, 1) + ) ), } } @@ -1587,34 +1614,7 @@ export default class SyncProcess { default: throw new Error('Unknown strategy: ' + json.strategy) } - strategy.setProgress(json) - if (json.serverTreeRoot) { - strategy.serverTreeRoot = Folder.hydrate(json.serverTreeRoot) - } - if (json.localTreeRoot) { - strategy.localTreeRoot = Folder.hydrate(json.localTreeRoot) - } - if (json.cacheTreeRoot) { - strategy.cacheTreeRoot = Folder.hydrate(json.cacheTreeRoot) - } - strategy.getMembersToPersist().forEach((member) => { - if (member in json) { - if (member.toLowerCase().includes('scanresult') || member.toLowerCase().includes('plan')) { - strategy[member] = { - CREATE: Diff.fromJSON(json[member].CREATE), - UPDATE: Diff.fromJSON(json[member].UPDATE), - MOVE: Diff.fromJSON(json[member].MOVE), - REMOVE: Diff.fromJSON(json[member].REMOVE), - REORDER: Diff.fromJSON(json[member].REORDER), - } - } else if (member.toLowerCase().includes('reorders')) { - strategy[member] = Diff.fromJSON(json[member]) - } else { - strategy[member] = json[member] - } - } - }) - + await strategy.setProgress(json) return strategy } } diff --git a/src/lib/strategies/Merge.ts b/src/lib/strategies/Merge.ts index 0726ba6956..63db482d9b 100644 --- a/src/lib/strategies/Merge.ts +++ b/src/lib/strategies/Merge.ts @@ -2,12 +2,13 @@ import { Folder, ItemLocation, TItem, TItemLocation, TOppositeLocation } from '. import Diff, { CreateAction, MoveAction, PlanStage1 } from '../Diff' import Scanner, { ScanResult } from '../Scanner' import * as Parallel from 'async-parallel' -import DefaultSyncProcess, { ISerializedSyncProcess } from './Default' +import DefaultSyncProcess, { + ISerializedSyncProcess, + ACTION_CONCURRENCY, +} from './Default' import Mappings from '../Mappings' import Logger from '../Logger' -const ACTION_CONCURRENCY = 12 - export default class MergeSyncProcess extends DefaultSyncProcess { async getDiffs(): Promise<{ localScanResult: ScanResult @@ -362,16 +363,9 @@ export default class MergeSyncProcess extends DefaultSyncProcess { ).children } - toJSON(): ISerializedSyncProcess { - return { - ...DefaultSyncProcess.prototype.toJSON.apply(this), - strategy: 'merge', - } - } - async toJSONAsync(): Promise { return { - ...(await DefaultSyncProcess.prototype.toJSON.apply(this)), + ...(await DefaultSyncProcess.prototype.toJSONAsync.apply(this)), strategy: 'merge', } } diff --git a/src/lib/strategies/Unidirectional.ts b/src/lib/strategies/Unidirectional.ts index 136121cce3..d54d408c4b 100644 --- a/src/lib/strategies/Unidirectional.ts +++ b/src/lib/strategies/Unidirectional.ts @@ -1,5 +1,5 @@ -import DefaultStrategy, { ISerializedSyncProcess } from './Default' -import Diff, { ActionType, PlanRevert, PlanStage1, PlanStage3, ReorderAction } from '../Diff' +import DefaultStrategy, { ISerializedSyncProcess , ACTION_CONCURRENCY } from './Default' +import Diff, { Action, ActionType, PlanRevert, PlanStage1, PlanStage3, ReorderAction } from '../Diff' import * as Parallel from 'async-parallel' import Mappings, { MappingSnapshot } from '../Mappings' import { Folder, ItemLocation, TItem, TItemLocation, TOppositeLocation } from '../Tree' @@ -7,59 +7,103 @@ import Logger from '../Logger' import { CancelledSyncError } from '../../errors/Error' import TResource from '../interfaces/Resource' import Scanner, { ScanResult } from '../Scanner' -import DefaultSyncProcess from './Default' - -const ACTION_CONCURRENCY = 12 +import { yieldToEventLoop } from '../yieldToEventLoop' export default class UnidirectionalSyncProcess extends DefaultStrategy { protected direction: TItemLocation + protected scanResult: ScanResult protected revertPlan: PlanStage1< TItemLocation, TOppositeLocation - > + > = null protected revertDonePlan: PlanRevert< TItemLocation, TOppositeLocation - > + > = null protected revertReorders: Diff< TItemLocation, TOppositeLocation, ReorderAction> - > + > = null setDirection(direction: TItemLocation): void { this.direction = direction } getMembersToPersist() { - return [ - // Stage 0 - 'localScanResult', - 'serverScanResult', + const members = [] + // Stage 0 + if (!this.revertPlan && this.actionsPlanned === 0) { + members.push('scanResult') + } + + // Stage 1 + if (this.actionsDone < this.actionsPlanned) { + members.push('revertPlan') + members.push('revertDonePlan') + } - // Stage 1 - 'revertPlan', - 'revertDonePlan', + // Stage 2 + members.push('revertReorders') - // Stage 2 - 'revertReorders', + members.push('direction') + return members + } - 'direction', - ] + async setProgress(json: any) { + if (json.serverTreeRoot) { + this.serverTreeRoot = Folder.hydrate(json.serverTreeRoot) + delete json.serverTreeRoot + } + if (json.localTreeRoot) { + this.localTreeRoot = Folder.hydrate(json.localTreeRoot) + delete json.localTreeRoot + } + if (json.cacheTreeRoot) { + this.cacheTreeRoot = Folder.hydrate(json.cacheTreeRoot) + delete json.cacheTreeRoot + } + for (const member of Object.keys(json)) { + if ( + member.toLowerCase().includes('scanresult') || + member.toLowerCase().includes('plan') + ) { + this[member] = { + CREATE: await Diff.fromJSONAsync(json[member].CREATE), + UPDATE: await Diff.fromJSONAsync(json[member].UPDATE), + MOVE: await Diff.fromJSONAsync(json[member].MOVE), + REMOVE: await Diff.fromJSONAsync(json[member].REMOVE), + REORDER: await Diff.fromJSONAsync(json[member].REORDER), + } + } else if (member.toLowerCase().includes('reorders')) { + this[member] = await Diff.fromJSONAsync(json[member]) + } else { + this[member] = json[member] + } + } } - async getDiffs(): Promise<{ - localScanResult: ScanResult - serverScanResult: ScanResult - }> { + async getDiff(): Promise> { const mappingsSnapshot = this.mappings.getSnapshot() const newMappings = [] - const localScanner = new Scanner( - this.serverTreeRoot, - this.localTreeRoot, + const slaveTree = + this.direction === ItemLocation.SERVER + ? this.serverTreeRoot + : this.localTreeRoot + const masterTree = + this.direction === ItemLocation.SERVER + ? this.localTreeRoot + : this.serverTreeRoot + const scanner = new Scanner( + slaveTree, + masterTree, // We can't rely on a cacheTree, thus we have to accept canMergeWith results as well - (serverItem, localItem) => { + (slaveItem, masterItem) => { + const localItem = + this.direction === ItemLocation.SERVER ? masterItem : slaveItem + const serverItem = + this.direction === ItemLocation.SERVER ? slaveItem : masterItem if (localItem.type !== serverItem.type) { return false } @@ -85,40 +129,10 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { false, false ) - const serverScanner = new Scanner( - this.localTreeRoot, - this.serverTreeRoot, - (localItem, serverItem) => { - if (serverItem.type !== localItem.type) { - return false - } - // If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints - if ( - serverItem.type === 'bookmark' && - localItem.type === 'bookmark' && - serverItem.url !== localItem.url - ) { - return false - } - if (serverItem.canMergeWith(localItem)) { - newMappings.push([localItem, serverItem]) - return true - } - if (Mappings.mappable(mappingsSnapshot, serverItem, localItem)) { - newMappings.push([localItem, serverItem]) - return true - } - return false - }, - this.hashSettings, - false, - false - ) Logger.log( 'Unidirectional: Calculating the diff between local and server trees' ) - const localScanResult = await localScanner.run() - const serverScanResult = await serverScanner.run() + const scanResult = await scanner.run() await Parallel.map( newMappings, ([localItem, serverItem]) => { @@ -127,7 +141,7 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { 1 ) - return { localScanResult, serverScanResult } + return scanResult } async loadChildren( @@ -160,11 +174,9 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { cacheTreeRoot: this.cacheTreeRoot, }) - if (!this.localScanResult && !this.serverScanResult) { - const { localScanResult, serverScanResult } = await this.getDiffs() - Logger.log({ localScanResult, serverScanResult }) - this.localScanResult = localScanResult - this.serverScanResult = serverScanResult + if (!this.scanResult && !this.revertPlan) { + this.scanResult = await this.getDiff() + Logger.log({ scanResult: this.scanResult }) this.throttledProgressCb(0.45, 0) } @@ -172,27 +184,17 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { throw new CancelledSyncError() } - let sourceScanResult: ScanResult, - targetScanResult: ScanResult, - target: TResource + let target: TResource if (this.direction === ItemLocation.SERVER) { - sourceScanResult = this.localScanResult - targetScanResult = this.serverScanResult target = this.server } else { - sourceScanResult = this.serverScanResult - targetScanResult = this.localScanResult target = this.localTree } // First revert slave modifications if (!this.revertPlan) { - this.revertPlan = await this.revertDiff( - targetScanResult, - sourceScanResult, - this.direction - ) + this.revertPlan = await this.revertDiff(this.scanResult, this.direction) Logger.log({ revertPlan: this.revertPlan }) } @@ -200,56 +202,64 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { throw new CancelledSyncError() } - this.actionsPlanned = Object.values(this.revertPlan).reduce( - (acc, diff) => diff.getActions().length + acc, - 0 - ) - - if (this.direction === ItemLocation.LOCAL) { - this.applyDeletionFailsafe( - ItemLocation.LOCAL, - this.localTreeRoot, - this.revertPlan.REMOVE - ) - this.applyAdditionFailsafe( - ItemLocation.LOCAL, - this.localTreeRoot, - this.revertPlan.CREATE - ) - } else { - this.applyDeletionFailsafe( - ItemLocation.SERVER, - this.serverTreeRoot, - this.revertPlan.REMOVE - ) - this.applyAdditionFailsafe( - ItemLocation.SERVER, - this.serverTreeRoot, - this.revertPlan.CREATE + if (!this.actionsPlanned && this.revertPlan) { + this.actionsPlanned = Object.values(this.revertPlan).reduce( + (acc, diff) => diff.getActions().length + acc, + 0 ) } + if (this.revertPlan) { + if (this.direction === ItemLocation.LOCAL) { + this.applyDeletionFailsafe( + ItemLocation.LOCAL, + this.localTreeRoot, + this.revertPlan.REMOVE + ) + this.applyAdditionFailsafe( + ItemLocation.LOCAL, + this.localTreeRoot, + this.revertPlan.CREATE + ) + } else { + this.applyDeletionFailsafe( + ItemLocation.SERVER, + this.serverTreeRoot, + this.revertPlan.REMOVE + ) + this.applyAdditionFailsafe( + ItemLocation.SERVER, + this.serverTreeRoot, + this.revertPlan.CREATE + ) + } + } + if (this.canceled) { throw new CancelledSyncError() } - Logger.log('Executing ' + this.direction + ' revert plan') + if (this.revertPlan) { + Logger.log('Executing ' + this.direction + ' revert plan') - this.revertDonePlan = { - CREATE: new Diff(), - UPDATE: new Diff(), - MOVE: new Diff(), - REMOVE: new Diff(), - REORDER: new Diff(), - } + if (!this.revertDonePlan) { + this.revertDonePlan = { + CREATE: new Diff(), + UPDATE: new Diff(), + MOVE: new Diff(), + REMOVE: new Diff(), + REORDER: new Diff(), + } + } - await this.executeRevert( - target, - this.revertPlan, - this.direction, - this.revertDonePlan, - sourceScanResult.REORDER - ) + await this.executeRevert( + target, + this.revertPlan, + this.direction, + this.revertDonePlan, + this.scanResult.REORDER + ) + } if (this.direction === ItemLocation.LOCAL) { this.revertDonePlan.REMOVE.getActions().forEach((action) => @@ -261,16 +271,16 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { ) } - if ('orderFolder' in this.server && !this.revertReorders) { + if ('orderFolder' in target && !this.revertReorders) { const mappingsSnapshot = this.mappings.getSnapshot() Logger.log('Mapping reorderings') - this.revertReorders = sourceScanResult.REORDER.map( + this.revertReorders = this.scanResult.REORDER.map( mappingsSnapshot, this.direction ) } - if ('orderFolder' in this.server && 'orderFolder' in target) { + if (this.revertReorders && 'orderFolder' in target) { await this.executeReorderings(target, this.revertReorders) } @@ -278,8 +288,7 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { } async revertDiff( - targetScanResult: ScanResult, - sourceScanResult: ScanResult, + scanResult: ScanResult, targetLocation: L1 ): Promise> { const mappingsSnapshot = this.mappings.getSnapshot() @@ -289,14 +298,14 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { UPDATE: new Diff(), MOVE: new Diff(), REMOVE: new Diff(), - REORDER: targetScanResult.REORDER.clone(), + REORDER: scanResult.REORDER.clone(), } - // Prepare slave plan for reversing slave changes + // Prepare slave plan for matching master state await Parallel.each( - sourceScanResult.CREATE.getActions(), - async (action) => { + scanResult.CREATE.getActions(), + async(action) => { // recreate it on slave resource otherwise const payload = await this.translateCompleteItem( action.payload, @@ -318,44 +327,38 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { ) await Parallel.each( - targetScanResult.CREATE.getActions(), - async (action) => { + scanResult.REMOVE.getActions(), + async(action) => { slavePlan.REMOVE.commit({ ...action, type: ActionType.REMOVE }) }, ACTION_CONCURRENCY ) await Parallel.each( - targetScanResult.UPDATE.getActions(), - async (action) => { - const payload = action.oldItem.cloneWithLocation( + scanResult.UPDATE.getActions(), + async(action) => { + const payload = action.payload.cloneWithLocation( false, - action.payload.location + action.oldItem.location ) - payload.id = action.payload.id - payload.parentId = action.payload.parentId + payload.id = action.oldItem.id + payload.parentId = action.oldItem.parentId - const oldItem = action.payload.cloneWithLocation( + const oldItem = action.oldItem.cloneWithLocation( false, - action.oldItem.location + action.payload.location ) - oldItem.id = action.oldItem.id - oldItem.parentId = action.oldItem.parentId + oldItem.id = action.payload.id + oldItem.parentId = action.payload.parentId slavePlan.UPDATE.commit({ type: ActionType.UPDATE, payload, oldItem }) }, ACTION_CONCURRENCY ) await Parallel.each( - targetScanResult.MOVE.getActions(), - async (action) => { - const payload = action.payload.cloneWithLocation( - false, - action.oldItem.location - ) - payload.id = action.oldItem.id - payload.parentId = action.oldItem.parentId - + scanResult.MOVE.getActions(), + async(action) => { + const payload = action.payload.clone(false) slavePlan.MOVE.commit({ type: ActionType.MOVE, payload }) // no oldItem, because we want to map the id after having executed the CREATEs }, ACTION_CONCURRENCY @@ -377,9 +380,9 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { ) if (newItem instanceof Folder) { const nonexistingItems = [] - await newItem.traverse(async (child, parentFolder) => { + await newItem.traverse(async(child, parentFolder) => { child.id = Mappings.mapId(mappingsSnapshot, child, fakeLocation) - if (typeof child.id === 'undefined') { + if (typeof child.id === 'undefined' || child.id === null) { nonexistingItems.push(child) } child.parentId = parentFolder.id @@ -508,17 +511,48 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { ) } - toJSON(): ISerializedSyncProcess { - return { - ...DefaultSyncProcess.prototype.toJSON.apply(this), - strategy: 'unidirectional', - } - } - async toJSONAsync(): Promise { + if (!this.staticContinuation) { + this.staticContinuation = { + // Do not store these as the continuation size can get huge otherwise + localTreeRoot: null, + cacheTreeRoot: null, + serverTreeRoot: null, + } + } + const membersToPersist = this.getMembersToPersist() return { - ...(await DefaultSyncProcess.prototype.toJSON.apply(this)), strategy: 'unidirectional', + ...this.staticContinuation, + ...(Object.fromEntries( + await Parallel.map( + Object.entries(this) + .filter(([key]) => membersToPersist.includes(key)), + async([key, value]) => { + if (value && value.CREATE && value.REMOVE && value.UPDATE && value.MOVE && value.REORDER) { + // property holds a Plan + return [key, Object.fromEntries(await Parallel.map(Object.entries(value), async([key, diff]: [string, Diff>]) => { + if (diff && diff.toJSONAsync) { + return [key, await diff.toJSONAsync()] + } + if (diff && diff.toJSON) { + await yieldToEventLoop() + return [key, diff.toJSON()] + } + return [key, diff] + }))] + } + if (value && value.toJSONAsync) { + return [key, await value.toJSONAsync()] + } + if (value && value.toJSON) { + await yieldToEventLoop() + return [key, value.toJSON()] + } + return [key, value] + }, 1) + ) + ), } } } diff --git a/src/test/test.js b/src/test/test.js index abae5ce46e..dc25d6aea9 100644 --- a/src/test/test.js +++ b/src/test/test.js @@ -2674,7 +2674,7 @@ describe('Floccus', function() { await withSyncConnection(account, async() => { // move first separator - await account.server.updateBookmark({...tree.children[0].children[0].children[1], parentId: tree.children[0].id}) + await account.server.updateBookmark(new Bookmark({...tree.children[0].children[0].children[1], parentId: tree.children[0].id})) }) console.log('move done') @@ -5768,7 +5768,7 @@ describe('Floccus', function() { new Bookmark(serverMark2) ) - await adapter.updateBookmark({ ...serverMark, id: serverMarkId, url: TEST_URL + '#test2', title: TEST_URL_TITLE, parentId: tree.children[0].id }) + await adapter.updateBookmark(new Bookmark({ ...serverMark, id: serverMarkId, url: TEST_URL + '#test2', title: TEST_URL_TITLE, parentId: tree.children[0].id })) }) await account.setData({ strategy: 'slave'}) @@ -5855,7 +5855,7 @@ describe('Floccus', function() { new Bookmark(serverMark2) ) - await adapter.updateBookmark({ ...serverMark, id: serverMarkId, url: TEST_URL + '#test2', title: TEST_URL_TITLE, parentId: tree.children[0].id }) + await adapter.updateBookmark(new Bookmark({ ...serverMark, id: serverMarkId, url: TEST_URL + '#test2', title: TEST_URL_TITLE, parentId: tree.children[0].id })) }) await browser.tabs.create({url: TEST_URL + '#test4'}) @@ -6853,10 +6853,10 @@ describe('Floccus', function() { // Move the bookmarks out of the group for (const bookmark of tabGroupFolder.children) { - await account.server.updateBookmark({ + await account.server.updateBookmark(new Bookmark({ ...bookmark, parentId: windowFolder.id - }) + })) } // Remove the now-empty group folder