Merge pull request #54 from dangerginger/update-enforces-schema

[WIP] .update() enforces schema

lib/table.js

@@ -230,6 +230,60 @@ internals.updateExpressions = (schema, data, options) => {
   });
 };
 
+internals.validateItemFragment = (item, schema) => {
+  const result = {};
+  const error = {};
+
+  // get the list of attributes to remove
+  const removeAttributes = _.pickBy(item, _.isNull);
+
+  // get the list of attributes whose value is an object
+  const setOperationValues = _.pickBy(item, i => _.isPlainObject(i) && (i.$add || i.$del));
+
+  // get the list of attributes to modify
+  const updateAttributes = _.omit(
+    item,
+    Object.keys(removeAttributes).concat(Object.keys(setOperationValues))
+  );
+
+  // check attribute removals for .required() schema violation
+  const removalValidation = schema.validate(
+    {},
+    { abortEarly: false }
+  );
+
+  if (removalValidation.error) {
+    const errors = _.pickBy(
+      removalValidation.error.details,
+      e => _.isEqual(e.type, 'any.required')
+      && Object.prototype.hasOwnProperty.call(removeAttributes, e.path)
+    );
+    if (!_.isEmpty(errors)) {
+      error.remove = errors;
+      result.error = error;
+    }
+  }
+
+  // check attribute updates match the schema
+  const updateValidation = schema.validate(
+    updateAttributes,
+    { abortEarly: false }
+  );
+
+  if (updateValidation.error) {
+    const errors = _.omitBy(
+      updateValidation.error.details,
+      e => _.isEqual(e.type, 'any.required')
+    );
+    if (!_.isEmpty(errors)) {
+      error.update = errors;
+      result.error = error;
+    }
+  }
+
+  return result;
+};
+
 Table.prototype.update = function (item, options, callback) {
   const self = this;
 
@@ -241,6 +295,11 @@ Table.prototype.update = function (item, options, callback) {
   callback = callback || _.noop;
   options = options || {};
 
+  const schemaValidation = internals.validateItemFragment(item, self.schema);
+  if (schemaValidation.error) {
+    return callback(new Error(schemaValidation.error));
+  }
+
   const start = callback => {
     const paramName = _.isString(self.schema.updatedAt) ? self.schema.updatedAt : 'updatedAt';
 

test/integration/integration-test.js

@@ -430,6 +430,61 @@ describe('Dynogels Integration Tests', function () {
         return done();
       });
     });
+
+    it('should fail to update an attribute not in the schema', (done) => {
+      User.update({
+        id: '123456789',
+        invalidAttribute: 'Invalid Value'
+      }, (err, account) => {
+        expect(err).to.exist;
+        expect(account).to.not.exist;
+        return done();
+      });
+    });
+
+    it('should fail to remove a required attribute', (done) => {
+      User.update({
+        id: '123456789',
+        email: null
+      }, (err, account) => {
+        expect(err).to.exist;
+        expect(account).to.not.exist;
+        done();
+      });
+    });
+
+    it('should successfully remove an optional attribute', (done) => {
+      User.update({
+        id: '123456789',
+        name: null,
+      }, (err, acc) => {
+        expect(err).to.be.null;
+        expect(acc).to.exist;
+        done();
+      });
+    });
+
+    it('should fail for attribute mismatch to schema type', (done) => {
+      User.update({
+        id: '123456789',
+        name: 1
+      }, (err, account) => {
+        expect(err).to.exist;
+        expect(account).to.not.exist;
+        done();
+      });
+    });
+
+    it('should fail to use $add for an invalid attribute', (done) => {
+      User.update({
+        id: '123456789',
+        name: { $add: '$addname' }
+      }, (err, acc) => {
+        expect(err).to.exist;
+        expect(acc).to.not.exist;
+        done();
+      });
+    });
   });
 
   describe('#getItems', () => {

test/table-test.js

@@ -706,8 +706,8 @@ describe('table', () => {
         hashKey: 'userId',
         rangeKey: 'timeOffset',
         schema: {
-          hashKey: Joi.number(),
-          rangeKey: Joi.number()
+          userId: Joi.number(),
+          timeOffset: Joi.number()
         }
       };
 
@@ -727,6 +727,8 @@ describe('table', () => {
 
       const item = { userId: 0, timeOffset: 0 };
       table.update(item, (err, user) => {
+        expect(err).to.be.null;
+
         user.should.be.instanceof(Item);
 
         user.get('userId').should.equal(0);