From 1b09c69c39b9059494cc8974149376d1a63bd159 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Mon, 11 May 2026 18:16:46 +0200 Subject: [PATCH] refactor(file v2) - deletion (#20356) --- .../__tests__/file-storage.service.spec.ts | 76 ------------------- .../file-storage/file-storage.service.ts | 34 +++++---- .../engine/core-modules/file/file.module.ts | 4 - .../file/jobs/file-deletion.job.ts | 35 ++++----- .../file-workspace-folder-deletion.job.ts | 1 - .../listeners/file-attachment.listener.ts | 40 ---------- .../file-workspace-member.listener.ts | 43 ----------- .../file/services/file.service.ts | 29 +------ 8 files changed, 34 insertions(+), 228 deletions(-) delete mode 100644 packages/twenty-server/src/engine/core-modules/file/listeners/file-attachment.listener.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file/listeners/file-workspace-member.listener.ts diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts index 3f4a036c68..1cbe772e04 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/__tests__/file-storage.service.spec.ts @@ -73,52 +73,6 @@ describe('FileStorageService', () => { mockFileStorageDriverFactory.getCurrentDriver.mockReturnValue(mockDriver); }); - describe('deleteLegacy', () => { - it('should delegate to the current driver with filename', async () => { - const deleteParams = { - folderPath: 'documents', - filename: 'test.txt', - }; - - mockDriver.delete.mockResolvedValue(undefined); - - await service.deleteLegacy(deleteParams); - - expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled(); - expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams); - }); - - it('should delegate to the current driver without filename (delete folder)', async () => { - const deleteParams = { - folderPath: 'documents', - }; - - mockDriver.delete.mockResolvedValue(undefined); - - await service.deleteLegacy(deleteParams); - - expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled(); - expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams); - }); - - it('should handle delete errors', async () => { - const deleteParams = { - folderPath: 'documents', - filename: 'test.txt', - }; - - const error = new Error('Delete failed'); - - mockDriver.delete.mockRejectedValue(error); - - await expect(service.deleteLegacy(deleteParams)).rejects.toThrow( - 'Delete failed', - ); - expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled(); - expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams); - }); - }); - describe('copyLegacy', () => { it('should delegate to the current driver', async () => { const copyParams = { @@ -192,35 +146,5 @@ describe('FileStorageService', () => { expect(result).toBeNull(); }); }); - - describe('checkFolderExistsLegacy', () => { - it('should delegate to the current driver and return true', async () => { - const checkParams = { - folderPath: 'documents', - }; - - mockDriver.checkFolderExists.mockResolvedValue(true); - - const result = await service.checkFolderExistsLegacy(checkParams); - - expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled(); - expect(mockDriver.checkFolderExists).toHaveBeenCalledWith(checkParams); - expect(result).toBe(true); - }); - - it('should delegate to the current driver and return false', async () => { - const checkParams = { - folderPath: 'nonexistent', - }; - - mockDriver.checkFolderExists.mockResolvedValue(false); - - const result = await service.checkFolderExistsLegacy(checkParams); - - expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled(); - expect(mockDriver.checkFolderExists).toHaveBeenCalledWith(checkParams); - expect(result).toBe(false); - }); - }); }); }); diff --git a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts index c27390d441..95d64004db 100644 --- a/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file-storage/file-storage.service.ts @@ -152,15 +152,6 @@ export class FileStorageService { }); } - deleteLegacy(params: { - folderPath: string; - filename?: string; - }): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - return driver.delete(params); - } - async deleteApplicationFiles({ applicationUniversalIdentifier, workspaceId, @@ -224,16 +215,33 @@ export class FileStorageService { path: Like(`${fileFolder}/%`), }, }); + + const application = await this.applicationRepository.findOneOrFail({ + where: { id: file.applicationId, workspaceId: file.workspaceId }, + }); + const driver = this.fileStorageDriverFactory.getCurrentDriver(); await driver.delete({ - folderPath: `${file.workspaceId}/${file.applicationId}`, + folderPath: `${file.workspaceId}/${application.universalIdentifier}`, filename: file.path, }); await this.fileRepository.delete(fileId); } + async checkIfWorkspaceFolderExists(workspaceId: string): Promise { + const driver = this.fileStorageDriverFactory.getCurrentDriver(); + + return driver.checkFolderExists({ folderPath: workspaceId }); + } + + async deleteWorkspaceFolder(workspaceId: string): Promise { + const driver = this.fileStorageDriverFactory.getCurrentDriver(); + + await driver.delete({ folderPath: workspaceId }); + } + copyLegacy(params: { from: { folderPath: string; filename?: string }; to: { folderPath: string; filename?: string }; @@ -270,12 +278,6 @@ export class FileStorageService { }); } - checkFolderExistsLegacy(params: { folderPath: string }): Promise { - const driver = this.fileStorageDriverFactory.getCurrentDriver(); - - return driver.checkFolderExists(params); - } - checkFileExists(params: ResourceIdentifier): Promise { const driver = this.fileStorageDriverFactory.getCurrentDriver(); const onStoragePath = this.buildOnStoragePath(params); diff --git a/packages/twenty-server/src/engine/core-modules/file/file.module.ts b/packages/twenty-server/src/engine/core-modules/file/file.module.ts index 4effdaa9b3..aec7d0d78c 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file.module.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file.module.ts @@ -7,8 +7,6 @@ import { FileAiChatModule } from 'src/engine/core-modules/file/file-ai-chat/file import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-guard'; import { FileDeletionJob } from 'src/engine/core-modules/file/jobs/file-deletion.job'; import { FileWorkspaceFolderDeletionJob } from 'src/engine/core-modules/file/jobs/file-workspace-folder-deletion.job'; -import { FileAttachmentListener } from 'src/engine/core-modules/file/listeners/file-attachment.listener'; -import { FileWorkspaceMemberListener } from 'src/engine/core-modules/file/listeners/file-workspace-member.listener'; import { JwtModule } from 'src/engine/core-modules/jwt/jwt.module'; import { SecureHttpClientModule } from 'src/engine/core-modules/secure-http-client/secure-http-client.module'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; @@ -42,8 +40,6 @@ import { FileService } from './services/file.service'; FileService, FilePathGuard, FileByIdGuard, - FileAttachmentListener, - FileWorkspaceMemberListener, FileWorkspaceFolderDeletionJob, FileDeletionJob, ], diff --git a/packages/twenty-server/src/engine/core-modules/file/jobs/file-deletion.job.ts b/packages/twenty-server/src/engine/core-modules/file/jobs/file-deletion.job.ts index 32d7dde21d..74561660cf 100644 --- a/packages/twenty-server/src/engine/core-modules/file/jobs/file-deletion.job.ts +++ b/packages/twenty-server/src/engine/core-modules/file/jobs/file-deletion.job.ts @@ -1,42 +1,35 @@ -import { UnrecoverableError } from 'bullmq'; -import { extractFolderPathFilenameAndTypeOrThrow } from 'twenty-shared/utils'; +import { FileFolder } from 'twenty-shared/types'; -import { FileService } from 'src/engine/core-modules/file/services/file.service'; +import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service'; import { Process } from 'src/engine/core-modules/message-queue/decorators/process.decorator'; import { Processor } from 'src/engine/core-modules/message-queue/decorators/processor.decorator'; import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants'; export type FileDeletionJobData = { workspaceId: string; - fullPath: string; + fileId: string; + fileFolder: FileFolder; }; @Processor(MessageQueue.deleteCascadeQueue) export class FileDeletionJob { - constructor(private readonly fileService: FileService) {} + constructor(private readonly fileStorageService: FileStorageService) {} @Process(FileDeletionJob.name) - async handle(data: FileDeletionJobData): Promise { - const { workspaceId, fullPath } = data; - - const { folderPath, filename } = - extractFolderPathFilenameAndTypeOrThrow(fullPath); - - if (!filename) { - throw new UnrecoverableError( - `[${FileDeletionJob.name}] Cannot parse filename from full path - ${fullPath}`, - ); - } - + async handle({ + workspaceId, + fileId, + fileFolder, + }: FileDeletionJobData): Promise { try { - await this.fileService.deleteFile({ + await this.fileStorageService.deleteByFileId({ + fileId, workspaceId, - filename, - folderPath, + fileFolder, }); } catch { throw new Error( - `[${FileDeletionJob.name}] Cannot delete file - ${fullPath}`, + `[${FileDeletionJob.name}] Cannot delete file - ${fileId} in folder ${fileFolder}`, ); } } diff --git a/packages/twenty-server/src/engine/core-modules/file/jobs/file-workspace-folder-deletion.job.ts b/packages/twenty-server/src/engine/core-modules/file/jobs/file-workspace-folder-deletion.job.ts index 84956df11f..62f651ef88 100644 --- a/packages/twenty-server/src/engine/core-modules/file/jobs/file-workspace-folder-deletion.job.ts +++ b/packages/twenty-server/src/engine/core-modules/file/jobs/file-workspace-folder-deletion.job.ts @@ -18,7 +18,6 @@ export class FileWorkspaceFolderDeletionJob { try { await this.fileService.deleteWorkspaceFolder(workspaceId); } catch (error) { - //todo: clean up error message once issue on workspace folder deletion is fixed + in s3 driver file throw new Error( `[${FileWorkspaceFolderDeletionJob.name}] Cannot delete workspace folder - ${workspaceId} - ${error?.message || error}`, ); diff --git a/packages/twenty-server/src/engine/core-modules/file/listeners/file-attachment.listener.ts b/packages/twenty-server/src/engine/core-modules/file/listeners/file-attachment.listener.ts deleted file mode 100644 index a930923d43..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file/listeners/file-attachment.listener.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { Injectable } from '@nestjs/common'; - -import { type ObjectRecordDestroyEvent } from 'twenty-shared/database-events'; - -import { OnDatabaseBatchEvent } from 'src/engine/api/graphql/graphql-query-runner/decorators/on-database-batch-event.decorator'; -import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action'; -import { - FileDeletionJob, - type FileDeletionJobData, -} from 'src/engine/core-modules/file/jobs/file-deletion.job'; -import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator'; -import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants'; -import { MessageQueueService } from 'src/engine/core-modules/message-queue/services/message-queue.service'; -import { WorkspaceEventBatch } from 'src/engine/workspace-event-emitter/types/workspace-event-batch.type'; -import { type AttachmentWorkspaceEntity } from 'src/modules/attachment/standard-objects/attachment.workspace-entity'; - -@Injectable() -export class FileAttachmentListener { - constructor( - @InjectMessageQueue(MessageQueue.deleteCascadeQueue) - private readonly messageQueueService: MessageQueueService, - ) {} - - @OnDatabaseBatchEvent('attachment', DatabaseEventAction.DESTROYED) - async handleDestroyEvent( - payload: WorkspaceEventBatch< - ObjectRecordDestroyEvent - >, - ) { - for (const event of payload.events) { - await this.messageQueueService.add( - FileDeletionJob.name, - { - workspaceId: payload.workspaceId, - fullPath: event.properties.before.fullPath ?? '', - }, - ); - } - } -} diff --git a/packages/twenty-server/src/engine/core-modules/file/listeners/file-workspace-member.listener.ts b/packages/twenty-server/src/engine/core-modules/file/listeners/file-workspace-member.listener.ts deleted file mode 100644 index cf7cc5af99..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file/listeners/file-workspace-member.listener.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { Injectable } from '@nestjs/common'; - -import { type ObjectRecordDestroyEvent } from 'twenty-shared/database-events'; - -import { OnDatabaseBatchEvent } from 'src/engine/api/graphql/graphql-query-runner/decorators/on-database-batch-event.decorator'; -import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action'; -import { - FileDeletionJob, - type FileDeletionJobData, -} from 'src/engine/core-modules/file/jobs/file-deletion.job'; -import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator'; -import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants'; -import { MessageQueueService } from 'src/engine/core-modules/message-queue/services/message-queue.service'; -import { WorkspaceEventBatch } from 'src/engine/workspace-event-emitter/types/workspace-event-batch.type'; -import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; - -@Injectable() -export class FileWorkspaceMemberListener { - constructor( - @InjectMessageQueue(MessageQueue.deleteCascadeQueue) - private readonly messageQueueService: MessageQueueService, - ) {} - - @OnDatabaseBatchEvent('workspaceMember', DatabaseEventAction.DESTROYED) - async handleDestroyEvent( - payload: WorkspaceEventBatch< - ObjectRecordDestroyEvent - >, - ) { - for (const event of payload.events) { - const avatarUrl = event.properties.before.avatarUrl; - - if (!avatarUrl) { - continue; - } - - this.messageQueueService.add(FileDeletionJob.name, { - workspaceId: payload.workspaceId, - fullPath: event.properties.before.avatarUrl ?? '', - }); - } - } -} diff --git a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts index 1cf234676a..adee8c3c66 100644 --- a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts @@ -189,39 +189,14 @@ export class FileService { }; } - /** @deprecated Use FileStorageService.deleteByFileId instead */ - async deleteFile({ - folderPath, - filename, - workspaceId, - }: { - folderPath: string; - filename: string; - workspaceId: string; - }) { - const workspaceFolderPath = `workspace-${workspaceId}/${folderPath}`; - - return await this.fileStorageService.deleteLegacy({ - folderPath: workspaceFolderPath, - filename, - }); - } - - /** @deprecated */ async deleteWorkspaceFolder(workspaceId: string) { - const workspaceFolderPath = `workspace-${workspaceId}`; - const isWorkspaceFolderFound = - await this.fileStorageService.checkFolderExistsLegacy({ - folderPath: workspaceFolderPath, - }); + await this.fileStorageService.checkIfWorkspaceFolderExists(workspaceId); if (!isWorkspaceFolderFound) { return; } - return await this.fileStorageService.deleteLegacy({ - folderPath: workspaceFolderPath, - }); + return await this.fileStorageService.deleteWorkspaceFolder(workspaceId); } }