Merge pull request #98 from brave/staging

v1.3.0

client/recordUtil.js

@@ -87,6 +87,23 @@ module.exports.createFromUpdate = (record) => {
   }
 }
 
+const ACTION_NUMBERS_TO_STRINGS = Object.keys(proto.actions)
+  .reduce((obj, key) => Object.assign({}, obj, { [proto.actions[key]]: key }), {})
+
+/**
+ * @param {number} action e.g. 0
+ * @returns {string} action string e.g. CREATE
+ */
+const humanAction = (action) => {
+  const string = ACTION_NUMBERS_TO_STRINGS[action]
+  if (string) { return string }
+  if (typeof action.toString === 'function') {
+    return action.toString()
+  } else {
+    return undefined
+  }
+}
+
 const pickFields = (object, fields) => {
   return fields.reduce((a, x) => {
     if (object.hasOwnProperty(x)) { a[x] = object[x] }
@@ -96,12 +113,38 @@ const pickFields = (object, fields) => {
 
 /**
  * Given a SyncRecord and a browser's matching existing object, resolve
- * objectData to only have the applicable fields.
+ * objectData to the final object that should be applied by the browser.
  * @param {Object} record SyncRecord JS object
  * @param {Object=} existingObject Browser object as syncRecord JS object
  * @returns {Object|null} Resolved syncRecord to apply to browser data
  */
 const resolveRecordWithObject = (record, existingObject) => {
+  const type = record.objectData
+  if (type === 'siteSetting') {
+    return resolveSiteSettingsRecordWithObject(record, existingObject)
+  }
+  if (record.action === proto.actions.UPDATE) {
+    if (valueEquals(record[type], existingObject[type])) {
+      // no-op
+      return null
+    }
+    return record
+  } else if (record.action === proto.actions.DELETE) {
+    return record
+  } else {
+    throw new Error('Invalid record action')
+  }
+}
+
+/**
+ * Given a siteSettings SyncRecord and a browser's matching existing object, resolve
+ * objectData to only have the applicable fields.
+ * TODO: Maybe make behavior for siteSettings same as for other types.
+ * @param {Object} record SyncRecord JS object
+ * @param {Object=} existingObject Browser object as syncRecord JS object
+ * @returns {Object|null} Resolved syncRecord to apply to browser data
+ */
+const resolveSiteSettingsRecordWithObject = (record, existingObject) => {
   const commonFields = ['hostPattern']
   const type = record.objectData
   const recordFields = new Set(Object.keys(record[type]))
@@ -143,7 +186,7 @@ const resolveRecordWithObject = (record, existingObject) => {
 module.exports.resolve = (record, existingObject) => {
   if (!record) { throw new Error('Missing syncRecord JS object.') }
   const nullIgnore = () => {
-    console.log(`Ignoring ${record.action} of object ${record.objectId}.`)
+    console.log(`Ignoring ${humanAction(record.action)} of object ${record.objectId}.`)
     return null
   }
   switch (record.action) {
@@ -176,7 +219,13 @@ const mergeRecord = (record1, record2) => {
   if (record1.objectData !== record2.objectData) {
     throw new Error('Records with same objectId have mismatched objectData!')
   }
-  return merge(record1, record2)
+  const newRecord = {}
+  merge(newRecord, record1)
+  merge(newRecord, record2)
+  if (record2.action === proto.actions.UPDATE && record1.action === proto.actions.CREATE) {
+    newRecord.action = proto.actions.CREATE
+  }
+  return newRecord
 }
 
 /**

test/client/recordUtil.js

@@ -13,6 +13,9 @@ const Record = (props) => {
   }
   return Object.assign({}, baseProps, props)
 }
+const CreateRecord = (props) => {
+  return Record(Object.assign({action: proto.actions.CREATE}, props))
+}
 const UpdateRecord = (props) => {
   return Record(Object.assign({action: proto.actions.UPDATE}, props))
 }
@@ -68,7 +71,7 @@ const updateSiteSetting = UpdateRecord({
 })
 
 test('recordUtil.resolve', (t) => {
-  t.plan(12)
+  t.plan(14)
 
   const forRecordsWithAction = (t, action, callback) => {
     t.plan(baseRecords.length)
@@ -137,6 +140,30 @@ test('recordUtil.resolve', (t) => {
     t.equals(resolved, null, `${t.name}`)
   })
 
+  t.test('DELETE site, existing, no props -> identity', (t) => {
+    t.plan(1)
+    const deleteSite = DeleteRecord({
+      objectId: recordBookmark.objectId,
+      deviceId: [0],
+      objectData: 'bookmark',
+      bookmark: {}
+    })
+    const resolved = recordUtil.resolve(deleteSite, recordBookmark)
+    t.equals(resolved, deleteSite, `${t.name}`)
+  })
+
+  t.test('DELETE site, no existing object, no props -> null', (t) => {
+    t.plan(1)
+    const deleteSite = DeleteRecord({
+      objectId: recordHistorySite.objectId,
+      deviceId: [0],
+      objectData: 'historySite',
+      historySite: {}
+    })
+    const resolved = recordUtil.resolve(deleteSite, null)
+    t.equals(resolved, null, `${t.name}`)
+  })
+
   t.test('DELETE, no existing object -> null', (t) => {
     forRecordsWithAction(t, proto.actions.DELETE, (record) => {
       const resolved = recordUtil.resolve(record, undefined)
@@ -342,7 +369,7 @@ test('recordUtil.resolve', (t) => {
 })
 
 test('recordUtil.resolveRecords()', (t) => {
-  t.plan(2)
+  t.plan(4)
 
   t.test(`${t.name} takes [ [{syncRecord}, {existingObject || null}], ... ] and returns resolved records [{syncRecord}, ...]`, (t) => {
     t.plan(1)
@@ -375,6 +402,65 @@ test('recordUtil.resolveRecords()', (t) => {
     const resolved = recordUtil.resolveRecords(input)
     t.deepEquals(resolved, [], t.name)
   })
+
+  t.test(`${t.name} Create + Update of a new object should resolve to a single Create`, (t) => {
+    t.plan(1)
+    const expectedRecord = CreateRecord({
+      objectId: recordBookmark.objectId,
+      objectData: 'bookmark',
+      bookmark: Object.assign(
+        {},
+        props.bookmark,
+        { site: Object.assign({}, siteProps, updateSiteProps) }
+      )
+    })
+    const input = [[recordBookmark, null], [updateBookmark, null]]
+    const resolved = recordUtil.resolveRecords(input)
+    t.deepEquals(resolved, [expectedRecord], t.name)
+  })
+
+  t.test(`${t.name} resolves bookmark records with same parent folder`, (t) => {
+    t.plan(1)
+    const record = {
+      action: 1,
+      deviceId: [0],
+      objectId: [16, 84, 219, 81, 33, 13, 44, 121, 211, 208, 1, 203, 114, 18, 215, 244],
+      objectData: 'bookmark',
+      bookmark: {
+        site: {
+          location: 'https://www.bobsclamhut.com/',
+          title: "Bob's Clam Hut",
+          customTitle: 'best seafood in Kittery',
+          favicon: '',
+          lastAccessedTime: 0,
+          creationTime: 0
+        },
+        isFolder: false,
+        parentFolderObjectId: [119, 148, 37, 242, 165, 20, 119, 15, 53, 57, 223, 116, 155, 99, 9, 128]
+      }
+    }
+    const existingObject = {
+      action: 1,
+      deviceId: [12],
+      objectId: [16, 84, 219, 81, 33, 13, 44, 121, 211, 208, 1, 203, 114, 18, 215, 244],
+      objectData: 'bookmark',
+      bookmark: {
+        site: {
+          location: 'https://www.bobsclamhut.com/',
+          title: "Bob's Clam Hut",
+          customTitle: '',
+          favicon: '',
+          lastAccessedTime: 0,
+          creationTime: 0
+        },
+        isFolder: false,
+        parentFolderObjectId: [119, 148, 37, 242, 165, 20, 119, 15, 53, 57, 223, 116, 155, 99, 9, 128]
+      }
+    }
+    const recordsAndExistingObjects = [[record, existingObject]]
+    const resolved = recordUtil.resolveRecords(recordsAndExistingObjects)
+    t.deepEquals(resolved, [record], t.name)
+  })
 })
 
 test('recordUtil.syncRecordAsJS()', (t) => {