From c640c25c74cc7a1084159263d895eb8b3ceedbb2 Mon Sep 17 00:00:00 2001 From: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com> Date: Wed, 26 Apr 2023 13:48:46 +0200 Subject: [PATCH] [Files] Adds bulk delete method (#155628) ## Summary Closes https://github.com/elastic/kibana/issues/154286 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../server/file_service/file_action_types.ts | 10 ++++++++ .../files/server/file_service/file_service.ts | 8 +++++++ .../file_service/file_service_factory.ts | 5 +++- .../file_service/internal_file_service.ts | 13 +++++++++++ .../integration_tests/file_service.test.ts | 23 +++++++++++++++++-- src/plugins/files/server/mocks.ts | 1 + 6 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/plugins/files/server/file_service/file_action_types.ts b/src/plugins/files/server/file_service/file_action_types.ts index 96795ac93b38..f0aad5e86e8e 100644 --- a/src/plugins/files/server/file_service/file_action_types.ts +++ b/src/plugins/files/server/file_service/file_action_types.ts @@ -62,6 +62,16 @@ export interface DeleteFileArgs { id: string; } +/** + * Arguments to delete files in a bulk request. + */ +export interface BulkDeleteFilesArgs { + /** + * File IDs. + */ + ids: string[]; +} + /** * Arguments to get a file by ID. */ diff --git a/src/plugins/files/server/file_service/file_service.ts b/src/plugins/files/server/file_service/file_service.ts index 9dc1b0769ced..0828d03ada7f 100644 --- a/src/plugins/files/server/file_service/file_service.ts +++ b/src/plugins/files/server/file_service/file_service.ts @@ -12,6 +12,7 @@ import type { CreateFileArgs, UpdateFileArgs, DeleteFileArgs, + BulkDeleteFilesArgs, GetByIdArgs, FindFileArgs, } from './file_action_types'; @@ -43,6 +44,13 @@ export interface FileServiceStart { */ delete(args: DeleteFileArgs): Promise; + /** + * Delete multiple files at once. + * + * @param args - delete files args + */ + bulkDelete(args: BulkDeleteFilesArgs): Promise>>; + /** * Get a file by ID. Will throw if file cannot be found. * diff --git a/src/plugins/files/server/file_service/file_service_factory.ts b/src/plugins/files/server/file_service/file_service_factory.ts index 50ceafb6fc24..bebb5b72c2b3 100644 --- a/src/plugins/files/server/file_service/file_service_factory.ts +++ b/src/plugins/files/server/file_service/file_service_factory.ts @@ -94,7 +94,10 @@ export class FileServiceFactoryImpl implements FileServiceFactory { await internalFileService.updateFile(args); }, async delete(args) { - return internalFileService.deleteFile(args); + return await internalFileService.deleteFile(args); + }, + async bulkDelete(args) { + return await internalFileService.bulkDeleteFiles(args); }, async getById(args: GetByIdArgs) { return internalFileService.getById(args) as Promise>; diff --git a/src/plugins/files/server/file_service/internal_file_service.ts b/src/plugins/files/server/file_service/internal_file_service.ts index cd7a45740de4..653929f7b123 100644 --- a/src/plugins/files/server/file_service/internal_file_service.ts +++ b/src/plugins/files/server/file_service/internal_file_service.ts @@ -8,6 +8,7 @@ import { Logger, SavedObjectsErrorHelpers } from '@kbn/core/server'; import { AuditEvent, AuditLogger } from '@kbn/security-plugin/server'; +import pLimit from 'p-limit'; import { BlobStorageService } from '../blob_storage_service'; import { InternalFileShareService } from '../file_share_service'; @@ -20,10 +21,14 @@ import type { CreateFileArgs, UpdateFileArgs, DeleteFileArgs, + BulkDeleteFilesArgs, FindFileArgs, GetByIdArgs, } from './file_action_types'; import { createFileClient, FileClientImpl } from '../file_client/file_client'; + +const bulkDeleteConcurrency = pLimit(10); + /** * Service containing methods for working with files. * @@ -64,6 +69,14 @@ export class InternalFileService { await file.delete(); } + public async bulkDeleteFiles({ + ids, + }: BulkDeleteFilesArgs): Promise>> { + const promises = ids.map((id) => bulkDeleteConcurrency(() => this.deleteFile({ id }))); + const result = await Promise.allSettled(promises); + return result; + } + private async get(id: string) { try { const { metadata } = await this.metadataClient.get({ id }); diff --git a/src/plugins/files/server/integration_tests/file_service.test.ts b/src/plugins/files/server/integration_tests/file_service.test.ts index 3492eb8e5f12..5ca1fa00bdd0 100644 --- a/src/plugins/files/server/integration_tests/file_service.test.ts +++ b/src/plugins/files/server/integration_tests/file_service.test.ts @@ -99,7 +99,7 @@ describe('FileService', () => { return file; } afterEach(async () => { - await Promise.all(disposables.map((file) => file.delete())); + await fileService.bulkDelete({ ids: disposables.map((d) => d.id) }); const { files } = await fileService.find({ kind: [fileKind] }); expect(files.length).toBe(0); disposables = []; @@ -246,7 +246,7 @@ describe('FileService', () => { expect(result3.files.length).toBe(2); }); - it('deletes files', async () => { + it('deletes a single file', async () => { const file = await fileService.create({ fileKind, name: 'test' }); const result = await fileService.find({ kind: [fileKind] }); expect(result.files.length).toBe(1); @@ -254,6 +254,25 @@ describe('FileService', () => { expect(await fileService.find({ kind: [fileKind] })).toEqual({ files: [], total: 0 }); }); + it('deletes a single file using the bulk method', async () => { + const file = await fileService.create({ fileKind, name: 'test' }); + const result = await fileService.find({ kind: [fileKind] }); + expect(result.files.length).toBe(1); + await fileService.bulkDelete({ ids: [file.id] }); + expect(await fileService.find({ kind: [fileKind] })).toEqual({ files: [], total: 0 }); + }); + + it('deletes multiple files using the bulk method', async () => { + const promises = Array.from({ length: 15 }, (v, i) => + fileService.create({ fileKind, name: 'test ' + i }) + ); + const files = await Promise.all(promises); + const result = await fileService.find({ kind: [fileKind] }); + expect(result.files.length).toBe(15); + await fileService.bulkDelete({ ids: files.map((file) => file.id) }); + expect(await fileService.find({ kind: [fileKind] })).toEqual({ files: [], total: 0 }); + }); + interface CustomMeta { some: string; } diff --git a/src/plugins/files/server/mocks.ts b/src/plugins/files/server/mocks.ts index 8472717b544a..da94d95b273c 100644 --- a/src/plugins/files/server/mocks.ts +++ b/src/plugins/files/server/mocks.ts @@ -15,6 +15,7 @@ import { FileClient, FileServiceFactory, FileServiceStart, FilesSetup } from '.' export const createFileServiceMock = (): DeeplyMockedKeys => ({ create: jest.fn(), delete: jest.fn(), + bulkDelete: jest.fn(), deleteShareObject: jest.fn(), find: jest.fn(), getById: jest.fn(),