From 27bd19bb0d06f2e92c7bac45f7e8338034566ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Thu, 19 May 2022 14:47:23 +0200 Subject: [PATCH 1/3] remove pagination on GET /folders --- packages/core/upload/tests/admin/folder-structure.test.e2e.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/upload/tests/admin/folder-structure.test.e2e.js b/packages/core/upload/tests/admin/folder-structure.test.e2e.js index cd41186671..3ab8e23924 100644 --- a/packages/core/upload/tests/admin/folder-structure.test.e2e.js +++ b/packages/core/upload/tests/admin/folder-structure.test.e2e.js @@ -29,7 +29,7 @@ describe('Folder structure', () => { // delete all possibly existing folders const res = await rq({ method: 'GET', - url: '/upload/folders?pagination[pageSize]=-1', + url: '/upload/folders', }); await rq({ From cd69fca6f9ce1aafddb7a2b1b185fcfaf2ef8d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Wed, 18 May 2022 19:00:43 +0200 Subject: [PATCH 2/3] add transaction for folder update --- packages/core/database/lib/index.js | 4 + .../core/database/lib/query/query-builder.js | 20 +++ .../core/upload/server/services/folder.js | 124 ++++++++++++------ 3 files changed, 110 insertions(+), 38 deletions(-) diff --git a/packages/core/database/lib/index.js b/packages/core/database/lib/index.js index 12104f51ee..253fc389ee 100644 --- a/packages/core/database/lib/index.js +++ b/packages/core/database/lib/index.js @@ -58,6 +58,10 @@ class Database { return schema ? trx.schema.withSchema(schema) : trx.schema; } + transaction() { + return this.connection.transaction(); + } + queryBuilder(uid) { return this.entityManager.createQueryBuilder(uid); } diff --git a/packages/core/database/lib/query/query-builder.js b/packages/core/database/lib/query/query-builder.js index de39d962c7..bdec9fe00b 100644 --- a/packages/core/database/lib/query/query-builder.js +++ b/packages/core/database/lib/query/query-builder.js @@ -19,6 +19,8 @@ const createQueryBuilder = (uid, db) => { populate: null, limit: null, offset: null, + transaction: null, + forUpdate: false, orderBy: [], groupBy: [], }; @@ -115,6 +117,16 @@ const createQueryBuilder = (uid, db) => { return this; }, + transacting(transaction) { + state.transaction = transaction; + return this; + }, + + forUpdate() { + state.forUpdate = true; + return this; + }, + init(params = {}) { const { _q, filters, where, select, limit, offset, orderBy, groupBy, populate } = params; @@ -308,6 +320,14 @@ const createQueryBuilder = (uid, db) => { } } + if (state.transaction) { + qb.transacting(state.transaction); + } + + if (state.forUpdate) { + qb.forUpdate(); + } + if (state.limit) { qb.limit(state.limit); } diff --git a/packages/core/upload/server/services/folder.js b/packages/core/upload/server/services/folder.js index 7f3addffdf..eb8ed95c1a 100644 --- a/packages/core/upload/server/services/folder.js +++ b/packages/core/upload/server/services/folder.js @@ -74,53 +74,101 @@ const deleteByIds = async (ids = []) => { * @returns {Promise} */ const update = async (id, { name, parent }, { user }) => { - const existingFolder = await strapi.entityService.findOne(FOLDER_MODEL_UID, id); + // only name is updated + if (isUndefined(parent)) { + const existingFolder = await strapi.entityService.findOne(FOLDER_MODEL_UID, id); - if (!existingFolder) { - return undefined; - } + if (!existingFolder) { + return undefined; + } - if (!isUndefined(parent)) { - const folderPathColumnName = strapi.db.metadata.get(FILE_MODEL_UID).attributes.folderPath - .columnName; - const pathColumnName = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.path.columnName; + const newFolder = setCreatorFields({ user, isEdition: true })({ name, parent }); - // Todo wrap into a transaction - const destinationFolder = - parent === null ? '/' : (await strapi.entityService.findOne(FOLDER_MODEL_UID, parent)).path; + if (isUndefined(parent)) { + const folder = await strapi.entityService.update(FOLDER_MODEL_UID, id, { data: newFolder }); + return folder; + } + // location is updated => using transaction + } else { + const trx = await strapi.db.transaction(); + try { + // fetch existing folder + const existingFolder = await strapi.db + .queryBuilder(FOLDER_MODEL_UID) + .select(['uid', 'path']) + .where({ id }) + .transacting(trx) + .forUpdate() + .first() + .execute(); - const folderTable = strapi.getModel(FOLDER_MODEL_UID).collectionName; - const fileTable = strapi.getModel(FILE_MODEL_UID).collectionName; + // update parent folder + const joinTable = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.parent.joinTable; + await strapi.db + .queryBuilder(joinTable.name) + .transacting(trx) + .update({ [joinTable.inverseJoinColumn.name]: parent }) + .where({ [joinTable.joinColumn.name]: id }) + .execute(); - await strapi.db - .connection(folderTable) - .where(pathColumnName, 'like', `${existingFolder.path}%`) - .update( - pathColumnName, - strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + // fetch destinationFolder path + let destinationFolderPath = '/'; + if (parent !== null) { + const destinationFolder = await strapi.db + .queryBuilder(FOLDER_MODEL_UID) + .select('path') + .where({ id: parent }) + .transacting(trx) + .first() + .execute(); + destinationFolderPath = destinationFolder.path; + } + + const folderTable = strapi.getModel(FOLDER_MODEL_UID).collectionName; + const fileTable = strapi.getModel(FILE_MODEL_UID).collectionName; + const folderPathColumnName = strapi.db.metadata.get(FILE_MODEL_UID).attributes.folderPath + .columnName; + const pathColumnName = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.path.columnName; + + // update folders below + await strapi.db + .connection(folderTable) + .transacting(trx) + .where(pathColumnName, 'like', `${existingFolder.path}%`) + .update( pathColumnName, - existingFolder.path, - joinBy('/', destinationFolder, existingFolder.uid), - ]) - ); + strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + pathColumnName, + existingFolder.path, + joinBy('/', destinationFolderPath, existingFolder.uid), + ]) + ); - await strapi.db - .connection(fileTable) - .where(folderPathColumnName, 'like', `${existingFolder.path}%`) - .update( - folderPathColumnName, - strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + // update files below + await strapi.db + .connection(fileTable) + .transacting(trx) + .where(folderPathColumnName, 'like', `${existingFolder.path}%`) + .update( folderPathColumnName, - existingFolder.path, - joinBy('/', destinationFolder, existingFolder.uid), - ]) - ); + strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + folderPathColumnName, + existingFolder.path, + joinBy('/', destinationFolderPath, existingFolder.uid), + ]) + ); + + await trx.commit(); + } catch (e) { + await trx.rollback(); + throw e; + } + + // update less critical information (name + updatedBy) + const newFolder = setCreatorFields({ user, isEdition: true })({ name }); + const folder = await strapi.entityService.update(FOLDER_MODEL_UID, id, { data: newFolder }); + return folder; } - - const newFolder = setCreatorFields({ user, isEdition: true })({ name, parent }); - const folder = await strapi.entityService.update(FOLDER_MODEL_UID, id, { data: newFolder }); - - return folder; }; /** From 71f43f0f7ad155f359d15ec063322ed3c933f95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Fri, 20 May 2022 19:25:46 +0200 Subject: [PATCH 3/3] use a transaction for bulk move --- .../server/controllers/admin-folder-file.js | 146 ++++++++++++++++-- .../core/upload/server/services/folder.js | 10 +- 2 files changed, 137 insertions(+), 19 deletions(-) diff --git a/packages/core/upload/server/controllers/admin-folder-file.js b/packages/core/upload/server/controllers/admin-folder-file.js index 30c3e6c34b..7261f1e0d9 100644 --- a/packages/core/upload/server/controllers/admin-folder-file.js +++ b/packages/core/upload/server/controllers/admin-folder-file.js @@ -1,5 +1,6 @@ 'use strict'; +const { joinBy } = require('@strapi/utils'); const { getService } = require('../utils'); const { ACTIONS, FOLDER_MODEL_UID, FILE_MODEL_UID } = require('../constants'); const { @@ -43,7 +44,7 @@ module.exports = { async moveMany(ctx) { const { body } = ctx.request; const { - state: { userAbility, user }, + state: { userAbility }, } = ctx; const pmFolder = strapi.admin.services.permission.createPermissionsManager({ @@ -60,25 +61,136 @@ module.exports = { await validateMoveManyFoldersFiles(body); const { folderIds = [], fileIds = [], destinationFolderId } = body; - const uploadService = getService('upload'); - const folderService = getService('folder'); + const trx = await strapi.db.transaction(); + try { + // fetch folders + const existingFolders = await strapi.db + .queryBuilder(FOLDER_MODEL_UID) + .select(['id', 'uid', 'path']) + .where({ id: { $in: folderIds } }) + .transacting(trx) + .forUpdate() + .execute(); - const updatedFolders = []; - // updates are done in order (not in parallele) to avoid mixing queries (path) - for (let folderId of folderIds) { - const updatedFolder = await folderService.update( - folderId, - { parent: destinationFolderId }, - { user } - ); - updatedFolders.push(updatedFolder); + // fetch files + const existingFiles = await strapi.db + .queryBuilder(FILE_MODEL_UID) + .select(['id']) + .where({ id: { $in: fileIds } }) + .transacting(trx) + .forUpdate() + .execute(); + + // fetch destinationFolder path + let destinationFolderPath = '/'; + if (destinationFolderId !== null) { + const destinationFolder = await strapi.db + .queryBuilder(FOLDER_MODEL_UID) + .select('path') + .where({ id: destinationFolderId }) + .transacting(trx) + .first() + .execute(); + destinationFolderPath = destinationFolder.path; + } + + const fileTable = strapi.getModel(FILE_MODEL_UID).collectionName; + const folderTable = strapi.getModel(FOLDER_MODEL_UID).collectionName; + const folderPathColName = strapi.db.metadata.get(FILE_MODEL_UID).attributes.folderPath + .columnName; + const pathColName = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.path.columnName; + + if (existingFolders.length > 0) { + // update folders' parent relation (delete + insert; upsert not possible) + const joinTable = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.parent.joinTable; + await strapi.db + .queryBuilder(joinTable.name) + .transacting(trx) + .delete() + .where({ [joinTable.joinColumn.name]: { $in: folderIds } }) + .execute(); + await strapi.db + .queryBuilder(joinTable.name) + .transacting(trx) + .insert( + existingFolders.map(folder => ({ + [joinTable.inverseJoinColumn.name]: destinationFolderId, + [joinTable.joinColumn.name]: folder.id, + })) + ) + .execute(); + + for (const existingFolder of existingFolders) { + // update path for folders themselves & folders below + await strapi.db + .connection(folderTable) + .transacting(trx) + .where(pathColName, 'like', `${existingFolder.path}%`) + .update( + pathColName, + strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + pathColName, + existingFolder.path, + joinBy('/', destinationFolderPath, existingFolder.uid), + ]) + ); + + // update path of files below + await strapi.db + .connection(fileTable) + .transacting(trx) + .where(folderPathColName, 'like', `${existingFolder.path}%`) + .update( + folderPathColName, + strapi.db.connection.raw('REPLACE(??, ?, ?)', [ + folderPathColName, + existingFolder.path, + joinBy('/', destinationFolderPath, existingFolder.uid), + ]) + ); + } + } + + if (existingFiles.length > 0) { + // update files' folder relation (delete + insert; upsert not possible) + const fileJoinTable = strapi.db.metadata.get(FILE_MODEL_UID).attributes.folder.joinTable; + await strapi.db + .queryBuilder(fileJoinTable.name) + .transacting(trx) + .delete() + .where({ [fileJoinTable.joinColumn.name]: { $in: fileIds } }) + .execute(); + await strapi.db + .queryBuilder(fileJoinTable.name) + .transacting(trx) + .insert( + existingFiles.map(file => ({ + [fileJoinTable.inverseJoinColumn.name]: destinationFolderId, + [fileJoinTable.joinColumn.name]: file.id, + })) + ) + .execute(); + + // update files main fields (path + updatedBy) + await strapi.db + .connection(fileTable) + .transacting(trx) + .whereIn('id', fileIds) + .update(folderPathColName, destinationFolderPath); + } + + await trx.commit(); + } catch (e) { + await trx.rollback(); + throw e; } - const updatedFiles = await Promise.all( - fileIds.map(fileId => - uploadService.updateFileInfo(fileId, { folder: destinationFolderId }, { user }) - ) - ); + const updatedFolders = await strapi.entityService.findMany(FOLDER_MODEL_UID, { + filters: { id: { $in: folderIds } }, + }); + const updatedFiles = await strapi.entityService.findMany(FILE_MODEL_UID, { + filters: { id: { $in: fileIds } }, + }); ctx.body = { data: { diff --git a/packages/core/upload/server/services/folder.js b/packages/core/upload/server/services/folder.js index eb8ed95c1a..757b4f0c9f 100644 --- a/packages/core/upload/server/services/folder.js +++ b/packages/core/upload/server/services/folder.js @@ -102,12 +102,18 @@ const update = async (id, { name, parent }, { user }) => { .first() .execute(); - // update parent folder + // update parent folder (delete + insert; upsert not possible) const joinTable = strapi.db.metadata.get(FOLDER_MODEL_UID).attributes.parent.joinTable; await strapi.db .queryBuilder(joinTable.name) .transacting(trx) - .update({ [joinTable.inverseJoinColumn.name]: parent }) + .delete() + .where({ [joinTable.joinColumn.name]: id }) + .execute(); + await strapi.db + .queryBuilder(joinTable.name) + .transacting(trx) + .insert({ [joinTable.inverseJoinColumn.name]: parent, [joinTable.joinColumn.name]: id }) .where({ [joinTable.joinColumn.name]: id }) .execute();