From 07ae49badd63340b3e5fbcb9aba5e7a6fc300a61 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Fri, 6 Aug 2021 10:51:34 +0200 Subject: [PATCH] Avoid making empty queries in populate --- .../server/controllers/authentication.js | 2 +- .../core/admin/server/services/metrics.js | 4 +- packages/core/admin/server/services/user.js | 2 +- .../controllers/collection-types.js | 7 +- packages/core/database/lib/query/helpers.js | 105 ++++++++++-------- .../core/database/lib/query/query-builder.js | 14 +-- packages/core/utils/lib/relations.js | 2 +- 7 files changed, 77 insertions(+), 59 deletions(-) diff --git a/packages/core/admin/server/controllers/authentication.js b/packages/core/admin/server/controllers/authentication.js index a840224b0e..a04dbccc1d 100644 --- a/packages/core/admin/server/controllers/authentication.js +++ b/packages/core/admin/server/controllers/authentication.js @@ -135,7 +135,7 @@ module.exports = { roles: superAdminRole ? [superAdminRole.id] : [], }); - await strapi.telemetry.send('didCreateFirstAdmin'); + strapi.telemetry.send('didCreateFirstAdmin'); ctx.body = { data: { diff --git a/packages/core/admin/server/services/metrics.js b/packages/core/admin/server/services/metrics.js index d58255e19e..38bcf4fb96 100644 --- a/packages/core/admin/server/services/metrics.js +++ b/packages/core/admin/server/services/metrics.js @@ -5,11 +5,11 @@ const { getService } = require('../utils'); const sendDidInviteUser = async () => { const numberOfUsers = await getService('user').count(); const numberOfRoles = await getService('role').count(); - return strapi.telemetry.send('didInviteUser', { numberOfRoles, numberOfUsers }); + strapi.telemetry.send('didInviteUser', { numberOfRoles, numberOfUsers }); }; const sendDidUpdateRolePermissions = async () => { - return strapi.telemetry.send('didUpdateRolePermissions'); + strapi.telemetry.send('didUpdateRolePermissions'); }; module.exports = { diff --git a/packages/core/admin/server/services/user.js b/packages/core/admin/server/services/user.js index a996c5defc..a9baea855e 100644 --- a/packages/core/admin/server/services/user.js +++ b/packages/core/admin/server/services/user.js @@ -41,7 +41,7 @@ const create = async attributes => { .query('strapi::user') .create({ data: user, populate: ['roles'] }); - await getService('metrics').sendDidInviteUser(); + getService('metrics').sendDidInviteUser(); return createdUser; }; diff --git a/packages/core/content-manager/controllers/collection-types.js b/packages/core/content-manager/controllers/collection-types.js index 094de2548b..e9dc9aff46 100644 --- a/packages/core/content-manager/controllers/collection-types.js +++ b/packages/core/content-manager/controllers/collection-types.js @@ -62,6 +62,8 @@ module.exports = { const { model } = ctx.params; const { body } = ctx.request; + const totalEntries = await strapi.query(model).count(); + const entityManager = getService('entity-manager'); const permissionChecker = getService('permission-checker').create({ userAbility, model }); @@ -77,9 +79,12 @@ module.exports = { await wrapBadRequest(async () => { const entity = await entityManager.create(sanitizeFn(body), model); + ctx.body = permissionChecker.sanitizeOutput(entity); - await strapi.telemetry.send('didCreateFirstContentTypeEntry', { model }); + if (totalEntries === 0) { + strapi.telemetry.send('didCreateFirstContentTypeEntry', { model }); + } })(); }, diff --git a/packages/core/database/lib/query/helpers.js b/packages/core/database/lib/query/helpers.js index 162f5b1892..23b28b8f01 100644 --- a/packages/core/database/lib/query/helpers.js +++ b/packages/core/database/lib/query/helpers.js @@ -516,6 +516,10 @@ const applyPopulate = async (results, populate, ctx) => { const { db, uid, qb } = ctx; const meta = db.metadata.get(uid); + if (_.isEmpty(results)) { + return results; + } + for (const key in populate) { // NOTE: Omit limit & offset to avoid needing a query per result to avoid making too many queries const populateValue = _.pick( @@ -579,34 +583,16 @@ const applyPopulate = async (results, populate, ctx) => { } = joinTable.joinColumn; const alias = qb.getAlias(); + const joinColAlias = `${alias}.${joinColumnName}`; - if (isCount) { - const rows = await qb - .init(populateValue) - .join({ - alias: alias, - referencedTable: joinTable.name, - referencedColumn: joinTable.inverseJoinColumn.name, - rootColumn: joinTable.inverseJoinColumn.referencedColumn, - rootTable: qb.alias, - on: joinTable.on, - }) - .select([`${alias}.${joinColumnName}`, qb.raw('count(*) AS count')]) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) - .groupBy(`${alias}.${joinColumnName}`) - .execute({ mapResults: false }); - - const map = rows.reduce((map, row) => { - map[row[joinColumnName]] = { count: row.count }; - return map; - }, {}); + const referencedValues = _.uniq( + results.map(r => r[referencedColumnName]).filter(value => !_.isNil(value)) + ); + if (_.isEmpty(referencedValues)) { results.forEach(result => { - result[key] = map[result[referencedColumnName]] || { count: 0 }; + result[key] = null; }); - continue; } @@ -620,10 +606,8 @@ const applyPopulate = async (results, populate, ctx) => { rootTable: qb.alias, on: joinTable.on, }) - .addSelect(`${alias}.${joinColumnName}`) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) + .addSelect(joinColAlias) + .where({ [joinColAlias]: referencedValues }) .execute({ mapResults: false }); const map = _.groupBy(joinColumnName, rows); @@ -681,8 +665,20 @@ const applyPopulate = async (results, populate, ctx) => { } = joinTable.joinColumn; const alias = qb.getAlias(); + const joinColAlias = `${alias}.${joinColumnName}`; + + const referencedValues = _.uniq( + results.map(r => r[referencedColumnName]).filter(value => !_.isNil(value)) + ); if (isCount) { + if (_.isEmpty(referencedValues)) { + results.forEach(result => { + result[key] = { count: 0 }; + }); + continue; + } + const rows = await qb .init(populateValue) .join({ @@ -693,11 +689,9 @@ const applyPopulate = async (results, populate, ctx) => { rootTable: qb.alias, on: joinTable.on, }) - .select([`${alias}.${joinColumnName}`, qb.raw('count(*) AS count')]) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) - .groupBy(`${alias}.${joinColumnName}`) + .select([joinColAlias, qb.raw('count(*) AS count')]) + .where({ [joinColAlias]: referencedValues }) + .groupBy(joinColAlias) .execute({ mapResults: false }); const map = rows.reduce((map, row) => { @@ -712,6 +706,13 @@ const applyPopulate = async (results, populate, ctx) => { continue; } + if (_.isEmpty(referencedValues)) { + results.forEach(result => { + result[key] = []; + }); + continue; + } + const rows = await qb .init(populateValue) .join({ @@ -722,10 +723,8 @@ const applyPopulate = async (results, populate, ctx) => { rootTable: qb.alias, on: joinTable.on, }) - .addSelect(`${alias}.${joinColumnName}`) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) + .addSelect(joinColAlias) + .where({ [joinColAlias]: referencedValues }) .execute({ mapResults: false }); const map = _.groupBy(joinColumnName, rows); @@ -745,8 +744,19 @@ const applyPopulate = async (results, populate, ctx) => { const { name: joinColumnName, referencedColumn: referencedColumnName } = joinTable.joinColumn; const alias = qb.getAlias(); + const joinColAlias = `${alias}.${joinColumnName}`; + const referencedValues = _.uniq( + results.map(r => r[referencedColumnName]).filter(value => !_.isNil(value)) + ); if (isCount) { + if (_.isEmpty(referencedValues)) { + results.forEach(result => { + result[key] = { count: 0 }; + }); + continue; + } + const rows = await qb .init(populateValue) .join({ @@ -757,11 +767,9 @@ const applyPopulate = async (results, populate, ctx) => { rootTable: qb.alias, on: joinTable.on, }) - .select([`${alias}.${joinColumnName}`, qb.raw('count(*) AS count')]) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) - .groupBy(`${alias}.${joinColumnName}`) + .select([joinColAlias, qb.raw('count(*) AS count')]) + .where({ [joinColAlias]: referencedValues }) + .groupBy(joinColAlias) .execute({ mapResults: false }); const map = rows.reduce((map, row) => { @@ -776,6 +784,13 @@ const applyPopulate = async (results, populate, ctx) => { continue; } + if (_.isEmpty(referencedValues)) { + results.forEach(result => { + result[key] = []; + }); + continue; + } + const rows = await qb .init(populateValue) .join({ @@ -786,10 +801,8 @@ const applyPopulate = async (results, populate, ctx) => { rootTable: qb.alias, on: joinTable.on, }) - .addSelect(`${alias}.${joinColumnName}`) - .where({ - [`${alias}.${joinColumnName}`]: results.map(r => r[referencedColumnName]), - }) + .addSelect(joinColAlias) + .where({ [joinColAlias]: referencedValues }) .execute({ mapResults: false }); const map = _.groupBy(joinColumnName, rows); diff --git a/packages/core/database/lib/query/query-builder.js b/packages/core/database/lib/query/query-builder.js index 87776e49a5..1fe6bcfb3b 100644 --- a/packages/core/database/lib/query/query-builder.js +++ b/packages/core/database/lib/query/query-builder.js @@ -30,6 +30,13 @@ const createQueryBuilder = (uid, db) => { alias: getAlias(), getAlias, + select(args) { + state.type = 'select'; + state.select = _.castArray(args).map(col => this.aliasColumn(col)); + + return this; + }, + insert(data) { state.type = 'insert'; state.data = data; @@ -65,13 +72,6 @@ const createQueryBuilder = (uid, db) => { return this; }, - select(args) { - state.type = 'select'; - state.select = _.castArray(args).map(col => this.aliasColumn(col)); - - return this; - }, - addSelect(args) { state.select.push(..._.castArray(args).map(col => this.aliasColumn(col))); return this; diff --git a/packages/core/utils/lib/relations.js b/packages/core/utils/lib/relations.js index 1e6b3e213b..0eef9fefba 100644 --- a/packages/core/utils/lib/relations.js +++ b/packages/core/utils/lib/relations.js @@ -1,6 +1,6 @@ 'use strict'; -const MANY_RELATIONS = ['oneToMany', 'manyToMany', 'manyWay']; +const MANY_RELATIONS = ['oneToMany', 'manyToMany']; const getRelationalFields = contentType => { return Object.keys(contentType.attributes).filter(attributeName => {