Skip to content

Commit 661ca6b

Browse files
mfahadahmedoakbani
authored andcommitted
fix(decision_service): Return null variation if experiment key does not exist in feature's experimentIds array. (#206)
(cherry picked from commit cfea9ce)
1 parent ffd427c commit 661ca6b

File tree

4 files changed

+114
-6
lines changed

4 files changed

+114
-6
lines changed

packages/optimizely-sdk/lib/core/decision_service/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2017-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -314,7 +314,7 @@ DecisionService.prototype._getVariationForFeatureExperiment = function(feature,
314314
var group = this.configObj.groupIdMap[feature.groupId];
315315
if (group) {
316316
experiment = this._getExperimentInGroup(group, userId);
317-
if (experiment) {
317+
if (experiment && feature.experimentIds.indexOf(experiment.id) !== -1) {
318318
variationKey = this.getVariation(experiment.key, userId, attributes);
319319
}
320320
}

packages/optimizely-sdk/lib/core/decision_service/index.tests.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2017-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -888,6 +888,18 @@ describe('lib/core/decision_service', function() {
888888
assert.deepEqual(decision, expectedDecision);
889889
sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_with_group.');
890890
});
891+
892+
it('returns null decision for group experiment not referenced by the feature', function() {
893+
var noTrafficExpFeature = configObj.featureKeyMap.feature_exp_no_traffic;
894+
var decision = decisionServiceInstance.getVariationForFeature(noTrafficExpFeature, 'user1');
895+
var expectedDecision = {
896+
experiment: null,
897+
variation: null,
898+
decisionSource: null,
899+
};
900+
assert.deepEqual(decision, expectedDecision);
901+
sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_exp_no_traffic.');
902+
});
891903
});
892904

893905
describe('user not bucketed into the group', function() {

packages/optimizely-sdk/lib/optimizely/index.tests.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2016-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2016-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -2955,7 +2955,7 @@ describe('lib/optimizely', function() {
29552955
assert.strictEqual(result.length, 2);
29562956
assert.isAbove(result.indexOf('test_feature'), -1);
29572957
assert.isAbove(result.indexOf('test_feature_for_experiment'), -1);
2958-
sinon.assert.callCount(optlyInstance.isFeatureEnabled, 6);
2958+
sinon.assert.callCount(optlyInstance.isFeatureEnabled, 7);
29592959
sinon.assert.calledWithExactly(
29602960
optlyInstance.isFeatureEnabled,
29612961
'test_feature',
@@ -2992,6 +2992,12 @@ describe('lib/optimizely', function() {
29922992
'user1',
29932993
attributes
29942994
);
2995+
sinon.assert.calledWithExactly(
2996+
optlyInstance.isFeatureEnabled,
2997+
'feature_exp_no_traffic',
2998+
'user1',
2999+
attributes
3000+
);
29953001
});
29963002
});
29973003

packages/optimizely-sdk/lib/tests/test_data.js

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2018, Optimizely
2+
* Copyright 2016-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -417,6 +417,15 @@ var configWithFeatures = {
417417
'id': '599110',
418418
'experimentIds': [],
419419
'variables': []
420+
},
421+
{
422+
'rolloutId': '',
423+
'key': 'feature_exp_no_traffic',
424+
'id': '4482920079',
425+
'experimentIds': [
426+
'12115595439'
427+
],
428+
'variables': []
420429
}
421430
],
422431
'experiments': [
@@ -625,6 +634,74 @@ var configWithFeatures = {
625634
'entityId': '599082'
626635
}
627636
]
637+
},
638+
{
639+
'policy': 'random',
640+
'id': '595025',
641+
'experiments': [
642+
{
643+
'trafficAllocation': [
644+
{
645+
'endOfRange': 10000,
646+
'entityId': '12098126627'
647+
}
648+
],
649+
'layerId': '595005',
650+
'forcedVariations': {},
651+
'audienceIds': [],
652+
'variations': [
653+
{
654+
'key': 'all_traffic_variation',
655+
'id': '12098126627',
656+
'variables': []
657+
},
658+
{
659+
'key': 'no_traffic_variation',
660+
'id': '12098126628',
661+
'variables': []
662+
}
663+
],
664+
'status': 'Running',
665+
'key': 'all_traffic_experiment',
666+
'id': '12198292375'
667+
},
668+
{
669+
'trafficAllocation': [
670+
{
671+
'endOfRange': 5000,
672+
'entityId': '12098126629'
673+
},
674+
{
675+
'endOfRange': 10000,
676+
'entityId': '12098126630'
677+
}
678+
],
679+
'layerId': '12187694826',
680+
'forcedVariations': {},
681+
'audienceIds': [],
682+
'variations': [
683+
{
684+
'key': 'variation_5000',
685+
'id': '12098126629',
686+
'variables': []
687+
},
688+
{
689+
'key': 'variation_10000',
690+
'id': '12098126630',
691+
'variables': []
692+
}
693+
],
694+
'status': 'Running',
695+
'key': 'no_traffic_experiment',
696+
'id': '12115595439'
697+
}
698+
],
699+
'trafficAllocation': [
700+
{
701+
'endOfRange': 10000,
702+
'entityId': '12198292375'
703+
}
704+
]
628705
}
629706
],
630707
'attributes': [
@@ -1321,6 +1398,10 @@ var datafileWithFeaturesExpectedData = {
13211398
},
13221399
599080: {},
13231400
599081: {},
1401+
12098126627: {},
1402+
12098126628: {},
1403+
12098126629: {},
1404+
12098126630: {},
13241405
},
13251406

13261407
featureKeyMap: {
@@ -1535,6 +1616,15 @@ var datafileWithFeaturesExpectedData = {
15351616
'variables': [],
15361617
variableKeyMap: {},
15371618
},
1619+
feature_exp_no_traffic: {
1620+
'rolloutId': '',
1621+
'key': 'feature_exp_no_traffic',
1622+
'id': '4482920079',
1623+
'experimentIds': ['12115595439'],
1624+
'variables': [],
1625+
variableKeyMap: {},
1626+
groupId: '595025',
1627+
},
15381628
},
15391629
};
15401630

0 commit comments

Comments
 (0)