diff --git a/src/lib/features/dependent-features/createDependentFeaturesService.ts b/src/lib/features/dependent-features/createDependentFeaturesService.ts index f00fa202b8f8..c040c8e51376 100644 --- a/src/lib/features/dependent-features/createDependentFeaturesService.ts +++ b/src/lib/features/dependent-features/createDependentFeaturesService.ts @@ -14,6 +14,8 @@ import { createChangeRequestAccessReadModel, createFakeChangeRequestAccessService, } from '../change-request-access-service/createChangeRequestAccessReadModel'; +import { FeaturesReadModel } from '../feature-toggle/features-read-model'; +import { FakeFeaturesReadModel } from '../feature-toggle/fake-features-read-model'; export const createDependentFeaturesService = ( db: Db, @@ -35,12 +37,14 @@ export const createDependentFeaturesService = ( db, config, ); - return new DependentFeaturesService( + const featuresReadModel = new FeaturesReadModel(db); + return new DependentFeaturesService({ dependentFeaturesStore, dependentFeaturesReadModel, changeRequestAccessReadModel, + featuresReadModel, eventService, - ); + }); }; export const createFakeDependentFeaturesService = ( @@ -58,11 +62,13 @@ export const createFakeDependentFeaturesService = ( const dependentFeaturesStore = new FakeDependentFeaturesStore(); const dependentFeaturesReadModel = new FakeDependentFeaturesReadModel(); const changeRequestAccessReadModel = createFakeChangeRequestAccessService(); + const featuresReadModel = new FakeFeaturesReadModel(); - return new DependentFeaturesService( + return new DependentFeaturesService({ dependentFeaturesStore, dependentFeaturesReadModel, changeRequestAccessReadModel, + featuresReadModel, eventService, - ); + }); }; diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts index b33bf73c4434..12b536e65b96 100644 --- a/src/lib/features/dependent-features/dependent-features-service.ts +++ b/src/lib/features/dependent-features/dependent-features-service.ts @@ -8,6 +8,15 @@ import { User } from '../../server-impl'; import { SKIP_CHANGE_REQUEST } from '../../types'; import { IChangeRequestAccessReadModel } from '../change-request-access-service/change-request-access-read-model'; import { extractUsernameFromUser } from '../../util'; +import { IFeaturesReadModel } from '../feature-toggle/features-read-model-type'; + +interface IDependentFeaturesServiceDeps { + dependentFeaturesStore: IDependentFeaturesStore; + dependentFeaturesReadModel: IDependentFeaturesReadModel; + changeRequestAccessReadModel: IChangeRequestAccessReadModel; + featuresReadModel: IFeaturesReadModel; + eventService: EventService; +} export class DependentFeaturesService { private dependentFeaturesStore: IDependentFeaturesStore; @@ -16,17 +25,21 @@ export class DependentFeaturesService { private changeRequestAccessReadModel: IChangeRequestAccessReadModel; + private featuresReadModel: IFeaturesReadModel; + private eventService: EventService; - constructor( - dependentFeaturesStore: IDependentFeaturesStore, - dependentFeaturesReadModel: IDependentFeaturesReadModel, - changeRequestAccessReadModel: IChangeRequestAccessReadModel, - eventService: EventService, - ) { + constructor({ + featuresReadModel, + dependentFeaturesReadModel, + dependentFeaturesStore, + eventService, + changeRequestAccessReadModel, + }: IDependentFeaturesServiceDeps) { this.dependentFeaturesStore = dependentFeaturesStore; this.dependentFeaturesReadModel = dependentFeaturesReadModel; this.changeRequestAccessReadModel = changeRequestAccessReadModel; + this.featuresReadModel = featuresReadModel; this.eventService = eventService; } @@ -77,15 +90,23 @@ export class DependentFeaturesService { ): Promise { const { enabled, feature: parent, variants } = dependentFeature; - const children = await this.dependentFeaturesReadModel.getChildren([ - child, + const [children, parentExists] = await Promise.all([ + this.dependentFeaturesReadModel.getChildren([child]), + this.featuresReadModel.featureExists(parent), ]); + if (children.length > 0) { throw new InvalidOperationError( 'Transitive dependency detected. Cannot add a dependency to the feature that other features depend on.', ); } + if (!parentExists) { + throw new InvalidOperationError( + `No active feature ${parent} exists`, + ); + } + const featureDependency: FeatureDependency = enabled === false ? { diff --git a/src/lib/features/dependent-features/dependent.features.e2e.test.ts b/src/lib/features/dependent-features/dependent.features.e2e.test.ts index be378a58fa26..5d1980780068 100644 --- a/src/lib/features/dependent-features/dependent.features.e2e.test.ts +++ b/src/lib/features/dependent-features/dependent.features.e2e.test.ts @@ -136,3 +136,34 @@ test('should not allow to add a parent dependency to a feature that already has 403, ); }); + +test('should not allow to add non-existent parent dependency', async () => { + const grandparent = uuidv4(); + const parent = uuidv4(); + const child = uuidv4(); + await app.createFeature(child); + + await addFeatureDependency( + child, + { + feature: parent, + }, + 403, + ); +}); + +test('should not allow to add archived parent dependency', async () => { + const parent = uuidv4(); + const child = uuidv4(); + await app.createFeature(child); + await app.createFeature(parent); + await app.archiveFeature(parent); + + await addFeatureDependency( + child, + { + feature: parent, + }, + 403, + ); +}); diff --git a/src/lib/features/feature-toggle/fake-features-read-model.ts b/src/lib/features/feature-toggle/fake-features-read-model.ts new file mode 100644 index 000000000000..402f07019ff1 --- /dev/null +++ b/src/lib/features/feature-toggle/fake-features-read-model.ts @@ -0,0 +1,7 @@ +import { IFeaturesReadModel } from './features-read-model-type'; + +export class FakeFeaturesReadModel implements IFeaturesReadModel { + featureExists(): Promise { + return Promise.resolve(false); + } +} diff --git a/src/lib/features/feature-toggle/features-read-model-type.ts b/src/lib/features/feature-toggle/features-read-model-type.ts new file mode 100644 index 000000000000..2455c4ed30ee --- /dev/null +++ b/src/lib/features/feature-toggle/features-read-model-type.ts @@ -0,0 +1,3 @@ +export interface IFeaturesReadModel { + featureExists(parent: string): Promise; +} diff --git a/src/lib/features/feature-toggle/features-read-model.ts b/src/lib/features/feature-toggle/features-read-model.ts new file mode 100644 index 000000000000..0e5c2e381767 --- /dev/null +++ b/src/lib/features/feature-toggle/features-read-model.ts @@ -0,0 +1,19 @@ +import { Db } from '../../db/db'; +import { IFeaturesReadModel } from './features-read-model-type'; + +export class FeaturesReadModel implements IFeaturesReadModel { + private db: Db; + + constructor(db: Db) { + this.db = db; + } + + async featureExists(parent: string): Promise { + const rows = await this.db('features') + .where('name', parent) + .andWhere('archived_at', null) + .select('name'); + + return rows.length > 0; + } +}