Merge pull request #1726 from strongloop/fix/nested-property-coercion

fix: allow coercion of nested properties

Handle model definitions with nested property
definitions for coercion of primitive values.

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

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

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

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