From 8050f25add648bb98c58e8c16c314bf56bfdd477 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 4 Jun 2025 09:09:52 +0200 Subject: [PATCH] chore(adminapi)!: Remove feature variant endpoints (#10071) BREAKING CHANGE: This removes the GET /api/admin/projects/{project}/features/{featureName}/variants PATCH /api/admin/projects/{project}/features/{featureName}/variants PUT /api/admin/projects/{project}/features/{featureName}/variants endpoints Users should move to environment or strategy specific variant methods rather than feature level variant methods. --- .../actions/useFeatureApi/useFeatureApi.ts | 15 - .../fakes/fake-feature-toggle-store.ts | 5 - .../feature-toggle/feature-toggle-service.ts | 52 +- .../tests/feature-toggles.e2e.test.ts | 88 +- .../types/feature-toggle-store-type.ts | 21 - src/lib/routes/admin-api/project/variants.ts | 121 --- .../types/stores/feature-environment-store.ts | 1 - src/test/e2e/api/admin/playground.e2e.test.ts | 4 +- .../admin/project/variants-sunset.e2e.test.ts | 40 +- .../api/admin/project/variants.e2e.test.ts | 981 +----------------- .../api/client/feature.optimal304.e2e.test.ts | 22 +- .../e2e/services/playground-service.test.ts | 4 +- 12 files changed, 79 insertions(+), 1275 deletions(-) diff --git a/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts b/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts index 82b8d45ef9..b8500f1402 100644 --- a/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts +++ b/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts @@ -208,20 +208,6 @@ const useFeatureApi = () => { return makeRequest(req.caller, req.id); }; - const patchFeatureVariants = async ( - projectId: string, - featureId: string, - patchPayload: Operation[], - ) => { - const path = `api/admin/projects/${projectId}/features/${featureId}/variants`; - const req = createRequest(path, { - method: 'PATCH', - body: JSON.stringify(patchPayload), - }); - - return makeRequest(req.caller, req.id); - }; - const patchFeatureEnvironmentVariants = async ( projectId: string, featureId: string, @@ -279,7 +265,6 @@ const useFeatureApi = () => { updateFeatureTags, archiveFeatureToggle, patchFeatureToggle, - patchFeatureVariants, patchFeatureEnvironmentVariants, overrideVariantsInEnvironments, cloneFeatureToggle, diff --git a/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts b/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts index 19a7ee1144..64e8df88af 100644 --- a/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts @@ -231,11 +231,6 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { } } - async getVariants(featureName: string): Promise { - const feature = await this.get(featureName); - return feature.variants as IVariant[]; - } - async getAllVariants(): Promise { const features = await this.getAll(); const variants = features.flatMap((feature) => ({ diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 91a10e207f..cb48c05cbe 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -16,7 +16,6 @@ import { type FeatureToggleDTO, type FeatureToggleView, type FeatureToggleWithEnvironment, - FeatureVariantEvent, type IAuditUser, type IConstraint, type IDependency, @@ -1155,16 +1154,6 @@ export class FeatureToggleService { } } - /** - * GET /api/admin/projects/:project/features/:featureName/variants - * @deprecated - Variants should be fetched from FeatureEnvironmentStore (since variants are now; since 4.18, connected to environments) - * @param featureName - * @return The list of variants - */ - async getVariants(featureName: string): Promise { - return this.featureToggleStore.getVariants(featureName); - } - async getVariantsForEnv( featureName: string, environment: string, @@ -2255,24 +2244,31 @@ export class FeatureToggleService { ): Promise { await variantsArraySchema.validateAsync(newVariants); const fixedVariants = this.fixVariantWeights(newVariants); - const oldVariants = - await this.featureToggleStore.getVariants(featureName); - const featureToggle = await this.featureToggleStore.saveVariants( - project, - featureName, - fixedVariants, - ); - - await this.eventService.storeEvent( - new FeatureVariantEvent({ - project, + const environments = + await this.featureEnvironmentStore.getEnvironmentsForFeature( featureName, - auditUser, - oldVariants, - newVariants: featureToggle.variants as IVariant[], - }), - ); - return featureToggle; + ); + for (const env of environments) { + const oldVariants = env.variants || []; + await this.featureEnvironmentStore.setVariantsToFeatureEnvironments( + featureName, + [env.environment], + fixedVariants, + ); + await this.eventService.storeEvent( + new EnvironmentVariantEvent({ + project, + environment: env.environment, + featureName, + auditUser, + oldVariants, + newVariants: fixedVariants, + }), + ); + } + + const toggle = await this.featureToggleStore.get(featureName); + return toggle!; } private async verifyLegacyVariants(featureName: string) { diff --git a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts index e148f1afbc..3bf485b6e8 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts @@ -18,12 +18,7 @@ import { import ApiUser from '../../../types/api-user.js'; import { ApiTokenType, type IApiToken } from '../../../types/model.js'; import IncompatibleProjectError from '../../../error/incompatible-project-error.js'; -import { - type IStrategyConfig, - type IVariant, - RoleName, - WeightType, -} from '../../../types/model.js'; +import { type IStrategyConfig, RoleName } from '../../../types/model.js'; import { v4 as uuidv4 } from 'uuid'; import type supertest from 'supertest'; import { randomId } from '../../../util/random-id.js'; @@ -916,46 +911,6 @@ test('Should patch feature flag', async () => { expect(updateForOurFlag?.data.type).toBe('kill-switch'); }); -test('Should patch feature flag and not remove variants', async () => { - const url = '/api/admin/projects/default/features'; - const name = 'new.flag.variants'; - await app.request - .post(url) - .send({ name, description: 'some', type: 'release' }) - .expect(201); - await app.request - .put(`${url}/${name}/variants`) - .send([ - { - name: 'variant1', - weightType: 'variable', - weight: 500, - stickiness: 'default', - }, - { - name: 'variant2', - weightType: 'variable', - weight: 500, - stickiness: 'default', - }, - ]) - .expect(200); - await app.request - .patch(`${url}/${name}`) - .send([ - { op: 'replace', path: '/description', value: 'New desc' }, - { op: 'replace', path: '/type', value: 'kill-switch' }, - ]) - .expect(200); - - const { body: flag } = await app.request.get(`${url}/${name}`); - - expect(flag.name).toBe(name); - expect(flag.description).toBe('New desc'); - expect(flag.type).toBe('kill-switch'); - expect(flag.variants).toHaveLength(2); -}); - test('Patching feature flags to stale should trigger FEATURE_STALE_ON event', async () => { const url = '/api/admin/projects/default/features'; const name = 'flag.stale.on.patch'; @@ -2413,47 +2368,6 @@ test('Should allow changing project to target project with the same enabled envi ).resolves; }); -test(`a feature's variants should be sorted by name in increasing order`, async () => { - const featureName = 'variants.are.sorted'; - const project = 'default'; - await app.createFeature(featureName, project); - - const newVariants: IVariant[] = [ - { - name: 'z', - stickiness: 'default', - weight: 250, - weightType: WeightType.FIX, - }, - { - name: 'f', - stickiness: 'default', - weight: 375, - weightType: WeightType.VARIABLE, - }, - { - name: 'a', - stickiness: 'default', - weight: 450, - weightType: WeightType.VARIABLE, - }, - ]; - - await app.request - .put(`/api/admin/projects/${project}/features/${featureName}/variants`) - .send(newVariants) - .expect(200); - - await app.request - .get(`/api/admin/projects/${project}/features/${featureName}`) - .expect(200) - .expect((res) => { - expect(res.body.variants[0].name).toBe('a'); - expect(res.body.variants[1].name).toBe('f'); - expect(res.body.variants[2].name).toBe('z'); - }); -}); - test('should validate context when calling update with PUT', async () => { const name = 'new.flag.validate.context'; await app.request diff --git a/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts b/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts index 384f63be9b..9e96da5a09 100644 --- a/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts +++ b/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts @@ -3,7 +3,6 @@ import type { FeatureToggleDTO, IFeatureToggleQuery, IFeatureTypeCount, - IVariant, } from '../../../types/model.js'; import type { FeatureToggleInsert } from '../feature-toggle-store.js'; import type { Store } from '../../../types/stores/store.js'; @@ -76,26 +75,6 @@ export interface IFeatureToggleStore extends Store { isPotentiallyStale(featureName: string): 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, - newVariants: IVariant[], - ): Promise; - disableAllEnvironmentsForFeatures(names: string[]): Promise; getFeatureTypeCounts( diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index 6eba1e01c4..3f5f7f7ba9 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -8,10 +8,8 @@ import type { Operation } from 'fast-json-patch'; import { NONE, UPDATE_FEATURE_ENVIRONMENT_VARIANTS, - UPDATE_FEATURE_VARIANTS, } from '../../../types/permissions.js'; import { type IVariant, WeightType } from '../../../types/model.js'; -import { extractUsername } from '../../../util/extract-user.js'; import type { IAuthRequest } from '../../unleash-types.js'; import type { FeatureVariantsSchema } from '../../../openapi/spec/feature-variants-schema.js'; import { createRequestSchema } from '../../../openapi/util/create-request-schema.js'; @@ -59,75 +57,6 @@ export default class VariantsController extends Controller { this.logger = config.getLogger('admin-api/project/variants.ts'); this.featureService = featureToggleService; this.accessService = accessService; - this.route({ - method: 'get', - path: PREFIX, - permission: NONE, - handler: this.getVariants, - middleware: [ - openApiService.validPath({ - summary: 'Retrieve variants for a feature (deprecated) ', - description: - '(deprecated from 4.21) Retrieve the variants for the specified feature. From Unleash 4.21 onwards, this endpoint will attempt to choose a [production-type environment](https://docs.getunleash.io/reference/environments) as the source of truth. If more than one production environment is found, the first one will be used.', - deprecated: true, - tags: ['Features'], - operationId: 'getFeatureVariants', - responses: { - 200: createResponseSchema('featureVariantsSchema'), - ...getStandardResponses(401, 403, 404), - }, - }), - ], - }); - this.route({ - method: 'patch', - path: PREFIX, - permission: UPDATE_FEATURE_VARIANTS, - handler: this.patchVariants, - middleware: [ - openApiService.validPath({ - summary: - "Apply a patch to a feature's variants (in all environments).", - description: `Apply a list of patches patch to the specified feature's variants. The patch objects should conform to the [JSON-patch format (RFC 6902)](https://www.rfc-editor.org/rfc/rfc6902). - -⚠️ **Warning**: This method is not atomic. If something fails in the middle of applying the patch, you can be left with a half-applied patch. We recommend that you instead [patch variants on a per-environment basis](/docs/reference/api/unleash/patch-environments-feature-variants.api.mdx), which **is** an atomic operation.`, - tags: ['Features'], - operationId: 'patchFeatureVariants', - requestBody: createRequestSchema('patchesSchema'), - responses: { - 200: createResponseSchema('featureVariantsSchema'), - ...getStandardResponses(400, 401, 403, 404), - }, - }), - ], - }); - this.route({ - method: 'put', - path: PREFIX, - permission: UPDATE_FEATURE_VARIANTS, - handler: this.overwriteVariants, - middleware: [ - openApiService.validPath({ - summary: - 'Create (overwrite) variants for a feature flag in all environments', - description: `This overwrites the current variants for the feature specified in the :featureName parameter in all environments. - -The backend will validate the input for the following invariants - -* If there are variants, there needs to be at least one variant with \`weightType: variable\` -* The sum of the weights of variants with \`weightType: fix\` must be strictly less than 1000 (< 1000) - -The backend will also distribute remaining weight up to 1000 after adding the variants with \`weightType: fix\` together amongst the variants of \`weightType: variable\``, - tags: ['Features'], - operationId: 'overwriteFeatureVariants', - requestBody: createRequestSchema('variantsSchema'), - responses: { - 200: createResponseSchema('featureVariantsSchema'), - ...getStandardResponses(400, 401, 403, 404), - }, - }), - ], - }); this.route({ method: 'get', path: ENV_PREFIX, @@ -215,56 +144,6 @@ The backend will also distribute remaining weight up to 1000 after adding the va }); } - /** - * @deprecated - Variants should be fetched from featureService.getVariantsForEnv (since variants are now; since 4.18, connected to environments) - * @param req - * @param res - */ - async getVariants( - req: Request, - res: Response, - ): Promise { - const { featureName } = req.params; - const variants = await this.featureService.getVariants(featureName); - res.status(200).json({ version: 1, variants: variants || [] }); - } - - async patchVariants( - req: IAuthRequest, - res: Response, - ): Promise { - const { projectId, featureName } = req.params; - const updatedFeature = await this.featureService.updateVariants( - featureName, - projectId, - req.body, - req.user, - req.audit, - ); - res.status(200).json({ - version: 1, - variants: updatedFeature.variants || [], - }); - } - - async overwriteVariants( - req: IAuthRequest, - res: Response, - ): Promise { - const { projectId, featureName } = req.params; - const userName = extractUsername(req); - const updatedFeature = await this.featureService.saveVariants( - featureName, - projectId, - req.body, - req.audit, - ); - res.status(200).json({ - version: 1, - variants: updatedFeature.variants || [], - }); - } - async pushVariantsToEnvironments( req: IAuthRequest< FeatureEnvironmentParams, diff --git a/src/lib/types/stores/feature-environment-store.ts b/src/lib/types/stores/feature-environment-store.ts index 6ed431a92f..4880446145 100644 --- a/src/lib/types/stores/feature-environment-store.ts +++ b/src/lib/types/stores/feature-environment-store.ts @@ -80,7 +80,6 @@ export interface IFeatureEnvironmentStore environments: string[], variants: IVariant[], ): Promise; - addFeatureEnvironment( featureEnvironment: IFeatureEnvironment, ): Promise; diff --git a/src/test/e2e/api/admin/playground.e2e.test.ts b/src/test/e2e/api/admin/playground.e2e.test.ts index 5ece07e920..e2de166a61 100644 --- a/src/test/e2e/api/admin/playground.e2e.test.ts +++ b/src/test/e2e/api/admin/playground.e2e.test.ts @@ -107,9 +107,9 @@ describe('Playground API E2E', () => { feature.enabled, ); - await database.stores.featureToggleStore.saveVariants( - feature.project!, + await database.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( feature.name, + environment, [ ...(feature.variants ?? []).map((variant) => ({ ...variant, diff --git a/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts index 4aa3d906d9..eed3a5c28b 100644 --- a/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts @@ -84,14 +84,18 @@ test('Can add environment variants when existing ones exist for this feature', a 'development', true, ); - await db.stores.featureToggleStore.saveVariants('default', featureName, [ - { - name: 'existing-variant', - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'default', + [ + { + name: 'existing-variant', + stickiness: 'default', + weight: 1000, + weightType: WeightType.VARIABLE, + }, + ], + ); const patch = [ { @@ -125,14 +129,18 @@ test('Patching variants with an invalid patch payload should return a BadDataErr 'development', true, ); - await db.stores.featureToggleStore.saveVariants('default', featureName, [ - { - name: 'existing-variant', - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'development', + [ + { + name: 'existing-variant', + stickiness: 'default', + weight: 1000, + weightType: WeightType.VARIABLE, + }, + ], + ); const patch = [ { 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 aee0718363..ed71cda1ff 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -43,16 +43,22 @@ test('Can get variants for a feature', async () => { 'default', true, ); - await db.stores.featureToggleStore.saveVariants('default', featureName, [ - { - name: variantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'default', + [ + { + name: variantName, + stickiness: 'default', + weight: 1000, + weightType: WeightType.VARIABLE, + }, + ], + ); await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) + .get( + `/api/admin/projects/default/features/${featureName}/environments/default/variants`, + ) .expect(200) .expect((res) => { expect(res.body.version).toBe(1); @@ -95,140 +101,6 @@ test('Trying to do operations on a non-existing feature yields 404', async () => .expect(404); }); -test('Can patch variants for a feature and get a response of new variant', async () => { - const featureName = 'feature-variants-patch'; - const variantName = 'fancy-variant-patch'; - const expectedVariantName = 'not-so-cool-variant-name'; - const variants = [ - { - name: variantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]; - - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - await db.stores.featureToggleStore.saveVariants( - 'default', - featureName, - variants, - ); - - const observer = jsonpatch.observe(variants); - variants[0].name = expectedVariantName; - const patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.version).toBe(1); - expect(res.body.variants).toHaveLength(1); - expect(res.body.variants[0].name).toBe(expectedVariantName); - }); -}); - -test('Can patch variants for a feature patches all environments independently', async () => { - const featureName = 'feature-to-patch'; - const addedVariantName = 'patched-variant-name'; - const variants = (name: string) => [ - { - name, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]; - - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'development', - true, - ); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'production', - true, - ); - await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( - featureName, - 'development', - variants('dev-variant'), - ); - await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( - featureName, - 'production', - variants('prod-variant'), - ); - - const patch = [ - { - op: 'add', - path: '/1', - value: { - name: addedVariantName, - weightType: WeightType.FIX, - weight: 50, - }, - }, - ]; - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.version).toBe(1); - expect(res.body.variants).toHaveLength(2); - // it picks variants from the first environment (sorted by name) - expect(res.body.variants[0].name).toBe('dev-variant'); - expect(res.body.variants[1].name).toBe(addedVariantName); - }); - - await app.request - .get( - `/api/admin/projects/default/features/${featureName}?variantEnvironments=true`, - ) - .expect((res) => { - const environments = res.body.environments; - expect(environments).toHaveLength(2); - const developmentVariants = environments.find( - (x) => x.name === 'development', - ).variants; - const productionVariants = environments.find( - (x) => x.name === 'production', - ).variants; - expect(developmentVariants).toHaveLength(2); - expect(productionVariants).toHaveLength(2); - expect( - developmentVariants.find((x) => x.name === addedVariantName), - ).toBeTruthy(); - expect( - productionVariants.find((x) => x.name === addedVariantName), - ).toBeTruthy(); - expect( - developmentVariants.find((x) => x.name === 'dev-variant'), - ).toBeTruthy(); - expect( - productionVariants.find((x) => x.name === 'prod-variant'), - ).toBeTruthy(); - }); -}); - test('Can push variants to multiple environments', async () => { const featureName = 'feature-to-override'; const variant = (name: string, weight: number) => ({ @@ -326,826 +198,3 @@ test("Returns proper error if project and/or feature flag doesn't exist", async }) .expect(404); }); - -test('Can add variant for a feature', async () => { - const featureName = 'feature-variants-patch-add'; - const variantName = 'fancy-variant-patch'; - const expectedVariantName = 'not-so-cool-variant-name'; - const variants = [ - { - name: variantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]; - - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - await db.stores.featureToggleStore.saveVariants( - 'default', - featureName, - variants, - ); - - const observer = jsonpatch.observe(variants); - variants.push({ - name: expectedVariantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }); - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200); - - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect((res) => { - expect(res.body.version).toBe(1); - expect(res.body.variants).toHaveLength(2); - expect( - res.body.variants.find((x) => x.name === expectedVariantName), - ).toBeTruthy(); - expect( - res.body.variants.find((x) => x.name === variantName), - ).toBeTruthy(); - }); -}); - -test('Can remove variant for a feature', async () => { - const featureName = 'feature-variants-patch-remove'; - const variantName = 'fancy-variant-patch'; - const variants = [ - { - name: variantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]; - - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - await db.stores.featureToggleStore.saveVariants( - 'default', - featureName, - variants, - ); - - const observer = jsonpatch.observe(variants); - variants.pop(); - const patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200); - - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect((res) => { - expect(res.body.version).toBe(1); - expect(res.body.variants).toHaveLength(0); - }); -}); - -test('PUT overwrites current variant on feature', async () => { - const featureName = 'variant-put-overwrites'; - const variantName = 'overwriting-for-fun'; - const variants = [ - { - name: variantName, - stickiness: 'default', - weight: 1000, - weightType: WeightType.VARIABLE, - }, - ]; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - await db.stores.featureToggleStore.saveVariants( - 'default', - featureName, - variants, - ); - - const newVariants: IVariant[] = [ - { - name: 'variant1', - stickiness: 'default', - weight: 250, - weightType: WeightType.FIX, - }, - { - name: 'variant2', - stickiness: 'default', - weight: 375, - weightType: WeightType.VARIABLE, - }, - { - name: 'variant3', - stickiness: 'default', - weight: 450, - weightType: WeightType.VARIABLE, - }, - ]; - await app.request - .put(`/api/admin/projects/default/features/${featureName}/variants`) - .send(newVariants) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(3); - }); - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(3); - expect(res.body.variants.reduce((a, v) => a + v.weight, 0)).toEqual( - 1000, - ); - }); -}); - -test('PUTing an invalid variant throws 400 exception', async () => { - const featureName = 'variants-validation-feature'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - const invalidJson = [ - { - name: 'variant', - weight: 500, - weightType: 'party', - stickiness: 'userId', - }, - ]; - await app.request - .put(`/api/admin/projects/default/features/${featureName}/variants`) - .send(invalidJson) - .expect(400) - .expect((res) => { - expect(res.body.details).toHaveLength(1); - expect(res.body.details[0].message).toMatch( - /.*weightType` property must be equal to one of the allowed values/, - ); - }); -}); - -test('Invalid variant in PATCH also throws 400 exception', async () => { - const featureName = 'patch-validation-feature'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const invalidPatch = `[{ - "op": "add", - "path": "/1", - "value": { - "name": "not-so-cool-variant-name", - "stickiness": "default", - "weight": 2000, - "weightType": "variable" - } - }]`; - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .set('Content-Type', 'application/json') - .send(invalidPatch) - .expect(400) - .expect((res) => { - expect(res.body.details).toHaveLength(1); - expect(res.body.details[0].message).toMatch( - /.*weight" must be less than or equal to 1000/, - ); - }); -}); - -test('PATCHING with all variable weightTypes forces weights to sum to no less than 1000 minus the number of variable variants', async () => { - const featureName = 'variants-validation-with-all-variable-weights'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 700, - weightType: WeightType.VARIABLE, - }); - let patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(1); - expect(res.body.variants[0].weight).toEqual(1000); - }); - - newVariants.push({ - name: 'variant2', - stickiness: 'default', - weight: 700, - weightType: WeightType.VARIABLE, - }); - - patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(2); - expect( - res.body.variants.every((x) => x.weight === 500), - ).toBeTruthy(); - }); - - newVariants.push({ - name: 'variant3', - stickiness: 'default', - weight: 700, - weightType: WeightType.VARIABLE, - }); - - patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - res.body.variants.sort((v, other) => other.weight - v.weight); - expect(res.body.variants).toHaveLength(3); - expect(res.body.variants[0].weight).toBe(334); - expect(res.body.variants[1].weight).toBe(333); - expect(res.body.variants[2].weight).toBe(333); - }); - - newVariants.push({ - name: 'variant4', - stickiness: 'default', - weight: 700, - weightType: WeightType.VARIABLE, - }); - - patch = jsonpatch.generate(observer); - - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(4); - expect( - res.body.variants.every((x) => x.weight === 250), - ).toBeTruthy(); - }); -}); - -test('PATCHING with no variable variants fails with 400', async () => { - const featureName = 'variants-validation-with-no-variable-weights'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 900, - weightType: WeightType.FIX, - }); - - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(400) - .expect((res) => { - expect(res.body.details).toHaveLength(1); - expect(res.body.details[0].message).toEqual( - 'There must be at least one "variable" variant', - ); - }); -}); - -test('Patching with a fixed variant and variable variants splits remaining weight among variable variants', async () => { - const featureName = 'variants-fixed-and-variable'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 900, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant2', - stickiness: 'default', - weight: 20, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant3', - stickiness: 'default', - weight: 123, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant4', - stickiness: 'default', - weight: 123, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant5', - stickiness: 'default', - weight: 123, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant6', - stickiness: 'default', - weight: 123, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant7', - stickiness: 'default', - weight: 123, - weightType: WeightType.VARIABLE, - }); - - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200); - - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect(200) - .expect((res) => { - const body = res.body; - expect(body.variants).toHaveLength(7); - expect( - body.variants.reduce((total, v) => total + v.weight, 0), - ).toEqual(1000); - body.variants.sort((a, b) => b.weight - a.weight); - expect( - body.variants.find((v) => v.name === 'variant1').weight, - ).toEqual(900); - expect( - body.variants.find((v) => v.name === 'variant2').weight, - ).toEqual(17); - expect( - body.variants.find((v) => v.name === 'variant3').weight, - ).toEqual(17); - expect( - body.variants.find((v) => v.name === 'variant4').weight, - ).toEqual(17); - expect( - body.variants.find((v) => v.name === 'variant5').weight, - ).toEqual(17); - expect( - body.variants.find((v) => v.name === 'variant6').weight, - ).toEqual(16); - expect( - body.variants.find((v) => v.name === 'variant7').weight, - ).toEqual(16); - }); -}); - -test('Multiple fixed variants gets added together to decide how much weight variable variants should get', async () => { - const featureName = 'variants-multiple-fixed-and-variable'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 600, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant2', - stickiness: 'default', - weight: 350, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant3', - stickiness: 'default', - weight: 350, - weightType: WeightType.VARIABLE, - }); - - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200); - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect(200) - .expect((res) => { - const body = res.body; - expect(body.variants).toHaveLength(3); - expect( - body.variants.find((v) => v.name === 'variant3').weight, - ).toEqual(50); - }); -}); - -test('If sum of fixed variant weight exceed 1000 fails with 400', async () => { - const featureName = 'variants-fixed-weight-over-1000'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 900, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant2', - stickiness: 'default', - weight: 900, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant3', - stickiness: 'default', - weight: 350, - weightType: WeightType.VARIABLE, - }); - - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(400) - .expect((res) => { - expect(res.body.details).toHaveLength(1); - expect(res.body.details[0].message).toEqual( - 'The traffic distribution total must equal 100%', - ); - }); -}); - -test('If sum of fixed variant weight equals 1000 variable variants gets weight 0', async () => { - const featureName = 'variants-fixed-weight-equals-1000-no-variable-weight'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - stickiness: 'default', - weight: 900, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant2', - stickiness: 'default', - weight: 100, - weightType: WeightType.FIX, - }); - newVariants.push({ - name: 'variant3', - stickiness: 'default', - weight: 350, - weightType: WeightType.VARIABLE, - }); - newVariants.push({ - name: 'variant4', - stickiness: 'default', - weight: 350, - weightType: WeightType.VARIABLE, - }); - - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200); - await app.request - .get(`/api/admin/projects/default/features/${featureName}/variants`) - .expect(200) - .expect((res) => { - const body = res.body; - expect(body.variants).toHaveLength(4); - expect( - body.variants.find((v) => v.name === 'variant3').weight, - ).toEqual(0); - expect( - body.variants.find((v) => v.name === 'variant4').weight, - ).toEqual(0); - }); -}); - -test('PATCH endpoint validates uniqueness of variant names', async () => { - const featureName = 'variants-uniqueness-names'; - const variants = [ - { - name: 'variant1', - weight: 1000, - weightType: WeightType.VARIABLE, - stickiness: 'default', - }, - ]; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - await db.stores.featureToggleStore.saveVariants( - 'default', - featureName, - variants, - ); - - const newVariants: IVariant[] = []; - - const observer = jsonpatch.observe(newVariants); - newVariants.push({ - name: 'variant1', - weight: 550, - weightType: WeightType.VARIABLE, - stickiness: 'default', - }); - newVariants.push({ - name: 'variant2', - weight: 550, - weightType: WeightType.VARIABLE, - stickiness: 'default', - }); - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(400) - .expect((res) => { - expect(res.body.details[0].message).toMatch( - /contains a duplicate value/, - ); - }); -}); - -test('PUT endpoint validates uniqueness of variant names', async () => { - const featureName = 'variants-put-uniqueness-names'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - await app.request - .put(`/api/admin/projects/default/features/${featureName}/variants`) - .send([ - { - name: 'variant1', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - { - name: 'variant1', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - ]) - .expect(400) - .expect((res) => { - expect(res.body.details[0].message).toMatch( - /contains a duplicate value/, - ); - }); -}); - -test('Variants should be sorted by their name when PUT', async () => { - const featureName = 'variants-sort-by-name'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - await app.request - .put(`/api/admin/projects/default/features/${featureName}/variants`) - .send([ - { - name: 'zvariant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - { - name: 'variant-a', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - { - name: 'g-variant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - { - name: 'variant-g', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }, - ]) - .expect(200) - .expect((res) => { - expect(res.body.variants[0].name).toBe('g-variant'); - expect(res.body.variants[1].name).toBe('variant-a'); - expect(res.body.variants[2].name).toBe('variant-g'); - expect(res.body.variants[3].name).toBe('zvariant'); - }); -}); - -test('Variants should be sorted by name when PATCHed as well', async () => { - const featureName = 'variants-patch-sort-by-name'; - await db.stores.featureToggleStore.create('default', { - name: featureName, - createdByUserId: 9999, - }); - - await db.stores.featureEnvironmentStore.addEnvironmentToFeature( - featureName, - 'default', - true, - ); - - const variants: IVariant[] = []; - const observer = jsonpatch.observe(variants); - variants.push({ - name: 'g-variant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }); - variants.push({ - name: 'a-variant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }); - const patch = jsonpatch.generate(observer); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(patch) - .expect(200) - .expect((res) => { - expect(res.body.variants[0].name).toBe('a-variant'); - expect(res.body.variants[1].name).toBe('g-variant'); - }); - variants.push({ - name: '00-variant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }); - variants.push({ - name: 'z-variant', - weightType: WeightType.VARIABLE, - weight: 500, - stickiness: 'default', - }); - const secondPatch = jsonpatch.generate(observer); - expect(secondPatch).toHaveLength(2); - await app.request - .patch(`/api/admin/projects/default/features/${featureName}/variants`) - .send(secondPatch) - .expect(200) - .expect((res) => { - expect(res.body.variants).toHaveLength(4); - expect(res.body.variants[0].name).toBe('00-variant'); - expect(res.body.variants[1].name).toBe('a-variant'); - expect(res.body.variants[2].name).toBe('g-variant'); - expect(res.body.variants[3].name).toBe('z-variant'); - }); -}); diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts index f852106ff1..953c66ad8a 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -152,26 +152,26 @@ describe.each([ .expect(200); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:16:${etagVariant.name}"`, + `"61824cd0:17:${etagVariant.name}"`, ); } else { - expect(res.headers.etag).toBe('"61824cd0:16"'); - expect(res.body.meta.etag).toBe('"61824cd0:16"'); + expect(res.headers.etag).toBe('"61824cd0:17"'); + expect(res.body.meta.etag).toBe('"61824cd0:17"'); } }); test(`returns ${etagVariant.feature_enabled ? 200 : 304} for pre-calculated hash${etagVariant.feature_enabled ? ' because hash changed' : ''}`, async () => { const res = await app.request .get('/api/client/features') - .set('if-none-match', '"61824cd0:16"') + .set('if-none-match', '"61824cd0:17"') .expect(etagVariant.feature_enabled ? 200 : 304); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:16:${etagVariant.name}"`, + `"61824cd0:17:${etagVariant.name}"`, ); } }); @@ -193,13 +193,13 @@ describe.each([ .expect(200); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:16:${etagVariant.name}"`, + `"61824cd0:17:${etagVariant.name}"`, ); } else { - expect(res.headers.etag).toBe('"61824cd0:16"'); - expect(res.body.meta.etag).toBe('"61824cd0:16"'); + expect(res.headers.etag).toBe('"61824cd0:17"'); + expect(res.body.meta.etag).toBe('"61824cd0:17"'); } }); }); diff --git a/src/test/e2e/services/playground-service.test.ts b/src/test/e2e/services/playground-service.test.ts index 9b60cd39d3..dcaaa27ef9 100644 --- a/src/test/e2e/services/playground-service.test.ts +++ b/src/test/e2e/services/playground-service.test.ts @@ -136,9 +136,9 @@ export const seedDatabaseForPlaygroundTest = async ( feature.enabled, ); - await database.stores.featureToggleStore.saveVariants( - feature.project!, + await database.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( feature.name, + environment, [ ...(feature.variants ?? []).map((variant) => ({ ...variant,