From 791276e80f5745c8885352b7979c0faa979110f4 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Tue, 27 May 2025 12:22:11 +0530 Subject: [PATCH] feat: introduce bulkDelete method for IFileProvider (#12614) Fixes: FRMW-2974 Currently during the product imports, we create multiple chunks that must be deleted after the import has finished (either successfully or with an error). Deleting files one by one leads to multiple network calls and slows down everything. The `bulkDelete` method deletes multiple files (with their fileKey) in one go --- .changeset/brown-fans-draw.md | 9 +++++ packages/core/types/src/file/provider.ts | 11 +++--- .../utils/src/file/abstract-file-provider.ts | 18 +++++----- .../file/src/services/file-module-service.ts | 8 ++--- .../src/services/file-provider-service.ts | 6 +++- .../file-local/src/services/local-file.ts | 36 +++++++++++-------- .../__tests__/services.spec.ts | 27 ++++++++++++++ .../providers/file-s3/src/services/s3-file.ts | 34 ++++++++++++++---- 8 files changed, 111 insertions(+), 38 deletions(-) create mode 100644 .changeset/brown-fans-draw.md diff --git a/.changeset/brown-fans-draw.md b/.changeset/brown-fans-draw.md new file mode 100644 index 0000000000..848b8874e7 --- /dev/null +++ b/.changeset/brown-fans-draw.md @@ -0,0 +1,9 @@ +--- +"@medusajs/file": patch +"@medusajs/file-local": patch +"@medusajs/file-s3": patch +"@medusajs/types": patch +"@medusajs/utils": patch +--- + +feat: introduce bulkDelete method for IFileProvider diff --git a/packages/core/types/src/file/provider.ts b/packages/core/types/src/file/provider.ts index 8fc2c51210..a7942787b5 100644 --- a/packages/core/types/src/file/provider.ts +++ b/packages/core/types/src/file/provider.ts @@ -119,14 +119,17 @@ export interface IFileProvider { * */ upload(file: ProviderUploadFileDTO): Promise + /** - * This method is used to delete a file from storage. + * This method is used to delete one or more files from the storage * - * @param {ProviderDeleteFileDTO} fileData - The details of the file to remove. + * @param {ProviderDeleteFileDTO | ProviderDeleteFileDTO[]} fileData - The details of the file to remove. * @returns {Promise} Resolves when the file is deleted successfully. * */ - delete(fileData: ProviderDeleteFileDTO): Promise + delete( + fileData: ProviderDeleteFileDTO | ProviderDeleteFileDTO[] + ): Promise /** * This method is used to retrieve a download URL of the file. For some file services, such as S3, a presigned URL indicates a temporary URL to get access to a file. @@ -142,7 +145,7 @@ export interface IFileProvider { /** * This method is used to get a presigned upload URL for a file. For some providers, * such as S3, a presigned URL indicates a temporary URL to get upload a file. - * + * * If your provider doesn’t perform or offer a similar functionality, you don't have to * implement this method. Instead, an error is thrown when the method is called. * diff --git a/packages/core/utils/src/file/abstract-file-provider.ts b/packages/core/utils/src/file/abstract-file-provider.ts index 62149b1fdb..d08cba20a5 100644 --- a/packages/core/utils/src/file/abstract-file-provider.ts +++ b/packages/core/utils/src/file/abstract-file-provider.ts @@ -125,10 +125,10 @@ export class AbstractFileProviderService implements IFileProvider { } /** - * This method deletes the file from storage. It's used when an admin user deletes a product image, - * or other custom file deletions. + * This method deletes one or more files from the storage. It's used when an admin user + * deletes a product image, or other custom file deletions. * - * @param {FileTypes.ProviderDeleteFileDTO} file - The details of the file to delete. + * @param {FileTypes.ProviderDeleteFileDTO | FileTypes.ProviderDeleteFileDTO[]} files - The details of the file(s) to delete. * @returns {Promise} Resolves when the file is deleted. * * @example @@ -143,7 +143,9 @@ export class AbstractFileProviderService implements IFileProvider { * } * } */ - async delete(file: FileTypes.ProviderDeleteFileDTO): Promise { + async delete( + files: FileTypes.ProviderDeleteFileDTO | FileTypes.ProviderDeleteFileDTO[] + ): Promise { throw Error("delete must be overridden by the child class") } @@ -181,10 +183,10 @@ export class AbstractFileProviderService implements IFileProvider { /** * This method retrieves an uploaded file as a stream. This is useful when streaming * a file to clients or you want to process the file in chunks. - * + * * @param {FileTypes.ProviderGetFileDTO} fileData - The details of the file to get its stream. * @returns {Promise} The file's stream. - * + * * @version 2.8.0 * * @example @@ -206,10 +208,10 @@ export class AbstractFileProviderService implements IFileProvider { /** * This method retrieves an uploaded file as a buffer. This is useful when you want to * process the entire file in memory or send it as a response. - * + * * @param {FileTypes.ProviderGetFileDTO} fileData - The details of the file to get its buffer. * @returns {Promise} The file's buffer. - * + * * @version 2.8.0 * * @example diff --git a/packages/modules/file/src/services/file-module-service.ts b/packages/modules/file/src/services/file-module-service.ts index def65a53b6..d162ca6593 100644 --- a/packages/modules/file/src/services/file-module-service.ts +++ b/packages/modules/file/src/services/file-module-service.ts @@ -81,11 +81,11 @@ export default class FileModuleService implements FileTypes.IFileModuleService { async deleteFiles(id: string, sharedContext?: Context): Promise async deleteFiles(ids: string[] | string): Promise { const input = Array.isArray(ids) ? ids : [ids] - await Promise.all( - input.map((id) => this.fileProviderService_.delete({ fileKey: id })) + await this.fileProviderService_.delete( + input.map((id) => { + return { fileKey: id } + }) ) - - return } async retrieveFile(id: string): Promise { diff --git a/packages/modules/file/src/services/file-provider-service.ts b/packages/modules/file/src/services/file-provider-service.ts index 13a2c1fbb7..959ab9c254 100644 --- a/packages/modules/file/src/services/file-provider-service.ts +++ b/packages/modules/file/src/services/file-provider-service.ts @@ -40,7 +40,11 @@ export default class FileProviderService { return this.fileProvider_.upload(file) } - delete(fileData: FileTypes.ProviderDeleteFileDTO): Promise { + delete( + fileData: + | FileTypes.ProviderDeleteFileDTO + | FileTypes.ProviderDeleteFileDTO[] + ): Promise { return this.fileProvider_.delete(fileData) } diff --git a/packages/modules/providers/file-local/src/services/local-file.ts b/packages/modules/providers/file-local/src/services/local-file.ts index 903e510076..7eacc6cba2 100644 --- a/packages/modules/providers/file-local/src/services/local-file.ts +++ b/packages/modules/providers/file-local/src/services/local-file.ts @@ -66,21 +66,29 @@ export class LocalFileService extends AbstractFileProviderService { } } - async delete(file: FileTypes.ProviderDeleteFileDTO): Promise { - const baseDir = file.fileKey.startsWith("private-") - ? this.privateUploadDir_ - : this.uploadDir_ + async delete( + files: FileTypes.ProviderDeleteFileDTO | FileTypes.ProviderDeleteFileDTO[] + ): Promise { + files = Array.isArray(files) ? files : [files] - const filePath = this.getUploadFilePath(baseDir, file.fileKey) - try { - await fs.access(filePath, fs.constants.W_OK) - await fs.unlink(filePath) - } catch (e) { - // The file does not exist, we don't do anything - if (e.code !== "ENOENT") { - throw e - } - } + await Promise.all( + files.map(async (file) => { + const baseDir = file.fileKey.startsWith("private-") + ? this.privateUploadDir_ + : this.uploadDir_ + + const filePath = this.getUploadFilePath(baseDir, file.fileKey) + try { + await fs.access(filePath, fs.constants.W_OK) + await fs.unlink(filePath) + } catch (e) { + // The file does not exist, we don't do anything + if (e.code !== "ENOENT") { + throw e + } + } + }) + ) return } diff --git a/packages/modules/providers/file-s3/integration-tests/__tests__/services.spec.ts b/packages/modules/providers/file-s3/integration-tests/__tests__/services.spec.ts index 95e1473c52..d874e88c3a 100644 --- a/packages/modules/providers/file-s3/integration-tests/__tests__/services.spec.ts +++ b/packages/modules/providers/file-s3/integration-tests/__tests__/services.spec.ts @@ -160,4 +160,31 @@ describe.skip("S3 File Plugin", () => { await s3Service.delete({ fileKey: resp.key }) }) + + it("deletes multiple files in bulk", async () => { + const fileContent = await fs.readFile(fixtureImagePath) + const fixtureAsBinary = fileContent.toString("binary") + + const cat = await s3Service.upload({ + filename: "catphoto.jpg", + mimeType: "image/jpeg", + content: fixtureAsBinary, + }) + const cat1 = await s3Service.upload({ + filename: "catphoto-1.jpg", + mimeType: "image/jpeg", + content: fixtureAsBinary, + }) + const cat2 = await s3Service.upload({ + filename: "catphoto-2.jpg", + mimeType: "image/jpeg", + content: fixtureAsBinary, + }) + + await s3Service.delete([ + { fileKey: cat.key }, + { fileKey: cat1.key }, + { fileKey: cat2.key }, + ]) + }) }) diff --git a/packages/modules/providers/file-s3/src/services/s3-file.ts b/packages/modules/providers/file-s3/src/services/s3-file.ts index 60d5ff934a..4ddc0f1fcd 100644 --- a/packages/modules/providers/file-s3/src/services/s3-file.ts +++ b/packages/modules/providers/file-s3/src/services/s3-file.ts @@ -1,5 +1,6 @@ import { DeleteObjectCommand, + DeleteObjectsCommand, GetObjectCommand, ObjectCannedACL, PutObjectCommand, @@ -152,14 +153,33 @@ export class S3FileService extends AbstractFileProviderService { } } - async delete(file: FileTypes.ProviderDeleteFileDTO): Promise { - const command = new DeleteObjectCommand({ - Bucket: this.config_.bucket, - Key: file.fileKey, - }) - + async delete( + files: FileTypes.ProviderDeleteFileDTO | FileTypes.ProviderDeleteFileDTO[] + ): Promise { try { - await this.client_.send(command) + /** + * Bulk delete files + */ + if (Array.isArray(files)) { + await this.client_.send( + new DeleteObjectsCommand({ + Bucket: this.config_.bucket, + Delete: { + Objects: files.map((file) => ({ + Key: file.fileKey, + })), + Quiet: true, + }, + }) + ) + } else { + await this.client_.send( + new DeleteObjectCommand({ + Bucket: this.config_.bucket, + Key: files.fileKey, + }) + ) + } } catch (e) { // TODO: Rethrow depending on the error (eg. a file not found error is fine, but a failed request should be rethrown) this.logger_.error(e)