Merge pull request #1143 from doublemarked/hm/coerce-array-like-objec…

…ts-as-arrays Coercing conditions with array-like objects into arrays
loopbackio · Jan 10, 2017 · b530ec2 · b530ec2
2 parents 6f2a075 + 2377792
commit b530ec2
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 13 deletions.
diff --git a/lib/dao.js b/lib/dao.js
@@ -1526,6 +1526,27 @@ function NumberType(val) {
   return !isNaN(num) ? num : val;
 }
 
+function coerceArray(val) {
+  if (Array.isArray(val)) {
+    return val;
+  }
+
+  if (!utils.isPlainObject(val)) {
+    throw new Error(g.f('Value is not an {{array}} or {{object}} with sequential numeric indices'));
+  }
+
+  var arrayVal = new Array(Object.keys(val).length);
+  for (var i = 0; i < arrayVal.length; ++i) {
+    if (!val.hasOwnProperty(i)) {
+      throw new Error(g.f('Value is not an {{array}} or {{object}} with sequential numeric indices'));
+    }
+
+    arrayVal[i] = val[i];
+  }
+
+  return arrayVal;
+}
+
 /*
  * Coerce values based the property types
  * @param {Object} where The where clause
@@ -1550,16 +1571,18 @@ DataAccessObject._coerce = function(where) {
     // Handle logical operators
     if (p === 'and' || p === 'or' || p === 'nor') {
       var clauses = where[p];
-      if (Array.isArray(clauses)) {
-        for (var k = 0; k < clauses.length; k++) {
-          self._coerce(clauses[k]);
-        }
-      } else {
-        err = new Error(g.f('The %s operator has invalid clauses %j', p, clauses));
+      try {
+        clauses = coerceArray(clauses);
+      } catch (e) {
+        err = new Error(g.f('The %s operator has invalid clauses %j: %s', p, clauses, e.message));
         err.statusCode = 400;
         throw err;
       }
 
+      for (var k = 0; k < clauses.length; k++) {
+        self._coerce(clauses[k]);
+      }
+
       continue;
     }
     var DataType = props[p] && props[p].type;
@@ -1614,15 +1637,21 @@ DataAccessObject._coerce = function(where) {
           switch (operator) {
             case 'inq':
             case 'nin':
-              if (!Array.isArray(val)) {
-                err = new Error(g.f('The %s property has invalid clause %j', p, where[p]));
+            case 'between':
+              try {
+                val = coerceArray(val);
+              } catch (e) {
+                err = new Error(g.f('The %s property has invalid clause %j: %s', p, where[p], e));
                 err.statusCode = 400;
                 throw err;
               }
-              break;
-            case 'between':
-              if (!Array.isArray(val) || val.length !== 2) {
-                err = new Error(g.f('The %s property has invalid clause %j', p, where[p]));
+
+              if (operator === 'between' && val.length !== 2) {
+                err = new Error(g.f(
+                  'The %s property has invalid clause %j: Expected precisely 2 values, received %d',
+                  p,
+                  where[p],
+                  val.length));
                 err.statusCode = 400;
                 throw err;
               }
@@ -1632,7 +1661,10 @@ DataAccessObject._coerce = function(where) {
             case 'ilike':
             case 'nilike':
               if (!(typeof val === 'string' || val instanceof RegExp)) {
-                err = new Error(g.f('The %s property has invalid clause %j', p, where[p]));
+                err = new Error(g.f(
+                  'The %s property has invalid clause %j: Expected a string or RegExp',
+                  p,
+                  where[p]));
                 err.statusCode = 400;
                 throw err;
               }
@@ -1649,6 +1681,14 @@ DataAccessObject._coerce = function(where) {
         }
       }
     }
+
+    try {
+      // Coerce val into an array if it resembles an array-like object
+      val = coerceArray(val);
+    } catch (e) {
+      // NOOP when not coercable into an array.
+    }
+
     // Coerce the array items
     if (Array.isArray(val)) {
       for (var i = 0; i < val.length; i++) {

diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js
@@ -1412,6 +1412,53 @@ describe('DataAccessObject', function() {
     assert.deepEqual(where, {and: [{age: 10}], vip: true});
   });
 
+  const COERCIONS = [
+    {
+      in: {scores: {0: '10', 1: '20'}},
+      out: {scores: [10, 20]},
+    },
+    {
+      in: {and: {0: {age: '10'}, 1: {vip: 'true'}}},
+      out: {and: [{age: 10}, {vip: true}]},
+    },
+    {
+      in: {or: {0: {age: '10'}, 1: {vip: 'true'}}},
+      out: {or: [{age: 10}, {vip: true}]},
+    },
+    {
+      in: {id: {inq: {0: 'aaa', 1: 'bbb'}}},
+      out: {id: {inq: ['aaa', 'bbb']}},
+    },
+    {
+      in: {id: {nin: {0: 'aaa', 1: 'bbb'}}},
+      out: {id: {nin: ['aaa', 'bbb']}},
+    },
+    {
+      in: {scores: {between: {0: '0', 1: '42'}}},
+      out: {scores: {between: [0, 42]}},
+    },
+  ];
+
+  COERCIONS.forEach(coercion => {
+    var inStr = JSON.stringify(coercion.in);
+    it('coerces where clause with array-like objects ' + inStr, () => {
+      assert.deepEqual(model._coerce(coercion.in), coercion.out);
+    });
+  });
+
+  const INVALID_CLAUSES = [
+    {scores: {inq: {0: '10', 1: '20', 4: '30'}}},
+    {scores: {inq: {0: '10', 1: '20', bogus: 'true'}}},
+    {scores: {between: {0: '10', 1: '20', 2: '30'}}},
+  ];
+
+  INVALID_CLAUSES.forEach((where) => {
+    var whereStr = JSON.stringify(where);
+    it('throws an error on malformed array-like object ' + whereStr, () => {
+      assert.throws(() => model._coerce(where), /property has invalid clause/);
+    });
+  });
+
   it('throws an error if the where property is not an object', function() {
     try {
       // The where clause has to be an object