diff --git a/packages/amazonq/.changes/next-release/Bug Fix-e18d0bfd-d423-4c64-9454-b221eff45b7f.json b/packages/amazonq/.changes/next-release/Bug Fix-e18d0bfd-d423-4c64-9454-b221eff45b7f.json new file mode 100644 index 00000000000..d3cc074daf8 --- /dev/null +++ b/packages/amazonq/.changes/next-release/Bug Fix-e18d0bfd-d423-4c64-9454-b221eff45b7f.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "Add getFeature and isEnabled utility methods to FeatureConfigProvider" +} diff --git a/packages/core/src/shared/featureConfig.ts b/packages/core/src/shared/featureConfig.ts index 9f5ef487f78..085b3e3672a 100644 --- a/packages/core/src/shared/featureConfig.ts +++ b/packages/core/src/shared/featureConfig.ts @@ -27,17 +27,21 @@ export class FeatureContext { ) {} } -const testFeatureName = 'testFeature' -const customizationArnOverrideName = 'customizationArnOverride' const featureConfigPollIntervalInMs = 30 * 60 * 1000 // 30 mins -const dataCollectionFeatureName = 'IDEProjectContextDataCollection' -// TODO: add real feature later -export const featureDefinitions = new Map([ - [testFeatureName, new FeatureContext(testFeatureName, 'CONTROL', { stringValue: 'testValue' })], +export const Features = { + customizationArnOverride: 'customizationArnOverride', + dataCollectionFeature: 'IDEProjectContextDataCollection', + test: 'testFeature', +} as const + +export type FeatureName = (typeof Features)[keyof typeof Features] + +export const featureDefinitions = new Map([ + [Features.test, new FeatureContext(Features.test, 'CONTROL', { stringValue: 'testValue' })], [ - customizationArnOverrideName, - new FeatureContext(customizationArnOverrideName, 'customizationARN', { stringValue: '' }), + Features.customizationArnOverride, + new FeatureContext(Features.customizationArnOverride, 'customizationARN', { stringValue: '' }), ], ]) @@ -103,11 +107,12 @@ export class FeatureConfigProvider { }) getLogger().info('AB Testing Cohort Assignments %s', JSON.stringify(response.featureEvaluations)) - const customizationArnOverride = this.featureConfigs.get(customizationArnOverrideName)?.value?.stringValue + const customizationArnOverride = this.featureConfigs.get(Features.customizationArnOverride)?.value + ?.stringValue if (customizationArnOverride !== undefined) { // Double check if server-side wrongly returns a customizationArn to BID users if (isBuilderIdConnection(AuthUtil.instance.conn)) { - this.featureConfigs.delete(customizationArnOverrideName) + this.featureConfigs.delete(Features.customizationArnOverride) } else if (isIdcSsoConnection(AuthUtil.instance.conn)) { let availableCustomizations = undefined try { @@ -131,12 +136,12 @@ export class FeatureConfigProvider { getLogger().debug( `Customization arn ${customizationArnOverride} not available in listAvailableCustomizations, not using` ) - this.featureConfigs.delete(customizationArnOverrideName) + this.featureConfigs.delete(Features.customizationArnOverride) } } } - const dataCollectionValue = this.featureConfigs.get(dataCollectionFeatureName)?.value.stringValue + const dataCollectionValue = this.featureConfigs.get(Features.dataCollectionFeature)?.value.stringValue if (dataCollectionValue === 'data-collection') { this._isDataCollectionGroup = true // Enable local workspace index by default, for Amzn users. @@ -172,16 +177,16 @@ export class FeatureConfigProvider { // 6) Add a test case for this feature. // 7) In case `getXXX()` returns undefined, it should be treated as a default/control group. getTestFeature(): string | undefined { - return this.getFeatureValueForKey(testFeatureName).stringValue + return this.getFeatureValueForKey(Features.test).stringValue } getCustomizationArnOverride(): string | undefined { - return this.getFeatureValueForKey(customizationArnOverrideName).stringValue + return this.getFeatureValueForKey(Features.customizationArnOverride).stringValue } // Get the feature value for the given key. // In case of a misconfiguration, it will return a default feature value of Boolean true. - private getFeatureValueForKey(name: string): FeatureValue { + private getFeatureValueForKey(name: FeatureName): FeatureValue { return this.featureConfigs.get(name)?.value ?? featureDefinitions.get(name)?.value ?? { boolValue: true } } @@ -193,4 +198,28 @@ export class FeatureConfigProvider { public static getFeatureConfigs(): Map { return FeatureConfigProvider.instance.featureConfigs } + + /** + * Retrieves the FeatureContext object for a given feature name. + * + * @param {string} featureName - The name of the feature. + * @returns {FeatureContext | undefined} The FeatureContext object for the specified feature, or undefined if the feature doesn't exist. + */ + public static getFeature(featureName: FeatureName): FeatureContext | undefined { + return FeatureConfigProvider.instance.featureConfigs.get(featureName) + } + + /** + * Checks if a feature is active or not. + * + * @param {string} featureName - The name of the feature to check. + * @returns {boolean} False if the variation is not CONTROL, otherwise True + */ + public static isEnabled(featureName: FeatureName): boolean { + const featureContext = FeatureConfigProvider.getFeature(featureName) + if (featureContext && featureContext.variation.toLocaleLowerCase() !== 'control') { + return true + } + return false + } } diff --git a/packages/core/src/test/fake/mockFeatureConfigData.ts b/packages/core/src/test/fake/mockFeatureConfigData.ts index b75887f4481..52e1bdb41af 100644 --- a/packages/core/src/test/fake/mockFeatureConfigData.ts +++ b/packages/core/src/test/fake/mockFeatureConfigData.ts @@ -11,4 +11,14 @@ export const mockFeatureConfigsData: FeatureEvaluation[] = [ variation: 'TREATMENT', value: { stringValue: 'testValue' }, }, + { + feature: 'featureA', + variation: 'CONTROL', + value: { stringValue: 'testValue' }, + }, + { + feature: 'featureB', + variation: 'TREATMENT', + value: { stringValue: 'testValue' }, + }, ] diff --git a/packages/core/src/test/shared/featureConfig.test.ts b/packages/core/src/test/shared/featureConfig.test.ts index 5c9df0abb0b..abd073de922 100644 --- a/packages/core/src/test/shared/featureConfig.test.ts +++ b/packages/core/src/test/shared/featureConfig.test.ts @@ -6,19 +6,33 @@ import assert from 'assert' import sinon from 'sinon' import { AWSError, Request } from 'aws-sdk' -import { FeatureConfigProvider, featureDefinitions } from '../../shared/featureConfig' +import { Features, FeatureConfigProvider, featureDefinitions, FeatureName } from '../../shared/featureConfig' import { ListFeatureEvaluationsResponse } from '../../codewhisperer' import { createSpyClient } from '../codewhisperer/testUtil' import { mockFeatureConfigsData } from '../fake/mockFeatureConfigData' describe('FeatureConfigProvider', () => { + beforeEach(async () => { + const clientSpy = await createSpyClient() + sinon.stub(clientSpy, 'listFeatureEvaluations').returns({ + promise: () => + Promise.resolve({ + $response: { + requestId: '', + }, + featureEvaluations: mockFeatureConfigsData, + }), + } as Request) + await FeatureConfigProvider.instance.fetchFeatureConfigs() + }) + afterEach(function () { sinon.restore() }) it('featureDefinitions map is not empty', () => { assert.notStrictEqual(featureDefinitions.size, 0) - assert.ok(featureDefinitions.has('testFeature')) + assert.ok(featureDefinitions.has(Features.test)) }) it('provider has getters for all the features', () => { @@ -32,18 +46,52 @@ describe('FeatureConfigProvider', () => { }) it('test getFeatureConfigsTelemetry will return expected string', async () => { - const clientSpy = await createSpyClient() - sinon.stub(clientSpy, 'listFeatureEvaluations').returns({ - promise: () => - Promise.resolve({ - $response: { - requestId: '', + assert.strictEqual( + FeatureConfigProvider.instance.getFeatureConfigsTelemetry(), + `{testFeature: TREATMENT, featureA: CONTROL, featureB: TREATMENT}` + ) + }) + + it('should should return all feature flags', async () => { + it('should should return all feature flags', async () => { + const featureConfigs = FeatureConfigProvider.getFeatureConfigs() + const expectedFeatureConfigs = { + featureA: { + name: 'featureA', + value: { + stringValue: 'testValue', }, - featureEvaluations: mockFeatureConfigsData, - }), - } as Request) + variation: 'CONTROL', + }, + featureB: { + name: 'featureB', + value: { + stringValue: 'testValue', + }, + variation: 'TREATMENT', + }, + testFeature: { + name: 'testFeature', + value: { + stringValue: 'testValue', + }, + variation: 'TREATMENT', + }, + } - await FeatureConfigProvider.instance.fetchFeatureConfigs() - assert.strictEqual(FeatureConfigProvider.instance.getFeatureConfigsTelemetry(), `{testFeature: TREATMENT}`) + assert.deepStrictEqual(Object.fromEntries(featureConfigs), expectedFeatureConfigs) + }) + }) + + it('should test featureA as disabled', async () => { + assert.strictEqual(FeatureConfigProvider.isEnabled('featureA' as FeatureName), false) + }) + + it('should test featureB as enabled', async () => { + assert.strictEqual(FeatureConfigProvider.isEnabled('featureB' as FeatureName), true) + }) + + it('should test feature-does-not-exist as disabled', async () => { + assert.strictEqual(FeatureConfigProvider.isEnabled('feature-does-not-exist' as FeatureName), false) }) })