From 9a7c0f25a5b7a3b3ba0536f196a3d99ab46be743 Mon Sep 17 00:00:00 2001 From: Abdul Rahman <81605929+abdulrahmancodes@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:45:48 +0530 Subject: [PATCH] fix(server): resolve foreign key violation blocking application uninstall (#22502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes [sonarly issue #54192](https://sonarly.com/issue/54192) ## Problem Uninstalling an application fails with a DB error when its `packageJsonFileId` / `yarnLockFileId` columns are populated: update or delete on table "file" violates foreign key constraint "FK_3818380258798f9ffa9963b6dc4" on table "application" Storage was also wiped before the failing DB delete, leaving the app half-uninstalled. ## Root cause `application` and `file` reference each other through `ON DELETE RESTRICT` FKs (`application.packageJsonFileId/yarnLockFileId → file.id` and `file.applicationId → application.id`), so no deletion order works on its own. The deferrable-FK migration doesn't help: in Postgres, `RESTRICT` fires immediately even on `DEFERRABLE INITIALLY DEFERRED` constraints (only `NO ACTION` honors deferral). Uninstall deleted file rows first, in autocommit statements. ## Fix `ApplicationService.delete()` now runs in a single transaction: 1. Clear `packageJsonFileId` / `yarnLockFileId` (breaks the FK cycle) 2. Delete the app's `file` rows 3. Delete the `application` row Storage cleanup moved after commit and made non-fatal, so a failure can no longer leave partial state. `deleteApplicationFiles` is split into `deleteApplicationFileRows` (DB, transactional) and `deleteApplicationFilesFromStorage` (blobs). The test cleanup util had the same file-first ordering bug and is fixed the same way. ## Questions / Follow-ups - **Should the FK cycle be resolved at the schema level?** Both legs could be switched to `ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED`, which appears to be what the deferrable-FK migration intended — deferral would then actually apply to deletes, making transactional deletion order-independent. Happy to open a separate PR if there's interest. - **Should the marketplace install path set the package file FKs?** It stores `package.json` in the `file` table but never populates `application.packageJsonFileId` / `yarnLockFileId` — today only workspace creation and `application:rebuild-default-deps` set them. Marketplace packages also don't ship a `yarn.lock`, so this needs a product decision. Review in cubic --- .../application/application.service.ts | 71 +++++++++--- .../file-storage/file-storage.service.ts | 29 +++-- ...-with-package-file-fks.integration-spec.ts | 105 ++++++++++++++++++ ...p-application-and-app-registration.util.ts | 7 ++ 4 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/successful-uninstall-application-with-package-file-fks.integration-spec.ts 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