refactor: read model for change request access checking (#3380)

Unleash · Mar 24, 2023 · ab91322 · ab91322
1 parent 9cdd870
commit ab91322
Show file tree

Hide file tree

Showing 18 changed files with 184 additions and 47 deletions.
diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts
@@ -475,19 +475,4 @@ export class AccessStore implements IAccessStore {
             [destinationEnvironment, sourceEnvironment],
         );
     }
-
-    async isChangeRequestsEnabled(
-        project: string,
-        environment: string,
-    ): Promise<boolean> {
-        const result = await this.db.raw(
-            `SELECT EXISTS(SELECT 1
-                           FROM ${T.CHANGE_REQUEST_SETTINGS}
-                           WHERE environment = ?
-                             and project = ?) AS present`,
-            [environment, project],
-        );
-        const { present } = result.rows[0];
-        return present;
-    }
 }
diff --git a/src/lib/features/change-request-access-service/change-request-access-read-model.ts b/src/lib/features/change-request-access-service/change-request-access-read-model.ts
@@ -0,0 +1,13 @@
+import User from '../../types/user';
+
+export interface IChangeRequestAccessReadModel {
+    canBypassChangeRequest(
+        project: string,
+        environment: string,
+        user?: User,
+    ): Promise<boolean>;
+    isChangeRequestsEnabled(
+        project: string,
+        environment: string,
+    ): Promise<boolean>;
+}
diff --git a/src/lib/features/change-request-access-service/createChangeRequestAccessReadModel.ts b/src/lib/features/change-request-access-service/createChangeRequestAccessReadModel.ts
@@ -0,0 +1,19 @@
+import { Db, IUnleashConfig } from 'lib/server-impl';
+import { ChangeRequestAccessReadModel } from './sql-change-request-access-read-model';
+import { createAccessService } from '../access/createAccessService';
+import { FakeChangeRequestAccessReadModel } from './fake-change-request-access-read-model';
+import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
+
+export const createChangeRequestAccessReadModel = (
+    db: Db,
+    config: IUnleashConfig,
+): IChangeRequestAccessReadModel => {
+    const accessService = createAccessService(db, config);
+
+    return new ChangeRequestAccessReadModel(db, accessService);
+};
+
+export const createFakeChangeRequestAccessService =
+    (): IChangeRequestAccessReadModel => {
+        return new FakeChangeRequestAccessReadModel();
+    };
diff --git a/src/lib/features/change-request-access-service/fake-change-request-access-read-model.ts b/src/lib/features/change-request-access-service/fake-change-request-access-read-model.ts
@@ -0,0 +1,22 @@
+import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
+
+export class FakeChangeRequestAccessReadModel
+    implements IChangeRequestAccessReadModel
+{
+    private canBypass: boolean;
+
+    private isChangeRequestEnabled: boolean;
+
+    constructor(canBypass = true, isChangeRequestEnabled = true) {
+        this.canBypass = canBypass;
+        this.isChangeRequestEnabled = isChangeRequestEnabled;
+    }
+
+    public async canBypassChangeRequest(): Promise<boolean> {
+        return this.canBypass;
+    }
+
+    public async isChangeRequestsEnabled(): Promise<boolean> {
+        return this.isChangeRequestEnabled;
+    }
+}
diff --git a/src/lib/features/change-request-access-service/sql-change-request-access-read-model.ts b/src/lib/features/change-request-access-service/sql-change-request-access-read-model.ts
@@ -0,0 +1,52 @@
+import { SKIP_CHANGE_REQUEST } from '../../types';
+import { Db } from '../../db/db';
+import { AccessService } from '../../services';
+import User from '../../types/user';
+import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
+
+export class ChangeRequestAccessReadModel
+    implements IChangeRequestAccessReadModel
+{
+    private db: Db;
+
+    private accessService: AccessService;
+
+    constructor(db: Db, accessService: AccessService) {
+        this.db = db;
+        this.accessService = accessService;
+    }
+
+    public async canBypassChangeRequest(
+        project: string,
+        environment: string,
+        user?: User,
+    ): Promise<boolean> {
+        const [canSkipChangeRequest, changeRequestEnabled] = await Promise.all([
+            user
+                ? this.accessService.hasPermission(
+                      user,
+                      SKIP_CHANGE_REQUEST,
+                      project,
+                      environment,
+                  )
+                : Promise.resolve(false),
+            this.isChangeRequestsEnabled(project, environment),
+        ]);
+        return !(changeRequestEnabled && !canSkipChangeRequest);
+    }
+
+    public async isChangeRequestsEnabled(
+        project: string,
+        environment: string,
+    ): Promise<boolean> {
+        const result = await this.db.raw(
+            `SELECT EXISTS(SELECT 1
+                           FROM change_request_settings
+                           WHERE environment = ?
+                             and project = ?) AS present`,
+            [environment, project],
+        );
+        const { present } = result.rows[0];
+        return present;
+    }
+}
diff --git a/src/lib/features/feature-toggle/createFeatureToggleService.ts b/src/lib/features/feature-toggle/createFeatureToggleService.ts
@@ -34,6 +34,10 @@ import FakeAccessStore from '../../../test/fixtures/fake-access-store';
 import FakeRoleStore from '../../../test/fixtures/fake-role-store';
 import FakeEnvironmentStore from '../../../test/fixtures/fake-environment-store';
 import EventStore from '../../db/event-store';
+import {
+    createChangeRequestAccessReadModel,
+    createFakeChangeRequestAccessService,
+} from '../change-request-access-service/createChangeRequestAccessReadModel';
 
 export const createFeatureToggleService = (
     db: Db,
@@ -85,6 +89,10 @@ export const createFeatureToggleService = (
         { segmentStore, featureStrategiesStore, eventStore },
         config,
     );
+    const changeRequestAccessReadMode = createChangeRequestAccessReadModel(
+        db,
+        config,
+    );
     const featureToggleService = new FeatureToggleService(
         {
             featureStrategiesStore,
@@ -99,6 +107,7 @@ export const createFeatureToggleService = (
         { getLogger, flagResolver },
         segmentService,
         accessService,
+        changeRequestAccessReadMode,
     );
     return featureToggleService;
 };
@@ -134,6 +143,7 @@ export const createFakeFeatureToggleService = (
         { segmentStore, featureStrategiesStore, eventStore },
         config,
     );
+    const changeRequestAccessReadMode = createFakeChangeRequestAccessService();
     const featureToggleService = new FeatureToggleService(
         {
             featureStrategiesStore,
@@ -148,6 +158,7 @@ export const createFakeFeatureToggleService = (
         { getLogger, flagResolver },
         segmentService,
         accessService,
+        changeRequestAccessReadMode,
     );
     return featureToggleService;
 };
diff --git a/src/lib/features/index.ts b/src/lib/features/index.ts
@@ -2,3 +2,4 @@ export * from './access/createAccessService';
 export * from './export-import-toggles/createExportImportService';
 export * from './feature-toggle/createFeatureToggleService';
 export * from './project/createProjectService';
+export * from './change-request-access-service/createChangeRequestAccessReadModel';
diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts
@@ -550,11 +550,4 @@ export class AccessService {
         await this.validateRoleIsUnique(role.name, existingId);
         return cleanedRole;
     }
-
-    async isChangeRequestsEnabled(
-        project: string,
-        environment: string,
-    ): Promise<boolean> {
-        return this.store.isChangeRequestsEnabled(project, environment);
-    }
 }
diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts
@@ -83,6 +83,7 @@ import NoAccessError from '../error/no-access-error';
 import { IFeatureProjectUserParams } from '../routes/admin-api/project/project-features';
 import { unique } from '../util/unique';
 import { ISegmentService } from 'lib/segments/segment-service-interface';
+import { IChangeRequestAccessReadModel } from '../features/change-request-access-service/change-request-access-read-model';
 
 interface IFeatureContext {
     featureName: string;
@@ -130,6 +131,8 @@ class FeatureToggleService {
 
     private flagResolver: IFlagResolver;
 
+    private changeRequestAccessReadModel: IChangeRequestAccessReadModel;
+
     constructor(
         {
             featureStrategiesStore,
@@ -157,6 +160,7 @@ class FeatureToggleService {
         }: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
         segmentService: ISegmentService,
         accessService: AccessService,
+        changeRequestAccessReadModel: IChangeRequestAccessReadModel,
     ) {
         this.logger = getLogger('services/feature-toggle-service.ts');
         this.featureStrategiesStore = featureStrategiesStore;
@@ -170,6 +174,7 @@ class FeatureToggleService {
         this.segmentService = segmentService;
         this.accessService = accessService;
         this.flagResolver = flagResolver;
+        this.changeRequestAccessReadModel = changeRequestAccessReadModel;
     }
 
     async validateFeaturesContext(
@@ -1734,18 +1739,13 @@ class FeatureToggleService {
         environment: string,
         user?: User,
     ) {
-        const [canSkipChangeRequest, changeRequestEnabled] = await Promise.all([
-            user
-                ? this.accessService.hasPermission(
-                      user,
-                      SKIP_CHANGE_REQUEST,
-                      project,
-                      environment,
-                  )
-                : Promise.resolve(false),
-            this.accessService.isChangeRequestsEnabled(project, environment),
-        ]);
-        if (changeRequestEnabled && !canSkipChangeRequest) {
+        const canBypass =
+            await this.changeRequestAccessReadModel.canBypassChangeRequest(
+                project,
+                environment,
+                user,
+            );
+        if (!canBypass) {
             throw new NoAccessError(SKIP_CHANGE_REQUEST);
         }
     }

diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts
@@ -52,6 +52,10 @@ import {
     createFakeExportImportTogglesService,
 } from '../features/export-import-toggles/createExportImportService';
 import { Db } from '../db/db';
+import {
+    createChangeRequestAccessReadModel,
+    createFakeChangeRequestAccessService,
+} from '../features/change-request-access-service/createChangeRequestAccessReadModel';
 
 // TODO: will be moved to scheduler feature directory
 export const scheduleServices = (
@@ -149,11 +153,15 @@ export const createServices = (
     const healthService = new HealthService(stores, config);
     const userFeedbackService = new UserFeedbackService(stores, config);
     const segmentService = new SegmentService(stores, config);
+    const changeRequestAccessReadModel = db
+        ? createChangeRequestAccessReadModel(db, config)
+        : createFakeChangeRequestAccessService();
     const featureToggleServiceV2 = new FeatureToggleService(
         stores,
         config,
         segmentService,
         accessService,
+        changeRequestAccessReadModel,
     );
     const environmentService = new EnvironmentService(stores, config);
     const featureTagService = new FeatureTagService(stores, config);

diff --git a/src/lib/types/stores/access-store.ts b/src/lib/types/stores/access-store.ts
@@ -138,9 +138,4 @@ export interface IAccessStore extends Store<IRole, number> {
         sourceEnvironment: string,
         destinationEnvironment: string,
     ): Promise<void>;
-
-    isChangeRequestsEnabled(
-        project: string,
-        environment: string,
-    ): Promise<boolean>;
 }
diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts
@@ -15,6 +15,7 @@ import { ALL_PROJECTS } from '../../../lib/util/constants';
 import { SegmentService } from '../../../lib/services/segment-service';
 import { GroupService } from '../../../lib/services/group-service';
 import { FavoritesService } from '../../../lib/services';
+import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
 
 let db: ITestDb;
 let stores: IUnleashStores;
@@ -216,11 +217,16 @@ beforeAll(async () => {
     editorRole = roles.find((r) => r.name === RoleName.EDITOR);
     adminRole = roles.find((r) => r.name === RoleName.ADMIN);
     readRole = roles.find((r) => r.name === RoleName.VIEWER);
+    const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
+        db.rawDatabase,
+        accessService,
+    );
     featureToggleService = new FeatureToggleService(
         stores,
         config,
         new SegmentService(stores, config),
         accessService,
+        changeRequestAccessReadModel,
     );
     favoritesService = new FavoritesService(stores, config);
     projectService = new ProjectService(

diff --git a/src/test/e2e/services/api-token-service.e2e.test.ts b/src/test/e2e/services/api-token-service.e2e.test.ts
@@ -11,6 +11,7 @@ import { AccessService } from '../../../lib/services/access-service';
 import { SegmentService } from '../../../lib/services/segment-service';
 import { GroupService } from '../../../lib/services/group-service';
 import { FavoritesService } from '../../../lib/services';
+import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
 
 let db;
 let stores;
@@ -26,11 +27,16 @@ beforeAll(async () => {
     stores = db.stores;
     const groupService = new GroupService(stores, config);
     const accessService = new AccessService(stores, config, groupService);
+    const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
+        db.rawDatabase,
+        accessService,
+    );
     const featureToggleService = new FeatureToggleService(
         stores,
         config,
         new SegmentService(stores, config),
         accessService,
+        changeRequestAccessReadModel,
     );
     const project = {
         id: 'test-project',