From 49242410280599cb6ff691bac59ad9535216742d Mon Sep 17 00:00:00 2001 From: biniam Date: Thu, 25 Apr 2019 09:49:11 -0400 Subject: [PATCH] fix: allow coercion of nested properties Handle model definitions with nested property definitions for coercion of primitive values. --- lib/dao.js | 1 - lib/model-utils.js | 44 ++++++++++++++---- test/manipulation.test.js | 2 +- test/model-utils.test.js | 94 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 test/model-utils.test.js diff --git a/lib/dao.js b/lib/dao.js index 584fcdd0..75bb0b09 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -2303,7 +2303,6 @@ DataAccessObject.updateAll = function(where, data, options, cb) { if (!(data instanceof Model)) { try { inst = new Model(data, {applyDefaultValues: false}); - ctx.data = inst.toObject(true); } catch (err) { return cb(err); } diff --git a/lib/model-utils.js b/lib/model-utils.js index 5abd9715..05c529a1 100644 --- a/lib/model-utils.js +++ b/lib/model-utils.js @@ -350,11 +350,12 @@ ModelUtils._sanitizeData = function(data, options) { * Coerce values based the property types * @param {Object} where The where clause * @options {Object} [options] Optional options to use. + * @param {Object} Optional model definition to use. * @property {Boolean} allowExtendedOperators. * @returns {Object} The coerced where clause * @private */ -ModelUtils._coerce = function(where, options) { +ModelUtils._coerce = function(where, options, modelDef) { const self = this; if (where == null) { return where; @@ -367,8 +368,13 @@ ModelUtils._coerce = function(where, options) { err.statusCode = 400; throw err; } + let props; + if (modelDef && modelDef.properties) { + props = modelDef.properties; + } else { + props = self.definition.properties; + } - const props = self.definition.properties; for (const p in where) { // Handle logical operators if (p === 'and' || p === 'or' || p === 'nor') { @@ -393,7 +399,7 @@ ModelUtils._coerce = function(where, options) { if (!DataType) { continue; } - if (Array.isArray(DataType) || DataType === Array) { + if ((Array.isArray(DataType) || DataType === Array) && !isNestedModel(DataType)) { DataType = DataType[0]; } if (DataType === Date) { @@ -413,10 +419,6 @@ ModelUtils._coerce = function(where, options) { continue; } - if (DataType.prototype instanceof BaseModel) { - continue; - } - if (DataType === geo.GeoPoint) { // Skip the GeoPoint as the near operator breaks the assumption that // an operation has only one property @@ -497,7 +499,7 @@ ModelUtils._coerce = function(where, options) { const allowExtendedOperators = self._allowExtendedOperators(options); // Coerce the array items - if (Array.isArray(val)) { + if (Array.isArray(val) && !isNestedModel(DataType)) { for (let i = 0; i < val.length; i++) { if (val[i] !== null && val[i] !== undefined) { if (!(val[i] instanceof RegExp)) { @@ -534,7 +536,19 @@ ModelUtils._coerce = function(where, options) { throw err; } } - val = DataType(val); + if (isNestedModel(DataType)) { + if (Array.isArray(DataType) && Array.isArray(val)) { + if (val === null || val === undefined) continue; + for (const it of val) { + self._coerce(it, options, DataType[0].definition); + } + } else { + self._coerce(val, options, DataType.definition); + } + continue; + } else { + val = DataType(val); + } } } } @@ -553,3 +567,15 @@ ModelUtils._coerce = function(where, options) { return where; }; +/** +* A utility function which checks for nested property definitions +* +* @param {*} propType Property type metadata +* +*/ +function isNestedModel(propType) { + if (!propType) return false; + if (Array.isArray(propType)) return isNestedModel(propType[0]); + return propType.hasOwnProperty('definition') && propType.definition.hasOwnProperty('properties'); +} + diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 917fbdda..7a42726d 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -2390,7 +2390,7 @@ describe('manipulation', function() { it('should not coerce invalid values provided in where conditions', function(done) { Person.update({name: 'Brett Boe'}, {dob: 'notadate'}, function(err) { should.exist(err); - err.message.should.equal('Invalid date: Invalid Date'); + err.message.should.equal('Invalid date: notadate'); done(); }); }); diff --git a/test/model-utils.test.js b/test/model-utils.test.js new file mode 100644 index 00000000..39d2e6ca --- /dev/null +++ b/test/model-utils.test.js @@ -0,0 +1,94 @@ +'use strict'; +let db; + +describe('model-utils', () => { + context('coerce', () => { + before(() => { + // eslint-disable-next-line no-undef + db = getSchema(); + }); + it('coerces nested properties', () => { + const nestedModel = db.define('nestedModel', { + rootProp: { + numProp: Number, + dateProp: Date, + nestedArray: [{ + numProp: Number, + dateProp: Date, + arrayWithinArray: [{ + numProp: Number, + dateProp: Date, + }], + objectWithinArray: { + numProp: Number, + dateProp: Date, + }, + }], + nestedObject: { + numProp: Number, + dateProp: Date, + arrayWithinObject: [{ + numProp: Number, + dateProp: Date, + }], + objectWithinObject: { + numProp: Number, + dateProp: Date, + }, + }, + }, + }); + const dateVal = new Date().toString(); + const data = { + rootProp: { + numProp: '0', + dateProp: dateVal, + nestedArray: [{ + numProp: '1', + dateProp: dateVal, + arrayWithinArray: [ + { + numProp: '2', + dateProp: dateVal, + }, + ], + objectWithinArray: { + numProp: '3', + dateProp: dateVal, + }, + }], + nestedObject: { + numProp: '5', + dateProp: dateVal, + arrayWithinObject: [{ + numProp: '6', + dateProp: dateVal, + }], + objectWithinObject: { + numProp: '7', + dateProp: dateVal, + }, + }, + }, + }; + const coercedData = nestedModel._coerce(data, {}); + assertInstanceOf(coercedData.rootProp.numProp, Number); + assertInstanceOf(coercedData.rootProp.dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedArray[0].numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedArray[0].dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedArray[0].arrayWithinArray[0].numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedArray[0].arrayWithinArray[0].dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedArray[0].objectWithinArray.numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedArray[0].objectWithinArray.dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedObject.numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedObject.dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedObject.objectWithinObject.numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedObject.objectWithinObject.dateProp, Date); + assertInstanceOf(coercedData.rootProp.nestedObject.arrayWithinObject[0].numProp, Number); + assertInstanceOf(coercedData.rootProp.nestedObject.arrayWithinObject[0].dateProp, Date); + }); + function assertInstanceOf(val, type) { + val.should.be.instanceOf(type); + } + }); +});