From 79c3f8e975bd7cc870d47450ed074d7f50e79567 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 19 Aug 2024 08:46:50 +0200 Subject: [PATCH] refactor: switch `projectStore.getProjects` with `projectReadModel.getProjectsForAdminUi` in project service (#7904) Hooks up the new project read model and updates the existing project service to use it instead when the flag is on. In doing: - creates a composition root for the read model - includes it in IUnleashStores - updates some existing methods to accept either the old or the new model - updates the OpenAPI schema to deprecate the old properties --- src/lib/db/index.ts | 6 +++++ .../project/createProjectReadModel.ts | 18 +++++++++++++++ .../features/project/createProjectService.ts | 14 +++++++++++ .../project/fake-project-owners-read-model.ts | 8 +++---- .../project/fake-project-read-model.ts | 14 +++++++++++ .../project/project-owners-read-model.test.ts | 17 +++++++------- .../project/project-owners-read-model.ts | 13 ++++++----- .../project/project-owners-read-model.type.ts | 8 +++---- .../project/project-read-model-type.ts | 5 +++- .../features/project/project-read-model.ts | 9 +++----- .../project/project-service.e2e.test.ts | 10 +++----- src/lib/features/project/project-service.ts | 23 ++++++++++++------- src/lib/openapi/spec/project-schema.ts | 8 ++++++- src/lib/types/model.ts | 1 + src/lib/types/stores.ts | 3 +++ src/test/fixtures/store.ts | 2 ++ 16 files changed, 114 insertions(+), 45 deletions(-) create mode 100644 src/lib/features/project/createProjectReadModel.ts create mode 100644 src/lib/features/project/fake-project-read-model.ts diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index fa85597fbd..7fc5776373 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -51,6 +51,7 @@ import { FeatureLifecycleReadModel } from '../features/feature-lifecycle/feature import { LargestResourcesReadModel } from '../features/metrics/sizes/largest-resources-read-model'; import { IntegrationEventsStore } from '../features/integration-events/integration-events-store'; import { FeatureCollaboratorsReadModel } from '../features/feature-toggle/feature-collaborators-read-model'; +import { createProjectReadModel } from '../features/project/createProjectReadModel'; export const createStores = ( config: IUnleashConfig, @@ -177,6 +178,11 @@ export const createStores = ( largestResourcesReadModel: new LargestResourcesReadModel(db), integrationEventsStore: new IntegrationEventsStore(db, { eventBus }), featureCollaboratorsReadModel: new FeatureCollaboratorsReadModel(db), + projectReadModel: createProjectReadModel( + db, + eventBus, + config.flagResolver, + ), }; }; diff --git a/src/lib/features/project/createProjectReadModel.ts b/src/lib/features/project/createProjectReadModel.ts new file mode 100644 index 0000000000..2a589475f6 --- /dev/null +++ b/src/lib/features/project/createProjectReadModel.ts @@ -0,0 +1,18 @@ +import type EventEmitter from 'events'; +import type { Db } from '../../server-impl'; +import type { IProjectReadModel } from './project-read-model-type'; +import type { IFlagResolver } from '../../types'; +import { ProjectReadModel } from './project-read-model'; +import { FakeProjectReadModel } from './fake-project-read-model'; + +export const createProjectReadModel = ( + db: Db, + eventBus: EventEmitter, + flagResolver: IFlagResolver, +): IProjectReadModel => { + return new ProjectReadModel(db, eventBus, flagResolver); +}; + +export const createFakeProjectReadModel = (): IProjectReadModel => { + return new FakeProjectReadModel(); +}; diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index 4441324618..ec31ad30ab 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -48,6 +48,10 @@ import { createEventsService, createFakeEventsService, } from '../events/createEventsService'; +import { + createFakeProjectReadModel, + createProjectReadModel, +} from './createProjectReadModel'; export const createProjectService = ( db: Db, @@ -120,6 +124,12 @@ export const createProjectService = ( eventService, ); + const projectReadModel = createProjectReadModel( + db, + eventBus, + config.flagResolver, + ); + return new ProjectService( { projectStore, @@ -131,6 +141,7 @@ export const createProjectService = ( projectStatsStore, projectOwnersReadModel, projectFlagCreatorsReadModel, + projectReadModel, }, config, accessService, @@ -184,6 +195,8 @@ export const createFakeProjectService = ( eventService, ); + const projectReadModel = createFakeProjectReadModel(); + return new ProjectService( { projectStore, @@ -195,6 +208,7 @@ export const createFakeProjectService = ( featureEnvironmentStore, accountStore, projectStatsStore, + projectReadModel, }, config, accessService, diff --git a/src/lib/features/project/fake-project-owners-read-model.ts b/src/lib/features/project/fake-project-owners-read-model.ts index 85227b9ff5..dc99ecb837 100644 --- a/src/lib/features/project/fake-project-owners-read-model.ts +++ b/src/lib/features/project/fake-project-owners-read-model.ts @@ -1,13 +1,13 @@ -import type { IProjectWithCount } from '../../types'; import type { IProjectOwnersReadModel, - IProjectWithCountAndOwners, + IProjectForUiWithOwners, } from './project-owners-read-model.type'; +import type { ProjectForUi } from './project-read-model-type'; export class FakeProjectOwnersReadModel implements IProjectOwnersReadModel { async addOwners( - projects: IProjectWithCount[], - ): Promise { + projects: ProjectForUi[], + ): Promise { return projects.map((project) => ({ ...project, owners: [{ ownerType: 'system' }], diff --git a/src/lib/features/project/fake-project-read-model.ts b/src/lib/features/project/fake-project-read-model.ts new file mode 100644 index 0000000000..fb9f4b5042 --- /dev/null +++ b/src/lib/features/project/fake-project-read-model.ts @@ -0,0 +1,14 @@ +import type { IProjectReadModel } from '../../types'; +import type { + ProjectForUi, + ProjectForInsights, +} from './project-read-model-type'; + +export class FakeProjectReadModel implements IProjectReadModel { + getProjectsForAdminUi(): Promise { + throw new Error('Method not implemented.'); + } + getProjectsForInsights(): Promise { + throw new Error('Method not implemented.'); + } +} diff --git a/src/lib/features/project/project-owners-read-model.test.ts b/src/lib/features/project/project-owners-read-model.test.ts index d4ec6ce752..139d2d8ec6 100644 --- a/src/lib/features/project/project-owners-read-model.test.ts +++ b/src/lib/features/project/project-owners-read-model.test.ts @@ -3,23 +3,24 @@ import getLogger from '../../../test/fixtures/no-logger'; import { type IUser, RoleName, type IGroup } from '../../types'; import { randomId } from '../../util'; import { ProjectOwnersReadModel } from './project-owners-read-model'; +import type { ProjectForUi } from './project-read-model-type'; jest.mock('../../util', () => ({ ...jest.requireActual('../../util'), generateImageUrl: jest.fn((input) => `https://${input.image_url}`), })); -const mockProjectWithCounts = (name: string) => ({ +const mockProjectData = (name: string): ProjectForUi => ({ name, id: name, - description: '', featureCount: 0, memberCount: 0, mode: 'open' as const, - defaultStickiness: 'default' as const, - staleFeatureCount: 0, - potentiallyStaleFeatureCount: 0, - avgTimeToProduction: 0, + health: 100, + createdAt: new Date(), + favorite: false, + lastReportedFlagUsage: null, + lastFlagUpdate: null, }); describe('unit tests', () => { @@ -351,8 +352,8 @@ describe('integration tests', () => { ); const projectsWithOwners = await readModel.addOwners([ - mockProjectWithCounts(projectIdA), - mockProjectWithCounts(projectIdB), + mockProjectData(projectIdA), + mockProjectData(projectIdB), ]); expect(projectsWithOwners).toMatchObject([ diff --git a/src/lib/features/project/project-owners-read-model.ts b/src/lib/features/project/project-owners-read-model.ts index 0c983f9982..c4c39dbfbd 100644 --- a/src/lib/features/project/project-owners-read-model.ts +++ b/src/lib/features/project/project-owners-read-model.ts @@ -1,13 +1,14 @@ import type { Db } from '../../db/db'; -import { RoleName, type IProjectWithCount } from '../../types'; +import { RoleName } from '../../types'; import { anonymise, generateImageUrl } from '../../util'; import type { GroupProjectOwner, IProjectOwnersReadModel, - IProjectWithCountAndOwners, + IProjectForUiWithOwners, ProjectOwnersDictionary, UserProjectOwner, } from './project-owners-read-model.type'; +import type { ProjectForUi } from './project-read-model-type'; const T = { ROLE_USER: 'role_user', @@ -24,9 +25,9 @@ export class ProjectOwnersReadModel implements IProjectOwnersReadModel { } static addOwnerData( - projects: IProjectWithCount[], + projects: ProjectForUi[], owners: ProjectOwnersDictionary, - ): IProjectWithCountAndOwners[] { + ): IProjectForUiWithOwners[] { return projects.map((project) => ({ ...project, owners: owners[project.id] || [{ ownerType: 'system' }], @@ -138,9 +139,9 @@ export class ProjectOwnersReadModel implements IProjectOwnersReadModel { } async addOwners( - projects: IProjectWithCount[], + projects: ProjectForUi[], anonymizeProjectOwners: boolean = false, - ): Promise { + ): Promise { const owners = await this.getAllProjectOwners(anonymizeProjectOwners); return ProjectOwnersReadModel.addOwnerData(projects, owners); diff --git a/src/lib/features/project/project-owners-read-model.type.ts b/src/lib/features/project/project-owners-read-model.type.ts index c01436fda2..8a68bc5e4f 100644 --- a/src/lib/features/project/project-owners-read-model.type.ts +++ b/src/lib/features/project/project-owners-read-model.type.ts @@ -1,4 +1,4 @@ -import type { IProjectWithCount } from '../../types'; +import type { TransitionalProjectData } from './project-read-model-type'; export type SystemOwner = { ownerType: 'system' }; export type UserProjectOwner = { @@ -17,13 +17,13 @@ type ProjectOwners = export type ProjectOwnersDictionary = Record; -export type IProjectWithCountAndOwners = IProjectWithCount & { +export type IProjectForUiWithOwners = TransitionalProjectData & { owners: ProjectOwners; }; export interface IProjectOwnersReadModel { addOwners( - projects: IProjectWithCount[], + projects: TransitionalProjectData[], anonymizeProjectOwners?: boolean, - ): Promise; + ): Promise; } diff --git a/src/lib/features/project/project-read-model-type.ts b/src/lib/features/project/project-read-model-type.ts index 5bc504d083..09301f0326 100644 --- a/src/lib/features/project/project-read-model-type.ts +++ b/src/lib/features/project/project-read-model-type.ts @@ -1,4 +1,4 @@ -import type { ProjectMode } from '../../types'; +import type { IProjectWithCount, ProjectMode } from '../../types'; import type { IProjectQuery } from './project-store-type'; export type ProjectForUi = { @@ -16,6 +16,9 @@ export type ProjectForUi = { lastFlagUpdate: Date | null; }; +// @todo remove with flag useProjectReadModel +export type TransitionalProjectData = ProjectForUi | IProjectWithCount; + export type ProjectForInsights = { id: string; health: number; diff --git a/src/lib/features/project/project-read-model.ts b/src/lib/features/project/project-read-model.ts index c3880181b3..e6e6f47c75 100644 --- a/src/lib/features/project/project-read-model.ts +++ b/src/lib/features/project/project-read-model.ts @@ -49,13 +49,10 @@ export class ProjectReadModel implements IProjectReadModel { private db: Db; private timer: Function; - private flagResolver: IFlagResolver; - constructor( - db: Db, - eventBus: EventEmitter, - flagResolver: IFlagResolver, - ) { + private flagResolver: IFlagResolver; + + constructor(db: Db, eventBus: EventEmitter, flagResolver: IFlagResolver) { this.db = db; this.timer = (action) => metricsHelper.wrapTimer(eventBus, DB_TIME, { diff --git a/src/lib/features/project/project-service.e2e.test.ts b/src/lib/features/project/project-service.e2e.test.ts index 33e4f3b268..cbc23e2ff6 100644 --- a/src/lib/features/project/project-service.e2e.test.ts +++ b/src/lib/features/project/project-service.e2e.test.ts @@ -82,7 +82,9 @@ beforeAll(async () => { await stores.accessStore.addUserToRole(opsUser.id, 1, ''); const config = createTestConfig({ getLogger, - experimental: { flags: { archiveProjects: true } }, + experimental: { + flags: { archiveProjects: true, useProjectReadModel: true }, + }, }); eventService = createEventsService(db.rawDatabase, config); accessService = createAccessService(db.rawDatabase, config); @@ -323,9 +325,7 @@ test('should revive project', async () => { const project = { id: 'test-revive', name: 'New project', - description: 'Blah', mode: 'open' as const, - defaultStickiness: 'default', }; await projectService.createProject(project, user, TEST_AUDIT_USER); @@ -347,9 +347,7 @@ test('should not be able to archive project with flags', async () => { const project = { id: 'test-archive-with-flags', name: 'New project', - description: 'Blah', mode: 'open' as const, - defaultStickiness: 'default', }; await projectService.createProject(project, user, auditUser); await stores.featureToggleStore.create(project.id, { @@ -2748,9 +2746,7 @@ test('should get project settings with mode', async () => { ); expect(foundProjectOne!.mode).toBe('private'); - expect(foundProjectOne!.defaultStickiness).toBe('clientId'); expect(foundProjectTwo!.mode).toBe('open'); - expect(foundProjectTwo!.defaultStickiness).toBe('default'); }); describe('create project with environments', () => { diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 4d9c1e31c1..081aba318d 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -32,7 +32,6 @@ import { type IProjectRoleUsage, type IProjectStore, type IProjectUpdate, - type IProjectWithCount, type IUnleashConfig, type IUnleashStores, MOVE_FEATURE_TOGGLE, @@ -54,6 +53,7 @@ import { ProjectUserUpdateRoleEvent, RoleName, SYSTEM_USER_ID, + type IProjectReadModel, } from '../../types'; import type { IProjectAccessModel, @@ -87,6 +87,7 @@ import type { IProjectFlagCreatorsReadModel } from './project-flag-creators-read import { throwExceedsLimitError } from '../../error/exceeds-limit-error'; import type EventEmitter from 'events'; import type { ApiTokenService } from '../../services/api-token-service'; +import type { TransitionalProjectData } from './project-read-model-type'; type Days = number; type Count = number; @@ -161,6 +162,8 @@ export default class ProjectService { private eventBus: EventEmitter; + private projectReadModel: IProjectReadModel; + constructor( { projectStore, @@ -172,6 +175,7 @@ export default class ProjectService { featureEnvironmentStore, accountStore, projectStatsStore, + projectReadModel, }: Pick< IUnleashStores, | 'projectStore' @@ -183,6 +187,7 @@ export default class ProjectService { | 'featureEnvironmentStore' | 'accountStore' | 'projectStatsStore' + | 'projectReadModel' >, config: IUnleashConfig, accessService: AccessService, @@ -214,16 +219,18 @@ export default class ProjectService { this.isEnterprise = config.isEnterprise; this.resourceLimits = config.resourceLimits; this.eventBus = config.eventBus; + this.projectReadModel = projectReadModel; } async getProjects( query?: IProjectQuery, userId?: number, - ): Promise { - const projects = await this.projectStore.getProjectsWithCounts( - query, - userId, - ); + ): Promise { + const getProjects = this.flagResolver.isEnabled('useProjectReadModel') + ? () => this.projectReadModel.getProjectsForAdminUi(query, userId) + : () => this.projectStore.getProjectsWithCounts(query, userId); + + const projects = await getProjects(); if (userId) { const projectAccess = @@ -243,8 +250,8 @@ export default class ProjectService { } async addOwnersToProjects( - projects: IProjectWithCount[], - ): Promise { + projects: TransitionalProjectData[], + ): Promise { const anonymizeProjectOwners = this.flagResolver.isEnabled( 'anonymizeProjectOwners', ); diff --git a/src/lib/openapi/spec/project-schema.ts b/src/lib/openapi/spec/project-schema.ts index fc86a10481..117cc45be5 100644 --- a/src/lib/openapi/spec/project-schema.ts +++ b/src/lib/openapi/spec/project-schema.ts @@ -3,7 +3,7 @@ import type { FromSchema } from 'json-schema-to-ts'; export const projectSchema = { $id: '#/components/schemas/projectSchema', type: 'object', - additionalProperties: false, + // additionalProperties: false, // todo: re-enable when flag projectListImprovements is removed required: ['id', 'name'], description: 'A definition of the project used for projects listing purposes', @@ -19,6 +19,7 @@ export const projectSchema = { description: 'The name of this project', }, description: { + deprecated: true, type: 'string', nullable: true, example: 'DX squad feature release', @@ -36,11 +37,13 @@ export const projectSchema = { description: 'The number of features this project has', }, staleFeatureCount: { + deprecated: true, type: 'number', example: 10, description: 'The number of stale features this project has', }, potentiallyStaleFeatureCount: { + deprecated: true, type: 'number', example: 10, description: @@ -58,6 +61,7 @@ export const projectSchema = { format: 'date-time', }, updatedAt: { + deprecated: true, type: 'string', format: 'date-time', nullable: true, @@ -85,12 +89,14 @@ export const projectSchema = { "The project's [collaboration mode](https://docs.getunleash.io/reference/project-collaboration-mode). Determines whether non-project members can submit change requests or not.", }, defaultStickiness: { + deprecated: true, type: 'string', example: 'userId', description: 'A default stickiness for the project affecting the default stickiness value for variants and Gradual Rollout strategy', }, avgTimeToProduction: { + deprecated: true, type: 'number', example: 10, description: diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 5aa5a4d4b3..19d3a27bbb 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -568,6 +568,7 @@ export interface ICustomRole extends IRole { description: string; } +// @deprecated Remove with flag useProjectReadModel export interface IProjectWithCount extends IProject { featureCount: number; staleFeatureCount: number; diff --git a/src/lib/types/stores.ts b/src/lib/types/stores.ts index eaf56c5a4c..b4e064732d 100644 --- a/src/lib/types/stores.ts +++ b/src/lib/types/stores.ts @@ -48,6 +48,7 @@ import { IFeatureLifecycleReadModel } from '../features/feature-lifecycle/featur import { ILargestResourcesReadModel } from '../features/metrics/sizes/largest-resources-read-model-type'; import type { IntegrationEventsStore } from '../features/integration-events/integration-events-store'; import { IFeatureCollaboratorsReadModel } from '../features/feature-toggle/types/feature-collaborators-read-model-type'; +import type { IProjectReadModel } from '../features/project/project-read-model-type'; export interface IUnleashStores { accessStore: IAccessStore; @@ -100,6 +101,7 @@ export interface IUnleashStores { largestResourcesReadModel: ILargestResourcesReadModel; integrationEventsStore: IntegrationEventsStore; featureCollaboratorsReadModel: IFeatureCollaboratorsReadModel; + projectReadModel: IProjectReadModel; } export { @@ -151,4 +153,5 @@ export { ILargestResourcesReadModel, IFeatureCollaboratorsReadModel, type IntegrationEventsStore, + type IProjectReadModel, }; diff --git a/src/test/fixtures/store.ts b/src/test/fixtures/store.ts index 1c5b59d024..78b4765b9e 100644 --- a/src/test/fixtures/store.ts +++ b/src/test/fixtures/store.ts @@ -51,6 +51,7 @@ import { FakeFeatureStrategiesReadModel } from '../../lib/features/feature-toggl import { FakeFeatureLifecycleReadModel } from '../../lib/features/feature-lifecycle/fake-feature-lifecycle-read-model'; import { FakeLargestResourcesReadModel } from '../../lib/features/metrics/sizes/fake-largest-resources-read-model'; import { FakeFeatureCollaboratorsReadModel } from '../../lib/features/feature-toggle/fake-feature-collaborators-read-model'; +import { createFakeProjectReadModel } from '../../lib/features/project/createProjectReadModel'; const db = { select: () => ({ @@ -111,6 +112,7 @@ const createStores: () => IUnleashStores = () => { largestResourcesReadModel: new FakeLargestResourcesReadModel(), integrationEventsStore: {} as IntegrationEventsStore, featureCollaboratorsReadModel: new FakeFeatureCollaboratorsReadModel(), + projectReadModel: createFakeProjectReadModel(), }; };