diff --git a/src/lib/db/feature-environment-store.ts b/src/lib/db/feature-environment-store.ts index b7ef3261ea..fb4f816d79 100644 --- a/src/lib/db/feature-environment-store.ts +++ b/src/lib/db/feature-environment-store.ts @@ -371,16 +371,30 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { featureName: string, environment: string, variants: IVariant[], + ): Promise { + return this.setVariantsToFeatureEnvironments( + featureName, + [environment], + variants, + ); + } + + async setVariantsToFeatureEnvironments( + featureName: string, + environments: string[], + variants: IVariant[], ): Promise { let v = variants || []; v.sort((a, b) => a.name.localeCompare(b.name)); + const variantsString = JSON.stringify(v); + const records = environments.map((env) => ({ + variants: variantsString, + enabled: false, // default value for enabled in case it's not set + feature_name: featureName, + environment: env, + })); await this.db(T.featureEnvs) - .insert({ - variants: JSON.stringify(v), - enabled: false, - feature_name: featureName, - environment: environment, - }) + .insert(records) .onConflict(['feature_name', 'environment']) .merge(['variants']); } diff --git a/src/lib/error/no-access-error.ts b/src/lib/error/no-access-error.ts index 2034576c55..73fd2d486d 100644 --- a/src/lib/error/no-access-error.ts +++ b/src/lib/error/no-access-error.ts @@ -5,13 +5,20 @@ class NoAccessError extends Error { message: string; - constructor(permission: string) { + environment?: string; + + constructor(permission: string, environment?: string) { super(); Error.captureStackTrace(this, this.constructor); this.name = this.constructor.name; this.permission = permission; - this.message = `You need permission=${permission} to perform this action`; + this.environment = environment; + if (environment) { + this.message = `You need permission=${permission} to perform this action on environment=${environment}`; + } else { + this.message = `You need permission=${permission} to perform this action`; + } } toJSON(): any { diff --git a/src/lib/openapi/index.ts b/src/lib/openapi/index.ts index 90c46eef3e..3d6faa4579 100644 --- a/src/lib/openapi/index.ts +++ b/src/lib/openapi/index.ts @@ -87,6 +87,7 @@ import { publicSignupTokenSchema, publicSignupTokensSchema, publicSignupTokenUpdateSchema, + pushVariantsSchema, resetPasswordSchema, requestsPerSecondSchema, requestsPerSecondSegmentedSchema, @@ -223,6 +224,7 @@ export const schemas = { publicSignupTokenSchema, publicSignupTokensSchema, publicSignupTokenUpdateSchema, + pushVariantsSchema, resetPasswordSchema, requestsPerSecondSchema, requestsPerSecondSegmentedSchema, diff --git a/src/lib/openapi/spec/index.ts b/src/lib/openapi/spec/index.ts index 23378a128c..30c41fa3bd 100644 --- a/src/lib/openapi/spec/index.ts +++ b/src/lib/openapi/spec/index.ts @@ -124,3 +124,4 @@ export * from './requests-per-second-schema'; export * from './requests-per-second-segmented-schema'; export * from './export-result-schema'; export * from './export-query-schema'; +export * from './push-variants-schema'; diff --git a/src/lib/openapi/spec/push-variants-schema.ts b/src/lib/openapi/spec/push-variants-schema.ts new file mode 100644 index 0000000000..1b41bd52d5 --- /dev/null +++ b/src/lib/openapi/spec/push-variants-schema.ts @@ -0,0 +1,30 @@ +import { variantSchema } from './variant-schema'; +import { FromSchema } from 'json-schema-to-ts'; +import { overrideSchema } from './override-schema'; + +export const pushVariantsSchema = { + $id: '#/components/schemas/pushVariantsSchema', + type: 'object', + properties: { + variants: { + type: 'array', + items: { + $ref: '#/components/schemas/variantSchema', + }, + }, + environments: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + components: { + schemas: { + variantSchema, + overrideSchema, + }, + }, +} as const; + +export type PushVariantsSchema = FromSchema; diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index d0741b2e3e..afab6ba7e6 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -10,12 +10,16 @@ import { UPDATE_FEATURE_ENVIRONMENT_VARIANTS, UPDATE_FEATURE_VARIANTS, } from '../../../types/permissions'; -import { IVariant } from '../../../types/model'; +import { IVariant, WeightType } from '../../../types/model'; import { extractUsername } from '../../../util/extract-user'; import { IAuthRequest } from '../../unleash-types'; import { FeatureVariantsSchema } from '../../../openapi/spec/feature-variants-schema'; import { createRequestSchema } from '../../../openapi/util/create-request-schema'; import { createResponseSchema } from '../../../openapi/util/create-response-schema'; +import { AccessService } from '../../../services'; +import { BadDataError, NoAccessError } from '../../../../lib/error'; +import { User } from 'lib/server-impl'; +import { PushVariantsSchema } from 'lib/openapi/spec/push-variants-schema'; const PREFIX = '/:projectId/features/:featureName/variants'; const ENV_PREFIX = @@ -37,16 +41,23 @@ export default class VariantsController extends Controller { private featureService: FeatureToggleService; + private accessService: AccessService; + constructor( config: IUnleashConfig, { featureToggleService, openApiService, - }: Pick, + accessService, + }: Pick< + IUnleashServices, + 'featureToggleService' | 'openApiService' | 'accessService' + >, ) { super(config); this.logger = config.getLogger('admin-api/project/variants.ts'); this.featureService = featureToggleService; + this.accessService = accessService; this.route({ method: 'get', path: PREFIX, @@ -141,6 +152,22 @@ export default class VariantsController extends Controller { }), ], }); + this.route({ + method: 'put', + path: `${PREFIX}-batch`, + permission: NONE, + handler: this.pushVariantsToEnvironments, + middleware: [ + openApiService.validPath({ + tags: ['Features'], + operationId: 'overwriteFeatureVariantsOnEnvironments', + requestBody: createRequestSchema('pushVariantsSchema'), + responses: { + 200: createResponseSchema('featureVariantsSchema'), + }, + }), + ], + }); } /** @@ -193,6 +220,72 @@ export default class VariantsController extends Controller { }); } + async pushVariantsToEnvironments( + req: IAuthRequest< + FeatureEnvironmentParams, + any, + PushVariantsSchema, + any + >, + res: Response, + ): Promise { + const { projectId, featureName } = req.params; + const { environments, variants } = req.body; + const userName = extractUsername(req); + + if (environments === undefined || environments.length === 0) { + throw new BadDataError('No environments provided'); + } + + await this.checkAccess( + req.user, + projectId, + environments, + UPDATE_FEATURE_ENVIRONMENT_VARIANTS, + ); + + const variantsWithDefaults = variants.map((variant) => ({ + weightType: WeightType.VARIABLE, + stickiness: 'default', + ...variant, + })); + + await this.featureService.setVariantsOnEnvs( + projectId, + featureName, + environments, + variantsWithDefaults, + userName, + ); + res.status(200).json({ + version: 1, + variants: variantsWithDefaults, + }); + } + + async checkAccess( + user: User, + projectId: string, + environments: string[], + permission: string, + ): Promise { + for (const environment of environments) { + if ( + !(await this.accessService.hasPermission( + user, + permission, + projectId, + environment, + )) + ) { + throw new NoAccessError( + UPDATE_FEATURE_ENVIRONMENT_VARIANTS, + environment, + ); + } + } + } + async getVariantsOnEnv( req: Request, res: Response, diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 13f5df35ad..6b4629ddc1 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -750,13 +750,12 @@ class FeatureToggleService { await this.featureEnvironmentStore.getEnvironmentsForFeature( featureName, ); - environments.forEach(async (featureEnv) => { - await this.featureEnvironmentStore.addVariantsToFeatureEnvironment( - featureName, - featureEnv.environment, - value.variants, - ); - }); + + await this.featureEnvironmentStore.setVariantsToFeatureEnvironments( + featureName, + environments.map((env) => env.environment), + value.variants, + ); } const tags = await this.tagStore.getAllTagsForFeature(featureName); @@ -1350,9 +1349,51 @@ class FeatureToggleService { newVariants: fixedVariants, }), ); - await this.featureEnvironmentStore.addVariantsToFeatureEnvironment( + await this.featureEnvironmentStore.setVariantsToFeatureEnvironments( featureName, - environment, + [environment], + fixedVariants, + ); + return fixedVariants; + } + + async setVariantsOnEnvs( + projectId: string, + featureName: string, + environments: string[], + newVariants: IVariant[], + createdBy: string, + ): Promise { + await variantsArraySchema.validateAsync(newVariants); + const fixedVariants = this.fixVariantWeights(newVariants); + const oldVariants: { [env: string]: IVariant[] } = environments.reduce( + async (result, environment) => { + result[environment] = await this.featureEnvironmentStore.get({ + featureName, + environment, + }); + return result; + }, + {}, + ); + + await this.eventStore.batchStore( + environments.map( + (environment) => + new EnvironmentVariantEvent({ + featureName, + environment, + project: projectId, + createdBy, + oldVariants: oldVariants[environment], + newVariants: fixedVariants, + }), + ), + ); + + await this.featureEnvironmentStore.setVariantsToFeatureEnvironments( + featureName, + environments, fixedVariants, ); return fixedVariants; diff --git a/src/lib/types/stores/feature-environment-store.ts b/src/lib/types/stores/feature-environment-store.ts index e7435c7145..f31174cb85 100644 --- a/src/lib/types/stores/feature-environment-store.ts +++ b/src/lib/types/stores/feature-environment-store.ts @@ -75,6 +75,12 @@ export interface IFeatureEnvironmentStore variants: IVariant[], ): Promise; + setVariantsToFeatureEnvironments( + featureName: string, + environments: string[], + variants: IVariant[], + ): Promise; + addFeatureEnvironment( featureEnvironment: IFeatureEnvironment, ): Promise; 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 d3da1ece46..115b536d95 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -10,6 +10,14 @@ let db: ITestDb; beforeAll(async () => { db = await dbInit('project_feature_variants_api_serial', getLogger); app = await setupApp(db.stores); + await db.stores.environmentStore.create({ + name: 'development', + type: 'development', + }); + await db.stores.environmentStore.create({ + name: 'production', + type: 'production', + }); }); afterAll(async () => { @@ -132,14 +140,6 @@ test('Can patch variants for a feature patches all environments independently', }, ]; - await db.stores.environmentStore.create({ - name: 'development', - type: 'development', - }); - await db.stores.environmentStore.create({ - name: 'production', - type: 'production', - }); await db.stores.featureToggleStore.create('default', { name: featureName, }); @@ -218,6 +218,84 @@ test('Can patch variants for a feature patches all environments independently', }); }); +test('Can push variants to multiple environments', async () => { + const featureName = 'feature-to-override'; + const variant = (name: string, weight: number) => ({ + name, + stickiness: 'default', + weight, + weightType: WeightType.VARIABLE, + }); + await db.stores.featureToggleStore.create('default', { + name: featureName, + }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'development', + true, + ); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'production', + true, + ); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'development', + [ + variant('dev-variant-1', 250), + variant('dev-variant-2', 250), + variant('dev-variant-3', 250), + variant('dev-variant-4', 250), + ], + ); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'production', + [variant('prod-variant', 1000)], + ); + + const overrideWith = { + variants: [ + variant('new-variant-1', 500), + variant('new-variant-2', 500), + ], + environments: ['development', 'production'], + }; + + await app.request + .put( + `/api/admin/projects/default/features/${featureName}/variants-batch`, + ) + .send(overrideWith) + .expect(200) + .expect((res) => { + expect(res.body.version).toBe(1); + expect(res.body.variants).toHaveLength(2); + expect(res.body.variants[0].name).toBe('new-variant-1'); + expect(res.body.variants[1].name).toBe('new-variant-2'); + }); + + 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[0].name).toBe('new-variant-1'); + expect(developmentVariants[1].name).toBe('new-variant-2'); + }); +}); + test('Can add variant for a feature', async () => { const featureName = 'feature-variants-patch-add'; const variantName = 'fancy-variant-patch'; 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 c6d35f72da..e059bea6d3 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 @@ -2766,6 +2766,23 @@ exports[`should serve the OpenAPI spec 1`] = ` ], "type": "object", }, + "pushVariantsSchema": { + "properties": { + "environments": { + "items": { + "type": "string", + }, + "type": "array", + }, + "variants": { + "items": { + "$ref": "#/components/schemas/variantSchema", + }, + "type": "array", + }, + }, + "type": "object", + }, "requestsPerSecondSchema": { "properties": { "data": { @@ -6600,6 +6617,55 @@ If the provided project does not exist, the list of events will be empty.", ], }, }, + "/api/admin/projects/{projectId}/features/{featureName}/variants-batch": { + "put": { + "operationId": "overwriteFeatureVariantsOnEnvironments", + "parameters": [ + { + "in": "path", + "name": "projectId", + "required": true, + "schema": { + "type": "string", + }, + }, + { + "in": "path", + "name": "featureName", + "required": true, + "schema": { + "type": "string", + }, + }, + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/pushVariantsSchema", + }, + }, + }, + "description": "pushVariantsSchema", + "required": true, + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/featureVariantsSchema", + }, + }, + }, + "description": "featureVariantsSchema", + }, + }, + "tags": [ + "Features", + ], + }, + }, "/api/admin/projects/{projectId}/health-report": { "get": { "operationId": "getProjectHealthReport", diff --git a/src/test/fixtures/fake-feature-environment-store.ts b/src/test/fixtures/fake-feature-environment-store.ts index f21192e597..507a06e09d 100644 --- a/src/test/fixtures/fake-feature-environment-store.ts +++ b/src/test/fixtures/fake-feature-environment-store.ts @@ -22,12 +22,24 @@ export default class FakeFeatureEnvironmentStore featureName: string, environment: string, variants: IVariant[], + ): Promise { + this.setVariantsToFeatureEnvironments( + featureName, + [environment], + variants, + ); + } + + async setVariantsToFeatureEnvironments( + featureName: string, + environments: string[], + variants: IVariant[], ): Promise { this.featureEnvironments .filter( (fe) => fe.featureName === featureName && - fe.environment === environment, + environments.indexOf(fe.environment) !== -1, ) .map((fe) => (fe.variants = variants)); }