Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Count issue with related models using though model #1795 #1801

Closed
wants to merge 1 commit into from
+654 −54
Closed

Count issue with related models using though model #1795 #1801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
290 changes: 236 additions & 54 deletions lib/scope.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -431,11 +416,49 @@ function defineScope(cls, targetClass, name, params, methods, options) {
options = {};
}
options = options || {};

cb = cb || utils.createPromiseCallback();
const targetModel = definition.targetModel(this._receiver);
// If there is a through model
// run another query to apply filter on relatedModel(targetModel)
// see github.com/strongloop/loopback-datasource-juggler/issues/1795
let scopeOnRelatedModel = false;
let queryRelated, keyFrom, relatedModel, IdKey, fieldsRelated;
if (this._scope && this._scope.collect &&
where !== null && typeof where === 'object') {
queryRelated = {
relation: this._scope.collect,
scope: {
where: where,
},
};
scopeOnRelatedModel = true;
relatedModel = targetModel.relations[queryRelated.relation].modelTo;
IdKey = idName(relatedModel);
keyFrom = targetModel.relations[queryRelated.relation].keyFrom || IdKey;
fieldsRelated = [keyFrom];
if (where[IdKey]) {
where = {
[keyFrom]: where[IdKey],
};
} else {
where = {};
}
}
Comment on lines +424 to +446
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found repeated code in these CRUD methods. Maybe we can refactor them to have a helper function.

const scoped = (this._scope && this._scope.where) || {};
const filter = mergeQuery({where: scoped}, {where: where || {}});
return targetModel.destroyAll(filter.where, options, cb);
const filter = mergeQuery({where: scoped}, {where: where || {}, fields: fieldsRelated});
if (!scopeOnRelatedModel) {
targetModel.destroyAll(filter.where, options, cb);
} else {
targetModel.find(filter, options, function(err, findData) {
const smartMergeSuccessful =
smartMergeRelatedModelQuery(findData, queryRelated.scope, keyFrom, IdKey);
if (!smartMergeSuccessful) {
return cb(null, {count: 0});
}
return relatedModel.destroyAll(queryRelated.scope.where, options, cb);
});
}
return cb.promise;
}

function updateAll(where, data, options, cb) {
Expand All @@ -452,10 +475,49 @@ function defineScope(cls, targetClass, name, params, methods, options) {
options = {};
}
options = options || {};
cb = cb || utils.createPromiseCallback();
const targetModel = definition.targetModel(this._receiver);
// If there is a through model
// run another query to apply filter on relatedModel(targetModel)
// see github.com/strongloop/loopback-datasource-juggler/issues/1795
let scopeOnRelatedModel = false;
let queryRelated, keyFrom, relatedModel, IdKey, fieldsRelated;
if (this._scope && this._scope.collect &&
where !== null && typeof where === 'object') {
queryRelated = {
relation: this._scope.collect,
scope: {
where: where,
},
};
scopeOnRelatedModel = true;
relatedModel = targetModel.relations[queryRelated.relation].modelTo;
IdKey = idName(relatedModel);
keyFrom = targetModel.relations[queryRelated.relation].keyFrom || IdKey;
fieldsRelated = [keyFrom];
if (where[IdKey]) {
where = {
[keyFrom]: where[IdKey],
};
} else {
where = {};
}
}
const scoped = (this._scope && this._scope.where) || {};
const filter = mergeQuery({where: scoped}, {where: where || {}});
return targetModel.updateAll(filter.where, data, options, cb);
const filter = mergeQuery({where: scoped}, {where: where || {}, fields: fieldsRelated});
if (!scopeOnRelatedModel) {
targetModel.updateAll(filter.where, data, options, cb);
} else {
targetModel.find(filter, options, function(err, findData) {
const smartMergeSuccessful =
smartMergeRelatedModelQuery(findData, queryRelated.scope, keyFrom, IdKey);
if (!smartMergeSuccessful) {
return cb(null, {count: 0});
}
return relatedModel.updateAll(queryRelated.scope.where, data, options, cb);
});
}
return cb.promise;
}

function findById(id, filter, options, cb) {
Expand Down Expand Up @@ -500,10 +562,51 @@ function defineScope(cls, targetClass, name, params, methods, options) {
options = {};
}
options = options || {};
cb = cb || utils.createPromiseCallback();
const targetModel = definition.targetModel(this._receiver);
// If there is a through model
// run another query to apply filter on relatedModel(targetModel)
// see github.com/strongloop/loopback-datasource-juggler/issues/1795
let scopeOnRelatedModel = false;
let queryRelated, keyFrom, relatedModel, IdKey;
if (this._scope && this._scope.collect &&
filter && filter.where !== null && typeof filter.where === 'object') {
queryRelated = {
relation: this._scope.collect,
scope: filter,
};
scopeOnRelatedModel = true;
relatedModel = targetModel.relations[queryRelated.relation].modelTo;
IdKey = idName(relatedModel);
keyFrom = targetModel.relations[queryRelated.relation].keyFrom || IdKey;
if (filter.where[IdKey]) {
filter = {
fields: [keyFrom],
where: {
[keyFrom]: filter.where [IdKey],
},
};
} else {
filter = {
fields: [keyFrom],
};
}
}
const scoped = (this._scope && this._scope.where) || {};
filter = mergeQuery({where: scoped}, filter || {});
return targetModel.findOne(filter, options, cb);
if (!scopeOnRelatedModel) {
targetModel.findOne(filter, options, cb);
} else {
targetModel.find(filter, options, function(err, findData) {
const smartMergeSuccessful =
smartMergeRelatedModelQuery(findData, queryRelated.scope, keyFrom, IdKey);
if (!smartMergeSuccessful) {
return cb(null, null);
}
return relatedModel.findOne(queryRelated.scope, options, cb);
});
}
return cb.promise;
}

function count(where, options, cb) {
Expand All @@ -517,12 +620,91 @@ function defineScope(cls, targetClass, name, params, methods, options) {
options = {};
}
options = options || {};

cb = cb || utils.createPromiseCallback();
const targetModel = definition.targetModel(this._receiver);
// If there is a through model
// run another query to apply filter on relatedModel(targetModel)
// see github.com/strongloop/loopback-datasource-juggler/issues/1795
let scopeOnRelatedModel = false;
let queryRelated, keyFrom, relatedModel, IdKey, fieldsRelated;
if (this._scope && this._scope.collect &&
where !== null && typeof where === 'object') {
queryRelated = {
relation: this._scope.collect,
scope: {
where: where,
},
};
scopeOnRelatedModel = true;
relatedModel = targetModel.relations[queryRelated.relation].modelTo;
IdKey = idName(relatedModel);
keyFrom = targetModel.relations[queryRelated.relation].keyFrom || IdKey;
fieldsRelated = [keyFrom];
if (where[IdKey]) {
where = {
[keyFrom]: where[IdKey],
};
} else {
where = {};
}
}
const scoped = (this._scope && this._scope.where) || {};
const filter = mergeQuery({where: scoped}, {where: where || {}});
return targetModel.count(filter.where, options, cb);
const filter = mergeQuery({where: scoped}, {where: where || {}, fields: fieldsRelated});
if (!scopeOnRelatedModel) {
targetModel.count(filter.where, options, cb);
} else {
targetModel.find(filter, options, function(err, findData) {
const smartMergeSuccessful =
smartMergeRelatedModelQuery(findData, queryRelated.scope, keyFrom, IdKey);
if (!smartMergeSuccessful) {
return cb(null, 0);
}
return relatedModel.count(queryRelated.scope.where, options, cb);
});
}
return cb.promise;
}

return definition;
}

function smartMergeRelatedModelQuery(findData, queryRelated, keyFrom, IdKey) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add API documentation for this function.

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(findData, keyFrom),
queryRelated.where);

// if the id in filter doesn't exist after the merge,
// return empty result
if (_.isObject(IdKeyCondition[IdKey]) && _.isEmpty(IdKeyCondition[IdKey])) {
return false;
}

const mergedWhere = {
and: [
IdKeyCondition,
_.omit(queryRelated.where, IdKey),
],
};
queryRelated.where = mergedWhere;
} else {
queryRelated.where = {};
queryRelated.where[IdKey] = collectTargetIds(findData, keyFrom);
}
return true;
}
Loading