Merge pull request #1919 from floccusaddon/fix/changes-while-syncing

test(sync): Making changes to the tree while syncing should not cause trouble

Author: marcelklehr

src/lib/Account.ts

@@ -14,6 +14,7 @@ import { isTest } from './isTest'
 import CachingAdapter from './adapters/Caching'
 import * as Sentry from '@sentry/vue'
 import AsyncLock from 'async-lock'
+import CachingTreeWrapper from './CachingTreeWrapper'
 
 declare const DEBUG: boolean
 
@@ -163,7 +164,7 @@ export default class Account {
     try {
       if (this.getData().syncing || this.syncing) return
 
-      const localResource = await this.getResource()
+      const localResource = new CachingTreeWrapper(await this.getResource())
       if (!(await this.server.isAvailable()) || !(await localResource.isAvailable())) return
 
       Logger.log('Starting sync process for account ' + this.getLabel())
@@ -283,9 +284,8 @@ export default class Account {
       await this.setData({ scheduled: false, syncing: 1 })
 
       // update cache
-      const cache = (await localResource.getBookmarksTree()).clone(false)
+      const cache = (await localResource.getCacheTree()).clone(false)
       this.syncProcess.filterOutUnacceptedBookmarks(cache)
-      this.syncProcess.filterOutUnmappedItems(cache, await mappings.getSnapshot())
       await this.storage.setCache(cache)
 
       if (this.server.onSyncComplete) {

src/lib/CacheTree.ts

@@ -0,0 +1,26 @@
+import CachingAdapter from './adapters/Caching'
+import { IResource } from './interfaces/Resource'
+import { Folder, ItemLocation, TItemLocation } from './Tree'
+
+export default class CacheTree extends CachingAdapter implements IResource<typeof ItemLocation.LOCAL> {
+  protected location: TItemLocation = ItemLocation.LOCAL
+
+  constructor() {
+    super({})
+  }
+
+  public setTree(tree: Folder<typeof ItemLocation.LOCAL>) {
+    this.bookmarksCache = tree.clone(false)
+    this.bookmarksCache.createIndex()
+  }
+
+  async getBookmarksTree(): Promise<Folder<typeof ItemLocation.LOCAL>> {
+    const tree = await super.getBookmarksTree()
+    tree.createIndex()
+    return tree as Folder<typeof ItemLocation.LOCAL>
+  }
+
+  isAvailable(): Promise<boolean> {
+    return Promise.resolve(true)
+  }
+}

src/lib/CachingTreeWrapper.ts

@@ -0,0 +1,77 @@
+import { CachingResource, OrderFolderResource } from './interfaces/Resource'
+import { Bookmark, Folder, ItemLocation } from './Tree'
+import CacheTree from './CacheTree'
+import Ordering from './interfaces/Ordering'
+
+export default class CachingTreeWrapper implements OrderFolderResource<typeof ItemLocation.LOCAL>, CachingResource<typeof ItemLocation.LOCAL> {
+  private innerTree: OrderFolderResource<typeof ItemLocation.LOCAL>
+  private cacheTree: CacheTree
+
+  constructor(innerTree: OrderFolderResource<typeof ItemLocation.LOCAL>) {
+    this.innerTree = innerTree
+    this.cacheTree = new CacheTree()
+  }
+
+  async getBookmarksTree(): Promise<Folder<typeof ItemLocation.LOCAL>> {
+    const tree = await this.innerTree.getBookmarksTree()
+    this.cacheTree.setTree(tree)
+    return tree
+  }
+
+  async createBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>): Promise<string|number> {
+    const id = await this.innerTree.createBookmark(bookmark)
+    const cacheId = await this.cacheTree.createBookmark(bookmark.copy(false))
+    const cacheBookmark = this.cacheTree.bookmarksCache.findBookmark(cacheId)
+    cacheBookmark.id = id
+    cacheBookmark.parentId = bookmark.parentId
+    this.cacheTree.bookmarksCache.createIndex()
+    return id
+  }
+
+  async updateBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>):Promise<void> {
+    await this.innerTree.updateBookmark(bookmark)
+    await this.cacheTree.updateBookmark(bookmark.copy(false))
+  }
+
+  async removeBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>): Promise<void> {
+    await this.innerTree.removeBookmark(bookmark)
+    await this.cacheTree.removeBookmark(bookmark)
+  }
+
+  async createFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<string|number> {
+    const id = await this.innerTree.createFolder(folder)
+    const cacheId = await this.cacheTree.createFolder(folder.copy(false))
+    const cacheFolder = this.cacheTree.bookmarksCache.findFolder(cacheId)
+    cacheFolder.id = id
+    cacheFolder.parentId = folder.parentId
+    this.cacheTree.bookmarksCache.createIndex()
+    return id
+  }
+
+  async orderFolder(id:string|number, order:Ordering<typeof ItemLocation.LOCAL>): Promise<void> {
+    await this.innerTree.orderFolder(id, order)
+    await this.cacheTree.orderFolder(id, order)
+  }
+
+  async updateFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<void> {
+    await this.innerTree.updateFolder(folder)
+    await this.cacheTree.updateFolder(folder.copy(false))
+  }
+
+  async removeFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<void> {
+    await this.innerTree.removeFolder(folder)
+    await this.cacheTree.removeFolder(folder)
+  }
+
+  isAvailable(): Promise<boolean> {
+    return this.innerTree.isAvailable()
+  }
+
+  async isUsingBrowserTabs() {
+    return this.innerTree.isUsingBrowserTabs?.()
+  }
+
+  getCacheTree(): Promise<Folder<typeof ItemLocation.LOCAL>> {
+    return this.cacheTree.getBookmarksTree()
+  }
+}

src/lib/Diff.ts

@@ -200,16 +200,16 @@ export default class Diff<L1 extends TItemLocation, L2 extends TItemLocation, A
           const newId = action.payload.id
           newAction = {
             ...action,
-            payload: action.payload.cloneWithLocation(false, targetLocation),
-            oldItem: action.oldItem.cloneWithLocation(false, action.payload.location)
+            payload: action.payload.copyWithLocation(false, targetLocation),
+            oldItem: action.oldItem.copyWithLocation(false, action.payload.location)
           }
           newAction.payload.id = oldId
           newAction.oldItem.id = newId
         } else {
           newAction = {
             ...action,
-            payload: action.payload.cloneWithLocation(false, targetLocation),
-            oldItem: action.payload.clone(false)
+            payload: action.payload.copyWithLocation(false, targetLocation),
+            oldItem: action.payload.copy(false)
           }
           newAction.payload.id = Mappings.mapId(mappingsSnapshot, action.payload, targetLocation)
         }
@@ -253,8 +253,8 @@ export default class Diff<L1 extends TItemLocation, L2 extends TItemLocation, A
     return this.getActions().map((action: A) => {
       return {
         ...action,
-        payload: action.payload.clone(false),
-        oldItem: action.oldItem && action.oldItem.clone(false),
+        payload: action.payload.copy(false),
+        oldItem: action.oldItem && action.oldItem.copy(false),
       }
     })
   }

src/lib/LocalTabs.ts

@@ -153,6 +153,10 @@ export default class LocalTabs implements OrderFolderResource<typeof ItemLocatio
     })
     return Boolean(tabs.length)
   }
+
+  async isUsingBrowserTabs() {
+    return true
+  }
 }
 
 function awaitTabsUpdated() {

src/lib/Scanner.ts

@@ -203,7 +203,7 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
               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.clone(true)
+              const removedItemClone = removedItem.copy(true)
               const oldParentClone = removedItemClone.findItem(ItemType.FOLDER, oldItem.parentId) as Folder<L1>
               const oldItemClone = removedItemClone.findItem(oldItem.type, oldItem.id)
               oldIndex = oldParentClone.children.indexOf(oldItemClone)
@@ -235,7 +235,7 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
                 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.clone(true)
+                const createdItemClone = createdItem.copy(true)
                 const newParentClone = createdItemClone.findItem(ItemType.FOLDER, newItem.parentId) as Folder<L2>
                 const newClonedItem = createdItemClone.findItem(newItem.type, newItem.id)
                 index = newParentClone.children.indexOf(newClonedItem)

src/lib/Tree.ts

@@ -88,16 +88,49 @@ export class Bookmark<L extends TItemLocation> {
   }
 
   clone(withHash?: boolean):Bookmark<L> {
-    return new Bookmark(this)
+    const bookmark = Object.create(this)
+    if (!withHash) {
+      bookmark.hashValue = {}
+    }
+    return bookmark
   }
 
   cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2): Bookmark<L2> {
+    const newBookmark = Object.create(this)
+    newBookmark.location = location
+    return newBookmark
+  }
+
+  copy(withHash?: boolean):Bookmark<L> {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    return new Bookmark(this.toJSON())
+  }
+
+  copyWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2): Bookmark<L2> {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     return new Bookmark({
-      ...this,
+      ...this.toJSON(),
       location,
     })
   }
 
+  toJSON() {
+    // Flatten inherited properties for serialization
+    const result = {}
+    let obj = this
+    while (obj) {
+      Object.entries(obj).forEach(([key, value]) => {
+        if (!(key in result)) {
+          result[key] = value
+        }
+      })
+      obj = Object.getPrototypeOf(obj)
+    }
+    return result
+  }
+
   createIndex():any {
     return { [this.id]: this }
   }
@@ -309,23 +342,64 @@ export class Folder<L extends TItemLocation> {
     return this.hashValue[String(preserveOrder)]
   }
 
-  clone(withHash?:boolean):Folder<L> {
+  copy(withHash?:boolean):Folder<L> {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     return new Folder({
-      ...this,
+      ...this.toJSON(),
       ...(!withHash && { hashValue: {} }),
-      children: this.children.map(child => child.clone(withHash))
+      children: this.children.map(child => child.copy(withHash))
     })
   }
 
-  cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
+  copyWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     return new Folder({
-      ...this,
+      ...this.toJSON(),
       location,
       ...(!withHash && { hashValue: {} }),
-      children: this.children.map(child => child.cloneWithLocation(withHash, location))
+      children: this.children.map(child => child.copyWithLocation(withHash, location))
     })
   }
 
+  clone(withHash?:boolean):Folder<L> {
+    const newFolder = Object.create(this)
+    newFolder.index = null
+    if (!withHash) {
+      newFolder.hashValue = {}
+    }
+    newFolder.children = this.children.map(child => child.clone(withHash))
+    return newFolder
+  }
+
+  cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
+    const newFolder = Object.create(this)
+    if (!withHash) {
+      newFolder.hashValue = {}
+    }
+    newFolder.index = null
+    newFolder.location = location
+    newFolder.children = this.children.map(child => child.cloneWithLocation(withHash, location))
+    return newFolder
+  }
+
+  toJSON(): Folder<L> {
+    // Flatten inherited properties for serialization
+    const result: Folder<L> = {} as any as Folder<L>
+    let obj = this
+    while (obj) {
+      Object.entries(obj).forEach(([key, value]) => {
+        if (key === 'index') return
+        if (!(key in result)) {
+          result[key] = value
+        }
+      })
+      obj = Object.getPrototypeOf(obj)
+    }
+    return result
+  }
+
   count():number {
     if (!this.index) {
       this.createIndex()

src/lib/adapters/Caching.ts

@@ -16,7 +16,7 @@ import { BulkImportResource } from '../interfaces/Resource'
 
 export default class CachingAdapter implements Adapter, BulkImportResource<TItemLocation> {
   protected highestId: number
-  protected bookmarksCache: Folder<TItemLocation>
+  public bookmarksCache: Folder<TItemLocation>
   protected server: any
   protected location: TItemLocation = ItemLocation.SERVER
 
@@ -35,7 +35,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
   }
 
   async getBookmarksTree(): Promise<Folder<TItemLocation>> {
-    return this.bookmarksCache.clone()
+    return this.bookmarksCache.copy()
   }
 
   acceptsBookmark(bm:Bookmark<TItemLocation>):boolean {
@@ -53,6 +53,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
 
   async createBookmark(bm:Bookmark<TItemLocation>):Promise<string|number> {
     Logger.log('CREATE', bm)
+    bm = bm.copy()
     bm.id = ++this.highestId
     const foundFolder = this.bookmarksCache.findFolder(bm.parentId)
     if (!foundFolder) {
@@ -207,7 +208,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
       throw new UnknownCreateTargetError()
     }
     // clone and adjust ids
-    const imported = folder.clone()
+    const imported = folder.copy()
     imported.id = id
     await imported.traverse(async(item, parentFolder) => {
       item.id = ++this.highestId

src/lib/adapters/Git.ts

@@ -184,7 +184,7 @@ export default class GitAdapter extends CachingAdapter {
     Logger.log('onSyncComplete')
     clearInterval(this.lockingInterval)
 
-    this.bookmarksCache = this.bookmarksCache.clone()
+    this.bookmarksCache = this.bookmarksCache.clone(false)
     const newTreeHash = await this.bookmarksCache.hash(true)
     if (newTreeHash !== this.initialTreeHash) {
       const fileContents = this.server.bookmark_file_type === 'xbel' ? createXBEL(this.bookmarksCache, this.highestId) : createHTML(this.bookmarksCache, this.highestId)