diff --git a/packages/twenty-server/src/engine/core-modules/application/application.service.ts b/packages/twenty-server/src/engine/core-modules/application/application.service.ts index 0dd1e04bf3..1c9a877cb6 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application.service.ts @@ -1,21 +1,21 @@ -import { Injectable } from '@nestjs/common'; -import { InjectRepository } from '@nestjs/typeorm'; +import { Injectable, Logger } from '@nestjs/common'; +import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; import { FileFolder } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; -import { type QueryRunner, type Repository } from 'typeorm'; +import { type DataSource, type QueryRunner, type Repository } from 'typeorm'; import { v4 } from 'uuid'; -import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; -import { - ApplicationException, - ApplicationExceptionCode, -} from 'src/engine/core-modules/application/application.exception'; import { getDefaultApplicationPackageFields } from 'src/engine/core-modules/application/application-package/utils/get-default-application-package-fields.util'; import { parseAvailablePackagesFromPackageJsonAndYarnLock } from 'src/engine/core-modules/application/application-package/utils/parse-available-packages-from-package-json-and-yarn-lock.util'; import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity'; import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum'; import { ApplicationVariableEntity } from 'src/engine/core-modules/application/application-variable/application-variable.entity'; +import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; +import { + ApplicationException, + ApplicationExceptionCode, +} from 'src/engine/core-modules/application/application.exception'; import { WORKSPACE_CUSTOM_APPLICATION_NAME } from 'src/engine/core-modules/application/constants/workspace-custom-application.constant'; import { type FlatApplication } from 'src/engine/core-modules/application/types/flat-application.type'; import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service'; @@ -34,7 +34,11 @@ import { TWENTY_STANDARD_APPLICATION } from 'src/engine/workspace-manager/twenty @Injectable() export class ApplicationService { + private readonly logger = new Logger(ApplicationService.name); + constructor( + @InjectDataSource() + private readonly coreDataSource: DataSource, @InjectRepository(ApplicationEntity) private readonly applicationRepository: Repository, @InjectRepository(ApplicationRegistrationEntity) @@ -584,15 +588,50 @@ export class ApplicationService { ); } - await this.fileStorageService.deleteApplicationFiles({ - workspaceId, - applicationUniversalIdentifier: universalIdentifier, - }); + const queryRunner = this.coreDataSource.createQueryRunner(); - await this.applicationRepository.delete({ - universalIdentifier, - workspaceId, - }); + await queryRunner.connect(); + await queryRunner.startTransaction(); + + try { + await queryRunner.manager.update( + ApplicationEntity, + { id: application.id }, + { packageJsonFileId: null, yarnLockFileId: null }, + ); + + await this.fileStorageService.deleteApplicationFileRows({ + applicationId: application.id, + workspaceId, + queryRunner, + }); + + await queryRunner.manager.delete(ApplicationEntity, { + universalIdentifier, + workspaceId, + }); + + await queryRunner.commitTransaction(); + } catch (error) { + if (queryRunner.isTransactionActive) { + await queryRunner.rollbackTransaction(); + } + + throw error; + } finally { + await queryRunner.release(); + } + + try { + await this.fileStorageService.deleteApplicationFilesFromStorage({ + workspaceId, + applicationUniversalIdentifier: universalIdentifier, + }); + } catch (error) { + this.logger.warn( + `Failed to delete storage folder for application ${universalIdentifier} in workspace ${workspaceId}: ${error instanceof Error ? error.message : String(error)}`, + ); + } await this.workspaceCacheService.invalidateAndRecompute(workspaceId, [ 'flatApplicationMaps', 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 d42c2c2867..2a504344d8 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 @@ -228,29 +228,34 @@ export class FileStorageService { }); } - async deleteApplicationFiles({ + async deleteApplicationFileRows({ + applicationId, + workspaceId, + queryRunner, + }: { + applicationId: string; + workspaceId: string; + queryRunner?: QueryRunner; + }) { + const fileRepository = queryRunner + ? this.fileRepository.withManager(queryRunner.manager) + : this.fileRepository; + + await fileRepository.delete(workspaceId, { applicationId }); + } + + async deleteApplicationFilesFromStorage({ applicationUniversalIdentifier, workspaceId, }: { applicationUniversalIdentifier: string; workspaceId: string; }) { - const application = await this.applicationRepository.findOneOrFail({ - where: { - universalIdentifier: applicationUniversalIdentifier, - workspaceId: workspaceId, - }, - }); - const driver = this.fileStorageDriverFactory.getCurrentDriver(); await driver.delete({ folderPath: `${workspaceId}/${applicationUniversalIdentifier}/`, }); - - await this.fileRepository.delete(workspaceId, { - applicationId: application.id, - }); } async deleteFile(params: ResourceIdentifier): Promise { diff --git a/packages/twenty-server/test/integration/metadata/suites/application/successful-uninstall-application-with-package-file-fks.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/successful-uninstall-application-with-package-file-fks.integration-spec.ts new file mode 100644 index 0000000000..ee60275355 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application/successful-uninstall-application-with-package-file-fks.integration-spec.ts @@ -0,0 +1,105 @@ +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uninstallApplication } from 'test/integration/metadata/suites/application/utils/uninstall-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); + +describe('Uninstall application with package file FKs populated', () => { + beforeEach(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Test Application', + description: 'App for testing uninstall with package file FKs', + sourcePath: 'test-uninstall-package-file-fks', + }); + + await syncApplication({ + manifest: buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + }), + expectToFail: false, + }); + }, 60000); + + afterEach(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it('uninstalls the application and deletes its file rows in one pass', async () => { + jest.useRealTimers(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: TEST_APP_ID, + fileFolder: 'Dependencies', + filePath: 'yarn.lock', + fileBuffer: Buffer.from('# test yarn.lock\n'), + filename: 'yarn.lock', + contentType: 'text/plain', + expectToFail: false, + }); + + jest.useFakeTimers(); + + const [application] = await globalThis.testDataSource.query( + `SELECT id FROM core."application" WHERE "universalIdentifier" = $1`, + [TEST_APP_ID], + ); + + const applicationId: string = application.id; + + await globalThis.testDataSource.query( + `UPDATE core."application" a + SET "packageJsonFileId" = ( + SELECT f.id FROM core."file" f + WHERE f."applicationId" = a.id + AND f.path = 'dependencies/package.json' + ), + "yarnLockFileId" = ( + SELECT f.id FROM core."file" f + WHERE f."applicationId" = a.id + AND f.path = 'dependencies/yarn.lock' + ) + WHERE a.id = $1`, + [applicationId], + ); + + const [applicationBeforeUninstall] = await globalThis.testDataSource.query( + `SELECT "packageJsonFileId", "yarnLockFileId" + FROM core."application" WHERE id = $1`, + [applicationId], + ); + + expect(applicationBeforeUninstall.packageJsonFileId).not.toBeNull(); + expect(applicationBeforeUninstall.yarnLockFileId).not.toBeNull(); + + const { data, errors } = await uninstallApplication({ + universalIdentifier: TEST_APP_ID, + expectToFail: false, + }); + + expect(errors).toBeUndefined(); + expect(data?.uninstallApplication).toBe(true); + + const applicationsAfterUninstall = await globalThis.testDataSource.query( + `SELECT id FROM core."application" WHERE "universalIdentifier" = $1`, + [TEST_APP_ID], + ); + + expect(applicationsAfterUninstall).toHaveLength(0); + + const fileRowsAfterUninstall = await globalThis.testDataSource.query( + `SELECT id FROM core."file" WHERE "applicationId" = $1`, + [applicationId], + ); + + expect(fileRowsAfterUninstall).toHaveLength(0); + }, 60000); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util.ts b/packages/twenty-server/test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util.ts index b397597580..64b9451f9c 100644 --- a/packages/twenty-server/test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util.ts +++ b/packages/twenty-server/test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util.ts @@ -14,6 +14,13 @@ export const cleanupApplicationAndAppRegistration = async ({ // May fail if the sync never succeeded } + await globalThis.testDataSource.query( + `UPDATE core."application" + SET "packageJsonFileId" = NULL, "yarnLockFileId" = NULL + WHERE "universalIdentifier" = $1`, + [applicationUniversalIdentifier], + ); + await globalThis.testDataSource.query( `DELETE FROM core."file" WHERE "applicationId" IN ( SELECT id FROM core."application" WHERE "universalIdentifier" = $1