From b8dc116ff69a91279ddae6288ada9f49f4a94e8b Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Thu, 28 Mar 2019 12:13:32 +0100 Subject: [PATCH] Fix relations and graphql nested lookups for both mongo and sql --- .../strapi-hook-bookshelf/lib/buildQuery.js | 3 +- .../strapi-hook-bookshelf/lib/relations.js | 327 ++++++++---------- .../strapi-hook-mongoose/lib/relations.js | 130 ++----- .../services/ContentManager.js | 3 +- .../strapi-plugin-graphql/services/Loaders.js | 1 + .../services/Resolvers.js | 4 +- .../test/graphqlRelations.test.e2e.js | 2 +- 7 files changed, 183 insertions(+), 287 deletions(-) diff --git a/packages/strapi-hook-bookshelf/lib/buildQuery.js b/packages/strapi-hook-bookshelf/lib/buildQuery.js index bf3bf6d526..1b1d1a1e55 100644 --- a/packages/strapi-hook-bookshelf/lib/buildQuery.js +++ b/packages/strapi-hook-bookshelf/lib/buildQuery.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const pluralize = require('pluralize'); const buildQuery = ({ model, filters }) => qb => { if (_.has(filters, 'where') && Array.isArray(filters.where)) { @@ -159,7 +160,7 @@ const buildSingleJoin = (qb, strapiModel, astModel, association) => { // Join on both ends qb.innerJoin( association.tableCollectionName, - `${association.tableCollectionName}.${strapiModel.info.name}_${strapiModel.primaryKey}`, + `${association.tableCollectionName}.${pluralize.singular(strapiModel.collectionName)}_${strapiModel.primaryKey}`, `${strapiModel.collectionName}.${strapiModel.primaryKey}` ); diff --git a/packages/strapi-hook-bookshelf/lib/relations.js b/packages/strapi-hook-bookshelf/lib/relations.js index 550c928b23..ab2d86af09 100644 --- a/packages/strapi-hook-bookshelf/lib/relations.js +++ b/packages/strapi-hook-bookshelf/lib/relations.js @@ -30,11 +30,13 @@ const transformToArrayID = (array, association) => { return []; }; -module.exports = { - getModel: function (model, plugin) { - return _.get(strapi.plugins, [plugin, 'models', model]) || _.get(strapi, ['models', model]) || undefined; - }, +const getModel = (model, plugin) => { + return _.get(strapi.plugins, [plugin, 'models', model]) || _.get(strapi, ['models', model]) || undefined; +}; +const removeUndefinedKeys = obj => _.pickBy(obj, _.negate(_.isUndefined)); + +module.exports = { findOne: async function (params, populate) { const record = await this .forge({ @@ -69,214 +71,177 @@ module.exports = { }, update: async function (params) { - const virtualFields = []; + const relationUpdates = []; + const primaryKeyValue = getValuePrimaryKey(params, this.primaryKey); const response = await module.exports.findOne.call(this, params); // Only update fields which are on this document. - const values = params.parseRelationships === false ? params.values : Object.keys(JSON.parse(JSON.stringify(params.values))).reduce((acc, current) => { + const values = params.parseRelationships === false ? params.values : Object.keys(removeUndefinedKeys(params.values)).reduce((acc, current) => { + const property = params.values[current]; const association = this.associations.filter(x => x.alias === current)[0]; const details = this._attributes[current]; - if (_.get(this._attributes, `${current}.isVirtual`) !== true && _.isUndefined(association)) { - acc[current] = params.values[current]; - } else { - switch (association.nature) { - case 'oneWay': - acc[current] = _.get(params.values[current], this.primaryKey, params.values[current]) || null; + if (!association && _.get(details, 'isVirtual') !== true) { + return _.set(acc, current, property); + } - break; - case 'oneToOne': - if (response[current] !== params.values[current]) { - const value = _.isNull(params.values[current]) ? response[current] : params.values; - const recordId = _.isNull(params.values[current]) ? getValuePrimaryKey(value, this.primaryKey) : value[current]; + const assocModel = getModel(details.model || details.collection, details.plugin); + switch (association.nature) { + case 'oneWay': { + return _.set(acc, current, _.get(property, assocModel.primaryKey, property)); + } + case 'oneToOne': { + if (response[current] === property) return acc; - const model = module.exports.getModel(details.collection || details.model, details.plugin); + if (_.isNull(property)) { + const updatePromise = assocModel.where({ + [assocModel.primaryKey]: getValuePrimaryKey(response[current], assocModel.primaryKey) + }).save({ [details.via]: null }, {method: 'update', patch: true, require: false}); - // Remove relation in the user side. - virtualFields.push( - module.exports.findOne - .call(model, { [model.primaryKey]: recordId }, [details.via]) - .then(record => { - if (record && _.isObject(record[details.via]) && record.id !== record[details.via][current]) { - return module.exports.update.call(this, { - id: getValuePrimaryKey(record[details.via], model.primaryKey), - values: { - [current]: null - }, - parseRelationships: false - }); - } + relationUpdates.push(updatePromise); + return _.set(acc, current, null); + } - return Promise.resolve(); - }) - .then(() => { - return module.exports.update.call(model, { - id: getValuePrimaryKey(response[current] || {}, this.primaryKey) || value[current], - values: { - [details.via]: null - }, - parseRelationships: false - }); - }) - .then(() => { - if (!_.isNull(params.values[current])) { - return module.exports.update.call(model, { - id: recordId, - values: { - [details.via]: getValuePrimaryKey(params, this.primaryKey) || null - }, - parseRelationships: false - }); - } - return Promise.resolve(); - }) - ); + // set old relations to null + const updateLink = this.where({ [current]: property }) + .save({ [current]: null }, {method: 'update', patch: true, require: false}) + .then(() => { + return assocModel + .where({ [this.primaryKey]: property }) + .save({ [details.via] : primaryKeyValue}, {method: 'update', patch: true, require: false}); + }); - acc[current] = _.isNull(params.values[current]) ? null : value[current]; - } + // set new relation + relationUpdates.push(updateLink); + return _.set(acc, current, property); + } + case 'oneToMany': { + // receive array of ids or array of objects with ids - break; - case 'oneToMany': - case 'manyToOne': - case 'manyToMany': - if (response[current] && _.isArray(response[current]) && current !== 'id') { - // Compare array of ID to find deleted files. - const currentValue = transformToArrayID(response[current], association).map(id => id.toString()); - const storedValue = transformToArrayID(params.values[current], association).map(id => id.toString()); + // set relation to null for all the ids not in the list + const currentIds = response[current]; + const diff = _.differenceWith(property, currentIds, (a, b) => { + return `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}`; + }); - const toAdd = _.difference(storedValue, currentValue); - const toRemove = _.difference(currentValue, storedValue); + const updatePromise = assocModel + .where(assocModel.primaryKey, 'in', currentIds.map(val => val[assocModel.primaryKey]||val)) + .save({ [details.via] : null }, { method: 'update', patch: true, require: false }) + .then(() => { + return assocModel + .where(assocModel.primaryKey, 'in', diff.map(val => val[assocModel.primaryKey]||val)) + .save({ [details.via] : primaryKeyValue }, { method: 'update', patch: true, require: false }); + }); - const model = module.exports.getModel(details.collection || details.model, details.plugin); + relationUpdates.push(updatePromise); + return acc; + } + case 'manyToOne': { + return _.set(acc, current, _.get(property, assocModel.primaryKey, property)); + } + case 'manyToMany': { + const currentValue = transformToArrayID(response[current], association).map(id => id.toString()); + const storedValue = transformToArrayID(params.values[current], association).map(id => id.toString()); - // Push the work into the flow process. - toAdd.forEach(value => { - value = _.isString(value) || _.isNumber(value) ? { [this.primaryKey]: value } : value; + const toAdd = _.difference(storedValue, currentValue); + const toRemove = _.difference(currentValue, storedValue); - value[details.via] = params.values[this.primaryKey] || params[this.primaryKey]; + const collection = this.forge({ [this.primaryKey]: primaryKeyValue })[association.alias](); + const updatePromise = collection + .detach(toRemove) + .then(() => collection.attach(toAdd)); - virtualFields.push( - module.exports.addRelation.call(model, { - id: getValuePrimaryKey(value, this.primaryKey), - values: value, - foreignKey: current - }) - ); - }); + relationUpdates.push(updatePromise); + return acc; + } + case 'manyMorphToMany': + case 'manyMorphToOne': + // Update the relational array. + params.values[current].forEach(obj => { + const model = obj.source && obj.source !== 'content-manager' ? + strapi.plugins[obj.source].models[obj.ref]: + strapi.models[obj.ref]; - toRemove.forEach(value => { - value = _.isString(value) || _.isNumber(value) ? { [this.primaryKey]: value } : value; - - value[details.via] = association.nature !== 'manyToMany' ? - null : - params.values[this.primaryKey] || params[this.primaryKey]; - - virtualFields.push( - module.exports.removeRelation.call(model, { - id: getValuePrimaryKey(value, this.primaryKey), - values: value, - foreignKey: current - }) - ); - }); - } else if (_.get(this._attributes, `${current}.isVirtual`) !== true) { - if (params.values[current] && typeof params.values[current] === 'object') { - acc[current] = _.get(params.values[current], this.primaryKey); - } else { - acc[current] = params.values[current]; - } - } - - break; - case 'manyMorphToMany': - case 'manyMorphToOne': - // Update the relational array. - params.values[current].forEach(obj => { - const model = obj.source && obj.source !== 'content-manager' ? - strapi.plugins[obj.source].models[obj.ref]: - strapi.models[obj.ref]; - - // Remove existing relationship because only one file - // can be related to this field. - if (association.nature === 'manyMorphToOne') { - virtualFields.push( - module.exports.removeRelationMorph.call(this, { - alias: association.alias, - ref: model.collectionName, - refId: obj.refId, - field: obj.field - }) - .then(() => - module.exports.addRelationMorph.call(this, { - id: response[this.primaryKey], - alias: association.alias, - ref: model.collectionName, - refId: obj.refId, - field: obj.field - }) - ) - ); - } else { - virtualFields.push(module.exports.addRelationMorph.call(this, { - id: response[this.primaryKey], + // Remove existing relationship because only one file + // can be related to this field. + if (association.nature === 'manyMorphToOne') { + relationUpdates.push( + module.exports.removeRelationMorph.call(this, { alias: association.alias, ref: model.collectionName, refId: obj.refId, field: obj.field - })); - } - }); - break; - case 'oneToManyMorph': - case 'manyToManyMorph': { - // Compare array of ID to find deleted files. - const currentValue = transformToArrayID(response[current], association).map(id => id.toString()); - const storedValue = transformToArrayID(params.values[current], association).map(id => id.toString()); - - const toAdd = _.difference(storedValue, currentValue); - const toRemove = _.difference(currentValue, storedValue); - - const model = module.exports.getModel(details.collection || details.model, details.plugin); - - toAdd.forEach(id => { - virtualFields.push( - module.exports.addRelationMorph.call(model, { - id, - alias: association.via, - ref: this.collectionName, - refId: response.id, - field: association.alias }) + .then(() => + module.exports.addRelationMorph.call(this, { + id: response[this.primaryKey], + alias: association.alias, + ref: model.collectionName, + refId: obj.refId, + field: obj.field + }) + ) ); - }); + } else { + relationUpdates.push(module.exports.addRelationMorph.call(this, { + id: response[this.primaryKey], + alias: association.alias, + ref: model.collectionName, + refId: obj.refId, + field: obj.field + })); + } + }); + break; + case 'oneToManyMorph': + case 'manyToManyMorph': { + // Compare array of ID to find deleted files. + const currentValue = transformToArrayID(response[current], association).map(id => id.toString()); + const storedValue = transformToArrayID(params.values[current], association).map(id => id.toString()); - // Update the relational array. - toRemove.forEach(id => { - virtualFields.push( - module.exports.removeRelationMorph.call(model, { - id, - alias: association.via, - ref: this.collectionName, - refId: response.id, - field: association.alias - }) - ); - }); - break; - } - case 'oneMorphToOne': - case 'oneMorphToMany': - break; - default: + const toAdd = _.difference(storedValue, currentValue); + const toRemove = _.difference(currentValue, storedValue); + + const model = getModel(details.collection || details.model, details.plugin); + + toAdd.forEach(id => { + relationUpdates.push( + module.exports.addRelationMorph.call(model, { + id, + alias: association.via, + ref: this.collectionName, + refId: response.id, + field: association.alias + }) + ); + }); + + // Update the relational array. + toRemove.forEach(id => { + relationUpdates.push( + module.exports.removeRelationMorph.call(model, { + id, + alias: association.via, + ref: this.collectionName, + refId: response.id, + field: association.alias + }) + ); + }); + break; } + case 'oneMorphToOne': + case 'oneMorphToMany': + break; + default: } return acc; }, {}); if (!_.isEmpty(values)) { - virtualFields.push( + relationUpdates.push( this .forge({ [this.primaryKey]: getValuePrimaryKey(params, this.primaryKey) @@ -286,11 +251,11 @@ module.exports = { }) ); } else { - virtualFields.push(Promise.resolve(_.assign(response, params.values))); + relationUpdates.push(Promise.resolve(_.assign(response, params.values))); } // Update virtuals fields. - await Promise.all(virtualFields); + await Promise.all(relationUpdates); return await this .forge({ diff --git a/packages/strapi-hook-mongoose/lib/relations.js b/packages/strapi-hook-mongoose/lib/relations.js index 9c28416f19..d0691d18c1 100644 --- a/packages/strapi-hook-mongoose/lib/relations.js +++ b/packages/strapi-hook-mongoose/lib/relations.js @@ -5,7 +5,6 @@ // Public node modules. const _ = require('lodash'); const mongoose = require('mongoose'); -const util = require('util'); // Utils const { models: { getValuePrimaryKey } } = require('strapi-utils'); @@ -14,23 +13,21 @@ const getModel = function (model, plugin) { return _.get(strapi.plugins, [plugin, 'models', model]) || _.get(strapi, ['models', model]) || undefined; }; +const removeUndefinedKeys = obj => _.pickBy(obj, _.negate(_.isUndefined)); + module.exports = { - - update: async function (params) { const relationUpdates = []; const populate = this.associations.map(x => x.alias).join(' '); const primaryKeyValue = getValuePrimaryKey(params, this.primaryKey); const response = await this - .findOne({ - [this.primaryKey]: primaryKeyValue - }) + .findOne({ [this.primaryKey]: primaryKeyValue }) .populate(populate) .lean(); // Only update fields which are on this document. - const values = params.parseRelationships === false ? params.values : Object.keys(params.values).reduce((acc, current) => { + const values = params.parseRelationships === false ? params.values : Object.keys(removeUndefinedKeys(params.values)).reduce((acc, current) => { const property = params.values[current]; const association = this.associations.find(x => x.alias === current); const details = this._attributes[current]; @@ -53,21 +50,17 @@ module.exports = { if (_.isNull(property)) { const updatePromise = assocModel.updateOne({ [assocModel.primaryKey]: getValuePrimaryKey(response[current], assocModel.primaryKey) - }, { [details.via]: null }) + }, { [details.via]: null }); - relationUpdates.push(updatePromise) + relationUpdates.push(updatePromise); return _.set(acc, current, null); } // set old relations to null - const updateLink = this.updateOne({ - [current]: new mongoose.Types.ObjectId(property) - }, { [current]: null }) - .then(() => { - return assocModel.updateOne({ - [this.primaryKey]: new mongoose.Types.ObjectId(property) - }, { [details.via] : primaryKeyValue}) - }) + const updateLink = this.updateOne({ [current]: new mongoose.Types.ObjectId(property) }, { [current]: null }) + .then(() => { + return assocModel.updateOne({ [this.primaryKey]: new mongoose.Types.ObjectId(property) }, { [details.via] : primaryKeyValue}); + }); // set new relation relationUpdates.push(updateLink); @@ -79,30 +72,29 @@ module.exports = { // set relation to null for all the ids not in the list const currentIds = response[current]; const diff = _.differenceWith(property, currentIds, (a, b) => { - `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}` - }) + return `${a[assocModel.primaryKey] || a}` === `${b[assocModel.primaryKey] || b}`; + }); const updatePromise = assocModel.updateMany({ [assocModel.primaryKey]: { $in: currentIds.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey]||val)) } }, { [details.via] : null }) - .then(() => { - return assocModel.updateMany({ - [assocModel.primaryKey]: { - $in: diff.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey]||val)) - } - }, { [details.via] : primaryKeyValue }) - }) + .then(() => { + return assocModel.updateMany({ + [assocModel.primaryKey]: { + $in: diff.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey]||val)) + } + }, { [details.via] : primaryKeyValue }); + }); - relationUpdates.push(updatePromise) + relationUpdates.push(updatePromise); return acc; } case 'manyToOne': { return _.set(acc, current, _.get(property, assocModel.primaryKey, property)); } case 'manyToMany': { - if (details.dominant) { return _.set(acc, current, property.map(val => val[assocModel.primaryKey] || val)); } @@ -114,82 +106,18 @@ module.exports = { }, { $pull: { [association.via]: new mongoose.Types.ObjectId(primaryKeyValue) } }) - .then(() => { - return assocModel.updateMany({ - [assocModel.primaryKey]: { - $in: property.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey] || val)) - } - }, { - $addToSet: { [association.via]: [primaryKeyValue] } - }) - }) + .then(() => { + return assocModel.updateMany({ + [assocModel.primaryKey]: { + $in: property.map(val => new mongoose.Types.ObjectId(val[assocModel.primaryKey] || val)) + } + }, { + $addToSet: { [association.via]: [primaryKeyValue] } + }); + }); relationUpdates.push(updatePomise); return acc; - - - // TODO: handle concat or remove from current - if (association.nature === 'manyToMany' && details.dominant === true) { - return _.set(acc, current, property); - } - - if (response[current] && _.isArray(response[current]) && current !== 'id') { - // Records to add in the relation. - const toAdd = _.differenceWith(property, response[current], (a, b) => - (a[this.primaryKey] || a).toString() === (b[this.primaryKey] || b).toString() - ); - - // Records to remove in the relation. - const toRemove = _.differenceWith(response[current], property, (a, b) => - (a[this.primaryKey] || a).toString() === (b[this.primaryKey] || b).toString() - ) - .filter(x => toAdd.find(y => x.id === y.id) === undefined); - - const model = getModel(details.model || details.collection, details.plugin); - - // Push the work into the flow process. - toAdd.forEach(value => { - value = _.isString(value) ? { [this.primaryKey]: value } : value; - - if (association.nature === 'manyToMany' && !_.isArray(params.values[this.primaryKey] || params[this.primaryKey])) { - value[details.via] = (value[details.via] || []) - .concat([(params.values[this.primaryKey] || params[this.primaryKey])]) - .filter(x => { - return x !== null && x !== undefined; - }); - } else { - value[details.via] = getValuePrimaryKey(params, this.primaryKey); - } - - relationUpdates.push( - module.exports.addRelation.call(model, { - id: getValuePrimaryKey(value, this.primaryKey), - values: _.pick(value, [this.primaryKey, details.via]), - foreignKey: current - }) - ); - }); - - toRemove.forEach(value => { - value = _.isString(value) ? { [this.primaryKey]: value } : value; - - if (association.nature === 'manyToMany' && !_.isArray(params.values[this.primaryKey] || params[this.primaryKey])) { - value[details.via] = value[details.via].filter(x => _.toString(x) !== _.toString(params.values[this.primaryKey] || params[this.primaryKey])); - } else { - value[details.via] = null; - } - - relationUpdates.push( - module.exports.removeRelation.call(model, { - id: getValuePrimaryKey(value, this.primaryKey), - values: _.pick(value, [this.primaryKey, details.via]), - foreignKey: current - }) - ); - }); - - return acc; - } } case 'manyMorphToMany': case 'manyMorphToOne': diff --git a/packages/strapi-plugin-content-manager/services/ContentManager.js b/packages/strapi-plugin-content-manager/services/ContentManager.js index a4f82078b6..929f2b0c38 100644 --- a/packages/strapi-plugin-content-manager/services/ContentManager.js +++ b/packages/strapi-plugin-content-manager/services/ContentManager.js @@ -162,7 +162,8 @@ module.exports = { } params[primaryKey] = response[primaryKey]; - params.values = Object.keys(JSON.parse(JSON.stringify(response))).reduce((acc, current) => { + + params.values = Object.keys(response).reduce((acc, current) => { const association = (strapi.models[params.model] || strapi.plugins[source].models[params.model]).associations.filter(x => x.alias === current)[0]; // Remove relationships. diff --git a/packages/strapi-plugin-graphql/services/Loaders.js b/packages/strapi-plugin-graphql/services/Loaders.js index bfbb4ca9e9..6d85066631 100644 --- a/packages/strapi-plugin-graphql/services/Loaders.js +++ b/packages/strapi-plugin-graphql/services/Loaders.js @@ -58,6 +58,7 @@ module.exports = { const { queries, map } = this.extractQueries(model, _.cloneDeep(keys)); // Run queries in parallel. const results = await Promise.all(queries.map(query => this.makeQuery(model, query))); + // Use to match initial queries order. const data = this.mapData(model, keys, map, results); diff --git a/packages/strapi-plugin-graphql/services/Resolvers.js b/packages/strapi-plugin-graphql/services/Resolvers.js index 40e9251b6e..8cdb7f977e 100644 --- a/packages/strapi-plugin-graphql/services/Resolvers.js +++ b/packages/strapi-plugin-graphql/services/Resolvers.js @@ -369,8 +369,8 @@ const buildShadowCRUD = (models, plugin) => { ...Query.convertToQuery(queryParams.where), }; - if (association.nature === 'manyToMany' && association.dominant) { - _.set(queryOpts, ['query', ref.primaryKey], obj[association.alias] || []); + if (model.orm === 'mongoose' && association.nature === 'manyToMany' && association.dominant) { + _.set(queryOpts, ['query', ref.primaryKey], obj[association.alias].map(val => val[ref.primaryKey] || val) || []); } _.set(queryOpts, ['query', association.via], obj[ref.primaryKey]); diff --git a/packages/strapi-plugin-graphql/test/graphqlRelations.test.e2e.js b/packages/strapi-plugin-graphql/test/graphqlRelations.test.e2e.js index 8334b89f59..2959ea3d70 100644 --- a/packages/strapi-plugin-graphql/test/graphqlRelations.test.e2e.js +++ b/packages/strapi-plugin-graphql/test/graphqlRelations.test.e2e.js @@ -8,7 +8,7 @@ let rq; let graphqlQuery; // utils -const selectFields = doc => _.pick(doc[('id', 'name')]); +const selectFields = doc => _.pick(doc, ['id', 'name']); const documentModel = { attributes: [