From bf0171518caa3f4aac142c67c6dcbab303db4e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Thu, 1 Dec 2022 12:13:49 +0100 Subject: [PATCH] task: continue to return export v3 when variants per env disabled (#2529) ## About the changes With the latest changes of variants per environment, we switched to export schema v4 without having the feature toggle enabled. This moves the variants to `featureEnvironments` when they were previously in `features`. The main problem is that it can create confusion as the exported file has the same variants for each one of the environments but after importing the file the UI will only show one set of variants attached to the feature. With this change, we're maintaining the previous schema until the feature toggle is enabled. --- src/lib/services/state-service.test.ts | 140 ++++++++++++++++++++++- src/lib/services/state-service.ts | 67 ++++++++++- src/test/e2e/api/admin/state.e2e.test.ts | 34 +++++- 3 files changed, 236 insertions(+), 5 deletions(-) diff --git a/src/lib/services/state-service.test.ts b/src/lib/services/state-service.test.ts index 8cba518de41..4232fb06b19 100644 --- a/src/lib/services/state-service.test.ts +++ b/src/lib/services/state-service.test.ts @@ -18,7 +18,95 @@ const oldExportExample = require('./state-service-export-v1.json'); function getSetup() { const stores = createStores(); return { - stateService: new StateService(stores, { getLogger }), + stateService: new StateService(stores, { + getLogger, + flagResolver: { isEnabled: () => true, getAll: () => ({}) }, + }), + stores, + }; +} + +async function setupV3VariantsCompatibilityScenario( + variantsPerEnvironment: boolean, +) { + const stores = createStores(); + await stores.environmentStore.create({ + name: 'env-1', + type: 'production', + sortOrder: 3, + }); + await stores.environmentStore.create({ + name: 'env-2', + type: 'production', + sortOrder: 1, + }); + await stores.environmentStore.create({ + name: 'env-3', + type: 'production', + sortOrder: 2, + }); + await stores.featureToggleStore.create('some-project', { + name: 'Feature-with-variants', + }); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'Feature-with-variants', + 'env-1', + true, + ); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'Feature-with-variants', + 'env-2', + true, + ); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'Feature-with-variants', + 'env-3', + true, + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'Feature-with-variants', + 'env-1', + [ + { + name: 'env-1-variant', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ], + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'Feature-with-variants', + 'env-2', + [ + { + name: 'env-2-variant', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ], + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'Feature-with-variants', + 'env-3', + [ + { + name: 'env-3-variant', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ], + ); + return { + stateService: new StateService(stores, { + getLogger, + flagResolver: { + isEnabled: () => variantsPerEnvironment, + getAll: () => ({}), + }, + }), stores, }; } @@ -525,7 +613,11 @@ test('Should export projects', async () => { }); test('exporting to new format works', async () => { - const { stateService, stores } = getSetup(); + const stores = createStores(); + const stateService = new StateService(stores, { + getLogger, + flagResolver: { isEnabled: () => true, getAll: () => ({}) }, + }); await stores.projectStore.create({ id: 'fancy', name: 'extra', @@ -564,6 +656,50 @@ test('exporting to new format works', async () => { expect(exported.featureStrategies).toHaveLength(1); }); +test('exporting with no environments should fail', async () => { + const { stateService, stores } = await setupV3VariantsCompatibilityScenario( + false, + ); + await stores.environmentStore.deleteAll(); + + expect(stateService.export({})).rejects.toThrowError(); +}); + +// This test should be removed as soon as variants per environment is GA +test('exporting variants to v3 format should pick variants from the correct env', async () => { + const { stateService } = await setupV3VariantsCompatibilityScenario(false); + + const exported = await stateService.export({}); + expect(exported.features).toHaveLength(1); + // env 2 has the lowest sortOrder so we expect env-2-variant to be selected + expect(exported.features[0].variants).toStrictEqual([ + { + name: 'env-2-variant', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ]); + exported.featureEnvironments.forEach((fe) => + expect(fe.variants).toBeUndefined(), + ); + expect(exported.environments).toHaveLength(3); +}); + +test('exporting variants to v4 format should not include variants in features', async () => { + const { stateService } = await setupV3VariantsCompatibilityScenario(true); + const exported = await stateService.export({}); + + expect(exported.features).toHaveLength(1); + expect(exported.features[0].variants).toBeUndefined(); + + exported.featureEnvironments.forEach((fe) => { + expect(fe.variants).toHaveLength(1); + expect(fe.variants[0].name).toBe(`${fe.environment}-variant`); + }); + expect(exported.environments).toHaveLength(3); +}); + test('featureStrategies can keep existing', async () => { const { stateService, stores } = getSetup(); await stores.projectStore.create({ diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index aac185bd56c..73ef43b6e5f 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -51,6 +51,7 @@ import { GLOBAL_ENV } from '../types/environment'; import { ISegmentStore } from '../types/stores/segment-store'; import { PartialSome } from '../types/partial'; import { IApiTokenStore } from 'lib/types/stores/api-token-store'; +import { IFlagResolver } from 'lib/types'; export interface IBackupOption { includeFeatureToggles: boolean; @@ -95,9 +96,14 @@ export default class StateService { private apiTokenStore: IApiTokenStore; + private flagResolver: IFlagResolver; + constructor( stores: IUnleashStores, - { getLogger }: Pick, + { + getLogger, + flagResolver, + }: Pick, ) { this.eventStore = stores.eventStore; this.toggleStore = stores.featureToggleStore; @@ -111,6 +117,7 @@ export default class StateService { this.environmentStore = stores.environmentStore; this.segmentStore = stores.segmentStore; this.apiTokenStore = stores.apiTokenStore; + this.flagResolver = flagResolver; this.logger = getLogger('services/state-service.js'); } @@ -697,7 +704,63 @@ export default class StateService { ); } - async export({ + async export(opts: IExportIncludeOptions): Promise<{ + features: FeatureToggle[]; + strategies: IStrategy[]; + version: number; + projects: IProject[]; + tagTypes: ITagType[]; + tags: ITag[]; + featureTags: IFeatureTag[]; + featureStrategies: IFeatureStrategy[]; + environments: IEnvironment[]; + featureEnvironments: IFeatureEnvironment[]; + }> { + if (this.flagResolver.isEnabled('variantsPerEnvironment')) { + return this.exportV4(opts); + } + // adapt v4 to v3. We need includeEnvironments set to true to filter the + // best environment from where we'll pick variants (cause now they are stored + // per environment despite being displayed as if they belong to the feature) + const v4 = await this.exportV4({ ...opts, includeEnvironments: true }); + // undefined defaults to true + if (opts.includeFeatureToggles !== false) { + const keepEnv = v4.environments + .filter((env) => env.enabled !== false) + .sort((e1, e2) => { + if (e1.type !== 'production' || e2.type !== 'production') { + if (e1.type === 'production') { + return -1; + } else if (e2.type === 'production') { + return 1; + } + } + return e1.sortOrder - e2.sortOrder; + })[0]; + + const featureEnvs = v4.featureEnvironments.filter( + (fE) => fE.environment === keepEnv.name, + ); + v4.features = v4.features.map((f) => { + const variants = featureEnvs.find( + (fe) => fe.enabled !== false && fe.featureName === f.name, + )?.variants; + return { ...f, variants }; + }); + v4.featureEnvironments = v4.featureEnvironments.map((fe) => { + delete fe.variants; + return fe; + }); + } + // only if explicitly set to false (i.e. undefined defaults to true) + if (opts.includeEnvironments === false) { + delete v4.environments; + } + v4.version = 3; + return v4; + } + + async exportV4({ includeFeatureToggles = true, includeStrategies = true, includeProjects = true, diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index c5436e0cc05..cfd142f4c50 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -4,6 +4,8 @@ import getLogger from '../../../fixtures/no-logger'; import { DEFAULT_ENV } from '../../../../lib/util/constants'; import { collectIds } from '../../../../lib/util/collect-ids'; import { ApiTokenType } from '../../../../lib/types/models/api-token'; +import variantsv3 from '../../../examples/variantsexport_v3.json'; +import { StateService } from '../../../../lib/services'; const importData = require('../../../examples/import.json'); @@ -317,7 +319,7 @@ test('Roundtrip with strategies in multiple environments works', async () => { const f = await app.services.featureToggleServiceV2.getFeature({ featureName, }); - expect(f.environments).toHaveLength(4); + expect(f.environments).toHaveLength(4); // NOTE: this depends on other tests, otherwise it should be 2 }); test(`Importing version 2 replaces :global: environment with 'default'`, async () => { @@ -466,3 +468,33 @@ test(`should not show environment on feature toggle, when environment is disable expect(result[1].name).toBe('production'); expect(result[1].enabled).toBeFalsy(); }); + +test(`should handle v3 export with variants in features`, async () => { + app.services.stateService = new StateService(db.stores, { + getLogger, + flagResolver: { + isEnabled: () => false, + getAll: () => ({}), + }, + }); + + await app.request + .post('/api/admin/state/import?drop=true') + .attach('file', 'src/test/examples/variantsexport_v3.json') + .expect(202); + + const exported = await app.services.stateService.export({}); + let exportedFeatures = exported.features + .map((f) => { + delete f.createdAt; + return f; + }) + .sort(); + let importedFeatures = variantsv3.features + .map((f) => { + delete f.createdAt; + return f; + }) + .sort(); + expect(exportedFeatures).toStrictEqual(importedFeatures); +});