From b8b1e70f3077252f257294be2b02d824ded92edc Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 3 May 2018 02:09:58 +0200 Subject: [PATCH 1/9] Implement serverRoot setting --- src/entries/options.js | 8 +++++++- src/lib/Account.js | 14 ++++++++++---- src/lib/Bookmark.js | 4 ++++ src/lib/Tree.js | 19 ++++++++++--------- src/lib/adapters/Nextcloud.js | 29 +++++++++++++++++++++++------ 5 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/entries/options.js b/src/entries/options.js index 198c16b528..3936ee1e9e 100644 --- a/src/entries/options.js +++ b/src/entries/options.js @@ -109,7 +109,13 @@ function renderAccounts (accounts, secured) { }) } { - 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
diff --git a/src/lib/Account.js b/src/lib/Account.js index abc79a2c62..d1a5ebefe2 100644 --- a/src/lib/Account.js +++ b/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 => bm.path.indexOf(serverRoot) === 0) + // 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 diff --git a/src/lib/Bookmark.js b/src/lib/Bookmark.js index 2ca356d666..73ca1163b4 100644 --- a/src/lib/Bookmark.js +++ b/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({ diff --git a/src/lib/Tree.js b/src/lib/Tree.js index b7b82f7fcd..5c6d7b730d 100644 --- a/src/lib/Tree.js +++ b/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,7 +171,7 @@ export default class Tree { ancestors = ancestors.slice(ancestors.indexOf(relativeToRoot) + 1) } - return '/' + (await Promise.all( + return (await Promise.all( ancestors .map(async ancestor => { try { @@ -198,7 +199,7 @@ export default class Tree { .split(/[/](?![\\])/) .reverse() .map(str => reverseStr(str)) - let pathSegment = pathArr[1] + let pathSegment = pathArr[0] let title = pathSegment.replace(/[\\][/]/g, '/') let child = await treeLock.acquire(rootId, async () => { @@ -216,7 +217,7 @@ export default class Tree { if (allAccounts.some(acc => acc.getData().localRoot === child.id)) { throw new Error('New path conflicts with existing nested floccus folder. Aborting.') } - return Tree.mkdirpPath('/' + pathArr.slice(2).join('/'), child.id, allAccounts) + return Tree.mkdirpPath(pathArr.slice(2).join('/'), child.id, allAccounts) } static async getIdPathFromLocalId (localId, path) { diff --git a/src/lib/adapters/Nextcloud.js b/src/lib/adapters/Nextcloud.js index 57b61027ae..31c242c851 100644 --- a/src/lib/adapters/Nextcloud.js +++ b/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
@@ -44,11 +56,16 @@ export default class NextcloudAdapter { - + - + + + + + + { data.syncing @@ -78,7 +95,7 @@ export default class NextcloudAdapter {

Sync folder

-
+
{ !data.syncing && ctl.update({...data, localRoot: null}) }}>Reset From 9edf4c5f8f2b55b40cf088341e2dda4d6bf00452 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 May 2018 15:43:26 +0200 Subject: [PATCH 2/9] UI: update placeholder for serverRoot --- src/lib/adapters/Nextcloud.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/adapters/Nextcloud.js b/src/lib/adapters/Nextcloud.js index 31c242c851..5aaf4807b7 100644 --- a/src/lib/adapters/Nextcloud.js +++ b/src/lib/adapters/Nextcloud.js @@ -64,7 +64,7 @@ export default class NextcloudAdapter { - + { From 57ff6cdcbca21d028f88f81677aa1da361c4438f Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 4 May 2018 17:22:44 +0200 Subject: [PATCH 3/9] Speedup fix for firefox --- src/lib/Tree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Tree.js b/src/lib/Tree.js index 5c6d7b730d..d490e4e8ad 100644 --- a/src/lib/Tree.js +++ b/src/lib/Tree.js @@ -226,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 From 89daa27c05b94225f6472f10c398ea9ed0e580a8 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 13:41:17 +0200 Subject: [PATCH 4/9] Some fixes --- src/lib/adapters/Nextcloud.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/adapters/Nextcloud.js b/src/lib/adapters/Nextcloud.js index 5aaf4807b7..57cc2d4952 100644 --- a/src/lib/adapters/Nextcloud.js +++ b/src/lib/adapters/Nextcloud.js @@ -64,7 +64,7 @@ export default class NextcloudAdapter { - + { @@ -359,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) { From 784dad4690fc205153e325cd241bbae1cbe6db4d Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 14:13:37 +0200 Subject: [PATCH 5/9] Fix syncing code path synchronization --- src/lib/Controller.js | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/lib/Controller.js b/src/lib/Controller.js index cb2b33b87f..c68bb808e4 100644 --- a/src/lib/Controller.js +++ b/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 () { From 886ff8bf9a3c8b7adbc5c421f38fe1605e292e12 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 19:19:07 +0200 Subject: [PATCH 6/9] Another firefox performance speedup --- src/lib/Tree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Tree.js b/src/lib/Tree.js index d490e4e8ad..2f27481e6f 100644 --- a/src/lib/Tree.js +++ b/src/lib/Tree.js @@ -175,7 +175,7 @@ export default class Tree { 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) { From 3cccd0c60a2ad15ddf879ebbd96418069d75a841 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 19:34:58 +0200 Subject: [PATCH 7/9] Make tests pass --- src/lib/Account.js | 2 +- src/lib/Tree.js | 4 ++-- src/test/index.js | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib/Account.js b/src/lib/Account.js index d1a5ebefe2..5bbe764412 100644 --- a/src/lib/Account.js +++ b/src/lib/Account.js @@ -129,7 +129,7 @@ export default class Account { let serverRoot = this.getData().serverRoot let serverList = (await this.server.pullBookmarks()) - .filter(bm => bm.path.indexOf(serverRoot) === 0) + .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) diff --git a/src/lib/Tree.js b/src/lib/Tree.js index 2f27481e6f..49f38e8622 100644 --- a/src/lib/Tree.js +++ b/src/lib/Tree.js @@ -199,7 +199,7 @@ export default class Tree { .split(/[/](?![\\])/) .reverse() .map(str => reverseStr(str)) - let pathSegment = pathArr[0] + let pathSegment = pathArr[1] let title = pathSegment.replace(/[\\][/]/g, '/') let child = await treeLock.acquire(rootId, async () => { @@ -217,7 +217,7 @@ export default class Tree { if (allAccounts.some(acc => acc.getData().localRoot === child.id)) { throw new Error('New path conflicts with existing nested floccus folder. Aborting.') } - return Tree.mkdirpPath(pathArr.slice(2).join('/'), child.id, allAccounts) + return Tree.mkdirpPath('/' + pathArr.slice(2).join('/'), child.id, allAccounts) } static async getIdPathFromLocalId (localId, path) { diff --git a/src/test/index.js b/src/test/index.js index cd18916fcb..7be9368be6 100644 --- a/src/test/index.js +++ b/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('') }) }) }) From b72a4ea4c9352106b14f58f0c5994b7f3a2a29f2 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 20:18:46 +0200 Subject: [PATCH 8/9] Fix some problems by overtaking data from the server on creation in case it was changed --- src/lib/Account.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/Account.js b/src/lib/Account.js index 5bbe764412..bb4547863a 100644 --- a/src/lib/Account.js +++ b/src/lib/Account.js @@ -185,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 ) From 37e7a53779457d5046df4fe10a358fce45b8ddd8 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 9 May 2018 20:28:38 +0200 Subject: [PATCH 9/9] v2.2.0 --- CHANGELOG.md | 5 +++++ ISSUE_TEMPLATE.md | 11 ++++++----- manifest.json | 2 +- package.json | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be270555f8..ac9f1ac50c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ISSUE_TEMPLATE.md b/ISSUE_TEMPLATE.md index 5e496b03da..df584abbdc 100644 --- a/ISSUE_TEMPLATE.md +++ b/ISSUE_TEMPLATE.md @@ -1,20 +1,21 @@ ### Software versions -* Browser: -* Nextcloud: -* Nextcloud Bookmarks app: -* Floccus: +* Browser: +* Nextcloud: +* Nextcloud Bookmarks app: +* Floccus: ### Steps to reproduce 1. ... -2. +2. ### Expected outcome diff --git a/manifest.json b/manifest.json index 576414f638..70208d6a68 100644 --- a/manifest.json +++ b/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" diff --git a/package.json b/package.json index 90fa3bf471..6a70b384dd 100644 --- a/package.json +++ b/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": {