From ada55539773d9c4434b1114c736b3ba72899c280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Mon, 4 Apr 2022 14:32:08 +0200 Subject: [PATCH] remove path for files + add base for delete folders --- .../server/content-types/file/schema.js | 5 - .../upload/server/controllers/admin-folder.js | 27 ++-- .../server/controllers/validation/folder.js | 19 +++ .../server/controllers/validation/upload.js | 13 +- packages/core/upload/server/routes/admin.js | 16 ++ .../server/services/__tests__/file.test.js | 25 ---- packages/core/upload/server/services/file.js | 32 ---- .../core/upload/server/services/folder.js | 23 +++ packages/core/upload/server/services/index.js | 2 - .../core/upload/server/services/upload.js | 6 - .../core/upload/tests/admin/file.test.e2e.js | 70 +++++++-- .../upload/tests/admin/folder.test.e2e.js | 139 ++++++++++++------ 12 files changed, 238 insertions(+), 139 deletions(-) delete mode 100644 packages/core/upload/server/services/__tests__/file.test.js delete mode 100644 packages/core/upload/server/services/file.js diff --git a/packages/core/upload/server/content-types/file/schema.js b/packages/core/upload/server/content-types/file/schema.js index 79cddbbd0f..c51a0a8a40 100644 --- a/packages/core/upload/server/content-types/file/schema.js +++ b/packages/core/upload/server/content-types/file/schema.js @@ -95,10 +95,5 @@ module.exports = { target: 'plugin::upload.folder', inversedBy: 'files', }, - path: { - type: 'string', - min: 1, - required: true, - }, }, }; diff --git a/packages/core/upload/server/controllers/admin-folder.js b/packages/core/upload/server/controllers/admin-folder.js index bf7aa62fe0..46a3582c9d 100644 --- a/packages/core/upload/server/controllers/admin-folder.js +++ b/packages/core/upload/server/controllers/admin-folder.js @@ -1,9 +1,8 @@ 'use strict'; const { setCreatorFields, pipeAsync } = require('@strapi/utils'); -const { ApplicationError } = require('@strapi/utils').errors; const { getService } = require('../utils'); -const { validateCreateFolder } = require('./validation/folder'); +const { validateCreateFolder, validateDeleteManyFolders } = require('./validation/folder'); const folderModel = 'plugin::upload.folder'; @@ -40,16 +39,6 @@ module.exports = { await validateCreateFolder(body); - const existingFolders = await strapi.entityService.findMany(folderModel, { - filters: { - parent: body.parent || null, - name: body.name, - }, - }); - if (existingFolders.length > 0) { - throw new ApplicationError('name already taken'); - } - const { setPathAndUID } = getService('folder'); // TODO: wrap with a transaction @@ -70,4 +59,18 @@ module.exports = { data: await permissionsManager.sanitizeOutput(folder), }; }, + // deleteMany WIP + async deleteMany(ctx) { + const { body } = ctx.request; + + await validateDeleteManyFolders(body); + + const { deleteByIds } = getService('folder'); + + const deletedFolders = await deleteByIds(body.ids); + + ctx.body = { + data: deletedFolders, + }; + }, }; diff --git a/packages/core/upload/server/controllers/validation/folder.js b/packages/core/upload/server/controllers/validation/folder.js index ed2d863c4a..cc2320a9a2 100644 --- a/packages/core/upload/server/controllers/validation/folder.js +++ b/packages/core/upload/server/controllers/validation/folder.js @@ -1,6 +1,7 @@ 'use strict'; const { yup, validateYupSchema } = require('@strapi/utils'); +const { getService } = require('../../utils'); const NO_SLASH_REGEX = /^[^/]+$/; const NO_SPACES_AROUND = /^(?! ).+(? { + const { exists } = getService('folder'); + const doesExist = await exists({ parent: folder.parent || null, name: folder.name }); + return !doesExist; + }) + .noUnknown() + .required(); + +const validateDeleteManyFoldersSchema = yup + .object() + .shape({ + ids: yup + .array() + .min(1) + .of(yup.strapiID().required()) + .required(), + }) .noUnknown() .required(); module.exports = { validateCreateFolder: validateYupSchema(validateCreateFolderSchema), + validateDeleteManyFolders: validateYupSchema(validateDeleteManyFoldersSchema), }; diff --git a/packages/core/upload/server/controllers/validation/upload.js b/packages/core/upload/server/controllers/validation/upload.js index e1628f60df..953135b428 100644 --- a/packages/core/upload/server/controllers/validation/upload.js +++ b/packages/core/upload/server/controllers/validation/upload.js @@ -1,12 +1,23 @@ 'use strict'; const { yup, validateYupSchema } = require('@strapi/utils'); +const { isNil } = require('lodash/fp'); +const { getService } = require('../../utils'); const fileInfoSchema = yup.object({ name: yup.string().nullable(), alternativeText: yup.string().nullable(), caption: yup.string().nullable(), - folder: yup.strapiID().nullable(), + folder: yup + .strapiID() + .nullable() + .test('folder-exists', "the folder doesn't exist", async folderId => { + if (isNil(folderId)) { + return true; + } + + return getService('folder').exists({ id: folderId }); + }), }); const uploadSchema = yup.object({ diff --git a/packages/core/upload/server/routes/admin.js b/packages/core/upload/server/routes/admin.js index 0845b2b39d..c55086108f 100644 --- a/packages/core/upload/server/routes/admin.js +++ b/packages/core/upload/server/routes/admin.js @@ -123,5 +123,21 @@ module.exports = { ], }, }, + { + method: 'POST', + path: '/folders/batch-delete', + handler: 'admin-folder.deleteMany', + config: { + policies: [ + 'admin::isAuthenticatedAdmin', + { + name: 'admin::hasPermissions', + config: { + actions: ['plugin::upload.read'], + }, + }, + ], + }, + }, ], }; diff --git a/packages/core/upload/server/services/__tests__/file.test.js b/packages/core/upload/server/services/__tests__/file.test.js deleted file mode 100644 index 3a68f5b797..0000000000 --- a/packages/core/upload/server/services/__tests__/file.test.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; - -const { getPath } = require('../file'); - -describe('file', () => { - describe('getPath', () => { - beforeAll(() => { - global.strapi = { - entityService: { - findOne: jest.fn(() => ({ path: '/parent-path' })), - }, - }; - }); - - test.each([ - [[1, 'myFile.txt'], '/parent-path/myFile.txt'], - [[undefined, 'myFile.txt'], '/myFile.txt'], - [[null, 'myFile.txt'], '/myFile.txt'], - ])('inputs %s should give %s', async (args, expectedResult) => { - const result = await getPath(...args); - - expect(result).toBe(expectedResult); - }); - }); -}); diff --git a/packages/core/upload/server/services/file.js b/packages/core/upload/server/services/file.js deleted file mode 100644 index b50307064f..0000000000 --- a/packages/core/upload/server/services/file.js +++ /dev/null @@ -1,32 +0,0 @@ -'use strict'; - -const { trimChars, trimCharsEnd, trimCharsStart } = require('lodash/fp'); - -// TODO: to use once https://github.com/strapi/strapi/pull/12534 is merged -// const { joinBy } = require('@strapi/utils'); - -const folderModel = 'plugin::upload.folder'; - -const joinBy = (joint, ...args) => { - const trim = trimChars(joint); - const trimEnd = trimCharsEnd(joint); - const trimStart = trimCharsStart(joint); - - return args.reduce((url, path, index) => { - if (args.length === 1) return path; - if (index === 0) return trimEnd(path); - if (index === args.length - 1) return url + joint + trimStart(path); - return url + joint + trim(path); - }, ''); -}; - -const getPath = async (folderId, fileName) => { - if (!folderId) return joinBy('/', '/', fileName); - - const parentFolder = await strapi.entityService.findOne(folderModel, folderId); - return joinBy('/', parentFolder.path, fileName); -}; - -module.exports = { - getPath, -}; diff --git a/packages/core/upload/server/services/folder.js b/packages/core/upload/server/services/folder.js index 245715dd65..ff81282bb2 100644 --- a/packages/core/upload/server/services/folder.js +++ b/packages/core/upload/server/services/folder.js @@ -36,6 +36,29 @@ const setPathAndUID = async folder => { }); }; +const deleteByIds = async ids => { + const deletedFolders = []; + for (const id of ids) { + const deletedFolder = await strapi.entityService.delete(folderModel, id); + + deletedFolders.push(deletedFolder); + } + + return deletedFolders; +}; + +/** + * Check if a folder exists in database + * @param params query params to find the folder + * @returns {Promise} + */ +const exists = async (params = {}) => { + const count = await strapi.query(folderModel).count({ where: params }); + return count > 0; +}; + module.exports = { + exists, + deleteByIds, setPathAndUID, }; diff --git a/packages/core/upload/server/services/index.js b/packages/core/upload/server/services/index.js index 8ab6d72f48..b648c62df7 100644 --- a/packages/core/upload/server/services/index.js +++ b/packages/core/upload/server/services/index.js @@ -4,12 +4,10 @@ const provider = require('./provider'); const upload = require('./upload'); const imageManipulation = require('./image-manipulation'); const folder = require('./folder'); -const file = require('./file'); module.exports = { provider, upload, folder, - file, 'image-manipulation': imageManipulation, }; diff --git a/packages/core/upload/server/services/upload.js b/packages/core/upload/server/services/upload.js index b915070102..fecd2d4c3e 100644 --- a/packages/core/upload/server/services/upload.js +++ b/packages/core/upload/server/services/upload.js @@ -64,8 +64,6 @@ module.exports = ({ strapi }) => ({ }, async formatFileInfo({ filename, type, size }, fileInfo = {}, metas = {}) { - const fileService = getService('file'); - const ext = path.extname(filename); const basename = path.basename(fileInfo.name || filename, ext); const usedName = fileInfo.name || filename; @@ -75,7 +73,6 @@ module.exports = ({ strapi }) => ({ alternativeText: fileInfo.alternativeText, caption: fileInfo.caption, folder: fileInfo.folder, - path: await fileService.getPath(fileInfo.folder, usedName), hash: generateFileName(basename), ext, mime: type, @@ -208,15 +205,12 @@ module.exports = ({ strapi }) => ({ throw new NotFoundError(); } - const fileService = getService('file'); - const newName = _.isNil(name) ? dbFile.name : name; const newInfos = { name: newName, alternativeText: _.isNil(alternativeText) ? dbFile.alternativeText : alternativeText, caption: _.isNil(caption) ? dbFile.caption : caption, folder: _.isUndefined(folder) ? dbFile.folder : folder, - path: _.isUndefined(folder) ? dbFile.path : await fileService.getPath(folder, newName), }; return this.update(id, newInfos, { user }); diff --git a/packages/core/upload/tests/admin/file.test.e2e.js b/packages/core/upload/tests/admin/file.test.e2e.js index fe04ef94b6..6c5ebd0772 100644 --- a/packages/core/upload/tests/admin/file.test.e2e.js +++ b/packages/core/upload/tests/admin/file.test.e2e.js @@ -26,13 +26,21 @@ describe('File', () => { const folderRes = await rq({ method: 'POST', url: '/upload/folders', - body: { name: `folder ${i}` }, + body: { name: `my folder ${i}` }, }); data.folders.push(folderRes.body.data); } }); afterAll(async () => { + await rq({ + method: 'POST', + url: '/upload/folders/batch-delete', + body: { + ids: data.folders.map(f => f.id), + }, + }); + await strapi.destroy(); await builder.cleanup(); }); @@ -67,7 +75,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/rec.jpg', folder: null, }); @@ -106,12 +113,27 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/folder 1/rec.jpg', - folder: { id: 1 }, + folder: { id: data.folders[0].id }, }); data.files.push(file); }); + + test("Cannot create a file inside a folder that doesn't exist", async () => { + const res = await rq({ + method: 'POST', + url: '/upload', + formData: { + files: fs.createReadStream(path.join(__dirname, '../utils/rec.jpg')), + fileInfo: JSON.stringify({ + folder: '1234', // id that doesn't exist + }), + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe("the folder doesn't exist"); + }); }); describe('Update info', () => { @@ -145,7 +167,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/folder 2/rec.pdf', folder: { id: data.folders[1].id }, }); data.files[1] = file; @@ -179,7 +200,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/folder 1/rec.pdf', folder: { id: data.folders[0].id }, }); data.files[1] = file; @@ -215,7 +235,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/folder 1/rec.pdf', folder: { id: data.folders[0].id }, }); data.files[0] = file; @@ -249,7 +268,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/folder 2/rec.pdf', folder: { id: data.folders[1].id }, }); data.files[1] = file; @@ -286,7 +304,6 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/rec.jpg', folder: null, }); data.files[0] = file; @@ -320,11 +337,44 @@ describe('File', () => { height: expect.any(Number), url: expect.any(String), provider: 'local', - path: '/rec.pdf', folder: null, }); data.files[1] = file; }); }); + + describe("Cannot create a file inside a folder that doesn't exist", () => { + test('when replacing the file', async () => { + const res = await rq({ + method: 'POST', + url: `/upload?id=${data.files[1].id}`, + formData: { + files: fs.createReadStream(path.join(__dirname, '../utils/rec.jpg')), + fileInfo: JSON.stringify({ + folder: '1234', // id that doesn't exist + }), + }, + }); + + console.log('res.body', res.body); + expect(res.status).toBe(400); + expect(res.body.error.message).toBe("the folder doesn't exist"); + }); + + test('whithout replacing the file', async () => { + const res = await rq({ + method: 'POST', + url: `/upload?id=${data.files[1].id}`, + formData: { + fileInfo: JSON.stringify({ + folder: '1234', // id that doesn't exist + }), + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe("the folder doesn't exist"); + }); + }); }); }); diff --git a/packages/core/upload/tests/admin/folder.test.e2e.js b/packages/core/upload/tests/admin/folder.test.e2e.js index 630556b536..90fe0edfd1 100644 --- a/packages/core/upload/tests/admin/folder.test.e2e.js +++ b/packages/core/upload/tests/admin/folder.test.e2e.js @@ -38,6 +38,7 @@ describe('Folder', () => { }, }); + expect(res.status).toBe(200); expect(res.body.data).toMatchObject({ id: expect.anything(), name: 'folder 1', @@ -51,51 +52,6 @@ describe('Folder', () => { data.folders.push(omit('parent', res.body.data)); }); - test('Cannot create a folder with duplicated name at root level', async () => { - const res = await rq({ - method: 'POST', - url: '/upload/folders?populate=parent', - body: { - name: 'folder 1', - parent: null, - }, - }); - - expect(res.status).toBe(400); - expect(res.body.error.message).toBe('name already taken'); - }); - - test('Cannot create a folder with name containing a slash', async () => { - const res = await rq({ - method: 'POST', - url: '/upload/folders?populate=parent', - body: { - name: 'folder 1/2', - parent: null, - }, - }); - - expect(res.status).toBe(400); - expect(res.body.error.message).toBe('name cannot contain slashes'); - }); - - test.each([[' abc'], [' abc '], ['abc '], [' abc '], [' abc ']])( - 'Cannot create a folder with name starting or ending with a whitespace', - async name => { - const res = await rq({ - method: 'POST', - url: '/upload/folders?populate=parent', - body: { - name, - parent: null, - }, - }); - - expect(res.status).toBe(400); - expect(res.body.error.message).toBe('name cannot start or end with a whitespace'); - } - ); - test('Can create a folder inside another folder', async () => { const res = await rq({ method: 'POST', @@ -118,6 +74,65 @@ describe('Folder', () => { data.folders.push(omit('parent', res.body.data)); }); + + test('Cannot create a folder with duplicated name at root level', async () => { + const res = await rq({ + method: 'POST', + url: '/upload/folders?populate=parent', + body: { + name: 'folder 1', + parent: null, + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe('name already taken'); + }); + + test('Cannot create a folder with duplicated name inside a folder', async () => { + const res = await rq({ + method: 'POST', + url: '/upload/folders?populate=parent', + body: { + name: 'folder-2', + parent: data.folders[0], + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe('name already taken'); + }); + + test('Cannot create a folder with name containing a slash', async () => { + const res = await rq({ + method: 'POST', + url: '/upload/folders?populate=parent', + body: { + name: 'folder 1/2', + parent: null, + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe('name cannot contain slashes'); + }); + + test.each([[' abc'], [' abc '], ['abc '], [' abc '], [' abc ']])( + 'Cannot create a folder with name starting or ending with a whitespace (%p)', + async name => { + const res = await rq({ + method: 'POST', + url: '/upload/folders?populate=parent', + body: { + name, + parent: null, + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error.message).toBe('name cannot start or end with a whitespace'); + } + ); }); describe('read', () => { @@ -133,7 +148,6 @@ describe('Folder', () => { pageSize: 10, total: 2, }); - expect(res.body.results).toHaveLength(2); expect(res.body.results).toEqual( expect.arrayContaining([ { @@ -193,4 +207,37 @@ describe('Folder', () => { ); }); }); + + describe('delete', () => { + test('Can delete folders', async () => { + const res = await rq({ + method: 'POST', + url: '/upload/folders/batch-delete', + body: { + ids: data.folders.map(f => f.id), + }, + }); + + expect(res.body.data).toEqual( + expect.arrayContaining([ + { + createdAt: expect.anything(), + id: expect.anything(), + name: 'folder 1', + path: '/folder 1', + uid: expect.anything(), + updatedAt: expect.anything(), + }, + { + createdAt: expect.anything(), + id: expect.anything(), + name: 'folder-2', + path: '/folder 1/folder-2', + uid: expect.anything(), + updatedAt: expect.anything(), + }, + ]) + ); + }); + }); });