From efd47b72a8d169075afa0b4ededd6756f87161f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Mon, 21 Nov 2022 10:37:16 +0100 Subject: [PATCH] feat: Add variants per env (#2471) ## About the changes Variants are now stored in each environment rather than in the feature toggle. This enables RBAC, suggest changes, etc to also apply to variants. Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item: #2254 ### Important files - **src/lib/db/feature-strategy-store.ts** a complex query was moved to a view named `features_view` - **src/lib/services/state-service.ts** export version number increased due to the new format ## Discussion points We're keeping the old column as a safeguard to be able to go back Co-authored-by: sighphyre Co-authored-by: Christopher Kolstad --- src/lib/db/feature-environment-store.ts | 92 ++++++- src/lib/db/feature-strategy-store.ts | 80 ++---- src/lib/db/feature-toggle-client-store.ts | 11 +- src/lib/db/feature-toggle-store.ts | 73 ++++-- src/lib/db/project-store.ts | 13 +- src/lib/routes/admin-api/project/features.ts | 2 + src/lib/routes/admin-api/project/variants.ts | 103 ++++++++ src/lib/schema/feature-schema.ts | 2 +- src/lib/services/environment-service.ts | 16 +- src/lib/services/feature-toggle-service.ts | 105 +++++++- src/lib/services/state-schema.ts | 7 +- src/lib/services/state-service.ts | 80 +++--- src/lib/types/events.ts | 26 ++ src/lib/types/model.ts | 2 + .../types/stores/feature-environment-store.ts | 16 +- .../types/stores/feature-strategies-store.ts | 4 + src/lib/types/stores/feature-toggle-store.ts | 12 + ...121635-move-variants-to-per-environment.js | 53 ++++ src/test/e2e/api/admin/playground.e2e.test.ts | 65 ++--- .../api/admin/project/variants.e2e.test.ts | 84 ++++++ src/test/e2e/api/admin/state.e2e.test.ts | 4 +- .../__snapshots__/openapi.e2e.test.ts.snap | 156 +++++++++++ .../feature-toggle-service-v2.e2e.test.ts | 47 ++++ .../e2e/services/playground-service.test.ts | 52 ++-- .../e2e/services/state-service.e2e.test.ts | 164 ++++++++++++ src/test/examples/variantsexport_v3.json | 245 ++++++++++++++++++ .../fake-feature-environment-store.ts | 39 ++- .../fixtures/fake-feature-strategies-store.ts | 7 + .../fixtures/fake-feature-toggle-store.ts | 40 ++- 29 files changed, 1399 insertions(+), 201 deletions(-) create mode 100644 src/migrations/20221107121635-move-variants-to-per-environment.js create mode 100644 src/test/e2e/services/state-service.e2e.test.ts create mode 100644 src/test/examples/variantsexport_v3.json diff --git a/src/lib/db/feature-environment-store.ts b/src/lib/db/feature-environment-store.ts index 9450dde876..0d916ff827 100644 --- a/src/lib/db/feature-environment-store.ts +++ b/src/lib/db/feature-environment-store.ts @@ -7,7 +7,7 @@ import { import { Logger, LogProvider } from '../logger'; import metricsHelper from '../util/metrics-helper'; import { DB_TIME } from '../metric-events'; -import { IFeatureEnvironment } from '../types/model'; +import { IFeatureEnvironment, IVariant } from '../types/model'; import NotFoundError from '../error/notfound-error'; import { v4 as uuidv4 } from 'uuid'; @@ -21,6 +21,7 @@ interface IFeatureEnvironmentRow { environment: string; feature_name: string; enabled: boolean; + variants?: []; } interface ISegmentRow { @@ -86,6 +87,7 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { enabled: md.enabled, featureName, environment, + variants: md.variants, }; } throw new NotFoundError( @@ -102,6 +104,7 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { enabled: r.enabled, featureName: r.feature_name, environment: r.environment, + variants: r.variants, })); } @@ -162,6 +165,24 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { return present; } + async getEnvironmentsForFeature( + featureName: string, + ): Promise { + const envs = await this.db(T.featureEnvs).where( + 'feature_name', + featureName, + ); + if (envs) { + return envs.map((r) => ({ + featureName: r.feature_name, + environment: r.environment, + variants: r.variants || [], + enabled: r.enabled, + })); + } + return []; + } + async getEnvironmentMetaData( environment: string, featureName: string, @@ -261,6 +282,41 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { .del(); } + // TODO: remove this once variants per env are GA + async clonePreviousVariants( + environment: string, + project: string, + ): Promise { + const rows = await this.db(`${T.features} as f`) + .select([ + this.db.raw(`'${environment}' as environment`), + 'fe.enabled', + 'fe.feature_name', + 'fe.variants', + ]) + .distinctOn(['environment', 'feature_name']) + .join(`${T.featureEnvs} as fe`, 'f.name', 'fe.feature_name') + .whereNot({ environment }) + .andWhere({ project }); + + const newRows = rows.map((row) => { + const r = row as any as IFeatureEnvironmentRow; + return { + variants: JSON.stringify(r.variants), + enabled: r.enabled, + environment: r.environment, + feature_name: r.feature_name, + }; + }); + + if (newRows.length > 0) { + await this.db(T.featureEnvs) + .insert(newRows) + .onConflict(['environment', 'feature_name']) + .merge(['variants']); + } + } + async connectFeatureToEnvironmentsForProject( featureName: string, projectId: string, @@ -295,6 +351,40 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { ); } + async addVariantsToFeatureEnvironment( + featureName: string, + environment: string, + variants: IVariant[], + ): Promise { + let v = variants || []; + v.sort((a, b) => a.name.localeCompare(b.name)); + await this.db(T.featureEnvs) + .insert({ + variants: JSON.stringify(v), + enabled: false, + feature_name: featureName, + environment: environment, + }) + .onConflict(['feature_name', 'environment']) + .merge(['variants']); + } + + async addFeatureEnvironment( + featureEnvironment: IFeatureEnvironment, + ): Promise { + let v = featureEnvironment.variants || []; + v.sort((a, b) => a.name.localeCompare(b.name)); + await this.db(T.featureEnvs) + .insert({ + variants: JSON.stringify(v), + enabled: featureEnvironment.enabled, + feature_name: featureEnvironment.featureName, + environment: featureEnvironment.environment, + }) + .onConflict(['feature_name', 'environment']) + .merge(['variants', 'enabled']); + } + async cloneStrategies( sourceEnvironment: string, destinationEnvironment: string, diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index 3e61a3d221..d0b6a0327f 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -215,58 +215,25 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { async getFeatureToggleWithEnvs( featureName: string, archived: boolean = false, + ): Promise { + return this.loadFeatureToggleWithEnvs(featureName, archived, false); + } + + async getFeatureToggleWithVariantEnvs( + featureName: string, + archived: boolean = false, + ): Promise { + return this.loadFeatureToggleWithEnvs(featureName, archived, true); + } + + async loadFeatureToggleWithEnvs( + featureName: string, + archived: boolean, + withEnvironmentVariants: boolean, ): Promise { const stopTimer = this.timer('getFeatureAdmin'); - const rows = await this.db('features') - .select( - 'features.name as name', - 'features.description as description', - 'features.type as type', - 'features.project as project', - 'features.stale as stale', - 'features.variants as variants', - 'features.impression_data as impression_data', - 'features.created_at as created_at', - 'features.last_seen_at as last_seen_at', - 'feature_environments.enabled as enabled', - 'feature_environments.environment as environment', - 'environments.name as environment_name', - 'environments.type as environment_type', - 'environments.sort_order as environment_sort_order', - 'feature_strategies.id as strategy_id', - 'feature_strategies.strategy_name as strategy_name', - 'feature_strategies.parameters as parameters', - 'feature_strategies.constraints as constraints', - 'feature_strategies.sort_order as sort_order', - 'fss.segment_id as segments', - ) - .leftJoin( - 'feature_environments', - 'feature_environments.feature_name', - 'features.name', - ) - .leftJoin('feature_strategies', function () { - this.on( - 'feature_strategies.feature_name', - '=', - 'feature_environments.feature_name', - ).andOn( - 'feature_strategies.environment', - '=', - 'feature_environments.environment', - ); - }) - .leftJoin( - 'environments', - 'feature_environments.environment', - 'environments.name', - ) - .leftJoin( - 'feature_strategy_segment as fss', - `fss.feature_strategy_id`, - `feature_strategies.id`, - ) - .where('features.name', featureName) + const rows = await this.db('features_view') + .where('name', featureName) .modify(FeatureToggleStore.filterByArchived, archived); stopTimer(); if (rows.length > 0) { @@ -280,7 +247,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { acc.description = r.description; acc.project = r.project; acc.stale = r.stale; - acc.variants = r.variants; + acc.createdAt = r.created_at; acc.lastSeenAt = r.last_seen_at; acc.type = r.type; @@ -289,8 +256,15 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { name: r.environment, }; } - const env = acc.environments[r.environment]; + + const variants = r.variants || []; + variants.sort((a, b) => a.name.localeCompare(b.name)); + if (withEnvironmentVariants) { + env.variants = variants; + } + acc.variants = variants; + env.enabled = r.enabled; env.type = r.environment_type; env.sortOrder = r.environment_sort_order; @@ -325,8 +299,6 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { ); return e; }); - featureToggle.variants = featureToggle.variants || []; - featureToggle.variants.sort((a, b) => a.name.localeCompare(b.name)); featureToggle.archived = archived; return featureToggle; } diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index a096e482d8..79775c1c2b 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -75,7 +75,7 @@ export default class FeatureToggleClientStore 'features.project as project', 'features.stale as stale', 'features.impression_data as impression_data', - 'features.variants as variants', + 'fe.variants as variants', 'features.created_at as created_at', 'features.last_seen_at as last_seen_at', 'fe.enabled as enabled', @@ -109,7 +109,12 @@ export default class FeatureToggleClientStore ) .leftJoin( this.db('feature_environments') - .select('feature_name', 'enabled', 'environment') + .select( + 'feature_name', + 'enabled', + 'environment', + 'variants', + ) .where({ environment }) .as('fe'), 'fe.feature_name', @@ -180,7 +185,7 @@ export default class FeatureToggleClientStore feature.project = r.project; feature.stale = r.stale; feature.type = r.type; - feature.variants = r.variants; + feature.variants = r.variants || []; feature.project = r.project; if (isAdmin) { feature.lastSeenAt = r.last_seen_at; diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts index 91db0006aa..9becf37c61 100644 --- a/src/lib/db/feature-toggle-store.ts +++ b/src/lib/db/feature-toggle-store.ts @@ -13,7 +13,6 @@ const FEATURE_COLUMNS = [ 'type', 'project', 'stale', - 'variants', 'created_at', 'impression_data', 'last_seen_at', @@ -25,7 +24,6 @@ export interface FeaturesTable { description: string; type: string; stale: boolean; - variants?: string; project: string; last_seen_at?: Date; created_at?: Date; @@ -34,7 +32,12 @@ export interface FeaturesTable { archived_at?: Date; } +interface VariantDTO { + variants: IVariant[]; +} + const TABLE = 'features'; +const FEATURE_ENVIRONMENTS_TABLE = 'feature_environments'; export default class FeatureToggleStore implements IFeatureToggleStore { private db: Knex; @@ -156,15 +159,12 @@ export default class FeatureToggleStore implements IFeatureToggleStore { if (!row) { throw new NotFoundError('No feature toggle found'); } - const sortedVariants = (row.variants as unknown as IVariant[]) || []; - sortedVariants.sort((a, b) => a.name.localeCompare(b.name)); return { name: row.name, description: row.description, type: row.type, project: row.project, stale: row.stale, - variants: sortedVariants, createdAt: row.created_at, lastSeenAt: row.last_seen_at, impressionData: row.impression_data, @@ -173,13 +173,14 @@ export default class FeatureToggleStore implements IFeatureToggleStore { }; } - rowToVariants(row: FeaturesTable): IVariant[] { - if (!row) { - throw new NotFoundError('No feature toggle found'); + rowToEnvVariants(variantRows: VariantDTO[]): IVariant[] { + if (!variantRows.length) { + return []; } - const sortedVariants = (row.variants as unknown as IVariant[]) || []; - sortedVariants.sort((a, b) => a.name.localeCompare(b.name)); + const sortedVariants = + (variantRows[0].variants as unknown as IVariant[]) || []; + sortedVariants.sort((a, b) => a.name.localeCompare(b.name)); return sortedVariants; } @@ -193,7 +194,6 @@ export default class FeatureToggleStore implements IFeatureToggleStore { stale: data.stale, created_at: data.createdAt, impression_data: data.impressionData, - variants: JSON.stringify(data.variants), }; if (!row.created_at) { delete row.created_at; @@ -253,10 +253,37 @@ export default class FeatureToggleStore implements IFeatureToggleStore { } async getVariants(featureName: string): Promise { - const row = await this.db(TABLE) - .select('variants') - .where({ name: featureName }); - return this.rowToVariants(row[0]); + if (!(await this.exists(featureName))) { + throw new NotFoundError('No feature toggle found'); + } + const row = await this.db(`${TABLE} as f`) + .select('fe.variants') + .join( + `${FEATURE_ENVIRONMENTS_TABLE} as fe`, + 'fe.feature_name', + 'f.name', + ) + .where({ name: featureName }) + .limit(1); + + return this.rowToEnvVariants(row); + } + + async getVariantsForEnv( + featureName: string, + environment: string, + ): Promise { + const row = await this.db(`${TABLE} as f`) + .select('fev.variants') + .join( + `${FEATURE_ENVIRONMENTS_TABLE} as fev`, + 'fev.feature_name', + 'f.name', + ) + .where({ name: featureName }) + .andWhere({ environment }); + + return this.rowToEnvVariants(row); } async saveVariants( @@ -264,11 +291,19 @@ export default class FeatureToggleStore implements IFeatureToggleStore { featureName: string, newVariants: IVariant[], ): Promise { + const variantsString = JSON.stringify(newVariants); + await this.db('feature_environments') + .update('variants', variantsString) + .where('feature_name', featureName); + const row = await this.db(TABLE) - .update({ variants: JSON.stringify(newVariants) }) - .where({ project: project, name: featureName }) - .returning(FEATURE_COLUMNS); - return this.rowToFeature(row[0]); + .select(FEATURE_COLUMNS) + .where({ project: project, name: featureName }); + + const toggle = this.rowToFeature(row[0]); + toggle.variants = newVariants; + + return toggle; } } diff --git a/src/lib/db/project-store.ts b/src/lib/db/project-store.ts index 8888cb3c15..07e93f2310 100644 --- a/src/lib/db/project-store.ts +++ b/src/lib/db/project-store.ts @@ -178,15 +178,12 @@ class ProjectStore implements IProjectStore { .returning(COLUMNS) .onConflict('id') .ignore(); - if (rows.length > 0) { - await this.addDefaultEnvironment(rows); - environments - ?.filter((env) => env.name !== DEFAULT_ENV) - .forEach((env) => { - projects.forEach((project) => { - this.addEnvironmentToProject(project.id, env.name); - }); + if (environments && rows.length > 0) { + environments.forEach((env) => { + projects.forEach(async (project) => { + await this.addEnvironmentToProject(project.id, env.name); }); + }); return rows.map(this.mapRow); } return []; diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index 31c5ba4eae..c547b877cb 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -462,10 +462,12 @@ export default class ProjectFeaturesController extends Controller { res: Response, ): Promise { const { featureName, projectId } = req.params; + const { variantEnvironments } = req.query; const feature = await this.featureService.getFeature( featureName, false, projectId, + variantEnvironments === 'true', ); res.status(200).json(feature); } diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index ef8402028c..ad182f8fe6 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -14,6 +14,12 @@ import { createRequestSchema } from '../../../openapi/util/create-request-schema import { createResponseSchema } from '../../../openapi/util/create-response-schema'; const PREFIX = '/:projectId/features/:featureName/variants'; +const ENV_PREFIX = + '/:projectId/features/:featureName/environments/:environment/variants'; + +interface FeatureEnvironmentParams extends FeatureParams { + environment: string; +} interface FeatureParams extends ProjectParam { featureName: string; @@ -84,6 +90,53 @@ export default class VariantsController extends Controller { }), ], }); + this.route({ + method: 'get', + path: ENV_PREFIX, + permission: NONE, + handler: this.getVariantsOnEnv, + middleware: [ + openApiService.validPath({ + tags: ['Features'], + operationId: 'getEnvironmentFeatureVariants', + responses: { + 200: createResponseSchema('featureVariantsSchema'), + }, + }), + ], + }); + this.route({ + method: 'patch', + path: ENV_PREFIX, + permission: UPDATE_FEATURE_VARIANTS, + handler: this.patchVariantsOnEnv, + middleware: [ + openApiService.validPath({ + tags: ['Features'], + operationId: 'patchEnvironmentsFeatureVariants', + requestBody: createRequestSchema('patchesSchema'), + responses: { + 200: createResponseSchema('featureVariantsSchema'), + }, + }), + ], + }); + this.route({ + method: 'put', + path: ENV_PREFIX, + permission: UPDATE_FEATURE_VARIANTS, + handler: this.overwriteVariantsOnEnv, + middleware: [ + openApiService.validPath({ + tags: ['Features'], + operationId: 'overwriteEnvironmentFeatureVariants', + requestBody: createRequestSchema('variantsSchema'), + responses: { + 200: createResponseSchema('featureVariantsSchema'), + }, + }), + ], + }); } async getVariants( @@ -131,4 +184,54 @@ export default class VariantsController extends Controller { variants: updatedFeature.variants, }); } + + async getVariantsOnEnv( + req: Request, + res: Response, + ): Promise { + const { featureName, environment } = req.params; + const variants = await this.featureService.getVariantsForEnv( + featureName, + environment, + ); + res.status(200).json({ version: 1, variants: variants || [] }); + } + + async patchVariantsOnEnv( + req: IAuthRequest, + res: Response, + ): Promise { + const { projectId, featureName, environment } = req.params; + const userName = extractUsername(req); + + const variants = await this.featureService.updateVariantsOnEnv( + featureName, + projectId, + environment, + req.body, + userName, + ); + res.status(200).json({ + version: 1, + variants, + }); + } + + async overwriteVariantsOnEnv( + req: IAuthRequest, + res: Response, + ): Promise { + const { featureName, environment } = req.params; + const userName = extractUsername(req); + const variants = await this.featureService.saveVariantsOnEnv( + featureName, + environment, + req.body, + userName, + ); + res.status(200).json({ + version: 1, + variants: variants, + }); + } } diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index c4659e22fd..c81afd4f30 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -28,7 +28,7 @@ export const strategiesSchema = joi.object().keys({ parameters: joi.object(), }); -const variantValueSchema = joi +export const variantValueSchema = joi .string() .required() // perform additional validation diff --git a/src/lib/services/environment-service.ts b/src/lib/services/environment-service.ts index 17a794c6e6..86562a8951 100644 --- a/src/lib/services/environment-service.ts +++ b/src/lib/services/environment-service.ts @@ -11,6 +11,7 @@ import { IFeatureStrategiesStore } from '../types/stores/feature-strategies-stor import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store'; import { IProjectStore } from 'lib/types/stores/project-store'; import MinimumOneEnvironmentError from '../error/minimum-one-environment-error'; +import { IFlagResolver } from 'lib/types/experimental'; export default class EnvironmentService { private logger: Logger; @@ -23,6 +24,8 @@ export default class EnvironmentService { private featureEnvironmentStore: IFeatureEnvironmentStore; + private flagResolver: IFlagResolver; + constructor( { environmentStore, @@ -36,13 +39,17 @@ export default class EnvironmentService { | 'featureEnvironmentStore' | 'projectStore' >, - { getLogger }: Pick, + { + getLogger, + flagResolver, + }: Pick, ) { this.logger = getLogger('services/environment-service.ts'); this.environmentStore = environmentStore; this.featureStrategiesStore = featureStrategiesStore; this.featureEnvironmentStore = featureEnvironmentStore; this.projectStore = projectStore; + this.flagResolver = flagResolver; } async getAll(): Promise { @@ -90,6 +97,13 @@ export default class EnvironmentService { environment, projectId, ); + + if (!this.flagResolver.isEnabled('variantsPerEnvironment')) { + await this.featureEnvironmentStore.clonePreviousVariants( + environment, + projectId, + ); + } } catch (e) { if (e.code === UNIQUE_CONSTRAINT_VIOLATION) { throw new NameExistsError( diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index e882f1b691..5e1d878add 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -25,6 +25,7 @@ import { FeatureStrategyRemoveEvent, FeatureStrategyUpdateEvent, FeatureVariantEvent, + EnvironmentVariantEvent, } from '../types/events'; import NotFoundError from '../error/notfound-error'; import { @@ -611,16 +612,23 @@ class FeatureToggleService { featureName: string, archived: boolean = false, projectId?: string, + environmentVariants: boolean = false, ): Promise { - const feature = - await this.featureStrategiesStore.getFeatureToggleWithEnvs( - featureName, - archived, - ); if (projectId) { await this.validateFeatureContext({ featureName, projectId }); } - return feature; + + if (environmentVariants) { + return this.featureStrategiesStore.getFeatureToggleWithVariantEnvs( + featureName, + archived, + ); + } else { + return this.featureStrategiesStore.getFeatureToggleWithEnvs( + featureName, + archived, + ); + } } /** @@ -632,6 +640,17 @@ class FeatureToggleService { return this.featureToggleStore.getVariants(featureName); } + async getVariantsForEnv( + featureName: string, + environment: string, + ): Promise { + const featureEnvironment = await this.featureEnvironmentStore.get({ + featureName, + environment, + }); + return featureEnvironment.variants || []; + } + async getFeatureMetadata(featureName: string): Promise { return this.featureToggleStore.get(featureName); } @@ -705,6 +724,20 @@ class FeatureToggleService { projectId, ); + if (value.variants && value.variants.length > 0) { + const environments = + await this.featureEnvironmentStore.getEnvironmentsForFeature( + featureName, + ); + environments.forEach(async (featureEnv) => { + await this.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + featureEnv.environment, + value.variants, + ); + }); + } + const tags = await this.tagStore.getAllTagsForFeature(featureName); await this.eventStore.store( @@ -762,7 +795,7 @@ class FeatureToggleService { }), ); - await Promise.allSettled(tasks); + await Promise.all(tasks); return created; } @@ -1195,10 +1228,30 @@ class FeatureToggleService { createdBy: string, ): Promise { const oldVariants = await this.getVariants(featureName); - const { newDocument } = await applyPatch(oldVariants, newVariants); + const { newDocument } = applyPatch(oldVariants, newVariants); return this.saveVariants(featureName, project, newDocument, createdBy); } + async updateVariantsOnEnv( + featureName: string, + project: string, + environment: string, + newVariants: Operation[], + createdBy: string, + ): Promise { + const oldVariants = await this.getVariantsForEnv( + featureName, + environment, + ); + const { newDocument } = await applyPatch(oldVariants, newVariants); + return this.saveVariantsOnEnv( + featureName, + environment, + newDocument, + createdBy, + ); + } + async saveVariants( featureName: string, project: string, @@ -1229,6 +1282,38 @@ class FeatureToggleService { return featureToggle; } + async saveVariantsOnEnv( + featureName: string, + environment: string, + newVariants: IVariant[], + createdBy: string, + ): Promise { + await variantsArraySchema.validateAsync(newVariants); + const fixedVariants = this.fixVariantWeights(newVariants); + const oldVariants = ( + await this.featureEnvironmentStore.get({ + featureName, + environment, + }) + ).variants; + + await this.eventStore.store( + new EnvironmentVariantEvent({ + featureName, + environment, + createdBy, + oldVariants, + newVariants: fixedVariants, + }), + ); + await this.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + environment, + fixedVariants, + ); + return fixedVariants; + } + fixVariantWeights(variants: IVariant[]): IVariant[] { let variableVariants = variants.filter((x) => { return x.weightType === WeightType.VARIABLE; @@ -1265,7 +1350,9 @@ class FeatureToggleService { } return x; }); - return variableVariants.concat(fixedVariants); + return variableVariants + .concat(fixedVariants) + .sort((a, b) => a.name.localeCompare(b.name)); } private async stopWhenChangeRequestsEnabled( diff --git a/src/lib/services/state-schema.ts b/src/lib/services/state-schema.ts index 5059cda465..c0b0f74251 100644 --- a/src/lib/services/state-schema.ts +++ b/src/lib/services/state-schema.ts @@ -1,5 +1,9 @@ import joi from 'joi'; -import { featureSchema, featureTagSchema } from '../schema/feature-schema'; +import { + featureSchema, + featureTagSchema, + variantsSchema, +} from '../schema/feature-schema'; import strategySchema from './strategy-schema'; import { tagSchema } from './tag-schema'; import { tagTypeSchema } from './tag-type-schema'; @@ -24,6 +28,7 @@ export const featureEnvironmentsSchema = joi.object().keys({ environment: joi.string(), featureName: joi.string(), enabled: joi.boolean(), + variants: joi.array().items(variantsSchema).optional(), }); export const environmentSchema = joi.object().keys({ diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index d4e56f4ca4..aac185bd56 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -46,7 +46,7 @@ import { IFeatureStrategiesStore } from '../types/stores/feature-strategies-stor import { IEnvironmentStore } from '../types/stores/environment-store'; import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store'; import { IUnleashStores } from '../types/stores'; -import { DEFAULT_ENV, ALL_ENVS } from '../util/constants'; +import { ALL_ENVS, DEFAULT_ENV } from '../util/constants'; import { GLOBAL_ENV } from '../types/environment'; import { ISegmentStore } from '../types/stores/segment-store'; import { PartialSome } from '../types/partial'; @@ -153,6 +153,18 @@ export default class StateService { }); } + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + moveVariantsToFeatureEnvironments(data: any) { + data.featureEnvironments?.forEach((featureEnvironment) => { + let feature = data.features?.find( + (f) => f.name === featureEnvironment.featureName, + ); + if (feature) { + featureEnvironment.variants = feature.variants || []; + } + }); + } + async import({ data, userName = 'importUser', @@ -162,6 +174,9 @@ export default class StateService { if (data.version === 2) { this.replaceGlobalEnvWithDefaultEnv(data); } + if (!data.version || data.version < 4) { + this.moveVariantsToFeatureEnvironments(data); + } const importData = await stateSchema.validateAsync(data); let importedEnvironments: IEnvironment[] = []; @@ -199,15 +214,10 @@ export default class StateService { userName, dropBeforeImport, keepExisting, + featureEnvironments, }); if (featureEnvironments) { - // make sure the project and environment are connected - // before importing featureEnvironments - await this.linkFeatureEnvironments({ - features, - featureEnvironments, - }); await this.importFeatureEnvironments({ featureEnvironments, }); @@ -267,27 +277,7 @@ export default class StateService { } // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - async linkFeatureEnvironments({ - features, - featureEnvironments, - }): Promise { - const linkTasks = featureEnvironments.map(async (fe) => { - const project = features.find( - (f) => f.project && f.name === fe.featureName, - ).project; - if (project) { - return this.featureEnvironmentStore.connectProject( - fe.environment, - project, - true, // make it idempotent - ); - } - }); - await Promise.all(linkTasks); - } - - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - enabledInConfiguration(feature: string, env) { + enabledIn(feature: string, env) { const config = {}; env.filter((e) => e.featureName === feature).forEach((e) => { config[e.environment] = e.enabled || false; @@ -298,20 +288,15 @@ export default class StateService { // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types async importFeatureEnvironments({ featureEnvironments }): Promise { await Promise.all( - featureEnvironments.map((env) => - this.toggleStore - .getProjectId(env.featureName) - .then((id) => - this.featureEnvironmentStore.connectFeatureToEnvironmentsForProject( - env.featureName, - id, - this.enabledInConfiguration( - env.featureName, - featureEnvironments, - ), - ), + featureEnvironments + .filter(async (env) => { + await this.environmentStore.exists(env.environment); + }) + .map(async (featureEnvironment) => + this.featureEnvironmentStore.addFeatureEnvironment( + featureEnvironment, ), - ), + ), ); } @@ -363,6 +348,7 @@ export default class StateService { featureName: feature.name, environment: DEFAULT_ENV, enabled: feature.enabled, + variants: feature.variants || [], })); return { features: newFeatures, @@ -377,6 +363,7 @@ export default class StateService { userName, dropBeforeImport, keepExisting, + featureEnvironments, }): Promise { this.logger.info(`Importing ${features.length} feature toggles`); const oldToggles = dropBeforeImport @@ -398,12 +385,11 @@ export default class StateService { .filter(filterExisting(keepExisting, oldToggles)) .filter(filterEqual(oldToggles)) .map(async (feature) => { - const { name, project, variants = [] } = feature; await this.toggleStore.create(feature.project, feature); - await this.toggleStore.saveVariants( - project, - name, - variants, + await this.featureEnvironmentStore.connectFeatureToEnvironmentsForProject( + feature.name, + feature.project, + this.enabledIn(feature.name, featureEnvironments), ); await this.eventStore.store({ type: FEATURE_IMPORT, @@ -772,7 +758,7 @@ export default class StateService { segments, featureStrategySegments, ]) => ({ - version: 3, + version: 4, features, strategies, projects, diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index 79ea3f0259..79b2df3008 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -8,6 +8,8 @@ export const FEATURE_DELETED = 'feature-deleted'; export const FEATURE_UPDATED = 'feature-updated'; export const FEATURE_METADATA_UPDATED = 'feature-metadata-updated'; export const FEATURE_VARIANTS_UPDATED = 'feature-variants-updated'; +export const FEATURE_ENVIRONMENT_VARIANTS_UPDATED = + 'feature-environment-variants-updated'; export const FEATURE_PROJECT_CHANGE = 'feature-project-change'; export const FEATURE_ARCHIVED = 'feature-archived'; export const FEATURE_REVIVED = 'feature-revived'; @@ -192,6 +194,30 @@ export class FeatureVariantEvent extends BaseEvent { } } +export class EnvironmentVariantEvent extends BaseEvent { + readonly environment: string; + + readonly featureName: string; + + readonly data: { variants: IVariant[] }; + + readonly preData: { variants: IVariant[] }; + + constructor(p: { + featureName: string; + environment: string; + createdBy: string; + newVariants: IVariant[]; + oldVariants: IVariant[]; + }) { + super(FEATURE_ENVIRONMENT_VARIANTS_UPDATED, p.createdBy); + this.featureName = p.featureName; + this.environment = p.environment; + this.data = { variants: p.newVariants }; + this.preData = { variants: p.oldVariants }; + } +} + export class FeatureChangeProjectEvent extends BaseEvent { readonly project: string; diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 120fc786b6..3955d7d332 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -91,6 +91,7 @@ export interface FeatureToggleLegacy extends FeatureToggle { export interface IEnvironmentDetail extends IEnvironmentOverview { strategies: IStrategyConfig[]; + variants: IVariant[]; } export interface ISortOrder { @@ -101,6 +102,7 @@ export interface IFeatureEnvironment { environment: string; featureName: string; enabled: boolean; + variants?: IVariant[]; } export interface IVariant { diff --git a/src/lib/types/stores/feature-environment-store.ts b/src/lib/types/stores/feature-environment-store.ts index cdfc2c0f89..0a538d0b42 100644 --- a/src/lib/types/stores/feature-environment-store.ts +++ b/src/lib/types/stores/feature-environment-store.ts @@ -1,4 +1,4 @@ -import { IFeatureEnvironment } from '../model'; +import { IFeatureEnvironment, IVariant } from '../model'; import { Store } from './store'; export interface FeatureEnvironmentKey { @@ -12,6 +12,9 @@ export interface IFeatureEnvironmentStore environment: string, featureName: string, ): Promise; + getEnvironmentsForFeature( + featureName: string, + ): Promise; isEnvironmentEnabled( featureName: string, environment: string, @@ -62,4 +65,15 @@ export interface IFeatureEnvironmentStore sourceEnvironment: string, destinationEnvironment: string, ): Promise; + addVariantsToFeatureEnvironment( + featureName: string, + environment: string, + variants: IVariant[], + ): Promise; + + addFeatureEnvironment( + featureEnvironment: IFeatureEnvironment, + ): Promise; + + clonePreviousVariants(environment: string, project: string): Promise; } diff --git a/src/lib/types/stores/feature-strategies-store.ts b/src/lib/types/stores/feature-strategies-store.ts index 78c2d751c0..fca2299aa6 100644 --- a/src/lib/types/stores/feature-strategies-store.ts +++ b/src/lib/types/stores/feature-strategies-store.ts @@ -34,6 +34,10 @@ export interface IFeatureStrategiesStore featureName: string, archived?: boolean, ): Promise; + getFeatureToggleWithVariantEnvs( + featureName: string, + archived?, + ): Promise; getFeatureOverview( projectId: string, archived: boolean, diff --git a/src/lib/types/stores/feature-toggle-store.ts b/src/lib/types/stores/feature-toggle-store.ts index 77e5626607..b4eaef28c2 100644 --- a/src/lib/types/stores/feature-toggle-store.ts +++ b/src/lib/types/stores/feature-toggle-store.ts @@ -16,7 +16,19 @@ export interface IFeatureToggleStore extends Store { archive(featureName: string): Promise; revive(featureName: string): Promise; getAll(query?: Partial): Promise; + /** + * @deprecated - Variants should be fetched from FeatureEnvironmentStore (since variants are now; since 4.18, connected to environments) + * @param featureName + * TODO: Remove before release 5.0 + */ getVariants(featureName: string): Promise; + /** + * TODO: Remove before release 5.0 + * @deprecated - Variants should be fetched from FeatureEnvironmentStore (since variants are now; since 4.18, connected to environments) + * @param project + * @param featureName + * @param newVariants + */ saveVariants( project: string, featureName: string, diff --git a/src/migrations/20221107121635-move-variants-to-per-environment.js b/src/migrations/20221107121635-move-variants-to-per-environment.js new file mode 100644 index 0000000000..3ce7abc116 --- /dev/null +++ b/src/migrations/20221107121635-move-variants-to-per-environment.js @@ -0,0 +1,53 @@ +'use strict'; + +exports.up = function (db, callback) { + db.runSql( + ` + ALTER TABLE feature_environments ADD COLUMN variants JSONB DEFAULT '[]'::jsonb NOT NULL; + WITH feature_variants AS (SELECT variants, name FROM features) + UPDATE feature_environments SET variants = feature_variants.variants FROM feature_variants WHERE feature_name = feature_variants.name; + + CREATE VIEW features_view AS + SELECT + features.name as name, + features.description as description, + features.type as type, + features.project as project, + features.stale as stale, + feature_environments.variants as variants, + features.impression_data as impression_data, + features.created_at as created_at, + features.last_seen_at as last_seen_at, + features.archived_at as archived_at, + feature_environments.enabled as enabled, + feature_environments.environment as environment, + environments.name as environment_name, + environments.type as environment_type, + environments.sort_order as environment_sort_order, + feature_strategies.id as strategy_id, + feature_strategies.strategy_name as strategy_name, + feature_strategies.parameters as parameters, + feature_strategies.constraints as constraints, + feature_strategies.sort_order as sort_order, + fss.segment_id as segments + FROM + features + LEFT JOIN feature_environments ON feature_environments.feature_name = features.name + LEFT JOIN feature_strategies ON feature_strategies.feature_name = feature_environments.feature_name + and feature_strategies.environment = feature_environments.environment + LEFT JOIN environments ON feature_environments.environment = environments.name + LEFT JOIN feature_strategy_segment as fss ON fss.feature_strategy_id = feature_strategies.id; + `, + callback, + ); +}; + +exports.down = function (db, callback) { + db.runSql( + ` + DROP VIEW features_view; + ALTER TABLE feature_environments DROP COLUMN variants; + `, + callback, + ); +}; diff --git a/src/test/e2e/api/admin/playground.e2e.test.ts b/src/test/e2e/api/admin/playground.e2e.test.ts index 96d48e05d1..38fa0a5ac8 100644 --- a/src/test/e2e/api/admin/playground.e2e.test.ts +++ b/src/test/e2e/api/admin/playground.e2e.test.ts @@ -69,12 +69,25 @@ const playgroundRequest = async ( describe('Playground API E2E', () => { // utility function for seeding the database before runs - const seedDatabase = ( + const seedDatabase = async ( database: ITestDb, features: ClientFeatureSchema[], environment: string, - ): Promise => - Promise.all( + ): Promise => { + // create environment if necessary + await database.stores.environmentStore + .create({ + name: environment, + type: 'development', + enabled: true, + }) + .catch(() => { + // purposefully left empty: env creation may fail if the + // env already exists, and because of the async nature + // of things, this is the easiest way to make it work. + }); + + return Promise.all( features.map(async (feature) => { // create feature const toggle = await database.stores.featureToggleStore.create( @@ -82,28 +95,28 @@ describe('Playground API E2E', () => { { ...feature, createdAt: undefined, - variants: [ - ...(feature.variants ?? []).map((variant) => ({ - ...variant, - weightType: WeightType.VARIABLE, - stickiness: 'default', - })), - ], + variants: null, }, ); - // create environment if necessary - await database.stores.environmentStore - .create({ - name: environment, - type: 'development', - enabled: true, - }) - .catch(() => { - // purposefully left empty: env creation may fail if the - // env already exists, and because of the async nature - // of things, this is the easiest way to make it work. - }); + // enable/disable the feature in environment + await database.stores.featureEnvironmentStore.addEnvironmentToFeature( + feature.name, + environment, + feature.enabled, + ); + + await database.stores.featureToggleStore.saveVariants( + feature.project, + feature.name, + [ + ...(feature.variants ?? []).map((variant) => ({ + ...variant, + weightType: WeightType.VARIABLE, + stickiness: 'default', + })), + ], + ); // assign strategies await Promise.all( @@ -122,16 +135,10 @@ describe('Playground API E2E', () => { ), ); - // enable/disable the feature in environment - await database.stores.featureEnvironmentStore.addEnvironmentToFeature( - feature.name, - environment, - feature.enabled, - ); - return toggle; }), ); + }; test('Returned features should be a subset of the provided toggles', async () => { await fc.assert( diff --git a/src/test/e2e/api/admin/project/variants.e2e.test.ts b/src/test/e2e/api/admin/project/variants.e2e.test.ts index 5044bea52e..e66b3d7bfe 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -21,6 +21,11 @@ test('Can get variants for a feature', async () => { const featureName = 'feature-variants'; const variantName = 'fancy-variant'; await db.stores.featureToggleStore.create('default', { name: featureName }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); await db.stores.featureToggleStore.saveVariants('default', featureName, [ { name: variantName, @@ -89,6 +94,11 @@ test('Can patch variants for a feature and get a response of new variant', async await db.stores.featureToggleStore.create('default', { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); await db.stores.featureToggleStore.saveVariants( 'default', featureName, @@ -126,6 +136,13 @@ test('Can add variant for a feature', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + await db.stores.featureToggleStore.saveVariants( 'default', featureName, @@ -174,6 +191,13 @@ test('Can remove variant for a feature', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + await db.stores.featureToggleStore.saveVariants( 'default', featureName, @@ -211,6 +235,11 @@ test('PUT overwrites current variant on feature', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); await db.stores.featureToggleStore.saveVariants( 'default', featureName, @@ -317,6 +346,12 @@ test('PATCHING with all variable weightTypes forces weights to sum to no less th name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const newVariants: IVariant[] = []; const observer = jsonpatch.observe(newVariants); @@ -434,6 +469,12 @@ test('Patching with a fixed variant and variable variants splits remaining weigh name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const newVariants: IVariant[] = []; const observer = jsonpatch.observe(newVariants); newVariants.push({ @@ -525,6 +566,12 @@ test('Multiple fixed variants gets added together to decide how much weight vari name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const newVariants: IVariant[] = []; const observer = jsonpatch.observe(newVariants); @@ -570,6 +617,12 @@ test('If sum of fixed variant weight exceed 1000 fails with 400', async () => { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const newVariants: IVariant[] = []; const observer = jsonpatch.observe(newVariants); @@ -611,6 +664,12 @@ test('If sum of fixed variant weight equals 1000 variable variants gets weight 0 name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const newVariants: IVariant[] = []; const observer = jsonpatch.observe(newVariants); @@ -673,6 +732,12 @@ test('PATCH endpoint validates uniqueness of variant names', async () => { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + await db.stores.featureToggleStore.saveVariants( 'default', featureName, @@ -711,6 +776,13 @@ test('PUT endpoint validates uniqueness of variant names', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + await app.request .put(`/api/admin/projects/default/features/${featureName}/variants`) .send([ @@ -741,6 +813,12 @@ test('Variants should be sorted by their name when PUT', async () => { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + await app.request .put(`/api/admin/projects/default/features/${featureName}/variants`) .send([ @@ -784,6 +862,12 @@ test('Variants should be sorted by name when PATCHed as well', async () => { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); + const variants: IVariant[] = []; const observer = jsonpatch.observe(variants); variants.push({ diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index b0e31d794c..5376654585 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -320,7 +320,7 @@ test('Roundtrip with strategies in multiple environments works', async () => { test(`Importing version 2 replaces :global: environment with 'default'`, async () => { await app.request - .post('/api/admin/state/import') + .post('/api/admin/state/import?drop=true') .attach('file', 'src/test/examples/exported412-version2.json') .expect(202); const env = await app.services.environmentService.get(DEFAULT_ENV); @@ -328,7 +328,7 @@ test(`Importing version 2 replaces :global: environment with 'default'`, async ( const feature = await app.services.featureToggleServiceV2.getFeatureToggle( 'this-is-fun', ); - expect(feature.environments).toHaveLength(4); + expect(feature.environments).toHaveLength(1); expect(feature.environments[0].name).toBe(DEFAULT_ENV); }); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index f4573509a6..e903c11726 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -5840,6 +5840,162 @@ If the provided project does not exist, the list of events will be empty.", ], }, }, + "/api/admin/projects/{projectId}/features/{featureName}/environments/{environment}/variants": { + "get": { + "operationId": "getEnvironmentFeatureVariants", + "parameters": [ + { + "in": "path", + "name": "projectId", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "featureName", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "environment", + "required": true, + "schema": { + "type": "string", + }, + }, + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/featureVariantsSchema", + }, + }, + }, + "description": "featureVariantsSchema", + }, + }, + "tags": [ + "Features", + ], + }, + "patch": { + "operationId": "patchEnvironmentsFeatureVariants", + "parameters": [ + { + "in": "path", + "name": "projectId", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "featureName", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "environment", + "required": true, + "schema": { + "type": "string", + }, + }, + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/patchesSchema", + }, + }, + }, + "description": "patchesSchema", + "required": true, + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/featureVariantsSchema", + }, + }, + }, + "description": "featureVariantsSchema", + }, + }, + "tags": [ + "Features", + ], + }, + "put": { + "operationId": "overwriteEnvironmentFeatureVariants", + "parameters": [ + { + "in": "path", + "name": "projectId", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "featureName", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "environment", + "required": true, + "schema": { + "type": "string", + }, + }, + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/variantsSchema", + }, + }, + }, + "description": "variantsSchema", + "required": true, + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/featureVariantsSchema", + }, + }, + }, + "description": "featureVariantsSchema", + }, + }, + "tags": [ + "Features", + ], + }, + }, "/api/admin/projects/{projectId}/features/{featureName}/variants": { "get": { "operationId": "getFeatureVariants", diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index ba53d411e7..f5f0672b89 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts @@ -8,11 +8,14 @@ import User from '../../../lib/types/user'; import { IConstraint } from '../../../lib/types/model'; import { AccessService } from '../../../lib/services/access-service'; import { GroupService } from '../../../lib/services/group-service'; +import EnvironmentService from '../../../lib/services/environment-service'; let stores; let db; let service: FeatureToggleService; let segmentService: SegmentService; +let environmentService: EnvironmentService; +let unleashConfig; const mockConstraints = (): IConstraint[] => { return Array.from({ length: 5 }).map(() => ({ @@ -28,6 +31,7 @@ beforeAll(async () => { 'feature_toggle_service_v2_service_serial', config.getLogger, ); + unleashConfig = config; stores = db.stores; segmentService = new SegmentService(stores, config); const groupService = new GroupService(stores, config); @@ -206,3 +210,46 @@ test('should not get empty rows as features', async () => { expect(features.length).toBe(7); expect(namelessFeature).toBeUndefined(); }); + +test('adding and removing an environment preserves variants when variants per env is off', async () => { + const featureName = 'something-that-has-variants'; + const prodEnv = 'mock-prod-env'; + + await stores.environmentStore.create({ + name: prodEnv, + type: 'production', + }); + + await service.createFeatureToggle( + 'default', + { + name: featureName, + description: 'Second toggle', + variants: [ + { + name: 'variant1', + weight: 100, + weightType: 'fix', + stickiness: 'default', + }, + ], + }, + 'random_user', + ); + + //force the variantEnvironments flag off so that we can test legacy behavior + environmentService = new EnvironmentService(stores, { + ...unleashConfig, + flagResolver: { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + isEnabled: (toggleName: string) => false, + }, + }); + + await environmentService.addEnvironmentToProject(prodEnv, 'default'); + await environmentService.removeEnvironmentFromProject(prodEnv, 'default'); + await environmentService.addEnvironmentToProject(prodEnv, 'default'); + + const toggle = await service.getFeature(featureName, false, null, false); + expect(toggle.variants).toHaveLength(1); +}); diff --git a/src/test/e2e/services/playground-service.test.ts b/src/test/e2e/services/playground-service.test.ts index 2c66fbf7ac..56833f76b3 100644 --- a/src/test/e2e/services/playground-service.test.ts +++ b/src/test/e2e/services/playground-service.test.ts @@ -93,22 +93,6 @@ export const seedDatabaseForPlaygroundTest = async ( return Promise.all( features.map(async (feature) => { - // create feature - const toggle = await database.stores.featureToggleStore.create( - feature.project, - { - ...feature, - createdAt: undefined, - variants: [ - ...(feature.variants ?? []).map((variant) => ({ - ...variant, - weightType: WeightType.VARIABLE, - stickiness: 'default', - })), - ], - }, - ); - // create environment if necessary await database.stores.environmentStore .create({ @@ -122,6 +106,35 @@ export const seedDatabaseForPlaygroundTest = async ( // of things, this is the easiest way to make it work. }); + // create feature + const toggle = await database.stores.featureToggleStore.create( + feature.project, + { + ...feature, + createdAt: undefined, + variants: null, + }, + ); + + // enable/disable the feature in environment + await database.stores.featureEnvironmentStore.addEnvironmentToFeature( + feature.name, + environment, + feature.enabled, + ); + + await database.stores.featureToggleStore.saveVariants( + feature.project, + feature.name, + [ + ...(feature.variants ?? []).map((variant) => ({ + ...variant, + weightType: WeightType.VARIABLE, + stickiness: 'default', + })), + ], + ); + // assign strategies await Promise.all( (feature.strategies || []).map( @@ -152,13 +165,6 @@ export const seedDatabaseForPlaygroundTest = async ( ), ); - // enable/disable the feature in environment - await database.stores.featureEnvironmentStore.addEnvironmentToFeature( - feature.name, - environment, - feature.enabled, - ); - return toggle; }), ); diff --git a/src/test/e2e/services/state-service.e2e.test.ts b/src/test/e2e/services/state-service.e2e.test.ts new file mode 100644 index 0000000000..0af7370356 --- /dev/null +++ b/src/test/e2e/services/state-service.e2e.test.ts @@ -0,0 +1,164 @@ +import { createTestConfig } from '../../config/test-config'; +import dbInit from '../helpers/database-init'; +import StateService from '../../../lib/services/state-service'; +import oldFormat from '../../examples/variantsexport_v3.json'; +import { WeightType } from '../../../lib/types/model'; + +let stores; +let db; +let stateService: StateService; + +beforeAll(async () => { + const config = createTestConfig(); + db = await dbInit('state_service_serial', config.getLogger); + stores = db.stores; + stateService = new StateService(stores, config); +}); + +afterAll(async () => { + db.destroy(); +}); +test('Exporting featureEnvironmentVariants should work', async () => { + await stores.projectStore.create({ + id: 'fancy', + name: 'extra', + description: 'No surprises here', + }); + await stores.environmentStore.create({ + name: 'dev', + type: 'development', + }); + await stores.environmentStore.create({ + name: 'prod', + type: 'production', + }); + await stores.featureToggleStore.create('fancy', { + name: 'Some-feature', + }); + await stores.featureToggleStore.create('fancy', { + name: 'another-feature', + }); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'Some-feature', + 'dev', + true, + ); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'another-feature', + 'dev', + true, + ); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'another-feature', + 'prod', + true, + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'Some-feature', + 'dev', + [ + { + name: 'blue', + weight: 333, + stickiness: 'default', + weightType: WeightType.VARIABLE, + }, + { + name: 'green', + weight: 333, + stickiness: 'default', + weightType: WeightType.VARIABLE, + }, + { + name: 'red', + weight: 333, + stickiness: 'default', + weightType: WeightType.VARIABLE, + }, + ], + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'another-feature', + 'dev', + [ + { + name: 'purple', + weight: 333, + stickiness: 'default', + weightType: '', + }, + { + name: 'lilac', + weight: 333, + stickiness: 'default', + weightType: '', + }, + { + name: 'azure', + weight: 333, + stickiness: 'default', + weightType: '', + }, + ], + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'another-feature', + 'prod', + [ + { + name: 'purple', + weight: 333, + stickiness: 'default', + weightType: '', + }, + { + name: 'lilac', + weight: 333, + stickiness: 'default', + weightType: '', + }, + { + name: 'azure', + weight: 333, + stickiness: 'default', + weightType: '', + }, + ], + ); + const exportedData = await stateService.export({}); + expect( + exportedData.featureEnvironments.find( + (fE) => fE.featureName === 'Some-feature', + ).variants, + ).toHaveLength(3); +}); + +test('Should import variants from old format and convert to new format (per environment)', async () => { + await stateService.import({ + data: oldFormat, + keepExisting: false, + dropBeforeImport: true, + }); + let featureEnvironments = await stores.featureEnvironmentStore.getAll(); + expect(featureEnvironments).toHaveLength(6); // There are 3 environments enabled and 2 features + expect( + featureEnvironments + .filter((fE) => fE.featureName === 'variants-tester' && fE.enabled) + .every((e) => e.variants.length === 4), + ).toBeTruthy(); +}); +test('Should import variants in new format (per environment)', async () => { + await stateService.import({ + data: oldFormat, + keepExisting: false, + dropBeforeImport: true, + }); + let exportedJson = await stateService.export({}); + await stateService.import({ + data: exportedJson, + keepExisting: false, + dropBeforeImport: true, + }); + let featureEnvironments = await stores.featureEnvironmentStore.getAll(); + expect(featureEnvironments).toHaveLength(6); // 3 environments, 2 features === 6 rows +}); diff --git a/src/test/examples/variantsexport_v3.json b/src/test/examples/variantsexport_v3.json new file mode 100644 index 0000000000..f3510236e8 --- /dev/null +++ b/src/test/examples/variantsexport_v3.json @@ -0,0 +1,245 @@ +{ + "version": 3, + "features": [ + { + "name": "UX-toggle1", + "description": "Toggle to make UX great", + "type": "release", + "project": "default", + "stale": false, + "variants": [], + "createdAt": "2022-08-19T11:12:02.559Z", + "lastSeenAt": null, + "impressionData": false, + "archivedAt": null, + "archived": false + }, + { + "name": "variants-tester", + "description": "", + "type": "release", + "project": "default", + "stale": false, + "variants": [ + { + "name": "azure", + "weight": 250, + "weightType": "variable", + "stickiness": "default", + "overrides": [] + }, + { + "name": "lilac", + "weight": 250, + "weightType": "variable", + "stickiness": "default", + "overrides": [] + }, + { + "name": "maroon", + "weight": 250, + "weightType": "variable", + "stickiness": "default", + "overrides": [] + }, + { + "name": "purple", + "weight": 250, + "weightType": "variable", + "stickiness": "default", + "overrides": [] + } + ], + "createdAt": "2022-11-14T12:06:52.562Z", + "lastSeenAt": null, + "impressionData": false, + "archivedAt": null, + "archived": false + } + ], + "strategies": [ + { + "name": "gradualRolloutRandom", + "description": "Randomly activate the feature toggle. No stickiness.", + "parameters": [ + { + "name": "percentage", + "type": "percentage", + "description": "", + "required": false + } + ], + "deprecated": true + }, + { + "name": "gradualRolloutSessionId", + "description": "Gradually activate feature toggle. Stickiness based on session id.", + "parameters": [ + { + "name": "percentage", + "type": "percentage", + "description": "", + "required": false + }, + { + "name": "groupId", + "type": "string", + "description": "Used to define a activation groups, which allows you to correlate across feature toggles.", + "required": true + } + ], + "deprecated": true + }, + { + "name": "gradualRolloutUserId", + "description": "Gradually activate feature toggle for logged in users. Stickiness based on user id.", + "parameters": [ + { + "name": "percentage", + "type": "percentage", + "description": "", + "required": false + }, + { + "name": "groupId", + "type": "string", + "description": "Used to define a activation groups, which allows you to correlate across feature toggles.", + "required": true + } + ], + "deprecated": true + } + ], + "projects": [ + { + "id": "default", + "name": "Default", + "description": "Default project", + "createdAt": "2022-06-15T09:04:10.979Z", + "health": 100, + "updatedAt": "2022-11-14T12:05:01.328Z" + } + ], + "tagTypes": [ + { + "name": "simple", + "description": "Used to simplify filtering of features", + "icon": "#" + } + ], + "tags": [], + "featureTags": [], + "featureStrategies": [ + { + "id": "6fe19ea2-c00e-41f4-a4b0-407dd87837c3", + "featureName": "UX-toggle1", + "projectId": "default", + "environment": "development", + "strategyName": "default", + "parameters": {}, + "constraints": [], + "createdAt": "2022-08-19T11:12:32.398Z" + }, + { + "id": "f1834062-484f-4211-a39b-41f229c12a01", + "featureName": "UX-toggle1", + "projectId": "default", + "environment": "production", + "strategyName": "flexibleRollout", + "parameters": { + "groupId": "UX-toggle1", + "rollout": "50", + "stickiness": "default" + }, + "constraints": [], + "createdAt": "2022-08-19T11:12:45.779Z" + }, + { + "id": "ff954a04-1155-4420-ba83-1ded384f137c", + "featureName": "UX-toggle1", + "projectId": "default", + "environment": "development", + "strategyName": "userWithId", + "parameters": { + "userIds": "" + }, + "constraints": [], + "createdAt": "2022-08-19T11:12:55.382Z" + }, + { + "id": "6a0f8da5-398f-4d26-9089-5a87e39dbca6", + "featureName": "variants-tester", + "projectId": "default", + "environment": "development", + "strategyName": "flexibleRollout", + "parameters": { + "groupId": "variants-tester", + "rollout": "100", + "stickiness": "default" + }, + "constraints": [], + "createdAt": "2022-11-14T12:07:37.873Z" + }, + { + "id": "5c35bdd1-e30b-491f-b3ed-b1c7e08c5abc", + "featureName": "variants-tester", + "projectId": "default", + "environment": "production", + "strategyName": "flexibleRollout", + "parameters": { + "groupId": "variants-tester", + "rollout": "100", + "stickiness": "default" + }, + "constraints": [], + "createdAt": "2022-11-14T12:07:38.227Z" + } + ], + "environments": [ + { + "name": "default", + "type": "development", + "sortOrder": 100, + "enabled": true, + "protected": true + }, + { + "name": "development", + "type": "development", + "sortOrder": 100, + "enabled": true, + "protected": false + }, + { + "name": "production", + "type": "production", + "sortOrder": 200, + "enabled": true, + "protected": false + } + ], + "featureEnvironments": [ + { + "enabled": true, + "featureName": "UX-toggle1", + "environment": "development" + }, + { + "enabled": true, + "featureName": "variants-tester", + "environment": "production" + }, + { + "enabled": true, + "featureName": "variants-tester", + "environment": "development" + }, + { + "enabled": true, + "featureName": "UX-toggle1", + "environment": "production" + } + ], + "segments": [], + "featureStrategySegments": [] +} diff --git a/src/test/fixtures/fake-feature-environment-store.ts b/src/test/fixtures/fake-feature-environment-store.ts index 9b8be77f30..23c782f5ea 100644 --- a/src/test/fixtures/fake-feature-environment-store.ts +++ b/src/test/fixtures/fake-feature-environment-store.ts @@ -2,7 +2,7 @@ import { FeatureEnvironmentKey, IFeatureEnvironmentStore, } from '../../lib/types/stores/feature-environment-store'; -import { IFeatureEnvironment } from '../../lib/types/model'; +import { IFeatureEnvironment, IVariant } from '../../lib/types/model'; import NotFoundError from '../../lib/error/notfound-error'; export default class FakeFeatureEnvironmentStore @@ -18,6 +18,20 @@ export default class FakeFeatureEnvironmentStore this.featureEnvironments.push({ environment, enabled, featureName }); } + async addVariantsToFeatureEnvironment( + featureName: string, + environment: string, + variants: IVariant[], + ): Promise { + this.featureEnvironments + .filter( + (fe) => + fe.featureName === featureName && + fe.environment === environment, + ) + .map((fe) => (fe.variants = variants)); + } + async delete(key: FeatureEnvironmentKey): Promise { this.featureEnvironments.splice( this.featureEnvironments.findIndex( @@ -181,4 +195,27 @@ export default class FakeFeatureEnvironmentStore ): Promise { throw new Error('Method not implemented.'); } + + async addFeatureEnvironment( + featureEnvironment: IFeatureEnvironment, + ): Promise { + this.featureEnvironments.push(featureEnvironment); + return Promise.resolve(); + } + + getEnvironmentsForFeature( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + featureName: string, + ): Promise { + throw new Error('Method not implemented.'); + } + + clonePreviousVariants( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + environment: string, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + project: string, + ): Promise { + throw new Error('Method not implemented.'); + } } diff --git a/src/test/fixtures/fake-feature-strategies-store.ts b/src/test/fixtures/fake-feature-strategies-store.ts index 3d22eeac24..718ae66b2c 100644 --- a/src/test/fixtures/fake-feature-strategies-store.ts +++ b/src/test/fixtures/fake-feature-strategies-store.ts @@ -153,6 +153,13 @@ export default class FakeFeatureStrategiesStore ); } + getFeatureToggleWithVariantEnvs( + featureName: string, + archived?: boolean, + ): Promise { + return this.getFeatureToggleWithEnvs(featureName, archived); + } + async getFeatureOverview( // eslint-disable-next-line @typescript-eslint/no-unused-vars projectId: string, diff --git a/src/test/fixtures/fake-feature-toggle-store.ts b/src/test/fixtures/fake-feature-toggle-store.ts index 412269ee6a..3e2c883aec 100644 --- a/src/test/fixtures/fake-feature-toggle-store.ts +++ b/src/test/fixtures/fake-feature-toggle-store.ts @@ -3,7 +3,12 @@ import { IFeatureToggleStore, } from '../../lib/types/stores/feature-toggle-store'; import NotFoundError from '../../lib/error/notfound-error'; -import { FeatureToggle, FeatureToggleDTO, IVariant } from 'lib/types/model'; +import { + FeatureToggle, + FeatureToggleDTO, + IFeatureEnvironment, + IVariant, +} from 'lib/types/model'; export default class FakeFeatureToggleStore implements IFeatureToggleStore { features: FeatureToggle[] = []; @@ -126,6 +131,25 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { return feature.variants as IVariant[]; } + async getAllVariants(): Promise { + let features = await this.getAll(); + let variants = features.flatMap((feature) => ({ + featureName: feature.name, + environment: 'development', + variants: feature.variants, + enabled: true, + })); + return Promise.resolve(variants); + } + + getVariantsForEnv( + featureName: string, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + environment_name: string, + ): Promise { + return this.getVariants(featureName); + } + async saveVariants( project: string, featureName: string, @@ -135,4 +159,18 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { feature.variants = newVariants; return feature; } + + async saveVariantsOnEnv( + featureName: string, + environment: string, + newVariants: IVariant[], + ): Promise { + await this.saveVariants('default', featureName, newVariants); + return Promise.resolve(newVariants); + } + + dropAllVariants(): Promise { + this.features.forEach((feature) => (feature.variants = [])); + return Promise.resolve(); + } }