diff --git a/client/constants/messages.js b/client/constants/messages.js index 46d120d..e788299 100644 --- a/client/constants/messages.js +++ b/client/constants/messages.js @@ -57,7 +57,7 @@ const messages = { * with new records, do * GET_EXISTING_OBJECTS -> RESOLVE_SYNC_RECORDS -> RESOLVED_SYNC_RECORDS */ - FETCH_SYNC_RECORDS: _, /* @param Array. categoryNames, @param {number} startAt (in seconds or milliseconds), @param {number=} maxRecords limit response to a given max number of records. set to 0 or falsey to not limit the response, @param {number} previousFetchTime (in milliseconds) */ + FETCH_SYNC_RECORDS: _, /* @param Array. categoryNames, @param {number} startAt (in seconds or milliseconds), @param {number=} maxRecords limit response to a given max number of records. set to 0 or falsey to not limit the response */ /** * browser -> webview * sent to fetch all sync devices. webview responds with RESOLVED_SYNC_RECORDS. diff --git a/client/requestUtil.js b/client/requestUtil.js index 1cfeee3..a17b49c 100644 --- a/client/requestUtil.js +++ b/client/requestUtil.js @@ -211,7 +211,7 @@ RequestUtil.prototype.parseAWSResponse = function (bytes) { * }} opts * @returns {Promise(Array.)} */ -RequestUtil.prototype.list = function (category, startAt, maxRecords, nextContinuationToken, previousFetchTime, opts = {}) { +RequestUtil.prototype.list = function (category, startAt, maxRecords, nextContinuationToken, opts = {}) { const prefix = `${this.apiVersion}/${this.userId}/${category}` const options = { MaxKeys: maxRecords || 1000, @@ -223,7 +223,7 @@ RequestUtil.prototype.list = function (category, startAt, maxRecords, nextContin } if (startAt) { options.StartAfter = `${prefix}/${startAt}` } return this.withRetry(() => { - if (this.shouldListObject(previousFetchTime, category) || opts.compaction) { + if (this.shouldListObject(startAt, category) || opts.compaction) { const s3ObjectsPromise = s3Helper.listObjects(this.s3, options, !!maxRecords) if (!opts.compaction) { return s3ObjectsPromise @@ -237,7 +237,7 @@ RequestUtil.prototype.list = function (category, startAt, maxRecords, nextContin // wait for 15 seconds between batches setTimeout(() => { if (s3Objects.isTruncated) { - return this.list(category, startAt, maxRecords, s3Objects.nextContinuationToken, previousFetchTime, opts) + return this.list(category, startAt, maxRecords, s3Objects.nextContinuationToken, opts) } return new Promise((resolve, reject) => { // compaction is done @@ -307,14 +307,16 @@ const CATEGORIES_FOR_SQS = [proto.categories.BOOKMARKS, proto.categories.PREFERE /** * Checks do we need to use s3 list Object or SQS notifications - * @param {number=} previousFetchTime - the previous fetch time. Could be seconds or milliseconds + * @param {number=} startAt return objects with timestamp >= startAt (e.g. 1482435340). Could be seconds or milliseconds * @param {string} category - the category ID * @returns {boolean} */ -RequestUtil.prototype.shouldListObject = function (previousFetchTime, category) { +RequestUtil.prototype.shouldListObject = function (startAt, category) { const currentTime = new Date().getTime() - return !previousFetchTime || - (currentTime - previousFetchTime) > parseInt(s3Helper.SQS_RETENTION, 10) * 1000 || + const startAtToCheck = this.normalizeTimestampToMs(startAt, currentTime) + + return !startAtToCheck || + (currentTime - startAtToCheck) > parseInt(s3Helper.SQS_RETENTION, 10) * 1000 || !CATEGORIES_FOR_SQS.includes(category) || this.listInProgress === true } @@ -336,20 +338,17 @@ RequestUtil.prototype.shouldRetireOldSQSQueue = function (createdTimestamp) { /** * Checks do we need to use s3 list Object or SQS notifications - * @param {number=} timeToNormalize could be seconds or milliseconds + * @param {number=} startAt return objects with timestamp >= startAt (e.g. 1482435340). Could be seconds or milliseconds * @param {number=} currentTime currentTime in milliseconds - * @returns {number=} the time for sure in milliseconds + * @returns {number=} */ -RequestUtil.prototype.normalizeTimestampToMs = function (timeToNormalize, currentTime) { - if (!timeToNormalize) { - return 0 - } - - if (timeToNormalize && currentTime.toString().length - timeToNormalize.toString().length >= 3) { - timeToNormalize *= 1000 +RequestUtil.prototype.normalizeTimestampToMs = function (startAt, currentTime) { + let startAtToCheck = startAt + if (startAtToCheck && currentTime.toString().length - startAtToCheck.toString().length >= 3) { + startAtToCheck *= 1000 } - return timeToNormalize + return startAtToCheck } /** diff --git a/client/sync.js b/client/sync.js index eba4ae5..3fbb37f 100644 --- a/client/sync.js +++ b/client/sync.js @@ -109,8 +109,8 @@ const startSync = (requester) => { return jsRecords } - ipc.on(messages.FETCH_SYNC_RECORDS, (e, categoryNames, startAt, limitResponse, previousFetchTime) => { - logSync(`fetching ${categoryNames} records after ${startAt} previous fetch is ${previousFetchTime}`) + ipc.on(messages.FETCH_SYNC_RECORDS, (e, categoryNames, startAt, limitResponse) => { + logSync(`fetching ${categoryNames} records after ${startAt}`) categoryNames.forEach((category) => { if (!proto.categories[category]) { throw new Error(`Unsupported sync category: ${category}`) @@ -119,7 +119,7 @@ const startSync = (requester) => { if (nextContinuationTokens[category]) { continuationToken = nextContinuationTokens[category] } - requester.list(proto.categories[category], startAt, limitResponse, continuationToken, previousFetchTime).then((s3Objects) => { + requester.list(proto.categories[category], startAt, limitResponse, continuationToken).then((s3Objects) => { const jsRecords = getJSRecords(s3Objects.contents) logSync(`got ${jsRecords.length} decrypted records in ${category} after ${startAt}`) let lastRecordTimestamp diff --git a/test/client/deviceIdV2.js b/test/client/deviceIdV2.js index 6322a76..faab9ce 100644 --- a/test/client/deviceIdV2.js +++ b/test/client/deviceIdV2.js @@ -97,7 +97,7 @@ test('deviceId V2 migration', (t) => { const testCanListFromOldQueue = (t) => { t.test('can list notification from old SQS queue', (t) => { const currentTime = new Date().getTime() - requestUtil.list(proto.categories.BOOKMARKS, currentTime, 0, '', currentTime) + requestUtil.list(proto.categories.BOOKMARKS, currentTime) .then(s3Objects => requestUtil.s3ObjectsToRecords(s3Objects.contents)) .then((response) => { t.equals(response.length, 1) @@ -127,7 +127,7 @@ test('deviceId V2 migration', (t) => { const testCanListFromBothQueues = (t) => { t.test('can list notifications from new and old SQS queues', (t) => { const currentTime = new Date().getTime() - requestUtil.list(proto.categories.BOOKMARKS, currentTime, 0, '', currentTime) + requestUtil.list(proto.categories.BOOKMARKS, currentTime) .then(s3Objects => requestUtil.s3ObjectsToRecords(s3Objects.contents)) .then((response) => { t.equals(response.length, 1) diff --git a/test/client/requestUtil.js b/test/client/requestUtil.js index ffa3a84..a3c88bd 100644 --- a/test/client/requestUtil.js +++ b/test/client/requestUtil.js @@ -52,25 +52,7 @@ test('client RequestUtil', (t) => { }) const serializer = requestUtil.serializer - t.plan(3) - - t.test('#should list object', (t) => { - t.plan(3) - - t.equals(true, - requestUtil.shouldListObject(0, proto.categories.BOOKMARKS), - `${t.name}: previous fetch time is empty - use S3`) - - t.equals(false, - requestUtil.shouldListObject(new Date().getTime()-1000*60, proto.categories.BOOKMARKS), - `${t.name}: previous fetch is not older than 24h - use SQS`) - - const delta25hours = 1000*60*60*25 - t.equals(true, - requestUtil.shouldListObject(new Date().getTime()-delta25hours, proto.categories.BOOKMARKS), - `${t.name}: previous fetch is older than 24h - use S3`) - }) - + t.plan(2) t.test('#put preference: device', (t) => { t.plan(2) const deviceId = new Uint8Array([0]) @@ -306,7 +288,7 @@ test('client RequestUtil', (t) => { const testCanListNotifications = (t) => { t.test(`${t.name} can list notification`, (t) => { let currentTime = new Date().getTime() - requestUtil.list(proto.categories.BOOKMARKS, currentTime, 1000, '', currentTime) + requestUtil.list(proto.categories.BOOKMARKS, currentTime) .then(s3Objects => requestUtil.s3ObjectsToRecords(s3Objects.contents)) .then((response) => { const s3Record = response[1] ? response[1].record : response[1] @@ -479,7 +461,7 @@ test('client RequestUtil', (t) => { } // limit batch size to 10 to test cross batch compaction for around 40 // objects - requestUtil.list(proto.categories.BOOKMARKS, 0, 10, '', 0, {compaction: true, + requestUtil.list(proto.categories.BOOKMARKS, 0, 10, '', {compaction: true, compactionUpdateCb: compactionUpdate, compactionDoneCb: () => { console.log = consoleLogBak @@ -492,7 +474,7 @@ test('client RequestUtil', (t) => { compactedRecord.filter(record => record.objectId.toString() === record2_update.objectId.toString()).length, 5) // we already have 15 second timeout for each batch so no need to // do another wait after compaction is done - requestUtil.list(proto.categories.BOOKMARKS, 0, 0, '', 0) + requestUtil.list(proto.categories.BOOKMARKS, 0, 0) .then(s3Objects => requestUtil.s3ObjectsToRecords(s3Objects.contents)) .then((response) => { t.equals(response.length, 2, `${t.name} check records number`)