From f65db49514c7db53c51753ab0bf6c7772ef3209f Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Sun, 6 Jul 2025 12:18:25 +0200 Subject: [PATCH] Fix broken data model translation (#13067) In this PR, I'm fixing a bug introduced in recent performance work on the cache. Bug context: https://github.com/twentyhq/twenty/issues/12865 Related PR opened by a contributor: https://github.com/twentyhq/twenty/pull/13003 ## Root cause We cache all objectMetadataItems at graphql level : see `useCachedMetadata` hook: - instead of going through the regular resolvers, we direlcty load data from the cache. However this data must be localized regarding labels and descriptions In a precedent refactoring, we introduced the notion of locale in the cache key. However, the user locale was not properly taken into account as we did not have the information in this hook. ## Fix 1. **Introduce locale in userWorkspace entity**. The locale is stored on workspaceMember in each postgres workspaceSchema (workspace_xxx) which is the alter ego of userWorkspace in postgres core schema. Note that we can't store it in user as a user can be part of multiple workspaces (the locale already there must be seen as a default for this user), and we cannot rely on workspaceMember as we would need to query the workspaceSchema in the authentication layer which we want to avoid for performance reasons. 2. During request hydration from token (containing the userWorkspaceId), we fetch the userWorkspace and store it in the Request (this impact both AuthContext and Request interface) 3. Leverage userWorkspace.locale in the useCachedMetadata hook ## Additional notes There is no need to change the way we store and retrieve the object-metadata-maps object itself which is different from the graphql layer cache. object-metadadata-maps are not localized --- packages/twenty-server/@types/express.d.ts | 4 ++- ...1700932529-add-locale-to-user-workspace.ts | 19 ++++++++++ .../hooks/use-cached-metadata.ts | 11 +++--- .../auth/strategies/jwt.auth.strategy.ts | 7 ++-- .../token/services/access-token.service.ts | 2 ++ .../auth/types/auth-context.type.ts | 4 ++- .../user-workspace/user-workspace.entity.ts | 4 +++ .../engine/middlewares/middleware.service.ts | 1 + .../workspace-cache-storage.service.ts | 16 ++++----- .../workspace-member-query-hook.module.ts | 8 ++++- ...kspace-member-update-one.pre-query.hook.ts | 35 ++++++++++++++++++- 11 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 packages/twenty-server/src/database/typeorm/core/migrations/common/1751700932529-add-locale-to-user-workspace.ts diff --git a/packages/twenty-server/@types/express.d.ts b/packages/twenty-server/@types/express.d.ts index 154ef0f234..902f2e2263 100644 --- a/packages/twenty-server/@types/express.d.ts +++ b/packages/twenty-server/@types/express.d.ts @@ -1,12 +1,14 @@ +import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { User } from 'src/engine/core-modules/user/user.entity'; +import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; -import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type'; declare module 'express-serve-static-core' { interface Request { user?: User | null; apiKey?: ApiKeyWorkspaceEntity | null; + userWorkspace?: UserWorkspace; workspace?: Workspace; workspaceId?: string; workspaceMetadataVersion?: number; diff --git a/packages/twenty-server/src/database/typeorm/core/migrations/common/1751700932529-add-locale-to-user-workspace.ts b/packages/twenty-server/src/database/typeorm/core/migrations/common/1751700932529-add-locale-to-user-workspace.ts new file mode 100644 index 0000000000..52203fe4a4 --- /dev/null +++ b/packages/twenty-server/src/database/typeorm/core/migrations/common/1751700932529-add-locale-to-user-workspace.ts @@ -0,0 +1,19 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class AddLocaleToUserWorkspace1751700932529 + implements MigrationInterface +{ + name = 'AddLocaleToUserWorkspace1751700932529'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."userWorkspace" ADD "locale" character varying NOT NULL DEFAULT 'en'`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."userWorkspace" DROP COLUMN "locale"`, + ); + } +} diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-config/hooks/use-cached-metadata.ts b/packages/twenty-server/src/engine/api/graphql/graphql-config/hooks/use-cached-metadata.ts index 771bacf50f..c30a2e5dee 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-config/hooks/use-cached-metadata.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-config/hooks/use-cached-metadata.ts @@ -1,6 +1,6 @@ import { createHash } from 'crypto'; -import { isDefined } from 'class-validator'; +import { isNonEmptyString } from '@sniptt/guards'; import { Plugin } from 'graphql-yoga'; export type CacheMetadataPluginConfig = { @@ -18,10 +18,11 @@ export function useCachedMetadata(config: CacheMetadataPluginConfig): Plugin { const workspaceMetadataVersion = serverContext.req.workspaceMetadataVersion ?? '0'; const operationName = getOperationName(serverContext); - const locale = serverContext.req.headers['x-locale'] ?? ''; - const localeCacheKey = isDefined(serverContext.req.headers['x-locale']) - ? `:${locale}` - : ''; + const locale = + serverContext.req.userWorkspace?.locale ?? + serverContext.req.headers['x-locale'] ?? + ''; + const localeCacheKey = isNonEmptyString(locale) ? `:${locale}` : ''; const queryHash = createHash('sha256') .update(serverContext.req.body.query) .digest('hex'); diff --git a/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts b/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts index 564167215b..a8ece6d9b6 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts @@ -19,13 +19,13 @@ import { } from 'src/engine/core-modules/auth/types/auth-context.type'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; +import { userWorkspaceValidator } from 'src/engine/core-modules/user-workspace/user-workspace.validate'; import { User } from 'src/engine/core-modules/user/user.entity'; +import { userValidator } from 'src/engine/core-modules/user/user.validate'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; -import { userWorkspaceValidator } from 'src/engine/core-modules/user-workspace/user-workspace.validate'; -import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate'; -import { userValidator } from 'src/engine/core-modules/user/user.validate'; @Injectable() export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') { @@ -162,6 +162,7 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') { user, workspace, authProvider: payload.authProvider, + userWorkspace, userWorkspaceId: userWorkspace.id, workspaceMemberId: payload.workspaceMemberId, }; diff --git a/packages/twenty-server/src/engine/core-modules/auth/token/services/access-token.service.ts b/packages/twenty-server/src/engine/core-modules/auth/token/services/access-token.service.ts index 22e0475844..732cee37ea 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/token/services/access-token.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/token/services/access-token.service.ts @@ -136,6 +136,7 @@ export class AccessTokenService { apiKey, workspace, workspaceMemberId, + userWorkspace, userWorkspaceId, authProvider, } = await this.jwtStrategy.validate(decoded); @@ -144,6 +145,7 @@ export class AccessTokenService { user, apiKey, workspace, + userWorkspace, workspaceMemberId, userWorkspaceId, authProvider, diff --git a/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts b/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts index d16b5c096c..06e77277f7 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts @@ -1,7 +1,8 @@ +import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { User } from 'src/engine/core-modules/user/user.entity'; +import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; -import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type'; export type AuthContext = { user?: User | null | undefined; @@ -9,6 +10,7 @@ export type AuthContext = { workspaceMemberId?: string; workspace?: Workspace; userWorkspaceId?: string; + userWorkspace?: UserWorkspace; authProvider?: AuthProviderEnum; }; diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.entity.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.entity.ts index 517f75c181..6cda98867c 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.entity.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.entity.ts @@ -70,6 +70,10 @@ export class UserWorkspace { @Column({ nullable: true }) defaultAvatarUrl: string; + @Field(() => String, { nullable: false }) + @Column({ nullable: false, default: 'en' }) + locale: string; + @Field() @CreateDateColumn({ type: 'timestamptz' }) createdAt: Date; diff --git a/packages/twenty-server/src/engine/middlewares/middleware.service.ts b/packages/twenty-server/src/engine/middlewares/middleware.service.ts index 7949b62830..84a1bec274 100644 --- a/packages/twenty-server/src/engine/middlewares/middleware.service.ts +++ b/packages/twenty-server/src/engine/middlewares/middleware.service.ts @@ -151,6 +151,7 @@ export class MiddlewareService { ) { request.user = data.user; request.apiKey = data.apiKey; + request.userWorkspace = data.userWorkspace; request.workspace = data.workspace; request.workspaceId = data.workspace?.id; request.workspaceMetadataVersion = metadataVersion; diff --git a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts index e974a33940..01fc5aa2ff 100644 --- a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts +++ b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts @@ -36,7 +36,7 @@ export enum WorkspaceCacheKeys { MetadataPermissionsUserWorkspaceRoleMapVersion = 'metadata:permissions:user-workspace-role-map-version', } -const TTL_INFINITE = 0; +const TTL_ONE_WEEK = 1000 * 60 * 60 * 24 * 7; @Injectable() export class WorkspaceCacheStorageService { @@ -55,7 +55,7 @@ export class WorkspaceCacheStorageService { return this.cacheStorageService.set[]>( `${WorkspaceCacheKeys.ORMEntitySchemas}:${workspaceId}:${metadataVersion}`, entitySchemas, - TTL_INFINITE, + TTL_ONE_WEEK, ); } @@ -77,7 +77,7 @@ export class WorkspaceCacheStorageService { return this.cacheStorageService.set( `${WorkspaceCacheKeys.MetadataVersion}:${workspaceId}`, metadataVersion, - TTL_INFINITE, + TTL_ONE_WEEK, ); } @@ -95,7 +95,7 @@ export class WorkspaceCacheStorageService { return this.cacheStorageService.set( `${WorkspaceCacheKeys.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersion}`, objectMetadataMaps, - TTL_INFINITE, + TTL_ONE_WEEK, ); } @@ -141,7 +141,7 @@ export class WorkspaceCacheStorageService { return this.cacheStorageService.set( `${WorkspaceCacheKeys.GraphQLTypeDefs}:${workspaceId}:${metadataVersion}`, typeDefs, - TTL_INFINITE, + TTL_ONE_WEEK, ); } @@ -162,7 +162,7 @@ export class WorkspaceCacheStorageService { return this.cacheStorageService.set( `${WorkspaceCacheKeys.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersion}`, usedScalarNames, - TTL_INFINITE, + TTL_ONE_WEEK, ); } @@ -189,7 +189,7 @@ export class WorkspaceCacheStorageService { await this.cacheStorageService.set( `${WorkspaceCacheKeys.FeatureFlagMapVersion}:${workspaceId}`, featureFlagMapVersion, - TTL_INFINITE, + TTL_ONE_WEEK, ); return featureFlagMapVersion; @@ -205,7 +205,7 @@ export class WorkspaceCacheStorageService { this.cacheStorageService.set( `${WorkspaceCacheKeys.FeatureFlagMap}:${workspaceId}`, featureFlagMap, - TTL_INFINITE, + TTL_ONE_WEEK, ), this.setFeatureFlagsMapVersion(workspaceId), ]); diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts index dae6180ab2..0c7a2d2406 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts @@ -1,6 +1,8 @@ import { Module } from '@nestjs/common'; +import { TypeOrmModule } from '@nestjs/typeorm'; import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; +import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { WorkspaceMemberCreateManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook'; import { WorkspaceMemberCreateOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook'; @@ -28,6 +30,10 @@ import { WorkspaceMemberUpdateOnePreQueryHook } from 'src/modules/workspace-memb WorkspaceMemberUpdateOnePreQueryHook, WorkspaceMemberUpdateManyPreQueryHook, ], - imports: [FeatureFlagModule, PermissionsModule], + imports: [ + FeatureFlagModule, + PermissionsModule, + TypeOrmModule.forFeature([UserWorkspace], 'core'), + ], }) export class WorkspaceMemberQueryHookModule {} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts index 3262b30318..109f00f48e 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts @@ -1,10 +1,20 @@ +import { InjectRepository } from '@nestjs/typeorm'; + +import { isDefined } from 'class-validator'; +import { Repository } from 'typeorm'; + import { WorkspacePreQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; import { UpdateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { + AuthException, + AuthExceptionCode, +} from 'src/engine/core-modules/auth/auth.exception'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; -import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; +import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; @WorkspaceQueryHook(`workspaceMember.updateOne`) export class WorkspaceMemberUpdateOnePreQueryHook @@ -12,6 +22,8 @@ export class WorkspaceMemberUpdateOnePreQueryHook { constructor( private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + @InjectRepository(UserWorkspace, 'core') + private readonly userWorkspaceRepository: Repository, ) {} async execute( @@ -33,6 +45,27 @@ export class WorkspaceMemberUpdateOnePreQueryHook }, ); + // TODO: remove this code once we have migrated locale update to userWorkspace update + if (payload.data.locale) { + const userWorkspace = await this.userWorkspaceRepository.findOne({ + where: { + id: authContext.userWorkspaceId, + }, + }); + + if (!isDefined(userWorkspace)) { + throw new AuthException( + 'User workspace not found', + AuthExceptionCode.USER_WORKSPACE_NOT_FOUND, + ); + } + + await this.userWorkspaceRepository.save({ + ...userWorkspace, + locale: payload.data.locale, + }); + } + return payload; } }