Skip to content

Commit 3466dda

Browse files
ivikashctlai95
andauthored
fix(fetureconfigprovider): add additional utility methods (#5539)
Add Additional Utility Methods to FeatureConfigProvider. License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Co-authored-by: Tai Lai <[email protected]>
1 parent faffcd0 commit 3466dda

File tree

4 files changed

+119
-28
lines changed

4 files changed

+119
-28
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Add getFeature and isEnabled utility methods to FeatureConfigProvider"
4+
}

packages/core/src/shared/featureConfig.ts

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,21 @@ export class FeatureContext {
2727
) {}
2828
}
2929

30-
const testFeatureName = 'testFeature'
31-
const customizationArnOverrideName = 'customizationArnOverride'
3230
const featureConfigPollIntervalInMs = 30 * 60 * 1000 // 30 mins
33-
const dataCollectionFeatureName = 'IDEProjectContextDataCollection'
3431

35-
// TODO: add real feature later
36-
export const featureDefinitions = new Map([
37-
[testFeatureName, new FeatureContext(testFeatureName, 'CONTROL', { stringValue: 'testValue' })],
32+
export const Features = {
33+
customizationArnOverride: 'customizationArnOverride',
34+
dataCollectionFeature: 'IDEProjectContextDataCollection',
35+
test: 'testFeature',
36+
} as const
37+
38+
export type FeatureName = (typeof Features)[keyof typeof Features]
39+
40+
export const featureDefinitions = new Map<FeatureName, FeatureContext>([
41+
[Features.test, new FeatureContext(Features.test, 'CONTROL', { stringValue: 'testValue' })],
3842
[
39-
customizationArnOverrideName,
40-
new FeatureContext(customizationArnOverrideName, 'customizationARN', { stringValue: '' }),
43+
Features.customizationArnOverride,
44+
new FeatureContext(Features.customizationArnOverride, 'customizationARN', { stringValue: '' }),
4145
],
4246
])
4347

@@ -103,11 +107,12 @@ export class FeatureConfigProvider {
103107
})
104108
getLogger().info('AB Testing Cohort Assignments %s', JSON.stringify(response.featureEvaluations))
105109

106-
const customizationArnOverride = this.featureConfigs.get(customizationArnOverrideName)?.value?.stringValue
110+
const customizationArnOverride = this.featureConfigs.get(Features.customizationArnOverride)?.value
111+
?.stringValue
107112
if (customizationArnOverride !== undefined) {
108113
// Double check if server-side wrongly returns a customizationArn to BID users
109114
if (isBuilderIdConnection(AuthUtil.instance.conn)) {
110-
this.featureConfigs.delete(customizationArnOverrideName)
115+
this.featureConfigs.delete(Features.customizationArnOverride)
111116
} else if (isIdcSsoConnection(AuthUtil.instance.conn)) {
112117
let availableCustomizations = undefined
113118
try {
@@ -131,12 +136,12 @@ export class FeatureConfigProvider {
131136
getLogger().debug(
132137
`Customization arn ${customizationArnOverride} not available in listAvailableCustomizations, not using`
133138
)
134-
this.featureConfigs.delete(customizationArnOverrideName)
139+
this.featureConfigs.delete(Features.customizationArnOverride)
135140
}
136141
}
137142
}
138143

139-
const dataCollectionValue = this.featureConfigs.get(dataCollectionFeatureName)?.value.stringValue
144+
const dataCollectionValue = this.featureConfigs.get(Features.dataCollectionFeature)?.value.stringValue
140145
if (dataCollectionValue === 'data-collection') {
141146
this._isDataCollectionGroup = true
142147
// Enable local workspace index by default, for Amzn users.
@@ -172,16 +177,16 @@ export class FeatureConfigProvider {
172177
// 6) Add a test case for this feature.
173178
// 7) In case `getXXX()` returns undefined, it should be treated as a default/control group.
174179
getTestFeature(): string | undefined {
175-
return this.getFeatureValueForKey(testFeatureName).stringValue
180+
return this.getFeatureValueForKey(Features.test).stringValue
176181
}
177182

178183
getCustomizationArnOverride(): string | undefined {
179-
return this.getFeatureValueForKey(customizationArnOverrideName).stringValue
184+
return this.getFeatureValueForKey(Features.customizationArnOverride).stringValue
180185
}
181186

182187
// Get the feature value for the given key.
183188
// In case of a misconfiguration, it will return a default feature value of Boolean true.
184-
private getFeatureValueForKey(name: string): FeatureValue {
189+
private getFeatureValueForKey(name: FeatureName): FeatureValue {
185190
return this.featureConfigs.get(name)?.value ?? featureDefinitions.get(name)?.value ?? { boolValue: true }
186191
}
187192

@@ -193,4 +198,28 @@ export class FeatureConfigProvider {
193198
public static getFeatureConfigs(): Map<string, FeatureContext> {
194199
return FeatureConfigProvider.instance.featureConfigs
195200
}
201+
202+
/**
203+
* Retrieves the FeatureContext object for a given feature name.
204+
*
205+
* @param {string} featureName - The name of the feature.
206+
* @returns {FeatureContext | undefined} The FeatureContext object for the specified feature, or undefined if the feature doesn't exist.
207+
*/
208+
public static getFeature(featureName: FeatureName): FeatureContext | undefined {
209+
return FeatureConfigProvider.instance.featureConfigs.get(featureName)
210+
}
211+
212+
/**
213+
* Checks if a feature is active or not.
214+
*
215+
* @param {string} featureName - The name of the feature to check.
216+
* @returns {boolean} False if the variation is not CONTROL, otherwise True
217+
*/
218+
public static isEnabled(featureName: FeatureName): boolean {
219+
const featureContext = FeatureConfigProvider.getFeature(featureName)
220+
if (featureContext && featureContext.variation.toLocaleLowerCase() !== 'control') {
221+
return true
222+
}
223+
return false
224+
}
196225
}

packages/core/src/test/fake/mockFeatureConfigData.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,14 @@ export const mockFeatureConfigsData: FeatureEvaluation[] = [
1111
variation: 'TREATMENT',
1212
value: { stringValue: 'testValue' },
1313
},
14+
{
15+
feature: 'featureA',
16+
variation: 'CONTROL',
17+
value: { stringValue: 'testValue' },
18+
},
19+
{
20+
feature: 'featureB',
21+
variation: 'TREATMENT',
22+
value: { stringValue: 'testValue' },
23+
},
1424
]

packages/core/src/test/shared/featureConfig.test.ts

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,33 @@
66
import assert from 'assert'
77
import sinon from 'sinon'
88
import { AWSError, Request } from 'aws-sdk'
9-
import { FeatureConfigProvider, featureDefinitions } from '../../shared/featureConfig'
9+
import { Features, FeatureConfigProvider, featureDefinitions, FeatureName } from '../../shared/featureConfig'
1010
import { ListFeatureEvaluationsResponse } from '../../codewhisperer'
1111
import { createSpyClient } from '../codewhisperer/testUtil'
1212
import { mockFeatureConfigsData } from '../fake/mockFeatureConfigData'
1313

1414
describe('FeatureConfigProvider', () => {
15+
beforeEach(async () => {
16+
const clientSpy = await createSpyClient()
17+
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
18+
promise: () =>
19+
Promise.resolve({
20+
$response: {
21+
requestId: '',
22+
},
23+
featureEvaluations: mockFeatureConfigsData,
24+
}),
25+
} as Request<ListFeatureEvaluationsResponse, AWSError>)
26+
await FeatureConfigProvider.instance.fetchFeatureConfigs()
27+
})
28+
1529
afterEach(function () {
1630
sinon.restore()
1731
})
1832

1933
it('featureDefinitions map is not empty', () => {
2034
assert.notStrictEqual(featureDefinitions.size, 0)
21-
assert.ok(featureDefinitions.has('testFeature'))
35+
assert.ok(featureDefinitions.has(Features.test))
2236
})
2337

2438
it('provider has getters for all the features', () => {
@@ -32,18 +46,52 @@ describe('FeatureConfigProvider', () => {
3246
})
3347

3448
it('test getFeatureConfigsTelemetry will return expected string', async () => {
35-
const clientSpy = await createSpyClient()
36-
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
37-
promise: () =>
38-
Promise.resolve({
39-
$response: {
40-
requestId: '',
49+
assert.strictEqual(
50+
FeatureConfigProvider.instance.getFeatureConfigsTelemetry(),
51+
`{testFeature: TREATMENT, featureA: CONTROL, featureB: TREATMENT}`
52+
)
53+
})
54+
55+
it('should should return all feature flags', async () => {
56+
it('should should return all feature flags', async () => {
57+
const featureConfigs = FeatureConfigProvider.getFeatureConfigs()
58+
const expectedFeatureConfigs = {
59+
featureA: {
60+
name: 'featureA',
61+
value: {
62+
stringValue: 'testValue',
4163
},
42-
featureEvaluations: mockFeatureConfigsData,
43-
}),
44-
} as Request<ListFeatureEvaluationsResponse, AWSError>)
64+
variation: 'CONTROL',
65+
},
66+
featureB: {
67+
name: 'featureB',
68+
value: {
69+
stringValue: 'testValue',
70+
},
71+
variation: 'TREATMENT',
72+
},
73+
testFeature: {
74+
name: 'testFeature',
75+
value: {
76+
stringValue: 'testValue',
77+
},
78+
variation: 'TREATMENT',
79+
},
80+
}
4581

46-
await FeatureConfigProvider.instance.fetchFeatureConfigs()
47-
assert.strictEqual(FeatureConfigProvider.instance.getFeatureConfigsTelemetry(), `{testFeature: TREATMENT}`)
82+
assert.deepStrictEqual(Object.fromEntries(featureConfigs), expectedFeatureConfigs)
83+
})
84+
})
85+
86+
it('should test featureA as disabled', async () => {
87+
assert.strictEqual(FeatureConfigProvider.isEnabled('featureA' as FeatureName), false)
88+
})
89+
90+
it('should test featureB as enabled', async () => {
91+
assert.strictEqual(FeatureConfigProvider.isEnabled('featureB' as FeatureName), true)
92+
})
93+
94+
it('should test feature-does-not-exist as disabled', async () => {
95+
assert.strictEqual(FeatureConfigProvider.isEnabled('feature-does-not-exist' as FeatureName), false)
4896
})
4997
})

0 commit comments

Comments
 (0)