Don't merge records on resolve operation, keep the latest record instead

client/recordUtil.js

@@ -1,7 +1,5 @@
 'use strict'
 
-const deepEqual = require('deep-equal')
-const merge = require('lodash.merge')
 const proto = require('./constants/proto')
 const serializer = require('../lib/serializer')
 const {api} = require('../lib/api.proto.js')
@@ -11,108 +9,6 @@ const syncTypes = require('../lib/syncTypes.js')
 // webkit on iOS <= 10.2 does not support Object.values
 module.exports.CATEGORY_IDS = Object.keys(proto.categories).map((key) => proto.categories[key])
 
-/**
- * @param {string} type e.g. 'historySite'
- * @param {Function} isValidRecord checks if the update record has enough props to make a create record
- * @param {Function} mapUpdateToCreate converts props from the update record to the create record
- * @returns {Function}
- */
-const CreateFromUpdate = (type, isValidRecord, mapUpdateToCreate) => {
-  if (!type || !isValidRecord || !mapUpdateToCreate) {
-    throw new Error('type, isValidRecord, mapUpdateToCreate are required args')
-  }
-  return (record) => {
-    if (!record[type]) { throw new Error(`Record missing ${type}`) }
-    if (!isValidRecord(record)) { return null }
-    const resolvedTypeProps = mapUpdateToCreate(record[type])
-    return Object.assign(
-      record,
-      { action: proto.actions.CREATE, [type]: resolvedTypeProps }
-    )
-  }
-}
-
-const createSitePropsFromUpdateSite = (site) => {
-  const defaultProps = {
-    title: '',
-    favicon: '',
-    customTitle: site.location,
-    lastAccessedTime: Date.now(),
-    creationTime: Date.now()
-  }
-  return Object.assign({}, defaultProps, site)
-}
-
-const createFromUpdateBookmark = CreateFromUpdate(
-  'bookmark',
-  (record) => {
-    const site = record.bookmark.site
-    if (!site) { return false }
-    if (record.bookmark.isFolder) {
-      return site.customTitle || site.title
-    } else {
-      return site.location
-    }
-  },
-  (bookmark) => {
-    if (bookmark.isFolder) {
-      return bookmark
-    } else {
-      const defaultProps = {
-        isFolder: false
-      }
-      const site = createSitePropsFromUpdateSite(bookmark.site)
-      return Object.assign({}, defaultProps, bookmark, {site})
-    }
-  }
-)
-
-const createFromUpdateHistorySite = CreateFromUpdate(
-  'historySite',
-  (record) => { return !!record.historySite.location },
-  createSitePropsFromUpdateSite
-)
-
-const createFromUpdateSiteSetting = CreateFromUpdate(
-  'siteSetting',
-  (record) => { return !!record.siteSetting.hostPattern },
-  (siteSetting) => { return siteSetting }
-)
-
-module.exports.createFromUpdate = (record) => {
-  if (!record || record.action !== proto.actions.UPDATE) {
-    throw new Error('Missing UPDATE syncRecord.')
-  }
-  switch (record.objectData) {
-    case 'bookmark':
-      return createFromUpdateBookmark(record)
-    case 'historySite':
-      return createFromUpdateHistorySite(record)
-    case 'siteSetting':
-      return createFromUpdateSiteSetting(record)
-    default:
-      console.log(`Warning: invalid objectData ${record.objectData}`)
-      return null
-  }
-}
-
-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] }
@@ -132,151 +28,63 @@ const normalizeBookmark = (record) => {
 }
 
 /**
- * Given a SyncRecord and a browser's matching existing object, resolve
- * 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
+ * Keeps only most the right pairs in array for each unique objectId from recordsAndExistingObjects
+ * Pairs in recordsAndExistingObjects are expected to be sorted by timestamp
  */
-const resolveRecordWithObject = (record, existingObject) => {
-  const type = record.objectData
-  if (type === 'siteSetting') {
-    return resolveSiteSettingsRecordWithObject(record, existingObject)
-  }
-  if (record.action === proto.actions.UPDATE) {
-    if (deepEqual(record[type], existingObject[type])) {
-      // no-op
-      return null
+module.exports.keepMostRecent = (recordsAndExistingObjects) => {
+  var alreadySeenObjectId = new Set()
+  for (var i = recordsAndExistingObjects.length - 1; i >= 0; --i) {
+    if (recordsAndExistingObjects[i][0]) {
+      const stringObjectId = JSON.stringify(recordsAndExistingObjects[i][0].objectId)
+      if (alreadySeenObjectId.has(stringObjectId)) {
+        recordsAndExistingObjects.splice(i, 1)
+      } else {
+        alreadySeenObjectId.add(stringObjectId)
+      }
     }
-    return record
-  } else if (record.action === proto.actions.DELETE) {
-    return record
-  } else {
-    throw new Error('Invalid record action')
   }
+  return recordsAndExistingObjects
 }
 
 /**
- * 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
+ * Gets the record from if its timestamp greater, or action is DELETE
+ * If any of the records does not have the timestamp, choose server record
  */
-const resolveSiteSettingsRecordWithObject = (record, existingObject) => {
-  const commonFields = ['hostPattern']
-  const type = record.objectData
-  const recordFields = new Set(Object.keys(record[type]))
-  const existingFields = new Set(Object.keys(existingObject[type]))
-
-  let resolveField = null
-  if (record.action === proto.actions.UPDATE) {
-    resolveField = (field) => {
-      return !commonFields.includes(field) &&
-      (!existingFields.has(field) || !deepEqual(existingObject[type][field], record[type][field]))
-    }
-  } else if (record.action === proto.actions.DELETE) {
-    resolveField = (field) => {
-      return !commonFields.includes(field) && existingFields.has(field)
-    }
+module.exports.getThisPairWinner = (recordAndLocalObject) => {
+  const serverRecord = recordAndLocalObject[0]
+  const localRecord = recordAndLocalObject[1]
+
+  if (serverRecord.action === proto.actions.DELETE ||
+    !localRecord) {
+    return serverRecord
+  } else if (localRecord.action === proto.actions.DELETE) {
+    return localRecord
+  } else if ((!serverRecord.syncTimestamp || !localRecord.syncTimestamp) ||
+    (serverRecord.syncTimestamp >= localRecord.syncTimestamp)) {
+    return serverRecord
   } else {
-    throw new Error('Invalid record action')
-  }
-  const resolvedFields = [...recordFields].filter(resolveField)
-  const resolvedData = pickFields(record[type], resolvedFields)
-  if (Object.keys(resolvedData).length === 0) {
-    return null
-  }
-  let resolved = Object.assign({}, record, {[type]: resolvedData})
-  for (let field of commonFields) {
-    if (!recordFields.has(field)) { continue }
-    resolved[type][field] = record[type][field]
-  }
-  return resolved
-}
-
-/**
- * Given a new SyncRecord and a browser's matching existing object if available,
- * resolve the write to perform on the browser's data.
- * @param {Object} record syncRecord as a JS object
- * @param {Object=} existingObject Browser object as syncRecord JS object
- * @returns {Object} Resolved syncRecord to apply to browser data
- */
-module.exports.resolve = (record, existingObject) => {
-  if (!record) { throw new Error('Missing syncRecord JS object.') }
-  const nullIgnore = () => {
-    console.log(`Ignoring ${humanAction(record.action)} of object ${record.objectId}.`)
-    return null
-  }
-  switch (record.action) {
-    case proto.actions.CREATE:
-      return existingObject
-        ? nullIgnore()
-        : record
-    case proto.actions.UPDATE:
-      const resolvedUpdate = existingObject
-        ? resolveRecordWithObject(record, existingObject)
-        : this.createFromUpdate(record)
-      return resolvedUpdate || nullIgnore()
-    case proto.actions.DELETE:
-      const resolvedDelete = existingObject
-        ? resolveRecordWithObject(record, existingObject)
-        : null
-      return resolvedDelete || nullIgnore()
-    default:
-      throw new Error(`Invalid record action: ${record.action}`)
+    return localRecord
   }
 }
 
 /**
- * Given two SyncRecords, merge objectData of record2 into record1.
- * @param {Object} record1
- * @param {Object} record2
- * @returns {Object} merged record
+ * Gets the record from if its timestamp greater, or action is DELETE
+ * If both records does not have timestamp, choose server record
  */
-const mergeRecord = (record1, record2) => {
-  if (record1.objectData !== record2.objectData) {
-    throw new Error('Records with same objectId have mismatched objectData!')
-  }
-  const newRecord = {}
-  merge(newRecord, record1)
-  merge(newRecord, record2)
-  if (record2.action === proto.actions.UPDATE && record1.action === proto.actions.CREATE) {
-    newRecord.action = proto.actions.CREATE
+const getPerPairWinners = (recordsAndExistingObjects) => {
+  let resolvedRecords = []
+  for (var i = 0; i < recordsAndExistingObjects.length; ++i) {
+    resolvedRecords.push(this.getThisPairWinner(recordsAndExistingObjects[i]))
   }
-  return newRecord
-}
-
-/**
- * Within an array of [record, object], merge items whose records have the
- * same objectId.
- * @param {Array} recordsAndObjects Same format as input to resolveRecords().
- * @returns {Array}
- */
-const mergeRecords = (recordsAndObjects) => {
-  const objectIdMap = {} // map of objectId to [record, object]
-  recordsAndObjects.forEach((recordAndObject) => {
-    const record = recordAndObject[0]
-    const object = recordAndObject[1]
-    const id = JSON.stringify(record.objectId)
-    if (objectIdMap[id]) {
-      const mergedRecord = mergeRecord(objectIdMap[id][0], record)
-      objectIdMap[id] = [mergedRecord, object]
-    } else {
-      objectIdMap[id] = recordAndObject
-    }
-  })
-  // webkit does not support Object.values
-  return Object.keys(objectIdMap).map((key) => objectIdMap[key])
+  return resolvedRecords
 }
 
 /**
- * Given a list of new SyncRecords and matching browser objects, resolve
- * writes to perform on the browser's data.
+ * Given a list of new SyncRecords and matching browser objects, finds the latest
+ * changes to perform modification of the browser's data.
  * @param {Array} recordsAndExistingObjects
  * @returns {Array.<Object>} Resolved syncRecords to apply to browser data.
- */
+*/
 module.exports.resolveRecords = (recordsAndExistingObjects) => {
   let resolvedRecords = []
   recordsAndExistingObjects.forEach((item) => {
@@ -285,11 +93,9 @@ module.exports.resolveRecords = (recordsAndExistingObjects) => {
       normalizeBookmark(item[1])
     }
   })
-  const merged = mergeRecords(recordsAndExistingObjects)
-  merged.forEach(([record, existingObject]) => {
-    const resolved = this.resolve(record, existingObject)
-    if (resolved) { resolvedRecords.push(resolved) }
-  })
+
+  recordsAndExistingObjects = this.keepMostRecent(recordsAndExistingObjects)
+  resolvedRecords = getPerPairWinners(recordsAndExistingObjects)
   return resolvedRecords
 }
 

package.json

@@ -64,9 +64,7 @@
     "config": "^1.24.0",
     "cors": "^2.8.1",
     "crc": "^3.4.4",
-    "deep-equal": "^1.0.1",
     "express": "^4.14.0",
-    "lodash.merge": "^4.6.2",
     "protobufjs": "^6.8.0",
     "raven": "^0.12.3",
     "redis": "^2.8.0",