From 418d09e48a47a101147f9c88af3d30c55e487ae1 Mon Sep 17 00:00:00 2001 From: regevbr Date: Sat, 30 Nov 2019 19:27:45 +0200 Subject: [PATCH] Fix bug #1795 Count issues related models --- lib/scope.js | 143 ++++++++---------- test/relations.test.js | 335 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 399 insertions(+), 79 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index b5aeafb0d..712bcd9b6 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -91,7 +91,7 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres actualRefresh; if (refreshIsNeeded) { // It either doesn't hit the cache or refresh is required - const params = mergeQuery(actualCond, scopeParams, {nestedInclude: true}); + let params = mergeQuery(actualCond, scopeParams, {nestedInclude: true}); const targetModel = this.targetModel(receiver); // If there is a through model @@ -100,64 +100,39 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres const scopeOnRelatedModel = params.collect && params.include.scope !== null && typeof params.include.scope === 'object'; - let filter, queryRelated; + let filter, queryRelated, keyFrom, relatedModel, IdKey; if (scopeOnRelatedModel) { filter = params.include; // The filter applied on relatedModel queryRelated = filter.scope; - delete params.include.scope; + delete params.include; + relatedModel = targetModel.relations[filter.relation].modelTo; + IdKey = idName(relatedModel); + keyFrom = targetModel.relations[filter.relation].keyFrom || IdKey; + params.fields = [keyFrom]; + if (queryRelated && queryRelated.where && queryRelated.where[IdKey]) { + params = mergeQuery(params, { + where: { + [keyFrom]: queryRelated.where [IdKey], + }, + }); + } } - targetModel.find(params, options, function(err, data) { + targetModel.find(params, options, function(err, findData) { if (!err && saveOnCache) { defineCachedRelations(self); - self.__cachedRelations[name] = data; + self.__cachedRelations[name] = findData; } if (scopeOnRelatedModel === true) { - const relatedModel = targetModel.relations[filter.relation].modelTo; - const IdKey = idName(relatedModel); - - // return {inq: [1,2,3]}} - const smartMerge = function(idCollection, qWhere) { - if (!qWhere[IdKey]) return idCollection; - let merged = {}; - - const idsA = idCollection.inq; - const idsB = qWhere[IdKey].inq ? qWhere[IdKey].inq : [qWhere[IdKey]]; - - const intersect = _.intersectionWith(idsA, idsB, _.isEqual); - if (intersect.length === 1) merged = intersect[0]; - if (intersect.length > 1) merged = {inq: intersect}; - - return merged; - }; - - if (queryRelated.where !== undefined) { - // Merge queryRelated filter and targetId filter - const IdKeyCondition = {}; - IdKeyCondition[IdKey] = smartMerge(collectTargetIds(data, IdKey), - queryRelated.where); - - // if the id in filter doesn't exist after the merge, - // return empty result - if (_.isObject(IdKeyCondition[IdKey]) && _.isEmpty(IdKeyCondition[IdKey])) return cb(null, []); - - const mergedWhere = { - and: [ - IdKeyCondition, - _.omit(queryRelated.where, IdKey), - ], - }; - queryRelated.where = mergedWhere; - } else { - queryRelated.where = {}; - queryRelated.where[IdKey] = collectTargetIds(data, IdKey); + const smartMergeSuccessful = smartMergeRelatedModelQuery(findData, queryRelated, keyFrom, IdKey); + if (!smartMergeSuccessful) { + return cb(null, []); } - relatedModel.find(queryRelated, options, cb); } else { - cb(err, data); + cb(err, findData); } }); } else { @@ -298,6 +273,16 @@ function defineScope(cls, targetClass, name, params, methods, options) { options = {}; } options = options || {}; + // Check if there is a through model + // see https://github.com/strongloop/loopback/issues/1076 + if (f._scope.collect && + condOrRefresh !== null && typeof condOrRefresh === 'object') { + f._scope.include = { + relation: f._scope.collect, + scope: condOrRefresh, + }; + condOrRefresh = {}; + } return definition.related(self, f._scope, condOrRefresh, options, cb); }; @@ -679,46 +664,46 @@ function defineScope(cls, targetClass, name, params, methods, options) { }); } - function smartMergeRelatedModelQuery(findData, queryRelated, keyFrom, IdKey) { - const smartMerge = function(idCollection, qWhere) { - if (!qWhere[IdKey]) return idCollection; - let merged = {}; + return definition; +} - const idsA = idCollection.inq; - const idsB = qWhere[IdKey].inq ? qWhere[IdKey].inq : [qWhere[IdKey]]; +function smartMergeRelatedModelQuery(findData, queryRelated, keyFrom, IdKey) { + const smartMerge = function(idCollection, qWhere) { + if (!qWhere[IdKey]) return idCollection; + let merged = {}; - const intersect = _.intersectionWith(idsA, idsB, _.isEqual); - if (intersect.length === 1) merged = intersect[0]; - if (intersect.length > 1) merged = {inq: intersect}; + const idsA = idCollection.inq; + const idsB = qWhere[IdKey].inq ? qWhere[IdKey].inq : [qWhere[IdKey]]; - return merged; - }; + const intersect = _.intersectionWith(idsA, idsB, _.isEqual); + if (intersect.length === 1) merged = intersect[0]; + if (intersect.length > 1) merged = {inq: intersect}; - if (queryRelated.where !== undefined) { - // Merge queryRelated filter and targetId filter - const IdKeyCondition = {}; - IdKeyCondition[IdKey] = smartMerge(collectTargetIds(findData, keyFrom), - queryRelated.where); + return merged; + }; - // if the id in filter doesn't exist after the merge, - // return empty result - if (_.isObject(IdKeyCondition[IdKey]) && _.isEmpty(IdKeyCondition[IdKey])) { - return false; - } + if (queryRelated.where !== undefined) { + // Merge queryRelated filter and targetId filter + const IdKeyCondition = {}; + IdKeyCondition[IdKey] = smartMerge(collectTargetIds(findData, keyFrom), + queryRelated.where); - const mergedWhere = { - and: [ - IdKeyCondition, - _.omit(queryRelated.where, IdKey), - ], - }; - queryRelated.where = mergedWhere; - } else { - queryRelated.where = {}; - queryRelated.where[IdKey] = collectTargetIds(findData, keyFrom); + // if the id in filter doesn't exist after the merge, + // return empty result + if (_.isObject(IdKeyCondition[IdKey]) && _.isEmpty(IdKeyCondition[IdKey])) { + return false; } - return true; - } - return definition; + const mergedWhere = { + and: [ + IdKeyCondition, + _.omit(queryRelated.where, IdKey), + ], + }; + queryRelated.where = mergedWhere; + } else { + queryRelated.where = {}; + queryRelated.where[IdKey] = collectTargetIds(findData, keyFrom); + } + return true; } diff --git a/test/relations.test.js b/test/relations.test.js index 5b6fb8153..289a80b1d 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -605,6 +605,80 @@ describe('relations', function() { db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], done); }); + it('should allow to use fields on related model', function(done) { + Physician.create(function(err, physician) { + physician.patients.create({name: 'a', age: 5}, function(err, patient) { + Address.create({name: 'z'}, function(err, address) { + if (err) return done(err); + patient.address(address); + patient.save(function() { + verify(physician, address.id); + }); + }); + }); + }); + function verify(physician, addressId) { + physician.patients({fields: 'age'}, function(err, ch) { + if (err) return done(err); + should.exist(ch); + ch.should.have.lengthOf(1); + ch[0].age.should.eql(5); + should.not.exist(ch[0].name); + done(); + }); + } + }); + + it('should find related model using model id', function(done) { + Physician.create(function(err, physician) { + if (err) return done(err); + physician.patients.create({name: 'a', age: 5}, function(err, patient) { + if (err) return done(err); + physician.patients.create({name: 'b', age: 5}, function(err) { + if (err) return done(err); + physician.patients.create({name: 'c', age: 5}, function(err) { + if (err) return done(err); + verify(physician, patient.id); + }); + }); + }); + }); + function verify(physician, patientId) { + physician.patients({where: {id: patientId}}, function(err, ch) { + if (err) return done(err); + should.exist(ch); + ch.should.have.lengthOf(1); + ch[0].age.should.eql(5); + done(); + }); + } + }); + + it('should find related model using model id list', function(done) { + Physician.create(function(err, physician) { + if (err) return done(err); + physician.patients.create({name: 'a', age: 5}, function(err, patient) { + if (err) return done(err); + physician.patients.create({name: 'b', age: 5}, function(err) { + if (err) return done(err); + physician.patients.create({name: 'c', age: 5}, function(err) { + if (err) return done(err); + verify(physician, patient.id); + }); + }); + }); + }); + function verify(physician, patientId) { + physician.patients({where: {id: {inq: [patientId]}}}, function(err, ch) { + if (err) return done(err); + should.exist(ch); + ch.should.have.lengthOf(1); + ch[0].age.should.eql(5); + done(); + }); + } + }); + it('should count scoped record with promises based on related model properties', function(done) { let id; Physician.create() @@ -846,6 +920,267 @@ describe('relations', function() { } }); + it('should find scoped record with promises based on related model properties', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients({ + where: { + name: 'a', + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + + it('should find scoped record with promises based on related model' + + ' properties with empty results', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients({ + where: { + id: 'bar', + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(0); + done(); + }); + } + }); + + it('should find scoped record with promises based on related model id', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients({ + where: { + id, + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + + it('should find scoped record with promises based on related model id list', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients({ + where: { + id: { + inq: [id], + }, + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + + it('should find (explicit) scoped record with promises based on related' + + ' model properties', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients.find({ + where: { + name: 'a', + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + + it('should find (explicit) scoped record with promises based on related model' + + ' properties with empty results', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients.find({ + where: { + id: 'bar', + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(0); + done(); + }); + } + }); + + it('should find (explicit) scoped record with promises based on related model id', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients.find({ + where: { + id, + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + + it('should find (explicit) scoped record with promises based on related model id list', function(done) { + let id; + Physician.create() + .then(function(physician) { + return physician.patients.create({name: 'a'}) + .then(function(ch) { + id = ch.id; + return physician.patients.create({name: 'z'}); + }) + .then(function() { + return physician.patients.create({name: 'c'}); + }) + .then(function() { + return verify(physician); + }); + }).catch(done); + + function verify(physician) { + return physician.patients.find({ + where: { + id: { + inq: [id], + }, + }, + }, function(err, patients) { + if (err) return done(err); + should.exist(patients); + patients.should.have.lengthOf(1); + patients[0].name.should.equal('a'); + done(); + }); + } + }); + it('should update all scoped record with promises based on related model properties', function(done) { let id; Physician.create()