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
This commit is contained in:
Charles Bochet
2024-07-22 15:07:35 +02:00
committed by GitHub
parent 284e75791c
commit d212aedf81
7 changed files with 92 additions and 88 deletions

View File

@ -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 { EntitySchemaRelationFactory } from 'src/engine/twenty-orm/factories/entity-schema-relation.factory';
import { WorkspaceEntitiesStorage } from 'src/engine/twenty-orm/storage/workspace-entities.storage'; import { WorkspaceEntitiesStorage } from 'src/engine/twenty-orm/storage/workspace-entities.storage';
import { computeTableName } from 'src/engine/utils/compute-table-name.util'; import { computeTableName } from 'src/engine/utils/compute-table-name.util';
import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service';
@Injectable() @Injectable()
export class EntitySchemaFactory { export class EntitySchemaFactory {
constructor( constructor(
private readonly entitySchemaColumnFactory: EntitySchemaColumnFactory, private readonly entitySchemaColumnFactory: EntitySchemaColumnFactory,
private readonly entitySchemaRelationFactory: EntitySchemaRelationFactory, private readonly entitySchemaRelationFactory: EntitySchemaRelationFactory,
private readonly workspaceCacheStorageService: WorkspaceCacheStorageService,
) {} ) {}
async create( async create(

View File

@ -16,19 +16,20 @@ import {
SaveOptions, SaveOptions,
UpdateResult, UpdateResult,
} from 'typeorm'; } from 'typeorm';
import { PickKeysByType } from 'typeorm/common/PickKeysByType';
import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity';
import { UpsertOptions } from 'typeorm/repository/UpsertOptions'; 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 { 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 { 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 { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
import { isPlainObject } from 'src/utils/is-plain-object'; import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; 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< export class WorkspaceRepository<
Entity extends ObjectLiteral, Entity extends ObjectLiteral,
@ -607,10 +608,13 @@ export class WorkspaceRepository<
* PRIVATE METHODS * PRIVATE METHODS
*/ */
private async getObjectMetadataFromTarget() { private async getObjectMetadataFromTarget() {
const objectMetadataName = WorkspaceEntitiesStorage.getObjectMetadataName( const objectMetadataName =
this.internalContext.workspaceId, typeof this.target === 'string'
this.target as EntitySchema, ? this.target
); : WorkspaceEntitiesStorage.getObjectMetadataName(
this.internalContext.workspaceId,
this.target as EntitySchema,
);
if (!objectMetadataName) { if (!objectMetadataName) {
throw new Error('Object metadata name is missing'); throw new Error('Object metadata name is missing');
@ -751,20 +755,30 @@ export class WorkspaceRepository<
]); ]);
}), }),
); );
const relationMetadataMap = new Map( const relationMetadataMap = new Map(
objectMetadata.fields objectMetadata.fields
.filter(({ type }) => isRelationFieldMetadataType(type)) .filter(({ type }) => isRelationFieldMetadataType(type))
.map((fieldMetadata) => [ .map((fieldMetadata) => [
fieldMetadata.name, fieldMetadata.name,
fieldMetadata.fromRelationMetadata ?? {
fieldMetadata.toRelationMetadata, relationMetadata:
fieldMetadata.fromRelationMetadata ??
fieldMetadata.toRelationMetadata,
relationType: computeRelationType(
fieldMetadata,
fieldMetadata.fromRelationMetadata ??
fieldMetadata.toRelationMetadata,
),
},
]), ]),
); );
const newData: object = {}; const newData: object = {};
for (const [key, value] of Object.entries(data)) { for (const [key, value] of Object.entries(data)) {
const compositePropertyArgs = compositeFieldMetadataMap.get(key); const compositePropertyArgs = compositeFieldMetadataMap.get(key);
const relationMetadata = relationMetadataMap.get(key); const { relationMetadata, relationType } =
relationMetadataMap.get(key) ?? {};
if (!compositePropertyArgs && !relationMetadata) { if (!compositePropertyArgs && !relationMetadata) {
if (isPlainObject(value)) { if (isPlainObject(value)) {
@ -776,22 +790,38 @@ export class WorkspaceRepository<
} }
if (relationMetadata) { if (relationMetadata) {
const inverseSideObjectName = const toObjectMetadata =
relationMetadata.toObjectMetadata.nameSingular;
const objectMetadata =
await this.internalContext.workspaceCacheStorage.getObjectMetadata( await this.internalContext.workspaceCacheStorage.getObjectMetadata(
this.internalContext.workspaceId, relationMetadata.workspaceId,
(objectMetadata) => (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( throw new Error(
`Object metadata for object metadata "${inverseSideObjectName}" is missing`, `Object metadata for object metadataId "${relationMetadata.toObjectMetadataId}" is missing`,
); );
} }
newData[key] = await this.formatResult(value, objectMetadata); if (!fromObjectMetadata) {
throw new Error(
`Object metadata for object metadataId "${relationMetadata.fromObjectMetadataId}" is missing`,
);
}
newData[key] = await this.formatResult(
value,
relationType === 'one-to-many'
? toObjectMetadata
: fromObjectMetadata,
);
continue; continue;
} }

View File

@ -56,47 +56,7 @@ export class TwentyORMManager {
const workspaceId = this.workspaceDataSource.getWorkspaceId(); const workspaceId = this.workspaceDataSource.getWorkspaceId();
let objectMetadataCollection = return this.buildRepositoryForWorkspace<T>(workspaceId, objectMetadataName);
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<T>(entitySchema);
} }
async getRepositoryForWorkspace<T extends ObjectLiteral>( async getRepositoryForWorkspace<T extends ObjectLiteral>(
@ -115,9 +75,6 @@ export class TwentyORMManager {
): Promise< ): Promise<
WorkspaceRepository<T> | WorkspaceRepository<CustomWorkspaceEntity> WorkspaceRepository<T> | WorkspaceRepository<CustomWorkspaceEntity>
> { > {
const cacheVersion =
await this.workspaceCacheVersionService.getVersion(workspaceId);
let objectMetadataName: string; let objectMetadataName: string;
if (typeof entityClassOrobjectMetadataName === 'string') { if (typeof entityClassOrobjectMetadataName === 'string') {
@ -128,6 +85,23 @@ export class TwentyORMManager {
); );
} }
return this.buildRepositoryForWorkspace<T>(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 = let objectMetadataCollection =
await this.workspaceCacheStorageService.getObjectMetadataCollection( await this.workspaceCacheStorageService.getObjectMetadataCollection(
workspaceId, workspaceId,
@ -157,7 +131,7 @@ export class TwentyORMManager {
), ),
); );
const workspaceDataSource = await workspaceDataSourceCacheInstance.execute( return await workspaceDataSourceCacheInstance.execute(
`${workspaceId}-${cacheVersion}`, `${workspaceId}-${cacheVersion}`,
async () => { async () => {
const workspaceDataSource = const workspaceDataSource =
@ -167,28 +141,19 @@ export class TwentyORMManager {
}, },
(dataSource) => dataSource.destroy(), (dataSource) => dataSource.destroy(),
); );
}
const objectMetadata = objectMetadataCollection.find( async buildRepositoryForWorkspace<T extends ObjectLiteral>(
(objectMetadata) => objectMetadata.nameSingular === objectMetadataName, workspaceId: string,
); objectMetadataName: string,
) {
if (!objectMetadata) { const workspaceDataSource =
throw new Error('Object metadata not found'); await this.buildDatasourceForWorkspace(workspaceId);
}
const entitySchema = await this.entitySchemaFactory.create(
workspaceId,
objectMetadata,
);
if (!workspaceDataSource) { if (!workspaceDataSource) {
throw new Error('Workspace data source not found'); throw new Error('Workspace data source not found');
} }
if (!entitySchema) { return workspaceDataSource.getRepository<T>(objectMetadataName);
throw new Error('Entity schema not found');
}
return workspaceDataSource.getRepository<T>(entitySchema);
} }
} }

View File

@ -1,10 +1,10 @@
import { RelationType } from 'typeorm/metadata/types/RelationTypes'; 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 { 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 { 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 { 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 { interface RelationDetails {
relationType: RelationType; relationType: RelationType;
@ -31,7 +31,7 @@ export async function determineRelationDetails(
toObjectMetadata = await workspaceCacheStorageService.getObjectMetadata( toObjectMetadata = await workspaceCacheStorageService.getObjectMetadata(
workspaceId, workspaceId,
(objectMetadata) => (objectMetadata) =>
objectMetadata.id === relationMetadata.toObjectMetadataId, objectMetadata.id === relationMetadata.fromObjectMetadataId,
); );
} }

View File

@ -119,7 +119,10 @@ export class CalendarSaveEventsService {
const savedCalendarEventParticipantsToEmit: CalendarEventParticipantWorkspaceEntity[] = 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(eventsToSave, {}, transactionManager);
await calendarEventRepository.save( await calendarEventRepository.save(

View File

@ -1,6 +1,7 @@
import { Module } from '@nestjs/common'; import { Module } from '@nestjs/common';
import { TypeOrmModule } from '@nestjs/typeorm'; 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 { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-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'; 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({ @Module({
imports: [ imports: [
WorkspaceDataSourceModule, WorkspaceDataSourceModule,
WorkspaceModule,
TwentyORMModule.forFeature([CalendarEventParticipantWorkspaceEntity]), TwentyORMModule.forFeature([CalendarEventParticipantWorkspaceEntity]),
ObjectMetadataRepositoryModule.forFeature([PersonWorkspaceEntity]), ObjectMetadataRepositoryModule.forFeature([PersonWorkspaceEntity]),
TypeOrmModule.forFeature( TypeOrmModule.forFeature(

View File

@ -1,5 +1,6 @@
import { Scope } from '@nestjs/common'; 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 { Process } from 'src/engine/integrations/message-queue/decorators/process.decorator';
import { Processor } from 'src/engine/integrations/message-queue/decorators/processor.decorator'; import { Processor } from 'src/engine/integrations/message-queue/decorators/processor.decorator';
import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants'; import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants';
@ -19,6 +20,7 @@ export type CalendarEventParticipantMatchParticipantJobData = {
export class CalendarEventParticipantMatchParticipantJob { export class CalendarEventParticipantMatchParticipantJob {
constructor( constructor(
private readonly calendarEventParticipantService: CalendarEventParticipantService, private readonly calendarEventParticipantService: CalendarEventParticipantService,
private readonly workspaceService: WorkspaceService,
) {} ) {}
@Process(CalendarEventParticipantMatchParticipantJob.name) @Process(CalendarEventParticipantMatchParticipantJob.name)
@ -27,6 +29,10 @@ export class CalendarEventParticipantMatchParticipantJob {
): Promise<void> { ): Promise<void> {
const { workspaceId, email, personId, workspaceMemberId } = data; const { workspaceId, email, personId, workspaceMemberId } = data;
if (!this.workspaceService.isWorkspaceActivated(workspaceId)) {
return;
}
await this.calendarEventParticipantService.matchCalendarEventParticipants( await this.calendarEventParticipantService.matchCalendarEventParticipants(
workspaceId, workspaceId,
email, email,