From 0877ba2ffd1d4d62da523e99e4c924d3c0841b06 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Thu, 4 Jun 2026 18:53:25 +0200 Subject: [PATCH] Fix `getApplicationSubAllFlatEntityMaps` to prune unrelated appIds universal identifier aggregators (#21234) # Introduction Atm when computing the `fromAllFlatEntityMaps` we're retrieving all the applicationIds related metadata entities to build a flat entity maps scoped to them ( atm always the applicationId + twenty-standard application id ) only inter app dependency we manage for the moment A flat entity contains universal identifier aggregator to its related entities The issue was that the `getApplicationSubAllFlatEntityMaps` wasn't pruning the aggregator by app Now added a new process phase after the initial one that will check that all the aggregators contains universal identifiers that has been retrieve from the appId + appId standard intersection ## TDD test Created a very human readable ( that's a joke ) test --- ...plication-sub-all-flat-entity-maps.util.ts | 5 + ...-entity-maps-through-mutation.util.spec.ts | 182 ++++++++++++++ ...-flat-entity-maps-through-mutation.util.ts | 121 +++++++++ ...s-app-owned-view-field.integration-spec.ts | 229 ++++++++++++++++++ 4 files changed, 537 insertions(+) create mode 100644 packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/__tests__/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.spec.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/successful-resync-application-with-cross-app-owned-view-field.integration-spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/get-application-sub-all-flat-entity-maps.util.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/get-application-sub-all-flat-entity-maps.util.ts index b9696e45e7..5354d1af57 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/get-application-sub-all-flat-entity-maps.util.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/utils/get-application-sub-all-flat-entity-maps.util.ts @@ -5,6 +5,7 @@ import { type AllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/ import { type MetadataFlatEntity } from 'src/engine/metadata-modules/flat-entity/types/metadata-flat-entity.type'; import { getMetadataFlatEntityMapsKey } from 'src/engine/metadata-modules/flat-entity/utils/get-metadata-flat-entity-maps-key.util'; import { getSubFlatEntityMapsByApplicationIdsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/get-sub-flat-entity-maps-by-application-ids-or-throw.util'; +import { pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation } from 'src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util'; export const getApplicationSubAllFlatEntityMaps = ({ applicationIds, @@ -31,5 +32,9 @@ export const getApplicationSubAllFlatEntityMaps = ({ emptyAllFlatEntityMaps[flatEntityMapsKey] = applicationSubFlatEntityMaps; } + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: emptyAllFlatEntityMaps, + }); + return emptyAllFlatEntityMaps; }; diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/__tests__/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.spec.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/__tests__/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.spec.ts new file mode 100644 index 0000000000..db8cd49b08 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/__tests__/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.spec.ts @@ -0,0 +1,182 @@ +import { createEmptyAllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-all-flat-entity-maps.constant'; +import { type AllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/all-flat-entity-maps.type'; +import { pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation } from 'src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util'; +import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; +import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +import { type FlatViewField } from 'src/engine/metadata-modules/flat-view-field/types/flat-view-field.type'; +import { type FlatView } from 'src/engine/metadata-modules/flat-view/types/flat-view.type'; + +const buildFlatViewField = (universalIdentifier: string): FlatViewField => + ({ + id: universalIdentifier, + universalIdentifier, + }) as unknown as FlatViewField; + +const buildFlatView = ({ + universalIdentifier, + viewFieldUniversalIdentifiers, +}: { + universalIdentifier: string; + viewFieldUniversalIdentifiers: string[]; +}): FlatView => + ({ + id: universalIdentifier, + universalIdentifier, + viewFieldUniversalIdentifiers, + viewFilterUniversalIdentifiers: [], + viewGroupUniversalIdentifiers: [], + viewFilterGroupUniversalIdentifiers: [], + viewFieldGroupUniversalIdentifiers: [], + viewSortUniversalIdentifiers: [], + }) as unknown as FlatView; + +const buildAllFlatEntityMapsWithView = ({ + view, + viewFields, +}: { + view: FlatView; + viewFields: FlatViewField[]; +}): AllFlatEntityMaps => { + const allFlatEntityMaps = createEmptyAllFlatEntityMaps(); + + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier[ + view.universalIdentifier + ] = view; + + for (const viewField of viewFields) { + allFlatEntityMaps.flatViewFieldMaps.byUniversalIdentifier[ + viewField.universalIdentifier + ] = viewField; + } + + return allFlatEntityMaps; +}; + +describe('pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation', () => { + it('should drop references to children missing from the slice while keeping present ones', () => { + const view = buildFlatView({ + universalIdentifier: 'view-body', + viewFieldUniversalIdentifiers: ['vf-arm', 'vf-head', 'vf-leg'], + }); + + // vf-leg is owned by another application and therefore not part of the slice + const allFlatEntityMaps = buildAllFlatEntityMapsWithView({ + view, + viewFields: [buildFlatViewField('vf-arm'), buildFlatViewField('vf-head')], + }); + + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: allFlatEntityMaps, + }); + + expect( + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier['view-body'] + ?.viewFieldUniversalIdentifiers, + ).toEqual(['vf-arm', 'vf-head']); + }); + + it('should leave aggregators untouched when every reference is present in the slice', () => { + const view = buildFlatView({ + universalIdentifier: 'view-body', + viewFieldUniversalIdentifiers: ['vf-arm', 'vf-head'], + }); + + const allFlatEntityMaps = buildAllFlatEntityMapsWithView({ + view, + viewFields: [buildFlatViewField('vf-arm'), buildFlatViewField('vf-head')], + }); + + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: allFlatEntityMaps, + }); + + const prunedView = + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier['view-body']; + + expect(prunedView?.viewFieldUniversalIdentifiers).toEqual([ + 'vf-arm', + 'vf-head', + ]); + // No dangling reference means the entity reference is preserved as-is + expect(prunedView).toBe(view); + }); + + it('should drop all references when no children remain in the slice', () => { + const view = buildFlatView({ + universalIdentifier: 'view-body', + viewFieldUniversalIdentifiers: ['vf-arm', 'vf-head'], + }); + + const allFlatEntityMaps = buildAllFlatEntityMapsWithView({ + view, + viewFields: [], + }); + + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: allFlatEntityMaps, + }); + + expect( + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier['view-body'] + ?.viewFieldUniversalIdentifiers, + ).toEqual([]); + }); + + it('should prune dangling references generically across entity types (objectMetadata fields)', () => { + const allFlatEntityMaps = createEmptyAllFlatEntityMaps(); + + allFlatEntityMaps.flatObjectMetadataMaps.byUniversalIdentifier['object-1'] = + { + id: 'object-1', + universalIdentifier: 'object-1', + fieldUniversalIdentifiers: ['field-present', 'field-missing'], + viewUniversalIdentifiers: [], + indexMetadataUniversalIdentifiers: [], + objectPermissionUniversalIdentifiers: [], + fieldPermissionUniversalIdentifiers: [], + } as unknown as FlatObjectMetadata; + + allFlatEntityMaps.flatFieldMetadataMaps.byUniversalIdentifier[ + 'field-present' + ] = { + id: 'field-present', + universalIdentifier: 'field-present', + } as unknown as FlatFieldMetadata; + + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: allFlatEntityMaps, + }); + + expect( + allFlatEntityMaps.flatObjectMetadataMaps.byUniversalIdentifier['object-1'] + ?.fieldUniversalIdentifiers, + ).toEqual(['field-present']); + }); + + it('should not mutate the shared parent entity object when pruning', () => { + const view = buildFlatView({ + universalIdentifier: 'view-body', + viewFieldUniversalIdentifiers: ['vf-arm', 'vf-leg'], + }); + + const allFlatEntityMaps = buildAllFlatEntityMapsWithView({ + view, + viewFields: [buildFlatViewField('vf-arm')], + }); + + pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation({ + allFlatEntityMapsToMutate: allFlatEntityMaps, + }); + + // The map entry is swapped for a new pruned object, leaving the original + // (shared with other slices/cache) entity untouched + expect(view.viewFieldUniversalIdentifiers).toEqual(['vf-arm', 'vf-leg']); + expect( + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier['view-body'], + ).not.toBe(view); + expect( + allFlatEntityMaps.flatViewMaps.byUniversalIdentifier['view-body'] + ?.viewFieldUniversalIdentifiers, + ).toEqual(['vf-arm']); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.ts new file mode 100644 index 0000000000..18e486981f --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/flat-entity/utils/prune-dangling-foreign-key-aggregators-in-all-flat-entity-maps-through-mutation.util.ts @@ -0,0 +1,121 @@ +import { + ALL_METADATA_NAME, + type AllMetadataName, +} from 'twenty-shared/metadata'; +import { isDefined } from 'twenty-shared/utils'; + +import { ALL_ONE_TO_MANY_METADATA_RELATIONS } from 'src/engine/metadata-modules/flat-entity/constant/all-one-to-many-metadata-relations.constant'; +import { type AllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/all-flat-entity-maps.type'; +import { type SyncableFlatEntity } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-from.type'; +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +import { getMetadataFlatEntityMapsKey } from 'src/engine/metadata-modules/flat-entity/utils/get-metadata-flat-entity-maps-key.util'; + +type LooseAllFlatEntityMaps = Record< + string, + FlatEntityMaps +>; + +type OneToManyRelation = { + metadataName: AllMetadataName; + universalFlatEntityForeignKeyAggregator: string; +} | null; + +type OneToManyAggregatorWithChildIdentifiers = { + aggregatorProperty: string; + childUniversalIdentifiers: Set; +}; + +const pruneFlatEntityForeignKeyAggregators = ({ + flatEntity, + aggregatorsWithChildIdentifiers, +}: { + flatEntity: SyncableFlatEntity; + aggregatorsWithChildIdentifiers: OneToManyAggregatorWithChildIdentifiers[]; +}): SyncableFlatEntity => + aggregatorsWithChildIdentifiers.reduce( + (prunedFlatEntity, { aggregatorProperty, childUniversalIdentifiers }) => { + const aggregatedUniversalIdentifiers = ( + prunedFlatEntity as Record + )[aggregatorProperty]; + + if ( + !isDefined(aggregatedUniversalIdentifiers) || + !Array.isArray(aggregatedUniversalIdentifiers) + ) { + return prunedFlatEntity; + } + + const prunedUniversalIdentifiers = aggregatedUniversalIdentifiers.filter( + (childUniversalIdentifier: string) => + childUniversalIdentifiers.has(childUniversalIdentifier), + ); + + if ( + prunedUniversalIdentifiers.length === + aggregatedUniversalIdentifiers.length + ) { + return prunedFlatEntity; + } + + return { + ...prunedFlatEntity, + [aggregatorProperty]: prunedUniversalIdentifiers, + }; + }, + flatEntity, + ); + +// Drops one-to-many aggregator references to children absent from the slice +// (e.g. an app-owned view referencing a view field owned by another +// application), keeping the slice a closed, internally consistent graph. +// Mutates the passed maps in place: the parent entry is replaced by a new +// pruned entity object, never mutating the shared entity referenced elsewhere. +export const pruneDanglingForeignKeyAggregatorsInAllFlatEntityMapsThroughMutation = + ({ + allFlatEntityMapsToMutate, + }: { + allFlatEntityMapsToMutate: AllFlatEntityMaps; + }): void => { + const looseAllFlatEntityMaps = + allFlatEntityMapsToMutate as unknown as LooseAllFlatEntityMaps; + + for (const metadataName of Object.values(ALL_METADATA_NAME)) { + const oneToManyRelations = Object.values( + ALL_ONE_TO_MANY_METADATA_RELATIONS[metadataName], + ) as OneToManyRelation[]; + + const aggregatorsWithChildIdentifiers: OneToManyAggregatorWithChildIdentifiers[] = + oneToManyRelations.filter(isDefined).map((relation) => ({ + aggregatorProperty: relation.universalFlatEntityForeignKeyAggregator, + childUniversalIdentifiers: new Set( + Object.keys( + looseAllFlatEntityMaps[ + getMetadataFlatEntityMapsKey(relation.metadataName) + ].byUniversalIdentifier, + ), + ), + })); + + if (aggregatorsWithChildIdentifiers.length === 0) { + continue; + } + + const flatEntityByUniversalIdentifier = + looseAllFlatEntityMaps[getMetadataFlatEntityMapsKey(metadataName)] + .byUniversalIdentifier; + + for (const [universalIdentifier, parentFlatEntity] of Object.entries( + flatEntityByUniversalIdentifier, + )) { + if (!isDefined(parentFlatEntity)) { + continue; + } + + flatEntityByUniversalIdentifier[universalIdentifier] = + pruneFlatEntityForeignKeyAggregators({ + flatEntity: parentFlatEntity, + aggregatorsWithChildIdentifiers, + }); + } + } + }; diff --git a/packages/twenty-server/test/integration/metadata/suites/application/successful-resync-application-with-cross-app-owned-view-field.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/successful-resync-application-with-cross-app-owned-view-field.integration-spec.ts new file mode 100644 index 0000000000..4a2fc3c285 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application/successful-resync-application-with-cross-app-owned-view-field.integration-spec.ts @@ -0,0 +1,229 @@ +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { buildDefaultObjectManifest } from 'test/integration/metadata/suites/application/utils/build-default-object-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { findManyObjectMetadataWithIndexes } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata-with-indexes.util'; +import { createOneViewField } from 'test/integration/metadata/suites/view-field/utils/create-one-view-field.util'; +import { findViewFields } from 'test/integration/metadata/suites/view-field/utils/find-view-fields.util'; +import { findViews } from 'test/integration/metadata/suites/view/utils/find-views.util'; +import { type Manifest } from 'twenty-shared/application'; +import { FieldMetadataType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; +import { v4 as uuidv4 } from 'uuid'; + +const TEST_APP_ID = uuidv4(); +const TEST_ROLE_ID = uuidv4(); + +const ARM_FIELD_ID = uuidv4(); +const LEG_FIELD_ID = uuidv4(); +const HEAD_FIELD_ID = uuidv4(); +const TAIL_FIELD_ID = uuidv4(); + +const BODY_VIEW_ID = uuidv4(); +const ARM_VIEW_FIELD_ID = uuidv4(); +const HEAD_VIEW_FIELD_ID = uuidv4(); +const TAIL_VIEW_FIELD_ID = uuidv4(); + +const HUMAN_OBJECT = buildDefaultObjectManifest({ + nameSingular: 'human', + namePlural: 'humans', + labelSingular: 'Human', + labelPlural: 'Humans', + description: 'A human being', + icon: 'IconUser', + additionalFields: [ + { + universalIdentifier: ARM_FIELD_ID, + type: FieldMetadataType.TEXT, + name: 'arm', + label: 'Arm', + }, + { + universalIdentifier: LEG_FIELD_ID, + type: FieldMetadataType.TEXT, + name: 'leg', + label: 'Leg', + }, + { + universalIdentifier: HEAD_FIELD_ID, + type: FieldMetadataType.TEXT, + name: 'head', + label: 'Head', + }, + ], +}); + +const buildInitialManifest = (): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + objects: [HUMAN_OBJECT], + views: [ + { + universalIdentifier: BODY_VIEW_ID, + name: 'Body', + objectUniversalIdentifier: HUMAN_OBJECT.universalIdentifier, + fields: [ + { + universalIdentifier: ARM_VIEW_FIELD_ID, + fieldMetadataUniversalIdentifier: ARM_FIELD_ID, + position: 0, + }, + { + universalIdentifier: HEAD_VIEW_FIELD_ID, + fieldMetadataUniversalIdentifier: HEAD_FIELD_ID, + position: 1, + }, + ], + }, + ], + }, + }); + +const buildResyncManifest = (): Manifest => + buildBaseManifest({ + appId: TEST_APP_ID, + roleId: TEST_ROLE_ID, + overrides: { + objects: [ + { + ...HUMAN_OBJECT, + fields: [ + ...HUMAN_OBJECT.fields, + { + universalIdentifier: TAIL_FIELD_ID, + type: FieldMetadataType.TEXT, + name: 'tail', + label: 'Tail', + }, + ], + }, + ], + views: [ + { + universalIdentifier: BODY_VIEW_ID, + name: 'Body', + objectUniversalIdentifier: HUMAN_OBJECT.universalIdentifier, + fields: [ + { + universalIdentifier: ARM_VIEW_FIELD_ID, + fieldMetadataUniversalIdentifier: ARM_FIELD_ID, + position: 0, + }, + { + universalIdentifier: HEAD_VIEW_FIELD_ID, + fieldMetadataUniversalIdentifier: HEAD_FIELD_ID, + position: 1, + }, + { + universalIdentifier: TAIL_VIEW_FIELD_ID, + fieldMetadataUniversalIdentifier: TAIL_FIELD_ID, + position: 2, + }, + ], + }, + ], + }, + }); + +const findHumanObject = async () => { + const objects = await findManyObjectMetadataWithIndexes({ + expectToFail: false, + }); + + return objects.find( + (objectMetadata) => + objectMetadata.universalIdentifier === HUMAN_OBJECT.universalIdentifier, + ); +}; + +describe('Successful re-sync of an application whose app-owned view references a cross-app-owned view field', () => { + beforeAll(async () => { + await setupApplicationForSync({ + applicationUniversalIdentifier: TEST_APP_ID, + name: 'Human Application', + description: 'App for testing view field ownership conflicts on re-sync', + sourcePath: 'test-api-owned-view-field', + }); + }, 60000); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: TEST_APP_ID, + }); + }); + + it('should re-sync successfully when the app-owned Body view carries a view field owned by another application', async () => { + await syncApplication({ + manifest: buildInitialManifest(), + expectToFail: false, + }); + + const humanObject = await findHumanObject(); + + expect(humanObject).toBeDefined(); + + const legField = humanObject?.fieldsList.find( + (field) => field.name === 'leg', + ); + + expect(legField).toBeDefined(); + + const { data: viewsData } = await findViews({ + objectMetadataId: humanObject?.id, + expectToFail: false, + }); + + const bodyView = viewsData?.getViews.find((view) => view.name === 'Body'); + + expect(bodyView).toBeDefined(); + + const { data: createViewFieldData } = await createOneViewField({ + input: { + fieldMetadataId: legField?.id ?? '', + viewId: bodyView?.id ?? '', + position: 2, + }, + expectToFail: false, + }); + + const legViewFieldId = createViewFieldData?.createViewField.id; + + expect(isDefined(legViewFieldId)).toBe(true); + + const { errors } = await syncApplication({ + manifest: buildResyncManifest(), + expectToFail: false, + }); + + expect(isDefined(errors)).toBe(false); + + const humanObjectAfterResync = await findHumanObject(); + const tailField = humanObjectAfterResync?.fieldsList.find( + (field) => field.name === 'tail', + ); + + expect(tailField).toBeDefined(); + + const { data: viewFieldsData } = await findViewFields({ + viewId: bodyView?.id ?? '', + expectToFail: false, + }); + + const viewFieldFieldMetadataIds = (viewFieldsData?.getViewFields ?? []).map( + (viewField) => viewField.fieldMetadataId, + ); + + expect(viewFieldFieldMetadataIds).toEqual( + expect.arrayContaining([legField?.id, tailField?.id]), + ); + + const legViewFieldStillPresent = (viewFieldsData?.getViewFields ?? []).some( + (viewField) => viewField.id === legViewFieldId, + ); + + expect(legViewFieldStillPresent).toBe(true); + }, 120000); +});