From 41becaaea46e3786a5363a3de1415ad942b23936 Mon Sep 17 00:00:00 2001 From: Weiko Date: Wed, 2 Jul 2025 19:21:26 +0200 Subject: [PATCH] Refactor migration runner within transaction (#12941) Modifying the data-model can sometimes fail in the middle of your operation, due to the way we handle both metadata update and schema migration separately, a field can be created while the associated column creation failed (same for object/table and such). This is also an issue because WorkspaceMigrations are then stored as FAILED can never really recovered by themselves so the schema is broken and we can't update the models anymore. This PR adds a executeMigrationFromPendingMigrationsWithinTransaction method where we can (and must) pass a queryRunner executing a transaction, which should come from the metadata services so that if anything during metadata update OR schema update fails, it rolls back everything (this also mean a workspaceMigration should never stay in a failed state now). This also fixes some issues with migration not running in the correct order due to having the same timestamp and having to do some weird logic to fix that. This is a first step and fix before working on a much more reliable solution in the upcoming weeks where we will refactor the way we interact with the data model. --------- Co-authored-by: Charles Bochet --- .../src/database/typeorm/typeorm.service.ts | 40 +- .../field-metadata/field-metadata.service.ts | 143 ++-- .../index-metadata/index-metadata.service.ts | 91 ++- .../object-metadata/object-metadata.module.ts | 2 + .../object-metadata.service.ts | 687 ++++++++++-------- .../object-metadata-field-relation.service.ts | 39 +- .../object-metadata-migration.service.ts | 21 +- .../search-vector/search-vector.service.ts | 47 +- .../workspace-migration.service.ts | 19 +- .../workspace-migration-runner.service.ts | 88 ++- .../workspace-sync-metadata.service.ts | 8 +- ...ct-records-permissions.integration-spec.ts | 24 +- ...ct-records-permissions.integration-spec.ts | 16 +- ...ct-records-permissions.integration-spec.ts | 8 +- 14 files changed, 726 insertions(+), 507 deletions(-) diff --git a/packages/twenty-server/src/database/typeorm/typeorm.service.ts b/packages/twenty-server/src/database/typeorm/typeorm.service.ts index 384dfd91b..574e9034c 100644 --- a/packages/twenty-server/src/database/typeorm/typeorm.service.ts +++ b/packages/twenty-server/src/database/typeorm/typeorm.service.ts @@ -2,55 +2,23 @@ import { Injectable, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; import { DataSource } from 'typeorm'; -import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; -import { ApprovedAccessDomain } from 'src/engine/core-modules/approved-access-domain/approved-access-domain.entity'; -import { BillingCustomer } from 'src/engine/core-modules/billing/entities/billing-customer.entity'; -import { BillingEntitlement } from 'src/engine/core-modules/billing/entities/billing-entitlement.entity'; -import { BillingMeter } from 'src/engine/core-modules/billing/entities/billing-meter.entity'; -import { BillingPrice } from 'src/engine/core-modules/billing/entities/billing-price.entity'; -import { BillingProduct } from 'src/engine/core-modules/billing/entities/billing-product.entity'; -import { BillingSubscriptionItem } from 'src/engine/core-modules/billing/entities/billing-subscription-item.entity'; -import { BillingSubscription } from 'src/engine/core-modules/billing/entities/billing-subscription.entity'; -import { FeatureFlag } from 'src/engine/core-modules/feature-flag/feature-flag.entity'; -import { KeyValuePair } from 'src/engine/core-modules/key-value-pair/key-value-pair.entity'; -import { PostgresCredentials } from 'src/engine/core-modules/postgres-credentials/postgres-credentials.entity'; -import { WorkspaceSSOIdentityProvider } from 'src/engine/core-modules/sso/workspace-sso-identity-provider.entity'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; -import { TwoFactorMethod } from 'src/engine/core-modules/two-factor-method/two-factor-method.entity'; -import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; -import { User } from 'src/engine/core-modules/user/user.entity'; -import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; -import { AgentEntity } from 'src/engine/metadata-modules/agent/agent.entity'; @Injectable() export class TypeORMService implements OnModuleInit, OnModuleDestroy { private mainDataSource: DataSource; constructor(private readonly twentyConfigService: TwentyConfigService) { + const isJest = process.argv.some((arg) => arg.includes('jest')); + this.mainDataSource = new DataSource({ url: twentyConfigService.get('PG_DATABASE_URL'), type: 'postgres', logging: twentyConfigService.getLoggingConfig(), schema: 'core', entities: [ - User, - Workspace, - UserWorkspace, - AppToken, - KeyValuePair, - FeatureFlag, - BillingSubscription, - BillingSubscriptionItem, - BillingMeter, - BillingCustomer, - BillingProduct, - BillingPrice, - BillingEntitlement, - PostgresCredentials, - WorkspaceSSOIdentityProvider, - ApprovedAccessDomain, - TwoFactorMethod, - AgentEntity, + `${isJest ? '' : 'dist/'}src/engine/core-modules/**/*.entity{.ts,.js}`, + `${isJest ? '' : 'dist/'}src/engine/metadata-modules/**/*.entity{.ts,.js}`, ], metadataTableName: '_typeorm_generated_columns_and_materialized_views', ssl: twentyConfigService.get('PG_SSL_ALLOW_SELF_SIGNED') diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts index d082cafc1..927181427 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts @@ -158,6 +158,11 @@ export class FieldMetadataService extends TypeOrmQueryService( + FieldMetadataEntity, + ); + if ( !isDefined( objectMetadataItemWithFieldMaps.labelIdentifierFieldMetadataId, @@ -224,10 +229,9 @@ export class FieldMetadataService extends TypeOrmQueryService) @@ -402,6 +406,7 @@ export class FieldMetadataService extends TypeOrmQueryService[], - isUnique: boolean, - isCustom: boolean, - indexType?: IndexType, - indexWhereClause?: string, - ) { + async createIndexMetadata({ + workspaceId, + objectMetadata, + fieldMetadataToIndex, + isUnique, + isCustom, + indexType, + indexWhereClause, + queryRunner, + }: { + workspaceId: string; + objectMetadata: ObjectMetadataEntity; + fieldMetadataToIndex: Partial[]; + isUnique: boolean; + isCustom: boolean; + indexType?: IndexType; + indexWhereClause?: string; + queryRunner?: QueryRunner; + }) { const tableName = computeObjectTargetTable(objectMetadata); const columnNames: string[] = fieldMetadataToIndex.map( @@ -53,7 +63,11 @@ export class IndexMetadataService { let result: IndexMetadataEntity; - const existingIndex = await this.indexMetadataRepository.findOne({ + const indexMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(IndexMetadataEntity) + : this.indexMetadataRepository; + + const existingIndex = await indexMetadataRepository.findOne({ where: { name: indexName, workspaceId, @@ -68,7 +82,7 @@ export class IndexMetadataService { } try { - result = await this.indexMetadataRepository.save({ + result = await indexMetadataRepository.save({ name: indexName, indexFieldMetadatas: fieldMetadataToIndex.map( (fieldMetadata, index) => ({ @@ -93,15 +107,15 @@ export class IndexMetadataService { ); } - await this.createIndexCreationMigration( + await this.createIndexCreationMigration({ workspaceId, objectMetadata, fieldMetadataToIndex, isUnique, - isCustom, indexType, indexWhereClause, - ); + queryRunner, + }); } async recomputeIndexMetadataForObject( @@ -110,8 +124,13 @@ export class IndexMetadataService { ObjectMetadataEntity, 'nameSingular' | 'isCustom' | 'id' >, + queryRunner?: QueryRunner, ) { - const indexesToRecompute = await this.indexMetadataRepository.find({ + const indexMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(IndexMetadataEntity) + : this.indexMetadataRepository; + + const indexesToRecompute = await indexMetadataRepository.find({ where: { objectMetadataId: updatedObjectMetadata.id, workspaceId, @@ -142,7 +161,7 @@ export class IndexMetadataService { ...columnNames, ])}`; - await this.indexMetadataRepository.update(index.id, { + await indexMetadataRepository.update(index.id, { name: newIndexName, }); @@ -160,6 +179,7 @@ export class IndexMetadataService { workspaceId: string, objectMetadata: ObjectMetadataEntity, fieldMetadataToIndex: Partial[], + queryRunner?: QueryRunner, ) { const tableName = computeObjectTargetTable(objectMetadata); @@ -173,7 +193,11 @@ export class IndexMetadataService { const indexName = `IDX_${generateDeterministicIndexName([tableName, ...columnNames])}`; - const indexMetadata = await this.indexMetadataRepository.findOne({ + const indexMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(IndexMetadataEntity) + : this.indexMetadataRepository; + + const indexMetadata = await indexMetadataRepository.findOne({ where: { name: indexName, objectMetadataId: objectMetadata.id, @@ -186,7 +210,7 @@ export class IndexMetadataService { } try { - await this.indexMetadataRepository.delete(indexMetadata.id); + await indexMetadataRepository.delete(indexMetadata.id); } catch (error) { throw new Error( `Failed to delete index metadata with name ${indexName} (error: ${error.message})`, @@ -194,15 +218,23 @@ export class IndexMetadataService { } } - async createIndexCreationMigration( - workspaceId: string, - objectMetadata: ObjectMetadataEntity, - fieldMetadataToIndex: Partial[], - isUnique: boolean, - isCustom: boolean, - indexType?: IndexType, - indexWhereClause?: string, - ) { + async createIndexCreationMigration({ + workspaceId, + objectMetadata, + fieldMetadataToIndex, + isUnique, + indexType, + indexWhereClause, + queryRunner, + }: { + workspaceId: string; + objectMetadata: ObjectMetadataEntity; + fieldMetadataToIndex: Partial[]; + isUnique: boolean; + indexType?: IndexType; + indexWhereClause?: string; + queryRunner?: QueryRunner; + }) { const tableName = computeObjectTargetTable(objectMetadata); const columnNames: string[] = fieldMetadataToIndex.map( @@ -230,6 +262,7 @@ export class IndexMetadataService { generateMigrationName(`create-${objectMetadata.nameSingular}-index`), workspaceId, [migration], + queryRunner, ); } @@ -244,6 +277,7 @@ export class IndexMetadataService { previousName: string; newName: string; }[], + queryRunner?: QueryRunner, ) { for (const recomputedIndex of recomputedIndexes) { const { previousName, newName, indexMetadata } = recomputedIndex; @@ -283,6 +317,7 @@ export class IndexMetadataService { generateMigrationName(`update-${objectMetadata.nameSingular}-index`), workspaceId, [migration], + queryRunner, ); } } diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts index aa69cea79..0cb9ab9e5 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts @@ -31,6 +31,7 @@ import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/work import { WorkspaceMigrationModule } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.module'; import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; +import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module'; import { WorkspaceMigrationRunnerModule } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.module'; import { ObjectMetadataEntity } from './object-metadata.entity'; @@ -61,6 +62,7 @@ import { UpdateObjectPayload } from './dtos/update-object.input'; WorkspacePermissionsCacheModule, WorkspaceCacheStorageModule, WorkspaceMetadataCacheModule, + WorkspaceDataSourceModule, ], services: [ ObjectMetadataService, diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts index c7341aec7..d8981d6a1 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts @@ -6,7 +6,13 @@ import { Query, QueryOptions } from '@ptc-org/nestjs-query-core'; import { TypeOrmQueryService } from '@ptc-org/nestjs-query-typeorm'; import { APP_LOCALES } from 'twenty-shared/translations'; import { capitalize, isDefined } from 'twenty-shared/utils'; -import { FindManyOptions, FindOneOptions, In, Repository } from 'typeorm'; +import { + FindManyOptions, + FindOneOptions, + In, + QueryRunner, + Repository, +} from 'typeorm'; import { generateMessageId } from 'src/engine/core-modules/i18n/utils/generateMessageId'; import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; @@ -31,7 +37,6 @@ import { validateObjectMetadataInputLabelsOrThrow, validateObjectMetadataInputNamesOrThrow, } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util'; -import { RemoteTableRelationsService } from 'src/engine/metadata-modules/remote-server/remote-table/remote-table-relations/remote-table-relations.service'; import { SearchVectorService } from 'src/engine/metadata-modules/search-vector/search-vector.service'; import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; import { validateMetadataIdentifierFieldMetadataIds } from 'src/engine/metadata-modules/utils/validate-metadata-identifier-field-metadata-id.utils'; @@ -41,6 +46,7 @@ import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/works import { WorkspaceMetadataVersionService } from 'src/engine/metadata-modules/workspace-metadata-version/services/workspace-metadata-version.service'; import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service'; import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; +import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service'; import { WorkspaceMigrationRunnerService } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service'; import { CUSTOM_OBJECT_STANDARD_FIELD_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids'; import { isSearchableFieldType } from 'src/engine/workspace-manager/workspace-sync-metadata/utils/is-searchable-field.util'; @@ -55,10 +61,6 @@ export class ObjectMetadataService extends TypeOrmQueryService, - @InjectRepository(FieldMetadataEntity, 'core') - private readonly fieldMetadataRepository: Repository, - - private readonly remoteTableRelationsService: RemoteTableRelationsService, private readonly dataSourceService: DataSourceService, private readonly workspaceMetadataCacheService: WorkspaceMetadataCacheService, private readonly workspaceMigrationRunnerService: WorkspaceMigrationRunnerService, @@ -69,6 +71,7 @@ export class ObjectMetadataService extends TypeOrmQueryService { - const { objectMetadataMaps } = - await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps( + const mainDataSource = + await this.workspaceDataSourceService.connectToMainDataSource(); + const queryRunner = mainDataSource.createQueryRunner(); + + await queryRunner.connect(); + await queryRunner.startTransaction(); + + try { + const objectMetadataRepository = + queryRunner.manager.getRepository(ObjectMetadataEntity); + + const { objectMetadataMaps } = + await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps( + { + workspaceId: objectMetadataInput.workspaceId, + }, + ); + + const lastDataSourceMetadata = + await this.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail( + objectMetadataInput.workspaceId, + ); + + objectMetadataInput.labelSingular = capitalize( + objectMetadataInput.labelSingular, + ); + objectMetadataInput.labelPlural = capitalize( + objectMetadataInput.labelPlural, + ); + + validateObjectMetadataInputNamesOrThrow(objectMetadataInput); + validateObjectMetadataInputLabelsOrThrow(objectMetadataInput); + + validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ + inputs: [ + objectMetadataInput.nameSingular, + objectMetadataInput.namePlural, + ], + message: + 'The singular and plural names cannot be the same for an object', + }); + validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ + inputs: [ + objectMetadataInput.labelPlural, + objectMetadataInput.labelSingular, + ], + message: + 'The singular and plural labels cannot be the same for an object', + }); + + if (objectMetadataInput.isLabelSyncedWithName === true) { + validateNameAndLabelAreSyncOrThrow( + objectMetadataInput.labelSingular, + objectMetadataInput.nameSingular, + ); + validateNameAndLabelAreSyncOrThrow( + objectMetadataInput.labelPlural, + objectMetadataInput.namePlural, + ); + } + + validatesNoOtherObjectWithSameNameExistsOrThrows({ + objectMetadataNamePlural: objectMetadataInput.namePlural, + objectMetadataNameSingular: objectMetadataInput.nameSingular, + objectMetadataMaps, + }); + + const baseCustomFields = buildDefaultFieldsForCustomObject( + objectMetadataInput.workspaceId, + ); + + const labelIdentifierFieldMetadataId = baseCustomFields.find( + (field) => field.standardId === CUSTOM_OBJECT_STANDARD_FIELD_IDS.name, + )?.id; + + if (!labelIdentifierFieldMetadataId) { + throw new ObjectMetadataException( + 'Label identifier field metadata not created properly', + ObjectMetadataExceptionCode.MISSING_CUSTOM_OBJECT_DEFAULT_LABEL_IDENTIFIER_FIELD, + ); + } + + const createdObjectMetadata = await objectMetadataRepository.save({ + ...objectMetadataInput, + dataSourceId: lastDataSourceMetadata.id, + targetTableName: 'DEPRECATED', + isActive: true, + isCustom: !objectMetadataInput.isRemote, + isSystem: false, + isRemote: objectMetadataInput.isRemote, + isSearchable: !objectMetadataInput.isRemote, + fields: objectMetadataInput.isRemote ? [] : baseCustomFields, + labelIdentifierFieldMetadataId, + }); + + if (objectMetadataInput.isRemote) { + throw new Error('Remote objects are not supported yet'); + } else { + const createdRelatedObjectMetadataCollection = + await this.objectMetadataFieldRelationService.createRelationsAndForeignKeysMetadata( + objectMetadataInput.workspaceId, + createdObjectMetadata, + objectMetadataMaps, + queryRunner, + ); + + await this.objectMetadataMigrationService.createTableMigration( + createdObjectMetadata, + queryRunner, + ); + + await this.objectMetadataMigrationService.createColumnsMigrations( + createdObjectMetadata, + createdObjectMetadata.fields, + queryRunner, + ); + + await this.objectMetadataMigrationService.createRelationMigrations( + createdObjectMetadata, + createdRelatedObjectMetadataCollection, + queryRunner, + ); + + await this.searchVectorService.createSearchVectorFieldForObject( + objectMetadataInput, + createdObjectMetadata, + queryRunner, + ); + } + + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrationsWithinTransaction( + createdObjectMetadata.workspaceId, + queryRunner, + ); + + await queryRunner.commitTransaction(); + + // After commit, do non-transactional work + await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache( { workspaceId: objectMetadataInput.workspaceId, }, ); + await this.objectMetadataRelatedRecordsService.createObjectRelatedRecords( + createdObjectMetadata, + ); - const lastDataSourceMetadata = - await this.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail( + await this.workspaceMetadataVersionService.incrementMetadataVersion( objectMetadataInput.workspaceId, ); - objectMetadataInput.labelSingular = capitalize( - objectMetadataInput.labelSingular, - ); - objectMetadataInput.labelPlural = capitalize( - objectMetadataInput.labelPlural, - ); - - validateObjectMetadataInputNamesOrThrow(objectMetadataInput); - validateObjectMetadataInputLabelsOrThrow(objectMetadataInput); - - validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ - inputs: [ - objectMetadataInput.nameSingular, - objectMetadataInput.namePlural, - ], - message: 'The singular and plural names cannot be the same for an object', - }); - validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ - inputs: [ - objectMetadataInput.labelPlural, - objectMetadataInput.labelSingular, - ], - message: - 'The singular and plural labels cannot be the same for an object', - }); - - if (objectMetadataInput.isLabelSyncedWithName === true) { - validateNameAndLabelAreSyncOrThrow( - objectMetadataInput.labelSingular, - objectMetadataInput.nameSingular, - ); - validateNameAndLabelAreSyncOrThrow( - objectMetadataInput.labelPlural, - objectMetadataInput.namePlural, - ); + return createdObjectMetadata; + } catch (error) { + if (queryRunner.isTransactionActive) { + await queryRunner.rollbackTransaction(); + } + throw error; + } finally { + await queryRunner.release(); } - - validatesNoOtherObjectWithSameNameExistsOrThrows({ - objectMetadataNamePlural: objectMetadataInput.namePlural, - objectMetadataNameSingular: objectMetadataInput.nameSingular, - objectMetadataMaps, - }); - - const baseCustomFields = buildDefaultFieldsForCustomObject( - objectMetadataInput.workspaceId, - ); - - const labelIdentifierFieldMetadataId = baseCustomFields.find( - (field) => field.standardId === CUSTOM_OBJECT_STANDARD_FIELD_IDS.name, - )?.id; - - if (!labelIdentifierFieldMetadataId) { - throw new ObjectMetadataException( - 'Label identifier field metadata not created properly', - ObjectMetadataExceptionCode.MISSING_CUSTOM_OBJECT_DEFAULT_LABEL_IDENTIFIER_FIELD, - ); - } - - const createdObjectMetadata = await this.objectMetadataRepository.save({ - ...objectMetadataInput, - dataSourceId: lastDataSourceMetadata.id, - targetTableName: 'DEPRECATED', - isActive: true, - isCustom: !objectMetadataInput.isRemote, - isSystem: false, - isRemote: objectMetadataInput.isRemote, - isSearchable: !objectMetadataInput.isRemote, - fields: objectMetadataInput.isRemote ? [] : baseCustomFields, - labelIdentifierFieldMetadataId, - }); - - if (objectMetadataInput.isRemote) { - await this.remoteTableRelationsService.createForeignKeysMetadataAndMigrations( - objectMetadataInput.workspaceId, - createdObjectMetadata, - objectMetadataInput.primaryKeyFieldMetadataSettings, - objectMetadataInput.primaryKeyColumnType, - ); - } else { - const createdRelatedObjectMetadataCollection = - await this.objectMetadataFieldRelationService.createRelationsAndForeignKeysMetadata( - objectMetadataInput.workspaceId, - createdObjectMetadata, - objectMetadataMaps, - ); - - await this.objectMetadataMigrationService.createTableMigration( - createdObjectMetadata, - ); - - await this.objectMetadataMigrationService.createColumnsMigrations( - createdObjectMetadata, - createdObjectMetadata.fields, - ); - - await this.objectMetadataMigrationService.createRelationMigrations( - createdObjectMetadata, - createdRelatedObjectMetadataCollection, - ); - - await this.searchVectorService.createSearchVectorFieldForObject( - objectMetadataInput, - createdObjectMetadata, - ); - } - - await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( - createdObjectMetadata.workspaceId, - ); - - await this.objectMetadataRelatedRecordsService.createObjectRelatedRecords( - createdObjectMetadata, - ); - - await this.workspaceMetadataVersionService.incrementMetadataVersion( - objectMetadataInput.workspaceId, - ); - - await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({ - workspaceId: objectMetadataInput.workspaceId, - }); - - return createdObjectMetadata; } public async updateOneObject( input: UpdateOneObjectInput, workspaceId: string, ): Promise { - const { objectMetadataMaps } = - await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps( - { workspaceId }, - ); + const mainDataSource = + await this.workspaceDataSourceService.connectToMainDataSource(); + const queryRunner = mainDataSource.createQueryRunner(); - const inputId = input.id; + await queryRunner.connect(); + await queryRunner.startTransaction(); - const inputPayload = { - ...input.update, - ...(isDefined(input.update.labelSingular) - ? { labelSingular: capitalize(input.update.labelSingular) } - : {}), - ...(isDefined(input.update.labelPlural) - ? { labelPlural: capitalize(input.update.labelPlural) } - : {}), - }; + try { + const objectMetadataRepository = + queryRunner.manager.getRepository(ObjectMetadataEntity); - validateObjectMetadataInputNamesOrThrow(inputPayload); + const { objectMetadataMaps } = + await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps( + { workspaceId }, + ); + const inputId = input.id; + const inputPayload = { + ...input.update, + ...(isDefined(input.update.labelSingular) + ? { labelSingular: capitalize(input.update.labelSingular) } + : {}), + ...(isDefined(input.update.labelPlural) + ? { labelPlural: capitalize(input.update.labelPlural) } + : {}), + }; - const existingObjectMetadata = objectMetadataMaps.byId[inputId]; + validateObjectMetadataInputNamesOrThrow(inputPayload); + const existingObjectMetadata = objectMetadataMaps.byId[inputId]; - if (!existingObjectMetadata) { - throw new ObjectMetadataException( - 'Object does not exist', - ObjectMetadataExceptionCode.OBJECT_METADATA_NOT_FOUND, - ); - } + if (!existingObjectMetadata) { + throw new ObjectMetadataException( + 'Object does not exist', + ObjectMetadataExceptionCode.OBJECT_METADATA_NOT_FOUND, + ); + } + const existingObjectMetadataCombinedWithUpdateInput = { + ...existingObjectMetadata, + ...inputPayload, + }; - const existingObjectMetadataCombinedWithUpdateInput = { - ...existingObjectMetadata, - ...inputPayload, - }; - - validatesNoOtherObjectWithSameNameExistsOrThrows({ - objectMetadataNameSingular: - existingObjectMetadataCombinedWithUpdateInput.nameSingular, - objectMetadataNamePlural: - existingObjectMetadataCombinedWithUpdateInput.namePlural, - existingObjectMetadataId: - existingObjectMetadataCombinedWithUpdateInput.id, - objectMetadataMaps, - }); - - if (existingObjectMetadataCombinedWithUpdateInput.isLabelSyncedWithName) { - validateNameAndLabelAreSyncOrThrow( - existingObjectMetadataCombinedWithUpdateInput.labelSingular, - existingObjectMetadataCombinedWithUpdateInput.nameSingular, - ); - validateNameAndLabelAreSyncOrThrow( - existingObjectMetadataCombinedWithUpdateInput.labelPlural, - existingObjectMetadataCombinedWithUpdateInput.namePlural, - ); - } - - if ( - isDefined(inputPayload.nameSingular) || - isDefined(inputPayload.namePlural) - ) { - validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ - inputs: [ + validatesNoOtherObjectWithSameNameExistsOrThrows({ + objectMetadataNameSingular: existingObjectMetadataCombinedWithUpdateInput.nameSingular, + objectMetadataNamePlural: existingObjectMetadataCombinedWithUpdateInput.namePlural, - ], - message: - 'The singular and plural names cannot be the same for an object', + existingObjectMetadataId: + existingObjectMetadataCombinedWithUpdateInput.id, + objectMetadataMaps, }); - } - - validateMetadataIdentifierFieldMetadataIds({ - fieldMetadataItems: Object.values(existingObjectMetadata.fieldsById), - labelIdentifierFieldMetadataId: - inputPayload.labelIdentifierFieldMetadataId, - imageIdentifierFieldMetadataId: - inputPayload.imageIdentifierFieldMetadataId, - }); - - const updatedObject = await super.updateOne(inputId, inputPayload); - - await this.handleObjectNameAndLabelUpdates( - existingObjectMetadata, - existingObjectMetadataCombinedWithUpdateInput, - inputPayload, - ); - - await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( - workspaceId, - ); - if (inputPayload.labelIdentifierFieldMetadataId) { - const labelIdentifierFieldMetadata = - await this.fieldMetadataRepository.findOneByOrFail({ - id: inputPayload.labelIdentifierFieldMetadataId, - objectMetadataId: inputId, - workspaceId: workspaceId, - }); - - if (isSearchableFieldType(labelIdentifierFieldMetadata.type)) { - await this.searchVectorService.updateSearchVector( - inputId, - [ - { - name: labelIdentifierFieldMetadata.name, - type: labelIdentifierFieldMetadata.type, - }, + if (existingObjectMetadataCombinedWithUpdateInput.isLabelSyncedWithName) { + validateNameAndLabelAreSyncOrThrow( + existingObjectMetadataCombinedWithUpdateInput.labelSingular, + existingObjectMetadataCombinedWithUpdateInput.nameSingular, + ); + validateNameAndLabelAreSyncOrThrow( + existingObjectMetadataCombinedWithUpdateInput.labelPlural, + existingObjectMetadataCombinedWithUpdateInput.namePlural, + ); + } + if ( + isDefined(inputPayload.nameSingular) || + isDefined(inputPayload.namePlural) + ) { + validateLowerCasedAndTrimmedStringsAreDifferentOrThrow({ + inputs: [ + existingObjectMetadataCombinedWithUpdateInput.nameSingular, + existingObjectMetadataCombinedWithUpdateInput.namePlural, ], + message: + 'The singular and plural names cannot be the same for an object', + }); + } + validateMetadataIdentifierFieldMetadataIds({ + fieldMetadataItems: Object.values(existingObjectMetadata.fieldsById), + labelIdentifierFieldMetadataId: + inputPayload.labelIdentifierFieldMetadataId, + imageIdentifierFieldMetadataId: + inputPayload.imageIdentifierFieldMetadataId, + }); + const updatedObject = await objectMetadataRepository.save({ + ...existingObjectMetadata, + ...inputPayload, + }); + + const { didUpdateLabelOrIcon } = + await this.handleObjectNameAndLabelUpdates( + existingObjectMetadata, + existingObjectMetadataCombinedWithUpdateInput, + inputPayload, + queryRunner, + ); + + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrationsWithinTransaction( + workspaceId, + queryRunner, + ); + if (inputPayload.labelIdentifierFieldMetadataId) { + const labelIdentifierFieldMetadata = + existingObjectMetadata.fieldsById[ + inputPayload.labelIdentifierFieldMetadataId + ]; + + if (isSearchableFieldType(labelIdentifierFieldMetadata.type)) { + await this.searchVectorService.updateSearchVector( + inputId, + [ + { + name: labelIdentifierFieldMetadata.name, + type: labelIdentifierFieldMetadata.type, + }, + ], + workspaceId, + queryRunner, + ); + } + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrationsWithinTransaction( + workspaceId, + queryRunner, + ); + } + + await queryRunner.commitTransaction(); + + // After commit, do non-transactional work + await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache( + { + workspaceId, + }, + ); + + if (didUpdateLabelOrIcon) { + await this.objectMetadataRelatedRecordsService.updateObjectViews( + updatedObject, workspaceId, ); } - await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( + await this.workspaceMetadataVersionService.incrementMetadataVersion( workspaceId, ); + + const formattedUpdatedObject = { + ...updatedObject, + createdAt: new Date(updatedObject.createdAt), + }; + + return formattedUpdatedObject; + } catch (error) { + if (queryRunner.isTransactionActive) { + await queryRunner.rollbackTransaction(); + } + throw error; + } finally { + await queryRunner.release(); } - - await this.workspaceMetadataVersionService.incrementMetadataVersion( - workspaceId, - ); - - return updatedObject; } public async deleteOneObject( input: DeleteOneObjectInput, workspaceId: string, - ): Promise { - const objectMetadata = await this.objectMetadataRepository.findOne({ - relations: [ - 'fields', - 'fields.object', - 'fields.relationTargetFieldMetadata', - 'fields.relationTargetFieldMetadata.object', - ], - where: { - id: input.id, + ): Promise> { + const mainDataSource = + await this.workspaceDataSourceService.connectToMainDataSource(); + const queryRunner = mainDataSource.createQueryRunner(); + + await queryRunner.connect(); + await queryRunner.startTransaction(); + + try { + const objectMetadataRepository = + queryRunner.manager.getRepository(ObjectMetadataEntity); + const fieldMetadataRepository = + queryRunner.manager.getRepository(FieldMetadataEntity); + + const objectMetadata = await objectMetadataRepository.findOne({ + relations: [ + 'fields', + 'fields.object', + 'fields.relationTargetFieldMetadata', + 'fields.relationTargetFieldMetadata.object', + ], + where: { + id: input.id, + workspaceId, + }, + }); + + if (!objectMetadata) { + throw new ObjectMetadataException( + 'Object does not exist', + ObjectMetadataExceptionCode.OBJECT_METADATA_NOT_FOUND, + ); + } + + if (objectMetadata.isRemote) { + throw new ObjectMetadataException( + 'Remote objects are not supported yet', + ObjectMetadataExceptionCode.INVALID_OBJECT_INPUT, + ); + } else { + await this.objectMetadataMigrationService.deleteAllRelationsAndDropTable( + objectMetadata, + workspaceId, + queryRunner, + ); + } + + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrationsWithinTransaction( workspaceId, - }, - }); - - if (!objectMetadata) { - throw new ObjectMetadataException( - 'Object does not exist', - ObjectMetadataExceptionCode.OBJECT_METADATA_NOT_FOUND, + queryRunner, ); - } - if (objectMetadata.isRemote) { - await this.remoteTableRelationsService.deleteForeignKeysMetadataAndCreateMigrations( - objectMetadata.workspaceId, - objectMetadata, + const fieldMetadataIds = objectMetadata.fields.map((field) => field.id); + const relationMetadataIds = objectMetadata.fields + .map((field) => field.relationTargetFieldMetadata?.id) + .filter(isDefined); + + await fieldMetadataRepository.delete({ + id: In(fieldMetadataIds.concat(relationMetadataIds)), + }); + + await objectMetadataRepository.delete(objectMetadata.id); + + await queryRunner.commitTransaction(); + + // After commit, do non-transactional work + await this.workspaceMetadataVersionService.incrementMetadataVersion( + workspaceId, ); - } else { - await this.objectMetadataMigrationService.deleteAllRelationsAndDropTable( + + await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache( + { + workspaceId, + }, + ); + + await this.objectMetadataRelatedRecordsService.deleteObjectViews( objectMetadata, workspaceId, ); + + return objectMetadata; + } catch (error) { + if (queryRunner.isTransactionActive) { + await queryRunner.rollbackTransaction(); + } + throw error; + } finally { + await queryRunner.release(); } - - await this.objectMetadataRelatedRecordsService.deleteObjectViews( - objectMetadata, - workspaceId, - ); - - await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( - workspaceId, - ); - - const fieldMetadataIds = objectMetadata.fields.map((field) => field.id); - const relationMetadataIds = objectMetadata.fields - .map((field) => field.relationTargetFieldMetadata?.id) - .filter(isDefined); - - await this.fieldMetadataRepository.delete({ - id: In(fieldMetadataIds.concat(relationMetadataIds)), - }); - - await this.objectMetadataRepository.delete(objectMetadata.id); - - await this.workspaceMetadataVersionService.incrementMetadataVersion( - workspaceId, - ); - - await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({ - workspaceId, - }); - - return objectMetadata; } public async findOneWithinWorkspace( @@ -476,7 +569,8 @@ export class ObjectMetadataService extends TypeOrmQueryService, inputPayload: UpdateObjectPayload, - ) { + queryRunner: QueryRunner, + ): Promise<{ didUpdateLabelOrIcon: boolean }> { const newTargetTableName = computeObjectTargetTable( objectMetadataForUpdate, ); @@ -489,12 +583,14 @@ export class ObjectMetadataService extends TypeOrmQueryService, objectMetadataMaps: ObjectMetadataMaps, + queryRunner?: QueryRunner, ) { const relatedObjectMetadataCollection = await Promise.all( DEFAULT_RELATIONS_OBJECTS_STANDARD_IDS.map( @@ -57,6 +58,7 @@ export class ObjectMetadataFieldRelationService { sourceObjectMetadata, relationObjectMetadataStandardId, objectMetadataMaps, + queryRunner, }), ), ); @@ -69,6 +71,7 @@ export class ObjectMetadataFieldRelationService { sourceObjectMetadata, relationObjectMetadataStandardId, objectMetadataMaps, + queryRunner, }: { workspaceId: string; sourceObjectMetadata: Pick< @@ -77,6 +80,7 @@ export class ObjectMetadataFieldRelationService { >; objectMetadataMaps: ObjectMetadataMaps; relationObjectMetadataStandardId: string; + queryRunner?: QueryRunner; }) { const targetObjectMetadata = Object.values(objectMetadataMaps.byId).find( (objectMetadata) => @@ -93,6 +97,7 @@ export class ObjectMetadataFieldRelationService { workspaceId, sourceObjectMetadata, targetObjectMetadata, + queryRunner, ); return targetObjectMetadata; @@ -105,6 +110,7 @@ export class ObjectMetadataFieldRelationService { 'id' | 'nameSingular' | 'labelSingular' >, targetObjectMetadata: ObjectMetadataItemWithFieldMaps, + queryRunner?: QueryRunner, ): Promise[]> { const sourceFieldMetadata = this.createSourceFieldMetadata( workspaceId, @@ -118,7 +124,11 @@ export class ObjectMetadataFieldRelationService { targetObjectMetadata, ); - return this.fieldMetadataRepository.save([ + const fieldMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(FieldMetadataEntity) + : this.fieldMetadataRepository; + + return fieldMetadataRepository.save([ { ...sourceFieldMetadata, settings: { @@ -146,6 +156,7 @@ export class ObjectMetadataFieldRelationService { ObjectMetadataEntity, 'nameSingular' | 'isCustom' | 'id' | 'labelSingular' >, + queryRunner?: QueryRunner, ): Promise< { targetObjectMetadata: ObjectMetadataEntity; @@ -160,6 +171,7 @@ export class ObjectMetadataFieldRelationService { workspaceId, updatedObjectMetadata, relationObjectMetadataStandardId, + queryRunner, ), ), ); @@ -172,20 +184,29 @@ export class ObjectMetadataFieldRelationService { 'nameSingular' | 'id' | 'isCustom' | 'labelSingular' >, targetObjectMetadataStandardId: string, + queryRunner?: QueryRunner, ) { - const targetObjectMetadata = - await this.objectMetadataRepository.findOneByOrFail({ + const objectMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(ObjectMetadataEntity) + : this.objectMetadataRepository; + const fieldMetadataRepository = queryRunner + ? queryRunner.manager.getRepository(FieldMetadataEntity) + : this.fieldMetadataRepository; + + const targetObjectMetadata = await objectMetadataRepository.findOneByOrFail( + { standardId: targetObjectMetadataStandardId, workspaceId: workspaceId, isCustom: false, - }); + }, + ); const targetFieldMetadataUpdateData = this.updateTargetFieldMetadata( sourceObjectMetadata, targetObjectMetadata, ); const targetFieldMetadataToUpdate = - await this.fieldMetadataRepository.findOneByOrFail({ + await fieldMetadataRepository.findOneByOrFail({ standardId: createRelationDeterministicUuid({ objectId: sourceObjectMetadata.id, standardId: @@ -201,7 +222,7 @@ export class ObjectMetadataFieldRelationService { targetFieldMetadataToUpdate as FieldMetadataEntity ).settings?.relationType === RelationType.MANY_TO_ONE; - const targetFieldMetadata = await this.fieldMetadataRepository.save({ + const targetFieldMetadata = await fieldMetadataRepository.save({ id: targetFieldMetadataToUpdate.id, ...targetFieldMetadataUpdateData, settings: { @@ -220,7 +241,7 @@ export class ObjectMetadataFieldRelationService { ); const sourceFieldMetadataToUpdate = - await this.fieldMetadataRepository.findOneByOrFail({ + await fieldMetadataRepository.findOneByOrFail({ standardId: // @ts-expect-error legacy noImplicitAny CUSTOM_OBJECT_STANDARD_FIELD_IDS[targetObjectMetadata.namePlural], @@ -233,7 +254,7 @@ export class ObjectMetadataFieldRelationService { sourceFieldMetadataToUpdate as FieldMetadataEntity ).settings?.relationType === RelationType.MANY_TO_ONE; - const sourceFieldMetadata = await this.fieldMetadataRepository.save({ + const sourceFieldMetadata = await fieldMetadataRepository.save({ id: sourceFieldMetadataToUpdate.id, ...sourceFieldMetadataUpdateData, settings: { diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/services/object-metadata-migration.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/services/object-metadata-migration.service.ts index 23683d271..8bdaca8f7 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/services/object-metadata-migration.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/services/object-metadata-migration.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { FieldMetadataType } from 'twenty-shared/types'; -import { Repository } from 'typeorm'; +import { QueryRunner, Repository } from 'typeorm'; import { RelationType } from 'src/engine/metadata-modules/field-metadata/interfaces/relation-type.interface'; @@ -36,6 +36,7 @@ export class ObjectMetadataMigrationService { public async createTableMigration( createdObjectMetadata: ObjectMetadataEntity, + queryRunner?: QueryRunner, ) { await this.workspaceMigrationService.createCustomMigration( generateMigrationName(`create-${createdObjectMetadata.nameSingular}`), @@ -46,12 +47,14 @@ export class ObjectMetadataMigrationService { action: WorkspaceMigrationTableActionType.CREATE, } satisfies WorkspaceMigrationTableAction, ], + queryRunner, ); } public async createColumnsMigrations( createdObjectMetadata: ObjectMetadataEntity, fieldMetadataCollection: FieldMetadataEntity[], + queryRunner?: QueryRunner, ) { await this.workspaceMigrationService.createCustomMigration( generateMigrationName( @@ -70,6 +73,7 @@ export class ObjectMetadataMigrationService { ), }, ], + queryRunner, ); } @@ -82,6 +86,7 @@ export class ObjectMetadataMigrationService { ObjectMetadataItemWithFieldMaps, 'nameSingular' | 'isCustom' >[], + queryRunner?: QueryRunner, ) { await this.workspaceMigrationService.createCustomMigration( generateMigrationName( @@ -92,6 +97,7 @@ export class ObjectMetadataMigrationService { createdObjectMetadata, relatedObjectMetadataCollection, ), + queryRunner, ); } @@ -105,6 +111,7 @@ export class ObjectMetadataMigrationService { 'nameSingular' | 'isCustom' >, workspaceId: string, + queryRunner?: QueryRunner, ) { const newTargetTableName = computeObjectTargetTable( objectMetadataForUpdate, @@ -113,7 +120,7 @@ export class ObjectMetadataMigrationService { existingObjectMetadata, ); - this.workspaceMigrationService.createCustomMigration( + await this.workspaceMigrationService.createCustomMigration( generateMigrationName(`rename-${existingObjectMetadata.nameSingular}`), workspaceId, [ @@ -123,6 +130,7 @@ export class ObjectMetadataMigrationService { action: WorkspaceMigrationTableActionType.ALTER, }, ], + queryRunner, ); } @@ -135,6 +143,7 @@ export class ObjectMetadataMigrationService { sourceFieldMetadata: FieldMetadataEntity; }[], workspaceId: string, + queryRunner?: QueryRunner, ) { for (const { targetObjectMetadata } of relationMetadataCollection) { const targetTableName = computeObjectTargetTable(targetObjectMetadata); @@ -168,6 +177,7 @@ export class ObjectMetadataMigrationService { ], }, ], + queryRunner, ); } } @@ -180,6 +190,7 @@ export class ObjectMetadataMigrationService { foreignKeyFieldMetadata: FieldMetadataEntity; }[], workspaceId: string, + queryRunner?: QueryRunner, ) { for (const { relatedObjectMetadata, @@ -221,6 +232,7 @@ export class ObjectMetadataMigrationService { ], }, ], + queryRunner, ); } } @@ -228,6 +240,7 @@ export class ObjectMetadataMigrationService { public async deleteAllRelationsAndDropTable( objectMetadata: ObjectMetadataEntity, workspaceId: string, + queryRunner?: QueryRunner, ) { const relationFields = objectMetadata.fields.filter((field) => isFieldMetadataInterfaceOfType(field, FieldMetadataType.RELATION), @@ -279,6 +292,7 @@ export class ObjectMetadataMigrationService { ], }, ], + queryRunner, ); } @@ -291,6 +305,7 @@ export class ObjectMetadataMigrationService { action: WorkspaceMigrationTableActionType.DROP, }, ], + queryRunner, ); } @@ -300,6 +315,7 @@ export class ObjectMetadataMigrationService { 'nameSingular' | 'isCustom' | 'id' | 'fieldsById' >, workspaceId: string, + queryRunner?: QueryRunner, ) { const enumFieldMetadataTypes = [ FieldMetadataType.SELECT, @@ -330,6 +346,7 @@ export class ObjectMetadataMigrationService { ), }, ], + queryRunner, ); } } diff --git a/packages/twenty-server/src/engine/metadata-modules/search-vector/search-vector.service.ts b/packages/twenty-server/src/engine/metadata-modules/search-vector/search-vector.service.ts index 93f5ce814..e61fab5c2 100644 --- a/packages/twenty-server/src/engine/metadata-modules/search-vector/search-vector.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/search-vector/search-vector.service.ts @@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { FieldMetadataType } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; -import { Repository } from 'typeorm'; +import { QueryRunner, Repository } from 'typeorm'; import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface'; @@ -46,8 +46,13 @@ export class SearchVectorService { public async createSearchVectorFieldForObject( objectMetadataInput: CreateObjectInput, createdObjectMetadata: ObjectMetadataEntity, + queryRunner?: QueryRunner, ) { - const searchVectorFieldMetadata = await this.fieldMetadataRepository.save({ + const repository = queryRunner + ? queryRunner.manager.getRepository(FieldMetadataEntity) + : this.fieldMetadataRepository; + + const searchVectorFieldMetadata = await repository.save({ standardId: CUSTOM_OBJECT_STANDARD_FIELD_IDS.searchVector, objectMetadataId: createdObjectMetadata.id, workspaceId: objectMetadataInput.workspaceId, @@ -101,24 +106,31 @@ export class SearchVectorService { } as FieldMetadataInterface), }, ], + queryRunner, ); - await this.indexMetadataService.createIndexMetadata( - objectMetadataInput.workspaceId, - createdObjectMetadata, - [searchVectorFieldMetadata], - false, - false, - IndexType.GIN, - ); + await this.indexMetadataService.createIndexMetadata({ + workspaceId: objectMetadataInput.workspaceId, + objectMetadata: createdObjectMetadata, + fieldMetadataToIndex: [searchVectorFieldMetadata], + isUnique: false, + isCustom: false, + indexType: IndexType.GIN, + queryRunner, + }); } public async updateSearchVector( objectMetadataId: string, fieldMetadataNameAndTypeForSearch: FieldTypeAndNameMetadata[], workspaceId: string, + queryRunner?: QueryRunner, ) { - const objectMetadata = await this.objectMetadataRepository.findOneByOrFail({ + const repository = queryRunner + ? queryRunner.manager.getRepository(ObjectMetadataEntity) + : this.objectMetadataRepository; + + const objectMetadata = await repository.findOneByOrFail({ id: objectMetadataId, }); @@ -152,16 +164,17 @@ export class SearchVectorService { ), }, ], + queryRunner, ); // index needs to be recreated as typeorm deletes then recreates searchVector column at alter - await this.indexMetadataService.createIndexCreationMigration( + await this.indexMetadataService.createIndexCreationMigration({ workspaceId, objectMetadata, - [existingSearchVectorFieldMetadata], - false, - false, - IndexType.GIN, - ); + fieldMetadataToIndex: [existingSearchVectorFieldMetadata], + isUnique: false, + indexType: IndexType.GIN, + queryRunner, + }); } } diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-migration/workspace-migration.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-migration/workspace-migration.service.ts index b01c56e4d..85c92e938 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-migration/workspace-migration.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-migration/workspace-migration.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { IsNull, Repository } from 'typeorm'; +import { IsNull, QueryRunner, Repository } from 'typeorm'; import { WorkspaceMigrationEntity, @@ -23,8 +23,13 @@ export class WorkspaceMigrationService { */ public async getPendingMigrations( workspaceId: string, + queryRunner?: QueryRunner, ): Promise { - const pendingMigrations = await this.workspaceMigrationRepository.find({ + const workspaceMigrationRepository = queryRunner + ? queryRunner.manager.getRepository(WorkspaceMigrationEntity) + : this.workspaceMigrationRepository; + + const pendingMigrations = await workspaceMigrationRepository.find({ order: { createdAt: 'ASC', name: 'ASC' }, where: { appliedAt: IsNull(), @@ -113,13 +118,21 @@ export class WorkspaceMigrationService { name: string, workspaceId: string, migrations: WorkspaceMigrationTableAction[], + queryRunner?: QueryRunner, ) { - return this.workspaceMigrationRepository.save({ + const workspaceMigrationRepository = queryRunner + ? queryRunner.manager.getRepository(WorkspaceMigrationEntity) + : this.workspaceMigrationRepository; + + const migration = await workspaceMigrationRepository.save({ name, migrations, workspaceId, isCustom: true, + createdAt: new Date(), }); + + return migration; } public async deleteAllWithinWorkspace(workspaceId: string) { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts index 581dbd222..dc364886f 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts @@ -32,6 +32,54 @@ export class WorkspaceMigrationRunnerService { private readonly workspaceMigrationColumnService: WorkspaceMigrationColumnService, ) {} + public async executeMigrationFromPendingMigrationsWithinTransaction( + workspaceId: string, + transactionQueryRunner: QueryRunner, + ): Promise { + const pendingMigrations = + await this.workspaceMigrationService.getPendingMigrations( + workspaceId, + transactionQueryRunner, + ); + + if (pendingMigrations.length === 0) { + return []; + } + + const migrationActionsWithParent = pendingMigrations.flatMap( + (pendingMigration) => + (pendingMigration.migrations || []).map((tableAction) => ({ + tableAction, + parentMigrationId: pendingMigration.id, + })), + ); + + const schemaName = + this.workspaceDataSourceService.getSchemaName(workspaceId); + + await transactionQueryRunner.query( + `SET LOCAL search_path TO ${schemaName}`, + ); + + for (const { + tableAction, + parentMigrationId, + } of migrationActionsWithParent) { + await this.handleTableChanges( + transactionQueryRunner as PostgresQueryRunner, + schemaName, + tableAction, + ); + + await transactionQueryRunner.query( + `UPDATE "core"."workspaceMigration" SET "appliedAt" = NOW() WHERE "id" = $1 AND "workspaceId" = $2`, + [parentMigrationId, workspaceId], + ); + } + + return migrationActionsWithParent.map((item) => item.tableAction); + } + public async executeMigrationFromPendingMigrations( workspaceId: string, ): Promise { @@ -42,58 +90,32 @@ export class WorkspaceMigrationRunnerService { throw new Error('Main data source not found'); } - const pendingMigrations = - await this.workspaceMigrationService.getPendingMigrations(workspaceId); - - if (pendingMigrations.length === 0) { - return []; - } - - const flattenedPendingMigrations: WorkspaceMigrationTableAction[] = - pendingMigrations.reduce((acc, pendingMigration) => { - return [...acc, ...pendingMigration.migrations]; - }, []); - const queryRunner = mainDataSource.createQueryRunner() as PostgresQueryRunner; await queryRunner.connect(); await queryRunner.startTransaction(); - const schemaName = - this.workspaceDataSourceService.getSchemaName(workspaceId); - - await queryRunner.query(`SET LOCAL search_path TO ${schemaName}`); - try { - // Loop over each migration and create or update the table - for (const migration of flattenedPendingMigrations) { - await this.handleTableChanges(queryRunner, schemaName, migration); - } + const result = + await this.executeMigrationFromPendingMigrationsWithinTransaction( + workspaceId, + queryRunner, + ); await queryRunner.commitTransaction(); + + return result; } catch (error) { this.logger.error( `Error executing migration: ${error.message}`, error.stack, ); - await queryRunner.rollbackTransaction(); throw error; } finally { await queryRunner.release(); } - - // Update appliedAt date for each migration - // TODO: Should be done after the migration is successful - for (const pendingMigration of pendingMigrations) { - await this.workspaceMigrationService.setAppliedAtForMigration( - workspaceId, - pendingMigration, - ); - } - - return flattenedPendingMigrations; } private async handleTableChanges( diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/workspace-sync-metadata.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/workspace-sync-metadata.service.ts index 4fced8501..993306956 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/workspace-sync-metadata.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/workspace-sync-metadata.service.ts @@ -191,15 +191,17 @@ export class WorkspaceSyncMetadataService { }; } - await queryRunner.commitTransaction(); - // Execute migrations this.logger.log('Executing pending migrations'); const executeMigrationsStart = performance.now(); - await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrationsWithinTransaction( context.workspaceId, + queryRunner, ); + + await queryRunner.commitTransaction(); + const executeMigrationsEnd = performance.now(); this.logger.log( diff --git a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/delete-many-object-records-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/delete-many-object-records-permissions.integration-spec.ts index 2811493b6..c30a4d68b 100644 --- a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/delete-many-object-records-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/delete-many-object-records-permissions.integration-spec.ts @@ -69,16 +69,12 @@ describe('deleteManyObjectRecordsPermissions', () => { expect(response.body.data).toBeDefined(); expect(response.body.data.deletePeople).toBeDefined(); expect(response.body.data.deletePeople).toHaveLength(2); - expect( - response.body.data.deletePeople.some( - (person: { id: string }) => person.id === personId1, - ), - ).toBe(true); - expect( - response.body.data.deletePeople.some( - (person: { id: string }) => person.id === personId2, - ), - ).toBe(true); + expect(response.body.data.deletePeople).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: personId1 }), + expect.objectContaining({ id: personId2 }), + ]), + ); }); it('should delete multiple object records when executed by api key', async () => { @@ -119,7 +115,11 @@ describe('deleteManyObjectRecordsPermissions', () => { expect(response.body.data).toBeDefined(); expect(response.body.data.deletePeople).toBeDefined(); expect(response.body.data.deletePeople).toHaveLength(2); - expect(response.body.data.deletePeople[0].id).toBe(personId1); - expect(response.body.data.deletePeople[1].id).toBe(personId2); + expect(response.body.data.deletePeople).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: personId1 }), + expect.objectContaining({ id: personId2 }), + ]), + ); }); }); diff --git a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/destroy-many-object-records-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/destroy-many-object-records-permissions.integration-spec.ts index ce7aa7df2..13459b814 100644 --- a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/destroy-many-object-records-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/destroy-many-object-records-permissions.integration-spec.ts @@ -68,15 +68,11 @@ describe('destroyManyObjectRecordsPermissions', () => { expect(response.body.data).toBeDefined(); expect(response.body.data.destroyPeople).toBeDefined(); expect(response.body.data.destroyPeople).toHaveLength(2); - expect( - response.body.data.destroyPeople.some( - (person: { id: string }) => person.id === personId1, - ), - ).toBe(true); - expect( - response.body.data.destroyPeople.some( - (person: { id: string }) => person.id === personId2, - ), - ).toBe(true); + expect(response.body.data.destroyPeople).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: personId1 }), + expect.objectContaining({ id: personId2 }), + ]), + ); }); }); diff --git a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/restore-many-object-records-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/restore-many-object-records-permissions.integration-spec.ts index d4cccbcf6..7b387b3a5 100644 --- a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/restore-many-object-records-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/restore-many-object-records-permissions.integration-spec.ts @@ -86,7 +86,11 @@ describe('restoreManyObjectRecordsPermissions', () => { expect(response.body.data).toBeDefined(); expect(response.body.data.restorePeople).toBeDefined(); expect(response.body.data.restorePeople).toHaveLength(2); - expect(response.body.data.restorePeople[0].id).toBe(personId1); - expect(response.body.data.restorePeople[1].id).toBe(personId2); + expect(response.body.data.restorePeople).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: personId1 }), + expect.objectContaining({ id: personId2 }), + ]), + ); }); });