Skip to content

Commit

Permalink
task: continue to return export v3 when variants per env disabled (#2529
Browse files Browse the repository at this point in the history
)

## 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.
  • Loading branch information
Gastón Fournier committed Dec 1, 2022
1 parent ee1ce16 commit bf01715
Show file tree
Hide file tree
Showing 3 changed files with 236 additions and 5 deletions.
140 changes: 138 additions & 2 deletions src/lib/services/state-service.test.ts
Expand Up @@ -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,
};
}
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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({
Expand Down
67 changes: 65 additions & 2 deletions src/lib/services/state-service.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -95,9 +96,14 @@ export default class StateService {

private apiTokenStore: IApiTokenStore;

private flagResolver: IFlagResolver;

constructor(
stores: IUnleashStores,
{ getLogger }: Pick<IUnleashConfig, 'getLogger'>,
{
getLogger,
flagResolver,
}: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
) {
this.eventStore = stores.eventStore;
this.toggleStore = stores.featureToggleStore;
Expand All @@ -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');
}

Expand Down Expand Up @@ -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,
Expand Down
34 changes: 33 additions & 1 deletion src/test/e2e/api/admin/state.e2e.test.ts
Expand Up @@ -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');

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
});

0 comments on commit bf01715

Please sign in to comment.