Refactor: Add location to tree items

- Merge Overwrite and Slave into Unidirectional
- Dry up syncProcess#reconcile

Author: marcelklehr

package-lock.json

@@ -22,8 +22,8 @@
     "@babel/core": "^7.12.10",
     "@babel/preset-env": "^7.12.11",
     "@babel/preset-react": "^7.12.10",
-    "@typescript-eslint/eslint-plugin": "^4.10.0",
-    "@typescript-eslint/parser": "^4.10.0",
+    "@typescript-eslint/eslint-plugin": "^4.13.0",
+    "@typescript-eslint/parser": "^4.13.0",
     "all-contributors-cli": "^6.19.0",
     "babel-eslint": "^10.1.0",
     "babel-loader": "^8.2.2",
@@ -42,19 +42,19 @@
     "mocha": "^8.2.1",
     "node-fetch": "^2.6.1",
     "prettier": "^2.2.1",
-    "sass": "^1.30.0",
+    "sass": "^1.32.3",
     "sass-loader": "^8.0.2",
     "seedrandom": "^3.0.5",
     "selenium-webdriver": "^4.0.0-alpha.8",
-    "ts-loader": "^8.0.12",
+    "ts-loader": "^8.0.14",
     "typescript": "^4.1.3",
     "vuetify-loader": "^1.6.0",
-    "webpack": "^4.44.2",
+    "webpack": "^4.46.0",
     "webpack-cli": "^3.3.12",
     "webpack-merge": "^4.2.2"
   },
   "dependencies": {
-    "async-lock": "^1.2.6",
+    "async-lock": "^1.2.8",
     "async-parallel": "^1.2.3",
     "babel-polyfill": "^6.20.0",
     "batching-toposort": "^1.2.0",
@@ -68,13 +68,13 @@
     "eslint-plugin-promise": "^4.1.1",
     "eslint-plugin-standard": "^4.1.0",
     "eslint-plugin-vue": "^6.2.2",
-    "humanize-duration": "^3.25.0",
+    "humanize-duration": "^3.25.1",
     "hyperapp": "^1.2.10",
     "install": "^0.13.0",
     "js-base64": "^3.6.0",
     "lodash": "^4.17.20",
     "murmur2js": "^1.0.0",
-    "npm": "^6.14.10",
+    "npm": "^6.14.11",
     "p-queue": "^5.0.0",
     "picostyle": "^1.0.2",
     "punycode": "^2.1.1",
@@ -86,7 +86,7 @@
     "vue-router": "^3.4.9",
     "vue-template-compiler": "^2.6.11",
     "vuelidate": "^0.7.6",
-    "vuetify": "^2.3.21",
+    "vuetify": "^2.4.2",
     "vuex": "^3.6.0",
     "vuex-router-sync": "^5.0.0"
   }

package.json

@@ -4,16 +4,14 @@ import WebDavAdapter from './adapters/WebDav'
 import FakeAdapter from './adapters/Fake'
 import LocalTree from './LocalTree'
 import DefaultSyncProcess from './strategies/Default'
-import SlaveSyncProcess from './strategies/Slave'
-import OverwriteSyncProcess from './strategies/Overwrite'
+import UnidirectionalSyncProcess from './strategies/Unidirectional'
 import Logger from './Logger'
 import browser from './browser-api'
 import AdapterFactory from './AdapterFactory'
 import MergeSyncProcess from './strategies/Merge'
 import LocalTabs from './LocalTabs'
-import { Folder } from './Tree'
-import MergeOverwrite from './strategies/OverwriteMerge'
-import MergeSlave from './strategies/SlaveMerge'
+import { Folder, ItemLocation } from './Tree'
+import UnidirectionalMergeSyncProcess from './strategies/UnidirectionalMerge'
 
 // register Adapters
 AdapterFactory.register('nextcloud-folders', NextcloudFoldersAdapter)
@@ -174,25 +172,27 @@ export default class Account {
       mappings = await this.storage.getMappings()
       const cacheTree = localResource instanceof LocalTree ? await this.storage.getCache() : new Folder({title: '', id: 'tabs'})
 
-      let strategy
+      let strategy, direction
       switch (this.getData().strategy) {
         case 'slave':
           if (!cacheTree.children.length) {
             Logger.log('Using "merge slave" strategy (no cache available)')
-            strategy = MergeSlave
+            strategy = UnidirectionalMergeSyncProcess
           } else {
             Logger.log('Using slave strategy')
-            strategy = SlaveSyncProcess
+            strategy = UnidirectionalSyncProcess
           }
+          direction = ItemLocation.LOCAL
           break
         case 'overwrite':
           if (!cacheTree.children.length) {
             Logger.log('Using "merge overwrite" strategy (no cache available)')
-            strategy = MergeOverwrite
+            strategy = UnidirectionalMergeSyncProcess
           } else {
             Logger.log('Using "overwrite" strategy')
-            strategy = OverwriteSyncProcess
+            strategy = UnidirectionalSyncProcess
           }
+          direction = ItemLocation.SERVER
           break
         default:
           if (!cacheTree.children.length) {
@@ -214,6 +214,9 @@ export default class Account {
           this.setData({ ...this.getData(), syncing: progress })
         }
       )
+      if (direction) {
+        this.syncing.setDirection(direction)
+      }
       await this.syncing.sync()
 
       // update cache

src/lib/Account.js

@@ -1,5 +1,5 @@
-import { Folder, TItem, ItemType } from './Tree'
-import { Mapping } from './Mappings'
+import { Folder, TItem, ItemType, TItemLocation, ItemLocation } from './Tree'
+import Mappings, { MappingSnapshot } from './Mappings'
 import Ordering from './interfaces/Ordering'
 import batchingToposort from 'batching-toposort'
 import Logger from './Logger'
@@ -40,7 +40,7 @@ export interface ReorderAction {
   type: 'REORDER',
   payload: TItem,
   oldItem?: TItem,
-  order?: Ordering,
+  order: Ordering,
   oldOrder?: Ordering,
 }
 
@@ -112,16 +112,6 @@ export default class Diff {
     }
   }
 
-  add(diff: Diff, types:TActionType[] = []):void {
-    if (types.length === 0) {
-      diff.getActions().forEach(action => this.commit({...action}))
-      return
-    }
-    types.forEach(type =>
-      diff.getActions(type).forEach(action => this.commit({...action}))
-    )
-  }
-
   getActions(type?: TActionType):Action[] {
     if (type) {
       return this.actions[type].slice()
@@ -170,62 +160,76 @@ export default class Diff {
   /**
    * on ServerToLocal: don't map removals
    * on LocalToServer:
-   * @param mappings
-   * @param isLocalToServer
+   * @param mappingsSnapshot
+   * @param targetLocation
    * @param filter
    */
-  map(mappings:Mapping, isLocalToServer: boolean, filter: (Action)=>boolean = () => true):void {
+  map(mappingsSnapshot:MappingSnapshot, targetLocation: TItemLocation, filter: (Action)=>boolean = () => true): Diff {
+    const newDiff = new Diff
+
     // Map payloads
-    this.getActions().forEach(action => {
-      if (action.type === ActionType.REMOVE && !isLocalToServer) {
-        return
-      }
+    this.getActions()
+      .map(a => a as Action)
+      .forEach(action => {
+        let newAction
+
+        if (!filter(action)) {
+          newDiff.commit(action)
+          return
+        }
 
-      if (!filter(action)) {
-        return
-      }
+        /* if (action.type === ActionType.REMOVE && targetLocation !== ItemLocation.SERVER) {
+          newDiff.commit({...action})
+          return
+        } */
 
-      Logger.log('Mapping action ' + action.type + ' ' + (isLocalToServer ? 'LocalToServer' : 'ServerToLocal'), {...action})
+        Logger.log('Mapping action ' + action.type + ' to ' + targetLocation, {...action})
 
-      if (action.type === ActionType.REORDER) {
-        action.oldOrder = action.order
-        action.order = action.order.slice().map(item => {
-          return {...item, id: mappings[item.type ][item.id]}
-        })
-      }
-
-      // needed because we set oldItem in the first section, so we wouldn't know anymore if it was set before
-      const oldItem = action.oldItem
-
-      // We have two sections here, because we want to be able to take IDs from oldItem even for moves
-      // but not parentIds (which do change during moves, obviously)
-
-      if (oldItem && !isLocalToServer) {
-        const oldId = action.oldItem.id
-        const newId = action.payload.id
-        action.oldItem = action.oldItem.clone()
-        action.payload = action.payload.clone()
-        action.payload.id = oldId
-        action.oldItem.id = newId
-      } else {
-        const newPayload = action.payload.clone()
-        newPayload.id = mappings[newPayload.type][newPayload.id]
-        action.oldItem = action.payload.clone()
-        action.payload = newPayload
-      }
-
-      if (oldItem && !isLocalToServer && action.type !== ActionType.MOVE) {
-        const oldParent = action.oldItem.parentId
-        const newParent = action.payload.parentId
-        action.payload.parentId = oldParent
-        action.oldItem.parentId = newParent
-      } else {
-        if (typeof action.payload.parentId !== 'undefined' && typeof mappings.folder[action.payload.parentId] === 'undefined') {
-          throw new Error('Cannot map parentId:' + action.payload.parentId)
+        // needed because we set oldItem in the first section, so we wouldn't know anymore if it was set before
+        const oldItem = action.oldItem
+
+        // We have two sections here, because we want to be able to take IDs from oldItem even for moves
+        // but not parentIds (which do change during moves, obviously)
+
+        if (oldItem && targetLocation !== ItemLocation.SERVER) {
+          const oldId = action.oldItem.id
+          const newId = action.payload.id
+          newAction = {
+            ...action,
+            payload: action.payload.clone(false, targetLocation),
+            oldItem: action.oldItem.clone(false)
+          }
+          newAction.payload.id = oldId
+          newAction.oldItem.id = newId
+        } else {
+          newAction = {
+            ...action,
+            payload: action.payload.clone(false, targetLocation),
+            oldItem: action.payload.clone(false)
+          }
+          newAction.payload.id = Mappings.mapId(mappingsSnapshot, action.payload, targetLocation)
+        }
+
+        if (oldItem && targetLocation !== ItemLocation.SERVER && action.type !== ActionType.MOVE) {
+          newAction.payload.parentId = action.oldItem.parentId
+          newAction.oldItem.parentId = action.payload.parentId
+        } else {
+          newAction.oldItem.parentId = action.payload.parentId
+          newAction.payload.parentId = Mappings.mapParentId(mappingsSnapshot, action.payload, targetLocation)
+          if (typeof newAction.payload.parentId === 'undefined' && typeof action.payload.parentId !== 'undefined') {
+            throw new Error('Failed to map parentId: ' + action.payload.parentId)
+          }
+        }
+
+        if (action.type === ActionType.REORDER) {
+          newAction.oldOrder = action.order
+          newAction.order = action.order.slice().map(item => {
+            return {...item, id: mappingsSnapshot[(targetLocation === ItemLocation.LOCAL ? ItemLocation.SERVER : ItemLocation.LOCAL) + 'To' + targetLocation][item.type][item.id]}
+          })
         }
-        action.oldItem.parentId = action.payload.parentId
-        action.payload.parentId = mappings.folder[action.payload.parentId]
-      }
-    })
+
+        newDiff.commit(newAction)
+      })
+    return newDiff
   }
 }

src/lib/Diff.ts

@@ -2,7 +2,7 @@ import browser from './browser-api'
 import Logger from './Logger'
 import { IResource } from './interfaces/Resource'
 import PQueue from 'p-queue'
-import { Bookmark, Folder } from './Tree'
+import { Bookmark, Folder, ItemLocation } from './Tree'
 import Ordering from './interfaces/Ordering'
 import uniq from 'lodash/uniq'
 
@@ -23,16 +23,19 @@ export default class LocalTabs implements IResource {
     return new Folder({
       title: '',
       id: 'tabs',
+      location: ItemLocation.LOCAL,
       children: uniq(tabs.map(t => t.windowId)).map(windowId => {
         return new Folder({
           title: '',
           id: windowId,
           parentId: 'tabs',
+          location: ItemLocation.LOCAL,
           children: tabs.filter(t => t.windowId === windowId).sort(t => t.index).map(t => new Bookmark({
             id: t.id,
             title: '',
             url: t.url,
             parentId: windowId,
+            location: ItemLocation.LOCAL,
           }))
         })
       })

src/lib/LocalTabs.ts

@@ -4,7 +4,7 @@ import * as Tree from './Tree'
 import { IResource } from './interfaces/Resource'
 import PQueue from 'p-queue'
 import Account from './Account'
-import { Bookmark, Folder } from './Tree'
+import { Bookmark, Folder, ItemLocation } from './Tree'
 import Ordering from './interfaces/Ordering'
 
 export default class LocalTree implements IResource {
@@ -62,6 +62,7 @@ export default class LocalTree implements IResource {
       }
       if (node.children) {
         const folder = new Tree.Folder({
+          location: ItemLocation.LOCAL,
           id: node.id,
           parentId,
           title: parentId ? overrideTitle || node.title : undefined,
@@ -73,6 +74,7 @@ export default class LocalTree implements IResource {
         return folder
       } else {
         return new Tree.Bookmark({
+          location: ItemLocation.LOCAL,
           id: node.id,
           parentId,
           title: node.title,

src/lib/LocalTree.ts

@@ -1,4 +1,4 @@
-import { TItemType } from './Tree'
+import { TItem, TItemLocation, TItemType } from './Tree'
 
 type InternalItemTypeMapping = { LocalToServer: Record<string, string>, ServerToLocal: Record<string, string> }
 
@@ -101,4 +101,28 @@ export default class Mappings {
       }
     }
   }
+
+  static mapId(mappingsSnapshot:MappingSnapshot, item: TItem, target: TItemLocation) : string|number {
+    if (item.location === target) {
+      return item.id
+    }
+    return mappingsSnapshot[item.location + 'To' + target][item.type][item.id]
+  }
+
+  static mapParentId(mappingsSnapshot:MappingSnapshot, item: TItem, target: TItemLocation) : string|number {
+    if (item.location === target) {
+      return item.parentId
+    }
+    return mappingsSnapshot[item.location + 'To' + target].folder[item.parentId]
+  }
+
+  static mappable(mappingsSnapshot: MappingSnapshot, item1: TItem, item2: TItem) : boolean {
+    if (Mappings.mapId(mappingsSnapshot, item1, item2.location) === item2.id) {
+      return true
+    }
+    if (Mappings.mapId(mappingsSnapshot, item2, item1.location) === item1.id) {
+      return true
+    }
+    return false
+  }
 }