From 6c8e806bc80c49e59800c7feabe0a26c7c9f0704 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Thu, 27 Oct 2016 10:18:54 -0400 Subject: [PATCH] Apply hasManyThrough filter on target model --- lib/include.js | 10 +-- lib/scope.js | 51 ++++++++++--- lib/utils.js | 29 ++++++++ test/relations.test.js | 159 +++++++++++++++++++++++++++++++++-------- 4 files changed, 202 insertions(+), 47 deletions(-) diff --git a/lib/include.js b/lib/include.js index d36d4976..1044ee84 100644 --- a/lib/include.js +++ b/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 diff --git a/lib/scope.js b/lib/scope.js index f5ca6e34..aeb72179 100644 --- a/lib/scope.js +++ b/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, diff --git a/lib/utils.js b/lib/utils.js index 0c774bd8..839301d2 100644 --- a/lib/utils.js +++ b/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'; +} diff --git a/test/relations.test.js b/test/relations.test.js index e7821258..ebdcd1d7 100644 --- a/test/relations.test.js +++ b/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(); }); }); }); - 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('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(); }); }); }); - } + }); + 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) {