tests: New test: Concurrent insert and reorder

and make it pass the test

Signed-off-by: Marcel Klehr <[email protected]>

Author: marcelklehr

src/lib/Scanner.ts

@@ -74,13 +74,22 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
       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
+    // Generate REORDERS before diffing anything to make sure REORDERS are from top to bottom (necessary for tab sync)
     if (newFolder.children.length > 1) {
-      this.result.REORDER.commit({
-        type: ActionType.REORDER,
-        payload: newFolder,
-        order: newFolder.children.map(i => ({ type: i.type, id: i.id })),
-      })
+      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])) {
+          needReorder = true
+          break
+        }
+      }
+      if (needReorder) {
+        this.result.REORDER.commit({
+          type: ActionType.REORDER,
+          payload: newFolder,
+          order: newFolder.children.map(i => ({ type: i.type, id: i.id })),
+        })
+      }
     }
 
     // Preserved Items and removed Items
@@ -105,12 +114,12 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
 
     // created Items
     // (using map here, because 'each' doesn't provide indices)
-    await Parallel.map(unmatchedChildren, async(newChild, index) => {
+    await Parallel.map(unmatchedChildren, async(newChild) => {
       if (oldFolder.isRoot && oldFolder.location === ItemLocation.LOCAL) {
         // We can't create root folders locally
         return
       }
-      this.result.CREATE.commit({type: ActionType.CREATE, payload: newChild, index})
+      this.result.CREATE.commit({type: ActionType.CREATE, payload: newChild, index: newFolder.children.findIndex(child => child === newChild)})
     }, 1)
   }
 

src/lib/strategies/Default.ts

@@ -772,7 +772,7 @@ export default class SyncProcess {
         return
       }
 
-      if (targetLocation === this.masterLocation) {
+      if (targetLocation !== this.masterLocation) {
         const concurrentReorder = targetReorders.find(a =>
           action.payload.type === a.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, a.payload))
         if (concurrentReorder) {

src/test/test.js

@@ -5232,6 +5232,132 @@ describe('Floccus', function() {
             expectTreeEqual(secondLocalTree1, secondLocalTree2, true, true)
           })
 
+          it('should synchronize ordering (2)', async function() {
+            if (ACCOUNT_DATA.type === 'linkwarden' || ACCOUNT_DATA.type === 'karakeep') {
+              return this.skip()
+            }
+            expect(
+              (await getAllBookmarks(account1)).children
+            ).to.have.lengthOf(0)
+
+            const localRoot1 = account1.getData().localRoot
+            const fooFolder = await browser.bookmarks.create({
+              title: 'foo',
+              parentId: localRoot1
+            })
+            const folder11 = await browser.bookmarks.create({
+              title: 'folder11',
+              parentId: fooFolder.id
+            })
+            const folder12 = await browser.bookmarks.create({
+              title: 'folder12',
+              parentId: fooFolder.id
+            })
+            const bookmark11 = await browser.bookmarks.create({
+              title: 'url11',
+              url: 'http://ur.l/',
+              parentId: fooFolder.id
+            })
+            const bookmark12 = await browser.bookmarks.create({
+              title: 'url12',
+              url: 'http://ur.ll/',
+              parentId: fooFolder.id
+            })
+
+            await account1.sync()
+            expect(account1.getData().error).to.not.be.ok
+
+            await account2.sync()
+            expect(account2.getData().error).to.not.be.ok
+
+            console.log('Checking local folder after initial sync')
+
+            const localTree1 = await account1.localTree.getBookmarksTree(true)
+            const localTree2 = await account2.localTree.getBookmarksTree(true)
+            localTree2.title = localTree1.title
+            expectTreeEqual(localTree1, localTree2, true, true)
+
+            const localTree1Foo = localTree1.children.find(item => item.title === 'foo')
+            const localTree2Foo = localTree2.children.find(item => item.title === 'foo')
+
+            const newBookmark1 = await browser.bookmarks.create({
+              title: 'newBookmark1',
+              url: 'http://ur.lllll/',
+              parentId: localTree1Foo.id
+            })
+            await browser.bookmarks.move(newBookmark1.id, { index: 0 })
+            await browser.bookmarks.move(folder11.id, { index: 4 })
+            await browser.bookmarks.move(folder12.id, { index: 3 })
+            await browser.bookmarks.move(bookmark11.id, { index: 2 })
+            await browser.bookmarks.move(bookmark12.id, { index: 1 })
+
+            const newBookmark2 = await browser.bookmarks.create({
+              title: 'newBookmark2',
+              url: 'http://ur.llllll/',
+              parentId: localTree2Foo.id
+            })
+            await browser.bookmarks.move(newBookmark2.id, { index: 3 })
+
+            await account1.sync()
+            expect(account1.getData().error).to.not.be.ok
+
+            await account2.sync()
+            expect(account2.getData().error).to.not.be.ok
+
+            await account1.sync()
+            expect(account1.getData().error).to.not.be.ok
+
+            const secondLocalTree1 = await account1.localTree.getBookmarksTree(
+              true
+            )
+
+            expectTreeEqual(
+              secondLocalTree1,
+              new Folder({
+                title: secondLocalTree1.title,
+                children: [
+                  new Folder({
+                    title: 'foo',
+                    children: [
+                      new Bookmark({
+                        title: 'newBookmark1',
+                        url: newBookmark1.url
+                      }),
+                      new Bookmark({
+                        title: 'url12',
+                        url: bookmark12.url
+                      }),
+                      new Bookmark({
+                        title: 'url11',
+                        url: bookmark11.url
+                      }),
+                      new Folder({
+                        title: 'folder12',
+                        children: []
+                      }),
+                      new Bookmark({
+                        title: 'newBookmark2',
+                        url: newBookmark2.url
+                      }),
+                      new Folder({
+                        title: 'folder11',
+                        children: []
+                      }),
+                    ]
+                  }),
+                ]
+              }),
+              true,
+              true
+            )
+
+            const secondLocalTree2 = await account2.localTree.getBookmarksTree(
+              true
+            )
+            secondLocalTree2.title = secondLocalTree1.title
+            expectTreeEqual(secondLocalTree1, secondLocalTree2, true, true)
+          })
+
           // Skipping this, because nextcloud adapter currently
           // isn't able to track bookmarks across dirs, thus in this
           // scenario both bookmarks survive :/