From 3dfc8237165468265b1993d77b4d266b0070640b Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 21:27:28 -0400 Subject: [PATCH 01/25] Add `Attachment.removeSchemaVersion` --- js/modules/types/attachment.js | 6 ++++++ test/modules/types/attachment_test.js | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/js/modules/types/attachment.js b/js/modules/types/attachment.js index 11c313b86..27e7416b5 100644 --- a/js/modules/types/attachment.js +++ b/js/modules/types/attachment.js @@ -176,6 +176,12 @@ exports._replaceUnicodeOrderOverridesSync = (attachment) => { exports.replaceUnicodeOrderOverrides = async attachment => exports._replaceUnicodeOrderOverridesSync(attachment); +exports.removeSchemaVersion = async (attachment) => { + const attachmentWithoutSchemaVersion = Object.assign({}, attachment); + delete attachmentWithoutSchemaVersion.schemaVersion; + return attachmentWithoutSchemaVersion; +}; + // Public API const toVersion1 = exports.withSchemaVersion(1, autoOrientJPEG); const toVersion2 = exports.withSchemaVersion(2, exports.replaceUnicodeOrderOverrides); diff --git a/test/modules/types/attachment_test.js b/test/modules/types/attachment_test.js index 4de9d8526..bc79b3768 100644 --- a/test/modules/types/attachment_test.js +++ b/test/modules/types/attachment_test.js @@ -243,4 +243,26 @@ describe('Attachment', () => { } ); }); + + describe('removeSchemaVersion', () => { + it('should remove existing schema version', async () => { + const input = { + contentType: 'image/jpeg', + data: null, + fileName: 'foo.jpg', + size: 1111, + schemaVersion: 1, + }; + + const expected = { + contentType: 'image/jpeg', + data: null, + fileName: 'foo.jpg', + size: 1111, + }; + + const actual = await Attachment.removeSchemaVersion(input); + assert.deepEqual(actual, expected); + }); + }); }); From add4b11df3812b19eea6a83e3ac7c62526fe8fff Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 21:47:56 -0400 Subject: [PATCH 02/25] Skip invalid attachments and make function sync --- js/modules/types/attachment.js | 7 ++++++- test/modules/types/attachment_test.js | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/js/modules/types/attachment.js b/js/modules/types/attachment.js index 27e7416b5..1e09ad338 100644 --- a/js/modules/types/attachment.js +++ b/js/modules/types/attachment.js @@ -176,7 +176,12 @@ exports._replaceUnicodeOrderOverridesSync = (attachment) => { exports.replaceUnicodeOrderOverrides = async attachment => exports._replaceUnicodeOrderOverridesSync(attachment); -exports.removeSchemaVersion = async (attachment) => { +exports.removeSchemaVersion = (attachment) => { + if (!exports.isValid(attachment)) { + console.log('Attachment.removeSchemaVersion: Invalid input attachment:', attachment); + return attachment; + } + const attachmentWithoutSchemaVersion = Object.assign({}, attachment); delete attachmentWithoutSchemaVersion.schemaVersion; return attachmentWithoutSchemaVersion; diff --git a/test/modules/types/attachment_test.js b/test/modules/types/attachment_test.js index bc79b3768..305d16194 100644 --- a/test/modules/types/attachment_test.js +++ b/test/modules/types/attachment_test.js @@ -245,7 +245,7 @@ describe('Attachment', () => { }); describe('removeSchemaVersion', () => { - it('should remove existing schema version', async () => { + it('should remove existing schema version', () => { const input = { contentType: 'image/jpeg', data: null, @@ -261,7 +261,7 @@ describe('Attachment', () => { size: 1111, }; - const actual = await Attachment.removeSchemaVersion(input); + const actual = Attachment.removeSchemaVersion(input); assert.deepEqual(actual, expected); }); }); From e9e46464c27e5155dd404888ebbd1452fc37541b Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 21:54:54 -0400 Subject: [PATCH 03/25] Add `SchemaVersion` type --- js/modules/types/schema_version.js | 5 +++++ test/modules/types/schema_version_test.js | 25 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 js/modules/types/schema_version.js create mode 100644 test/modules/types/schema_version_test.js diff --git a/js/modules/types/schema_version.js b/js/modules/types/schema_version.js new file mode 100644 index 000000000..3a0d08980 --- /dev/null +++ b/js/modules/types/schema_version.js @@ -0,0 +1,5 @@ +const isNumber = require('lodash/isNumber'); + + +exports.isValid = value => + isNumber(value) && value >= 0; diff --git a/test/modules/types/schema_version_test.js b/test/modules/types/schema_version_test.js new file mode 100644 index 000000000..a3205c485 --- /dev/null +++ b/test/modules/types/schema_version_test.js @@ -0,0 +1,25 @@ +require('mocha-testcheck').install(); +const { assert } = require('chai'); + +const SchemaVersion = require('../../../js/modules/types/schema_version'); + + +describe('SchemaVersion', () => { + describe('isValid', () => { + check.it( + 'should return true for positive integers', + gen.posInt, + (input) => { + assert.isTrue(SchemaVersion.isValid(input)); + } + ); + + check.it( + 'should return false for any other value', + gen.primitive.suchThat(value => typeof value !== 'number' || value < 0), + (input) => { + assert.isFalse(SchemaVersion.isValid(input)); + } + ); + }); +}); From c27746b79e3c22cf75ce43bc22a6e335e47eb3b9 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 22:01:23 -0400 Subject: [PATCH 04/25] Add `Message.withInheritedSchemaVersion` --- js/modules/types/message.js | 39 ++++++++++++++++++ test/modules/types/message_test.js | 63 ++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 test/modules/types/message_test.js diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 195359b64..2e7015ba0 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -1,9 +1,12 @@ const Attachment = require('./attachment'); +const SchemaVersion = require('./schema_version'); const GROUP = 'group'; const PRIVATE = 'private'; +const INITIAL_SCHEMA_VERSION = 0; + // Public API exports.GROUP = GROUP; exports.PRIVATE = PRIVATE; @@ -15,3 +18,39 @@ exports.upgradeSchema = async message => attachments: await Promise.all(message.attachments.map(Attachment.upgradeSchema)), }); + +// Inherits existing schema from attachments: +exports.withInheritedSchemaVersion = (message) => { + const isInitialized = SchemaVersion.isValid(message.schemaVersion) && + message.schemaVersion >= 1; + if (isInitialized) { + return message; + } + + const numAttachments = Array.isArray(message.attachments) + ? message.attachments.length : 0; + const hasAttachments = numAttachments > 0; + if (!hasAttachments) { + return Object.assign( + {}, + message, + { schemaVersion: INITIAL_SCHEMA_VERSION } + ); + } + + // All attachments should have the same schema version, so we just pick + // the first one: + const firstAttachment = message.attachments[0]; + const inheritedSchemaVersion = SchemaVersion.isValid(firstAttachment.schemaVersion) + ? firstAttachment.schemaVersion : INITIAL_SCHEMA_VERSION; + const messageWithInitialSchema = Object.assign( + {}, + message, + { + schemaVersion: inheritedSchemaVersion, + attachments: message.attachments.map(Attachment.removeSchemaVersion), + } + ); + + return messageWithInitialSchema; +}; diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js new file mode 100644 index 000000000..d36eaf030 --- /dev/null +++ b/test/modules/types/message_test.js @@ -0,0 +1,63 @@ +const { assert } = require('chai'); + +const Message = require('../../../js/modules/types/message'); + + +describe('Message', () => { + describe('withInheritedSchemaVersion', () => { + it('should ignore messages with previously inherited schema', () => { + const input = { + body: 'Imagine there is no heaven…', + schemaVersion: 2, + }; + const expected = { + body: 'Imagine there is no heaven…', + schemaVersion: 2, + }; + + const actual = Message.withInheritedSchemaVersion(input); + assert.deepEqual(actual, expected); + }); + + context('for message without attachments', () => { + it('should initialize schema version to zero', () => { + const input = { + body: 'Imagine there is no heaven…', + attachments: [], + }; + const expected = { + body: 'Imagine there is no heaven…', + attachments: [], + schemaVersion: 0, + }; + + const actual = Message.withInheritedSchemaVersion(input); + assert.deepEqual(actual, expected); + }); + }); + + context('for message with attachments', () => { + it('should inherit existing attachment schema version', () => { + const input = { + body: 'Imagine there is no heaven…', + attachments: [{ + contentType: 'image/jpeg', + fileName: 'lennon.jpg', + schemaVersion: 7, + }], + }; + const expected = { + body: 'Imagine there is no heaven…', + attachments: [{ + contentType: 'image/jpeg', + fileName: 'lennon.jpg', + }], + schemaVersion: 7, + }; + + const actual = Message.withInheritedSchemaVersion(input); + assert.deepEqual(actual, expected); + }); + }); + }); +}); From c81ce1dc92cb359d577618c6cb0e4bd803618af4 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 22:03:53 -0400 Subject: [PATCH 05/25] Fix log line --- js/database.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/database.js b/js/database.js index 19d524bda..4bcd6bc82 100644 --- a/js/database.js +++ b/js/database.js @@ -127,7 +127,7 @@ { version: '12.0', migrate(transaction, next) { - console.log('migration 1.0'); + console.log('migration 12.0'); console.log('creating object stores'); const messages = transaction.db.createObjectStore('messages'); messages.createIndex('conversation', ['conversationId', 'received_at'], { From df693ade7c4e130281b59340ffb6b5bf9a039eee Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 22:20:10 -0400 Subject: [PATCH 06/25] Allow `Attachment.fileName` to be `null` --- js/modules/types/attachment.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/js/modules/types/attachment.js b/js/modules/types/attachment.js index 1e09ad338..6c0f43343 100644 --- a/js/modules/types/attachment.js +++ b/js/modules/types/attachment.js @@ -27,7 +27,7 @@ exports.CURRENT_SCHEMA_VERSION = 2; // contentType: MIMEType // data: ArrayBuffer // digest: ArrayBuffer -// fileName: string +// fileName: string | null // flags: null // key: ArrayBuffer // size: integer @@ -53,8 +53,10 @@ exports.isValid = (rawAttachment) => { return false; } - return isString(rawAttachment.contentType) && - isString(rawAttachment.fileName); + const hasValidContentType = isString(rawAttachment.contentType); + const hasValidFileName = + isString(rawAttachment.fileName) || rawAttachment.fileName === null; + return hasValidContentType && hasValidFileName; }; // Middleware From b9e9f5e19a9637f9959427fa3653f69425af4061 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 22:20:40 -0400 Subject: [PATCH 07/25] :art: `withInheritedSchemaVersion` to `inheritSchemaVersion` --- js/modules/types/message.js | 2 +- test/modules/types/message_test.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 2e7015ba0..2c09845fa 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -20,7 +20,7 @@ exports.upgradeSchema = async message => }); // Inherits existing schema from attachments: -exports.withInheritedSchemaVersion = (message) => { +exports.inheritSchemaVersion = (message) => { const isInitialized = SchemaVersion.isValid(message.schemaVersion) && message.schemaVersion >= 1; if (isInitialized) { diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index d36eaf030..40c3cb2ff 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -4,7 +4,7 @@ const Message = require('../../../js/modules/types/message'); describe('Message', () => { - describe('withInheritedSchemaVersion', () => { + describe('inheritSchemaVersion', () => { it('should ignore messages with previously inherited schema', () => { const input = { body: 'Imagine there is no heaven…', @@ -15,7 +15,7 @@ describe('Message', () => { schemaVersion: 2, }; - const actual = Message.withInheritedSchemaVersion(input); + const actual = Message.inheritSchemaVersion(input); assert.deepEqual(actual, expected); }); @@ -31,7 +31,7 @@ describe('Message', () => { schemaVersion: 0, }; - const actual = Message.withInheritedSchemaVersion(input); + const actual = Message.inheritSchemaVersion(input); assert.deepEqual(actual, expected); }); }); @@ -55,7 +55,7 @@ describe('Message', () => { schemaVersion: 7, }; - const actual = Message.withInheritedSchemaVersion(input); + const actual = Message.inheritSchemaVersion(input); assert.deepEqual(actual, expected); }); }); From 7e3cc432d661f9e1ea377c803eba1af8dbad796c Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 22:54:48 -0400 Subject: [PATCH 08/25] Whitelist `js/database.js` for ESLint --- .eslintignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintignore b/.eslintignore index ab00fab0f..cd0a44a04 100644 --- a/.eslintignore +++ b/.eslintignore @@ -16,6 +16,7 @@ test/views/*.js # ES2015+ files !js/background.js +!js/database.js !js/logging.js !js/models/conversations.js !js/views/attachment_view.js From ed336d31d7bdd8a58daef4390b98898ffa526dfb Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 13 Mar 2018 23:45:19 -0400 Subject: [PATCH 09/25] Move schema versioning from `Attachment` to `Message` --- js/modules/types/attachment.js | 91 +------------ js/modules/types/message.js | 115 ++++++++++++++++- test/modules/types/attachment_test.js | 164 ------------------------ test/modules/types/message_test.js | 177 ++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 261 deletions(-) diff --git a/js/modules/types/attachment.js b/js/modules/types/attachment.js index 6c0f43343..74c46bf12 100644 --- a/js/modules/types/attachment.js +++ b/js/modules/types/attachment.js @@ -1,26 +1,9 @@ -const isFunction = require('lodash/isFunction'); -const isNumber = require('lodash/isNumber'); const isString = require('lodash/isString'); -const isUndefined = require('lodash/isUndefined'); const MIME = require('./mime'); const { arrayBufferToBlob, blobToArrayBuffer, dataURLToBlob } = require('blob-util'); const { autoOrientImage } = require('../auto_orient_image'); -// Increment this version number every time we change how attachments are upgraded. This -// will allow us to retroactively upgrade existing attachments. As we add more upgrade -// steps, we could design a pipeline that does this incrementally, e.g. from -// version 0 / unknown -> 1, 1 --> 2, etc., similar to how we do database migrations: -exports.CURRENT_SCHEMA_VERSION = 2; - -// Schema version history -// -// Version 1 -// - Auto-orient JPEG attachments using EXIF `Orientation` data -// - Add `schemaVersion` property -// Version 2 -// - Sanitize Unicode order override characters - // // Incoming message attachment fields // { // id: string @@ -59,72 +42,8 @@ exports.isValid = (rawAttachment) => { return hasValidContentType && hasValidFileName; }; -// Middleware -// type UpgradeStep = Attachment -> Promise Attachment - -// SchemaVersion -> UpgradeStep -> UpgradeStep -exports.withSchemaVersion = (schemaVersion, upgrade) => { - if (!isNumber(schemaVersion)) { - throw new TypeError('`schemaVersion` must be a number'); - } - if (!isFunction(upgrade)) { - throw new TypeError('`upgrade` must be a function'); - } - - return async (attachment) => { - if (!exports.isValid(attachment)) { - console.log('Attachment.withSchemaVersion: Invalid input attachment:', attachment); - return attachment; - } - - const isAlreadyUpgraded = attachment.schemaVersion >= schemaVersion; - if (isAlreadyUpgraded) { - return attachment; - } - - const expectedVersion = schemaVersion - 1; - const isUnversioned = isUndefined(attachment.schemaVersion); - const hasExpectedVersion = isUnversioned || - attachment.schemaVersion === expectedVersion; - if (!hasExpectedVersion) { - console.log( - 'WARNING: Attachment.withSchemaVersion: Unexpected version:' + - ` Expected attachment to have version ${expectedVersion},` + - ` but got ${attachment.schemaVersion}.`, - attachment - ); - return attachment; - } - - let upgradedAttachment; - try { - upgradedAttachment = await upgrade(attachment); - } catch (error) { - console.log( - 'Attachment.withSchemaVersion: error:', - error && error.stack ? error.stack : error - ); - return attachment; - } - - if (!exports.isValid(upgradedAttachment)) { - console.log( - 'Attachment.withSchemaVersion: Invalid upgraded attachment:', - upgradedAttachment - ); - return attachment; - } - - return Object.assign( - {}, - upgradedAttachment, - { schemaVersion } - ); - }; -}; - // Upgrade steps -const autoOrientJPEG = async (attachment) => { +exports.autoOrientJPEG = async (attachment) => { if (!MIME.isJPEG(attachment.contentType)) { return attachment; } @@ -188,11 +107,3 @@ exports.removeSchemaVersion = (attachment) => { delete attachmentWithoutSchemaVersion.schemaVersion; return attachmentWithoutSchemaVersion; }; - -// Public API -const toVersion1 = exports.withSchemaVersion(1, autoOrientJPEG); -const toVersion2 = exports.withSchemaVersion(2, exports.replaceUnicodeOrderOverrides); - -// UpgradeStep -exports.upgradeSchema = async attachment => - toVersion2(await toVersion1(attachment)); diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 2c09845fa..e62f05af6 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -1,3 +1,5 @@ +const isFunction = require('lodash/isFunction'); + const Attachment = require('./attachment'); const SchemaVersion = require('./schema_version'); @@ -7,18 +9,29 @@ const PRIVATE = 'private'; const INITIAL_SCHEMA_VERSION = 0; +// Increment this version number every time we add a message schema upgrade +// step. This will allow us to retroactively upgrade existing messages. As we +// add more upgrade steps, we could design a pipeline that does this +// incrementally, e.g. from version 0 / unknown -> 1, 1 --> 2, etc., similar to +// how we do database migrations: +exports.CURRENT_SCHEMA_VERSION = 2; + +// Schema version history +// +// Version 1 +// - Attachments: Auto-orient JPEG attachments using EXIF `Orientation` data +// Version 2 +// - Attachments: Sanitize Unicode order override characters + // Public API exports.GROUP = GROUP; exports.PRIVATE = PRIVATE; -// Schema -// Message -> Promise Message -exports.upgradeSchema = async message => - Object.assign({}, message, { - attachments: - await Promise.all(message.attachments.map(Attachment.upgradeSchema)), - }); +// Placeholder until we have stronger preconditions: +exports.isValid = () => + true; +// Schema // Inherits existing schema from attachments: exports.inheritSchemaVersion = (message) => { const isInitialized = SchemaVersion.isValid(message.schemaVersion) && @@ -54,3 +67,91 @@ exports.inheritSchemaVersion = (message) => { return messageWithInitialSchema; }; + +// Middleware +// type UpgradeStep = Message -> Promise Message + +// SchemaVersion -> UpgradeStep -> UpgradeStep +exports.withSchemaVersion = (schemaVersion, upgrade) => { + if (!SchemaVersion.isValid(schemaVersion)) { + throw new TypeError('`schemaVersion` is invalid'); + } + if (!isFunction(upgrade)) { + throw new TypeError('`upgrade` must be a function'); + } + + return async (message) => { + if (!exports.isValid(message)) { + console.log('Message.withSchemaVersion: Invalid input message:', message); + return message; + } + + const isAlreadyUpgraded = message.schemaVersion >= schemaVersion; + if (isAlreadyUpgraded) { + return message; + } + + const expectedVersion = schemaVersion - 1; + const hasExpectedVersion = message.schemaVersion === expectedVersion; + if (!hasExpectedVersion) { + console.log( + 'WARNING: Message.withSchemaVersion: Unexpected version:', + `Expected message to have version ${expectedVersion},`, + `but got ${message.schemaVersion}.`, + message + ); + return message; + } + + let upgradedMessage; + try { + upgradedMessage = await upgrade(message); + } catch (error) { + console.log( + 'Message.withSchemaVersion: error:', + // TODO: Use `Errors.toLogFormat`: + error && error.stack ? error.stack : error + ); + return message; + } + + if (!exports.isValid(upgradedMessage)) { + console.log( + 'Message.withSchemaVersion: Invalid upgraded message:', + upgradedMessage + ); + return message; + } + + return Object.assign( + {}, + upgradedMessage, + { schemaVersion } + ); + }; +}; + + +// Public API +// mapAttachments :: (Attachment -> Promise Attachment) -> Message -> Promise Message +exports.mapAttachments = upgradeAttachment => async message => + Object.assign( + {}, + message, + { + attachments: await Promise.all(message.attachments.map(upgradeAttachment)), + } + ); + +const toVersion1 = exports.withSchemaVersion( + 1, + exports.mapAttachments(Attachment.autoOrientJPEG) +); +const toVersion2 = exports.withSchemaVersion( + 2, + exports.mapAttachments(Attachment.replaceUnicodeOrderOverrides) +); + +// UpgradeStep +exports.upgradeSchema = async message => + toVersion2(await toVersion1(message)); diff --git a/test/modules/types/attachment_test.js b/test/modules/types/attachment_test.js index 305d16194..bf4b8f6a4 100644 --- a/test/modules/types/attachment_test.js +++ b/test/modules/types/attachment_test.js @@ -5,163 +5,6 @@ const { assert } = require('chai'); const Attachment = require('../../../js/modules/types/attachment'); describe('Attachment', () => { - describe('upgradeSchema', () => { - it('should upgrade an unversioned attachment to the latest version', async () => { - const input = { - contentType: 'application/json', - data: null, - fileName: 'test\u202Dfig.exe', - size: 1111, - }; - const expected = { - contentType: 'application/json', - data: null, - fileName: 'test\uFFFDfig.exe', - size: 1111, - schemaVersion: Attachment.CURRENT_SCHEMA_VERSION, - }; - - const actual = await Attachment.upgradeSchema(input); - assert.deepEqual(actual, expected); - }); - - context('with multiple upgrade steps', () => { - it('should return last valid attachment when any upgrade step fails', async () => { - const input = { - contentType: 'application/json', - data: null, - fileName: 'test\u202Dfig.exe', - size: 1111, - }; - const expected = { - contentType: 'application/json', - data: null, - fileName: 'test\u202Dfig.exe', - size: 1111, - schemaVersion: 1, - hasUpgradedToVersion1: true, - }; - - const v1 = async attachment => - Object.assign({}, attachment, { hasUpgradedToVersion1: true }); - const v2 = async () => { - throw new Error('boom'); - }; - const v3 = async attachment => - Object.assign({}, attachment, { hasUpgradedToVersion3: true }); - - const toVersion1 = Attachment.withSchemaVersion(1, v1); - const toVersion2 = Attachment.withSchemaVersion(2, v2); - const toVersion3 = Attachment.withSchemaVersion(3, v3); - - const upgradeSchema = async attachment => - toVersion3(await toVersion2(await toVersion1(attachment))); - - const actual = await upgradeSchema(input); - assert.deepEqual(actual, expected); - }); - - it('should skip out-of-order upgrade steps', async () => { - const input = { - contentType: 'application/json', - data: null, - fileName: 'test\u202Dfig.exe', - size: 1111, - }; - const expected = { - contentType: 'application/json', - data: null, - fileName: 'test\u202Dfig.exe', - size: 1111, - schemaVersion: 2, - hasUpgradedToVersion1: true, - hasUpgradedToVersion2: true, - }; - - const v1 = async attachment => - Object.assign({}, attachment, { hasUpgradedToVersion1: true }); - const v2 = async attachment => - Object.assign({}, attachment, { hasUpgradedToVersion2: true }); - const v3 = async attachment => - Object.assign({}, attachment, { hasUpgradedToVersion3: true }); - - const toVersion1 = Attachment.withSchemaVersion(1, v1); - const toVersion2 = Attachment.withSchemaVersion(2, v2); - const toVersion3 = Attachment.withSchemaVersion(3, v3); - - // NOTE: We upgrade to 3 before 2, i.e. the pipeline should abort: - const upgradeSchema = async attachment => - toVersion2(await toVersion3(await toVersion1(attachment))); - - const actual = await upgradeSchema(input); - assert.deepEqual(actual, expected); - }); - }); - }); - - describe('withSchemaVersion', () => { - it('should require a version number', () => { - const toVersionX = () => {}; - assert.throws( - () => Attachment.withSchemaVersion(toVersionX, 2), - '`schemaVersion` must be a number' - ); - }); - - it('should require an upgrade function', () => { - assert.throws( - () => Attachment.withSchemaVersion(2, 3), - '`upgrade` must be a function' - ); - }); - - it('should skip upgrading if attachment has already been upgraded', async () => { - const upgrade = async attachment => - Object.assign({}, attachment, { foo: true }); - const upgradeWithVersion = Attachment.withSchemaVersion(3, upgrade); - - const input = { - contentType: 'image/gif', - data: null, - fileName: 'foo.gif', - size: 1111, - schemaVersion: 4, - }; - const actual = await upgradeWithVersion(input); - assert.deepEqual(actual, input); - }); - - it('should return original attachment if upgrade function throws', async () => { - const upgrade = async () => { - throw new Error('boom!'); - }; - const upgradeWithVersion = Attachment.withSchemaVersion(3, upgrade); - - const input = { - contentType: 'image/gif', - data: null, - fileName: 'foo.gif', - size: 1111, - }; - const actual = await upgradeWithVersion(input); - assert.deepEqual(actual, input); - }); - - it('should return original attachment if upgrade function returns null', async () => { - const upgrade = async () => null; - const upgradeWithVersion = Attachment.withSchemaVersion(3, upgrade); - - const input = { - contentType: 'image/gif', - data: null, - fileName: 'foo.gif', - size: 1111, - }; - const actual = await upgradeWithVersion(input); - assert.deepEqual(actual, input); - }); - }); - describe('replaceUnicodeOrderOverrides', () => { it('should sanitize left-to-right order override character', async () => { const input = { @@ -169,14 +12,12 @@ describe('Attachment', () => { data: null, fileName: 'test\u202Dfig.exe', size: 1111, - schemaVersion: 1, }; const expected = { contentType: 'image/jpeg', data: null, fileName: 'test\uFFFDfig.exe', size: 1111, - schemaVersion: 1, }; const actual = await Attachment.replaceUnicodeOrderOverrides(input); @@ -189,14 +30,12 @@ describe('Attachment', () => { data: null, fileName: 'test\u202Efig.exe', size: 1111, - schemaVersion: 1, }; const expected = { contentType: 'image/jpeg', data: null, fileName: 'test\uFFFDfig.exe', size: 1111, - schemaVersion: 1, }; const actual = await Attachment.replaceUnicodeOrderOverrides(input); @@ -209,14 +48,12 @@ describe('Attachment', () => { data: null, fileName: 'test\u202e\u202dlol\u202efig.exe', size: 1111, - schemaVersion: 1, }; const expected = { contentType: 'image/jpeg', data: null, fileName: 'test\uFFFD\uFFFDlol\uFFFDfig.exe', size: 1111, - schemaVersion: 1, }; const actual = await Attachment.replaceUnicodeOrderOverrides(input); @@ -235,7 +72,6 @@ describe('Attachment', () => { data: null, fileName, size: 1111, - schemaVersion: 1, }; const actual = Attachment._replaceUnicodeOrderOverridesSync(input); diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index 40c3cb2ff..3f57d70e7 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -60,4 +60,181 @@ describe('Message', () => { }); }); }); + + describe('upgradeSchema', () => { + it('should upgrade an unversioned message to the latest version', async () => { + const input = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\u202Dfig.exe', + size: 1111, + }], + schemaVersion: 0, + }; + const expected = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\uFFFDfig.exe', + size: 1111, + }], + schemaVersion: Message.CURRENT_SCHEMA_VERSION, + }; + + const actual = await Message.upgradeSchema(input); + assert.deepEqual(actual, expected); + }); + + context('with multiple upgrade steps', () => { + it('should return last valid message when any upgrade step fails', async () => { + const input = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\u202Dfig.exe', + size: 1111, + }], + schemaVersion: 0, + }; + const expected = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\u202Dfig.exe', + size: 1111, + }], + hasUpgradedToVersion1: true, + schemaVersion: 1, + }; + + const v1 = async message => + Object.assign({}, message, { hasUpgradedToVersion1: true }); + const v2 = async () => { + throw new Error('boom'); + }; + const v3 = async message => + Object.assign({}, message, { hasUpgradedToVersion3: true }); + + const toVersion1 = Message.withSchemaVersion(1, v1); + const toVersion2 = Message.withSchemaVersion(2, v2); + const toVersion3 = Message.withSchemaVersion(3, v3); + + const upgradeSchema = async message => + toVersion3(await toVersion2(await toVersion1(message))); + + const actual = await upgradeSchema(input); + assert.deepEqual(actual, expected); + }); + + it('should skip out-of-order upgrade steps', async () => { + const input = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\u202Dfig.exe', + size: 1111, + }], + schemaVersion: 0, + }; + const expected = { + attachments: [{ + contentType: 'application/json', + data: null, + fileName: 'test\u202Dfig.exe', + size: 1111, + }], + schemaVersion: 2, + hasUpgradedToVersion1: true, + hasUpgradedToVersion2: true, + }; + + const v1 = async attachment => + Object.assign({}, attachment, { hasUpgradedToVersion1: true }); + const v2 = async attachment => + Object.assign({}, attachment, { hasUpgradedToVersion2: true }); + const v3 = async attachment => + Object.assign({}, attachment, { hasUpgradedToVersion3: true }); + + const toVersion1 = Message.withSchemaVersion(1, v1); + const toVersion2 = Message.withSchemaVersion(2, v2); + const toVersion3 = Message.withSchemaVersion(3, v3); + + // NOTE: We upgrade to 3 before 2, i.e. the pipeline should abort: + const upgradeSchema = async attachment => + toVersion2(await toVersion3(await toVersion1(attachment))); + + const actual = await upgradeSchema(input); + assert.deepEqual(actual, expected); + }); + }); + }); + + describe('withSchemaVersion', () => { + it('should require a version number', () => { + const toVersionX = () => {}; + assert.throws( + () => Message.withSchemaVersion(toVersionX, 2), + '`schemaVersion` is invalid' + ); + }); + + it('should require an upgrade function', () => { + assert.throws( + () => Message.withSchemaVersion(2, 3), + '`upgrade` must be a function' + ); + }); + + it('should skip upgrading if message has already been upgraded', async () => { + const upgrade = async message => + Object.assign({}, message, { foo: true }); + const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + + const input = { + id: 'guid-guid-guid-guid', + schemaVersion: 4, + }; + const expected = { + id: 'guid-guid-guid-guid', + schemaVersion: 4, + }; + const actual = await upgradeWithVersion(input); + assert.deepEqual(actual, expected); + }); + + it('should return original message if upgrade function throws', async () => { + const upgrade = async () => { + throw new Error('boom!'); + }; + const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + + const input = { + id: 'guid-guid-guid-guid', + schemaVersion: 0, + }; + const expected = { + id: 'guid-guid-guid-guid', + schemaVersion: 0, + }; + const actual = await upgradeWithVersion(input); + assert.deepEqual(actual, expected); + }); + + it('should return original message if upgrade function returns null', async () => { + const upgrade = async () => null; + const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + + const input = { + id: 'guid-guid-guid-guid', + schemaVersion: 0, + }; + const expected = { + id: 'guid-guid-guid-guid', + schemaVersion: 0, + }; + const actual = await upgradeWithVersion(input); + assert.deepEqual(actual, expected); + }); + }); }); From fa209fb98fa569331b9631be1f5989056cee8469 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:38:53 -0400 Subject: [PATCH 10/25] Add `idb` fork with `upgradeDBFromTransaction` support This allows us to use `async` / `await` with IndexedDB. --- package.json | 1 + yarn.lock | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/package.json b/package.json index 37eee2dbc..fedc452df 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "form-data": "^2.3.2", "google-libphonenumber": "^3.0.7", "got": "^8.2.0", + "idb": "https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20", "lodash": "^4.17.4", "mkdirp": "^0.5.1", "node-fetch": "https://github.com/scottnonnenberg/node-fetch.git#3e5f51e08c647ee5f20c43b15cf2d352d61c36b4", diff --git a/yarn.lock b/yarn.lock index 2e852329c..81afff81a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2724,6 +2724,10 @@ iconv-lite@0.4.19, iconv-lite@^0.4.17, iconv-lite@^0.4.19: version "0.4.19" resolved "https://registry.yarnpkg.com/iconv-lite/-/iconv-lite-0.4.19.tgz#f7468f60135f5e5dad3399c0a81be9a1603a082b" +"idb@https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20": + version "2.1.0" + resolved "https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20" + ieee754@^1.1.4: version "1.1.8" resolved "https://registry.yarnpkg.com/ieee754/-/ieee754-1.1.8.tgz#be33d40ac10ef1926701f6f08a2d86fbfd1ad3e4" From 8dfaa5619ffab86c21df05be33678fef9a4fb82d Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:45:35 -0400 Subject: [PATCH 11/25] Prefix private functions with underscore --- js/modules/types/message.js | 27 +++++++++++++------------ test/modules/types/message_test.js | 32 +++++++++++++++--------------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index e62f05af6..46843c3a5 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -32,8 +32,7 @@ exports.isValid = () => true; // Schema -// Inherits existing schema from attachments: -exports.inheritSchemaVersion = (message) => { +exports.initializeSchemaVersion = (message) => { const isInitialized = SchemaVersion.isValid(message.schemaVersion) && message.schemaVersion >= 1; if (isInitialized) { @@ -72,7 +71,7 @@ exports.inheritSchemaVersion = (message) => { // type UpgradeStep = Message -> Promise Message // SchemaVersion -> UpgradeStep -> UpgradeStep -exports.withSchemaVersion = (schemaVersion, upgrade) => { +exports._withSchemaVersion = (schemaVersion, upgrade) => { if (!SchemaVersion.isValid(schemaVersion)) { throw new TypeError('`schemaVersion` is invalid'); } @@ -82,7 +81,7 @@ exports.withSchemaVersion = (schemaVersion, upgrade) => { return async (message) => { if (!exports.isValid(message)) { - console.log('Message.withSchemaVersion: Invalid input message:', message); + console.log('Message._withSchemaVersion: Invalid input message:', message); return message; } @@ -95,7 +94,7 @@ exports.withSchemaVersion = (schemaVersion, upgrade) => { const hasExpectedVersion = message.schemaVersion === expectedVersion; if (!hasExpectedVersion) { console.log( - 'WARNING: Message.withSchemaVersion: Unexpected version:', + 'WARNING: Message._withSchemaVersion: Unexpected version:', `Expected message to have version ${expectedVersion},`, `but got ${message.schemaVersion}.`, message @@ -108,7 +107,7 @@ exports.withSchemaVersion = (schemaVersion, upgrade) => { upgradedMessage = await upgrade(message); } catch (error) { console.log( - 'Message.withSchemaVersion: error:', + 'Message._withSchemaVersion: error:', // TODO: Use `Errors.toLogFormat`: error && error.stack ? error.stack : error ); @@ -117,7 +116,7 @@ exports.withSchemaVersion = (schemaVersion, upgrade) => { if (!exports.isValid(upgradedMessage)) { console.log( - 'Message.withSchemaVersion: Invalid upgraded message:', + 'Message._withSchemaVersion: Invalid upgraded message:', upgradedMessage ); return message; @@ -133,8 +132,10 @@ exports.withSchemaVersion = (schemaVersion, upgrade) => { // Public API -// mapAttachments :: (Attachment -> Promise Attachment) -> Message -> Promise Message -exports.mapAttachments = upgradeAttachment => async message => +// _mapAttachments :: (Attachment -> Promise Attachment) -> +// Message -> +// Promise Message +exports._mapAttachments = upgradeAttachment => async message => Object.assign( {}, message, @@ -143,13 +144,13 @@ exports.mapAttachments = upgradeAttachment => async message => } ); -const toVersion1 = exports.withSchemaVersion( +const toVersion1 = exports._withSchemaVersion( 1, - exports.mapAttachments(Attachment.autoOrientJPEG) + exports._mapAttachments(Attachment.autoOrientJPEG) ); -const toVersion2 = exports.withSchemaVersion( +const toVersion2 = exports._withSchemaVersion( 2, - exports.mapAttachments(Attachment.replaceUnicodeOrderOverrides) + exports._mapAttachments(Attachment.replaceUnicodeOrderOverrides) ); // UpgradeStep diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index 3f57d70e7..e36538f88 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -4,7 +4,7 @@ const Message = require('../../../js/modules/types/message'); describe('Message', () => { - describe('inheritSchemaVersion', () => { + describe('initializeSchemaVersion', () => { it('should ignore messages with previously inherited schema', () => { const input = { body: 'Imagine there is no heaven…', @@ -15,7 +15,7 @@ describe('Message', () => { schemaVersion: 2, }; - const actual = Message.inheritSchemaVersion(input); + const actual = Message.initializeSchemaVersion(input); assert.deepEqual(actual, expected); }); @@ -31,7 +31,7 @@ describe('Message', () => { schemaVersion: 0, }; - const actual = Message.inheritSchemaVersion(input); + const actual = Message.initializeSchemaVersion(input); assert.deepEqual(actual, expected); }); }); @@ -55,7 +55,7 @@ describe('Message', () => { schemaVersion: 7, }; - const actual = Message.inheritSchemaVersion(input); + const actual = Message.initializeSchemaVersion(input); assert.deepEqual(actual, expected); }); }); @@ -116,9 +116,9 @@ describe('Message', () => { const v3 = async message => Object.assign({}, message, { hasUpgradedToVersion3: true }); - const toVersion1 = Message.withSchemaVersion(1, v1); - const toVersion2 = Message.withSchemaVersion(2, v2); - const toVersion3 = Message.withSchemaVersion(3, v3); + const toVersion1 = Message._withSchemaVersion(1, v1); + const toVersion2 = Message._withSchemaVersion(2, v2); + const toVersion3 = Message._withSchemaVersion(3, v3); const upgradeSchema = async message => toVersion3(await toVersion2(await toVersion1(message))); @@ -156,9 +156,9 @@ describe('Message', () => { const v3 = async attachment => Object.assign({}, attachment, { hasUpgradedToVersion3: true }); - const toVersion1 = Message.withSchemaVersion(1, v1); - const toVersion2 = Message.withSchemaVersion(2, v2); - const toVersion3 = Message.withSchemaVersion(3, v3); + const toVersion1 = Message._withSchemaVersion(1, v1); + const toVersion2 = Message._withSchemaVersion(2, v2); + const toVersion3 = Message._withSchemaVersion(3, v3); // NOTE: We upgrade to 3 before 2, i.e. the pipeline should abort: const upgradeSchema = async attachment => @@ -170,18 +170,18 @@ describe('Message', () => { }); }); - describe('withSchemaVersion', () => { + describe('_withSchemaVersion', () => { it('should require a version number', () => { const toVersionX = () => {}; assert.throws( - () => Message.withSchemaVersion(toVersionX, 2), + () => Message._withSchemaVersion(toVersionX, 2), '`schemaVersion` is invalid' ); }); it('should require an upgrade function', () => { assert.throws( - () => Message.withSchemaVersion(2, 3), + () => Message._withSchemaVersion(2, 3), '`upgrade` must be a function' ); }); @@ -189,7 +189,7 @@ describe('Message', () => { it('should skip upgrading if message has already been upgraded', async () => { const upgrade = async message => Object.assign({}, message, { foo: true }); - const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + const upgradeWithVersion = Message._withSchemaVersion(3, upgrade); const input = { id: 'guid-guid-guid-guid', @@ -207,7 +207,7 @@ describe('Message', () => { const upgrade = async () => { throw new Error('boom!'); }; - const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + const upgradeWithVersion = Message._withSchemaVersion(3, upgrade); const input = { id: 'guid-guid-guid-guid', @@ -223,7 +223,7 @@ describe('Message', () => { it('should return original message if upgrade function returns null', async () => { const upgrade = async () => null; - const upgradeWithVersion = Message.withSchemaVersion(3, upgrade); + const upgradeWithVersion = Message._withSchemaVersion(3, upgrade); const input = { id: 'guid-guid-guid-guid', From 752c8f97e659d0495e38846f3f0b848cc9b1ffde Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:45:46 -0400 Subject: [PATCH 12/25] :art: Format ternaries --- js/modules/types/message.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 46843c3a5..c9f4696ba 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -40,7 +40,8 @@ exports.initializeSchemaVersion = (message) => { } const numAttachments = Array.isArray(message.attachments) - ? message.attachments.length : 0; + ? message.attachments.length + : 0; const hasAttachments = numAttachments > 0; if (!hasAttachments) { return Object.assign( @@ -54,7 +55,8 @@ exports.initializeSchemaVersion = (message) => { // the first one: const firstAttachment = message.attachments[0]; const inheritedSchemaVersion = SchemaVersion.isValid(firstAttachment.schemaVersion) - ? firstAttachment.schemaVersion : INITIAL_SCHEMA_VERSION; + ? firstAttachment.schemaVersion + : INITIAL_SCHEMA_VERSION; const messageWithInitialSchema = Object.assign( {}, message, From 182e6ffe1067b0098f9818e39a499e89f251a0c2 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:46:22 -0400 Subject: [PATCH 13/25] Add version 17 migration --- js/modules/migrations/17/index.js | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 js/modules/migrations/17/index.js diff --git a/js/modules/migrations/17/index.js b/js/modules/migrations/17/index.js new file mode 100644 index 000000000..618c95156 --- /dev/null +++ b/js/modules/migrations/17/index.js @@ -0,0 +1,39 @@ +const idb = require('idb'); +const Message = require('../../types/message'); + + +exports.run = async (transaction) => { + const db = idb.upgradeDBFromTransaction(transaction); + const tx = db.transaction; + const messagesStore = tx.objectStore('messages'); + + console.log('Initialize messages schema version'); + await exports._initializeMessageSchemaVersion(messagesStore); + + console.log('Create index from attachment schema version to attachment'); + messagesStore.createIndex('schemaVersion', 'schemaVersion', { unique: false }); + + await db.transaction.complete; +}; + +exports._initializeMessageSchemaVersion = messagesStore => + new Promise((resolve, reject) => { + messagesStore.openCursor().then(async function cursorIterate(cursor) { + const hasMoreResults = Boolean(cursor); + if (!hasMoreResults) { + return resolve(); + } + + const message = cursor.value; + console.log('Initialize schema version for message:', message.id); + + const messageWithInitializedSchemaVersion = Message.initializeSchemaVersion(message); + try { + await messagesStore.put(messageWithInitializedSchemaVersion, message.id); + } catch (error) { + console.log('Failed to put message with initialized schema version:', message.id); + } + + cursor.continue().then(cursorIterate); + }).catch(reject); + }); From ac31dcbd7502433408a8f5725420e4d83f1850f0 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:46:59 -0400 Subject: [PATCH 14/25] Expose `Signal.Migrations` namespace --- preload.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/preload.js b/preload.js index 7ce82a33b..85cd358c0 100644 --- a/preload.js +++ b/preload.js @@ -106,9 +106,10 @@ // ES2015+ modules window.Signal = window.Signal || {}; - window.Signal.OS = require('./js/modules/os'); window.Signal.Logs = require('./js/modules/logs'); - + window.Signal.OS = require('./js/modules/os'); + window.Signal.Migrations = window.Signal.Migrations || {}; + window.Signal.Migrations.V17 = require('./js/modules/migrations/17'); window.Signal.Types = window.Signal.Types || {}; window.Signal.Types.Attachment = require('./js/modules/types/attachment'); window.Signal.Types.Errors = require('./js/modules/types/errors'); From dbdf6fd880f47aa1370df778c33e6aa759049cef Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:47:18 -0400 Subject: [PATCH 15/25] Run version 17 migration upon startup --- js/database.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/js/database.js b/js/database.js index 4bcd6bc82..e94f6c240 100644 --- a/js/database.js +++ b/js/database.js @@ -8,6 +8,8 @@ (function () { 'use strict'; + const { Migrations } = window.Signal; + window.Whisper = window.Whisper || {}; window.Whisper.Database = window.Whisper.Database || {}; window.Whisper.Database.id = window.Whisper.Database.id || 'signal'; @@ -233,5 +235,17 @@ next(); }, }, + { + version: 17, + async migrate(transaction, next) { + console.log('migration 17'); + console.log('Start migration to database version 17'); + + await Migrations.V17.run(transaction); + + console.log('Complete migration to database version 17'); + next(); + }, + }, ]; }()); From a76a6098c47920cac1456672cf397c58909648b6 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 11:55:08 -0400 Subject: [PATCH 16/25] Simplify log statement --- js/notifications.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/js/notifications.js b/js/notifications.js index f2e67138b..2c582e3b4 100644 --- a/js/notifications.js +++ b/js/notifications.js @@ -25,6 +25,7 @@ this.trigger('click', conversation); }, update: function() { + const {isEnabled} = this; const isFocused = window.isFocused(); const isAudioNotificationEnabled = storage.get('audio-notification') || false; const isAudioNotificationSupported = Settings.isAudioNotificationSupported(); @@ -33,13 +34,10 @@ const numNotifications = this.length; console.log( 'Update notifications:', - 'isFocused:', isFocused, - 'isEnabled:', this.isEnabled, - 'numNotifications:', numNotifications, - 'shouldPlayNotificationSound:', shouldPlayNotificationSound + {isFocused, isEnabled, numNotifications, shouldPlayNotificationSound} ); - if (!this.isEnabled) { + if (!isEnabled) { return; } From 5d927b73e602cf5d05772c74093462ee9c0aa0b8 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 12:09:48 -0400 Subject: [PATCH 17/25] Use `while` loop for IDB cursor iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, I messily combined promises and callbacks because I thought we were affected by the microtask issue: https://github.com/gasi/idb#iteratecursor--iteratekeycursor ESLint’s `more/no-then` encouraged me to revisit this and it works as expected. --- js/modules/migrations/17/index.js | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/js/modules/migrations/17/index.js b/js/modules/migrations/17/index.js index 618c95156..5a8472ec3 100644 --- a/js/modules/migrations/17/index.js +++ b/js/modules/migrations/17/index.js @@ -16,24 +16,24 @@ exports.run = async (transaction) => { await db.transaction.complete; }; -exports._initializeMessageSchemaVersion = messagesStore => - new Promise((resolve, reject) => { - messagesStore.openCursor().then(async function cursorIterate(cursor) { - const hasMoreResults = Boolean(cursor); - if (!hasMoreResults) { - return resolve(); - } - - const message = cursor.value; - console.log('Initialize schema version for message:', message.id); - - const messageWithInitializedSchemaVersion = Message.initializeSchemaVersion(message); - try { - await messagesStore.put(messageWithInitializedSchemaVersion, message.id); - } catch (error) { - console.log('Failed to put message with initialized schema version:', message.id); - } - - cursor.continue().then(cursorIterate); - }).catch(reject); - }); +// NOTE: We disable `no-await-in-loop` because we want this migration to happen +// in sequence and not in parallel: +// https://eslint.org/docs/rules/no-await-in-loop#when-not-to-use-it +exports._initializeMessageSchemaVersion = async (messagesStore) => { + let cursor = await messagesStore.openCursor(); + while (cursor) { + const message = cursor.value; + console.log('Initialize schema version for message:', message.id); + + const messageWithSchemaVersion = Message.initializeSchemaVersion(message); + try { + // eslint-disable-next-line no-await-in-loop + await messagesStore.put(messageWithSchemaVersion, message.id); + } catch (error) { + console.log('Failed to put message with initialized schema version:', message.id); + } + + // eslint-disable-next-line no-await-in-loop + cursor = await cursor.continue(); + } +}; From a5edbf8328930ea6437e91ee7bc6d18ff3b25468 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 15:46:28 -0400 Subject: [PATCH 18/25] Initialize schema as first step in `upgradeSchema` --- js/modules/types/message.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index c9f4696ba..b0253d4a1 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -146,6 +146,9 @@ exports._mapAttachments = upgradeAttachment => async message => } ); +const toVersion0 = async message => + exports.initializeSchemaVersion(message); + const toVersion1 = exports._withSchemaVersion( 1, exports._mapAttachments(Attachment.autoOrientJPEG) @@ -157,4 +160,4 @@ const toVersion2 = exports._withSchemaVersion( // UpgradeStep exports.upgradeSchema = async message => - toVersion2(await toVersion1(message)); + toVersion2(await toVersion1(await toVersion0(message))); From cd3aee962d7dbc83315377543a0a35863ac43e5e Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 16:00:12 -0400 Subject: [PATCH 19/25] Upgrade message schema before sending --- js/models/conversations.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index cfb935f86..d24882a77 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -617,18 +617,17 @@ now ); - const upgradedAttachments = - await Promise.all(attachments.map(Attachment.upgradeSchema)); - const message = this.messageCollection.add({ + const messageWithSchema = await Message.upgradeSchema({ + type: 'outgoing', body, conversationId: this.id, - type: 'outgoing', - attachments: upgradedAttachments, + attachments, sent_at: now, received_at: now, expireTimer: this.get('expireTimer'), recipients: this.getRecipients(), }); + const message = this.messageCollection.add(messageWithSchema); if (this.isPrivate()) { message.set({ destination: this.id }); } @@ -641,7 +640,7 @@ }); const conversationType = this.get('type'); - const sendFunc = (() => { + const sendFunction = (() => { switch (conversationType) { case Message.PRIVATE: return textsecure.messaging.sendMessageToNumber; @@ -657,10 +656,10 @@ profileKey = storage.get('profileKey'); } - message.send(sendFunc( + message.send(sendFunction( this.get('id'), body, - upgradedAttachments, + messageWithSchema.attachments, now, this.get('expireTimer'), profileKey From 0e20e8e2ea141d7b5992a4e85c7b06370bcc1b15 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 16:12:33 -0400 Subject: [PATCH 20/25] Use `Errors.toLogFormat` --- js/modules/types/message.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index b0253d4a1..ed951c7d9 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -1,6 +1,7 @@ const isFunction = require('lodash/isFunction'); const Attachment = require('./attachment'); +const Errors = require('./errors'); const SchemaVersion = require('./schema_version'); @@ -110,8 +111,7 @@ exports._withSchemaVersion = (schemaVersion, upgrade) => { } catch (error) { console.log( 'Message._withSchemaVersion: error:', - // TODO: Use `Errors.toLogFormat`: - error && error.stack ? error.stack : error + Errors.toLogFormat(error) ); return message; } From a619d48fac589f8ec9d0776b69ec7200c256fdd1 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 16:12:46 -0400 Subject: [PATCH 21/25] Update schema version history --- js/modules/types/message.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index ed951c7d9..71540a5ef 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -8,6 +8,14 @@ const SchemaVersion = require('./schema_version'); const GROUP = 'group'; const PRIVATE = 'private'; +// Schema version history +// +// Version 0 +// - Schema initialized +// Version 1 +// - Attachments: Auto-orient JPEG attachments using EXIF `Orientation` data +// Version 2 +// - Attachments: Sanitize Unicode order override characters const INITIAL_SCHEMA_VERSION = 0; // Increment this version number every time we add a message schema upgrade @@ -17,12 +25,6 @@ const INITIAL_SCHEMA_VERSION = 0; // how we do database migrations: exports.CURRENT_SCHEMA_VERSION = 2; -// Schema version history -// -// Version 1 -// - Attachments: Auto-orient JPEG attachments using EXIF `Orientation` data -// Version 2 -// - Attachments: Sanitize Unicode order override characters // Public API exports.GROUP = GROUP; From db2941cbb0bd0220fcf0f2f9f4dcc07df07137f1 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 14 Mar 2018 16:39:05 -0400 Subject: [PATCH 22/25] Measure duration of migration --- js/database.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/database.js b/js/database.js index e94f6c240..a7335abd3 100644 --- a/js/database.js +++ b/js/database.js @@ -241,9 +241,14 @@ console.log('migration 17'); console.log('Start migration to database version 17'); + const start = Date.now(); await Migrations.V17.run(transaction); + const duration = Date.now() - start; - console.log('Complete migration to database version 17'); + console.log( + 'Complete migration to database version 17.', + `Duration: ${duration}ms` + ); next(); }, }, From 2642844c27ceb2a8d3bf3d089b5391c61dd21103 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Fri, 16 Mar 2018 11:09:59 -0400 Subject: [PATCH 23/25] Rewrite migration 17 without `idb` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We ran into issues when doing async operations inside of an IndexedDB `onupgradeneeded` handler. The errors were ‘The transaction is not active’ or ‘Transaction has finished’. The following documentation confirmed that transactions are committed/terminated when control returns to the event loop: Spec - https://www.w3.org/TR/IndexedDB/#transaction-lifetime-concept - https://www.w3.org/TR/IndexedDB/#upgrade-transaction-construct Stack Overflow - https://stackoverflow.com/a/11059085 - https://stackoverflow.com/a/27338944 Since the initial database migration is so critical, I decided to avoid `idb` with promise support for IndexedDB for now, but will reconsider using it for other tasks in the future to improve readability of IndexedDB code. --- js/modules/migrations/17/index.js | 74 ++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/js/modules/migrations/17/index.js b/js/modules/migrations/17/index.js index 5a8472ec3..20631af15 100644 --- a/js/modules/migrations/17/index.js +++ b/js/modules/migrations/17/index.js @@ -1,39 +1,63 @@ -const idb = require('idb'); const Message = require('../../types/message'); exports.run = async (transaction) => { - const db = idb.upgradeDBFromTransaction(transaction); - const tx = db.transaction; - const messagesStore = tx.objectStore('messages'); + const messagesStore = transaction.objectStore('messages'); console.log('Initialize messages schema version'); - await exports._initializeMessageSchemaVersion(messagesStore); + const numUpgradedMessages = await _initializeMessageSchemaVersion(messagesStore); + console.log('Complete messages schema version initialization', { numUpgradedMessages }); console.log('Create index from attachment schema version to attachment'); messagesStore.createIndex('schemaVersion', 'schemaVersion', { unique: false }); - - await db.transaction.complete; }; -// NOTE: We disable `no-await-in-loop` because we want this migration to happen -// in sequence and not in parallel: -// https://eslint.org/docs/rules/no-await-in-loop#when-not-to-use-it -exports._initializeMessageSchemaVersion = async (messagesStore) => { - let cursor = await messagesStore.openCursor(); - while (cursor) { - const message = cursor.value; - console.log('Initialize schema version for message:', message.id); - - const messageWithSchemaVersion = Message.initializeSchemaVersion(message); +const _initializeMessageSchemaVersion = messagesStore => + new Promise((resolve, reject) => { + const messagePutOperations = []; + + const cursorRequest = messagesStore.openCursor(); + cursorRequest.onsuccess = (event) => { + const cursor = event.target.result; + const hasMoreData = Boolean(cursor); + if (!hasMoreData) { + // eslint-disable-next-line more/no-then + return Promise.all(messagePutOperations) + .then(() => resolve(messagePutOperations.length)); + } + + const message = cursor.value; + const messageWithSchemaVersion = Message.initializeSchemaVersion(message); + messagePutOperations.push(new Promise((resolvePut) => { + console.log( + 'Initialize schema version for message:', + messageWithSchemaVersion.id + ); + + resolvePut(putItem( + messagesStore, + messageWithSchemaVersion, + messageWithSchemaVersion.id + )); + })); + + return cursor.continue(); + }; + + cursorRequest.onerror = event => + reject(event.target.error); + }); + +// putItem :: IDBObjectStore -> Item -> Key -> Promise Item +const putItem = (store, item, key) => + new Promise((resolve, reject) => { try { - // eslint-disable-next-line no-await-in-loop - await messagesStore.put(messageWithSchemaVersion, message.id); + const request = store.put(item, key); + request.onsuccess = event => + resolve(event.target.result); + request.onerror = event => + reject(event.target.error); } catch (error) { - console.log('Failed to put message with initialized schema version:', message.id); + reject(error); } - - // eslint-disable-next-line no-await-in-loop - cursor = await cursor.continue(); - } -}; + }); From c88381efe3c695b4dbcbdf837e6fe9ae5fc81dee Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Fri, 16 Mar 2018 11:17:38 -0400 Subject: [PATCH 24/25] Use `async` / `await` to improve readability --- js/modules/migrations/17/index.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/js/modules/migrations/17/index.js b/js/modules/migrations/17/index.js index 20631af15..feffbb058 100644 --- a/js/modules/migrations/17/index.js +++ b/js/modules/migrations/17/index.js @@ -17,29 +17,21 @@ const _initializeMessageSchemaVersion = messagesStore => const messagePutOperations = []; const cursorRequest = messagesStore.openCursor(); - cursorRequest.onsuccess = (event) => { + cursorRequest.onsuccess = async (event) => { const cursor = event.target.result; const hasMoreData = Boolean(cursor); if (!hasMoreData) { - // eslint-disable-next-line more/no-then - return Promise.all(messagePutOperations) - .then(() => resolve(messagePutOperations.length)); + await Promise.all(messagePutOperations); + return resolve(messagePutOperations.length); } const message = cursor.value; const messageWithSchemaVersion = Message.initializeSchemaVersion(message); - messagePutOperations.push(new Promise((resolvePut) => { - console.log( - 'Initialize schema version for message:', - messageWithSchemaVersion.id - ); - - resolvePut(putItem( - messagesStore, - messageWithSchemaVersion, - messageWithSchemaVersion.id - )); - })); + messagePutOperations.push(putItem( + messagesStore, + messageWithSchemaVersion, + messageWithSchemaVersion.id + )); return cursor.continue(); }; From bde843682992cf1b11c68afd7ac1b8a8cb849ebe Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Fri, 16 Mar 2018 11:20:54 -0400 Subject: [PATCH 25/25] Remove `idb` dependency See f3c879a3b516645f908783a92c73bdfc143f20f2 for details. --- package.json | 1 - yarn.lock | 4 ---- 2 files changed, 5 deletions(-) diff --git a/package.json b/package.json index fedc452df..37eee2dbc 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,6 @@ "form-data": "^2.3.2", "google-libphonenumber": "^3.0.7", "got": "^8.2.0", - "idb": "https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20", "lodash": "^4.17.4", "mkdirp": "^0.5.1", "node-fetch": "https://github.com/scottnonnenberg/node-fetch.git#3e5f51e08c647ee5f20c43b15cf2d352d61c36b4", diff --git a/yarn.lock b/yarn.lock index 81afff81a..2e852329c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2724,10 +2724,6 @@ iconv-lite@0.4.19, iconv-lite@^0.4.17, iconv-lite@^0.4.19: version "0.4.19" resolved "https://registry.yarnpkg.com/iconv-lite/-/iconv-lite-0.4.19.tgz#f7468f60135f5e5dad3399c0a81be9a1603a082b" -"idb@https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20": - version "2.1.0" - resolved "https://github.com/gasi/idb.git#8efa534c41402a0685c5c716c9da38ff7435da20" - ieee754@^1.1.4: version "1.1.8" resolved "https://registry.yarnpkg.com/ieee754/-/ieee754-1.1.8.tgz#be33d40ac10ef1926701f6f08a2d86fbfd1ad3e4"