Skip to content

Commit

Permalink
Fix ORM (#6363)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
charlesBochet authored Jul 22, 2024
1 parent 284e757 commit d212aed
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
}

Expand Down
89 changes: 27 additions & 62 deletions packages/twenty-server/src/engine/twenty-orm/twenty-orm.manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(entitySchema);
return this.buildRepositoryForWorkspace<T>(workspaceId, objectMetadataName);
}

async getRepositoryForWorkspace<T extends ObjectLiteral>(
Expand All @@ -115,9 +75,6 @@ export class TwentyORMManager {
): Promise<
WorkspaceRepository<T> | WorkspaceRepository<CustomWorkspaceEntity>
> {
const cacheVersion =
await this.workspaceCacheVersionService.getVersion(workspaceId);

let objectMetadataName: string;

if (typeof entityClassOrobjectMetadataName === 'string') {
Expand All @@ -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 =
await this.workspaceCacheStorageService.getObjectMetadataCollection(
workspaceId,
Expand Down Expand Up @@ -157,7 +131,7 @@ export class TwentyORMManager {
),
);

const workspaceDataSource = await workspaceDataSourceCacheInstance.execute(
return await workspaceDataSourceCacheInstance.execute(
`${workspaceId}-${cacheVersion}`,
async () => {
const workspaceDataSource =
Expand All @@ -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<T extends ObjectLiteral>(
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<T>(entitySchema);
return workspaceDataSource.getRepository<T>(objectMetadataName);
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -31,7 +31,7 @@ export async function determineRelationDetails(
toObjectMetadata = await workspaceCacheStorageService.getObjectMetadata(
workspaceId,
(objectMetadata) =>
objectMetadata.id === relationMetadata.toObjectMetadataId,
objectMetadata.id === relationMetadata.fromObjectMetadataId,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -19,6 +20,7 @@ export type CalendarEventParticipantMatchParticipantJobData = {
export class CalendarEventParticipantMatchParticipantJob {
constructor(
private readonly calendarEventParticipantService: CalendarEventParticipantService,
private readonly workspaceService: WorkspaceService,
) {}

@Process(CalendarEventParticipantMatchParticipantJob.name)
Expand All @@ -27,6 +29,10 @@ export class CalendarEventParticipantMatchParticipantJob {
): Promise<void> {
const { workspaceId, email, personId, workspaceMemberId } = data;

if (!this.workspaceService.isWorkspaceActivated(workspaceId)) {
return;
}

await this.calendarEventParticipantService.matchCalendarEventParticipants(
workspaceId,
email,
Expand Down

0 comments on commit d212aed

Please sign in to comment.