Skip to content

Commit 06a0045

Browse files
author
Michael Ng
authored
Merge branch 'master' into yasir/datafilerevisionlog
2 parents 4ba497e + 8f15676 commit 06a0045

File tree

5 files changed

+210
-25
lines changed

5 files changed

+210
-25
lines changed

‎optimizely/decision/bucketer/experiment_bucketer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ func (b MurmurhashBucketer) Bucket(bucketingID string, experiment entities.Exper
5656
bucketKey := bucketingID + group.ID
5757
bucketedExperimentID := b.bucketToEntity(bucketKey, group.TrafficAllocation)
5858
if bucketedExperimentID == "" || bucketedExperimentID != experiment.ID {
59-
// User is not bucketed into an experiment in the exclusion group, return nil variation
60-
return nil, reasons.NotInGroup, nil
59+
// User is not bucketed into provided experiment in mutex group
60+
return nil, reasons.NotBucketedIntoVariation, nil
6161
}
6262
}
6363

‎optimizely/decision/bucketer/experiment_bucketer_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,5 @@ func TestBucketExclusionGroups(t *testing.T) {
114114
// since the bucket value maps to experiment 1, the user will not be bucketed for experiment 2
115115
bucketedVariation, reason, _ = bucketer.Bucket("ppid2", experiment2, exclusionGroup)
116116
assert.Nil(t, bucketedVariation)
117-
assert.Equal(t, reasons.NotInGroup, reason)
117+
assert.Equal(t, reasons.NotBucketedIntoVariation, reason)
118118
}

‎optimizely/decision/composite_feature_service.go

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,30 +46,32 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont
4646

4747
// Check if user is bucketed in feature experiment
4848
if f.featureExperimentService != nil && len(feature.FeatureExperiments) > 0 {
49-
// @TODO: add in a feature decision service that takes into account multiple experiments (via group mutex)
50-
experiment := feature.FeatureExperiments[0]
51-
experimentDecisionContext := ExperimentDecisionContext{
52-
Experiment: &experiment,
53-
ProjectConfig: decisionContext.ProjectConfig,
54-
}
55-
56-
experimentDecision, err := f.featureExperimentService.GetDecision(experimentDecisionContext, userContext)
57-
// Variation not nil means we got a decision and should return it
58-
if experimentDecision.Variation != nil {
59-
featureDecision := FeatureDecision{
60-
Experiment: experiment,
61-
Decision: experimentDecision.Decision,
62-
Variation: experimentDecision.Variation,
63-
Source: FeatureTest,
49+
// @TODO this can be improved by getting group ID first and determining experiment and then bucketing in experiment
50+
for _, experiment := range feature.FeatureExperiments {
51+
featureExperiment := experiment
52+
experimentDecisionContext := ExperimentDecisionContext{
53+
Experiment: &featureExperiment,
54+
ProjectConfig: decisionContext.ProjectConfig,
6455
}
6556

66-
cfLogger.Debug(fmt.Sprintf(
67-
`Decision made for feature test with key "%s" for user "%s" with the following reason: "%s".`,
68-
feature.Key,
69-
userContext.ID,
70-
featureDecision.Reason,
71-
))
72-
return featureDecision, err
57+
experimentDecision, err := f.featureExperimentService.GetDecision(experimentDecisionContext, userContext)
58+
// Variation not nil means we got a decision and should return it
59+
if experimentDecision.Variation != nil {
60+
featureDecision := FeatureDecision{
61+
Experiment: experiment,
62+
Decision: experimentDecision.Decision,
63+
Variation: experimentDecision.Variation,
64+
Source: FeatureTest,
65+
}
66+
67+
cfLogger.Debug(fmt.Sprintf(
68+
`Decision made for feature test with key "%s" for user "%s" with the following reason: "%s".`,
69+
feature.Key,
70+
userContext.ID,
71+
featureDecision.Reason,
72+
))
73+
return featureDecision, err
74+
}
7375
}
7476
}
7577

‎optimizely/decision/composite_feature_service_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/****************************************************************************
2+
* Copyright 2019, Optimizely, Inc. and contributors *
3+
* *
4+
* Licensed under the Apache License, Version 2.0 (the "License"); *
5+
* you may not use this file except in compliance with the License. *
6+
* You may obtain a copy of the License at *
7+
* *
8+
* http://www.apache.org/licenses/LICENSE-2.0 *
9+
* *
10+
* Unless required by applicable law or agreed to in writing, software *
11+
* distributed under the License is distributed on an "AS IS" BASIS, *
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
13+
* See the License for the specific language governing permissions and *
14+
* limitations under the License. *
15+
***************************************************************************/
16+
17+
package decision
18+
19+
import (
20+
"github.com/optimizely/go-sdk/optimizely/decision/reasons"
21+
"github.com/optimizely/go-sdk/optimizely/entities"
22+
"github.com/stretchr/testify/assert"
23+
"testing"
24+
)
25+
26+
func TestCompositeFeatureServiceGetDecisionFeatureExperiment(t *testing.T) {
27+
mockProjectConfig := new(mockProjectConfig)
28+
testFeatureDecisionContext := FeatureDecisionContext{
29+
Feature: &testFeat3335,
30+
ProjectConfig: mockProjectConfig,
31+
}
32+
testExperimentDecisionContext := ExperimentDecisionContext{
33+
Experiment: &testExp1113,
34+
ProjectConfig: mockProjectConfig,
35+
}
36+
mockExperimentDecision := ExperimentDecision{
37+
Decision: Decision{reasons.BucketedIntoVariation},
38+
Variation: &testExp1113Var2223,
39+
}
40+
41+
testUserContext := entities.UserContext{
42+
ID: "test_user_1",
43+
}
44+
45+
mockExperimentService := new(MockExperimentDecisionService)
46+
mockRolloutService := new(MockFeatureDecisionService)
47+
// Mock to return decision from feature experiment service
48+
mockExperimentService.On("GetDecision", testExperimentDecisionContext, testUserContext).Return(mockExperimentDecision, nil)
49+
50+
// Decision is returned from feature test evaluation
51+
expectedDecision := FeatureDecision{
52+
Decision: Decision{reasons.BucketedIntoVariation},
53+
Source: FeatureTest,
54+
Experiment: testExp1113,
55+
Variation: &testExp1113Var2223,
56+
}
57+
58+
compositeFeatureService := &CompositeFeatureService{
59+
featureExperimentService: mockExperimentService,
60+
rolloutDecisionService: mockRolloutService,
61+
}
62+
actualDecision, err := compositeFeatureService.GetDecision(testFeatureDecisionContext, testUserContext)
63+
assert.NoError(t, err)
64+
assert.Equal(t, expectedDecision, actualDecision)
65+
}
66+
67+
func TestCompositeFeatureServiceGetDecisionRollout(t *testing.T) {
68+
mockProjectConfig := new(mockProjectConfig)
69+
testFeatureDecisionContext := FeatureDecisionContext{
70+
Feature: &testFeat3335,
71+
ProjectConfig: mockProjectConfig,
72+
}
73+
testExperimentDecisionContext1 := ExperimentDecisionContext{
74+
Experiment: &testExp1113,
75+
ProjectConfig: mockProjectConfig,
76+
}
77+
testExperimentDecisionContext2 := ExperimentDecisionContext{
78+
Experiment: &testExp1114,
79+
ProjectConfig: mockProjectConfig,
80+
}
81+
82+
// Mock to not bucket user in feature experiment
83+
mockExperimentDecision := ExperimentDecision{
84+
Decision: Decision{reasons.NotBucketedIntoVariation},
85+
Variation: nil,
86+
}
87+
88+
testUserContext := entities.UserContext{
89+
ID: "test_user_1",
90+
}
91+
92+
mockFeatureDecision := FeatureDecision{
93+
Decision: Decision{reasons.BucketedIntoVariation},
94+
Source: Rollout,
95+
Experiment: testExp1115,
96+
Variation: &testExp1115Var2227,
97+
}
98+
99+
mockExperimentService := new(MockExperimentDecisionService)
100+
mockRolloutService := new(MockFeatureDecisionService)
101+
// Mock to return decision from feature experiment service which causes rollout service to be called
102+
mockExperimentService.On("GetDecision", testExperimentDecisionContext1, testUserContext).Return(mockExperimentDecision, nil)
103+
mockExperimentService.On("GetDecision", testExperimentDecisionContext2, testUserContext).Return(mockExperimentDecision, nil)
104+
mockRolloutService.On("GetDecision", testFeatureDecisionContext, testUserContext).Return(mockFeatureDecision, nil)
105+
106+
// Decision is returned from rollout evaluation
107+
expectedDecision := mockFeatureDecision
108+
109+
compositeFeatureService := &CompositeFeatureService{
110+
featureExperimentService: mockExperimentService,
111+
rolloutDecisionService: mockRolloutService,
112+
}
113+
actualDecision, err := compositeFeatureService.GetDecision(testFeatureDecisionContext, testUserContext)
114+
assert.NoError(t, err)
115+
assert.Equal(t, expectedDecision, actualDecision)
116+
}

‎optimizely/decision/helpers_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,70 @@ var testFeatRollout3334 = entities.Feature{
134134
Experiments: []entities.Experiment{testExp1112},
135135
},
136136
}
137+
138+
// Feature with test and rollout
139+
const testFeat3335Key = "test_feature_3335_key"
140+
// Will use this experiment for feature test
141+
const testExp1113Key = "test_experiment_1113"
142+
var testExp1113Var2223 = entities.Variation{ID: "2223", Key: "2223", FeatureEnabled: true}
143+
var testExp1113Var2224 = entities.Variation{ID: "2224", Key: "2224", FeatureEnabled: false}
144+
var testExp1113 = entities.Experiment{
145+
ID: "1113",
146+
Key: testExp1113Key,
147+
GroupID: "6666",
148+
Variations: map[string]entities.Variation{
149+
"2223": testExp1113Var2223,
150+
"2224": testExp1113Var2224,
151+
},
152+
TrafficAllocation: []entities.Range{
153+
entities.Range{EntityID: "2223", EndOfRange: 5000},
154+
entities.Range{EntityID: "2224", EndOfRange: 10000},
155+
},
156+
}
157+
const testExp1114Key = "test_experiment_1114"
158+
var testExp1114Var2225 = entities.Variation{ID: "2225", Key: "2225", FeatureEnabled: true}
159+
var testExp1114Var2226 = entities.Variation{ID: "2226", Key: "2226", FeatureEnabled: false}
160+
var testExp1114 = entities.Experiment{
161+
ID: "1114",
162+
Key: testExp1114Key,
163+
GroupID: "6666",
164+
Variations: map[string]entities.Variation{
165+
"2225": testExp1114Var2225,
166+
"2226": testExp1114Var2226,
167+
},
168+
TrafficAllocation: []entities.Range{
169+
entities.Range{EntityID: "2225", EndOfRange: 5000},
170+
entities.Range{EntityID: "2226", EndOfRange: 10000},
171+
},
172+
}
173+
var testGroup6666 = entities.Group{
174+
ID: "6666",
175+
Policy: "random",
176+
TrafficAllocation: []entities.Range{
177+
entities.Range{EntityID: "1113", EndOfRange: 3000},
178+
entities.Range{EntityID: "1114", EndOfRange: 6000},
179+
},
180+
}
181+
182+
// Will use this experiment for rollout
183+
const testExp1115Key = "test_experiment_1115"
184+
var testExp1115Var2227 = entities.Variation{ID: "2227", Key: "2227", FeatureEnabled: true}
185+
var testExp1115 = entities.Experiment{
186+
ID: "1115",
187+
Key: testExp1115Key,
188+
Variations: map[string]entities.Variation{
189+
"2227": testExp1115Var2227,
190+
},
191+
TrafficAllocation: []entities.Range{
192+
entities.Range{EntityID: "2227", EndOfRange: 5000},
193+
},
194+
}
195+
var testFeat3335 = entities.Feature{
196+
ID: "3335",
197+
Key: testFeat3335Key,
198+
FeatureExperiments: []entities.Experiment{testExp1113, testExp1114},
199+
Rollout: entities.Rollout{
200+
ID: "4445",
201+
Experiments: []entities.Experiment{testExp1115},
202+
},
203+
}

0 commit comments

Comments
 (0)