Merge branch 'develop'

CHANGELOG.md

@@ -1,3 +1,8 @@
+## v2.2.0
+ - NEW: Map local sync folder to a specific server-side folder
+ - FIX: Performance improvements for Firefox
+ - FIX: Race condition removed that would cause issues because same account would be synced twice in parallel
+
 ## v2.1.0
  - NEW: Allow using an extension key to secure entered credentials
  - FIX: Various fixes for Firefox

ISSUE_TEMPLATE.md

@@ -1,20 +1,21 @@
 <!--
 Hello there! Thank you for providing feedback!
 Please answer the below questions to allow for as smooth a process of tackling the issue as possible :)
+(Note: Everything surrounded by arrows, like this text for example, will not be visible.)
 -->
 
 ### Software versions
 <!-- Please provide the versions of the following software products in your set up -->
 
-* Browser: 
-* Nextcloud: 
-* Nextcloud Bookmarks app: 
-* Floccus: 
+* Browser:
+* Nextcloud:
+* Nextcloud Bookmarks app:
+* Floccus:
 
 ### Steps to reproduce
 <!-- What did you do? Be as specific as possible -->
 1. ...
-2. 
+2.
 
 ### Expected outcome
 <!-- What did you think was going to happen or what do you think should have happened? -->

manifest.json

@@ -2,7 +2,7 @@
   "manifest_version": 2,
   "name": "floccus",
   "short_name": "floccus",
-  "version": "2.1.0",
+  "version": "2.2.0",
   "description": "Sync your bookmarks with nextcloud",
   "icons": {
     "48": "icons/logo.png"

package.json

@@ -1,6 +1,6 @@
 {
   "name": "floccus",
-  "version": "2.1.0",
+  "version": "2.2.0",
   "description": "Sync your bookmarks with nextcloud",
   "main": "index.js",
   "scripts": {

src/entries/options.js

@@ -109,7 +109,13 @@ function renderAccounts (accounts, secured) {
       })
     }</div>
     <a href="" className="btn" id="addaccount" ev-click={() => {
-      Account.create({type: 'nextcloud', url: 'http://example.org', username: 'bob', password: 'password'})
+      Account.create({
+        type: 'nextcloud'
+        , url: 'http://example.org'
+        , username: 'bob'
+        , password: 'password'
+        , serverRoot: ''
+      })
         .then(() => triggerRender())
     }}>Add account</a>
     <div className="security">

src/lib/Account.js

@@ -11,8 +11,7 @@ export default class Account {
     let storage = new AccountStorage(id)
     let background = await browser.runtime.getBackgroundPage()
     let data = await storage.getAccountData(background.controller.key)
-    let localRoot = data.localRoot
-    let tree = new Tree(storage, localRoot)
+    let tree = new Tree(storage, data.localRoot, data.serverRoot)
     return new Account(id, storage, Adapter.factory(data), tree)
   }
 
@@ -64,6 +63,10 @@ export default class Account {
       ...ctl
       , update: (data) => {
         if (JSON.stringify(data) === JSON.stringify(originalData)) return
+        if (originalData.serverRoot !== data.serverRoot) {
+          this.storage.initCache()
+          this.storage.initMappings()
+        }
         ctl.update(data)
       }
     }
@@ -88,7 +91,7 @@ export default class Account {
     }
     await this.storage.initMappings()
     await this.storage.initCache()
-    this.tree = new Tree(this.storage, accData.localRoot)
+    this.tree = new Tree(this.storage, accData.localRoot, accData.serverRoot)
   }
 
   async isInitialized () {
@@ -124,7 +127,10 @@ export default class Account {
       // Server handles existing URLs that we think are new, client handles new URLs that are bookmarked twice locally
       await this.sync_createOnServer(mappings)
 
-      let serverList = await this.server.pullBookmarks()
+      let serverRoot = this.getData().serverRoot
+      let serverList = (await this.server.pullBookmarks())
+        .filter(bm => serverRoot ? bm.path.indexOf(serverRoot) === 0 : true)
+
       // deletes everything locally that is not new but doesn't exist on the server anymore
       await this.sync_deleteFromTree(serverList)
       // Goes through server's list and updates creates things locally as needed
@@ -179,9 +185,10 @@ export default class Account {
           // ignore this bookmark as it's not supported by the server
           return
         }
-        bookmark.id = serverMark.id
-        await this.storage.addToMappings(bookmark)
-        await this.storage.addToCache(bookmark.localId, await serverMark.hash())
+        serverMark.localId = bookmark.localId
+        await this.tree.updateNode(serverMark)
+        await this.storage.addToMappings(serverMark)
+        await this.storage.addToCache(serverMark.localId, await serverMark.hash())
       },
       BATCH_SIZE
     )

src/lib/Bookmark.js

@@ -9,6 +9,10 @@ export default class Bookmark {
     this.path = path
   }
 
+  getLocalPath (serverRoot) {
+    return this.path.substr(serverRoot.length)
+  }
+
   async hash () {
     if (!this.hashValue) {
       this.hashValue = Bookmark.murmur2(JSON.stringify({

src/lib/Controller.js

@@ -27,7 +27,6 @@ class AlarmManger {
 
 export default class Controller {
   constructor () {
-    this.syncing = {}
     this.schedule = {}
 
     this.alarms = new AlarmManger(this)
@@ -126,7 +125,7 @@ export default class Controller {
     // Filter out any accounts that are not tracking the bookmark
       .filter((account, i) => (trackingAccountsFilter[i]))
       // Filter out any accounts that are presently syncing
-      .filter(account => !this.syncing[account.id])
+      .filter(account => !account.getData().syncing)
 
     // We should now sync all accounts that are involved in this change (2 at max)
     accountsToSync.forEach((account) => {
@@ -142,7 +141,7 @@ export default class Controller {
 
     const containingAccount = await Account.getAccountContainingLocalId(localId, ancestors, allAccounts)
     if (containingAccount &&
-      !this.syncing[containingAccount.id] &&
+      !containingAccount.getData().syncing &&
       !accountsToSync.some(acc => acc.id === containingAccount.id)) {
       this.scheduleSyncAccount(containingAccount.id)
     }
@@ -155,29 +154,21 @@ export default class Controller {
     this.schedule[accountId] = setTimeout(() => this.syncAccount(accountId), INACTIVITY_TIMEOUT)
   }
 
-  syncAccount (accountId) {
+  async syncAccount (accountId) {
     if (!this.enabled) {
       return
     }
-    if (this.syncing[accountId]) {
-      return this.syncing[accountId].then(() => {
-        return this.syncAccount(accountId)
-      })
+    let account = await Account.get(accountId)
+    if (account.getData().syncing) {
+      return
     }
-    this.syncing[accountId] = Account.get(accountId)
-      .then((account) => {
-        setTimeout(() => this.updateBadge(), 500)
-        return account.sync()
-      })
-      .then(() => {
-        this.syncing[accountId] = false
-        this.updateBadge()
-      }, (error) => {
-        console.error(error)
-        this.syncing[accountId] = false
-        this.updateBadge()
-      })
-    return this.syncing[accountId]
+    setTimeout(() => this.updateBadge(), 500)
+    try {
+      await account.sync()
+    } catch (error) {
+      console.error(error)
+    }
+    this.updateBadge()
   }
 
   async updateBadge () {

src/lib/Tree.js

@@ -7,8 +7,9 @@ const treeLock = new AsyncLock()
 const reverseStr = (str) => str.split('').reverse().join('')
 
 export default class Tree {
-  constructor (storage, rootId) {
+  constructor (storage, rootId, serverRoot) {
     this.rootId = rootId
+    this.serverRoot = serverRoot
     this.storage = storage
   }
 
@@ -48,14 +49,14 @@ export default class Tree {
           , node.id
           , node.url
           , node.title
-          , parentPath || '/' // only root has a trailing slash
+          , parentPath
         )
         return
       }
-      const descendantPath = (parentPath || '') + '/' + node.title.replace(/[/]/g, '\\/') // other paths don't have a trailing slash
+      const descendantPath = parentPath + '/' + node.title.replace(/[/]/g, '\\/') // other paths don't have a trailing slash
       node.children.map((node) => recurse(node, descendantPath))
     }
-    tree.children.forEach(node => recurse(node))
+    tree.children.forEach(node => recurse(node, this.serverRoot))
   }
 
   getBookmarkByLocalId (localId) {
@@ -79,7 +80,7 @@ export default class Tree {
       throw new Error('trying to create a node for a bookmark that already has one')
     }
 
-    const parentId = await this.mkdirpPath(bookmark.path)
+    const parentId = await this.mkdirpPath(bookmark.getLocalPath(this.serverRoot))
     const node = await browser.bookmarks.create({
       parentId
       , title: bookmark.title
@@ -102,7 +103,7 @@ export default class Tree {
       title: bookmark.title
       , url: bookmark.url
     })
-    const parentId = await this.mkdirpPath(bookmark.path)
+    const parentId = await this.mkdirpPath(bookmark.getLocalPath(this.serverRoot))
     await browser.bookmarks.move(bookmark.localId, {parentId})
   }
 
@@ -170,11 +171,11 @@ export default class Tree {
       ancestors = ancestors.slice(ancestors.indexOf(relativeToRoot) + 1)
     }
 
-    return '/' + (await Promise.all(
+    return (await Promise.all(
       ancestors
         .map(async ancestor => {
           try {
-            let bms = await browser.bookmarks.getSubTree(ancestor)
+            let bms = await browser.bookmarks.get(ancestor)
             let bm = bms[0]
             return bm.title.replace(/[/]/g, '\\/')
           } catch (e) {
@@ -225,7 +226,7 @@ export default class Tree {
       return path
     }
     path.unshift(localId)
-    let bms = await browser.bookmarks.getSubTree(localId)
+    let bms = await browser.bookmarks.get(localId)
     let bm = bms[0]
     if (bm.parentId === localId) {
       return path // might be that the root is circular

src/lib/adapters/Nextcloud.js

@@ -23,17 +23,29 @@ export default class NextcloudAdapter {
 
   renderOptions (ctl, rootPath) {
     let data = this.getData()
+    const saveTimeout = 1000
     let onchangeURL = (e) => {
       if (this.saveTimeout) clearTimeout(this.saveTimeout)
-      this.saveTimeout = setTimeout(() => ctl.update({...data, url: e.target.value}), 300)
+      this.saveTimeout = setTimeout(() => ctl.update({...data, url: e.target.value}), saveTimeout)
     }
     let onchangeUsername = (e) => {
       if (this.saveTimeout) clearTimeout(this.saveTimeout)
-      this.saveTimeout = setTimeout(() => ctl.update({...data, username: e.target.value}), 300)
+      this.saveTimeout = setTimeout(() => ctl.update({...data, username: e.target.value}), saveTimeout)
     }
     let onchangePassword = (e) => {
       if (this.saveTimeout) clearTimeout(this.saveTimeout)
-      this.saveTimeout = setTimeout(() => ctl.update({...data, password: e.target.value}), 300)
+      this.saveTimeout = setTimeout(() => ctl.update({...data, password: e.target.value}), saveTimeout)
+    }
+    let onchangeServerRoot = (e) => {
+      if (this.saveTimeout) clearTimeout(this.saveTimeout)
+      this.saveTimeout = setTimeout(() => {
+        let val = e.target.value
+        if (val[val.length - 1] === '/') {
+          val = val.substr(0,  val.length - 1)
+          e.target.value = val
+        }
+        ctl.update({...data, serverRoot: e.target.value})
+      }, saveTimeout)
     }
     return <div className="account">
       <form>
@@ -44,11 +56,16 @@ export default class NextcloudAdapter {
           </tr>
           <tr>
             <td><label for="username">User name:</label></td>
-            <td><input value={new InputInitializeHook(data.username)} type="text" className="username" name="password" ev-keyup={onchangeUsername} ev-blur={onchangeUsername}/></td>
+            <td><input value={new InputInitializeHook(data.username)} type="text" className="username" name="username" ev-keyup={onchangeUsername} ev-blur={onchangeUsername}/></td>
           </tr>
           <tr>
             <td><label for="password">Password:</label></td>
-            <td><input value={new InputInitializeHook(data.password)} type="password" className="password" name="password" ev-keydown={onchangePassword} ev-blur={onchangePassword}/></td></tr>
+            <td><input value={new InputInitializeHook(data.password)} type="password" className="password" name="password" ev-keydown={onchangePassword} ev-blur={onchangePassword}/></td>
+          </tr>
+          <tr>
+            <td><label for="serverRoot">Server path:</label></td>
+            <td><input value={new InputInitializeHook(data.serverRoot || '')} type="text" className="serverRoot" name="serverRoot" placeholder="Default: root folder  Example: /my/subfolder" ev-keyup={onchangeServerRoot} ev-blur={onchangeServerRoot}/></td>
+          </tr>
           <tr><td></td><td>
             <span className="status">{
               data.syncing
@@ -78,7 +95,7 @@ export default class NextcloudAdapter {
             <div className="options">
               <formgroup>
                 <h4>Sync folder</h4>
-                <input type="text" disabled value={rootPath} /><br/>
+                <input type="text" disabled placeholder="*Root folder*" value={rootPath} /><br/>
                 <a href="" title="Reset synchronized folder to create a new one" className={'btn resetRoot ' + (data.syncing ? 'disabled' : '')} ev-click={() => {
                   !data.syncing && ctl.update({...data, localRoot: null})
                 }}>Reset</a>
@@ -342,7 +359,7 @@ export default class NextcloudAdapter {
   static getPathTagFromTags (tags) {
     return (tags || [])
       .filter(tag => tag.indexOf(TAG_PREFIX) === 0)
-      .concat([this.convertPathToTag('/')])[0]
+      .concat([this.convertPathToTag('')])[0]
   }
 
   static convertPathToTag (path) {

src/test/index.js

@@ -22,7 +22,7 @@ describe('Floccus', function () {
   describe('Account', function () {
     var account
     beforeEach('set up dummy account', async function () {
-      account = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r'})
+      account = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r', serverRoot: ''})
     })
     afterEach('clean up dummy account', async function () {
       if (account) await account.delete()
@@ -33,7 +33,7 @@ describe('Floccus', function () {
     })
     it('should save and restore an account', async function () {
       console.log(this.test.title)
-      const newData = {type: 'fake', username: 'bar', url: 'https://fo.o'}
+      const newData = {type: 'fake', username: 'bar', url: 'https://fo.o', serverRoot: ''}
       await account.setData(newData)
       expect(account.getData()).to.deep.equal(newData)
 
@@ -55,7 +55,7 @@ describe('Floccus', function () {
     context('with one client', function () {
       var account
       beforeEach('set up dummy account', async function () {
-        account = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r'})
+        account = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r', serverRoot: ''})
         await account.init()
       })
       afterEach('clean up dummy account', async function () {
@@ -202,9 +202,9 @@ describe('Floccus', function () {
     context('with two clients', function () {
       var account1, account2
       beforeEach('set up dummy accounts', async function () {
-        account1 = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r'})
+        account1 = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r', serverRoot: ''})
         await account1.init()
-        account2 = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r'})
+        account2 = await Account.create({type: 'fake', username: 'foo', url: 'http://ba.r', serverRoot: ''})
         await account2.init()
 
         // Wrire both accounts to the same fake db
@@ -285,12 +285,12 @@ describe('Floccus', function () {
 
         const bookmarksAfterSyncing = await adapter.pullBookmarks()
         expect(bookmarksAfterSyncing).to.have.lengthOf(1)
-        expect(bookmarksAfterSyncing[0].path).to.equal('/')
+        expect(bookmarksAfterSyncing[0].path).to.equal('')
 
         await account1.tree.load()
         await account2.tree.load()
-        expect(account1.tree.getBookmarkByLocalId(bookmark1.id).path).to.equal('/')
-        expect(account2.tree.getBookmarkByLocalId(bookmark2.id).path).to.equal('/')
+        expect(account1.tree.getBookmarkByLocalId(bookmark1.id).path).to.equal('')
+        expect(account2.tree.getBookmarkByLocalId(bookmark2.id).path).to.equal('')
       })
     })
   })