From 6efadd330f78056f46bcf7570d36b5ec78633dad Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Thu, 12 Jun 2025 16:33:52 +0200 Subject: [PATCH] Recompute cached permissions at feature flag update (#12554) If permissionsV2 feature flag is toggled, we should recompute the permissions. We decided to make each WorkspaceXxCacheService Xx-specific (feature flag, permissions...), so we are not recomputing permission cache from workspaceFeatureFlagCacheService where feature flags are recomputed, even if that would be a lower level than FeatureFlagService. This allows to avoid complex circuclar dependency and keeps a clear purpose for each service. --- .../feature-flag/feature-flag.module.ts | 2 ++ .../__tests__/feature-flag.service.spec.ts | 9 +++++++++ .../services/feature-flag.service.ts | 16 +++++++++++++++- .../workspace-permissions-cache.module.ts | 4 ++-- .../workspace-permissions-cache.service.ts | 16 ++++++++-------- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/feature-flag.module.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/feature-flag.module.ts index d4418059a..d1b423aaa 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/feature-flag.module.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/feature-flag.module.ts @@ -7,6 +7,7 @@ import { TypeORMModule } from 'src/database/typeorm/typeorm.module'; import { FeatureFlag } from 'src/engine/core-modules/feature-flag/feature-flag.entity'; import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { WorkspaceFeatureFlagsMapCacheModule } from 'src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.module'; +import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module'; @Module({ imports: [ @@ -17,6 +18,7 @@ import { WorkspaceFeatureFlagsMapCacheModule } from 'src/engine/metadata-modules resolvers: [], }), WorkspaceFeatureFlagsMapCacheModule, + WorkspacePermissionsCacheModule, ], exports: [FeatureFlagService], providers: [FeatureFlagService], diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts index 69666494e..d892b3da7 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/services/__tests__/feature-flag.service.spec.ts @@ -11,6 +11,7 @@ import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/service import { featureFlagValidator } from 'src/engine/core-modules/feature-flag/validates/feature-flag.validate'; import { publicFeatureFlagValidator } from 'src/engine/core-modules/feature-flag/validates/is-public-feature-flag.validate'; import { WorkspaceFeatureFlagsMapCacheService } from 'src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.service'; +import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service'; jest.mock( 'src/engine/core-modules/feature-flag/validates/is-public-feature-flag.validate', @@ -35,6 +36,10 @@ describe('FeatureFlagService', () => { recomputeFeatureFlagsMapCache: jest.fn(), }; + const mockWorkspacePermissionsCacheService = { + recomputeRolesPermissionsCache: jest.fn(), + }; + const workspaceId = 'workspace-id'; const featureFlag = FeatureFlagKey.IS_WORKFLOW_ENABLED; @@ -57,6 +62,10 @@ describe('FeatureFlagService', () => { provide: WorkspaceFeatureFlagsMapCacheService, useValue: mockWorkspaceFeatureFlagsMapCacheService, }, + { + provide: WorkspacePermissionsCacheService, + useValue: mockWorkspacePermissionsCacheService, + }, ], }).compile(); diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts index 2948ebdb6..3893594bd 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts @@ -15,6 +15,7 @@ import { import { featureFlagValidator } from 'src/engine/core-modules/feature-flag/validates/feature-flag.validate'; import { publicFeatureFlagValidator } from 'src/engine/core-modules/feature-flag/validates/is-public-feature-flag.validate'; import { WorkspaceFeatureFlagsMapCacheService } from 'src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.service'; +import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service'; @Injectable() export class FeatureFlagService { @@ -22,6 +23,7 @@ export class FeatureFlagService { @InjectRepository(FeatureFlag, 'core') private readonly featureFlagRepository: Repository, private readonly workspaceFeatureFlagsMapCacheService: WorkspaceFeatureFlagsMapCacheService, + private readonly workspacePermissionsCacheService: WorkspacePermissionsCacheService, ) {} public async isFeatureEnabled( @@ -71,9 +73,15 @@ export class FeatureFlagService { }, ); + if (keys.includes(FeatureFlagKey.IS_PERMISSIONS_V2_ENABLED)) { + await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache( + { workspaceId, ignoreLock: true }, + ); + } + await this.workspaceFeatureFlagsMapCacheService.recomputeFeatureFlagsMapCache( { - workspaceId: workspaceId, + workspaceId, ignoreLock: true, }, ); @@ -136,6 +144,12 @@ export class FeatureFlagService { }, ); + if (featureFlag === FeatureFlagKey.IS_PERMISSIONS_V2_ENABLED) { + await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache( + { workspaceId, ignoreLock: true }, + ); + } + return result; } } diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module.ts index 1fb27078c..0a2e3d630 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module.ts @@ -1,11 +1,11 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { UserWorkspaceRoleEntity } from 'src/engine/metadata-modules/role/user-workspace-role.entity'; +import { WorkspaceFeatureFlagsMapCacheModule } from 'src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.module'; import { WorkspacePermissionsCacheStorageService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; @@ -19,7 +19,7 @@ import { WorkspacePermissionsCacheService } from './workspace-permissions-cache. 'core', ), WorkspaceCacheStorageModule, - FeatureFlagModule, + WorkspaceFeatureFlagsMapCacheModule, ], providers: [ WorkspacePermissionsCacheService, diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts index b777daa38..de0ba70e9 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts @@ -9,16 +9,15 @@ import { isDefined } from 'twenty-shared/utils'; import { In, Repository } from 'typeorm'; import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; -import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { SettingPermissionType } from 'src/engine/metadata-modules/permissions/constants/setting-permission-type.constants'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { UserWorkspaceRoleEntity } from 'src/engine/metadata-modules/role/user-workspace-role.entity'; +import { WorkspaceFeatureFlagsMapCacheService } from 'src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.service'; import { UserWorkspaceRoleMap } from 'src/engine/metadata-modules/workspace-permissions-cache/types/user-workspace-role-map.type'; import { WorkspacePermissionsCacheStorageService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service'; import { TwentyORMExceptionCode } from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; import { getFromCacheWithRecompute } from 'src/engine/utils/get-data-from-cache-with-recompute.util'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; import { STANDARD_OBJECT_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids'; type CacheResult = { @@ -41,8 +40,7 @@ export class WorkspacePermissionsCacheService { @InjectRepository(UserWorkspaceRoleEntity, 'core') private readonly userWorkspaceRoleRepository: Repository, private readonly workspacePermissionsCacheStorageService: WorkspacePermissionsCacheStorageService, - private readonly workspaceCacheStorageService: WorkspaceCacheStorageService, - private readonly featureFlagService: FeatureFlagService, + private readonly workspaceFeatureFlagsMapCacheService: WorkspaceFeatureFlagsMapCacheService, ) {} async recomputeRolesPermissionsCache({ @@ -87,12 +85,14 @@ export class WorkspacePermissionsCacheService { ); } - const isPermissionsV2Enabled = - await this.featureFlagService.isFeatureEnabled( - FeatureFlagKey.IS_PERMISSIONS_V2_ENABLED, - workspaceId, + const workspaceFeatureFlagsMap = + await this.workspaceFeatureFlagsMapCacheService.getWorkspaceFeatureFlagsMap( + { workspaceId }, ); + const isPermissionsV2Enabled = + !!workspaceFeatureFlagsMap[FeatureFlagKey.IS_PERMISSIONS_V2_ENABLED]; + const recomputedRolesPermissions = await this.getObjectRecordPermissionsForRoles({ workspaceId,