Merge pull request #1153 from strongloop/fix/filter-on-related-model

Apply filter on related model

Author: Kevin Delisle

lib/include.js

@@ -13,6 +13,7 @@ var includeUtils = require('./include_utils');
 var isPlainObject = utils.isPlainObject;
 var defineCachedRelations = utils.defineCachedRelations;
 var uniq = utils.uniq;
+var idName = utils.idName;
 
 /*!
  * Normalize the include to be an array
@@ -68,15 +69,6 @@ IncludeScope.prototype.include = function() {
   return this._include;
 };
 
-/**
- * Find the idKey of a Model.
- * @param {ModelConstructor} m - Model Constructor
- * @returns {String}
- */
-function idName(m) {
-  return m.definition.idName() || 'id';
-}
-
 /*!
  * Look up a model by name from the list of given models
  * @param {Object} models Models keyed by name

lib/scope.js

@@ -10,6 +10,8 @@ var defineCachedRelations = utils.defineCachedRelations;
 var setScopeValuesFromWhere = utils.setScopeValuesFromWhere;
 var mergeQuery = utils.mergeQuery;
 var DefaultModelBaseClass = require('./model.js');
+var collectTargetIds = utils.collectTargetIds;
+var idName = utils.idName;
 
 /**
  * Module exports
@@ -86,12 +88,50 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres
     // It either doesn't hit the cache or refresh is required
     var params = mergeQuery(actualCond, scopeParams, {nestedInclude: true});
     var targetModel = this.targetModel(receiver);
+
+    // If there is a through model
+    // run another query to apply filter on relatedModel(targetModel)
+    // see github.com/strongloop/loopback-datasource-juggler/issues/166
+    var scopeOnRelatedModel = params.collect &&
+        params.include.scope !== null &&
+        typeof params.include.scope === 'object';
+    if (scopeOnRelatedModel) {
+      var filter = params.include;
+      // The filter applied on relatedModel
+      var queryRelated = filter.scope;
+      delete params.include.scope;
+    };
+
     targetModel.find(params, options, function(err, data) {
       if (!err && saveOnCache) {
         defineCachedRelations(self);
         self.__cachedRelations[name] = data;
       }
-      cb(err, data);
+
+      if (scopeOnRelatedModel === true) {
+        var relatedModel = targetModel.relations[filter.relation].modelTo;
+        var IdKey = idName(relatedModel);
+
+        // Merge queryRelated filter and targetId filter
+        var buildWhere = function() {
+          var IdKeyCondition = {};
+          IdKeyCondition[IdKey] = collectTargetIds(data, IdKey);
+          var mergedWhere = {
+            and: [IdKeyCondition, queryRelated.where],
+          };
+          return mergedWhere;
+        };
+        if (queryRelated.where !== undefined) {
+          queryRelated.where = buildWhere();
+        } else {
+          queryRelated.where = {};
+          queryRelated.where[IdKey] = collectTargetIds(data, IdKey);
+        }
+
+        relatedModel.find(queryRelated, cb);
+      } else {
+        cb(err, data);
+      }
     });
   } else {
     // Return from cache
@@ -198,15 +238,6 @@ function defineScope(cls, targetClass, name, params, methods, options) {
           // see https://github.com/strongloop/loopback/issues/1076
           if (f._scope.collect &&
             condOrRefresh !== null && typeof condOrRefresh === 'object') {
-            //extract the paging filters to the through model
-            ['limit', 'offset', 'skip', 'order'].forEach(function(pagerFilter) {
-              if (typeof(condOrRefresh[pagerFilter]) !== 'undefined') {
-                f._scope[pagerFilter] = condOrRefresh[pagerFilter];
-                delete condOrRefresh[pagerFilter];
-              }
-            });
-            // Adjust the include so that the condition will be applied to
-            // the target model
             f._scope.include = {
               relation: f._scope.collect,
               scope: condOrRefresh,

lib/utils.js

@@ -22,6 +22,8 @@ exports.toRegExp = toRegExp;
 exports.hasRegExpFlags = hasRegExpFlags;
 exports.idEquals = idEquals;
 exports.findIndexOf = findIndexOf;
+exports.collectTargetIds = collectTargetIds;
+exports.idName = idName;
 
 var g = require('strong-globalize')();
 var traverse = require('traverse');
@@ -587,3 +589,30 @@ function findIndexOf(arr, target, isEqual) {
 
   return -1;
 }
+
+/**
+ * Returns an object that queries targetIds.
+ * @param {Array} The array of targetData
+ * @param {String} The Id property name of target model
+ * @returns {Object} The object that queries targetIds
+ */
+function collectTargetIds(targetData, idPropertyName) {
+  var targetIds = [];
+  for (var i = 0; i < targetData.length; i++) {
+    var targetId = targetData[i][idPropertyName];
+    targetIds.push(targetId);
+  };
+  var IdQuery = {
+    inq: uniq(targetIds),
+  };
+  return IdQuery;
+}
+
+/**
+ * Find the idKey of a Model.
+ * @param {ModelConstructor} m - Model Constructor
+ * @returns {String}
+ */
+function idName(m) {
+  return m.definition.idName() || 'id';
+}

test/relations.test.js

@@ -6,6 +6,7 @@
 // This test written in mocha+should.js
 'use strict';
 var should = require('./init.js');
+var assert = require('assert');
 var jdb = require('../');
 var DataSource = jdb.DataSource;
 var createPromiseCallback = require('../lib/utils.js').createPromiseCallback;
@@ -562,7 +563,7 @@ describe('relations', function() {
     before(function(done) {
       // db = getSchema();
       Physician = db.define('Physician', {name: String});
-      Patient = db.define('Patient', {name: String});
+      Patient = db.define('Patient', {name: String, age: Number});
       Appointment = db.define('Appointment', {date: {type: Date,
         default: function() {
           return new Date();
@@ -714,40 +715,142 @@ describe('relations', function() {
       }
     });
 
-    it('should fetch scoped instances with paging filters', function(done) {
-      Physician.create(function(err, physician) {
-        physician.patients.create({name: 'a'}, function() {
-          physician.patients.create({name: 'z'}, function() {
-            physician.patients.create({name: 'c'}, function() {
-              verify(physician);
+    describe('fetch scoped instances with paging filters', function() {
+      var samplePatientId;
+      var physician;
+
+      beforeEach(createSampleData);
+
+      context('with filter skip', function() {
+        it('skips the first patient', function(done) {
+          physician.patients({skip: 1}, function(err, ch) {
+            should.not.exist(err);
+            should.exist(ch);
+            ch.should.have.lengthOf(2);
+            ch[0].name.should.eql('z');
+            ch[1].name.should.eql('c');
+            done();
+          });
+        });
+      });
+      context('with filter order', function() {
+        it('orders the result by patient name', function(done) {
+          physician.patients({order: 'name DESC'}, function(err, ch) {
+            should.not.exist(err);
+            should.exist(ch);
+            ch.should.have.lengthOf(3);
+            ch[0].name.should.eql('z');
+            ch[2].name.should.eql('a');
+            done();
+          });
+        });
+      });
+      context('with filter limit', function() {
+        it('limits to 1 result', function(done) {
+          physician.patients({limit: 1}, function(err, ch) {
+            should.not.exist(err);
+            should.exist(ch);
+            ch.should.have.lengthOf(1);
+            ch[0].name.should.eql('a');
+            done();
+          });
+        });
+      });
+      context('with filter fields', function() {
+        it('includes field \'name\' but not \'age\'', function(done) {
+          var fieldsFilter = {fields: {name: true, age: false}};
+          physician.patients(fieldsFilter, function(err, ch) {
+            should.not.exist(err);
+            should.exist(ch);
+            should.exist(ch[0].name);
+            ch[0].name.should.eql('a');
+            should.not.exist(ch[0].age);
+            done();
+          });
+        });
+      });
+      context('with filter include', function() {
+        it('returns physicians inluced in patient', function(done) {
+          var includeFilter = {include: 'physicians'};
+          physician.patients(includeFilter, function(err, ch) {
+            should.not.exist(err);
+            ch.should.have.lengthOf(3);
+            should.exist(ch[0].physicians);
+            done();
+          });
+        });
+      });
+      context('with filter where', function() {
+        it('returns patient where id equal to samplePatientId', function(done) {
+          var whereFilter = {where: {id: samplePatientId}};
+          physician.patients(whereFilter, function(err, ch) {
+            should.not.exist(err);
+            should.exist(ch);
+            ch.should.have.lengthOf(1);
+            ch[0].id.should.eql(samplePatientId);
+            done();
+          });
+        });
+        it('returns patients where id in an array', function(done) {
+          var idArr = [];
+          var whereFilter;
+          physician.patients.create({name: 'b'}, function(err, p) {
+            idArr.push(samplePatientId, p.id);
+            whereFilter = {where: {id: {inq: idArr}}};
+            physician.patients(whereFilter, function(err, ch) {
+              should.not.exist(err);
+              should.exist(ch);
+              ch.should.have.lengthOf(2);
+              var resultIdArr = [ch[0].id, ch[1].id];
+              assert.deepEqual(resultIdArr, idArr);
+              done();
             });
           });
         });
       });
-      function verify(physician) {
-        //limit plus skip
-        physician.patients({limit: 1, skip: 1}, function(err, ch) {
-          should.not.exist(err);
-          should.exist(ch);
-          ch.should.have.lengthOf(1);
-          ch[0].name.should.eql('z');
-          //offset plus skip
-          physician.patients({limit: 1, offset: 1}, function(err1, ch1) {
-            should.not.exist(err1);
-            should.exist(ch1);
-            ch1.should.have.lengthOf(1);
-            ch1[0].name.should.eql('z');
-            //order
-            physician.patients({order: 'patientId DESC'}, function(err2, ch2) {
-              should.not.exist(err2);
-              should.exist(ch2);
-              ch2.should.have.lengthOf(3);
-              ch2[0].name.should.eql('c');
+      context('findById with filter include', function() {
+        it('returns patient where id equal to \'samplePatientId\'' +
+          'with included physicians', function(done) {
+          var includeFilter = {include: 'physicians'};
+          physician.patients.findById(samplePatientId,
+            includeFilter, function(err, ch) {
+              should.not.exist(err);
+              should.exist(ch);
+              ch.id.should.eql(samplePatientId);
+              should.exist(ch.physicians);
+              done();
+            });
+        });
+      });
+      context('findById with filter fields', function() {
+        it('returns patient where id equal to \'samplePatientId\'' +
+          'with field \'name\' but not \'age\'', function(done) {
+          var fieldsFilter = {fields: {name: true, age: false}};
+          physician.patients.findById(samplePatientId,
+            fieldsFilter, function(err, ch) {
+              should.not.exist(err);
+              should.exist(ch);
+              should.exist(ch.name);
+              ch.name.should.eql('a');
+              should.not.exist(ch.age);
               done();
             });
+        });
+      });
+
+      function createSampleData(done) {
+        Physician.create(function(err, result) {
+          result.patients.create({name: 'a', age: '10'}, function(err, p) {
+            samplePatientId = p.id;
+            result.patients.create({name: 'z', age: '20'}, function() {
+              result.patients.create({name: 'c'}, function() {
+                physician = result;
+                done();
+              });
+            });
           });
         });
-      }
+      };
     });
 
     it('should find scoped record', function(done) {