fix(workspace-migration): stop leaking workspace ids in delete action payloads (#23377)
Closes https://github.com/twentyhq/core-team-issues/issues/2732 ## Problem Workspace migration delete actions embedded the raw workspace-cache flat entity as their `flatEntity` payload, leaking: - `id`, `workspaceId`, `applicationId` - raw many-to-one join columns (`objectMetadataId`, `relationTargetFieldMetadataId`, ...) - raw FK aggregators (`viewFieldIds`, ...) - raw jsonb properties containing serialized relations (`settings`, `overrides`, `configuration`) Create actions already expose universal identifiers only. The asymmetry made identical migrations non-portable across workspaces (payloads embed random workspace primary keys) and caused snapshot flakiness in integration suites. ## Fix - Add `deleteFlatEntityForeignKeyAggregators` (raw-side counterpart of `deleteUniversalFlatEntityForeignKeyAggregators`, following the `flatEntityForeignKeyAggregator` / `universalFlatEntityForeignKeyAggregator` naming of `ALL_ONE_TO_MANY_METADATA_RELATIONS`). It strips base workspace-scoped properties, every property registered with a `universalProperty` counterpart in `ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME` (covers raw join columns and serialized jsonb, including cases not modeled as many-to-one relations like `labelIdentifierFieldMetadataId`), and raw one-to-many `...Ids` aggregators. Its scope is disjoint from the universal-side util. - Apply it in the delete branch of `WorkspaceEntityMigrationBuilderService` — the single point where delete-action `flatEntity` is attached — so the payload matches its `MetadataUniversalFlatEntity<T>` type at runtime. Universal `...UniversalIdentifiers` aggregators are kept (they are portable), so `BaseUniversalDeleteWorkspaceMigrationAction` needs no type change. - Regenerate the affected `successful-sync-application-workspace-migration` snapshot: the delete payload now only carries universal identifiers. Safe downstream: the runner resolves delete targets via `universalIdentifier` lookups in current maps and metadata events fetch the deleted entity from maps by `entityId`; no consumer reads the stripped properties (only create handlers consume `action.flatEntity`). Note: the `normalizeIdCollections` mitigation flag mentioned in the issue does not exist on `main`, so there was nothing to remove. ## Tests - New snapshot-based unit spec for the strip util (objectMetadata and fieldMetadata shapes, plus input immutability). - Full twenty-server unit suite: 6922 passed. - Integration with live DB: full `metadata/suites/application` (50 suites), object/field/index/agent metadata suites, all 26 `graphql/suites/view` suites, `failing-agent-deletion`, `object-identifier-update-side-effect-on-view-field` — all green, no other snapshot changes.
This commit is contained in:
+7
-7
@@ -13,12 +13,12 @@ const serializeScalarValue = (value: unknown): unknown => {
|
||||
return value ?? null;
|
||||
};
|
||||
|
||||
const BASE_SCALAR_PROPERTY_NAMES = [
|
||||
'id',
|
||||
'workspaceId',
|
||||
'applicationId',
|
||||
'universalIdentifier',
|
||||
] as const;
|
||||
export const BASE_SCALAR_PROPERTY_NAME = {
|
||||
id: 'id',
|
||||
workspaceId: 'workspaceId',
|
||||
applicationId: 'applicationId',
|
||||
universalIdentifier: 'universalIdentifier',
|
||||
} as const;
|
||||
|
||||
// Allow-list counterpart to removePropertiesFromRecord: rather than forwarding
|
||||
// every entity property except the known relations (deny-list), this forwards only
|
||||
@@ -42,7 +42,7 @@ export const fromEntityToScalarEntity = <T extends AllMetadataName>({
|
||||
const scalarProperties: Record<string, unknown> = {};
|
||||
|
||||
for (const propertyName of [
|
||||
...BASE_SCALAR_PROPERTY_NAMES,
|
||||
...Object.values(BASE_SCALAR_PROPERTY_NAME),
|
||||
...Object.keys(propertiesConfiguration),
|
||||
]) {
|
||||
scalarProperties[propertyName] = serializeScalarValue(
|
||||
|
||||
+81
@@ -0,0 +1,81 @@
|
||||
// Jest Snapshot v1, https://goo.gl/fbAQLP
|
||||
|
||||
exports[`deleteFlatEntityForeignKeyAggregators should strip raw workspace-scoped properties from a flat field metadata 1`] = `
|
||||
{
|
||||
"applicationUniversalIdentifier": "application-universal-identifier",
|
||||
"calendarEndViewUniversalIdentifiers": [],
|
||||
"calendarViewUniversalIdentifiers": [],
|
||||
"createdAt": "2024-01-01T00:00:00.000Z",
|
||||
"defaultValue": null,
|
||||
"description": "default flat field metadata description",
|
||||
"fieldPermissionUniversalIdentifiers": [],
|
||||
"icon": "icon",
|
||||
"isActive": true,
|
||||
"isLabelSyncedWithName": false,
|
||||
"isNullable": true,
|
||||
"isSystem": false,
|
||||
"isSystemSideEffect": false,
|
||||
"isUIEditable": true,
|
||||
"isUnique": false,
|
||||
"kanbanAggregateOperationViewUniversalIdentifiers": [],
|
||||
"label": "flat field metadata label",
|
||||
"mainGroupByFieldMetadataViewUniversalIdentifiers": [],
|
||||
"morphId": null,
|
||||
"name": "flatFieldMetadataName",
|
||||
"objectMetadataUniversalIdentifier": "object-metadata-universal-identifier",
|
||||
"options": null,
|
||||
"overrides": null,
|
||||
"relationTargetFieldMetadataUniversalIdentifier": null,
|
||||
"relationTargetObjectMetadataUniversalIdentifier": null,
|
||||
"searchFieldMetadataUniversalIdentifiers": [],
|
||||
"type": "TEXT",
|
||||
"universalIdentifier": "field-universal-identifier",
|
||||
"universalSettings": null,
|
||||
"updatedAt": "2024-01-01T00:00:00.000Z",
|
||||
"viewFieldUniversalIdentifiers": [
|
||||
"view-field-universal-identifier",
|
||||
],
|
||||
"viewFilterUniversalIdentifiers": [],
|
||||
"viewSortUniversalIdentifiers": [],
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`deleteFlatEntityForeignKeyAggregators should strip raw workspace-scoped properties from a flat object metadata 1`] = `
|
||||
{
|
||||
"applicationUniversalIdentifier": "application-universal-identifier",
|
||||
"color": null,
|
||||
"createdAt": "2024-01-01T00:00:00.000Z",
|
||||
"description": "default flat object metadata description",
|
||||
"duplicateCriteria": null,
|
||||
"fieldPermissionUniversalIdentifiers": [],
|
||||
"fieldUniversalIdentifiers": [
|
||||
"field-universal-identifier",
|
||||
],
|
||||
"icon": "icon",
|
||||
"imageIdentifierFieldMetadataUniversalIdentifier": "image-identifier-field-metadata-universal-identifier",
|
||||
"indexMetadataUniversalIdentifiers": [],
|
||||
"isActive": true,
|
||||
"isAuditLogged": true,
|
||||
"isLabelSyncedWithName": false,
|
||||
"isRemote": false,
|
||||
"isSearchable": true,
|
||||
"isSystem": false,
|
||||
"isUICreatable": true,
|
||||
"isUIEditable": true,
|
||||
"labelIdentifierFieldMetadataUniversalIdentifier": "label-identifier-field-metadata-universal-identifier",
|
||||
"labelPlural": "default flat object metadata label plural",
|
||||
"labelSingular": "default flat object metadata label singular",
|
||||
"namePlural": "defaultflatObjectMetadataNamePlural",
|
||||
"nameSingular": "defaultflatObjectMetadataNameSingular",
|
||||
"objectPermissionUniversalIdentifiers": [],
|
||||
"overrides": null,
|
||||
"searchFieldMetadataUniversalIdentifiers": [],
|
||||
"shortcut": "shortcut",
|
||||
"targetTableName": "",
|
||||
"universalIdentifier": "object-universal-identifier",
|
||||
"updatedAt": "2024-01-01T00:00:00.000Z",
|
||||
"viewUniversalIdentifiers": [
|
||||
"view-universal-identifier",
|
||||
],
|
||||
}
|
||||
`;
|
||||
+66
@@ -0,0 +1,66 @@
|
||||
import { FieldMetadataType } from 'twenty-shared/types';
|
||||
|
||||
import { getFlatFieldMetadataMock } from 'src/engine/metadata-modules/flat-field-metadata/__mocks__/get-flat-field-metadata.mock';
|
||||
import { getFlatObjectMetadataMock } from 'src/engine/metadata-modules/flat-object-metadata/__mocks__/get-flat-object-metadata.mock';
|
||||
import { deleteFlatEntityForeignKeyAggregators } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/delete-flat-entity-foreign-key-aggregators.util';
|
||||
|
||||
describe('deleteFlatEntityForeignKeyAggregators', () => {
|
||||
it('should strip raw workspace-scoped properties from a flat object metadata', () => {
|
||||
const flatObjectMetadata = getFlatObjectMetadataMock({
|
||||
universalIdentifier: 'object-universal-identifier',
|
||||
applicationId: 'application-id',
|
||||
applicationUniversalIdentifier: 'application-universal-identifier',
|
||||
labelIdentifierFieldMetadataId: 'label-identifier-field-metadata-id',
|
||||
labelIdentifierFieldMetadataUniversalIdentifier:
|
||||
'label-identifier-field-metadata-universal-identifier',
|
||||
imageIdentifierFieldMetadataId: 'image-identifier-field-metadata-id',
|
||||
imageIdentifierFieldMetadataUniversalIdentifier:
|
||||
'image-identifier-field-metadata-universal-identifier',
|
||||
fieldIds: ['field-id'],
|
||||
fieldUniversalIdentifiers: ['field-universal-identifier'],
|
||||
viewIds: ['view-id'],
|
||||
viewUniversalIdentifiers: ['view-universal-identifier'],
|
||||
});
|
||||
|
||||
const result = deleteFlatEntityForeignKeyAggregators({
|
||||
universalFlatEntity: flatObjectMetadata,
|
||||
metadataName: 'objectMetadata',
|
||||
});
|
||||
|
||||
expect(result).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it('should strip raw workspace-scoped properties from a flat field metadata', () => {
|
||||
const flatFieldMetadata = getFlatFieldMetadataMock({
|
||||
universalIdentifier: 'field-universal-identifier',
|
||||
type: FieldMetadataType.TEXT,
|
||||
applicationUniversalIdentifier: 'application-universal-identifier',
|
||||
objectMetadataId: 'object-metadata-id',
|
||||
objectMetadataUniversalIdentifier: 'object-metadata-universal-identifier',
|
||||
viewFieldIds: ['view-field-id'],
|
||||
viewFieldUniversalIdentifiers: ['view-field-universal-identifier'],
|
||||
});
|
||||
|
||||
const result = deleteFlatEntityForeignKeyAggregators({
|
||||
universalFlatEntity: flatFieldMetadata,
|
||||
metadataName: 'fieldMetadata',
|
||||
});
|
||||
|
||||
expect(result).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it('should not mutate the input entity', () => {
|
||||
const flatObjectMetadata = getFlatObjectMetadataMock({
|
||||
universalIdentifier: 'object-universal-identifier',
|
||||
});
|
||||
|
||||
deleteFlatEntityForeignKeyAggregators({
|
||||
universalFlatEntity: flatObjectMetadata,
|
||||
metadataName: 'objectMetadata',
|
||||
});
|
||||
|
||||
expect(flatObjectMetadata).toHaveProperty('id');
|
||||
expect(flatObjectMetadata).toHaveProperty('workspaceId');
|
||||
expect(flatObjectMetadata).toHaveProperty('fieldIds');
|
||||
});
|
||||
});
|
||||
+54
@@ -0,0 +1,54 @@
|
||||
import { type AllMetadataName } from 'twenty-shared/metadata';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME } from 'src/engine/metadata-modules/flat-entity/constant/all-entity-properties-configuration-by-metadata-name.constant';
|
||||
import { ALL_ONE_TO_MANY_METADATA_RELATIONS } from 'src/engine/metadata-modules/flat-entity/constant/all-one-to-many-metadata-relations.constant';
|
||||
import { type MetadataUniversalFlatEntity } from 'src/engine/metadata-modules/flat-entity/types/metadata-universal-flat-entity.type';
|
||||
import { BASE_SCALAR_PROPERTY_NAME } from 'src/engine/metadata-modules/flat-entity/utils/from-entity-to-scalar-entity.util';
|
||||
|
||||
const BASE_WORKSPACE_SCOPED_PROPERTIES = [
|
||||
BASE_SCALAR_PROPERTY_NAME.id,
|
||||
BASE_SCALAR_PROPERTY_NAME.workspaceId,
|
||||
BASE_SCALAR_PROPERTY_NAME.applicationId,
|
||||
] as const;
|
||||
|
||||
export const deleteFlatEntityForeignKeyAggregators = <
|
||||
T extends AllMetadataName,
|
||||
>({
|
||||
universalFlatEntity,
|
||||
metadataName,
|
||||
}: {
|
||||
universalFlatEntity: MetadataUniversalFlatEntity<T>;
|
||||
metadataName: T;
|
||||
}): MetadataUniversalFlatEntity<T> => {
|
||||
const result = { ...universalFlatEntity } as Record<string, unknown>;
|
||||
|
||||
for (const baseProperty of BASE_WORKSPACE_SCOPED_PROPERTIES) {
|
||||
delete result[baseProperty];
|
||||
}
|
||||
|
||||
const propertiesConfiguration =
|
||||
ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME[metadataName];
|
||||
|
||||
for (const [propertyName, propertyConfiguration] of Object.entries(
|
||||
propertiesConfiguration,
|
||||
) as [string, { universalProperty?: string }][]) {
|
||||
if (isDefined(propertyConfiguration.universalProperty)) {
|
||||
delete result[propertyName];
|
||||
}
|
||||
}
|
||||
|
||||
const oneToManyRelations = ALL_ONE_TO_MANY_METADATA_RELATIONS[metadataName];
|
||||
|
||||
for (const relation of Object.values(oneToManyRelations) as ({
|
||||
flatEntityForeignKeyAggregator: string;
|
||||
} | null)[]) {
|
||||
if (!isDefined(relation)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
delete result[relation.flatEntityForeignKeyAggregator];
|
||||
}
|
||||
|
||||
return result as MetadataUniversalFlatEntity<T>;
|
||||
};
|
||||
+8
-1
@@ -23,6 +23,7 @@ import { UniversalFlatEntityMaps } from 'src/engine/workspace-manager/workspace-
|
||||
import { addUniversalFlatEntityToUniversalFlatEntityAndRelatedEntityMapsThroughMutationOrThrow } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/add-universal-flat-entity-to-universal-flat-entity-and-related-entity-maps-through-mutation-or-throw.util';
|
||||
import { deleteUniversalFlatEntityForeignKeyAggregators } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/delete-universal-flat-entity-foreign-key-aggregators.util';
|
||||
import { deleteUniversalFlatEntityFromUniversalFlatEntityAndRelatedEntityMapsThroughMutationOrThrow } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/delete-universal-flat-entity-from-universal-flat-entity-and-related-entity-maps-through-mutation-or-throw.util';
|
||||
import { deleteFlatEntityForeignKeyAggregators } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/delete-flat-entity-foreign-key-aggregators.util';
|
||||
import { deleteUniversalFlatEntityFromUniversalFlatEntityMapsThroughMutationOrThrow } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/delete-universal-flat-entity-from-universal-flat-entity-maps-through-mutation-or-throw.util';
|
||||
import { replaceUniversalFlatEntityInUniversalFlatEntityMapsThroughMutationOrThrow } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/replace-universal-flat-entity-in-universal-flat-entity-maps-through-mutation-or-throw.util';
|
||||
import { resetUniversalFlatEntityForeignKeyAggregators } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/reset-universal-flat-entity-foreign-key-aggregators.util';
|
||||
@@ -168,13 +169,19 @@ export abstract class WorkspaceEntityMigrationBuilderService<
|
||||
},
|
||||
);
|
||||
|
||||
const universalFlatEntityToDeletePayload =
|
||||
deleteFlatEntityForeignKeyAggregators({
|
||||
metadataName: this.metadataName,
|
||||
universalFlatEntity: universalFlatEntityToDelete,
|
||||
});
|
||||
|
||||
actionsResult.delete.push(
|
||||
...(Array.isArray(validationResult.action)
|
||||
? validationResult.action
|
||||
: [validationResult.action]
|
||||
).map((action) => ({
|
||||
...action,
|
||||
flatEntity: universalFlatEntityToDelete,
|
||||
flatEntity: universalFlatEntityToDeletePayload,
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
-16
@@ -773,19 +773,14 @@ exports[`syncApplication should delete old field and create equivalent one when
|
||||
"actions": [
|
||||
{
|
||||
"flatEntity": {
|
||||
"applicationId": Any<String>,
|
||||
"applicationUniversalIdentifier": Any<String>,
|
||||
"calendarEndViewIds": [],
|
||||
"calendarEndViewUniversalIdentifiers": [],
|
||||
"calendarViewIds": [],
|
||||
"calendarViewUniversalIdentifiers": [],
|
||||
"createdAt": Any<String>,
|
||||
"defaultValue": null,
|
||||
"description": "Ticket description",
|
||||
"fieldPermissionIds": [],
|
||||
"fieldPermissionUniversalIdentifiers": [],
|
||||
"icon": "IconFileDescription",
|
||||
"id": Any<String>,
|
||||
"isActive": true,
|
||||
"isLabelSyncedWithName": false,
|
||||
"isNullable": true,
|
||||
@@ -793,35 +788,24 @@ exports[`syncApplication should delete old field and create equivalent one when
|
||||
"isSystemSideEffect": false,
|
||||
"isUIEditable": true,
|
||||
"isUnique": false,
|
||||
"kanbanAggregateOperationViewIds": [],
|
||||
"kanbanAggregateOperationViewUniversalIdentifiers": [],
|
||||
"label": "Description",
|
||||
"mainGroupByFieldMetadataViewIds": [],
|
||||
"mainGroupByFieldMetadataViewUniversalIdentifiers": [],
|
||||
"morphId": null,
|
||||
"name": "description",
|
||||
"objectMetadataId": Any<String>,
|
||||
"objectMetadataUniversalIdentifier": Any<String>,
|
||||
"options": null,
|
||||
"overrides": null,
|
||||
"relationTargetFieldMetadataId": null,
|
||||
"relationTargetFieldMetadataUniversalIdentifier": null,
|
||||
"relationTargetObjectMetadataId": null,
|
||||
"relationTargetObjectMetadataUniversalIdentifier": null,
|
||||
"searchFieldMetadataIds": [],
|
||||
"searchFieldMetadataUniversalIdentifiers": [],
|
||||
"settings": null,
|
||||
"type": "TEXT",
|
||||
"universalIdentifier": Any<String>,
|
||||
"universalSettings": null,
|
||||
"updatedAt": Any<String>,
|
||||
"viewFieldIds": [],
|
||||
"viewFieldUniversalIdentifiers": [],
|
||||
"viewFilterIds": [],
|
||||
"viewFilterUniversalIdentifiers": [],
|
||||
"viewSortIds": [],
|
||||
"viewSortUniversalIdentifiers": [],
|
||||
"workspaceId": Any<String>,
|
||||
},
|
||||
"metadataName": "fieldMetadata",
|
||||
"type": "delete",
|
||||
|
||||
Reference in New Issue
Block a user