From c72ecde09453573fa240d28eccf54c580ad27f51 Mon Sep 17 00:00:00 2001 From: Guillim Date: Tue, 17 Jun 2025 18:22:08 +0200 Subject: [PATCH] fixing index on relations (#12602) ## Why After the changes on relations, index on relations were skipped by the syncmetadata service, so no more migrations were generated for relation fields. We wanted to fix this. ## Test This PR adds unit tests for the `createIndexMigration` utility in the workspace migration builder. The tests cover: - Creating index migrations for simple fields (e.g., text fields) - Creating index migrations for relation fields (ensuring correct column naming, e.g., `authorId` for the `author` objectmetadataname) ## Excluded The delete index on relation does not need the column names so i don't think i needed to work on this method. I might be wrong. ## Checklist - [x] Added/updated unit tests for index migration creation - [x] Verified correct handling of simple and relation fields - [x] Ensured all tests pass --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Charles Bochet --- .../utils/compute-object-target-table.util.ts | 2 +- .../utils/is-field-metadata-of-type.util.ts | 4 +- ...space-migration-index.factory.util.spec.ts | 77 +++++++++ ...workspace-migration-index.factory.utils.ts | 161 ++++++++++++++++++ .../workspace-migration-index.factory.ts | 138 +-------------- .../factories/standard-index.factory.ts | 58 +++---- 6 files changed, 268 insertions(+), 172 deletions(-) create mode 100644 packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/__tests__/workspace-migration-index.factory.util.spec.ts create mode 100644 packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils.ts diff --git a/packages/twenty-server/src/engine/utils/compute-object-target-table.util.ts b/packages/twenty-server/src/engine/utils/compute-object-target-table.util.ts index 03430009d..fc4eb9ccb 100644 --- a/packages/twenty-server/src/engine/utils/compute-object-target-table.util.ts +++ b/packages/twenty-server/src/engine/utils/compute-object-target-table.util.ts @@ -3,7 +3,7 @@ import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metad import { computeTableName } from './compute-table-name.util'; export const computeObjectTargetTable = ( - objectMetadata: ObjectMetadataInterface, + objectMetadata: Pick, ) => { return computeTableName(objectMetadata.nameSingular, objectMetadata.isCustom); }; diff --git a/packages/twenty-server/src/engine/utils/is-field-metadata-of-type.util.ts b/packages/twenty-server/src/engine/utils/is-field-metadata-of-type.util.ts index 0b06f1309..45c5e5956 100644 --- a/packages/twenty-server/src/engine/utils/is-field-metadata-of-type.util.ts +++ b/packages/twenty-server/src/engine/utils/is-field-metadata-of-type.util.ts @@ -8,7 +8,7 @@ export function isFieldMetadataInterfaceOfType< Field extends FieldMetadataInterface, Type extends FieldMetadataType, >( - fieldMetadata: Field, + fieldMetadata: Pick, type: Type, ): fieldMetadata is Field & FieldMetadataInterface { return fieldMetadata.type === type; @@ -18,7 +18,7 @@ export function isFieldMetadataEntityOfType< Field extends FieldMetadataEntity, Type extends FieldMetadataType, >( - fieldMetadata: Field, + fieldMetadata: Pick, type: Type, ): fieldMetadata is Field & FieldMetadataEntity { return fieldMetadata.type === type; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/__tests__/workspace-migration-index.factory.util.spec.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/__tests__/workspace-migration-index.factory.util.spec.ts new file mode 100644 index 000000000..7c80bba03 --- /dev/null +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/__tests__/workspace-migration-index.factory.util.spec.ts @@ -0,0 +1,77 @@ +import { FieldMetadataType } from 'twenty-shared/types'; + +import { RelationType } from 'src/engine/metadata-modules/field-metadata/interfaces/relation-type.interface'; + +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; +import { IndexMetadataEntity } from 'src/engine/metadata-modules/index-metadata/index-metadata.entity'; +import { createIndexMigration } from 'src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils'; + +describe('WorkspaceMigrationIndexFactory', () => { + it('should create index migrations for simple fields', async () => { + const objectMetadata = { + id: 'obj1', + workspaceId: 'ws1', + nameSingular: 'Test', + fields: [{ id: 'f1', name: 'simpleField', type: FieldMetadataType.TEXT }], + isCustom: false, + }; + const indexMetadata = { + name: 'idx_simple', + isUnique: true, + indexType: 'BTREE', + indexWhereClause: null, + indexFieldMetadatas: [{ fieldMetadataId: 'f1', order: 0 }], + } as IndexMetadataEntity; + const map = new Map([[objectMetadata, [indexMetadata]]]); + const result = (await createIndexMigration(map)) as any; + + expect(result).toHaveLength(1); + const firstMigration = result[0].migrations[0]; + + expect(firstMigration.action).toBe('alter_indexes'); + expect(firstMigration.indexes[0].name).toBe('idx_simple'); + expect(firstMigration.indexes[0].columns).toEqual(['simpleField']); + expect(firstMigration.indexes[0].type).toBe('BTREE'); + expect(firstMigration.indexes[0].isUnique).toBe(true); + expect(firstMigration.indexes[0].where).toBe( + '"simpleField" != \'\' AND "deletedAt" IS NULL', + ); + }); + + it('should create index migrations for relation fields', async () => { + const fieldMetadata: Pick< + FieldMetadataEntity, + 'id' | 'name' | 'type' | 'settings' | 'isCustom' + > = { + id: 'f2', + name: 'author', + type: FieldMetadataType.RELATION, + settings: { + relationType: RelationType.MANY_TO_ONE, + joinColumnName: 'authorId', + }, + isCustom: false, + }; + + const objectMetadata = { + id: 'obj2', + workspaceId: 'ws1', + nameSingular: 'Attachment', + fields: [fieldMetadata], + isCustom: false, + }; + const indexMetadata = { + name: 'idx_rel', + isUnique: false, + indexType: 'BTREE', + indexWhereClause: null, + indexFieldMetadatas: [{ fieldMetadataId: 'f2', order: 0 }], + } as IndexMetadataEntity; + const map = new Map([[objectMetadata, [indexMetadata]]]); + const result = (await createIndexMigration(map)) as any; + + const firstMigration = result[0].migrations[0]; + + expect(firstMigration.indexes[0].columns).toEqual(['authorId']); + }); +}); diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils.ts new file mode 100644 index 000000000..73004d178 --- /dev/null +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils.ts @@ -0,0 +1,161 @@ +import { FieldMetadataType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { CompositeType } from 'src/engine/metadata-modules/field-metadata/interfaces/composite-type.interface'; + +import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; +import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; +import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; +import { IndexMetadataEntity } from 'src/engine/metadata-modules/index-metadata/index-metadata.entity'; +import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; +import { + WorkspaceMigrationEntity, + WorkspaceMigrationIndexActionType, + WorkspaceMigrationTableActionType, +} from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; +import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; +import { isFieldMetadataEntityOfType } from 'src/engine/utils/is-field-metadata-of-type.util'; + +export const createIndexMigration = async ( + indexMetadataByObjectMetadataMap: Map< + Pick< + ObjectMetadataEntity, + 'id' | 'workspaceId' | 'nameSingular' | 'isCustom' + > & { + fields: Pick[]; + }, + IndexMetadataEntity[] + >, +): Promise[]> => { + const workspaceMigrations: Partial[] = []; + + for (const [ + objectMetadata, + indexMetadataCollection, + ] of indexMetadataByObjectMetadataMap) { + const targetTable = computeObjectTargetTable(objectMetadata); + + const fieldsById = Object.fromEntries( + objectMetadata.fields.map((field) => [field.id, field]), + ); + + const indexes = indexMetadataCollection.map((indexMetadata) => { + const columns = indexMetadata.indexFieldMetadatas + .sort((a, b) => a.order - b.order) + .map((indexFieldMetadata) => { + const fieldMetadata = fieldsById[indexFieldMetadata.fieldMetadataId]; + + if (!fieldMetadata) { + throw new Error( + `Field metadata with id ${indexFieldMetadata.fieldMetadataId} not found in object metadata with id ${objectMetadata.id}`, + ); + } + + if ( + isFieldMetadataEntityOfType( + fieldMetadata, + FieldMetadataType.RELATION, + ) + ) { + if (!fieldMetadata.settings) { + throw new Error( + `Join column name is not supported for relation fields`, + ); + } + + return fieldMetadata.settings.joinColumnName; + } + + if (!isCompositeFieldMetadataType(fieldMetadata.type)) { + return fieldMetadata.name; + } + + const compositeType = compositeTypeDefinitions.get( + fieldMetadata.type, + ) as CompositeType; + + const columns = compositeType.properties + .filter((property) => property.isIncludedInUniqueConstraint) + .map((property) => + computeCompositeColumnName(fieldMetadata, property), + ); + + return columns; + }) + .flat() + .filter(isDefined); + + const defaultWhereClause = indexMetadata.isUnique + ? `${columns.map((column) => `"${column}"`).join(" != '' AND ")} != '' AND "deletedAt" IS NULL` + : null; + + return { + name: indexMetadata.name, + action: WorkspaceMigrationIndexActionType.CREATE, + isUnique: indexMetadata.isUnique, + columns, + type: indexMetadata.indexType, + where: indexMetadata.indexWhereClause ?? defaultWhereClause, + }; + }); + + workspaceMigrations.push({ + workspaceId: objectMetadata.workspaceId, + name: generateMigrationName( + `create-${objectMetadata.nameSingular}-indexes`, + ), + isCustom: false, + migrations: [ + { + name: targetTable, + action: WorkspaceMigrationTableActionType.ALTER_INDEXES, + indexes, + }, + ], + }); + } + + return workspaceMigrations; +}; + +export const deleteIndexMigration = async ( + indexMetadataByObjectMetadataMap: Map< + ObjectMetadataEntity, + IndexMetadataEntity[] + >, +): Promise[]> => { + const workspaceMigrations: Partial[] = []; + + for (const [ + objectMetadata, + indexMetadataCollection, + ] of indexMetadataByObjectMetadataMap) { + const targetTable = computeObjectTargetTable(objectMetadata); + + const indexes = indexMetadataCollection.map((indexMetadata) => ({ + name: indexMetadata.name, + action: WorkspaceMigrationIndexActionType.DROP, + columns: [], + isUnique: indexMetadata.isUnique, + })); + + workspaceMigrations.push({ + workspaceId: objectMetadata.workspaceId, + name: generateMigrationName( + `delete-${objectMetadata.nameSingular}-indexes`, + ), + isCustom: false, + migrations: [ + { + name: targetTable, + action: WorkspaceMigrationTableActionType.ALTER_INDEXES, + indexes, + }, + ], + }); + } + + return workspaceMigrations; +}; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-index.factory.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-index.factory.ts index 480d5f87c..39f93d38f 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-index.factory.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-index.factory.ts @@ -1,20 +1,14 @@ import { Injectable } from '@nestjs/common'; -import { CompositeType } from 'src/engine/metadata-modules/field-metadata/interfaces/composite-type.interface'; import { WorkspaceMigrationBuilderAction } from 'src/engine/workspace-manager/workspace-migration-builder/interfaces/workspace-migration-builder-action.interface'; -import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; -import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; -import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { IndexMetadataEntity } from 'src/engine/metadata-modules/index-metadata/index-metadata.entity'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; -import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; +import { WorkspaceMigrationEntity } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; import { - WorkspaceMigrationEntity, - WorkspaceMigrationIndexActionType, - WorkspaceMigrationTableActionType, -} from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; -import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; + createIndexMigration, + deleteIndexMigration, +} from 'src/engine/workspace-manager/workspace-migration-builder/factories/utils/workspace-migration-index.factory.utils'; @Injectable() export class WorkspaceMigrationIndexFactory { @@ -55,131 +49,11 @@ export class WorkspaceMigrationIndexFactory { switch (action) { case WorkspaceMigrationBuilderAction.CREATE: - return this.createIndexMigration(indexMetadataByObjectMetadataMap); + return createIndexMigration(indexMetadataByObjectMetadataMap); case WorkspaceMigrationBuilderAction.DELETE: - return this.deleteIndexMigration(indexMetadataByObjectMetadataMap); + return deleteIndexMigration(indexMetadataByObjectMetadataMap); default: return []; } } - - private async createIndexMigration( - indexMetadataByObjectMetadataMap: Map< - ObjectMetadataEntity, - IndexMetadataEntity[] - >, - ): Promise[]> { - const workspaceMigrations: Partial[] = []; - - for (const [ - objectMetadata, - indexMetadataCollection, - ] of indexMetadataByObjectMetadataMap) { - const targetTable = computeObjectTargetTable(objectMetadata); - - const fieldsById = Object.fromEntries( - objectMetadata.fields.map((field) => [field.id, field]), - ); - - const indexes = indexMetadataCollection.map((indexMetadata) => { - const columns = indexMetadata.indexFieldMetadatas - .sort((a, b) => a.order - b.order) - .map((indexFieldMetadata) => { - const fieldMetadata = - fieldsById[indexFieldMetadata.fieldMetadataId]; - - if (!fieldMetadata) { - throw new Error( - `Field metadata with id ${indexFieldMetadata.fieldMetadataId} not found in object metadata with id ${objectMetadata.id}`, - ); - } - - if (!isCompositeFieldMetadataType(fieldMetadata.type)) { - return fieldMetadata.name; - } - - const compositeType = compositeTypeDefinitions.get( - fieldMetadata.type, - ) as CompositeType; - - return compositeType.properties - .filter((property) => property.isIncludedInUniqueConstraint) - .map((property) => - computeCompositeColumnName(fieldMetadata, property), - ); - }) - .flat(); - - const defaultWhereClause = indexMetadata.isUnique - ? `${columns.map((column) => `"${column}"`).join(" != '' AND ")} != '' AND "deletedAt" IS NULL` - : null; - - return { - name: indexMetadata.name, - action: WorkspaceMigrationIndexActionType.CREATE, - isUnique: indexMetadata.isUnique, - columns, - type: indexMetadata.indexType, - where: indexMetadata.indexWhereClause ?? defaultWhereClause, - }; - }); - - workspaceMigrations.push({ - workspaceId: objectMetadata.workspaceId, - name: generateMigrationName( - `create-${objectMetadata.nameSingular}-indexes`, - ), - isCustom: false, - migrations: [ - { - name: targetTable, - action: WorkspaceMigrationTableActionType.ALTER_INDEXES, - indexes, - }, - ], - }); - } - - return workspaceMigrations; - } - - private async deleteIndexMigration( - indexMetadataByObjectMetadataMap: Map< - ObjectMetadataEntity, - IndexMetadataEntity[] - >, - ): Promise[]> { - const workspaceMigrations: Partial[] = []; - - for (const [ - objectMetadata, - indexMetadataCollection, - ] of indexMetadataByObjectMetadataMap) { - const targetTable = computeObjectTargetTable(objectMetadata); - - const indexes = indexMetadataCollection.map((indexMetadata) => ({ - name: indexMetadata.name, - action: WorkspaceMigrationIndexActionType.DROP, - columns: [], - isUnique: indexMetadata.isUnique, - })); - - workspaceMigrations.push({ - workspaceId: objectMetadata.workspaceId, - name: generateMigrationName( - `delete-${objectMetadata.nameSingular}-indexes`, - ), - isCustom: false, - migrations: [ - { - name: targetTable, - action: WorkspaceMigrationTableActionType.ALTER_INDEXES, - indexes, - }, - ], - }); - } - - return workspaceMigrations; - } } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/factories/standard-index.factory.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/factories/standard-index.factory.ts index 5de0407e9..1da3582e1 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/factories/standard-index.factory.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/factories/standard-index.factory.ts @@ -69,46 +69,30 @@ export class StandardIndexFactory { ); }); - return ( - workspaceIndexMetadataArgsCollection - .map((workspaceIndexMetadataArgs) => { - const objectMetadata = - originalStandardObjectMetadataMap[workspaceEntity.nameSingular]; + return workspaceIndexMetadataArgsCollection.map( + (workspaceIndexMetadataArgs) => { + const objectMetadata = + originalStandardObjectMetadataMap[workspaceEntity.nameSingular]; - if (!objectMetadata) { - throw new Error( - `Object metadata not found for ${workspaceEntity.nameSingular}`, - ); - } - - const indexMetadata: PartialIndexMetadata = { - workspaceId: context.workspaceId, - objectMetadataId: objectMetadata.id, - name: workspaceIndexMetadataArgs.name, - columns: workspaceIndexMetadataArgs.columns, - isUnique: workspaceIndexMetadataArgs.isUnique, - isCustom: false, - indexWhereClause: workspaceIndexMetadataArgs.whereClause, - indexType: workspaceIndexMetadataArgs.type, - }; - - return indexMetadata; - }) - // TODO: remove this filter when we have a way to handle index on relations - .filter((workspaceIndexMetadataArgs) => { - const objectMetadata = - originalStandardObjectMetadataMap[workspaceEntity.nameSingular]; - - const hasAllFields = workspaceIndexMetadataArgs.columns.every( - (expectedField) => { - return objectMetadata.fields.some( - (field) => field.name === expectedField, - ); - }, + if (!objectMetadata) { + throw new Error( + `Object metadata not found for ${workspaceEntity.nameSingular}`, ); + } - return hasAllFields; - }) + const indexMetadata: PartialIndexMetadata = { + workspaceId: context.workspaceId, + objectMetadataId: objectMetadata.id, + name: workspaceIndexMetadataArgs.name, + columns: workspaceIndexMetadataArgs.columns, + isUnique: workspaceIndexMetadataArgs.isUnique, + isCustom: false, + indexWhereClause: workspaceIndexMetadataArgs.whereClause, + indexType: workspaceIndexMetadataArgs.type, + }; + + return indexMetadata; + }, ); }