Move schema versioning from Attachment to Message

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

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

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

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