From b49225df4bfff4b3b6c29e50b6d167d95bd582f4 Mon Sep 17 00:00:00 2001 From: martmull Date: Fri, 26 Jun 2026 10:17:00 +0200 Subject: [PATCH] Reorder validation execution to match migration action order (#22200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Reorders the validation execution sequence in the workspace entity migration builder to match the actual execution order of migration actions (delete → create → update). This ensures that optimistic entity maps accurately simulate the post-migration state during validation. ## Key Changes - **Moved creation validation before update validation** in `WorkspaceEntityMigrationBuilderService`: Creation validation now executes immediately after deletion validation, allowing updates to reference entities created in the same migration without validators needing to peek into to-be-created maps. - **Removed `remainingFlatEntityMapsToValidate` parameter from update validation**: Since creation validation now completes before update validation begins, the optimistic maps already contain all created entities. Updates can safely reference newly created entities through the optimistic maps without needing access to remaining-to-create maps. - **Simplified `FlatNavigationMenuItemValidatorService`**: Removed the logic that combined remaining-to-create maps with optimistic maps, now relying solely on the optimistic maps which contain all previously validated creations. - **Updated type definition**: Modified `FlatEntityUpdateValidationArgs` type to exclude `remainingFlatEntityMapsToValidate` since it's no longer needed. ## Implementation Details This change enables a more intuitive validation flow where: 1. Deletions are validated first 2. Creations are validated next (in topological order for self-referential FKs) 3. Updates are validated last (can safely reference newly created entities) The optimistic maps are progressively built during creation validation, so by the time update validation runs, they faithfully represent the post-migration state, eliminating the need for validators to access separate remaining-to-create maps. Review in cubic --- ...kspace-entity-migration-builder.service.ts | 178 +++++++++--------- ...flat-entity-update-validation-args.type.ts | 2 +- ...-navigation-menu-item-validator.service.ts | 14 +- 3 files changed, 91 insertions(+), 103 deletions(-) diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts index 4d1c16da83..d82a344238 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/services/workspace-entity-migration-builder.service.ts @@ -185,102 +185,13 @@ export abstract class WorkspaceEntityMigrationBuilderService< ); this.logger.perfTime( `EntityBuilder ${this.metadataName}`, - 'update validation', - ); - - for (const flatEntityToUpdateUniversalIdentifier in updatedFlatEntityMaps.byUniversalIdentifier) { - const flatEntityUpdate = - updatedFlatEntityMaps.byUniversalIdentifier[ - flatEntityToUpdateUniversalIdentifier - ]; - - if (!isDefined(flatEntityUpdate)) { - throw new FlatEntityMapsException( - 'Could not find flat entity updates in maps dispatcher should never occur', - FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND, - ); - } - - const validationResult = await this.validateFlatEntityUpdate({ - flatEntityUpdate: flatEntityUpdate.update, - optimisticFlatEntityMapsAndRelatedFlatEntityMaps, - workspaceId, - buildOptions, - additionalCacheDataMaps, - universalIdentifier: flatEntityToUpdateUniversalIdentifier, - remainingFlatEntityMapsToValidate: createdFlatEntityMaps, - }); - - if (validationResult.status === 'fail') { - allValidationResult.push(validationResult); - continue; - } - - const existingFlatEntity = findFlatEntityByUniversalIdentifier< - MetadataUniversalFlatEntity - >({ - universalIdentifier: flatEntityToUpdateUniversalIdentifier, - flatEntityMaps: - optimisticFlatEntityMapsAndRelatedFlatEntityMaps[flatEntityMapsKey], - }); - - if (!isDefined(existingFlatEntity)) { - throw new FlatEntityMapsException( - 'Existing flat entity to update post successful validation is not defined, should never occur', - FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND, - ); - } - - const updatedFlatEntity: MetadataUniversalFlatEntity = { - ...existingFlatEntity, - ...flatEntityUpdate.update, - }; - - const diff = Object.fromEntries( - Object.entries(flatEntityUpdate.update).map(([key, after]) => [ - key, - { - before: - existingFlatEntity[key as keyof MetadataUniversalFlatEntity], - after, - }, - ]), - ) as UniversalFlatEntityDiff; - - replaceUniversalFlatEntityInUniversalFlatEntityMapsThroughMutationOrThrow( - { - universalFlatEntity: updatedFlatEntity, - universalFlatEntityMapsToMutate: - optimisticFlatEntityMapsAndRelatedFlatEntityMaps[flatEntityMapsKey], - }, - ); - - actionsResult.update.push( - ...(Array.isArray(validationResult.action) - ? validationResult.action - : [validationResult.action] - ).map((action) => ({ - ...action, - flatEntity: updatedFlatEntity, - diff, - })), - ); - } - - this.logger.perfTimeEnd( - `EntityBuilder ${this.metadataName}`, - 'update validation', + 'creation validation', ); const remainingFlatEntityMapsToCreate = structuredClone( createdFlatEntityMaps, ); - this.logger.perfTime( - `EntityBuilder ${this.metadataName}`, - 'creation validation', - ); - const sortedCreateUniversalIdentifiers = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({ metadataName: this.metadataName, @@ -352,6 +263,93 @@ export abstract class WorkspaceEntityMigrationBuilderService< `EntityBuilder ${this.metadataName}`, 'creation validation', ); + this.logger.perfTime( + `EntityBuilder ${this.metadataName}`, + 'update validation', + ); + + for (const flatEntityToUpdateUniversalIdentifier in updatedFlatEntityMaps.byUniversalIdentifier) { + const flatEntityUpdate = + updatedFlatEntityMaps.byUniversalIdentifier[ + flatEntityToUpdateUniversalIdentifier + ]; + + if (!isDefined(flatEntityUpdate)) { + throw new FlatEntityMapsException( + 'Could not find flat entity updates in maps dispatcher should never occur', + FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND, + ); + } + + const validationResult = await this.validateFlatEntityUpdate({ + flatEntityUpdate: flatEntityUpdate.update, + optimisticFlatEntityMapsAndRelatedFlatEntityMaps, + workspaceId, + buildOptions, + additionalCacheDataMaps, + universalIdentifier: flatEntityToUpdateUniversalIdentifier, + }); + + if (validationResult.status === 'fail') { + allValidationResult.push(validationResult); + continue; + } + + const existingFlatEntity = findFlatEntityByUniversalIdentifier< + MetadataUniversalFlatEntity + >({ + universalIdentifier: flatEntityToUpdateUniversalIdentifier, + flatEntityMaps: + optimisticFlatEntityMapsAndRelatedFlatEntityMaps[flatEntityMapsKey], + }); + + if (!isDefined(existingFlatEntity)) { + throw new FlatEntityMapsException( + 'Existing flat entity to update post successful validation is not defined, should never occur', + FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND, + ); + } + + const updatedFlatEntity: MetadataUniversalFlatEntity = { + ...existingFlatEntity, + ...flatEntityUpdate.update, + }; + + const diff = Object.fromEntries( + Object.entries(flatEntityUpdate.update).map(([key, after]) => [ + key, + { + before: + existingFlatEntity[key as keyof MetadataUniversalFlatEntity], + after, + }, + ]), + ) as UniversalFlatEntityDiff; + + replaceUniversalFlatEntityInUniversalFlatEntityMapsThroughMutationOrThrow( + { + universalFlatEntity: updatedFlatEntity, + universalFlatEntityMapsToMutate: + optimisticFlatEntityMapsAndRelatedFlatEntityMaps[flatEntityMapsKey], + }, + ); + + actionsResult.update.push( + ...(Array.isArray(validationResult.action) + ? validationResult.action + : [validationResult.action] + ).map((action) => ({ + ...action, + flatEntity: updatedFlatEntity, + diff, + })), + ); + } + + this.logger.perfTimeEnd( + `EntityBuilder ${this.metadataName}`, + 'update validation', + ); this.logger.perfTimeEnd( `EntityBuilder ${this.metadataName}`, 'entity processing', diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts index 005da4ecd0..d1cf1a5560 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/universal-flat-entity-update-validation-args.type.ts @@ -5,7 +5,7 @@ import { type UniversalFlatEntityValidationArgs } from 'src/engine/workspace-man export type FlatEntityUpdateValidationArgs = Omit< UniversalFlatEntityValidationArgs, - 'flatEntityToValidate' + 'flatEntityToValidate' | 'remainingFlatEntityMapsToValidate' > & { flatEntityUpdate: UniversalFlatEntityUpdate; universalIdentifier: string; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts index 135119e99f..5972799eae 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-navigation-menu-item-validator.service.ts @@ -284,7 +284,6 @@ export class FlatNavigationMenuItemValidatorService { optimisticFlatEntityMapsAndRelatedFlatEntityMaps: { flatNavigationMenuItemMaps: optimisticFlatNavigationMenuItemMaps, }, - remainingFlatEntityMapsToValidate, }: FlatEntityUpdateValidationArgs< typeof ALL_METADATA_NAME.navigationMenuItem >): FailedFlatEntityValidation<'navigationMenuItem', 'update'> { @@ -353,21 +352,12 @@ export class FlatNavigationMenuItemValidatorService { const newFolderUniversalIdentifier = folderUniversalIdentifierUpdate; - const combinedFlatNavigationMenuItemMaps: MetadataUniversalFlatEntityMaps< - typeof ALL_METADATA_NAME.navigationMenuItem - > = { - byUniversalIdentifier: { - ...remainingFlatEntityMapsToValidate.byUniversalIdentifier, - ...optimisticFlatNavigationMenuItemMaps.byUniversalIdentifier, - }, - }; - const circularDependencyErrors = this.getCircularDependencyValidationErrors( { navigationMenuItemUniversalIdentifier: fromFlatNavigationMenuItem.universalIdentifier, folderUniversalIdentifier: newFolderUniversalIdentifier, - flatNavigationMenuItemMaps: combinedFlatNavigationMenuItemMaps, + flatNavigationMenuItemMaps: optimisticFlatNavigationMenuItemMaps, }, ); @@ -378,7 +368,7 @@ export class FlatNavigationMenuItemValidatorService { const referencedParentNavigationMenuItem = findFlatEntityByUniversalIdentifier({ universalIdentifier: newFolderUniversalIdentifier, - flatEntityMaps: combinedFlatNavigationMenuItemMaps, + flatEntityMaps: optimisticFlatNavigationMenuItemMaps, }); if (!isDefined(referencedParentNavigationMenuItem)) {