Fix syncApplication failing when navigation menu item child is listed before folder in manifest (#19599)
## Summary - Fix `syncApplication` crashing with `ENTITY_NOT_FOUND` when a navigation menu item child (with `folderUniversalIdentifier`) appears before its folder in the manifest's `navigationMenuItems` array - Add a generic topological sort in `WorkspaceEntityMigrationBuilderService.validateAndBuild()` that detects self-referential FKs from `ALL_MANY_TO_ONE_METADATA_RELATIONS` and ensures parents are created before children - This also covers other self-referential entities: `viewFilterGroup.parentViewFilterGroup`, `fieldMetadata.relationTargetFieldMetadata`, and `rowLevelPermissionPredicateGroup.parentRowLevelPermissionPredicateGroup` ## Root cause The builder iterated `createdFlatEntityMaps.byUniversalIdentifier` in insertion order (from the manifest). Validation passed because it checked both optimistic maps and remaining-to-create maps. But the runner processed create actions sequentially, so `resolveUniversalRelationIdentifiersToIds` threw when the folder hadn't been created yet.
This commit is contained in:
Charles Bochet
+208
@@ -0,0 +1,208 @@
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
|
||||
import { type UniversalFlatEntityMaps } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/universal-flat-entity-maps.type';
|
||||
import { topologicallySortUniversalFlatEntitiesForSelfReferentialFks } from 'src/engine/workspace-manager/workspace-migration/utils/topologically-sort-universal-flat-entities-for-self-referential-fks.util';
|
||||
|
||||
const APPLICATION_UNIVERSAL_IDENTIFIER = uuidv4();
|
||||
|
||||
type TestNavigationMenuItemEntity = {
|
||||
universalIdentifier: string;
|
||||
applicationUniversalIdentifier: string;
|
||||
folderUniversalIdentifier: string | null;
|
||||
};
|
||||
|
||||
const createEntity = (
|
||||
universalIdentifier: string,
|
||||
folderUniversalIdentifier: string | null = null,
|
||||
): TestNavigationMenuItemEntity => ({
|
||||
universalIdentifier,
|
||||
applicationUniversalIdentifier: APPLICATION_UNIVERSAL_IDENTIFIER,
|
||||
folderUniversalIdentifier,
|
||||
});
|
||||
|
||||
const buildMaps = (
|
||||
entities: TestNavigationMenuItemEntity[],
|
||||
): UniversalFlatEntityMaps<TestNavigationMenuItemEntity> => ({
|
||||
byUniversalIdentifier: Object.fromEntries(
|
||||
entities.map((entity) => [entity.universalIdentifier, entity]),
|
||||
),
|
||||
});
|
||||
|
||||
describe('topologicallySortUniversalFlatEntitiesForSelfReferentialFks', () => {
|
||||
it('returns empty array for empty maps', () => {
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns original order for entities without self-referential FKs', () => {
|
||||
const idA = uuidv4();
|
||||
const idB = uuidv4();
|
||||
|
||||
const maps = {
|
||||
byUniversalIdentifier: {
|
||||
[idA]: {
|
||||
universalIdentifier: idA,
|
||||
applicationUniversalIdentifier: APPLICATION_UNIVERSAL_IDENTIFIER,
|
||||
},
|
||||
[idB]: {
|
||||
universalIdentifier: idB,
|
||||
applicationUniversalIdentifier: APPLICATION_UNIVERSAL_IDENTIFIER,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'objectMetadata',
|
||||
universalFlatEntityMaps: maps as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([idA, idB]);
|
||||
});
|
||||
|
||||
it('returns original order when no entity references another in the batch', () => {
|
||||
const itemA = uuidv4();
|
||||
const itemB = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(itemA),
|
||||
createEntity(itemB),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([itemA, itemB]);
|
||||
});
|
||||
|
||||
it('sorts parent before child when child is listed first', () => {
|
||||
const parentId = uuidv4();
|
||||
const childId = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(childId, parentId),
|
||||
createEntity(parentId),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([parentId, childId]);
|
||||
});
|
||||
|
||||
it('keeps parent before child when already in correct order', () => {
|
||||
const parentId = uuidv4();
|
||||
const childId = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(parentId),
|
||||
createEntity(childId, parentId),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([parentId, childId]);
|
||||
});
|
||||
|
||||
it('sorts multi-level hierarchy: grandparent -> parent -> child', () => {
|
||||
const grandparentId = uuidv4();
|
||||
const parentId = uuidv4();
|
||||
const childId = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(childId, parentId),
|
||||
createEntity(grandparentId),
|
||||
createEntity(parentId, grandparentId),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([grandparentId, parentId, childId]);
|
||||
});
|
||||
|
||||
it('handles multiple roots with children', () => {
|
||||
const rootA = uuidv4();
|
||||
const rootB = uuidv4();
|
||||
const childA = uuidv4();
|
||||
const childB = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(childB, rootB),
|
||||
createEntity(childA, rootA),
|
||||
createEntity(rootA),
|
||||
createEntity(rootB),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result.indexOf(rootA)).toBeLessThan(result.indexOf(childA));
|
||||
expect(result.indexOf(rootB)).toBeLessThan(result.indexOf(childB));
|
||||
expect(result).toHaveLength(4);
|
||||
});
|
||||
|
||||
it('ignores references to entities outside the batch', () => {
|
||||
const externalParentId = uuidv4();
|
||||
const itemA = uuidv4();
|
||||
const itemB = uuidv4();
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(itemA, externalParentId),
|
||||
createEntity(itemB),
|
||||
]) as never,
|
||||
});
|
||||
|
||||
expect(result).toEqual([itemA, itemB]);
|
||||
});
|
||||
|
||||
it('throws on cycles for entities without expected cycles', () => {
|
||||
const idA = uuidv4();
|
||||
const idB = uuidv4();
|
||||
|
||||
expect(() =>
|
||||
topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'navigationMenuItem',
|
||||
universalFlatEntityMaps: buildMaps([
|
||||
createEntity(idA, idB),
|
||||
createEntity(idB, idA),
|
||||
]) as never,
|
||||
}),
|
||||
).toThrow(/Cyclic self-referential foreign key detected/);
|
||||
});
|
||||
|
||||
it('appends cyclic entities for fieldMetadata (expected bidirectional cycles)', () => {
|
||||
const idA = uuidv4();
|
||||
const idB = uuidv4();
|
||||
|
||||
const maps = {
|
||||
byUniversalIdentifier: {
|
||||
[idA]: {
|
||||
universalIdentifier: idA,
|
||||
applicationUniversalIdentifier: APPLICATION_UNIVERSAL_IDENTIFIER,
|
||||
relationTargetFieldMetadataUniversalIdentifier: idB,
|
||||
},
|
||||
[idB]: {
|
||||
universalIdentifier: idB,
|
||||
applicationUniversalIdentifier: APPLICATION_UNIVERSAL_IDENTIFIER,
|
||||
relationTargetFieldMetadataUniversalIdentifier: idA,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const result = topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: 'fieldMetadata',
|
||||
universalFlatEntityMaps: maps as never,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toContain(idA);
|
||||
expect(result).toContain(idB);
|
||||
});
|
||||
});
|
||||
+139
@@ -0,0 +1,139 @@
|
||||
import { type AllMetadataName } from 'twenty-shared/metadata';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { ALL_MANY_TO_ONE_METADATA_RELATIONS } from 'src/engine/metadata-modules/flat-entity/constant/all-many-to-one-metadata-relations.constant';
|
||||
import { type MetadataUniversalFlatEntityMaps } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/metadata-universal-flat-entity-maps.type';
|
||||
|
||||
// fieldMetadata has bidirectional self-references (relation field A -> B and
|
||||
// B -> A) that are handled by a separate pre-allocation code path in the
|
||||
// runner. Cycles are expected and safe to append unsorted here.
|
||||
const METADATA_NAMES_WITH_EXPECTED_CYCLES = new Set<AllMetadataName>([
|
||||
'fieldMetadata',
|
||||
]);
|
||||
|
||||
const getSelfReferentialUniversalForeignKeys = (
|
||||
metadataName: AllMetadataName,
|
||||
): string[] =>
|
||||
Object.values(ALL_MANY_TO_ONE_METADATA_RELATIONS[metadataName])
|
||||
.filter(
|
||||
(
|
||||
relation,
|
||||
): relation is {
|
||||
metadataName: AllMetadataName;
|
||||
universalForeignKey: string;
|
||||
} => isDefined(relation) && relation.metadataName === metadataName,
|
||||
)
|
||||
.map((relation) => relation.universalForeignKey);
|
||||
|
||||
const getParentUniversalIdentifier = ({
|
||||
entity,
|
||||
selfReferentialUniversalForeignKeys,
|
||||
}: {
|
||||
entity: Record<string, unknown>;
|
||||
selfReferentialUniversalForeignKeys: string[];
|
||||
}): string | null =>
|
||||
selfReferentialUniversalForeignKeys.reduce<string | null>(
|
||||
(found, universalForeignKey) =>
|
||||
found ??
|
||||
(entity[universalForeignKey] as string | null | undefined) ??
|
||||
null,
|
||||
null,
|
||||
);
|
||||
|
||||
// Sorts universal identifiers so that parents come before children
|
||||
// for entities with self-referential FKs (e.g. navigationMenuItem.folder,
|
||||
// viewFilterGroup.parentViewFilterGroup). Returns original order unchanged
|
||||
// for entities without self-referential FKs.
|
||||
export const topologicallySortUniversalFlatEntitiesForSelfReferentialFks = <
|
||||
T extends AllMetadataName,
|
||||
>({
|
||||
metadataName,
|
||||
universalFlatEntityMaps,
|
||||
}: {
|
||||
metadataName: T;
|
||||
universalFlatEntityMaps: MetadataUniversalFlatEntityMaps<T>;
|
||||
}): string[] => {
|
||||
const selfReferentialUniversalForeignKeys =
|
||||
getSelfReferentialUniversalForeignKeys(metadataName);
|
||||
|
||||
const allUniversalIdentifiers = Object.keys(
|
||||
universalFlatEntityMaps.byUniversalIdentifier,
|
||||
);
|
||||
|
||||
if (selfReferentialUniversalForeignKeys.length === 0) {
|
||||
return allUniversalIdentifiers;
|
||||
}
|
||||
|
||||
const universalIdentifierSet = new Set(allUniversalIdentifiers);
|
||||
|
||||
const childrenByParent = new Map<string, string[]>();
|
||||
const inDegree = new Map<string, number>(
|
||||
allUniversalIdentifiers.map((id) => [id, 0]),
|
||||
);
|
||||
|
||||
for (const universalIdentifier of allUniversalIdentifiers) {
|
||||
const entity = universalFlatEntityMaps.byUniversalIdentifier[
|
||||
universalIdentifier
|
||||
] as Record<string, unknown> | undefined;
|
||||
|
||||
if (!isDefined(entity)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const parentId = getParentUniversalIdentifier({
|
||||
entity,
|
||||
selfReferentialUniversalForeignKeys,
|
||||
});
|
||||
|
||||
if (isDefined(parentId) && universalIdentifierSet.has(parentId)) {
|
||||
childrenByParent.set(parentId, [
|
||||
...(childrenByParent.get(parentId) ?? []),
|
||||
universalIdentifier,
|
||||
]);
|
||||
inDegree.set(
|
||||
universalIdentifier,
|
||||
(inDegree.get(universalIdentifier) ?? 0) + 1,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const roots = allUniversalIdentifiers.filter(
|
||||
(id) => (inDegree.get(id) ?? 0) === 0,
|
||||
);
|
||||
|
||||
const sorted = roots.reduce<string[]>((accumulator, root) => {
|
||||
accumulator.push(root);
|
||||
|
||||
for (let i = accumulator.length - 1; i < accumulator.length; i++) {
|
||||
const children = childrenByParent.get(accumulator[i]) ?? [];
|
||||
|
||||
accumulator.push(...children);
|
||||
}
|
||||
|
||||
return accumulator;
|
||||
}, []);
|
||||
|
||||
if (sorted.length < allUniversalIdentifiers.length) {
|
||||
if (!METADATA_NAMES_WITH_EXPECTED_CYCLES.has(metadataName)) {
|
||||
throw new Error(
|
||||
`Cyclic self-referential foreign key detected for ${metadataName}: ` +
|
||||
`expected ${allUniversalIdentifiers.length} entities but sorted ${sorted.length}. ` +
|
||||
`This entity does not use deferrable FKs, so cycles are not supported.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Bidirectional self-references (e.g. fieldMetadata relation pairs where
|
||||
// A -> B and B -> A) create cycles that cannot be topologically sorted.
|
||||
// These rely on DEFERRABLE INITIALLY DEFERRED FKs at the DB level,
|
||||
// so we append them at the end in their original order.
|
||||
const sortedSet = new Set(sorted);
|
||||
|
||||
for (const universalIdentifier of allUniversalIdentifiers) {
|
||||
if (!sortedSet.has(universalIdentifier)) {
|
||||
sorted.push(universalIdentifier);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return sorted;
|
||||
};
|
||||
+9
-1
@@ -27,6 +27,7 @@ import { resetUniversalFlatEntityForeignKeyAggregators } from 'src/engine/worksp
|
||||
import { flatEntityDeletedCreatedUpdatedMatrixDispatcher } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/utils/universal-flat-entity-deleted-created-updated-matrix-dispatcher.util';
|
||||
import { getMetadataEmptyWorkspaceMigrationActionRecord } from 'src/engine/workspace-manager/workspace-migration/utils/get-metadata-empty-workspace-migration-action-record.util';
|
||||
import { shouldInferDeletionFromMissingEntities } from 'src/engine/workspace-manager/workspace-migration/utils/should-infer-deletion-from-missing-entities.util';
|
||||
import { topologicallySortUniversalFlatEntitiesForSelfReferentialFks } from 'src/engine/workspace-manager/workspace-migration/utils/topologically-sort-universal-flat-entities-for-self-referential-fks.util';
|
||||
import { FlatEntityValidationError } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/types/failed-flat-entity-validation.type';
|
||||
import { FailedFlatEntityValidateAndBuild } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/failed-flat-entity-validate-and-build.type';
|
||||
import { SuccessfulFlatEntityValidateAndBuild } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/successful-flat-entity-validate-and-build.type';
|
||||
@@ -247,7 +248,14 @@ export abstract class WorkspaceEntityMigrationBuilderService<
|
||||
`EntityBuilder ${this.metadataName}`,
|
||||
'creation validation',
|
||||
);
|
||||
for (const flatEntityToCreateUniversalIdentifier in createdFlatEntityMaps.byUniversalIdentifier) {
|
||||
|
||||
const sortedCreateUniversalIdentifiers =
|
||||
topologicallySortUniversalFlatEntitiesForSelfReferentialFks({
|
||||
metadataName: this.metadataName,
|
||||
universalFlatEntityMaps: createdFlatEntityMaps,
|
||||
});
|
||||
|
||||
for (const flatEntityToCreateUniversalIdentifier of sortedCreateUniversalIdentifiers) {
|
||||
const rawUniversalflatEntityToCreate =
|
||||
findFlatEntityByUniversalIdentifierOrThrow({
|
||||
universalIdentifier: flatEntityToCreateUniversalIdentifier,
|
||||
|
||||
Reference in New Issue
Block a user