Route pre-2.19 upgrade commands through a legacy validate-build path (#22884)
## Problem Since the centralized metadata side-effect engine landed in v2.19, `WorkspaceMigrationValidateBuildAndRunService.validateBuildAndRunWorkspaceMigrationFromRecord` runs `metadataSideEffectEngineService.expandWithSideEffects(...)` before building. As a result every historical upgrade command (`upgrade-version-command/1-21/*` … `2-18/*`), authored before the engine existed, now flows through it. Their operation matrix is no longer applied literally: the engine injects/cascades companions (system fields, `searchVector` field + GIN index, `searchFieldMetadata` rows, unique backing indexes) and can hard-fail on reserved-identifier collisions (`RESERVED_SYSTEM_UNIVERSAL_IDENTIFIER`). Two hazards for already-shipped commands: 1. **Collision → hard failure**: a command declaring a companion the engine now owns collides with the engine's deterministic `universalIdentifier`. 2. **Silent drift**: on object/field create/delete the engine adds/cascades companions the command author never intended, so workspaces upgraded now differ structurally from those upgraded incrementally before 2.19. Suspected real-world impact: a self-hosted user upgrading v2.6.1 → v2.21.0 hit `duplicate key value violates unique constraint "IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE"` in `upgrade:2-16:backfill-search-field-metadata`, because object-creating commands now cascade and pre-create the deterministic `searchFieldMetadata` rows the standalone backfill then re-inserts. ## Changes - `workspace-migration-validate-build-and-run-service.ts`: extract the shared compute-and-run tail into a private method, and add `validateBuildAndRunLegacyWorkspaceMigration` (marked `@deprecated`) that skips `expandWithSideEffects` and applies the matrix literally. The existing side-effect entry points are unchanged (the live API and application manifests depend on them). - Repoint **all** pre-2.19 upgrade command call sites (1-21 … 2-18, including `2-10 sync-call-recording-standard-objects`) to the legacy method. Only the four `2-20/*` commands (target version ≥ 2.19) remain on the side-effect path. - `2-16 backfill-search-field-metadata`: recompute `flatSearchFieldMetadataMaps` from the database before building the existing-rows dedupe set. The migration runner only invalidates the flat-maps keys a migration touched, so during a cross-version upgrade earlier commands can leave this map stale; a stale map breaks the dedupe and re-inserts rows, tripping `IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE`. This is the direct fix for the reported failure. - Export `FlatEntityMapsBundle` so the shared tail can be typed. - Document the side-effect vs legacy path and the selection rule in `packages/twenty-server/docs/UPGRADE_COMMANDS.md`. Selection rule: target version **< 2.19** → legacy path; **≥ 2.19** → side-effect path (default). No exceptions. ## Known gap / merge ordering The static twenty-standard definition declares all of `callRecording`'s fields (including the `searchVector` system field) but **not** its `searchVector` GIN index — every other searchable standard object declares its GIN index statically. On the legacy path, workspaces upgrading through `2-10 sync-call-recording-standard-objects` therefore create the `searchVector` column unindexed (`searchFieldMetadata` rows are created later in the same pipeline by the 2-16 backfill). The static GIN index declaration plus a backfill for already-upgraded workspaces land in a follow-up (twentyhq/core-team-issues#2672), which must ship in the same release as this PR. ## Out of scope (separate follow-ups) - `UpgradeMigrationService.getLastAttemptedInstanceCommand()` ordering. - callRecording `searchVector` GIN index static declaration + backfill (twentyhq/core-team-issues#2672, same-release dependency, see above). ## Test plan - `nx typecheck twenty-server` passes. - `nx lint:diff-with-main twenty-server` (oxlint + oxfmt) clean on changed files. - 2-20 command specs (which exercise the unchanged side-effect path) pass. --------- Co-authored-by: twenty <noreply@twenty.com>
This commit is contained in:
Paul Rastoin
+1
-1
@@ -28,7 +28,7 @@ import { InferDeletionFromMissingEntities } from 'src/engine/workspace-manager/w
|
||||
export type WorkspaceMigrationRelatedFlatEntityMaps =
|
||||
Partial<AllFlatEntityMaps> & WorkspaceMigrationBuilderAdditionalCacheDataMaps;
|
||||
|
||||
type FlatEntityMapsBundle = {
|
||||
export type FlatEntityMapsBundle = {
|
||||
flatApplicationMaps: FlatApplicationCacheMaps;
|
||||
allRelatedFlatEntityMaps: WorkspaceMigrationRelatedFlatEntityMaps;
|
||||
allMetadataNameCacheToCompute: AllMetadataName[];
|
||||
|
||||
+115
-15
@@ -19,7 +19,10 @@ import {
|
||||
IdByUniversalIdentifierByMetadataName,
|
||||
} from 'src/engine/workspace-manager/workspace-migration/services/utils/enrich-create-workspace-migration-action-with-ids.util';
|
||||
import { WorkspaceMigrationBuildOrchestratorService } from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-build-orchestrator.service';
|
||||
import { WorkspaceMigrationFlatEntityMapsService } from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-flat-entity-maps.service';
|
||||
import {
|
||||
FlatEntityMapsBundle,
|
||||
WorkspaceMigrationFlatEntityMapsService,
|
||||
} from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-flat-entity-maps.service';
|
||||
import {
|
||||
WorkspaceMigrationOrchestratorBuildArgs,
|
||||
WorkspaceMigrationOrchestratorFailedResult,
|
||||
@@ -43,6 +46,21 @@ type ValidateBuildAndRunWorkspaceMigrationFromRecordArgs = {
|
||||
dryRun?: boolean;
|
||||
};
|
||||
|
||||
type ValidateBuildAndRunWorkspaceMigrationFromRecordInternalArgs =
|
||||
ValidateBuildAndRunWorkspaceMigrationFromRecordArgs & {
|
||||
// Skips the metadata side-effect engine (expandWithSideEffects) and applies the
|
||||
// matrix literally. Only the deprecated legacy path sets this to true.
|
||||
skipSideEffectExpandEngine: boolean;
|
||||
};
|
||||
|
||||
type ComputeAndRunWorkspaceMigrationFromResolvedOperationsArgs = {
|
||||
workspaceId: string;
|
||||
allFlatEntityOperationRecordByMetadataName: AllFlatEntityOperationRecordByMetadataName;
|
||||
isSystemBuild: boolean;
|
||||
applicationUniversalIdentifier: string;
|
||||
dryRun?: boolean;
|
||||
} & FlatEntityMapsBundle;
|
||||
|
||||
@Injectable()
|
||||
export class WorkspaceMigrationValidateBuildAndRunService {
|
||||
private readonly isDebugEnabled: boolean;
|
||||
@@ -185,13 +203,60 @@ export class WorkspaceMigrationValidateBuildAndRunService {
|
||||
});
|
||||
}
|
||||
|
||||
public async validateBuildAndRunWorkspaceMigrationFromRecord({
|
||||
public async validateBuildAndRunWorkspaceMigrationFromRecord(
|
||||
args: ValidateBuildAndRunWorkspaceMigrationFromRecordArgs,
|
||||
): Promise<
|
||||
| WorkspaceMigrationOrchestratorFailedResult
|
||||
| (WorkspaceMigrationOrchestratorSuccessfulResult & {
|
||||
hasSchemaMetadataChanged: boolean;
|
||||
})
|
||||
> {
|
||||
return await this.validateBuildAndRunWorkspaceMigrationFromRecordInternal({
|
||||
...args,
|
||||
skipSideEffectExpandEngine: false,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated Legacy path for upgrade commands authored before the metadata
|
||||
* side-effect engine landed in v2.19. These commands declare their operation
|
||||
* matrix literally and must not flow through expandWithSideEffects, which
|
||||
* would inject engine-owned companions and collide on reserved identifiers.
|
||||
* See packages/twenty-server/docs/UPGRADE_COMMANDS.md.
|
||||
*/
|
||||
public async validateBuildAndRunLegacyWorkspaceMigration({
|
||||
allFlatEntityOperationByMetadataName,
|
||||
workspaceId,
|
||||
isSystemBuild = false,
|
||||
applicationUniversalIdentifier,
|
||||
dryRun,
|
||||
}: ValidateBuildAndRunWorkspaceMigrationFromMatriceArgs): Promise<
|
||||
| WorkspaceMigrationOrchestratorFailedResult
|
||||
| (WorkspaceMigrationOrchestratorSuccessfulResult & {
|
||||
hasSchemaMetadataChanged: boolean;
|
||||
})
|
||||
> {
|
||||
return await this.validateBuildAndRunWorkspaceMigrationFromRecordInternal({
|
||||
allFlatEntityOperationRecordByMetadataName:
|
||||
transpileFlatEntityOperationArrayToRecord(
|
||||
allFlatEntityOperationByMetadataName,
|
||||
),
|
||||
workspaceId,
|
||||
isSystemBuild,
|
||||
applicationUniversalIdentifier,
|
||||
dryRun,
|
||||
skipSideEffectExpandEngine: true,
|
||||
});
|
||||
}
|
||||
|
||||
private async validateBuildAndRunWorkspaceMigrationFromRecordInternal({
|
||||
allFlatEntityOperationRecordByMetadataName,
|
||||
workspaceId,
|
||||
isSystemBuild = false,
|
||||
applicationUniversalIdentifier,
|
||||
dryRun,
|
||||
}: ValidateBuildAndRunWorkspaceMigrationFromRecordArgs): Promise<
|
||||
skipSideEffectExpandEngine,
|
||||
}: ValidateBuildAndRunWorkspaceMigrationFromRecordInternalArgs): Promise<
|
||||
| WorkspaceMigrationOrchestratorFailedResult
|
||||
| (WorkspaceMigrationOrchestratorSuccessfulResult & {
|
||||
hasSchemaMetadataChanged: boolean;
|
||||
@@ -213,19 +278,55 @@ export class WorkspaceMigrationValidateBuildAndRunService {
|
||||
},
|
||||
);
|
||||
|
||||
const sideEffectExpansionResult =
|
||||
this.metadataSideEffectEngineService.expandWithSideEffects({
|
||||
allFlatEntityOperationRecordByMetadataName,
|
||||
sideEffectRelatedFlatEntityMaps: allRelatedFlatEntityMaps,
|
||||
context: {
|
||||
buildOptions: { isSystemBuild, applicationUniversalIdentifier },
|
||||
},
|
||||
});
|
||||
let resolvedFlatEntityOperationRecordByMetadataName =
|
||||
allFlatEntityOperationRecordByMetadataName;
|
||||
|
||||
if (sideEffectExpansionResult.status === 'fail') {
|
||||
return sideEffectExpansionResult;
|
||||
if (!skipSideEffectExpandEngine) {
|
||||
const sideEffectExpansionResult =
|
||||
this.metadataSideEffectEngineService.expandWithSideEffects({
|
||||
allFlatEntityOperationRecordByMetadataName,
|
||||
sideEffectRelatedFlatEntityMaps: allRelatedFlatEntityMaps,
|
||||
context: {
|
||||
buildOptions: { isSystemBuild, applicationUniversalIdentifier },
|
||||
},
|
||||
});
|
||||
|
||||
if (sideEffectExpansionResult.status === 'fail') {
|
||||
return sideEffectExpansionResult;
|
||||
}
|
||||
|
||||
resolvedFlatEntityOperationRecordByMetadataName =
|
||||
sideEffectExpansionResult.allFlatEntityOperationRecordByMetadataName;
|
||||
}
|
||||
|
||||
return await this.computeAndRunWorkspaceMigrationFromResolvedOperations({
|
||||
allFlatEntityOperationRecordByMetadataName:
|
||||
resolvedFlatEntityOperationRecordByMetadataName,
|
||||
workspaceId,
|
||||
isSystemBuild,
|
||||
applicationUniversalIdentifier,
|
||||
dryRun,
|
||||
flatApplicationMaps,
|
||||
allRelatedFlatEntityMaps,
|
||||
allMetadataNameCacheToCompute,
|
||||
});
|
||||
}
|
||||
|
||||
private async computeAndRunWorkspaceMigrationFromResolvedOperations({
|
||||
allFlatEntityOperationRecordByMetadataName,
|
||||
workspaceId,
|
||||
isSystemBuild,
|
||||
applicationUniversalIdentifier,
|
||||
dryRun,
|
||||
flatApplicationMaps,
|
||||
allRelatedFlatEntityMaps,
|
||||
allMetadataNameCacheToCompute,
|
||||
}: ComputeAndRunWorkspaceMigrationFromResolvedOperationsArgs): Promise<
|
||||
| WorkspaceMigrationOrchestratorFailedResult
|
||||
| (WorkspaceMigrationOrchestratorSuccessfulResult & {
|
||||
hasSchemaMetadataChanged: boolean;
|
||||
})
|
||||
> {
|
||||
const {
|
||||
fromToAllFlatEntityMaps,
|
||||
inferDeletionFromMissingEntities,
|
||||
@@ -235,8 +336,7 @@ export class WorkspaceMigrationValidateBuildAndRunService {
|
||||
} =
|
||||
this.workspaceMigrationFlatEntityMapsService.computeFromToAllFlatEntityMapsAndBuildOptions(
|
||||
{
|
||||
allFlatEntityOperationRecordByMetadataName:
|
||||
sideEffectExpansionResult.allFlatEntityOperationRecordByMetadataName,
|
||||
allFlatEntityOperationRecordByMetadataName,
|
||||
applicationUniversalIdentifier,
|
||||
flatApplicationMaps,
|
||||
allRelatedFlatEntityMaps,
|
||||
|
||||
Reference in New Issue
Block a user