Skip to content

Commit 688ec51

Browse files
yasirfolio3pawels-optimizely
yasirfolio3
authored and
pawels-optimizely
committed
feat(gherkin-FV): Implemented forced-variation for gherkin. (#200)
* Fv implementation for gherkin. * fixes. * Adding missing files. * Changed variationIdMap to previous implementation and added VariationsKeyToIDMap. * nits fixed. * Addressed review comments. * fixes. * nits fixed. * fixes for removing flakiness while comparing dispatched events. * nit fixed.
1 parent f5b8ffe commit 688ec51

File tree

13 files changed

+224
-77
lines changed

13 files changed

+224
-77
lines changed

‎pkg/config/datafileprojectconfig/mappers/experiment.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen
8585
LayerID: rawExperiment.LayerID,
8686
Key: rawExperiment.Key,
8787
Variations: make(map[string]entities.Variation),
88+
VariationKeyToIDMap: make(map[string]string),
8889
TrafficAllocation: make([]entities.Range, len(rawExperiment.TrafficAllocation)),
8990
AudienceConditionTree: audienceConditionTree,
9091
Whitelist: rawExperiment.ForcedVariations,
@@ -93,6 +94,7 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen
9394

9495
for _, variation := range rawExperiment.Variations {
9596
experiment.Variations[variation.ID] = mapVariation(variation)
97+
experiment.VariationKeyToIDMap[variation.Key] = variation.ID
9698
}
9799

98100
for i, allocation := range rawExperiment.TrafficAllocation {

‎pkg/config/datafileprojectconfig/mappers/experiment_test.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ func TestMapExperiments(t *testing.T) {
8888
FeatureEnabled: false,
8989
},
9090
},
91+
VariationKeyToIDMap: map[string]string{
92+
"variation_1": "21111",
93+
"variation_2": "21112",
94+
},
9195
TrafficAllocation: []entities.Range{
9296
{
9397
EntityID: "21111",
@@ -132,12 +136,13 @@ func TestMapExperimentsWithStringAudienceCondition(t *testing.T) {
132136
experiments, experimentKeyMap := MapExperiments(rawExperiments, experimentGroupMap)
133137
expectedExperiments := map[string]entities.Experiment{
134138
"11111": {
135-
AudienceIds: []string{"31111"},
136-
ID: "11111",
137-
GroupID: "15",
138-
Key: "test_experiment_11111",
139-
Variations: map[string]entities.Variation{},
140-
TrafficAllocation: []entities.Range{},
139+
AudienceIds: []string{"31111"},
140+
ID: "11111",
141+
GroupID: "15",
142+
Key: "test_experiment_11111",
143+
Variations: map[string]entities.Variation{},
144+
VariationKeyToIDMap: map[string]string{},
145+
TrafficAllocation: []entities.Range{},
141146
AudienceConditionTree: &entities.TreeNode{
142147
Operator: "or",
143148
Nodes: []*entities.TreeNode{

‎pkg/config/datafileprojectconfig/mappers/rollout_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ func TestMapRollouts(t *testing.T) {
4242
"21111": entities.Rollout{
4343
ID: "21111",
4444
Experiments: []entities.Experiment{
45-
entities.Experiment{ID: "11111", Key: "exp_11111", Variations: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}},
46-
entities.Experiment{ID: "11112", Key: "exp_11112", Variations: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}},
45+
entities.Experiment{ID: "11111", Key: "exp_11111", Variations: map[string]entities.Variation{}, VariationKeyToIDMap: map[string]string{}, TrafficAllocation: []entities.Range{}},
46+
entities.Experiment{ID: "11112", Key: "exp_11112", Variations: map[string]entities.Variation{}, VariationKeyToIDMap: map[string]string{}, TrafficAllocation: []entities.Range{}},
4747
},
4848
},
4949
}

‎pkg/decision/experiment_override_service.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,8 @@ func (s ExperimentOverrideService) GetDecision(decisionContext ExperimentDecisio
103103
return decision, nil
104104
}
105105

106-
// TODO(Matt): Implement and use a way to access variations by key
107-
for _, variation := range decisionContext.Experiment.Variations {
108-
variation := variation
109-
if variation.Key == variationKey {
106+
if variationID, ok := decisionContext.Experiment.VariationKeyToIDMap[variationKey]; ok {
107+
if variation, ok := decisionContext.Experiment.Variations[variationID]; ok {
110108
decision.Variation = &variation
111109
decision.Reason = reasons.OverrideVariationAssignmentFound
112110
eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID))

‎pkg/decision/experiment_whitelist_service.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,8 @@ func (s ExperimentWhitelistService) GetDecision(decisionContext ExperimentDecisi
4747
return decision, nil
4848
}
4949

50-
// TODO(Matt): Add a VariationsByKey map to the Experiment struct, and use it to look up Variation by key
51-
for _, variation := range decisionContext.Experiment.Variations {
52-
variation := variation
53-
if variation.Key == variationKey {
50+
if id, ok := decisionContext.Experiment.VariationKeyToIDMap[variationKey]; ok {
51+
if variation, ok := decisionContext.Experiment.Variations[id]; ok {
5452
decision.Reason = reasons.WhitelistVariationAssignmentFound
5553
decision.Variation = &variation
5654
return decision, nil

‎pkg/decision/helpers_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ var testExp1111 = entities.Experiment{
104104
Variations: map[string]entities.Variation{
105105
"2222": testExp1111Var2222,
106106
},
107+
VariationKeyToIDMap: map[string]string{
108+
"2222": "2222",
109+
},
107110
TrafficAllocation: []entities.Range{
108111
entities.Range{EntityID: "2222", EndOfRange: 10000},
109112
},
@@ -140,6 +143,9 @@ var testExp1112 = entities.Experiment{
140143
Variations: map[string]entities.Variation{
141144
"2222": testExp1111Var2222,
142145
},
146+
VariationKeyToIDMap: map[string]string{
147+
"2222": "2222",
148+
},
143149
TrafficAllocation: []entities.Range{
144150
entities.Range{EntityID: "2222", EndOfRange: 10000},
145151
},
@@ -172,6 +178,10 @@ var testExp1113 = entities.Experiment{
172178
"2223": testExp1113Var2223,
173179
"2224": testExp1113Var2224,
174180
},
181+
VariationKeyToIDMap: map[string]string{
182+
"2223": "2223",
183+
"2224": "2224",
184+
},
175185
TrafficAllocation: []entities.Range{
176186
entities.Range{EntityID: "2223", EndOfRange: 5000},
177187
entities.Range{EntityID: "2224", EndOfRange: 10000},
@@ -190,6 +200,10 @@ var testExp1114 = entities.Experiment{
190200
"2225": testExp1114Var2225,
191201
"2226": testExp1114Var2226,
192202
},
203+
VariationKeyToIDMap: map[string]string{
204+
"2225": "2225",
205+
"2226": "2226",
206+
},
193207
TrafficAllocation: []entities.Range{
194208
entities.Range{EntityID: "2225", EndOfRange: 5000},
195209
entities.Range{EntityID: "2226", EndOfRange: 10000},
@@ -214,6 +228,9 @@ var testExp1115 = entities.Experiment{
214228
Variations: map[string]entities.Variation{
215229
"2227": testExp1115Var2227,
216230
},
231+
VariationKeyToIDMap: map[string]string{
232+
"2227": "2227",
233+
},
217234
TrafficAllocation: []entities.Range{
218235
entities.Range{EntityID: "2227", EndOfRange: 5000},
219236
},
@@ -239,6 +256,9 @@ var testTargetedExp1116 = entities.Experiment{
239256
Variations: map[string]entities.Variation{
240257
"2228": testTargetedExp1116Var2228,
241258
},
259+
VariationKeyToIDMap: map[string]string{
260+
"2228": "2228",
261+
},
242262
TrafficAllocation: []entities.Range{
243263
entities.Range{EntityID: "2228", EndOfRange: 10000},
244264
},
@@ -254,6 +274,9 @@ var testExpWhitelist = entities.Experiment{
254274
Variations: map[string]entities.Variation{
255275
"2229": testExpWhitelistVar2229,
256276
},
277+
VariationKeyToIDMap: map[string]string{
278+
"var_2229": "2229",
279+
},
257280
TrafficAllocation: []entities.Range{
258281
entities.Range{EntityID: "2229", EndOfRange: 10000},
259282
},

‎pkg/entities/experiment.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type Experiment struct {
3232
LayerID string
3333
Key string
3434
Variations map[string]Variation // keyed by variation ID
35+
VariationKeyToIDMap map[string]string
3536
TrafficAllocation []Range
3637
GroupID string
3738
AudienceConditionTree *TreeNode

‎tests/integration/models/constants.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ const (
6161
Activate SDKAPI = "activate"
6262
// Track - the api type is Track
6363
Track SDKAPI = "track"
64+
// SetForcedVariation - the api type is SetForcedVariation
65+
SetForcedVariation SDKAPI = "set_forced_variation"
66+
// GetForcedVariation - the api type is GetForcedVariation
67+
GetForcedVariation SDKAPI = "get_forced_variation"
6468
)
6569

6670
// KeyListenerCalled - Key for listener called

‎tests/integration/models/forced_variation_params.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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 models
18+
19+
// ForcedVariationRequestParams represents params required for Get and Set ForcedVariation API
20+
type ForcedVariationRequestParams struct {
21+
ExperimentKey string `yaml:"experiment_key"`
22+
UserID string `yaml:"user_id"`
23+
VariationKey string `yaml:"forced_variation_key,omitempty"`
24+
}

‎tests/integration/optlyplugins/userprofileservice/utils.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,9 @@ func CreateUserProfileService(config pkg.ProjectConfig, apiOptions models.APIOpt
5454
for experimentKey, variationKey := range bucketMap {
5555
if experiment, err := config.GetExperimentByKey(experimentKey); err == nil {
5656
decisionKey := decision.NewUserDecisionKey(experiment.ID)
57-
for _, variation := range experiment.Variations {
58-
if variation.Key == variationKey {
57+
if variationID, ok := experiment.VariationKeyToIDMap[variationKey]; ok {
58+
if variation, ok := experiment.Variations[variationID]; ok {
5959
profile.ExperimentBucketMap[decisionKey] = variation.ID
60-
break
6160
}
6261
}
6362
}

0 commit comments

Comments
 (0)