Skip to content

Commit c5560ab

Browse files
marcelklehrmarcelklehr
committed
fix(SyncProcess): Refactor mergeable functions
Signed-off-by: Marcel Klehr <[email protected]>
1 parent da69650 commit c5560ab

File tree

2 files changed

+56
-40
lines changed

2 files changed

+56
-40
lines changed

‎src/lib/strategies/Default.ts

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -436,20 +436,19 @@ export default class SyncProcess {
436436
this.cacheTreeRoot,
437437
this.localTreeRoot,
438438
// We also allow canMergeWith for folders here, because Window IDs are not stable
439+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
439440
(oldItem, newItem) => {
440441
if (oldItem.type !== newItem.type) {
441442
return false
442443
}
443-
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark') {
444-
return oldItem.url === newItem.url
444+
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark' && oldItem.url !== newItem.url) {
445+
return false
445446
}
446-
if (oldItem.type === 'folder') {
447-
if (String(oldItem.id) === String(newItem.id)) {
448-
return true
449-
}
450-
if (oldItem.canMergeWith(newItem)) {
451-
return true
452-
}
447+
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
448+
return true
449+
}
450+
if (oldItem.type === 'folder' && oldItem.canMergeWith(newItem)) {
451+
return true
453452
}
454453
return false
455454
},
@@ -461,22 +460,21 @@ export default class SyncProcess {
461460
// We also allow canMergeWith here
462461
// (for bookmarks, because e.g. for NextcloudFolders the id of moved bookmarks changes (because their id is "<bookmarkID>;<folderId>")
463462
// (for folders because Window IDs are not stable)
463+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
464464
(oldItem, newItem) => {
465465
if (oldItem.type !== newItem.type) {
466466
return false
467467
}
468-
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark') {
469-
return oldItem.url === newItem.url
468+
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark' && oldItem.url !== newItem.url) {
469+
return false
470470
}
471-
if (oldItem.type === 'folder') {
472-
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
473-
newMappings.push([oldItem, newItem])
474-
return true
475-
}
476-
if (oldItem.canMergeWith(newItem)) {
477-
newMappings.push([oldItem, newItem])
478-
return true
479-
}
471+
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
472+
newMappings.push([oldItem, newItem])
473+
return true
474+
}
475+
if (oldItem.canMergeWith(newItem)) {
476+
newMappings.push([oldItem, newItem])
477+
return true
480478
}
481479
return false
482480
},
@@ -491,15 +489,12 @@ export default class SyncProcess {
491489
if (oldItem.type !== newItem.type) {
492490
return false
493491
}
494-
if (oldItem.type === 'folder') {
495-
if (String(oldItem.id) === String(newItem.id)) {
496-
return true
497-
}
492+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
493+
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark' && oldItem.url !== newItem.url) {
494+
return false
498495
}
499-
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark') {
500-
if (String(oldItem.id) === String(newItem.id) && oldItem.url === newItem.url) {
501-
return true
502-
}
496+
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
497+
return true
503498
}
504499
return false
505500
},
@@ -508,22 +503,20 @@ export default class SyncProcess {
508503
serverScanner = new Scanner(
509504
this.cacheTreeRoot,
510505
this.serverTreeRoot,
511-
// We also allow canMergeWith here, because e.g. for NextcloudFolders the id of moved bookmarks changes (because their id is "<bookmarkID>;<folderId>")
506+
// We also allow canMergeWith here, because e.g. for NextcloudBookmarks the id of moved bookmarks changes (because their id is "<bookmarkID>;<folderId>")
512507
(oldItem, newItem) => {
513508
if (oldItem.type !== newItem.type) {
514509
return false
515510
}
516-
if (oldItem.type === 'folder') {
517-
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
518-
newMappings.push([oldItem, newItem])
519-
return true
520-
}
511+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
512+
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark' && oldItem.url !== newItem.url) {
513+
return false
514+
}
515+
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem)) {
516+
newMappings.push([oldItem, newItem])
517+
return true
521518
}
522519
if (oldItem.type === 'bookmark' && newItem.type === 'bookmark') {
523-
if (Mappings.mappable(mappingsSnapshot, oldItem, newItem) && oldItem.url === newItem.url) {
524-
newMappings.push([oldItem, newItem])
525-
return true
526-
}
527520
if (oldItem.canMergeWith(newItem)) {
528521
newMappings.push([oldItem, newItem])
529522
return true

‎src/lib/strategies/Unidirectional.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,20 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy {
4343
const localScanner = new Scanner(
4444
this.serverTreeRoot,
4545
this.localTreeRoot,
46+
// We can't rely on a cacheTree, thus we have to accept canMergeWith results as well
4647
(serverItem, localItem) => {
47-
if (localItem.type === serverItem.type && (serverItem.canMergeWith(localItem) || Mappings.mappable(mappingsSnapshot, serverItem, localItem))) {
48+
if (localItem.type !== serverItem.type) {
49+
return false
50+
}
51+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
52+
if (serverItem.type === 'bookmark' && localItem.type === 'bookmark' && serverItem.url !== localItem.url) {
53+
return false
54+
}
55+
if (serverItem.canMergeWith(localItem)) {
56+
newMappings.push([localItem, serverItem])
57+
return true
58+
}
59+
if (Mappings.mappable(mappingsSnapshot, serverItem, localItem)) {
4860
newMappings.push([localItem, serverItem])
4961
return true
5062
}
@@ -58,7 +70,18 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy {
5870
this.localTreeRoot,
5971
this.serverTreeRoot,
6072
(localItem, serverItem) => {
61-
if (serverItem.type === localItem.type && (serverItem.canMergeWith(localItem) || Mappings.mappable(mappingsSnapshot, serverItem, localItem))) {
73+
if (serverItem.type !== localItem.type) {
74+
return false
75+
}
76+
// If a bookmark's URL has changed we want to recreate it instead of updating it, because of Nextcloud Bookmarks' uniqueness constraints
77+
if (serverItem.type === 'bookmark' && localItem.type === 'bookmark' && serverItem.url !== localItem.url) {
78+
return false
79+
}
80+
if (serverItem.canMergeWith(localItem)) {
81+
newMappings.push([localItem, serverItem])
82+
return true
83+
}
84+
if (Mappings.mappable(mappingsSnapshot, serverItem, localItem)) {
6285
newMappings.push([localItem, serverItem])
6386
return true
6487
}

0 commit comments

Comments
 (0)