From d212aedf811af4859b10c23b0441b0a94fde652d Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Mon, 22 Jul 2024 15:07:35 +0200 Subject: [PATCH] Fix ORM (#6363) This PR fixes a few bugs on TwentyORM: - fix many to one relations that were not properly queries - fix many to one relations that were not properly parsed - compute datasource (or use from cache) at run-time and do not use injected one that could be outdated We still need to refactor it to simplify, I feel the API are too complex and we have too many cache layers. Also the relation computation part is very complex and bug prone --- .../factories/entity-schema.factory.ts | 2 - .../repository/workspace.repository.ts | 70 ++++++++++----- .../engine/twenty-orm/twenty-orm.manager.ts | 89 ++++++------------- .../utils/determine-relation-details.util.ts | 6 +- .../services/calendar-save-events.service.ts | 5 +- ...lendar-event-participant-manager.module.ts | 2 + ...event-participant-match-participant.job.ts | 6 ++ 7 files changed, 92 insertions(+), 88 deletions(-) diff --git a/packages/twenty-server/src/engine/twenty-orm/factories/entity-schema.factory.ts b/packages/twenty-server/src/engine/twenty-orm/factories/entity-schema.factory.ts index 423cf458f88a..348dc5fa632c 100644 --- a/packages/twenty-server/src/engine/twenty-orm/factories/entity-schema.factory.ts +++ b/packages/twenty-server/src/engine/twenty-orm/factories/entity-schema.factory.ts @@ -7,14 +7,12 @@ import { EntitySchemaColumnFactory } from 'src/engine/twenty-orm/factories/entit import { EntitySchemaRelationFactory } from 'src/engine/twenty-orm/factories/entity-schema-relation.factory'; import { WorkspaceEntitiesStorage } from 'src/engine/twenty-orm/storage/workspace-entities.storage'; import { computeTableName } from 'src/engine/utils/compute-table-name.util'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; @Injectable() export class EntitySchemaFactory { constructor( private readonly entitySchemaColumnFactory: EntitySchemaColumnFactory, private readonly entitySchemaRelationFactory: EntitySchemaRelationFactory, - private readonly workspaceCacheStorageService: WorkspaceCacheStorageService, ) {} async create( diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts index 2c77677f3064..1e2dc264928d 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts @@ -16,19 +16,20 @@ import { SaveOptions, UpdateResult, } from 'typeorm'; +import { PickKeysByType } from 'typeorm/common/PickKeysByType'; import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; import { UpsertOptions } from 'typeorm/repository/UpsertOptions'; -import { PickKeysByType } from 'typeorm/common/PickKeysByType'; import { WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; -import { WorkspaceEntitiesStorage } from 'src/engine/twenty-orm/storage/workspace-entities.storage'; -import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types'; import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; -import { isPlainObject } from 'src/utils/is-plain-object'; -import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util'; +import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { WorkspaceEntitiesStorage } from 'src/engine/twenty-orm/storage/workspace-entities.storage'; +import { computeRelationType } from 'src/engine/twenty-orm/utils/compute-relation-type.util'; +import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util'; +import { isPlainObject } from 'src/utils/is-plain-object'; export class WorkspaceRepository< Entity extends ObjectLiteral, @@ -607,10 +608,13 @@ export class WorkspaceRepository< * PRIVATE METHODS */ private async getObjectMetadataFromTarget() { - const objectMetadataName = WorkspaceEntitiesStorage.getObjectMetadataName( - this.internalContext.workspaceId, - this.target as EntitySchema, - ); + const objectMetadataName = + typeof this.target === 'string' + ? this.target + : WorkspaceEntitiesStorage.getObjectMetadataName( + this.internalContext.workspaceId, + this.target as EntitySchema, + ); if (!objectMetadataName) { throw new Error('Object metadata name is missing'); @@ -751,20 +755,30 @@ export class WorkspaceRepository< ]); }), ); + const relationMetadataMap = new Map( objectMetadata.fields .filter(({ type }) => isRelationFieldMetadataType(type)) .map((fieldMetadata) => [ fieldMetadata.name, - fieldMetadata.fromRelationMetadata ?? - fieldMetadata.toRelationMetadata, + { + relationMetadata: + fieldMetadata.fromRelationMetadata ?? + fieldMetadata.toRelationMetadata, + relationType: computeRelationType( + fieldMetadata, + fieldMetadata.fromRelationMetadata ?? + fieldMetadata.toRelationMetadata, + ), + }, ]), ); const newData: object = {}; for (const [key, value] of Object.entries(data)) { const compositePropertyArgs = compositeFieldMetadataMap.get(key); - const relationMetadata = relationMetadataMap.get(key); + const { relationMetadata, relationType } = + relationMetadataMap.get(key) ?? {}; if (!compositePropertyArgs && !relationMetadata) { if (isPlainObject(value)) { @@ -776,22 +790,38 @@ export class WorkspaceRepository< } if (relationMetadata) { - const inverseSideObjectName = - relationMetadata.toObjectMetadata.nameSingular; - const objectMetadata = + const toObjectMetadata = await this.internalContext.workspaceCacheStorage.getObjectMetadata( - this.internalContext.workspaceId, + relationMetadata.workspaceId, (objectMetadata) => - objectMetadata.nameSingular === inverseSideObjectName, + objectMetadata.id === relationMetadata.toObjectMetadataId, ); - if (!objectMetadata) { + const fromObjectMetadata = + await this.internalContext.workspaceCacheStorage.getObjectMetadata( + relationMetadata.workspaceId, + (objectMetadata) => + objectMetadata.id === relationMetadata.fromObjectMetadataId, + ); + + if (!toObjectMetadata) { + throw new Error( + `Object metadata for object metadataId "${relationMetadata.toObjectMetadataId}" is missing`, + ); + } + + if (!fromObjectMetadata) { throw new Error( - `Object metadata for object metadata "${inverseSideObjectName}" is missing`, + `Object metadata for object metadataId "${relationMetadata.fromObjectMetadataId}" is missing`, ); } - newData[key] = await this.formatResult(value, objectMetadata); + newData[key] = await this.formatResult( + value, + relationType === 'one-to-many' + ? toObjectMetadata + : fromObjectMetadata, + ); continue; } diff --git a/packages/twenty-server/src/engine/twenty-orm/twenty-orm.manager.ts b/packages/twenty-server/src/engine/twenty-orm/twenty-orm.manager.ts index ce53d3a14fb6..3fb9a0bb1e82 100644 --- a/packages/twenty-server/src/engine/twenty-orm/twenty-orm.manager.ts +++ b/packages/twenty-server/src/engine/twenty-orm/twenty-orm.manager.ts @@ -56,47 +56,7 @@ export class TwentyORMManager { const workspaceId = this.workspaceDataSource.getWorkspaceId(); - let objectMetadataCollection = - await this.workspaceCacheStorageService.getObjectMetadataCollection( - workspaceId, - ); - - if (!objectMetadataCollection) { - objectMetadataCollection = await this.objectMetadataRepository.find({ - where: { workspaceId }, - relations: [ - 'fields.object', - 'fields', - 'fields.fromRelationMetadata', - 'fields.toRelationMetadata', - 'fields.fromRelationMetadata.toObjectMetadata', - ], - }); - - await this.workspaceCacheStorageService.setObjectMetadataCollection( - workspaceId, - objectMetadataCollection, - ); - } - - const objectMetadata = objectMetadataCollection.find( - (objectMetadata) => objectMetadata.nameSingular === objectMetadataName, - ); - - if (!objectMetadata) { - throw new Error('Object metadata not found'); - } - - const entitySchema = await this.entitySchemaFactory.create( - workspaceId, - objectMetadata, - ); - - if (!entitySchema) { - throw new Error('Entity schema not found'); - } - - return this.workspaceDataSource.getRepository(entitySchema); + return this.buildRepositoryForWorkspace(workspaceId, objectMetadataName); } async getRepositoryForWorkspace( @@ -115,9 +75,6 @@ export class TwentyORMManager { ): Promise< WorkspaceRepository | WorkspaceRepository > { - const cacheVersion = - await this.workspaceCacheVersionService.getVersion(workspaceId); - let objectMetadataName: string; if (typeof entityClassOrobjectMetadataName === 'string') { @@ -128,6 +85,23 @@ export class TwentyORMManager { ); } + return this.buildRepositoryForWorkspace(workspaceId, objectMetadataName); + } + + async getWorkspaceDatasource() { + if (!this.workspaceDataSource) { + throw new Error('Workspace data source not found'); + } + + const workspaceId = this.workspaceDataSource.getWorkspaceId(); + + return this.buildDatasourceForWorkspace(workspaceId); + } + + async buildDatasourceForWorkspace(workspaceId: string) { + const cacheVersion = + await this.workspaceCacheVersionService.getVersion(workspaceId); + let objectMetadataCollection = await this.workspaceCacheStorageService.getObjectMetadataCollection( workspaceId, @@ -157,7 +131,7 @@ export class TwentyORMManager { ), ); - const workspaceDataSource = await workspaceDataSourceCacheInstance.execute( + return await workspaceDataSourceCacheInstance.execute( `${workspaceId}-${cacheVersion}`, async () => { const workspaceDataSource = @@ -167,28 +141,19 @@ export class TwentyORMManager { }, (dataSource) => dataSource.destroy(), ); + } - const objectMetadata = objectMetadataCollection.find( - (objectMetadata) => objectMetadata.nameSingular === objectMetadataName, - ); - - if (!objectMetadata) { - throw new Error('Object metadata not found'); - } - - const entitySchema = await this.entitySchemaFactory.create( - workspaceId, - objectMetadata, - ); + async buildRepositoryForWorkspace( + workspaceId: string, + objectMetadataName: string, + ) { + const workspaceDataSource = + await this.buildDatasourceForWorkspace(workspaceId); if (!workspaceDataSource) { throw new Error('Workspace data source not found'); } - if (!entitySchema) { - throw new Error('Entity schema not found'); - } - - return workspaceDataSource.getRepository(entitySchema); + return workspaceDataSource.getRepository(objectMetadataName); } } diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/determine-relation-details.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/determine-relation-details.util.ts index 282c2ac26e0f..bcd8cb6b1b82 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/determine-relation-details.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/determine-relation-details.util.ts @@ -1,10 +1,10 @@ import { RelationType } from 'typeorm/metadata/types/RelationTypes'; -import { RelationMetadataEntity } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { RelationMetadataEntity } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; import { computeRelationType } from 'src/engine/twenty-orm/utils/compute-relation-type.util'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; interface RelationDetails { relationType: RelationType; @@ -31,7 +31,7 @@ export async function determineRelationDetails( toObjectMetadata = await workspaceCacheStorageService.getObjectMetadata( workspaceId, (objectMetadata) => - objectMetadata.id === relationMetadata.toObjectMetadataId, + objectMetadata.id === relationMetadata.fromObjectMetadataId, ); } diff --git a/packages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-save-events.service.ts b/packages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-save-events.service.ts index 3702ad2d323f..1dde06ef4ff8 100644 --- a/packages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-save-events.service.ts +++ b/packages/twenty-server/src/modules/calendar/calendar-event-import-manager/services/calendar-save-events.service.ts @@ -119,7 +119,10 @@ export class CalendarSaveEventsService { const savedCalendarEventParticipantsToEmit: CalendarEventParticipantWorkspaceEntity[] = []; - await this.workspaceDataSource?.transaction(async (transactionManager) => { + const workspaceDataSource = + await this.twentyORMManager.getWorkspaceDatasource(); + + await workspaceDataSource?.transaction(async (transactionManager) => { await calendarEventRepository.save(eventsToSave, {}, transactionManager); await calendarEventRepository.save( diff --git a/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/calendar-event-participant-manager.module.ts b/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/calendar-event-participant-manager.module.ts index 609c83429af4..3f8cb101b542 100644 --- a/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/calendar-event-participant-manager.module.ts +++ b/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/calendar-event-participant-manager.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; +import { WorkspaceModule } from 'src/engine/core-modules/workspace/workspace.module'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { ObjectMetadataRepositoryModule } from 'src/engine/object-metadata-repository/object-metadata-repository.module'; @@ -22,6 +23,7 @@ import { PersonWorkspaceEntity } from 'src/modules/person/standard-objects/perso @Module({ imports: [ WorkspaceDataSourceModule, + WorkspaceModule, TwentyORMModule.forFeature([CalendarEventParticipantWorkspaceEntity]), ObjectMetadataRepositoryModule.forFeature([PersonWorkspaceEntity]), TypeOrmModule.forFeature( diff --git a/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/jobs/calendar-event-participant-match-participant.job.ts b/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/jobs/calendar-event-participant-match-participant.job.ts index e8dc28c0c117..e194a64462b2 100644 --- a/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/jobs/calendar-event-participant-match-participant.job.ts +++ b/packages/twenty-server/src/modules/calendar/calendar-event-participant-manager/jobs/calendar-event-participant-match-participant.job.ts @@ -1,5 +1,6 @@ import { Scope } from '@nestjs/common'; +import { WorkspaceService } from 'src/engine/core-modules/workspace/services/workspace.service'; import { Process } from 'src/engine/integrations/message-queue/decorators/process.decorator'; import { Processor } from 'src/engine/integrations/message-queue/decorators/processor.decorator'; import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants'; @@ -19,6 +20,7 @@ export type CalendarEventParticipantMatchParticipantJobData = { export class CalendarEventParticipantMatchParticipantJob { constructor( private readonly calendarEventParticipantService: CalendarEventParticipantService, + private readonly workspaceService: WorkspaceService, ) {} @Process(CalendarEventParticipantMatchParticipantJob.name) @@ -27,6 +29,10 @@ export class CalendarEventParticipantMatchParticipantJob { ): Promise { const { workspaceId, email, personId, workspaceMemberId } = data; + if (!this.workspaceService.isWorkspaceActivated(workspaceId)) { + return; + } + await this.calendarEventParticipantService.matchCalendarEventParticipants( workspaceId, email,