Sync: Resend pending records periodically

Fix brave/sync#144

Also fixes some imported bookmarks lost until restart.
See also: brave/sync#112

Test Plan:
1. Pyramid 0: Open and enable Sync.
2. Pyramid 1: Open and join Pyramid 0's Sync profile. Close browser.
3. P_0: Import 1000s of bookmarks.
4. P_0: Wait for Sync uploads to finish (console shows "got 0 decrypted records in BOOKMARKS after {timestamp}"). Wait an additional 3 minutes.
6. P_0: Export bookmarks to HTML file.
7. P_1: Open and wait for Sync download (~1m).
8. P_1: Export bookmarks to HTML file.
9. Run `wc -l {P_0 bookmarks HTML}` and `wc -l {P_0 bookmarks HTML}` and compare. These should match.

app/common/lib/jsonUtil.js

@@ -0,0 +1,24 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+const JSON_POINTER_ESCAPE_CODES = {
+  '~0': '~',
+  '~1': '/'
+}
+
+const JSON_POINTER_REGEX = new RegExp(Object.keys(JSON_POINTER_ESCAPE_CODES).join('|'), 'gi')
+
+/**
+ * Unescape a JSON pointer path part. (i.e. ~
+ * https://stackoverflow.com/questions/31483170/purpose-of-tilde-in-json-pointer
+ * @param {string} string JSON pointer part.
+ * @returns {string}
+ */
+module.exports.unescapeJSONPointer = (string) => {
+  return string.replace(JSON_POINTER_REGEX, (match) => {
+    return JSON_POINTER_ESCAPE_CODES[match]
+  })
+}

app/common/state/syncPendState.js

@@ -54,6 +54,7 @@ module.exports.getPendingRecords = (state) => {
  * @returns {Immutable.Map} new app state
  */
 module.exports.confirmRecords = (state, downloadedRecords) => {
+  let recordsConfirmed = 0
   downloadedRecords.forEach(record => {
     // browser-laptop stores byte arrays like objectId as Arrays.
     // downloaded records use Uint8Arrays which we should convert back.
@@ -64,6 +65,11 @@ module.exports.confirmRecords = (state, downloadedRecords) => {
       return
     }
     state = state.deleteIn(['sync', 'pendingRecords', key])
+    recordsConfirmed += 1
   })
+  if (recordsConfirmed > 0) {
+    const t = new Date().getTime()
+    state = state.setIn(['sync', 'lastConfirmedRecordTimestamp'], t)
+  }
   return state
 }

app/sessionStore.js

@@ -623,7 +623,8 @@ module.exports.defaultAppState = () => {
       devices: {},
       lastFetchTimestamp: 0,
       objectsById: {},
-      pendingRecords: {}
+      pendingRecords: {},
+      lastConfirmedRecordTimestamp: 0
     },
     locationSiteKeysCache: undefined,
     sites: getTopSiteMap(),

app/sync.js

@@ -25,6 +25,7 @@ const syncPendState = require('./common/state/syncPendState')
 const getSetting = require('../js/settings').getSetting
 const settings = require('../js/constants/settings')
 const extensions = require('./extensions')
+const {unescapeJSONPointer} = require('./common/lib/jsonUtil')
 
 const CATEGORY_MAP = syncUtil.CATEGORY_MAP
 const CATEGORY_NAMES = Object.keys(categories)
@@ -79,7 +80,7 @@ const appStoreChangeCallback = function (diffs) {
       return
     }
 
-    const statePath = path.slice(1, 3).map((item) => item.replace(/~1/g, '/'))
+    const statePath = path.slice(1, 3).map(unescapeJSONPointer)
     if (field === 'skipSync' && diff.value === true) {
       // Remove the flag so that this gets synced next time it is updated
       appActions.setSkipSync(statePath, false)
@@ -231,11 +232,20 @@ module.exports.onSyncReady = (isFirstRun, e) => {
   AppStore.addChangeListener(appStoreChangeCallback)
   const appState = AppStore.getState()
 
-  // Sync records which haven't been confirmed yet.
-  const oldPendingRecords = syncPendState.getPendingRecords(appState)
-  if (oldPendingRecords.length > 0) {
-    log(`Sending ${oldPendingRecords.length} pending records`)
-    e.sender.send(syncMessages.SEND_SYNC_RECORDS, undefined, oldPendingRecords)
+  // poll() calls this periodically.
+  const resendPendingRecords = () => {
+    const state = AppStore.getState()
+    const timeSinceLastConfirm = new Date().getTime() - state.getIn(['sync', 'lastConfirmedRecordTimestamp'])
+    if (timeSinceLastConfirm < config.resendPendingRecordInterval) {
+      return
+    }
+    // Sync records which haven't been confirmed yet.
+    const pendingRecords = syncPendState.getPendingRecords(state)
+    if (pendingRecords.length === 0) {
+      return
+    }
+    log(`Resending ${pendingRecords.length} pending records`)
+    e.sender.send(syncMessages.SEND_SYNC_RECORDS, undefined, pendingRecords)
   }
 
   if (!deviceIdSent && isFirstRun) {
@@ -341,6 +351,7 @@ module.exports.onSyncReady = (isFirstRun, e) => {
     e.sender.send(syncMessages.FETCH_SYNC_RECORDS, categoryNames, startAt)
     startAt = syncUtil.now()
     appActions.saveSyncInitData(null, null, startAt)
+    resendPendingRecords()
   }
   poll()
   pollIntervalId = setInterval(poll, config.fetchInterval)

js/constants/appConfig.js

@@ -113,7 +113,8 @@ module.exports = {
     debug: !isProduction,
     testS3Url: 'https://brave-sync-test.s3.dualstack.us-west-2.amazonaws.com/',
     s3Url: isProduction ? 'https://brave-sync.s3.dualstack.us-west-2.amazonaws.com' : 'https://brave-sync-staging.s3.dualstack.us-west-2.amazonaws.com',
-    fetchInterval: isProduction ? 1000 * 60 * 3 : 1000 * 60
+    fetchInterval: isProduction ? (1000 * 60 * 3) : (1000 * 60),
+    resendPendingRecordInterval: isProduction ? (1000 * 60 * 12) : (1000 * 60 * 4)
   },
   urlSuggestions: {
     ageDecayConstant: 50

test/unit/app/common/lib/jsonUtilTest.js

@@ -0,0 +1,23 @@
+/* global describe, it */
+const jsonUtil = require('../../../../../app/common/lib/jsonUtil')
+const assert = require('assert')
+
+require('../../../braveUnit')
+
+describe('jsonUtil test', function () {
+  describe('unescapeJSONPointer', function () {
+    it('Unescapes ~1 and ~0', function () {
+      const input = 'http:~1~1people.ischool.berkeley.edu~1~0nick~1signal-protocol-js~1|0|3'
+      const expected = 'http://people.ischool.berkeley.edu/~nick/signal-protocol-js/|0|3'
+      const result = jsonUtil.unescapeJSONPointer(input)
+      assert.equal(result, expected)
+    })
+
+    it('Can do nothing', function () {
+      const input = 'tomato'
+      const expected = 'tomato'
+      const result = jsonUtil.unescapeJSONPointer(input)
+      assert.equal(result, expected)
+    })
+  })
+})