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); + }); + }); });