diff --git a/packages/twenty-server/src/engine/api/common/common-result-getters/__tests__/common-result-getters.service.spec.ts b/packages/twenty-server/src/engine/api/common/common-result-getters/__tests__/common-result-getters.service.spec.ts index 834ceddd35..45816a0edb 100644 --- a/packages/twenty-server/src/engine/api/common/common-result-getters/__tests__/common-result-getters.service.spec.ts +++ b/packages/twenty-server/src/engine/api/common/common-result-getters/__tests__/common-result-getters.service.spec.ts @@ -1,10 +1,12 @@ import { FieldMetadataType, FileFolder, + RelationType, type ObjectRecord, } from 'twenty-shared/types'; import { CommonResultGettersService } from 'src/engine/api/common/common-result-getters/common-result-getters.service'; +import { getFlatFieldsFromFlatObjectMetadata } from 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util'; import { type FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service'; import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant'; import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; @@ -14,6 +16,22 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m import { getFlatObjectMetadataMock } from 'src/engine/metadata-modules/flat-object-metadata/__mocks__/get-flat-object-metadata.mock'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +jest.mock( + 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util', + () => { + const actual = jest.requireActual( + 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util', + ); + + return { + ...actual, + getFlatFieldsFromFlatObjectMetadata: jest.fn( + actual.getFlatFieldsFromFlatObjectMetadata, + ), + }; + }, +); + const createField = ({ id, objectMetadataId, @@ -28,31 +46,39 @@ const createField = ({ ...overrides, }); -const buildFlatFieldMetadataMaps = ( - fields: FlatFieldMetadata[], -): FlatEntityMaps => - fields.reduce( - (maps, field) => +const buildFlatEntityMaps = < + TFlatEntity extends FlatFieldMetadata | FlatObjectMetadata, +>( + flatEntities: TFlatEntity[], +): FlatEntityMaps => + flatEntities.reduce( + (maps, flatEntity) => addFlatEntityToFlatEntityMapsOrThrow({ - flatEntity: field, + flatEntity, flatEntityMaps: maps, }), - createEmptyFlatEntityMaps() as FlatEntityMaps, + createEmptyFlatEntityMaps() as FlatEntityMaps, ); describe('CommonResultGettersService', () => { const signFileByIdUrl = jest.fn( async ({ fileId }: { fileId: string }) => `signed-${fileId}`, ); - const service = new CommonResultGettersService({ + const fileUrlService = { signFileByIdUrl, - } as unknown as FileUrlService); + } as unknown as FileUrlService; + + let service: CommonResultGettersService; + + beforeEach(() => { + service = new CommonResultGettersService(fileUrlService); + }); afterEach(() => { jest.clearAllMocks(); }); - it('runs a field handler once when multiple record fields share its type', async () => { + it('runs a shared field handler once and preserves fields without metadata', async () => { const objectMetadataId = 'document-object-id'; const filesField = createField({ id: 'document-files-field-id', @@ -73,7 +99,7 @@ describe('CommonResultGettersService', () => { namePlural: 'documents', fieldIds: [filesField.id, attachmentsField.id], }); - const flatFieldMetadataMaps = buildFlatFieldMetadataMaps([ + const flatFieldMetadataMaps = buildFlatEntityMaps([ filesField, attachmentsField, ]); @@ -95,9 +121,10 @@ describe('CommonResultGettersService', () => { extension: 'pdf', }, ], + ownerId: 'owner-id', }; - await service.processRecord( + const result = await service.processRecord( record, objectMetadata, flatObjectMetadataMaps, @@ -105,6 +132,25 @@ describe('CommonResultGettersService', () => { 'workspace-id', ); + expect(result).toEqual({ + ...record, + files: [ + { + fileId: 'file-id', + label: 'document', + extension: 'pdf', + url: 'signed-file-id', + }, + ], + attachments: [ + { + fileId: 'attachment-id', + label: 'attachment', + extension: 'pdf', + url: 'signed-attachment-id', + }, + ], + }); expect(signFileByIdUrl).toHaveBeenCalledTimes(2); expect(signFileByIdUrl).toHaveBeenCalledWith({ fileId: 'file-id', @@ -117,4 +163,88 @@ describe('CommonResultGettersService', () => { fileFolder: FileFolder.FilesField, }); }); + + it('resolves field metadata once per object type within a single invocation', async () => { + const companyObjectMetadataId = 'company-object-id'; + const personObjectMetadataId = 'person-object-id'; + const companyNameField = createField({ + id: 'company-name-field-id', + name: 'name', + objectMetadataId: companyObjectMetadataId, + type: FieldMetadataType.TEXT, + }); + const companyPeopleField = createField({ + id: 'company-people-field-id', + name: 'people', + objectMetadataId: companyObjectMetadataId, + type: FieldMetadataType.RELATION, + relationTargetObjectMetadataId: personObjectMetadataId, + settings: { relationType: RelationType.ONE_TO_MANY }, + }); + const personNameField = createField({ + id: 'person-name-field-id', + name: 'name', + objectMetadataId: personObjectMetadataId, + type: FieldMetadataType.TEXT, + }); + const companyObjectMetadata = getFlatObjectMetadataMock({ + id: companyObjectMetadataId, + universalIdentifier: 'company-object-universal-identifier', + nameSingular: 'company', + namePlural: 'companies', + fieldIds: [companyNameField.id, companyPeopleField.id], + }); + const personObjectMetadata = getFlatObjectMetadataMock({ + id: personObjectMetadataId, + universalIdentifier: 'person-object-universal-identifier', + nameSingular: 'person', + namePlural: 'people', + fieldIds: [personNameField.id], + }); + const flatFieldMetadataMaps = buildFlatEntityMaps([ + companyNameField, + companyPeopleField, + personNameField, + ]); + const flatObjectMetadataMaps = buildFlatEntityMaps([ + companyObjectMetadata, + personObjectMetadata, + ]); + const records: ObjectRecord[] = [ + { + id: 'company-1', + name: 'Acme', + people: [ + { id: 'person-1', name: 'Alice' }, + { id: 'person-2', name: 'Bob' }, + ], + }, + { + id: 'company-2', + name: 'Globex', + people: [{ id: 'person-3', name: 'Carol' }], + }, + ]; + + const result = await service.processRecordArray( + records, + companyObjectMetadata, + flatObjectMetadataMaps, + flatFieldMetadataMaps, + 'workspace-id', + ); + + expect(result).toEqual(records); + expect(getFlatFieldsFromFlatObjectMetadata).toHaveBeenCalledTimes(2); + + await service.processRecordArray( + records, + companyObjectMetadata, + flatObjectMetadataMaps, + flatFieldMetadataMaps, + 'workspace-id', + ); + + expect(getFlatFieldsFromFlatObjectMetadata).toHaveBeenCalledTimes(4); + }); }); diff --git a/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts b/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts index c0181d4f5c..7dc9accbf8 100644 --- a/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-result-getters/common-result-getters.service.ts @@ -13,18 +13,24 @@ import { type QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/w import { FilesFieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/files-field-query-result-getter.handler'; import { RichTextFieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-field-query-result-getter.handler'; import { WorkspaceMemberQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler'; +import { getFlatFieldsFromFlatObjectMetadata } from 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util'; import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service'; import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util'; -import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util'; import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; -import { - buildFieldMapsFromFlatObjectMetadata, - type FieldMapsForObject, -} from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util'; import { isFlatFieldMetadataOfType } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-flat-field-metadata-of-type.util'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +type ResultProcessingContext = { + flatObjectMetadataMaps: FlatEntityMaps; + flatFieldMetadataMaps: FlatEntityMaps; + workspaceId: string; + fieldMetadataByNameByObjectMetadataId: Map< + string, + ReadonlyMap + >; +}; + // TODO: find a way to prevent conflict between handlers executing logic on object relations // And this factory that is also executing logic on object relations // Right now the factory will override any change made on relations by the handlers @@ -66,60 +72,66 @@ export class CommonResultGettersService { ]); } - public async processRecordArray( + public processRecordArray( recordArray: ObjectRecord[], flatObjectMetadata: FlatObjectMetadata, flatObjectMetadataMaps: FlatEntityMaps, flatFieldMetadataMaps: FlatEntityMaps, workspaceId: string, - ) { - const fieldMaps = buildFieldMapsFromFlatObjectMetadata( + ): Promise { + return this.processRecordArrayWithContext(recordArray, flatObjectMetadata, { + flatObjectMetadataMaps, flatFieldMetadataMaps, - flatObjectMetadata, - ); - - return await Promise.all( - recordArray.map( - async (record: ObjectRecord) => - await this.processRecord( - record, - flatObjectMetadata, - flatObjectMetadataMaps, - flatFieldMetadataMaps, - workspaceId, - fieldMaps, - ), - ), - ); + workspaceId, + fieldMetadataByNameByObjectMetadataId: new Map(), + }); } - public async processRecord( + public processRecord( record: ObjectRecord, flatObjectMetadata: FlatObjectMetadata, flatObjectMetadataMaps: FlatEntityMaps, flatFieldMetadataMaps: FlatEntityMaps, workspaceId: string, - fieldMapsForObject?: FieldMapsForObject, ): Promise { - const fieldMaps = - fieldMapsForObject ?? - buildFieldMapsFromFlatObjectMetadata( - flatFieldMetadataMaps, - flatObjectMetadata, - ); + return this.processRecordWithContext(record, flatObjectMetadata, { + flatObjectMetadataMaps, + flatFieldMetadataMaps, + workspaceId, + fieldMetadataByNameByObjectMetadataId: new Map(), + }); + } - const { fieldIdByName } = fieldMaps; + private processRecordArrayWithContext( + recordArray: ObjectRecord[], + flatObjectMetadata: FlatObjectMetadata, + context: ResultProcessingContext, + ): Promise { + return Promise.all( + recordArray.map((record) => + this.processRecordWithContext(record, flatObjectMetadata, context), + ), + ); + } + + private async processRecordWithContext( + record: ObjectRecord, + flatObjectMetadata: FlatObjectMetadata, + context: ResultProcessingContext, + ): Promise { + const fieldMetadataByName = this.getOrBuildFieldMetadataByName( + flatObjectMetadata, + context, + ); + const recordFieldMetadataList = Object.keys(record) + .map((recordFieldName) => fieldMetadataByName.get(recordFieldName)) + .filter(isDefined); const fieldHandlers = new Set( - Object.keys(record) - .map((recordFieldName) => - findFlatEntityByIdInFlatEntityMaps({ - flatEntityId: fieldIdByName[recordFieldName], - flatEntityMaps: flatFieldMetadataMaps, - }), + recordFieldMetadataList + .map((recordFieldMetadata) => + this.fieldHandlers.get(recordFieldMetadata.type), ) - .filter(isDefined) - .map((fieldMetadata) => this.fieldHandlers.get(fieldMetadata.type)) .filter(isDefined), ); @@ -128,17 +140,13 @@ export class CommonResultGettersService { ...fieldHandlers, ]; - const relationFields = Object.keys(record) - .map((recordFieldName) => - findFlatEntityByIdInFlatEntityMaps({ - flatEntityId: fieldIdByName[recordFieldName], - flatEntityMaps: flatFieldMetadataMaps, - }), - ) - .filter(isDefined) - .filter((fieldMetadata) => - isFlatFieldMetadataOfType(fieldMetadata, FieldMetadataType.RELATION), - ); + const relationFields = recordFieldMetadataList.filter( + (recordFieldMetadata) => + isFlatFieldMetadataOfType( + recordFieldMetadata, + FieldMetadataType.RELATION, + ), + ); const relationFieldsProcessedMap = {} as Record< string, @@ -159,50 +167,61 @@ export class CommonResultGettersService { const targetFlatObjectMetadata = findFlatEntityByIdInFlatEntityMapsOrThrow({ flatEntityId: relationField.relationTargetObjectMetadataId, - flatEntityMaps: flatObjectMetadataMaps, + flatEntityMaps: context.flatObjectMetadataMaps, }); relationFieldsProcessedMap[relationField.name] = relationField.settings?.relationType === RelationType.ONE_TO_MANY - ? await this.processRecordArray( + ? await this.processRecordArrayWithContext( record[relationField.name], targetFlatObjectMetadata, - flatObjectMetadataMaps, - flatFieldMetadataMaps, - workspaceId, + context, ) - : await this.processRecord( + : await this.processRecordWithContext( record[relationField.name], targetFlatObjectMetadata, - flatObjectMetadataMaps, - flatFieldMetadataMaps, - workspaceId, + context, ); } - const fieldMetadata = Object.keys(record) - .map((recordFieldName) => - findFlatEntityByIdInFlatEntityMaps({ - flatEntityId: fieldIdByName[recordFieldName], - flatEntityMaps: flatFieldMetadataMaps, - }), - ) - .filter(isDefined); - const objectRecordProcessedWithoutRelationFields = await this.processObjectRecordWithoutRelationFields( record, - workspaceId, + context.workspaceId, handlers, - fieldMetadata, + recordFieldMetadataList, ); - const processedRecord = { + return { ...objectRecordProcessedWithoutRelationFields, ...relationFieldsProcessedMap, }; + } - return processedRecord; + private getOrBuildFieldMetadataByName( + flatObjectMetadata: FlatObjectMetadata, + context: ResultProcessingContext, + ): ReadonlyMap { + const cachedFieldMetadataByName = + context.fieldMetadataByNameByObjectMetadataId.get(flatObjectMetadata.id); + + if (isDefined(cachedFieldMetadataByName)) { + return cachedFieldMetadataByName; + } + + const fieldMetadataByName = new Map( + getFlatFieldsFromFlatObjectMetadata( + flatObjectMetadata, + context.flatFieldMetadataMaps, + ).map((fieldMetadata) => [fieldMetadata.name, fieldMetadata]), + ); + + context.fieldMetadataByNameByObjectMetadataId.set( + flatObjectMetadata.id, + fieldMetadataByName, + ); + + return fieldMetadataByName; } private async processObjectRecordWithoutRelationFields( @@ -228,10 +247,7 @@ export class CommonResultGettersService { objectType: string, ): QueryResultGetterHandlerInterface { return ( - (this.objectHandlers.get(objectType) || { - handle: (result: ObjectRecord): Promise => - Promise.resolve(result), - }) ?? { + this.objectHandlers.get(objectType) ?? { handle: (result: ObjectRecord): Promise => Promise.resolve(result), }