Merge branch 'develop'

Author: marcelklehr

CHANGELOG.md

@@ -1,3 +1,8 @@
+## v2.0.1
+ - FIX: Don't remove folders beyond the sync folder when the last bookmark is remove
+ - FIX: Declare incompatibility with Fx < v57
+ - FIX: Improve error reporting
+
 ## v2.0.0
  - NEW: Sync folder hierarchy
  - NEW: Allow custom folders to be chosen for syncing

README.md

@@ -41,9 +41,39 @@ If you want to sync all bookmarks in your browser you need to select the topmost
 If something goes wrong during the sync process the floccus icon will sport a red exclamation mark. In the options pane you can then hover over the status text to get more information about the error.
 
 ### Limitations
- * Note that currently you cannot sync the same folder with multiple accounts in order to avoid data corruption. If you sync the root folder with one account and sync a sub folder with a different account, that sub-folder will not be synced with the account connected to the root folder anymore.
+ * Note that currently you cannot sync the same folder with multiple nextcloud accounts in order to avoid data corruption. If you sync the root folder with one account and sync a sub folder with a different account, that sub-folder will not be synced with the account connected to the root folder anymore.
  * Floccus yields an error if you attempt to sync a folder with duplicate bookmarks (two or more bookmarks of the same URL). Remove one of the bookmarks for floccus to resume normal functionality.
 
+## Goals and Limitations aka. Is this a good idea?
+As there have been debates about whether this software product is a good idea, I've made a little section here with my considerations.
+
+### Goals
+The goals of this piece of software
+
+ * provide an open cross-platform sync solution for browser data with nextcloud
+ * performance is a plus, but not necessary
+ * (eventual) consistency is more important than intention preservation (i.e. when ever a mistake happens during sync, it's guaranteed to be eventually consistent on all sites)
+
+
+### Current status and Limitations
+The WebExtensions bookmarks API has a few limitations:
+
+1. No support for batching or transactions
+2. Record GUIDs can change, but are only known to change when Firefox Sync is used.
+3. The data format doesn't represent descriptions, tags or separators
+4. No way to create a per-device folder
+5. It's impossible to express safe operations, because there are no compare-and-set primitives.
+6. Triggering a sync after the first change, causing repeated syncs and inconsistency to spread to other devices.
+
+Nonetheless, I've chosen to utilize the WebExtensions API for implementing this sync client. As I'm aware, this decision has (at least) the following consequences:
+1. No transaction support (\#1) leads to bad performance
+2. No support for transactions (\#1) also can potentially cause intermediate states to be synced. However, all necessary precautions are taken to prevent this and even in the case that this happens, all sites will be eventually consistent, allowing you to manually resolve possible problems after the fact.
+3. Due to the modification of GUIDs (\#2), usage of Firefox Sync along with Floccus is discouraged.
+4. The incomplete data format (\#3) is an open problem, but doesn't impact the synchronization of the remaining accessible data.
+5. The inability to exclude folders from sync in 3rd-party extensions (\#4) is a problem, but manageable when users are able to manually choose folders to ignore. (Currently not implemented)
+6. The lack of safe write operations (\#5) can be dealt with similarly to the missing transaction support: Changes made during sync could lead to an unintended but consistent state, which can be resolved manually. Additionally, precautions are taken to prevent this.
+7. In order to avoid syncing prematurely (\#6) floccus can employ a timeout to wait until all pending bookmarks operations are done. (Currently not implemented.)
+
 ## What's with the name?
 [Cirrus floccus](https://en.wikipedia.org/wiki/Cirrus_floccus) is a type of cloud, that <del>can sync your browser data</del> looks very nice.
 

gulpfile.js

@@ -1,35 +1,47 @@
-var gulp = require('gulp');
-var browserify = require('browserify');
-var babelify = require('babelify');
-var tap = require('gulp-tap');
+var gulp = require('gulp')
+var browserify = require('browserify')
+var babelify = require('babelify')
+var tap = require('gulp-tap')
 
-gulp.task('js', ['src', 'polyfill'])
+gulp.task('default', ['html', 'js', '3rd-party'])
 
-gulp.task('src', function () {
+gulp.task('js', function () {
   return gulp.src('src/entries/*.js', {read: false}) // no need of reading file because browserify does.
     // transform file objects using gulp-tap plugin
     .pipe(tap(function (file) {
       // replace file contents with browserify's bundle stream
       file.contents = browserify(file.path, {
-				debug: true,
- 			})
-      .transform(babelify, {presets: ["es2015"], plugins: [
-        "transform-object-rest-spread"
-      , "syntax-jsx"
-      , "transform-react-jsx"
-      , "transform-async-to-generator"
-      ]}) 
-      .bundle()
+        debug: true,
+      })
+        .transform(babelify, {
+          presets: ['es2015']
+          , plugins: [
+            'transform-object-rest-spread'
+            , 'syntax-jsx'
+            , 'transform-react-jsx'
+            , 'transform-async-to-generator'
+          ]
+        })
+        .bundle()
     }))
-    .pipe(gulp.dest('./dist/js'));
+    .pipe(gulp.dest('./dist/js/'))
 })
 
-gulp.task('polyfill', function() {
-  return gulp.src('./node_modules/babel-polyfill/dist/polyfill.js').pipe(gulp.dest('./dist/js/'))
+gulp.task('html', function () {
+  return gulp.src('./views/*.html').pipe(gulp.dest('./dist/html/'))
 })
 
-gulp.task('html', function() {
-  return gulp.src('./views/*.html').pipe(gulp.dest('./dist/html/'))
+gulp.task('3rd-party', ['polyfill', 'mocha'])
+
+gulp.task('polyfill', function () {
+  return gulp.src('./node_modules/babel-polyfill/dist/polyfill.js').pipe(gulp.dest('./dist/js/'))
 })
 
-gulp.task('default', ['html', 'js'])
+gulp.task('mocha', ['mochajs', 'mochacss'])
+
+gulp.task('mochajs', function () {
+  return gulp.src('./node_modules/mocha/mocha.js').pipe(gulp.dest('./dist/js/'))
+})
+gulp.task('mochacss', function () {
+  return gulp.src('./node_modules/mocha/mocha.css').pipe(gulp.dest('./dist/css/'))
+})

manifest.json

@@ -2,7 +2,7 @@
   "manifest_version": 2,
   "name": "Floccus (nextcloud Sync)",
   "short_name": "Floccus",
-  "version": "2.0.0",
+  "version": "2.0.1",
   "description": "Sync your browser with nextcloud (currently only bookmarks; more to come)",
   "icons": {
     "48": "icons/logo.png"
@@ -11,7 +11,7 @@
   "applications": {
     "gecko": {
       "id": "floccus@handmadeideas.org",
-      "strict_min_version": "48.0"
+      "strict_min_version": "57.0"
     }
   },
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "floccus",
-  "version": "2.0.0",
+  "version": "2.0.1",
   "description": "The goal of this project is to build a browser extension that syncs your browser data with [OwnCloud](http://owncloud.org).",
   "main": "index.js",
   "scripts": {
@@ -24,8 +24,11 @@
     "babel-preset-es2015": "^6.24.1",
     "babelify": "^7.3.0",
     "browserify": "^14.3.0",
+    "chai": "^4.1.2",
+    "chai-as-promised": "^7.1.1",
     "gulp": "^3.9.1",
     "gulp-tap": "^1.0.1",
+    "mocha": "^5.0.4",
     "through2": "^2.0.3"
   },
   "dependencies": {

src/entries/background-script.js

@@ -54,9 +54,17 @@ class Controller {
     })
 
     window.syncAccount = (accountId) => this.syncAccount(accountId)
+    this.setEnabled(true)
+  }
+
+  setEnabled(enabled) {
+    this.enabled = enabled
   }
 
   async onchange (localId, details) {
+    if (!this.enabled) {
+      return
+    }
     const allAccounts = await Account.getAllAccounts()
 
     // Check which accounts contain the bookmark and which used to contain (track) it
@@ -94,6 +102,9 @@ class Controller {
   }
 
   syncAccount (accountId) {
+    if (!this.enabled) {
+      return
+    }
     if (this.syncing[accountId]) {
       return this.syncing[accountId].then(() => {
         return this.syncAccount(accountId)
@@ -148,4 +159,4 @@ class Controller {
   }
 }
 
-var controller = new Controller()
+window.controller = new Controller()

src/entries/options.js

@@ -92,6 +92,7 @@ function renderAccounts (accounts) {
       Account.create({type: 'nextcloud', url: 'http://example.org', username: 'bob', password: 'password'})
         .then(() => triggerRender())
     }}>Add account</a>
+    <a className="test-link" href="./test.html">run tests</a>
   </div>
 }
 function renderPicker (cb, tree) {

src/entries/test.js

@@ -0,0 +1,3 @@
+mocha.setup('bdd')
+require('../test/index.js')
+mocha.run();

src/lib/Account.js

@@ -16,7 +16,7 @@ export default class Account {
   }
 
   static async create (data) {
-    let id = Math.floor(Math.random() * 10000000000)
+    let id = '' + Math.floor(Math.random() * 10000000000)
     let storage = new AccountStorage(id)
 
     await storage.setAccountData(data)
@@ -44,7 +44,7 @@ export default class Account {
   }
 
   async setData (data) {
-    this.server = Adapter.factory(data)
+    this.server.setData(data)
     await this.storage.setAccountData(data)
   }
 
@@ -156,7 +156,7 @@ export default class Account {
     let nodes = await this.tree.getAllNodes()
     await ParallelArray.from(
       nodes
-        .filter(node => !mappings.LocalToServer[node.id])
+        .filter(node => typeof mappings.LocalToServer[node.id] === 'undefined')
     )
       .asyncForEach(async node => {
         if (mappings.UrlToLocal[node.url]) {
@@ -179,15 +179,17 @@ export default class Account {
     var [
       serverMarks
       , cache
+      , mappings
     ] = await Promise.all([
       this.server.pullBookmarks()
       , this.storage.getCache()
+      , this.storage.getMappings() // For detecting duplicates
     ])
     var received = {}
     // Update known ones and create new ones
     await ParallelArray.from(serverMarks)
       .asyncForEach(async serverMark => {
-        if (await this.tree.getLocalIdOf(serverMark)) {
+        if (typeof (await this.tree.getLocalIdOf(serverMark)) !== 'undefined') {
         // known to mappings: (LOCAL|SERVER)UPDATE
           received[serverMark.localId] = serverMark // .localId is only avaiable after Tree#getLocalIdOf(...)
 
@@ -220,6 +222,10 @@ export default class Account {
         } else {
         // Not yet known:
         // CREATE
+          if (mappings.UrlToLocal[serverMark.url]) {
+            console.error('Trying to create a URL that is already bookmarked. This shouldn\'t happen! Please tell the developer about this! url=' + serverMark.url)
+            throw new Error('Trying to create a URL that is already bookmarked. This shouldn\'t happen! Please tell the developer about this! url=' + serverMark.url)
+          }
           const node = await this.tree.createNode(serverMark)
           await this.storage.addToCache(node.id, await serverMark.hash())
           received[node.id] = serverMark

src/lib/Adapter.js

@@ -1,4 +1,5 @@
 import NextcloudAdapter from './adapters/Nextcloud'
+import FakeAdapter from './adapters/Fake'
 
 export default class Adapter {
   static factory (data) {
@@ -7,6 +8,9 @@ export default class Adapter {
       case 'nextcloud':
         adapter = new NextcloudAdapter(data)
         break
+      case 'fake':
+        adapter = new FakeAdapter(data)
+        break
       default:
         throw new Error('Unknown account type')
     }
@@ -17,6 +21,10 @@ export default class Adapter {
     throw new Error('Cannot instantiate abstract class')
   }
 
+  setData () {
+    throw new Error('Not implemented')
+  }
+
   getData () {
     throw new Error('Not implemented')
   }