Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: extend twenty orm #5238

Merged
merged 4 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import {
DataSource,
EntityManager,
EntityTarget,
ObjectLiteral,
QueryRunner,
} from 'typeorm';

import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
import { WorkspaceEntityManager } from 'src/engine/twenty-orm/entity-manager/entity.manager';

export class WorkspaceDataSource extends DataSource {
readonly manager: WorkspaceEntityManager;

override getRepository<Entity extends ObjectLiteral>(
target: EntityTarget<Entity>,
): WorkspaceRepository<Entity> {
return this.manager.getRepository(target);
}

override createEntityManager(queryRunner?: QueryRunner): EntityManager {
return new WorkspaceEntityManager(this, queryRunner);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ import { Inject } from '@nestjs/common';

import { TWENTY_ORM_WORKSPACE_DATASOURCE } from 'src/engine/twenty-orm/twenty-orm.constants';

// nit: The datasource can be null if it's used outside of an authenticated request context
export const InjectWorkspaceDatasource = () =>
Inject(TWENTY_ORM_WORKSPACE_DATASOURCE);
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EntityClassOrSchema } from '@nestjs/typeorm/dist/interfaces/entity-clas

import { getWorkspaceRepositoryToken } from 'src/engine/twenty-orm/utils/get-workspace-repository-token.util';

// nit: The repository can be null if it's used outside of an authenticated request context
export const InjectWorkspaceRepository = (
entity: EntityClassOrSchema,
): ReturnType<typeof Inject> => Inject(getWorkspaceRepositoryToken(entity));
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { EntityManager, EntityTarget, ObjectLiteral } from 'typeorm';

import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';

export class WorkspaceEntityManager extends EntityManager {
override getRepository<Entity extends ObjectLiteral>(
target: EntityTarget<Entity>,
): WorkspaceRepository<Entity> {
// find already created repository instance and return it if found
const repoFromMap = this.repositories.get(target);

if (repoFromMap) return repoFromMap as WorkspaceRepository<Entity>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's avoid using abbreviations (repo) in the code base. Also let's keep the { } around one liner if content instructions per project convention


const newRepository = new WorkspaceRepository<Entity>(
target,
this,
this.queryRunner,
);

this.repositories.set(target, newRepository);

return newRepository;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { EntitySchema } from 'typeorm';
import { EntitySchemaColumnFactory } from 'src/engine/twenty-orm/factories/entity-schema-column.factory';
import { EntitySchemaRelationFactory } from 'src/engine/twenty-orm/factories/entity-schema-relation.factory';
import { metadataArgsStorage } from 'src/engine/twenty-orm/storage/metadata-args.storage';
import { ObjectLiteralStorage } from 'src/engine/twenty-orm/storage/object-literal.storage';

@Injectable()
export class EntitySchemaFactory {
Expand Down Expand Up @@ -33,11 +34,15 @@ export class EntitySchemaFactory {
relationMetadataArgsCollection,
);

return new EntitySchema({
const entitySchema = new EntitySchema({
name: objectMetadataArgs.nameSingular,
tableName: objectMetadataArgs.nameSingular,
columns,
relations,
});

ObjectLiteralStorage.setObjectLiteral(entitySchema, target);

return entitySchema;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { EntitySchemaColumnFactory } from 'src/engine/twenty-orm/factories/entity-schema-column.factory';
import { EntitySchemaRelationFactory } from 'src/engine/twenty-orm/factories/entity-schema-relation.factory';
import { EntitySchemaFactory } from 'src/engine/twenty-orm/factories/entity-schema.factory';
import { ScopedWorkspaceDatasourceFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-datasource.factory';
import { WorkspaceDatasourceFactory } from 'src/engine/twenty-orm/factories/workspace-datasource.factory';

export const entitySchemaFactories = [
EntitySchemaColumnFactory,
EntitySchemaRelationFactory,
EntitySchemaFactory,
WorkspaceDatasourceFactory,
ScopedWorkspaceDatasourceFactory,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Inject, Injectable, Scope } from '@nestjs/common';
import { REQUEST } from '@nestjs/core';

import { EntitySchema } from 'typeorm';

import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { WorkspaceDatasourceFactory } from 'src/engine/twenty-orm/factories/workspace-datasource.factory';

@Injectable({ scope: Scope.REQUEST })
export class ScopedWorkspaceDatasourceFactory {
constructor(
@Inject(REQUEST) private readonly request: Request,
private readonly workspaceDataSourceFactory: WorkspaceDatasourceFactory,
) {}

public async create(entities: EntitySchema[]) {
const workspace: Workspace | undefined = this.request['req']?.['workspace'];

Copy link
Member

@Weiko Weiko May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is a bit dangerous since we don't provide the schema name here if I understood correctly. I guess ScopedWorkspaceDatasourceFactory should throw if the request object is not properly set and the token is not valid.
For other cases where we can't use the datasource without requests, the exception should tell us that we are probably not using the right class (for example if I'm using this in a command) as I don't think we want to use ScopedWorkspaceDatasourceFactory but explicitly the one that takes workspaceId as parameter in those cases (workspaceDataSourceFactory/TwentyORMManager). Wdyt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magrinj I'd rather not return anything if possible? this datasource won't be usable without a proper schema defined anyway and you would have to use TwentyORMManager.getRepositoryForWorkspace right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit complicated on my opinion to return something null or undefined or throw.
As the datasource is created when we inject it inside the class and we can have authenticated or unauthenticated resolver inside this class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested multiple things, and the "best" things I can do is returning null when workspace is not defined, so @InjectWorkspaceDataSource or @InjectWorkspaceReposiroty can now return null value.
We need to be careful with that as decorators doesn't enforce the type of the value under them.

if (!workspace) {
return null;
}

return this.workspaceDataSourceFactory.create(entities, workspace.id);
}
}
Original file line number Diff line number Diff line change
@@ -1,42 +1,33 @@
import { Inject, Injectable, Scope } from '@nestjs/common';
import { REQUEST } from '@nestjs/core';
import { Injectable } from '@nestjs/common';

import { DataSource, EntitySchema } from 'typeorm';
import { EntitySchema } from 'typeorm';

import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service';
import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { DataSourceStorage } from 'src/engine/twenty-orm/storage/data-source.storage';
import { WorkspaceDataSource } from 'src/engine/twenty-orm/datasource/workspace.datasource';

@Injectable({ scope: Scope.REQUEST })
@Injectable()
export class WorkspaceDatasourceFactory {
constructor(
@Inject(REQUEST) private readonly request: Request,
private readonly dataSourceService: DataSourceService,
private readonly environmentService: EnvironmentService,
) {}

public async createWorkspaceDatasource(entities: EntitySchema[]) {
const workspace: Workspace = this.request['req']['workspace'];

if (!workspace) {
return null;
}

const storedWorkspaceDataSource = DataSourceStorage.getDataSource(
workspace.id,
);
public async create(entities: EntitySchema[], workspaceId: string) {
const storedWorkspaceDataSource =
DataSourceStorage.getDataSource(workspaceId);

if (storedWorkspaceDataSource) {
return storedWorkspaceDataSource;
}

const dataSourceMetadata =
await this.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail(
workspace.id,
workspaceId,
);

const workspaceDataSource = new DataSource({
const workspaceDataSource = new WorkspaceDataSource({
url:
dataSourceMetadata.url ??
this.environmentService.get('PG_DATABASE_URL'),
Expand All @@ -51,7 +42,7 @@ export class WorkspaceDatasourceFactory {

await workspaceDataSource.initialize();

DataSourceStorage.setDataSource(workspace.id, workspaceDataSource);
DataSourceStorage.setDataSource(workspaceId, workspaceDataSource);

return workspaceDataSource;
}
Expand Down