From 112a3210676d7d0d646456bf6edd543ad31438b6 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 10:49:56 +0200 Subject: [PATCH 01/13] fix: Fix "failed to map parentId" errors Signed-off-by: Marcel Klehr --- src/lib/Diff.ts | 28 +++++++++++++++++++++++---- src/lib/Mappings.ts | 7 +++++++ src/lib/strategies/Default.ts | 36 +++++++++++++++++++++-------------- src/lib/strategies/Merge.ts | 4 ++-- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/src/lib/Diff.ts b/src/lib/Diff.ts index a4c9cb6e4d..718e97b030 100644 --- a/src/lib/Diff.ts +++ b/src/lib/Diff.ts @@ -109,25 +109,45 @@ export default class Diff[], itemTree: Folder, + actions: Action[], + itemTree: Folder, currentItem: TItem, targetAction: Action, chain: Action[] = [] ): boolean { - const targetItemInTree = itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) + const targetItemInTree = itemTree.findItem(targetAction.payload.type, Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) if ( + // target action payload contains currentItem's parent targetAction.payload.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location)) || - (targetItemInTree && targetItemInTree.findFolder(Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location))) + // target action payload contains currentItem + targetAction.payload.findItem(ItemType.FOLDER, + Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location)) || + // or target in tree contains currentItem + (targetItemInTree && targetItemInTree.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location))) || + // or target action payload is the currentItem + Mappings.mapId(mappingsSnapshot, targetAction.payload, currentItem.location) === currentItem.id || + // or target action payload is the currentItem's parent + Mappings.mapId(mappingsSnapshot, targetAction.payload, currentItem.location) === currentItem.parentId || + // or target action payload is the currentItem + Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location) === targetAction.payload.id || + // or target action payload is the currentItems parent + Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location) === targetAction.payload.id ) { return true } const newCurrentActions = actions.filter(targetAction => !chain.includes(targetAction) && ( targetAction.payload.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location)) || + targetAction.payload.findItem(currentItem.type, Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location)) || + ( + itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) && + itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)).findFolder(Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location)) + ) || ( itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) && - itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)).findFolder(Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location))) + itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)).findItem(currentItem.type, Mappings.mapId(mappingsSnapshot, currentItem, itemTree.location)) + ) ) ) if (newCurrentActions.length) { diff --git a/src/lib/Mappings.ts b/src/lib/Mappings.ts index 64bfd19400..0d7d9426e6 100644 --- a/src/lib/Mappings.ts +++ b/src/lib/Mappings.ts @@ -79,6 +79,13 @@ export default class Mappings { } } + static mapRawId(mappingsSnapshot:MappingSnapshot, id: string|number, type: TItemType, source: TItemLocation, target: TItemLocation) : string|number { + if (target === source) { + return id + } + return mappingsSnapshot[source + 'To' + target][type][id] + } + static mapId(mappingsSnapshot:MappingSnapshot, item: TItem, target: TItemLocation) : string|number { if (item.location === target) { return item.id diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index 6a73547ce5..37fc2a6906 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -9,6 +9,7 @@ import { } from '../Tree' import Logger from '../Logger' import Diff, { + Action, ActionType, CreateAction, MoveAction, @@ -296,8 +297,8 @@ export default class SyncProcess { if ('orderFolder' in this.server && !this.localReordersFinal) { // mappings have been updated, reload mappingsSnapshot = this.mappings.getSnapshot() - this.localReordersFinal = this.reconcileReorderings(this.localReorders, this.serverDonePlan, ItemLocation.LOCAL, mappingsSnapshot) - this.serverReorderFinal = this.reconcileReorderings(this.serverReorders, this.localDonePlan, ItemLocation.SERVER, mappingsSnapshot) + this.localReordersFinal = this.reconcileReorderings(this.localReorders, this.localDonePlan, ItemLocation.LOCAL, mappingsSnapshot) + this.serverReorderFinal = this.reconcileReorderings(this.serverReorders, this.serverDonePlan, ItemLocation.SERVER, mappingsSnapshot) } if (this.canceled) { @@ -763,8 +764,9 @@ export default class SyncProcess { } } - const concurrentRemoval = targetRemovals.find(a => - a.payload.findItem('folder', action.payload.id)) + const concurrentRemoval = targetRemovals.find(targetRemoval => + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval) + ) if (concurrentRemoval) { // Already deleted on target, do nothing. return @@ -1096,15 +1098,17 @@ export default class SyncProcess { reconcileReorderings( targetReorders: Diff>, - sourceDonePlan: PlanStage3, + targetDonePlan: PlanStage3, targetLocation: L1, mappingSnapshot: MappingSnapshot ) : Diff> { Logger.log('Reconciling reorders to create a plan') - const sourceCreations = sourceDonePlan.CREATE.getActions() - const sourceRemovals = sourceDonePlan.REMOVE.getActions() - const sourceMoves = sourceDonePlan.MOVE.getActions() + const sourceCreations = targetDonePlan.CREATE.getActions() + const sourceRemovals = targetDonePlan.REMOVE.getActions() + const sourceMoves = targetDonePlan.MOVE.getActions() + const sourceCreationsAndMoves : Action[] = (sourceCreations as Action[]).concat(sourceMoves) + const sourceTree = targetLocation === ItemLocation.LOCAL ? this.localTreeRoot : this.serverTreeRoot const newReorders = new Diff> @@ -1117,7 +1121,8 @@ export default class SyncProcess { const reorderAction = {...oldReorderAction, order: oldReorderAction.order.slice()} const removed = sourceRemovals - .filter(removal => removal.payload.findItem(reorderAction.payload.type, removal.payload.id)) + .filter(removal => + Diff.findChain(mappingSnapshot, sourceCreationsAndMoves, sourceTree, oldReorderAction.payload, removal)) if (removed.length) { return } @@ -1125,13 +1130,16 @@ export default class SyncProcess { // Find Away-moves const childAwayMoves = sourceMoves .filter(move => - (String(reorderAction.payload.id) !== String(move.payload.parentId) && // reorder IDs are from localTree (source of this plan), move.oldItem IDs are from server tree (source of other plan) - reorderAction.order.find(item => String(item.id) === String(move.payload.id) && item.type === move.payload.type))// move.payload IDs are from localTree (target of the other plan + Mappings.mapId(mappingSnapshot, reorderAction.payload, move.payload.location) !== String(move.payload.parentId) && + reorderAction.order.find(item => + Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location) === String(move.payload.id) && item.type === move.payload.type) ) // Find removals const concurrentRemovals = sourceRemovals - .filter(removal => reorderAction.order.find(item => String(item.id) === String(removal.payload.id) && item.type === removal.payload.type)) + .filter(removal => + reorderAction.order.find(item => + Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location) === String(removal.payload.id) && item.type === removal.payload.type)) // Remove away-moves and removals reorderAction.order = reorderAction.order.filter(item => { @@ -1139,7 +1147,7 @@ export default class SyncProcess { if ( // eslint-disable-next-line no-cond-assign action = childAwayMoves.find(move => - String(item.id) === String(move.payload.id) && move.payload.type === item.type)) { + Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location) === String(move.payload.id) && move.payload.type === item.type)) { Logger.log('ReconcileReorders: Removing moved item from order', {move: action, reorder: reorderAction}) return false } @@ -1147,7 +1155,7 @@ export default class SyncProcess { if ( // eslint-disable-next-line no-cond-assign action = concurrentRemovals.find(removal => - String(item.id) === String(removal.payload.id) && removal.payload.type === item.type) + Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location) === String(removal.payload.id) && removal.payload.type === item.type) ) { Logger.log('ReconcileReorders: Removing removed item from order', {item, reorder: reorderAction, removal: action}) return false diff --git a/src/lib/strategies/Merge.ts b/src/lib/strategies/Merge.ts index 2158bc11d5..910018d52a 100644 --- a/src/lib/strategies/Merge.ts +++ b/src/lib/strategies/Merge.ts @@ -175,11 +175,11 @@ export default class MergeSyncProcess extends DefaultSyncProcess { reconcileReorderings( targetReorders: Diff>, - sourceDonePlan: PlanStage3, + targetDonePlan: PlanStage3, targetLocation: L1, mappingSnapshot: MappingSnapshot ) : Diff> { - return super.reconcileReorderings(targetReorders, sourceDonePlan, targetLocation, mappingSnapshot) + return super.reconcileReorderings(targetReorders, targetDonePlan, targetLocation, mappingSnapshot) } async loadChildren(serverTreeRoot: Folder):Promise { From 0b65ac921d7d3ac57662baeb62aa337630e31f20 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 13:42:56 +0200 Subject: [PATCH 02/13] fix(SyncProcess): Correct variable names in reconcileReorderings() Signed-off-by: Marcel Klehr --- src/lib/strategies/Default.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index 37fc2a6906..597cd48d36 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -1104,11 +1104,11 @@ export default class SyncProcess { ) : Diff> { Logger.log('Reconciling reorders to create a plan') - const sourceCreations = targetDonePlan.CREATE.getActions() - const sourceRemovals = targetDonePlan.REMOVE.getActions() - const sourceMoves = targetDonePlan.MOVE.getActions() - const sourceCreationsAndMoves : Action[] = (sourceCreations as Action[]).concat(sourceMoves) - const sourceTree = targetLocation === ItemLocation.LOCAL ? this.localTreeRoot : this.serverTreeRoot + const targetCreations = targetDonePlan.CREATE.getActions() + const targetRemovals = targetDonePlan.REMOVE.getActions() + const targetMoves = targetDonePlan.MOVE.getActions() + const targetCreationsAndMoves : Action[] = (targetCreations as Action[]).concat(targetMoves) + const targetTree = targetLocation === ItemLocation.LOCAL ? this.localTreeRoot : this.serverTreeRoot const newReorders = new Diff> @@ -1120,15 +1120,15 @@ export default class SyncProcess { // clone action const reorderAction = {...oldReorderAction, order: oldReorderAction.order.slice()} - const removed = sourceRemovals + const removed = targetRemovals .filter(removal => - Diff.findChain(mappingSnapshot, sourceCreationsAndMoves, sourceTree, oldReorderAction.payload, removal)) + Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, oldReorderAction.payload, removal)) if (removed.length) { return } // Find Away-moves - const childAwayMoves = sourceMoves + const childAwayMoves = targetMoves .filter(move => Mappings.mapId(mappingSnapshot, reorderAction.payload, move.payload.location) !== String(move.payload.parentId) && reorderAction.order.find(item => @@ -1136,7 +1136,7 @@ export default class SyncProcess { ) // Find removals - const concurrentRemovals = sourceRemovals + const concurrentRemovals = targetRemovals .filter(removal => reorderAction.order.find(item => Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location) === String(removal.payload.id) && item.type === removal.payload.type)) @@ -1164,7 +1164,7 @@ export default class SyncProcess { }) // Find and insert creations - const concurrentCreations = sourceCreations + const concurrentCreations = targetCreations .filter(creation => String(reorderAction.payload.id) === String(creation.payload.parentId)) concurrentCreations .forEach(a => { @@ -1173,7 +1173,7 @@ export default class SyncProcess { }) // Find and insert moves at move target - const moves = sourceMoves + const moves = targetMoves .filter(move => String(reorderAction.payload.id) === String(move.payload.parentId) && !reorderAction.order.find(item => String(item.id) === String(move.payload.id) && item.type === move.payload.type) From ca77653db52e817875c319940f07df82cef1d0f6 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 13:43:26 +0200 Subject: [PATCH 03/13] fix: Refactor and speed up Diff.findChain Signed-off-by: Marcel Klehr --- src/lib/Diff.ts | 79 ++++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/src/lib/Diff.ts b/src/lib/Diff.ts index 718e97b030..d1bf07e836 100644 --- a/src/lib/Diff.ts +++ b/src/lib/Diff.ts @@ -106,6 +106,40 @@ 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) + if (typeof cache[cacheKey] !== 'undefined') { + return cache[cacheKey] + } + const item1InTree = itemTree.findItem(item1.type, Mappings.mapId(mappingsSnapshot, item1, itemTree.location)) + if ( + // target action payload contains item2's parent + item1.findItem(ItemType.FOLDER, + Mappings.mapParentId(mappingsSnapshot, item2, item1.location)) || + // target action payload contains item2 + item1.findItem(ItemType.FOLDER, + Mappings.mapId(mappingsSnapshot, item2, item1.location)) || + // or target in tree contains item2's parent + (item1InTree && item1InTree.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, item2, itemTree.location))) || + // or target in tree contains item2 + (item1InTree && item1InTree.findItem(item2.type, Mappings.mapId(mappingsSnapshot, item2, itemTree.location))) || + // or target action payload is the item2 + Mappings.mapId(mappingsSnapshot, item1, item2.location) === item2.id || + // or target action payload is the item2's parent + Mappings.mapId(mappingsSnapshot, item1, item2.location) === item2.parentId || + // or target action payload is the item2 + Mappings.mapId(mappingsSnapshot, item2, item1.location) === item1.id || + // or target action payload is the item2s parent + Mappings.mapParentId(mappingsSnapshot, item2, item1.location) === item1.id + ) { + cache[cacheKey] = true + return true + } + cache[cacheKey] = false + return false + } static findChain( mappingsSnapshot: MappingSnapshot, @@ -113,50 +147,29 @@ export default class Diff, currentItem: TItem, targetAction: Action, + cache: Record = {}, chain: Action[] = [] ): boolean { - const targetItemInTree = itemTree.findItem(targetAction.payload.type, Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) - if ( - // target action payload contains currentItem's parent - targetAction.payload.findItem(ItemType.FOLDER, - Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location)) || - // target action payload contains currentItem - targetAction.payload.findItem(ItemType.FOLDER, - Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location)) || - // or target in tree contains currentItem - (targetItemInTree && targetItemInTree.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location))) || - // or target action payload is the currentItem - Mappings.mapId(mappingsSnapshot, targetAction.payload, currentItem.location) === currentItem.id || - // or target action payload is the currentItem's parent - Mappings.mapId(mappingsSnapshot, targetAction.payload, currentItem.location) === currentItem.parentId || - // or target action payload is the currentItem - Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location) === targetAction.payload.id || - // or target action payload is the currentItems parent - Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location) === targetAction.payload.id - ) { + + const cacheKey = 'hasChain:' + Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.LOCAL) + ':' + Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.SERVER) + '-' + Mappings.mapId(mappingsSnapshot, targetAction.payload, ItemLocation.LOCAL) + ':' + Mappings.mapId(mappingsSnapshot, targetAction.payload, ItemLocation.SERVER) + if (typeof cache[cacheKey] !== 'undefined') { + return cache[cacheKey] + } + if (Diff.contains(mappingsSnapshot, targetAction.payload, currentItem, itemTree, cache)) { + cache[cacheKey] = true return true } - const newCurrentActions = actions.filter(targetAction => - !chain.includes(targetAction) && ( - targetAction.payload.findItem(ItemType.FOLDER, Mappings.mapParentId(mappingsSnapshot, currentItem, targetAction.payload.location)) || - targetAction.payload.findItem(currentItem.type, Mappings.mapId(mappingsSnapshot, currentItem, targetAction.payload.location)) || - ( - itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) && - itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)).findFolder(Mappings.mapParentId(mappingsSnapshot, currentItem, itemTree.location)) - ) || - ( - itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)) && - itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location)).findItem(currentItem.type, Mappings.mapId(mappingsSnapshot, currentItem, itemTree.location)) - ) - ) + const newCurrentActions = actions.filter(newTargetAction => + !chain.includes(newTargetAction) && Diff.contains(mappingsSnapshot, newTargetAction.payload, currentItem, itemTree, cache) ) if (newCurrentActions.length) { for (const newCurrentAction of newCurrentActions) { - if (Diff.findChain(mappingsSnapshot, actions, itemTree, newCurrentAction.payload, targetAction, [...chain, newCurrentAction])) { + if (Diff.findChain(mappingsSnapshot, actions, itemTree, newCurrentAction.payload, targetAction, cache,[...chain, newCurrentAction])) { return true } } } + cache[cacheKey] = false return false } From 45df376d1183b6fe270f3e436eff4aa16064b9d2 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 13:48:29 +0200 Subject: [PATCH 04/13] fix(Diff.findChain): Improve cache key construction perf Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/lib/Diff.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib/Diff.ts b/src/lib/Diff.ts index d1bf07e836..bca8cd1b03 100644 --- a/src/lib/Diff.ts +++ b/src/lib/Diff.ts @@ -151,7 +151,11 @@ export default class Diff[] = [] ): boolean { - const cacheKey = 'hasChain:' + Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.LOCAL) + ':' + Mappings.mapId(mappingsSnapshot, currentItem, ItemLocation.SERVER) + '-' + Mappings.mapId(mappingsSnapshot, targetAction.payload, ItemLocation.LOCAL) + ':' + 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] } From d573db771b8fdf57ca8245e68bf2fc7e6d7dece2 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 17:08:50 +0200 Subject: [PATCH 05/13] fix(BrowserTree): Do not move items across folders in orderFolder() Signed-off-by: Marcel Klehr --- src/lib/browser/BrowserTree.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/browser/BrowserTree.ts b/src/lib/browser/BrowserTree.ts index 67c94bc327..bd2c9d5a96 100644 --- a/src/lib/browser/BrowserTree.ts +++ b/src/lib/browser/BrowserTree.ts @@ -249,6 +249,10 @@ export default class BrowserTree implements IResource const [realTree] = await browser.bookmarks.getSubTree(id) try { for (let index = 0; index < order.length; index++) { + if (!realTree.children.find(item => String(item.id) === String(order[index].id))) { + Logger.log('(local)ORDERFOLDER: skipping item ', order[index]) + continue + } await browser.bookmarks.move(order[index].id, { parentId: id.toString(), index }) } } catch (e) { From cf56c72f860fe95b0be3531a583b7a1b3b3b041b Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 17:10:22 +0200 Subject: [PATCH 06/13] fix(Diff): Fix findChains() Signed-off-by: Marcel Klehr --- src/lib/Diff.ts | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/lib/Diff.ts b/src/lib/Diff.ts index bca8cd1b03..d21aa74c98 100644 --- a/src/lib/Diff.ts +++ b/src/lib/Diff.ts @@ -106,8 +106,8 @@ export default class Diff, item2: TItem, itemTree: Folder, cache: Record): boolean { + + 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') { @@ -115,24 +115,11 @@ 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 cacheKey = `hasChain:${currentItemLocalId}:${currentItemServerId}-${targetPayloadLocalId}:${targetPayloadServerId}`; + 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.contains(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.contains(mappingsSnapshot, newTargetAction.payload, currentItem, itemTree, cache) + !chain.includes(newTargetAction) && Diff.containsParent(mappingsSnapshot, newTargetAction.payload, currentItem, itemTree, cache) ) if (newCurrentActions.length) { for (const newCurrentAction of newCurrentActions) { From 2253d4285e2501d8b91acdadb1b96d95373c7c17 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 17:10:37 +0200 Subject: [PATCH 07/13] fix(Mappings): Fix mappable() Signed-off-by: Marcel Klehr --- src/lib/Mappings.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/Mappings.ts b/src/lib/Mappings.ts index 0d7d9426e6..5762a1042a 100644 --- a/src/lib/Mappings.ts +++ b/src/lib/Mappings.ts @@ -101,10 +101,10 @@ export default class Mappings { } static mappable(mappingsSnapshot: MappingSnapshot, item1: TItem, item2: TItem) : boolean { - if (Mappings.mapId(mappingsSnapshot, item1, item2.location) === item2.id) { + if (String(Mappings.mapId(mappingsSnapshot, item1, item2.location)) === String(item2.id)) { return true } - if (Mappings.mapId(mappingsSnapshot, item2, item1.location) === item1.id) { + if (String(Mappings.mapId(mappingsSnapshot, item2, item1.location)) === String(item1.id)) { return true } return false From 1be0c22308f561627380c9a64fafb7f8f23bb14e Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 17:16:23 +0200 Subject: [PATCH 08/13] fix(SyncProcess): reconcileReorderings based on both donePlans Signed-off-by: Marcel Klehr --- src/lib/strategies/Default.ts | 28 +++++++++++++++++----------- src/lib/strategies/Merge.ts | 6 +++--- src/lib/strategies/Unidirectional.ts | 6 ++++++ src/test/test.js | 13 ++++++++----- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index 597cd48d36..7ff68df1cd 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -290,6 +290,10 @@ export default class SyncProcess { Logger.log('Executing local plan') await this.execute(this.localTree, this.localPlanStage2, ItemLocation.LOCAL, this.localDonePlan, this.localReorders) + // Remove mappings only after both plans have been executed + this.localDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.localTree, action.payload)) + this.serverDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.localTree, action.payload)) + if (this.canceled) { throw new CancelledSyncError() } @@ -297,8 +301,10 @@ export default class SyncProcess { if ('orderFolder' in this.server && !this.localReordersFinal) { // mappings have been updated, reload mappingsSnapshot = this.mappings.getSnapshot() - this.localReordersFinal = this.reconcileReorderings(this.localReorders, this.localDonePlan, ItemLocation.LOCAL, mappingsSnapshot) - this.serverReorderFinal = this.reconcileReorderings(this.serverReorders, this.serverDonePlan, ItemLocation.SERVER, mappingsSnapshot) + const localReorders = this.reconcileReorderings(this.localReorders, this.localDonePlan, ItemLocation.LOCAL, mappingsSnapshot) + const serverReorders = this.reconcileReorderings(this.serverReorders, this.serverDonePlan, ItemLocation.SERVER, mappingsSnapshot) + this.localReordersFinal = this.reconcileReorderings(localReorders, this.serverDonePlan, ItemLocation.LOCAL, mappingsSnapshot).map(mappingsSnapshot, ItemLocation.LOCAL) + this.serverReorderFinal = this.reconcileReorderings(serverReorders, this.localDonePlan, ItemLocation.SERVER, mappingsSnapshot).map(mappingsSnapshot, ItemLocation.SERVER) } if (this.canceled) { @@ -623,7 +629,7 @@ export default class SyncProcess { const concurrentMove = targetMoves.find(a => action.payload.type === a.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, a.payload)) if (concurrentMove) { - // Moved both on target and sourcely, source has precedence: do nothing sourcely + // Moved both on target and sourcely, master has precedence: do nothing on master return } } @@ -1065,7 +1071,6 @@ export default class SyncProcess { } await action.payload.visitRemove(resource) - await this.removeMapping(resource, action.payload) diff.retract(action) donePlan.REMOVE.commit(action) this.updateProgress() @@ -1098,15 +1103,15 @@ export default class SyncProcess { reconcileReorderings( targetReorders: Diff>, - targetDonePlan: PlanStage3, + targetOrSourceDonePlan: PlanStage3, targetLocation: L1, mappingSnapshot: MappingSnapshot - ) : Diff> { + ) : Diff> { Logger.log('Reconciling reorders to create a plan') - const targetCreations = targetDonePlan.CREATE.getActions() - const targetRemovals = targetDonePlan.REMOVE.getActions() - const targetMoves = targetDonePlan.MOVE.getActions() + const targetCreations = targetOrSourceDonePlan.CREATE.getActions() + const targetRemovals = targetOrSourceDonePlan.REMOVE.getActions() + const targetMoves = targetOrSourceDonePlan.MOVE.getActions() const targetCreationsAndMoves : Action[] = (targetCreations as Action[]).concat(targetMoves) const targetTree = targetLocation === ItemLocation.LOCAL ? this.localTreeRoot : this.serverTreeRoot @@ -1122,7 +1127,8 @@ export default class SyncProcess { const removed = targetRemovals .filter(removal => - Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, oldReorderAction.payload, removal)) + removal.payload.findItem(reorderAction.payload.type, reorderAction.payload.id) || + Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, reorderAction.payload, removal)) if (removed.length) { return } @@ -1186,7 +1192,7 @@ export default class SyncProcess { newReorders.commit(reorderAction) }) - return newReorders.map(mappingSnapshot, targetLocation) + return newReorders } async executeReorderings(resource:OrderFolderResource, reorderings:Diff>):Promise { diff --git a/src/lib/strategies/Merge.ts b/src/lib/strategies/Merge.ts index 910018d52a..3bac4ccb6b 100644 --- a/src/lib/strategies/Merge.ts +++ b/src/lib/strategies/Merge.ts @@ -175,11 +175,11 @@ export default class MergeSyncProcess extends DefaultSyncProcess { reconcileReorderings( targetReorders: Diff>, - targetDonePlan: PlanStage3, + targetOrSourceDonePlan: PlanStage3, targetLocation: L1, mappingSnapshot: MappingSnapshot - ) : Diff> { - return super.reconcileReorderings(targetReorders, targetDonePlan, targetLocation, mappingSnapshot) + ) : Diff> { + return super.reconcileReorderings(targetReorders, targetOrSourceDonePlan, targetLocation, mappingSnapshot) } async loadChildren(serverTreeRoot: Folder):Promise { diff --git a/src/lib/strategies/Unidirectional.ts b/src/lib/strategies/Unidirectional.ts index b254661dd4..43f67b8e88 100644 --- a/src/lib/strategies/Unidirectional.ts +++ b/src/lib/strategies/Unidirectional.ts @@ -181,6 +181,12 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy { await this.executeRevert(target, this.revertPlan, this.direction, this.revertDonePlan, sourceScanResult.REORDER) + if (this.direction === ItemLocation.LOCAL) { + this.revertDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.localTree, action.payload)) + } else { + this.revertDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.server, action.payload)) + } + if ('orderFolder' in this.server && !this.revertReorders) { const mappingsSnapshot = this.mappings.getSnapshot() Logger.log('Mapping reorderings') diff --git a/src/test/test.js b/src/test/test.js index 66dec2742a..0a76c076b0 100644 --- a/src/test/test.js +++ b/src/test/test.js @@ -5321,15 +5321,18 @@ describe('Floccus', function() { const tree2AfterThirdSync = await account2.localTree.getBookmarksTree( true ) + console.log('Checking local tree of acc2') expectTreeEqual( tree2AfterThirdSync, tree2BeforeThirdSync, false ) - serverTreeAfterThirdSync.title = tree2AfterThirdSync.title + console.log('All good') + console.log('Checking server tree') + serverTreeAfterThirdSync.title = tree2BeforeThirdSync.title expectTreeEqual( serverTreeAfterThirdSync, - tree2AfterThirdSync, + tree2BeforeThirdSync, false ) console.log('Second round second half ok') @@ -7174,8 +7177,8 @@ describe('Floccus', function() { .filter(item => item.id !== tree2AfterFirstSync.id) } - await randomlyManipulateTree(account1, folders1, bookmarks1, 20) - await randomlyManipulateTree(account2, folders2, bookmarks2, 20) + await randomlyManipulateTree(account1, folders1, bookmarks1, RANDOM_MANIPULATION_ITERATIONS) + await randomlyManipulateTree(account2, folders2, bookmarks2, RANDOM_MANIPULATION_ITERATIONS) console.log(' acc1: Moved items') @@ -7300,7 +7303,7 @@ describe('Floccus', function() { } }) - it('should handle fuzzed changes with deletions from two clients', async function() { + it('should handle fuzzed changes with deletions from two clients (normal)', async function() { const localRoot = account1.getData().localRoot let bookmarks1 = [] let folders1 = [] From 226b029b0e63cc520ac5bad03cd39f74c8aa8229 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 30 Jul 2025 17:25:20 +0200 Subject: [PATCH 09/13] fixup Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/lib/strategies/Default.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index 7ff68df1cd..a2c39f2385 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -292,7 +292,7 @@ export default class SyncProcess { // Remove mappings only after both plans have been executed this.localDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.localTree, action.payload)) - this.serverDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.localTree, action.payload)) + this.serverDonePlan.REMOVE.getActions().forEach(action => this.removeMapping(this.server, action.payload)) if (this.canceled) { throw new CancelledSyncError() From db02934c6260c7689db83caa268422bc74ddbdba Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 1 Aug 2025 19:35:20 +0200 Subject: [PATCH 10/13] fix(reconcileReorderings): Fix ID comparisons make sure to convert to string before comparing Signed-off-by: Marcel Klehr --- src/lib/strategies/Default.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index a2c39f2385..1a74f5c382 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -1136,16 +1136,16 @@ export default class SyncProcess { // Find Away-moves const childAwayMoves = targetMoves .filter(move => - Mappings.mapId(mappingSnapshot, reorderAction.payload, move.payload.location) !== String(move.payload.parentId) && + String(Mappings.mapId(mappingSnapshot, reorderAction.payload, move.payload.location)) !== String(move.payload.parentId) && reorderAction.order.find(item => - Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location) === String(move.payload.id) && item.type === move.payload.type) + String(Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location)) === String(move.payload.id) && item.type === move.payload.type) ) // Find removals const concurrentRemovals = targetRemovals .filter(removal => reorderAction.order.find(item => - Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location) === String(removal.payload.id) && item.type === removal.payload.type)) + String(Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location)) === String(removal.payload.id) && item.type === removal.payload.type)) // Remove away-moves and removals reorderAction.order = reorderAction.order.filter(item => { @@ -1153,7 +1153,7 @@ export default class SyncProcess { if ( // eslint-disable-next-line no-cond-assign action = childAwayMoves.find(move => - Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location) === String(move.payload.id) && move.payload.type === item.type)) { + String(Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, move.payload.location)) === String(move.payload.id) && move.payload.type === item.type)) { Logger.log('ReconcileReorders: Removing moved item from order', {move: action, reorder: reorderAction}) return false } @@ -1161,7 +1161,7 @@ export default class SyncProcess { if ( // eslint-disable-next-line no-cond-assign action = concurrentRemovals.find(removal => - Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location) === String(removal.payload.id) && removal.payload.type === item.type) + String(Mappings.mapRawId(mappingSnapshot, item.id, item.type, reorderAction.payload.location, removal.payload.location)) === String(removal.payload.id) && removal.payload.type === item.type) ) { Logger.log('ReconcileReorders: Removing removed item from order', {item, reorder: reorderAction, removal: action}) return false @@ -1351,11 +1351,11 @@ export default class SyncProcess { mappingsSnapshot: MappingSnapshot, sourceReorders:Diff>, oldItem: TItem) { - const parentReorder = sourceReorders.getActions().find(action => Mappings.mapId(mappingsSnapshot, action.payload, oldItem.location) === oldItem.parentId) + const parentReorder = sourceReorders.getActions().find(action => String(Mappings.mapId(mappingsSnapshot, action.payload, oldItem.location)) === String(oldItem.parentId)) if (!parentReorder) { return } - parentReorder.order = parentReorder.order.filter(item => !(item.type === oldItem.type && Mappings.mapId(mappingsSnapshot, oldItem, parentReorder.payload.location) === item.id)) + parentReorder.order = parentReorder.order.filter(item => !(item.type === oldItem.type && String(Mappings.mapId(mappingsSnapshot, oldItem, parentReorder.payload.location)) === String(item.id))) } toJSON(): ISerializedSyncProcess { From d0eb476c69d36a98f23c20f90d1fb9b7605bcf9c Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 2 Aug 2025 09:39:32 +0200 Subject: [PATCH 11/13] perf(findChain): Cache findChain calls more aggressively improves processing times for large diffs by up to 29% Signed-off-by: Marcel Klehr --- src/lib/strategies/Default.ts | 37 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/lib/strategies/Default.ts b/src/lib/strategies/Default.ts index 1a74f5c382..853b0c3db1 100644 --- a/src/lib/strategies/Default.ts +++ b/src/lib/strategies/Default.ts @@ -561,10 +561,11 @@ export default class SyncProcess { REORDER: new Diff(), } + const findChainCacheForRemovals = {} await Parallel.each(sourceScanResult.REMOVE.getActions(), async(action) => { const concurrentRemoval = targetRemovals.find(targetRemoval => (action.payload.type === targetRemoval.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, targetRemoval.payload)) || - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, targetRemoval)) + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, targetRemoval, findChainCacheForRemovals)) if (concurrentRemoval) { // Already deleted on target, do nothing. return @@ -581,6 +582,7 @@ export default class SyncProcess { targetPlan.REMOVE.commit(action) }, ACTION_CONCURRENCY) + const findChainCacheForCreations = {} await Parallel.each(sourceScanResult.CREATE.getActions(), async(action) => { const concurrentCreation = targetCreations.find(a => ( action.payload.parentId === Mappings.mapParentId(mappingsSnapshot, a.payload, action.payload.location) && @@ -611,9 +613,10 @@ export default class SyncProcess { // TODO: subScanner may contain residual CREATE/REMOVE actions that need to be added to mappings return } + const concurrentRemoval = targetScanResult.REMOVE.getActions().find(targetRemoval => // target removal removed this creation's target (via some chain) - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval) + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCacheForCreations) ) if (concurrentRemoval) { avoidTargetReorders[action.payload.parentId] = true @@ -633,20 +636,24 @@ export default class SyncProcess { return } } + let findChainCache = {} // FInd out if there's a removal in the target diff which already deletes this item (via some chain of MOVE|CREATEs) const complexTargetTargetRemoval = targetRemovals.find(targetRemoval => { - return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval) + return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCache) }) + findChainCache = {} const concurrentTargetOriginRemoval = targetRemovals.find(targetRemoval => (action.payload.type === targetRemoval.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, targetRemoval.payload)) || - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.oldItem, targetRemoval) + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.oldItem, targetRemoval, findChainCache) ) + findChainCache = {} const concurrentSourceOriginRemoval = sourceRemovals.find(sourceRemoval => { - return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.oldItem, sourceRemoval) + return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.oldItem, sourceRemoval, findChainCache) }) + findChainCache = {} const concurrentSourceTargetRemoval = sourceRemovals.find(sourceRemoval => // We pass an empty folder here, because we don't want direct deletions of the moved folder's parent to count, as it's moved away anyway - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, new Folder({id: 0, location: targetLocation}), action.payload, sourceRemoval) + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, new Folder({id: 0, location: targetLocation}), action.payload, sourceRemoval, findChainCache) ) if (complexTargetTargetRemoval) { // target already deleted by a target|source REMOVE (connected via source MOVE|CREATEs) @@ -697,15 +704,18 @@ export default class SyncProcess { } return } + let findChainCache1 = {}, findChainCache2 = {} // Find concurrent moves that form a hierarchy reversal together with this one const concurrentHierarchyReversals = targetMoves.filter(targetMove => { - return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetMove) && - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, targetMove.payload, action) + return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetMove, findChainCache1) && + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, targetMove.payload, action, findChainCache2) }) if (concurrentHierarchyReversals.length) { if (targetLocation !== this.masterLocation) { targetPlan.MOVE.commit(action) + findChainCache1 = {} + findChainCache2 = {} concurrentHierarchyReversals.forEach(a => { // moved sourcely but moved in reverse hierarchical order on target const payload = a.oldItem.copyWithLocation(false, action.payload.location) @@ -718,8 +728,8 @@ export default class SyncProcess { targetPlan.MOVE.getActions().find(move => String(move.payload.id) === String(payload.id)) || sourceMoves.find(move => String(move.payload.id) === String(payload.id)) || // Don't move back into removed territory - targetRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, remove)) || - sourceRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, remove)) + targetRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, remove, findChainCache)) || + sourceRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, remove, findChainCache)) ) { return } @@ -770,8 +780,9 @@ export default class SyncProcess { } } + const findChainCache = {} const concurrentRemoval = targetRemovals.find(targetRemoval => - Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval) + Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCache) ) if (concurrentRemoval) { // Already deleted on target, do nothing. @@ -1117,6 +1128,8 @@ export default class SyncProcess { const newReorders = new Diff> + const findChainCache = {} + targetReorders .getActions() // MOVEs have oldItem from cacheTree and payload now mapped to their corresponding target tree @@ -1128,7 +1141,7 @@ export default class SyncProcess { const removed = targetRemovals .filter(removal => removal.payload.findItem(reorderAction.payload.type, reorderAction.payload.id) || - Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, reorderAction.payload, removal)) + Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, reorderAction.payload, removal, findChainCache)) if (removed.length) { return } From f16b78e8605d2151ec7460eaab8fb4617bb538f9 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 2 Aug 2025 15:01:23 +0200 Subject: [PATCH 12/13] fix(tests): Fix expectTreeEqual when checkOrder=false Signed-off-by: Marcel Klehr --- src/test/test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/test.js b/src/test/test.js index 0a76c076b0..c403a6205b 100644 --- a/src/test/test.js +++ b/src/test/test.js @@ -30,11 +30,15 @@ let expectTreeEqualRec = function(tree1, tree2, recDepth, ignoreEmptyFolders, ch tree2.children.sort((a, b) => { if (a.title < b.title) return -1 if (a.title > b.title) return 1 + if ((a.url || '') < (b.url || '')) return -1 + if ((a.url || '') > (b.url || '')) return 1 return 0 }) tree1.children.sort((a, b) => { if (a.title < b.title) return -1 if (a.title > b.title) return 1 + if ((a.url || '') < (b.url || '')) return -1 + if ((a.url || '') > (b.url || '')) return 1 return 0 }) } From 7af73edf244b5e6c2361f7ee7392a966aa22adf2 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sat, 2 Aug 2025 15:01:37 +0200 Subject: [PATCH 13/13] fix(tests): Do not check order in benchmarks Signed-off-by: Marcel Klehr --- src/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test.js b/src/test/test.js index c403a6205b..f907d137b4 100644 --- a/src/test/test.js +++ b/src/test/test.js @@ -6592,7 +6592,7 @@ describe('Floccus', function() { beforeEach('set up accounts', async function() { let _expectTreeEqual = expectTreeEqual - expectTreeEqual = (tree1, tree2, ignoreEmptyFolders, checkOrder) => _expectTreeEqual(tree1, tree2, ignoreEmptyFolders, !!checkOrder) + expectTreeEqual = (tree1, tree2, ignoreEmptyFolders, checkOrder) => _expectTreeEqual(tree1, tree2, ignoreEmptyFolders, false) // reset random seed random.use(seedrandom(SEED))