Skip to content

Commit fc1efa0

Browse files
raju-optiraju-opti
authored
[FSSDK-9778] return last experiment when duplicate key in config (#881)
1 parent d8daa0c commit fc1efa0

File tree

4 files changed

+275
-34
lines changed

4 files changed

+275
-34
lines changed

‎lib/core/optimizely_config/index.tests.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@
1515
*/
1616
import { assert } from 'chai';
1717
import { cloneDeep } from 'lodash';
18+
import sinon from 'sinon';
1819

1920
import { createOptimizelyConfig, OptimizelyConfig } from './';
2021
import { createProjectConfig } from '../project_config';
2122
import {
2223
getTestProjectConfigWithFeatures,
2324
getTypedAudiencesConfig,
2425
getSimilarRuleKeyConfig,
25-
getSimilarExperimentKeyConfig
26+
getSimilarExperimentKeyConfig,
27+
getDuplicateExperimentKeyConfig,
2628
} from '../../tests/test_data';
2729

2830
var datafile = getTestProjectConfigWithFeatures();
@@ -53,6 +55,7 @@ describe('lib/core/optimizely_config', function() {
5355
var projectSimilarRuleKeyConfigObject;
5456
var optimizelySimilarExperimentkeyConfigObject;
5557
var projectSimilarExperimentKeyConfigObject;
58+
5659
beforeEach(function() {
5760
projectConfigObject = createProjectConfig(cloneDeep(datafile));
5861
optimizelyConfigObject = createOptimizelyConfig(projectConfigObject, JSON.stringify(datafile));
@@ -85,6 +88,24 @@ describe('lib/core/optimizely_config', function() {
8588
});
8689
});
8790

91+
it('should keep the last experiment in case of duplicate key and log a warning', function() {
92+
const datafile = getDuplicateExperimentKeyConfig();
93+
const configObj = createProjectConfig(datafile, JSON.stringify(datafile));
94+
95+
const logger = {
96+
warn: sinon.spy(),
97+
}
98+
99+
const optimizelyConfig = createOptimizelyConfig(configObj, JSON.stringify(datafile), logger);
100+
const experimentsMap = optimizelyConfig.experimentsMap;
101+
102+
const duplicateKey = 'experiment_rule';
103+
const lastExperiment = datafile.experiments[datafile.experiments.length - 1];
104+
105+
assert.equal(experimentsMap['experiment_rule'].id, lastExperiment.id);
106+
assert.isTrue(logger.warn.calledWithExactly(`Duplicate experiment keys found in datafile: ${duplicateKey}`));
107+
});
108+
88109
it('should return all the feature flags', function() {
89110
var featureFlagsCount = Object.keys(optimizelyConfigObject.featuresMap).length;
90111
assert.equal(featureFlagsCount, 9);

‎lib/core/optimizely_config/index.ts

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
import { LoggerFacade, getLogger } from '../../modules/logging';
1617
import { ProjectConfig } from '../project_config';
1718
import { DEFAULT_OPERATOR_TYPES } from '../condition_tree_evaluator';
1819
import {
@@ -61,7 +62,8 @@ export class OptimizelyConfig {
6162
public events: OptimizelyEvent[];
6263
private datafile: string;
6364

64-
constructor(configObj: ProjectConfig, datafile: string) {
65+
66+
constructor(configObj: ProjectConfig, datafile: string, logger?: LoggerFacade) {
6567
this.sdkKey = configObj.sdkKey ?? '';
6668
this.environmentKey = configObj.environmentKey ?? '';
6769
this.attributes = configObj.attributes;
@@ -76,10 +78,12 @@ export class OptimizelyConfig {
7678

7779
const variableIdMap = OptimizelyConfig.getVariableIdMap(configObj);
7880

79-
const experimentsMapById = OptimizelyConfig.getExperimentsMapById(
80-
configObj, featureIdVariablesMap, variableIdMap
81+
const { experimentsMapById, experimentsMapByKey } = OptimizelyConfig.getExperimentsMap(
82+
configObj, featureIdVariablesMap, variableIdMap, logger,
8183
);
82-
this.experimentsMap = OptimizelyConfig.getExperimentsKeyMap(experimentsMapById);
84+
85+
this.experimentsMap = experimentsMapByKey;
86+
8387
this.featuresMap = OptimizelyConfig.getFeaturesMap(
8488
configObj, featureIdVariablesMap, experimentsMapById, variableIdMap
8589
);
@@ -347,39 +351,52 @@ export class OptimizelyConfig {
347351
* @param {ProjectConfig} configObj
348352
* @param {FeatureVariablesMap} featureIdVariableMap
349353
* @param {{[id: string]: FeatureVariable}} variableIdMap
350-
* @returns {[id: string]: OptimizelyExperiment} Experiments mapped by id
354+
* @returns { experimentsMapById: { [id: string]: OptimizelyExperiment }, experimentsMapByKey: OptimizelyExperimentsMap } Experiments mapped by id and key
351355
*/
352-
static getExperimentsMapById(
356+
static getExperimentsMap(
353357
configObj: ProjectConfig,
354358
featureIdVariableMap: FeatureVariablesMap,
355-
variableIdMap: {[id: string]: FeatureVariable}
356-
): { [id: string]: OptimizelyExperiment } {
359+
variableIdMap: {[id: string]: FeatureVariable},
360+
logger?: LoggerFacade,
361+
) : { experimentsMapById: { [id: string]: OptimizelyExperiment }, experimentsMapByKey: OptimizelyExperimentsMap } {
357362
const rolloutExperimentIds = this.getRolloutExperimentIds(configObj.rollouts);
358363

359-
const experiments = configObj.experiments;
364+
const experimentsMapById: { [id : string]: OptimizelyExperiment } = {};
365+
const experimentsMapByKey: OptimizelyExperimentsMap = {};
360366

361-
return (experiments || []).reduce((experimentsMap: { [id: string]: OptimizelyExperiment }, experiment) => {
362-
if (rolloutExperimentIds.indexOf(experiment.id) === -1) {
363-
const featureIds = configObj.experimentFeatureMap[experiment.id];
364-
let featureId = '';
365-
if (featureIds && featureIds.length > 0) {
366-
featureId = featureIds[0];
367-
}
368-
const variationsMap = OptimizelyConfig.getVariationsMap(
369-
experiment.variations,
370-
featureIdVariableMap,
371-
variableIdMap,
372-
featureId.toString()
373-
);
374-
experimentsMap[experiment.id] = {
375-
id: experiment.id,
376-
key: experiment.key,
377-
audiences: OptimizelyConfig.getExperimentAudiences(experiment, configObj),
378-
variationsMap: variationsMap,
379-
};
367+
const experiments = configObj.experiments || [];
368+
experiments.forEach((experiment) => {
369+
if (rolloutExperimentIds.indexOf(experiment.id) !== -1) {
370+
return;
380371
}
381-
return experimentsMap;
382-
}, {});
372+
373+
const featureIds = configObj.experimentFeatureMap[experiment.id];
374+
let featureId = '';
375+
if (featureIds && featureIds.length > 0) {
376+
featureId = featureIds[0];
377+
}
378+
const variationsMap = OptimizelyConfig.getVariationsMap(
379+
experiment.variations,
380+
featureIdVariableMap,
381+
variableIdMap,
382+
featureId.toString()
383+
);
384+
385+
const optimizelyExperiment: OptimizelyExperiment = {
386+
id: experiment.id,
387+
key: experiment.key,
388+
audiences: OptimizelyConfig.getExperimentAudiences(experiment, configObj),
389+
variationsMap: variationsMap,
390+
};
391+
392+
experimentsMapById[experiment.id] = optimizelyExperiment;
393+
if (experimentsMapByKey[experiment.key] && logger) {
394+
logger.warn(`Duplicate experiment keys found in datafile: ${experiment.key}`);
395+
}
396+
experimentsMapByKey[experiment.key] = optimizelyExperiment;
397+
});
398+
399+
return { experimentsMapById, experimentsMapByKey };
383400
}
384401

385402
/**
@@ -461,6 +478,6 @@ export class OptimizelyConfig {
461478
* @param {string} datafile
462479
* @returns {OptimizelyConfig} An instance of OptimizelyConfig
463480
*/
464-
export function createOptimizelyConfig(configObj: ProjectConfig, datafile: string): OptimizelyConfig {
465-
return new OptimizelyConfig(configObj, datafile);
481+
export function createOptimizelyConfig(configObj: ProjectConfig, datafile: string, logger?: LoggerFacade): OptimizelyConfig {
482+
return new OptimizelyConfig(configObj, datafile, logger);
466483
}

‎lib/core/project_config/project_config_manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ export class ProjectConfigManager {
210210
*/
211211
getOptimizelyConfig(): OptimizelyConfig | null {
212212
if (!this.optimizelyConfigObj && this.configObj) {
213-
this.optimizelyConfigObj = createOptimizelyConfig(this.configObj, toDatafile(this.configObj));
213+
this.optimizelyConfigObj = createOptimizelyConfig(this.configObj, toDatafile(this.configObj), logger);
214214
}
215215
return this.optimizelyConfigObj;
216216
}

‎lib/tests/test_data.js

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3951,6 +3951,208 @@ export var getSimilarExperimentKeyConfig = function() {
39513951
return cloneDeep(similarExperimentKeysConfig);
39523952
};
39533953

3954+
const duplicateExperimentKeyConfig = {
3955+
"accountId": "23793010390",
3956+
"projectId": "24812320344",
3957+
"revision": "24",
3958+
"attributes": [
3959+
{
3960+
"id": "24778491463",
3961+
"key": "country"
3962+
},
3963+
{
3964+
"id": "24802951640",
3965+
"key": "likes_donuts"
3966+
}
3967+
],
3968+
"audiences": [
3969+
{
3970+
"name": "mutext_feat",
3971+
"conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]",
3972+
"id": "24837020039"
3973+
},
3974+
{
3975+
"id": "$opt_dummy_audience",
3976+
"name": "Optimizely-Generated Audience for Backwards Compatibility",
3977+
"conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]"
3978+
}
3979+
],
3980+
"version": "4",
3981+
"events": [],
3982+
"integrations": [],
3983+
"anonymizeIP": true,
3984+
"botFiltering": false,
3985+
"typedAudiences": [
3986+
{
3987+
"name": "mutext_feat",
3988+
"conditions": [
3989+
"and",
3990+
[
3991+
"or",
3992+
[
3993+
"or",
3994+
{
3995+
"match": "exact",
3996+
"name": "country",
3997+
"type": "custom_attribute",
3998+
"value": "US"
3999+
}
4000+
],
4001+
[
4002+
"or",
4003+
{
4004+
"match": "exact",
4005+
"name": "likes_donuts",
4006+
"type": "custom_attribute",
4007+
"value": true
4008+
}
4009+
]
4010+
]
4011+
],
4012+
"id": "24837020039"
4013+
}
4014+
],
4015+
"variables": [],
4016+
"environmentKey": "production",
4017+
"sdkKey": "BBhivmjEBF1KLK8HkMrvj",
4018+
"featureFlags": [
4019+
{
4020+
"id": "101043",
4021+
"key": "mutext_feat2",
4022+
"rolloutId": "rollout-101043-24783691394",
4023+
"experimentIds": [
4024+
"9300000361925"
4025+
],
4026+
"variables": []
4027+
},
4028+
{
4029+
"id": "101044",
4030+
"key": "mutex_feat",
4031+
"rolloutId": "rollout-101044-24783691394",
4032+
"experimentIds": [
4033+
"9300000365056"
4034+
],
4035+
"variables": []
4036+
}
4037+
],
4038+
"rollouts": [
4039+
{
4040+
"id": "rollout-101043-24783691394",
4041+
"experiments": [
4042+
{
4043+
"id": "default-rollout-101043-24783691394",
4044+
"key": "default-rollout-101043-24783691394",
4045+
"status": "Running",
4046+
"layerId": "rollout-101043-24783691394",
4047+
"variations": [
4048+
{
4049+
"id": "321340",
4050+
"key": "off",
4051+
"featureEnabled": false,
4052+
"variables": []
4053+
}
4054+
],
4055+
"trafficAllocation": [
4056+
{
4057+
"entityId": "321340",
4058+
"endOfRange": 10000
4059+
}
4060+
],
4061+
"forcedVariations": {},
4062+
"audienceIds": [],
4063+
"audienceConditions": []
4064+
}
4065+
]
4066+
},
4067+
{
4068+
"id": "rollout-101044-24783691394",
4069+
"experiments": [
4070+
{
4071+
"id": "default-rollout-101044-24783691394",
4072+
"key": "default-rollout-101044-24783691394",
4073+
"status": "Running",
4074+
"layerId": "rollout-101044-24783691394",
4075+
"variations": [
4076+
{
4077+
"id": "321343",
4078+
"key": "off",
4079+
"featureEnabled": false,
4080+
"variables": []
4081+
}
4082+
],
4083+
"trafficAllocation": [
4084+
{
4085+
"entityId": "321343",
4086+
"endOfRange": 10000
4087+
}
4088+
],
4089+
"forcedVariations": {},
4090+
"audienceIds": [],
4091+
"audienceConditions": []
4092+
}
4093+
]
4094+
}
4095+
],
4096+
"experiments": [
4097+
{
4098+
"id": "9300000361925",
4099+
"key": "experiment_rule",
4100+
"status": "Running",
4101+
"layerId": "9300000284731",
4102+
"variations": [
4103+
{
4104+
"id": "321342",
4105+
"key": "variation_1",
4106+
"featureEnabled": true,
4107+
"variables": []
4108+
}
4109+
],
4110+
"trafficAllocation": [
4111+
{
4112+
"entityId": "321342",
4113+
"endOfRange": 10000
4114+
}
4115+
],
4116+
"forcedVariations": {},
4117+
"audienceIds": [
4118+
"24837020039"
4119+
],
4120+
"audienceConditions": [
4121+
"or",
4122+
"24837020039"
4123+
]
4124+
},
4125+
{
4126+
"id": "9300000365056",
4127+
"key": "experiment_rule",
4128+
"status": "Running",
4129+
"layerId": "9300000287826",
4130+
"variations": [
4131+
{
4132+
"id": "321345",
4133+
"key": "variation_1",
4134+
"featureEnabled": true,
4135+
"variables": []
4136+
}
4137+
],
4138+
"trafficAllocation": [
4139+
{
4140+
"entityId": "321345",
4141+
"endOfRange": 10000
4142+
}
4143+
],
4144+
"forcedVariations": {},
4145+
"audienceIds": [],
4146+
"audienceConditions": []
4147+
}
4148+
],
4149+
"groups": []
4150+
};
4151+
4152+
export const getDuplicateExperimentKeyConfig = function() {
4153+
return cloneDeep(duplicateExperimentKeyConfig);
4154+
};
4155+
39544156
export default {
39554157
getTestProjectConfig: getTestProjectConfig,
39564158
getTestDecideProjectConfig: getTestDecideProjectConfig,
@@ -3966,4 +4168,5 @@ export default {
39664168
getMutexFeatureTestsConfig: getMutexFeatureTestsConfig,
39674169
getSimilarRuleKeyConfig: getSimilarRuleKeyConfig,
39684170
getSimilarExperimentKeyConfig: getSimilarExperimentKeyConfig,
4171+
getDuplicateExperimentKeyConfig: getDuplicateExperimentKeyConfig,
39694172
};

0 commit comments

Comments
 (0)