diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index 111980f9ec3..ef643b07bbe 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -48,6 +48,7 @@ export interface IFlags { proPlanAutoCharge?: boolean; notifications?: boolean; loginHistory?: boolean; + bulkOperations?: boolean; projectScopedSegments?: boolean; } diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index b649f742fbb..f0f3bf41877 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -69,6 +69,7 @@ exports[`should create default config 1`] = ` "flags": { "ENABLE_DARK_MODE_SUPPORT": false, "anonymiseEventLog": false, + "bulkOperations": false, "caseInsensitiveInOperators": false, "crOnVariants": false, "embedProxy": true, @@ -92,6 +93,7 @@ exports[`should create default config 1`] = ` "experiments": { "ENABLE_DARK_MODE_SUPPORT": false, "anonymiseEventLog": false, + "bulkOperations": false, "caseInsensitiveInOperators": false, "crOnVariants": false, "embedProxy": true, diff --git a/src/lib/db/feature-tag-store.ts b/src/lib/db/feature-tag-store.ts index f3d68e51c57..b79d087878c 100644 --- a/src/lib/db/feature-tag-store.ts +++ b/src/lib/db/feature-tag-store.ts @@ -1,10 +1,8 @@ -import { EventEmitter } from 'stream'; import { Logger, LogProvider } from '../logger'; -import { ITag } from '../types/model'; +import { ITag } from '../types'; +import EventEmitter from 'events'; import metricsHelper from '../util/metrics-helper'; import { DB_TIME } from '../metric-events'; -import { UNIQUE_CONSTRAINT_VIOLATION } from '../error/db-error'; -import FeatureHasTagError from '../error/feature-has-tag-error'; import { IFeatureAndTag, IFeatureTag, @@ -123,34 +121,22 @@ class FeatureTagStore implements IFeatureTagStore { const stopTimer = this.timer('tagFeature'); await this.db(TABLE) .insert(this.featureAndTagToRow(featureName, tag)) - .catch((err) => { - if (err.code === UNIQUE_CONSTRAINT_VIOLATION) { - throw new FeatureHasTagError( - `${featureName} already has the tag: [${tag.type}:${tag.value}]`, - ); - } else { - throw err; - } - }); + .onConflict(COLUMNS) + .merge(); stopTimer(); return tag; } - async tagFeatures(featureNames: string[], tag: ITag): Promise { - const stopTimer = this.timer('tagFeatures'); - await this.db(TABLE) - .insert(this.featuresAndTagToRow(featureNames, tag)) - .catch((err) => { - if (err.code === UNIQUE_CONSTRAINT_VIOLATION) { - throw new FeatureHasTagError( - `Some of the features already have the tag: [${tag.type}:${tag.value}]`, - ); - } else { - throw err; - } - }); + async untagFeatures(featureTags: IFeatureTag[]): Promise { + const stopTimer = this.timer('untagFeatures'); + try { + await this.db(TABLE) + .whereIn(COLUMNS, featureTags.map(this.featureTagArray)) + .delete(); + } catch (err) { + this.logger.error(err); + } stopTimer(); - return tag; } /** @@ -176,11 +162,9 @@ class FeatureTagStore implements IFeatureTagStore { stopTimer(); } - async importFeatureTags( - featureTags: IFeatureTag[], - ): Promise { + async tagFeatures(featureTags: IFeatureTag[]): Promise { const rows = await this.db(TABLE) - .insert(featureTags.map(this.importToRow)) + .insert(featureTags.map(this.featureTagToRow)) .returning(COLUMNS) .onConflict(COLUMNS) .ignore(); @@ -222,7 +206,7 @@ class FeatureTagStore implements IFeatureTagStore { }; } - importToRow({ + featureTagToRow({ featureName, tagType, tagValue, @@ -234,6 +218,10 @@ class FeatureTagStore implements IFeatureTagStore { }; } + featureTagArray({ featureName, tagType, tagValue }: IFeatureTag): string[] { + return [featureName, tagType, tagValue]; + } + featureAndTagToRow( featureName: string, { type, value }: ITag, @@ -244,17 +232,6 @@ class FeatureTagStore implements IFeatureTagStore { tag_value: value, }; } - - featuresAndTagToRow( - featureNames: string[], - { type, value }: ITag, - ): FeatureTagTable[] { - return featureNames.map((featureName) => ({ - feature_name: featureName, - tag_type: type, - tag_value: value, - })); - } } module.exports = FeatureTagStore; diff --git a/src/lib/openapi/spec/tags-bulk-add-schema.test.ts b/src/lib/openapi/spec/tags-bulk-add-schema.test.ts index ecf8d4c0200..882dfdf03f1 100644 --- a/src/lib/openapi/spec/tags-bulk-add-schema.test.ts +++ b/src/lib/openapi/spec/tags-bulk-add-schema.test.ts @@ -4,9 +4,19 @@ import { TagsBulkAddSchema } from './tags-bulk-add-schema'; test('tagsBulkAddSchema', () => { const data: TagsBulkAddSchema = { features: ['my-feature'], - tag: { - type: 'simple', - value: 'besttag', + tags: { + addedTags: [ + { + type: 'simple', + value: 'besttag', + }, + ], + removedTags: [ + { + type: 'simple2', + value: 'besttag2', + }, + ], }, }; diff --git a/src/lib/openapi/spec/tags-bulk-add-schema.ts b/src/lib/openapi/spec/tags-bulk-add-schema.ts index 224c58941da..9d24b57b3ff 100644 --- a/src/lib/openapi/spec/tags-bulk-add-schema.ts +++ b/src/lib/openapi/spec/tags-bulk-add-schema.ts @@ -1,11 +1,12 @@ import { FromSchema } from 'json-schema-to-ts'; +import { updateTagsSchema } from './update-tags-schema'; import { tagSchema } from './tag-schema'; export const tagsBulkAddSchema = { $id: '#/components/schemas/tagsBulkAddSchema', type: 'object', additionalProperties: false, - required: ['features', 'tag'], + required: ['features', 'tags'], properties: { features: { type: 'array', @@ -14,12 +15,13 @@ export const tagsBulkAddSchema = { minLength: 1, }, }, - tag: { - $ref: '#/components/schemas/tagSchema', + tags: { + $ref: '#/components/schemas/updateTagsSchema', }, }, components: { schemas: { + updateTagsSchema, tagSchema, }, }, diff --git a/src/lib/openapi/spec/update-tags-schema.test.ts b/src/lib/openapi/spec/update-tags-schema.test.ts new file mode 100644 index 00000000000..d5a9a2f3d9b --- /dev/null +++ b/src/lib/openapi/spec/update-tags-schema.test.ts @@ -0,0 +1,23 @@ +import { validateSchema } from '../validate'; +import { UpdateTagsSchema } from './update-tags-schema'; + +test('updateTagsSchema', () => { + const data: UpdateTagsSchema = { + addedTags: [ + { + type: 'simple', + value: 'besttag', + }, + ], + removedTags: [ + { + type: 'simple2', + value: 'besttag2', + }, + ], + }; + + expect( + validateSchema('#/components/schemas/updateTagsSchema', data), + ).toBeUndefined(); +}); diff --git a/src/lib/routes/admin-api/tag.ts b/src/lib/routes/admin-api/tag.ts index de946c0bbfc..0d13202bf2b 100644 --- a/src/lib/routes/admin-api/tag.ts +++ b/src/lib/routes/admin-api/tag.ts @@ -24,6 +24,8 @@ import { import { emptyResponse } from '../../openapi/util/standard-responses'; import FeatureTagService from 'lib/services/feature-tag-service'; import { TagsBulkAddSchema } from '../../openapi/spec/tags-bulk-add-schema'; +import NotFoundError from '../../error/notfound-error'; +import { IFlagResolver } from '../../types'; const version = 1; @@ -36,6 +38,8 @@ class TagController extends Controller { private openApiService: OpenApiService; + private flagResolver: IFlagResolver; + constructor( config: IUnleashConfig, { @@ -52,6 +56,7 @@ class TagController extends Controller { this.openApiService = openApiService; this.featureTagService = featureTagService; this.logger = config.getLogger('/admin-api/tag.js'); + this.flagResolver = config.flagResolver; this.route({ method: 'get', @@ -85,18 +90,16 @@ class TagController extends Controller { ], }); this.route({ - method: 'post', + method: 'put', path: '/features', - handler: this.addTagToFeatures, + handler: this.updateFeaturesTags, permission: UPDATE_FEATURE, middleware: [ openApiService.validPath({ tags: ['Tags'], operationId: 'addTagToFeatures', requestBody: createRequestSchema('tagsBulkAddSchema'), - responses: { - 201: resourceCreatedResponseSchema('tagSchema'), - }, + responses: { 200: emptyResponse }, }), ], }); @@ -207,18 +210,22 @@ class TagController extends Controller { res.status(200).end(); } - async addTagToFeatures( + async updateFeaturesTags( req: IAuthRequest, res: Response, ): Promise { - const { features, tag } = req.body; + if (!this.flagResolver.isEnabled('bulkOperations')) { + throw new NotFoundError('Bulk operations are not enabled'); + } + const { features, tags } = req.body; const userName = extractUsername(req); - const addedTag = await this.featureTagService.addTags( + await this.featureTagService.updateTags( features, - tag, + tags.addedTags, + tags.removedTags, userName, ); - res.status(201).json(addedTag); + res.status(200).end(); } } export default TagController; diff --git a/src/lib/services/feature-tag-service.ts b/src/lib/services/feature-tag-service.ts index 20f831f4aa0..413f73f8865 100644 --- a/src/lib/services/feature-tag-service.ts +++ b/src/lib/services/feature-tag-service.ts @@ -4,7 +4,10 @@ import { FEATURE_TAGGED, FEATURE_UNTAGGED, TAG_CREATED } from '../types/events'; import { IUnleashConfig } from '../types/option'; import { IFeatureToggleStore, IUnleashStores } from '../types/stores'; import { tagSchema } from './tag-schema'; -import { IFeatureTagStore } from '../types/stores/feature-tag-store'; +import { + IFeatureTag, + IFeatureTagStore, +} from '../types/stores/feature-tag-store'; import { IEventStore } from '../types/stores/event-store'; import { ITagStore } from '../types/stores/tag-store'; import { ITag } from '../types/model'; @@ -64,30 +67,61 @@ class FeatureTagService { return validatedTag; } - async addTags( + async updateTags( featureNames: string[], - tag: ITag, + addedTags: ITag[], + removedTags: ITag[], userName: string, - ): Promise { + ): Promise { const featureToggles = await this.featureToggleStore.getAllByNames( featureNames, ); - const validatedTag = await tagSchema.validateAsync(tag); - await this.createTagIfNeeded(validatedTag, userName); - await this.featureTagStore.tagFeatures(featureNames, validatedTag); - await Promise.all( - featureToggles.map((featureToggle) => - this.eventStore.store({ - type: FEATURE_TAGGED, - createdBy: userName, - featureName: featureToggle.name, - project: featureToggle.project, - data: validatedTag, - }), - ), + addedTags.map((tag) => this.createTagIfNeeded(tag, userName)), ); - return validatedTag; + const createdFeatureTags: IFeatureTag[] = featureNames.flatMap( + (featureName) => + addedTags.map((addedTag) => ({ + featureName, + tagType: addedTag.type, + tagValue: addedTag.value, + })), + ); + + await this.featureTagStore.tagFeatures(createdFeatureTags); + + const removedFeatureTags: IFeatureTag[] = featureNames.flatMap( + (featureName) => + removedTags.map((addedTag) => ({ + featureName, + tagType: addedTag.type, + tagValue: addedTag.value, + })), + ); + + await this.featureTagStore.untagFeatures(removedFeatureTags); + + const creationEvents = featureToggles.flatMap((featureToggle) => + addedTags.map((addedTag) => ({ + type: FEATURE_TAGGED, + createdBy: userName, + featureName: featureToggle.name, + project: featureToggle.project, + data: addedTag, + })), + ); + + const removalEvents = featureToggles.flatMap((featureToggle) => + removedTags.map((removedTag) => ({ + type: FEATURE_UNTAGGED, + createdBy: userName, + featureName: featureToggle.name, + project: featureToggle.project, + data: removedTag, + })), + ); + + await this.eventStore.batchStore([...creationEvents, ...removalEvents]); } async createTagIfNeeded(tag: ITag, userName: string): Promise { diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index 977256110b7..64afc0c345c 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -600,10 +600,9 @@ export default class StateService { : true, ); if (featureTagsToInsert.length > 0) { - const importedFeatureTags = - await this.featureTagStore.importFeatureTags( - featureTagsToInsert, - ); + const importedFeatureTags = await this.featureTagStore.tagFeatures( + featureTagsToInsert, + ); const importedFeatureTagEvents = importedFeatureTags.map((tag) => ({ type: FEATURE_TAG_IMPORT, createdBy: userName, diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 9059dc55e6f..5f7bff67b71 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -64,6 +64,10 @@ const flags = { ), notifications: parseEnvVarBoolean(process.env.NOTIFICATIONS, false), loginHistory: parseEnvVarBoolean(process.env.UNLEASH_LOGIN_HISTORY, false), + bulkOperations: parseEnvVarBoolean( + process.env.UNLEASH_BULK_OPERATIONS, + false, + ), projectScopedSegments: parseEnvVarBoolean( process.env.PROJECT_SCOPED_SEGMENTS, false, diff --git a/src/lib/types/stores/feature-tag-store.ts b/src/lib/types/stores/feature-tag-store.ts index 1bbd7f2a461..eaa362cf3b9 100644 --- a/src/lib/types/stores/feature-tag-store.ts +++ b/src/lib/types/stores/feature-tag-store.ts @@ -15,7 +15,7 @@ export interface IFeatureTagStore extends Store { getAllTagsForFeature(featureName: string): Promise; getAllByFeatures(features: string[]): Promise; tagFeature(featureName: string, tag: ITag): Promise; - tagFeatures(featureNames: string[], tag: ITag): Promise; - importFeatureTags(featureTags: IFeatureTag[]): Promise; + tagFeatures(featureTags: IFeatureTag[]): Promise; untagFeature(featureName: string, tag: ITag): Promise; + untagFeatures(featureTags: IFeatureTag[]): Promise; } diff --git a/src/test/e2e/api/admin/feature.e2e.test.ts b/src/test/e2e/api/admin/feature.e2e.test.ts index 92879869f8b..4583c21c914 100644 --- a/src/test/e2e/api/admin/feature.e2e.test.ts +++ b/src/test/e2e/api/admin/feature.e2e.test.ts @@ -739,31 +739,6 @@ test('Querying with multiple filters ANDs the filters', async () => { }); }); -test('Tagging a feature with a tag it already has should return 409', async () => { - const feature1Name = `test.${randomId()}`; - await app.request.post('/api/admin/features').send({ - name: feature1Name, - type: 'killswitch', - enabled: true, - strategies: [{ name: 'default' }], - }); - - const tag = { value: randomId(), type: 'simple' }; - await app.request - .post(`/api/admin/features/${feature1Name}/tags`) - .send(tag) - .expect(201); - return app.request - .post(`/api/admin/features/${feature1Name}/tags`) - .send(tag) - .expect(409) - .expect((res) => { - expect(res.body.details[0].message).toBe( - `${feature1Name} already has the tag: [${tag.type}:${tag.value}]`, - ); - }); -}); - test('marks feature toggle as stale', async () => { expect.assertions(1); await app.request diff --git a/src/test/e2e/api/admin/tags.e2e.test.ts b/src/test/e2e/api/admin/tags.e2e.test.ts index c62cc2adb06..174b5ab419f 100644 --- a/src/test/e2e/api/admin/tags.e2e.test.ts +++ b/src/test/e2e/api/admin/tags.e2e.test.ts @@ -11,6 +11,7 @@ beforeAll(async () => { experimental: { flags: { strictSchemaValidation: true, + bulkOperations: true, }, }, }); @@ -112,16 +113,30 @@ test('Can delete a tag', async () => { test('Can tag features', async () => { const featureName = 'test.feature'; const featureName2 = 'test.feature2'; - const tag = { + const addedTag = { value: 'TeamRed', type: 'simple', }; + const removedTag = { + value: 'remove_me', + type: 'simple', + }; await app.request.post('/api/admin/features').send({ name: featureName, type: 'killswitch', enabled: true, strategies: [{ name: 'default' }], }); + + await db.stores.tagStore.createTag(removedTag); + await db.stores.featureTagStore.tagFeature(featureName, removedTag); + + const initialTagState = await app.request.get( + `/api/admin/features/${featureName}/tags`, + ); + + expect(initialTagState.body).toMatchObject({ tags: [removedTag] }); + await app.request.post('/api/admin/features').send({ name: featureName2, type: 'killswitch', @@ -129,9 +144,12 @@ test('Can tag features', async () => { strategies: [{ name: 'default' }], }); - await app.request.post('/api/admin/tags/features').send({ + await app.request.put('/api/admin/tags/features').send({ features: [featureName, featureName2], - tag: tag, + tags: { + addedTags: [addedTag], + removedTags: [removedTag], + }, }); const res = await app.request.get( `/api/admin/features/${featureName}/tags`, @@ -141,6 +159,6 @@ test('Can tag features', async () => { `/api/admin/features/${featureName2}/tags`, ); - expect(res.body).toMatchObject({ tags: [tag] }); - expect(res2.body).toMatchObject({ tags: [tag] }); + expect(res.body).toMatchObject({ tags: [addedTag] }); + expect(res2.body).toMatchObject({ tags: [addedTag] }); }); 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 fa9302f9c9b..e6bb1aba673 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 @@ -3652,13 +3652,13 @@ Stats are divided into current and previous **windows**. }, "type": "array", }, - "tag": { - "$ref": "#/components/schemas/tagSchema", + "tags": { + "$ref": "#/components/schemas/updateTagsSchema", }, }, "required": [ "features", - "tag", + "tags", ], "type": "object", }, @@ -7960,7 +7960,7 @@ If the provided project does not exist, the list of events will be empty.", }, }, "/api/admin/tags/features": { - "post": { + "put": { "operationId": "addTagToFeatures", "requestBody": { "content": { @@ -7974,24 +7974,8 @@ If the provided project does not exist, the list of events will be empty.", "required": true, }, "responses": { - "201": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/tagSchema", - }, - }, - }, - "description": "The resource was successfully created.", - "headers": { - "location": { - "description": "The location of the newly created resource.", - "schema": { - "format": "uri", - "type": "string", - }, - }, - }, + "200": { + "description": "This response has no body.", }, }, "tags": [ diff --git a/src/test/e2e/stores/feature-tag-store.e2e.test.ts b/src/test/e2e/stores/feature-tag-store.e2e.test.ts index 0f35e69fa27..b2e26d62e67 100644 --- a/src/test/e2e/stores/feature-tag-store.e2e.test.ts +++ b/src/test/e2e/stores/feature-tag-store.e2e.test.ts @@ -71,16 +71,6 @@ test('should untag feature', async () => { expect(featureTags).toHaveLength(0); }); -test('should throw if feature have tag', async () => { - expect.assertions(1); - await featureTagStore.tagFeature(featureName, tag); - try { - await featureTagStore.tagFeature(featureName, tag); - } catch (e) { - expect(e.message).toContain('already has the tag'); - } -}); - test('get all feature tags', async () => { await featureTagStore.tagFeature(featureName, tag); await featureToggleStore.create('default', { @@ -95,7 +85,7 @@ test('should import feature tags', async () => { await featureToggleStore.create('default', { name: 'some-other-toggle-import', }); - await featureTagStore.importFeatureTags([ + await featureTagStore.tagFeatures([ { featureName, tagType: tag.type, tagValue: tag.value }, { featureName: 'some-other-toggle-import', diff --git a/src/test/fixtures/fake-feature-tag-store.ts b/src/test/fixtures/fake-feature-tag-store.ts index 56acbb97cf6..34a43572271 100644 --- a/src/test/fixtures/fake-feature-tag-store.ts +++ b/src/test/fixtures/fake-feature-tag-store.ts @@ -57,9 +57,7 @@ export default class FakeFeatureTagStore implements IFeatureTagStore { return Promise.resolve(); } - async importFeatureTags( - featureTags: IFeatureTag[], - ): Promise { + async tagFeatures(featureTags: IFeatureTag[]): Promise { return Promise.all( featureTags.map(async (fT) => { const saved = await this.tagFeature(fT.featureName, { @@ -92,14 +90,9 @@ export default class FakeFeatureTagStore implements IFeatureTagStore { ); } - async tagFeatures(featureNames: string[], tag: ITag): Promise { - const featureTags = featureNames.map((featureName) => ({ - featureName, - tagType: tag.type, - tagValue: tag.value, - })); - this.featureTags.push(...featureTags); - return Promise.resolve(tag); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + untagFeatures(featureTags: IFeatureTag[]): Promise { + throw new Error('Method not implemented.'); } }