From 8aa50cd80c8317389aed694054be42e272e26431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Wed, 18 Mar 2020 17:15:24 +0100 Subject: [PATCH 1/4] Fix deep filtering for manyWay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Pierre Noël --- .../lib/buildQuery.js | 69 +++++---- .../__tests__/deepFiltering.test.e2e.js | 140 ++++++++++++++++++ 2 files changed, 180 insertions(+), 29 deletions(-) create mode 100644 packages/strapi/__tests__/deepFiltering.test.e2e.js diff --git a/packages/strapi-connector-bookshelf/lib/buildQuery.js b/packages/strapi-connector-bookshelf/lib/buildQuery.js index 115693b70a..b7c8c6e986 100644 --- a/packages/strapi-connector-bookshelf/lib/buildQuery.js +++ b/packages/strapi-connector-bookshelf/lib/buildQuery.js @@ -84,38 +84,52 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => { qb.leftJoin( `${originInfo.model.databaseName}.${assoc.tableCollectionName} AS ${joinTableAlias}`, - `${joinTableAlias}.${singular( - destinationInfo.model.attributes[assoc.via].attribute - )}_${destinationInfo.model.attributes[assoc.via].column}`, + `${joinTableAlias}.${singular(destinationInfo.model.attributes[assoc.via].attribute)}_${ + destinationInfo.model.attributes[assoc.via].column + }`, `${originInfo.alias}.${originInfo.model.primaryKey}` ); qb.leftJoin( `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, - `${joinTableAlias}.${singular( - originInfo.model.attributes[assoc.alias].attribute - )}_${originInfo.model.attributes[assoc.alias].column}`, + `${joinTableAlias}.${singular(originInfo.model.attributes[assoc.alias].attribute)}_${ + originInfo.model.attributes[assoc.alias].column + }`, `${destinationInfo.alias}.${destinationInfo.model.primaryKey}` ); - return; + } else if (assoc.nature === 'manyWay') { + const joinTableAlias = generateAlias(assoc.tableCollectionName); + + qb.leftJoin( + `${originInfo.model.databaseName}.${assoc.tableCollectionName} AS ${joinTableAlias}`, + `${joinTableAlias}.${singular(originInfo.alias)}_id`, + `${originInfo.alias}.${originInfo.model.primaryKey}` + ); + + qb.leftJoin( + `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, + `${joinTableAlias}.${singular(originInfo.model.attributes[assoc.alias].attribute)}_${ + originInfo.model.attributes[assoc.alias].column + }`, + `${destinationInfo.alias}.${destinationInfo.model.primaryKey}` + ); + } else { + const externalKey = + assoc.type === 'collection' + ? `${destinationInfo.alias}.${assoc.via || destinationInfo.model.primaryKey}` + : `${destinationInfo.alias}.${destinationInfo.model.primaryKey}`; + + const internalKey = + assoc.type === 'collection' + ? `${originInfo.alias}.${originInfo.model.primaryKey}` + : `${originInfo.alias}.${assoc.alias}`; + + qb.leftJoin( + `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, + externalKey, + internalKey + ); } - - const externalKey = - assoc.type === 'collection' - ? `${destinationInfo.alias}.${assoc.via || - destinationInfo.model.primaryKey}` - : `${destinationInfo.alias}.${destinationInfo.model.primaryKey}`; - - const internalKey = - assoc.type === 'collection' - ? `${originInfo.alias}.${originInfo.model.primaryKey}` - : `${originInfo.alias}.${assoc.alias}`; - - qb.leftJoin( - `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, - externalKey, - internalKey - ); }; /** @@ -208,9 +222,7 @@ const buildWhereClause = ({ qb, field, operator, value }) => { if (Array.isArray(value) && !['in', 'nin'].includes(operator)) { return qb.where(subQb => { for (let val of value) { - subQb.orWhere(q => - buildWhereClause({ qb: q, field, operator, value: val }) - ); + subQb.orWhere(q => buildWhereClause({ qb: q, field, operator, value: val })); } }); } @@ -258,7 +270,6 @@ const findModelByAssoc = assoc => { return models[assoc.collection || assoc.model]; }; -const findAssoc = (model, key) => - model.associations.find(assoc => assoc.alias === key); +const findAssoc = (model, key) => model.associations.find(assoc => assoc.alias === key); module.exports = buildQuery; diff --git a/packages/strapi/__tests__/deepFiltering.test.e2e.js b/packages/strapi/__tests__/deepFiltering.test.e2e.js new file mode 100644 index 0000000000..e320f761b0 --- /dev/null +++ b/packages/strapi/__tests__/deepFiltering.test.e2e.js @@ -0,0 +1,140 @@ +// Test an API with all the possible filed types and simple filterings (no deep filtering, no relations) + +const { registerAndLogin } = require('../../../test/helpers/auth'); +const createModelsUtils = require('../../../test/helpers/models'); +const { createAuthRequest } = require('../../../test/helpers/request'); + +let rq; +let modelsUtils; +let data = { + paniniCards: [], + collectors: [], +}; + +const paniniCard = { + name: 'paniniCard', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + }, +}; + +const collector = { + name: 'collector', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + panini_cards: { + nature: 'manyWay', + target: 'application::panini-card.panini-card', + unique: false, + }, + }, +}; + +const paniniCardFixtures = [ + { + name: 'Hugo LLORIS', + }, + { + name: 'Samuel UMTITI', + }, +]; + +async function createFixtures() { + for (let paniniCard of paniniCardFixtures) { + const res = await rq({ + method: 'POST', + url: '/panini-cards', + body: paniniCard, + }); + + data.paniniCards.push(res.body); + } + + const collector1Res = await rq({ + method: 'POST', + url: '/collectors', + body: { + name: 'Bernard', + panini_cards: [data.paniniCards[0].id, data.paniniCards[1].id], + }, + }); + data.collectors.push(collector1Res.body); + + const collector2Res = await rq({ + method: 'POST', + url: '/collectors', + body: { + name: 'Isabelle', + panini_cards: [data.paniniCards[0].id], + }, + }); + data.collectors.push(collector2Res.body); +} + +async function deleteFixtures() { + for (let paniniCard of data.paniniCards) { + await rq({ + method: 'DELETE', + url: `/panini-cards/${paniniCard.id}`, + }); + } + for (let collector of data.collectors) { + await rq({ + method: 'DELETE', + url: `/collectors/${collector.id}`, + }); + } +} + +describe('Deep Filtering API', () => { + beforeAll(async () => { + const token = await registerAndLogin(); + rq = createAuthRequest(token); + + modelsUtils = createModelsUtils({ rq }); + await modelsUtils.createContentTypes([paniniCard, collector]); + await createFixtures(); + }, 60000); + + afterAll(async () => { + await deleteFixtures(); + await modelsUtils.deleteContentTypes(['collector', 'panini-card']); + }, 60000); + + describe('Filter on a manyWay relation', () => { + test('Should return 2 results', async () => { + const res = await rq({ + method: 'GET', + url: '/collectors', + qs: { + 'panini_cards.name': data.paniniCards[0].name, + }, + }); + + expect(Array.isArray(res.body)).toBe(true); + expect(res.body.length).toBe(2); + expect(res.body[0]).toMatchObject(data.collectors[0]); + expect(res.body[1]).toMatchObject(data.collectors[1]); + }); + + test('Should return 1 result', async () => { + const res = await rq({ + method: 'GET', + url: '/collectors', + qs: { + 'panini_cards.name': data.paniniCards[1].name, + }, + }); + + expect(Array.isArray(res.body)).toBe(true); + expect(res.body.length).toBe(1); + expect(res.body[0]).toMatchObject(data.collectors[0]); + }); + }); +}); From 5ffe33c530ec96ef969625e4716b27d8ca5eef7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Fri, 20 Mar 2020 13:18:51 +0100 Subject: [PATCH 2/4] fix manyWay deep filtering when the relation is self MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Pierre Noël --- .../lib/buildQuery.js | 10 ++++---- .../__tests__/deepFiltering.test.e2e.js | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/strapi-connector-bookshelf/lib/buildQuery.js b/packages/strapi-connector-bookshelf/lib/buildQuery.js index b7c8c6e986..d7ebb1a8aa 100644 --- a/packages/strapi-connector-bookshelf/lib/buildQuery.js +++ b/packages/strapi-connector-bookshelf/lib/buildQuery.js @@ -99,18 +99,20 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => { ); } else if (assoc.nature === 'manyWay') { const joinTableAlias = generateAlias(assoc.tableCollectionName); + const isRelatedToSameTable = + destinationInfo.model.collectionName === originInfo.model.collectionName; qb.leftJoin( `${originInfo.model.databaseName}.${assoc.tableCollectionName} AS ${joinTableAlias}`, - `${joinTableAlias}.${singular(originInfo.alias)}_id`, + `${joinTableAlias}.${singular(originInfo.model.collectionName)}_id`, `${originInfo.alias}.${originInfo.model.primaryKey}` ); qb.leftJoin( `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, - `${joinTableAlias}.${singular(originInfo.model.attributes[assoc.alias].attribute)}_${ - originInfo.model.attributes[assoc.alias].column - }`, + `${joinTableAlias}.${isRelatedToSameTable ? 'related_' : ''}${singular( + originInfo.model.attributes[assoc.alias].attribute + )}_${originInfo.model.attributes[assoc.alias].column}`, `${destinationInfo.alias}.${destinationInfo.model.primaryKey}` ); } else { diff --git a/packages/strapi/__tests__/deepFiltering.test.e2e.js b/packages/strapi/__tests__/deepFiltering.test.e2e.js index e320f761b0..61c1f14a33 100644 --- a/packages/strapi/__tests__/deepFiltering.test.e2e.js +++ b/packages/strapi/__tests__/deepFiltering.test.e2e.js @@ -33,6 +33,11 @@ const collector = { target: 'application::panini-card.panini-card', unique: false, }, + collector_friends: { + nature: 'manyWay', + target: '__self__', + unique: false, + }, }, }; @@ -72,6 +77,7 @@ async function createFixtures() { body: { name: 'Isabelle', panini_cards: [data.paniniCards[0].id], + collector_friends: [data.collectors[0].id], }, }); data.collectors.push(collector2Res.body); @@ -137,4 +143,22 @@ describe('Deep Filtering API', () => { expect(res.body[0]).toMatchObject(data.collectors[0]); }); }); + + describe('Filter on a self manyWay relation', () => { + test('Should return 1 result', async () => { + const res = await rq({ + method: 'GET', + url: '/collectors', + qs: { + 'collector_friends.name': data.collectors[0].name, + }, + }); + + console.log('res', JSON.stringify(res.body, null, 2)); + + expect(Array.isArray(res.body)).toBe(true); + expect(res.body.length).toBe(1); + expect(res.body[0]).toMatchObject(data.collectors[1]); + }); + }); }); From ff1fbcdfba725130b8ce0cc8cec7ec4519b19863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Fri, 20 Mar 2020 16:44:25 +0100 Subject: [PATCH 3/4] small refacto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Pierre Noël --- .../lib/buildQuery.js | 35 +++++++------------ .../lib/mount-models.js | 1 + 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/strapi-connector-bookshelf/lib/buildQuery.js b/packages/strapi-connector-bookshelf/lib/buildQuery.js index d7ebb1a8aa..f8196418f2 100644 --- a/packages/strapi-connector-bookshelf/lib/buildQuery.js +++ b/packages/strapi-connector-bookshelf/lib/buildQuery.js @@ -79,14 +79,23 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => { * @param {Object} destinationInfo - destination with which we are making a join */ const buildJoin = (qb, assoc, originInfo, destinationInfo) => { - if (assoc.nature === 'manyToMany') { + if (['manyToMany', 'manyWay'].includes(assoc.nature)) { const joinTableAlias = generateAlias(assoc.tableCollectionName); + let originColumnNameInJoinTable = `${joinTableAlias}.`; + if (assoc.nature === 'manyToMany') { + originColumnNameInJoinTable += `${singular( + destinationInfo.model.attributes[assoc.via].attribute + )}_${destinationInfo.model.attributes[assoc.via].column}`; + } else if (assoc.nature === 'manyWay') { + originColumnNameInJoinTable += `${singular(originInfo.model.collectionName)}_${ + originInfo.model.primaryKey + }`; + } + qb.leftJoin( `${originInfo.model.databaseName}.${assoc.tableCollectionName} AS ${joinTableAlias}`, - `${joinTableAlias}.${singular(destinationInfo.model.attributes[assoc.via].attribute)}_${ - destinationInfo.model.attributes[assoc.via].column - }`, + originColumnNameInJoinTable, `${originInfo.alias}.${originInfo.model.primaryKey}` ); @@ -97,24 +106,6 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => { }`, `${destinationInfo.alias}.${destinationInfo.model.primaryKey}` ); - } else if (assoc.nature === 'manyWay') { - const joinTableAlias = generateAlias(assoc.tableCollectionName); - const isRelatedToSameTable = - destinationInfo.model.collectionName === originInfo.model.collectionName; - - qb.leftJoin( - `${originInfo.model.databaseName}.${assoc.tableCollectionName} AS ${joinTableAlias}`, - `${joinTableAlias}.${singular(originInfo.model.collectionName)}_id`, - `${originInfo.alias}.${originInfo.model.primaryKey}` - ); - - qb.leftJoin( - `${destinationInfo.model.databaseName}.${destinationInfo.model.collectionName} AS ${destinationInfo.alias}`, - `${joinTableAlias}.${isRelatedToSameTable ? 'related_' : ''}${singular( - originInfo.model.attributes[assoc.alias].attribute - )}_${originInfo.model.attributes[assoc.alias].column}`, - `${destinationInfo.alias}.${destinationInfo.model.primaryKey}` - ); } else { const externalKey = assoc.type === 'collection' diff --git a/packages/strapi-connector-bookshelf/lib/mount-models.js b/packages/strapi-connector-bookshelf/lib/mount-models.js index 06aa5c2e51..708b0b9285 100644 --- a/packages/strapi-connector-bookshelf/lib/mount-models.js +++ b/packages/strapi-connector-bookshelf/lib/mount-models.js @@ -236,6 +236,7 @@ module.exports = ({ models, target }, ctx) => { if (otherKey === foreignKey) { otherKey = `related_${otherKey}`; + details.attribute = `related_${details.attribute}`; } loadedModel[name] = function() { From 0c7029c2b1b61b587a193f46e0853b5330d250d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Mon, 23 Mar 2020 12:00:13 +0100 Subject: [PATCH 4/4] lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Pierre Noël --- packages/strapi-connector-bookshelf/lib/buildQuery.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/strapi-connector-bookshelf/lib/buildQuery.js b/packages/strapi-connector-bookshelf/lib/buildQuery.js index f8196418f2..a7d5fe2790 100644 --- a/packages/strapi-connector-bookshelf/lib/buildQuery.js +++ b/packages/strapi-connector-bookshelf/lib/buildQuery.js @@ -82,15 +82,15 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => { if (['manyToMany', 'manyWay'].includes(assoc.nature)) { const joinTableAlias = generateAlias(assoc.tableCollectionName); - let originColumnNameInJoinTable = `${joinTableAlias}.`; + let originColumnNameInJoinTable; if (assoc.nature === 'manyToMany') { - originColumnNameInJoinTable += `${singular( + originColumnNameInJoinTable = `${joinTableAlias}.${singular( destinationInfo.model.attributes[assoc.via].attribute )}_${destinationInfo.model.attributes[assoc.via].column}`; } else if (assoc.nature === 'manyWay') { - originColumnNameInJoinTable += `${singular(originInfo.model.collectionName)}_${ - originInfo.model.primaryKey - }`; + originColumnNameInJoinTable = `${joinTableAlias}.${singular( + originInfo.model.collectionName + )}_${originInfo.model.primaryKey}`; } qb.leftJoin(