From a953b45e0e5456e797c32f3badae811ec513722c Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 28 Nov 2019 15:40:19 +0500 Subject: [PATCH 01/11] Fv implementation for gherkin. --- pkg/client/client_test.go | 110 +++++++++--------- pkg/client/fixtures_test.go | 8 +- .../mappers/experiment.go | 6 +- .../mappers/experiment_test.go | 19 ++- .../mappers/rollout_test.go | 4 +- pkg/decision/bucketer/experiment_bucketer.go | 2 +- .../bucketer/experiment_bucketer_test.go | 6 +- .../composite_experiment_service_test.go | 4 +- pkg/decision/experiment_override_service.go | 64 +++++++--- .../experiment_override_service_test.go | 53 ++++++++- pkg/decision/experiment_whitelist_service.go | 12 +- .../feature_experiment_service_test.go | 4 +- pkg/decision/helpers_test.go | 37 ++++-- pkg/decision/persisting_experiment_service.go | 2 +- .../persisting_experiment_service_test.go | 2 +- pkg/entities/experiment.go | 3 +- tests/integration/models/constants.go | 4 + .../optlyplugins/userprofileservice/utils.go | 7 +- tests/integration/support/client_wrapper.go | 47 +++++++- tests/integration/support/utils.go | 9 +- 20 files changed, 279 insertions(+), 124 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index f3acfce11..0abebb9a4 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -192,8 +192,8 @@ func TestGetFeatureVariableBooleanWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -238,8 +238,8 @@ func TestGetFeatureVariableBooleanWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -285,8 +285,8 @@ func TestGetFeatureVariableBooleanWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -332,8 +332,8 @@ func TestGetFeatureVariableBooleanWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -379,8 +379,8 @@ func TestGetFeatureVariableBooleanReturnsDefaultValueIfFeatureNotEnabled(t *test } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -444,8 +444,8 @@ func TestGetFeatureVariableDoubleWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -490,8 +490,8 @@ func TestGetFeatureVariableDoubleWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -537,8 +537,8 @@ func TestGetFeatureVariableDoubleWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -584,8 +584,8 @@ func TestGetFeatureVariableDoubleWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -631,8 +631,8 @@ func TestGetFeatureVariableDoubleReturnsDefaultValueIfFeatureNotEnabled(t *testi } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -696,8 +696,8 @@ func TestGetFeatureVariableIntegerWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -742,8 +742,8 @@ func TestGetFeatureVariableIntegerWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -789,8 +789,8 @@ func TestGetFeatureVariableIntegerWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -836,8 +836,8 @@ func TestGetFeatureVariableIntegerWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -883,8 +883,8 @@ func TestGetFeatureVariableIntegerReturnsDefaultValueIfFeatureNotEnabled(t *test } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -948,8 +948,8 @@ func TestGetFeatureVariableStringWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -994,8 +994,8 @@ func TestGetFeatureVariableStringWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1041,8 +1041,8 @@ func TestGetFeatureVariableStringWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1088,8 +1088,8 @@ func TestGetFeatureVariableStringReturnsDefaultValueIfFeatureNotEnabled(t *testi } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1190,8 +1190,8 @@ func TestGetFeatureDecisionValid(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1235,8 +1235,8 @@ func TestGetFeatureDecisionErrProjectConfig(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1279,8 +1279,8 @@ func TestGetFeatureDecisionPanicProjectConfig(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1322,8 +1322,8 @@ func TestGetFeatureDecisionPanicDecisionService(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1357,8 +1357,8 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1403,8 +1403,8 @@ func TestGetAllFeatureVariables(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testVariation.FeatureEnabled = true testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) testFeature.VariableMap = map[string]entities.Variable{testVariable.Key: testVariable} @@ -1449,8 +1449,8 @@ func TestGetAllFeatureVariablesWithError(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - Variations: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) testFeature.VariableMap = map[string]entities.Variable{testVariable.Key: testVariable} @@ -1562,7 +1562,7 @@ func (s *ClientTestSuiteAB) TestActivate() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.Variations["v2"] + expectedVariation := testExperiment.VariationsIDMap["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } @@ -1628,7 +1628,7 @@ func (s *ClientTestSuiteAB) TestGetVariation() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.Variations["v2"] + expectedVariation := testExperiment.VariationsIDMap["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } @@ -1657,7 +1657,7 @@ func (s *ClientTestSuiteAB) TestGetVariationWithDecisionError() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.Variations["v2"] + expectedVariation := testExperiment.VariationsIDMap["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } diff --git a/pkg/client/fixtures_test.go b/pkg/client/fixtures_test.go index 143a0f9a3..7741dfa36 100644 --- a/pkg/client/fixtures_test.go +++ b/pkg/client/fixtures_test.go @@ -161,7 +161,7 @@ type MockUserProfileService struct { func makeTestExperiment(experimentKey string) entities.Experiment { return entities.Experiment{ Key: experimentKey, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ "v1": entities.Variation{Key: "v1"}, "v2": entities.Variation{Key: "v2"}, }, @@ -182,9 +182,9 @@ func makeTestExperimentWithVariations(experimentKey string, variations []entitie variationsMap[variation.ID] = variation } return entities.Experiment{ - Key: experimentKey, - ID: fmt.Sprintf("test_experiment_%s", experimentKey), - Variations: variationsMap, + Key: experimentKey, + ID: fmt.Sprintf("test_experiment_%s", experimentKey), + VariationsIDMap: variationsMap, } } diff --git a/pkg/config/datafileprojectconfig/mappers/experiment.go b/pkg/config/datafileprojectconfig/mappers/experiment.go index 1a4ed1e33..1e57e2f9b 100644 --- a/pkg/config/datafileprojectconfig/mappers/experiment.go +++ b/pkg/config/datafileprojectconfig/mappers/experiment.go @@ -84,7 +84,8 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen ID: rawExperiment.ID, LayerID: rawExperiment.LayerID, Key: rawExperiment.Key, - Variations: make(map[string]entities.Variation), + VariationsIDMap: make(map[string]entities.Variation), + VariationsKeyMap: make(map[string]entities.Variation), TrafficAllocation: make([]entities.Range, len(rawExperiment.TrafficAllocation)), AudienceConditionTree: audienceConditionTree, Whitelist: rawExperiment.ForcedVariations, @@ -92,7 +93,8 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen } for _, variation := range rawExperiment.Variations { - experiment.Variations[variation.ID] = mapVariation(variation) + experiment.VariationsIDMap[variation.ID] = mapVariation(variation) + experiment.VariationsKeyMap[variation.Key] = experiment.VariationsIDMap[variation.ID] } for i, allocation := range rawExperiment.TrafficAllocation { diff --git a/pkg/config/datafileprojectconfig/mappers/experiment_test.go b/pkg/config/datafileprojectconfig/mappers/experiment_test.go index 8d42ffa50..277559a65 100644 --- a/pkg/config/datafileprojectconfig/mappers/experiment_test.go +++ b/pkg/config/datafileprojectconfig/mappers/experiment_test.go @@ -74,7 +74,7 @@ func TestMapExperiments(t *testing.T) { ID: "11111", GroupID: "15", Key: "test_experiment_11111", - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ "21111": { ID: "21111", Variables: map[string]entities.VariationVariable{"1": entities.VariationVariable{ID: "1", Value: "1"}}, @@ -88,6 +88,20 @@ func TestMapExperiments(t *testing.T) { FeatureEnabled: false, }, }, + VariationsKeyMap: map[string]entities.Variation{ + "variation_1": { + ID: "21111", + Variables: map[string]entities.VariationVariable{"1": entities.VariationVariable{ID: "1", Value: "1"}}, + Key: "variation_1", + FeatureEnabled: true, + }, + "variation_2": { + ID: "21112", + Variables: map[string]entities.VariationVariable{"2": entities.VariationVariable{ID: "2", Value: "2"}}, + Key: "variation_2", + FeatureEnabled: false, + }, + }, TrafficAllocation: []entities.Range{ { EntityID: "21111", @@ -136,7 +150,8 @@ func TestMapExperimentsWithStringAudienceCondition(t *testing.T) { ID: "11111", GroupID: "15", Key: "test_experiment_11111", - Variations: map[string]entities.Variation{}, + VariationsIDMap: map[string]entities.Variation{}, + VariationsKeyMap: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}, AudienceConditionTree: &entities.TreeNode{ Operator: "or", diff --git a/pkg/config/datafileprojectconfig/mappers/rollout_test.go b/pkg/config/datafileprojectconfig/mappers/rollout_test.go index f87b6fddf..f9f1a9c9e 100644 --- a/pkg/config/datafileprojectconfig/mappers/rollout_test.go +++ b/pkg/config/datafileprojectconfig/mappers/rollout_test.go @@ -42,8 +42,8 @@ func TestMapRollouts(t *testing.T) { "21111": entities.Rollout{ ID: "21111", Experiments: []entities.Experiment{ - entities.Experiment{ID: "11111", Key: "exp_11111", Variations: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, - entities.Experiment{ID: "11112", Key: "exp_11112", Variations: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, + entities.Experiment{ID: "11111", Key: "exp_11111", VariationsIDMap: map[string]entities.Variation{}, VariationsKeyMap: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, + entities.Experiment{ID: "11112", Key: "exp_11112", VariationsIDMap: map[string]entities.Variation{}, VariationsKeyMap: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, }, }, } diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index c0fbe501e..3ab26de57 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -57,7 +57,7 @@ func (b MurmurhashExperimentBucketer) Bucket(bucketingID string, experiment enti return nil, reasons.NotBucketedIntoVariation, nil } - if variation, ok := experiment.Variations[bucketedVariationID]; ok { + if variation, ok := experiment.VariationsIDMap[bucketedVariationID]; ok { return &variation, reasons.BucketedIntoVariation, nil } diff --git a/pkg/decision/bucketer/experiment_bucketer_test.go b/pkg/decision/bucketer/experiment_bucketer_test.go index 7c486cb3a..5fde83f94 100644 --- a/pkg/decision/bucketer/experiment_bucketer_test.go +++ b/pkg/decision/bucketer/experiment_bucketer_test.go @@ -13,7 +13,7 @@ func TestBucketExclusionGroups(t *testing.T) { experiment1 := entities.Experiment{ ID: "1886780721", Key: "experiment_1", - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ "22222": entities.Variation{ID: "22222", Key: "exp_1_var_1"}, "22223": entities.Variation{ID: "22223", Key: "exp_1_var_2"}, }, @@ -26,7 +26,7 @@ func TestBucketExclusionGroups(t *testing.T) { experiment2 := entities.Experiment{ ID: "1886780723", Key: "experiment_2", - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ "22224": entities.Variation{ID: "22224", Key: "exp_2_var_1"}, "22225": entities.Variation{ID: "22225", Key: "exp_2_var_2"}, }, @@ -49,7 +49,7 @@ func TestBucketExclusionGroups(t *testing.T) { bucketer := NewMurmurhashExperimentBucketer(DefaultHashSeed) // ppid2 + 1886780722 (groupId) will generate bucket value of 2434 which maps to experiment 1 bucketedVariation, reason, _ := bucketer.Bucket("ppid2", experiment1, exclusionGroup) - assert.Equal(t, experiment1.Variations["22222"], *bucketedVariation) + assert.Equal(t, experiment1.VariationsIDMap["22222"], *bucketedVariation) assert.Equal(t, reasons.BucketedIntoVariation, reason) // since the bucket value maps to experiment 1, the user will not be bucketed for experiment 2 bucketedVariation, reason, _ = bucketer.Bucket("ppid2", experiment2, exclusionGroup) diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index 59210b8c6..97e43de21 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -51,7 +51,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecision() { ID: "test_user_1", } - expectedVariation := testExp1111.Variations["2222"] + expectedVariation := testExp1111.VariationsIDMap["2222"] expectedExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } @@ -74,7 +74,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { ID: "test_user_1", } - expectedVariation := testExp1111.Variations["2222"] + expectedVariation := testExp1111.VariationsIDMap["2222"] expectedExperimentDecision := ExperimentDecision{} s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext).Return(expectedExperimentDecision, nil) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index c327f7d75..256d5c228 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -20,8 +20,11 @@ package decision import ( "errors" "fmt" + "strings" "sync" + "github.com/optimizely/go-sdk/pkg" + "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" @@ -40,17 +43,33 @@ type ExperimentOverrideStore interface { GetVariation(overrideKey ExperimentOverrideKey) (string, bool) } +// MEOptionFunc is used to pass custom config options into the MapExperimentOverridesStore. +type MEOptionFunc func(*MapExperimentOverridesStore) + // MapExperimentOverridesStore is a map-based implementation of ExperimentOverridesStore that is safe to use concurrently type MapExperimentOverridesStore struct { overridesMap map[ExperimentOverrideKey]string + config pkg.ProjectConfig mutex sync.RWMutex } +// WithConfig sets the config on the MapExperimentOverridesStore +func WithConfig(config pkg.ProjectConfig) MEOptionFunc { + return func(f *MapExperimentOverridesStore) { + f.config = config + } +} + // NewMapExperimentOverridesStore returns a new MapExperimentOverridesStore -func NewMapExperimentOverridesStore() *MapExperimentOverridesStore { - return &MapExperimentOverridesStore{ +func NewMapExperimentOverridesStore(options ...MEOptionFunc) *MapExperimentOverridesStore { + overrideStore := &MapExperimentOverridesStore{ overridesMap: make(map[ExperimentOverrideKey]string), } + + for _, opts := range options { + opts(overrideStore) + } + return overrideStore } // GetVariation returns the override variation key associated with the given user+experiment key @@ -62,10 +81,29 @@ func (m *MapExperimentOverridesStore) GetVariation(overrideKey ExperimentOverrid } // SetVariation sets the given variation key as an override for the given user+experiment key -func (m *MapExperimentOverridesStore) SetVariation(overrideKey ExperimentOverrideKey, variationKey string) { - m.mutex.Lock() - m.overridesMap[overrideKey] = variationKey - m.mutex.Unlock() +func (m *MapExperimentOverridesStore) SetVariation(overrideKey ExperimentOverrideKey, variationKey string) bool { + + assignVariationKey := func() { + m.mutex.Lock() + m.overridesMap[overrideKey] = variationKey + m.mutex.Unlock() + } + // Assign directly if no config provided + if m.config == nil { + assignVariationKey() + return true + } + // Check if experiment and variation exist + if experiment, err := m.config.GetExperimentByKey(overrideKey.ExperimentKey); err == nil { + if strings.TrimSpace(variationKey) == "" { + return false + } + if _, ok := experiment.VariationsKeyMap[variationKey]; ok { + assignVariationKey() + return true + } + } + return false } // RemoveVariation removes the override variation key associated with the argument user+experiment key. @@ -103,15 +141,11 @@ func (s ExperimentOverrideService) GetDecision(decisionContext ExperimentDecisio return decision, nil } - // TODO(Matt): Implement and use a way to access variations by key - for _, variation := range decisionContext.Experiment.Variations { - variation := variation - if variation.Key == variationKey { - decision.Variation = &variation - decision.Reason = reasons.OverrideVariationAssignmentFound - eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) - return decision, nil - } + if variation, ok := decisionContext.Experiment.VariationsKeyMap[variationKey]; ok { + decision.Variation = &variation + decision.Reason = reasons.OverrideVariationAssignmentFound + eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) + return decision, nil } decision.Reason = reasons.InvalidOverrideVariationAssignment diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index 8f930008c..ab0d82604 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -18,6 +18,7 @@ package decision import ( + "fmt" "sync" "testing" @@ -28,15 +29,24 @@ import ( type ExperimentOverrideServiceTestSuite struct { suite.Suite - mockConfig *mockProjectConfig - overrides *MapExperimentOverridesStore - overrideService *ExperimentOverrideService + mockConfig *mockProjectConfig + overrides *MapExperimentOverridesStore + overrideService *ExperimentOverrideService + overridesWithConfig *MapExperimentOverridesStore + overrideServiceWithConfig *ExperimentOverrideService } func (s *ExperimentOverrideServiceTestSuite) SetupTest() { - s.mockConfig = new(mockProjectConfig) + config := new(mockProjectConfig) + s.mockConfig = config + s.mockConfig.On("GetExperimentByKey", testExp1111.Key).Return(testExp1111, nil) + s.mockConfig.On("GetExperimentByKey", testExp1113.Key).Return(testExp1113, nil) + s.mockConfig.On("GetExperimentByKey", "").Return(entities.Experiment{}, fmt.Errorf("")) + s.overrides = NewMapExperimentOverridesStore() s.overrideService = NewExperimentOverrideService(s.overrides) + s.overridesWithConfig = NewMapExperimentOverridesStore(WithConfig(config)) + s.overrideService = NewExperimentOverrideService(s.overridesWithConfig) } func (s *ExperimentOverrideServiceTestSuite) TestOverridesIncludeVariation() { @@ -116,6 +126,9 @@ func (s *ExperimentOverrideServiceTestSuite) TestNoOverrideForUserOrExperiment() } func (s *ExperimentOverrideServiceTestSuite) TestInvalidVariationInOverride() { + overrides := NewMapExperimentOverridesStore() + overrideService := NewExperimentOverrideService(overrides) + testDecisionContext := ExperimentDecisionContext{ Experiment: &testExp1111, ProjectConfig: s.mockConfig, @@ -124,13 +137,41 @@ func (s *ExperimentOverrideServiceTestSuite) TestInvalidVariationInOverride() { ID: "test_user_1", } // This override variation key does not exist in the experiment - s.overrides.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, "invalid_variation_key") - decision, err := s.overrideService.GetDecision(testDecisionContext, testUserContext) + overrides.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, "invalid_variation_key") + decision, err := overrideService.GetDecision(testDecisionContext, testUserContext) s.NoError(err) s.Nil(decision.Variation) s.Exactly(reasons.InvalidOverrideVariationAssignment, decision.Reason) } +// Test setVariation with projectconfig in overridestore +func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithInvalidExperimentKey() { + overrideKey := ExperimentOverrideKey{ExperimentKey: "", UserID: "test_user_1"} + success := s.overridesWithConfig.SetVariation(overrideKey, testExp1111Var2222.Key) + s.False(success) + variation, success := s.overridesWithConfig.GetVariation(overrideKey) + s.Exactly("", variation) + s.False(success) +} + +func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithInvalidVariationKey() { + overrideKey := ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"} + success := s.overridesWithConfig.SetVariation(overrideKey, "") + s.False(success) + variation, success := s.overridesWithConfig.GetVariation(overrideKey) + s.Exactly("", variation) + s.False(success) +} + +func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithValidVariationKeyAndExperimentKey() { + overrideKey := ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"} + success := s.overridesWithConfig.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, testExp1111Var2222.Key) + s.True(success) + variation, success := s.overridesWithConfig.GetVariation(overrideKey) + s.Exactly(testExp1111Var2222.Key, variation) + s.True(success) +} + // Test concurrent use of the MapExperimentOverrideStore // Create 3 goroutines that set and get variations, and assert that all their sets take effect at the end func (s *ExperimentOverrideServiceTestSuite) TestMapExperimentOverridesStoreConcurrent() { diff --git a/pkg/decision/experiment_whitelist_service.go b/pkg/decision/experiment_whitelist_service.go index f260f22bc..cf6b08a5c 100644 --- a/pkg/decision/experiment_whitelist_service.go +++ b/pkg/decision/experiment_whitelist_service.go @@ -47,14 +47,10 @@ func (s ExperimentWhitelistService) GetDecision(decisionContext ExperimentDecisi return decision, nil } - // TODO(Matt): Add a VariationsByKey map to the Experiment struct, and use it to look up Variation by key - for _, variation := range decisionContext.Experiment.Variations { - variation := variation - if variation.Key == variationKey { - decision.Reason = reasons.WhitelistVariationAssignmentFound - decision.Variation = &variation - return decision, nil - } + if variation, ok := decisionContext.Experiment.VariationsKeyMap[variationKey]; ok { + decision.Reason = reasons.WhitelistVariationAssignmentFound + decision.Variation = &variation + return decision, nil } decision.Reason = reasons.InvalidWhitelistVariationAssignment diff --git a/pkg/decision/feature_experiment_service_test.go b/pkg/decision/feature_experiment_service_test.go index 5b6b7a6bb..db7d7a633 100644 --- a/pkg/decision/feature_experiment_service_test.go +++ b/pkg/decision/feature_experiment_service_test.go @@ -45,7 +45,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecision() { ID: "test_user_1", } - expectedVariation := testExp1113.Variations["2223"] + expectedVariation := testExp1113.VariationsIDMap["2223"] returnExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } @@ -84,7 +84,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionMutex() { s.mockExperimentService.On("GetDecision", testExperimentDecisionContext1, testUserContext).Return(nilDecision, nil) // second experiment returns a valid decision to simulate user being bucketed into this experiment in the group - expectedVariation := testExp1114.Variations["2225"] + expectedVariation := testExp1114.VariationsIDMap["2225"] returnExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } diff --git a/pkg/decision/helpers_test.go b/pkg/decision/helpers_test.go index a899afb85..a3669e674 100644 --- a/pkg/decision/helpers_test.go +++ b/pkg/decision/helpers_test.go @@ -101,7 +101,10 @@ var testExp1111Var2222 = entities.Variation{ID: "2222", Key: "2222"} var testExp1111 = entities.Experiment{ ID: "1111", Key: testExp1111Key, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2222": testExp1111Var2222, + }, + VariationsKeyMap: map[string]entities.Variation{ "2222": testExp1111Var2222, }, TrafficAllocation: []entities.Range{ @@ -137,7 +140,10 @@ var testExp1112 = entities.Experiment{ }, ID: "1112", Key: testExp1111Key, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2222": testExp1111Var2222, + }, + VariationsKeyMap: map[string]entities.Variation{ "2222": testExp1111Var2222, }, TrafficAllocation: []entities.Range{ @@ -168,7 +174,11 @@ var testExp1113 = entities.Experiment{ ID: "1113", Key: testExp1113Key, GroupID: "6666", - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2223": testExp1113Var2223, + "2224": testExp1113Var2224, + }, + VariationsKeyMap: map[string]entities.Variation{ "2223": testExp1113Var2223, "2224": testExp1113Var2224, }, @@ -186,7 +196,11 @@ var testExp1114 = entities.Experiment{ ID: "1114", Key: testExp1114Key, GroupID: "6666", - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2225": testExp1114Var2225, + "2226": testExp1114Var2226, + }, + VariationsKeyMap: map[string]entities.Variation{ "2225": testExp1114Var2225, "2226": testExp1114Var2226, }, @@ -211,7 +225,10 @@ var testExp1115Var2227 = entities.Variation{ID: "2227", Key: "2227", FeatureEnab var testExp1115 = entities.Experiment{ ID: "1115", Key: testExp1115Key, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2227": testExp1115Var2227, + }, + VariationsKeyMap: map[string]entities.Variation{ "2227": testExp1115Var2227, }, TrafficAllocation: []entities.Range{ @@ -236,7 +253,10 @@ var testTargetedExp1116 = entities.Experiment{ AudienceConditionTree: &entities.TreeNode{Operator: "or", Item: "7771"}, ID: "1116", Key: testTargetedExp1116Key, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ + "2228": testTargetedExp1116Var2228, + }, + VariationsKeyMap: map[string]entities.Variation{ "2228": testTargetedExp1116Var2228, }, TrafficAllocation: []entities.Range{ @@ -251,9 +271,12 @@ var testExpWhitelistVar2229 = entities.Variation{ID: "2229", Key: "var_2229"} var testExpWhitelist = entities.Experiment{ ID: "1117", Key: testExpWhitelistKey, - Variations: map[string]entities.Variation{ + VariationsIDMap: map[string]entities.Variation{ "2229": testExpWhitelistVar2229, }, + VariationsKeyMap: map[string]entities.Variation{ + "var_2229": testExpWhitelistVar2229, + }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2229", EndOfRange: 10000}, }, diff --git a/pkg/decision/persisting_experiment_service.go b/pkg/decision/persisting_experiment_service.go index 482b935b7..389fe2b1f 100644 --- a/pkg/decision/persisting_experiment_service.go +++ b/pkg/decision/persisting_experiment_service.go @@ -78,7 +78,7 @@ func (p PersistingExperimentService) getSavedDecision(decisionContext Experiment } if savedVariationID, ok := userProfile.ExperimentBucketMap[decisionKey]; ok { - if variation, ok := decisionContext.Experiment.Variations[savedVariationID]; ok { + if variation, ok := decisionContext.Experiment.VariationsIDMap[savedVariationID]; ok { experimentDecision.Variation = &variation pesLogger.Debug(fmt.Sprintf(`User "%s" was previously bucketed into variation "%s" of experiment "%s".`, userContext.ID, variation.Key, decisionContext.Experiment.Key)) } else { diff --git a/pkg/decision/persisting_experiment_service_test.go b/pkg/decision/persisting_experiment_service_test.go index 434fafa60..a6966cd7a 100644 --- a/pkg/decision/persisting_experiment_service_test.go +++ b/pkg/decision/persisting_experiment_service_test.go @@ -47,7 +47,7 @@ func (s *PersistingExperimentServiceTestSuite) SetupTest() { ProjectConfig: s.mockProjectConfig, } - computedVariation := testExp1113.Variations["2223"] + computedVariation := testExp1113.VariationsIDMap["2223"] s.testComputedDecision = ExperimentDecision{ Variation: &computedVariation, } diff --git a/pkg/entities/experiment.go b/pkg/entities/experiment.go index 3eeebf5b7..eadb5452d 100644 --- a/pkg/entities/experiment.go +++ b/pkg/entities/experiment.go @@ -31,7 +31,8 @@ type Experiment struct { ID string LayerID string Key string - Variations map[string]Variation // keyed by variation ID + VariationsIDMap map[string]Variation // keyed by variation ID + VariationsKeyMap map[string]Variation // keyed by variation Key TrafficAllocation []Range GroupID string AudienceConditionTree *TreeNode diff --git a/tests/integration/models/constants.go b/tests/integration/models/constants.go index 3fcf81454..5bd17f49c 100644 --- a/tests/integration/models/constants.go +++ b/tests/integration/models/constants.go @@ -61,6 +61,10 @@ const ( Activate SDKAPI = "activate" // Track - the api type is Track Track SDKAPI = "track" + // SetForcedVariation - the api type is SetForcedVariation + SetForcedVariation SDKAPI = "set_forced_variation" + // GetForcedVariation - the api type is GetForcedVariation + GetForcedVariation SDKAPI = "get_forced_variation" ) // KeyListenerCalled - Key for listener called diff --git a/tests/integration/optlyplugins/userprofileservice/utils.go b/tests/integration/optlyplugins/userprofileservice/utils.go index ecc6cc1d2..c76030177 100644 --- a/tests/integration/optlyplugins/userprofileservice/utils.go +++ b/tests/integration/optlyplugins/userprofileservice/utils.go @@ -54,11 +54,8 @@ func CreateUserProfileService(config pkg.ProjectConfig, apiOptions models.APIOpt for experimentKey, variationKey := range bucketMap { if experiment, err := config.GetExperimentByKey(experimentKey); err == nil { decisionKey := decision.NewUserDecisionKey(experiment.ID) - for _, variation := range experiment.Variations { - if variation.Key == variationKey { - profile.ExperimentBucketMap[decisionKey] = variation.ID - break - } + if variation, ok := experiment.VariationsKeyMap[variationKey]; ok { + profile.ExperimentBucketMap[decisionKey] = variation.ID } } } diff --git a/tests/integration/support/client_wrapper.go b/tests/integration/support/client_wrapper.go index 10f83f5e7..69b790526 100644 --- a/tests/integration/support/client_wrapper.go +++ b/tests/integration/support/client_wrapper.go @@ -43,6 +43,7 @@ type ClientWrapper struct { DecisionService decision.Service EventDispatcher event.Dispatcher UserProfileService decision.UserProfileService + OverrideStore decision.ExperimentOverrideStore } // DeleteInstance deletes cached instance of optly wrapper @@ -84,10 +85,13 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { log.Fatal(err) } + overrideService := decision.NewMapExperimentOverridesStore(decision.WithConfig(config)) userProfileService := userprofileservice.CreateUserProfileService(config, apiOptions) compositeExperimentService := decision.NewCompositeExperimentService( decision.WithUserProfileService(userProfileService), + decision.WithOverrideStore(overrideService), ) + // @TODO: Add sdkKey dynamically once event-batching support is implemented compositeService := *decision.NewCompositeService("", decision.WithCompositeExperimentService(compositeExperimentService)) decisionService := &optlyplugins.TestCompositeService{CompositeService: compositeService} @@ -95,7 +99,8 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { client, err := optimizelyFactory.Client( client.WithConfigManager(configManager), client.WithDecisionService(decisionService), - client.WithEventProcessor(eventProcessor)) + client.WithEventProcessor(eventProcessor), + ) if err != nil { log.Fatal(err) } @@ -105,6 +110,7 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { DecisionService: decisionService, EventDispatcher: eventProcessor.EventDispatcher, UserProfileService: userProfileService, + OverrideStore: overrideService, } return clientInstance } @@ -147,6 +153,12 @@ func (c *ClientWrapper) InvokeAPI(request models.APIOptions) (models.APIResponse case models.Track: response, err = c.track(request) break + case models.SetForcedVariation: + response, err = c.setForcedVariation(request) + break + case models.GetForcedVariation: + response, err = c.getForcedVariation(request) + break default: break } @@ -328,3 +340,36 @@ func (c *ClientWrapper) track(request models.APIOptions) (models.APIResponse, er response.Result = "NULL" return response, err } + +func (c *ClientWrapper) setForcedVariation(request models.APIOptions) (models.APIResponse, error) { + var params models.ForcedVariationRequestParams + var response models.APIResponse + err := yaml.Unmarshal([]byte(request.Arguments), ¶ms) + response.Result = "NULL" + if err == nil { + // For removeForcedVariation cases + if params.VariationKey == "" { + c.OverrideStore.(*decision.MapExperimentOverridesStore).RemoveVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}) + } else { + result := c.OverrideStore.(*decision.MapExperimentOverridesStore).SetVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}, params.VariationKey) + if result { + response.Result = params.VariationKey + } + } + } + return response, err +} + +func (c *ClientWrapper) getForcedVariation(request models.APIOptions) (models.APIResponse, error) { + var params models.ForcedVariationRequestParams + var response models.APIResponse + err := yaml.Unmarshal([]byte(request.Arguments), ¶ms) + response.Result = "NULL" + if err == nil { + variation, success := c.OverrideStore.(*decision.MapExperimentOverridesStore).GetVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}) + if success { + response.Result = variation + } + } + return response, err +} diff --git a/tests/integration/support/utils.go b/tests/integration/support/utils.go index d1b5ea130..d8cf95dbc 100644 --- a/tests/integration/support/utils.go +++ b/tests/integration/support/utils.go @@ -101,12 +101,9 @@ func parseTemplate(s string, config pkg.ProjectConfig) string { if len(matches) > 1 { expVarKey := strings.Split(matches[1], ".") if exp, err := config.GetExperimentByKey(expVarKey[0]); err == nil { - for _, variation := range exp.Variations { - if variation.Key == expVarKey[1] { - parsedString = strings.Replace(parsedString, matches[0], variation.ID, -1) - replaceVariableID() - break - } + if variation, ok := exp.VariationsKeyMap[expVarKey[1]]; ok { + parsedString = strings.Replace(parsedString, matches[0], variation.ID, -1) + replaceVariableID() } } } From 48473e62bd63324812056cf9d9dbfbc771c019f6 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 28 Nov 2019 16:07:13 +0500 Subject: [PATCH 02/11] fixes. --- pkg/decision/experiment_override_service_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index ab0d82604..07b0aacad 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -46,7 +46,7 @@ func (s *ExperimentOverrideServiceTestSuite) SetupTest() { s.overrides = NewMapExperimentOverridesStore() s.overrideService = NewExperimentOverrideService(s.overrides) s.overridesWithConfig = NewMapExperimentOverridesStore(WithConfig(config)) - s.overrideService = NewExperimentOverrideService(s.overridesWithConfig) + s.overrideServiceWithConfig = NewExperimentOverrideService(s.overridesWithConfig) } func (s *ExperimentOverrideServiceTestSuite) TestOverridesIncludeVariation() { From a3af43930defa1be077fcd5ee0eeaa9838a8da03 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 28 Nov 2019 16:26:19 +0500 Subject: [PATCH 03/11] Adding missing files. --- .../models/forced_variation_params.go | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 tests/integration/models/forced_variation_params.go diff --git a/tests/integration/models/forced_variation_params.go b/tests/integration/models/forced_variation_params.go new file mode 100644 index 000000000..a6e2b5a88 --- /dev/null +++ b/tests/integration/models/forced_variation_params.go @@ -0,0 +1,24 @@ +/**************************************************************************** + * Copyright 2019, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +package models + +// ForcedVariationRequestParams represents params required for Get and Set ForcedVariation API +type ForcedVariationRequestParams struct { + ExperimentKey string `yaml:"experiment_key"` + UserID string `yaml:"user_id"` + VariationKey string `yaml:"forced_variation_key,omitempty"` +} From 0f784b46e70e03379b4ef7d9436d758a54c0b119 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Tue, 3 Dec 2019 00:02:12 +0500 Subject: [PATCH 04/11] Changed variationIdMap to previous implementation and added VariationsKeyToIDMap. --- pkg/client/client_test.go | 64 +++++++++---------- pkg/client/fixtures_test.go | 4 +- .../mappers/experiment.go | 8 +-- .../mappers/experiment_test.go | 32 ++++------ .../mappers/rollout_test.go | 4 +- pkg/decision/bucketer/experiment_bucketer.go | 2 +- .../bucketer/experiment_bucketer_test.go | 6 +- .../composite_experiment_service_test.go | 4 +- pkg/decision/experiment_override_service.go | 20 +++--- pkg/decision/experiment_whitelist_service.go | 10 +-- .../feature_experiment_service_test.go | 4 +- pkg/decision/helpers_test.go | 46 ++++++------- pkg/decision/persisting_experiment_service.go | 2 +- .../persisting_experiment_service_test.go | 2 +- pkg/entities/experiment.go | 4 +- .../optlyplugins/userprofileservice/utils.go | 6 +- tests/integration/support/utils.go | 8 ++- 17 files changed, 113 insertions(+), 113 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 0abebb9a4..ccdc816f2 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -192,8 +192,8 @@ func TestGetFeatureVariableBooleanWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -238,8 +238,8 @@ func TestGetFeatureVariableBooleanWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -285,8 +285,8 @@ func TestGetFeatureVariableBooleanWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + ID: "111111", + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -333,7 +333,7 @@ func TestGetFeatureVariableBooleanWithEmptyValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -380,7 +380,7 @@ func TestGetFeatureVariableBooleanReturnsDefaultValueIfFeatureNotEnabled(t *test testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -445,7 +445,7 @@ func TestGetFeatureVariableDoubleWithValidValue(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -491,7 +491,7 @@ func TestGetFeatureVariableDoubleWithInvalidValue(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -538,7 +538,7 @@ func TestGetFeatureVariableDoubleWithInvalidValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -585,7 +585,7 @@ func TestGetFeatureVariableDoubleWithEmptyValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -632,7 +632,7 @@ func TestGetFeatureVariableDoubleReturnsDefaultValueIfFeatureNotEnabled(t *testi testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -697,7 +697,7 @@ func TestGetFeatureVariableIntegerWithValidValue(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -743,7 +743,7 @@ func TestGetFeatureVariableIntegerWithInvalidValue(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -790,7 +790,7 @@ func TestGetFeatureVariableIntegerWithInvalidValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -837,7 +837,7 @@ func TestGetFeatureVariableIntegerWithEmptyValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -884,7 +884,7 @@ func TestGetFeatureVariableIntegerReturnsDefaultValueIfFeatureNotEnabled(t *test testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -949,7 +949,7 @@ func TestGetFeatureVariableStringWithValidValue(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -995,7 +995,7 @@ func TestGetFeatureVariableStringWithInvalidValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1042,7 +1042,7 @@ func TestGetFeatureVariableStringWithEmptyValueType(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1089,7 +1089,7 @@ func TestGetFeatureVariableStringReturnsDefaultValueIfFeatureNotEnabled(t *testi testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1191,7 +1191,7 @@ func TestGetFeatureDecisionValid(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1236,7 +1236,7 @@ func TestGetFeatureDecisionErrProjectConfig(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1280,7 +1280,7 @@ func TestGetFeatureDecisionPanicProjectConfig(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1323,7 +1323,7 @@ func TestGetFeatureDecisionPanicDecisionService(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1358,7 +1358,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) mockConfig := getMockConfig(testFeatureKey, testVariableKey, testFeature, testVariable) @@ -1404,7 +1404,7 @@ func TestGetAllFeatureVariables(t *testing.T) { testVariation.FeatureEnabled = true testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) testFeature.VariableMap = map[string]entities.Variable{testVariable.Key: testVariable} @@ -1450,7 +1450,7 @@ func TestGetAllFeatureVariablesWithError(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ ID: "111111", - VariationsIDMap: map[string]entities.Variation{"22222": testVariation}, + Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) testFeature.VariableMap = map[string]entities.Variable{testVariable.Key: testVariable} @@ -1562,7 +1562,7 @@ func (s *ClientTestSuiteAB) TestActivate() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.VariationsIDMap["v2"] + expectedVariation := testExperiment.Variations["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } @@ -1628,7 +1628,7 @@ func (s *ClientTestSuiteAB) TestGetVariation() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.VariationsIDMap["v2"] + expectedVariation := testExperiment.Variations["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } @@ -1657,7 +1657,7 @@ func (s *ClientTestSuiteAB) TestGetVariationWithDecisionError() { ProjectConfig: s.mockConfig, } - expectedVariation := testExperiment.VariationsIDMap["v2"] + expectedVariation := testExperiment.Variations["v2"] expectedExperimentDecision := decision.ExperimentDecision{ Variation: &expectedVariation, } diff --git a/pkg/client/fixtures_test.go b/pkg/client/fixtures_test.go index 7741dfa36..b422065a8 100644 --- a/pkg/client/fixtures_test.go +++ b/pkg/client/fixtures_test.go @@ -161,7 +161,7 @@ type MockUserProfileService struct { func makeTestExperiment(experimentKey string) entities.Experiment { return entities.Experiment{ Key: experimentKey, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "v1": entities.Variation{Key: "v1"}, "v2": entities.Variation{Key: "v2"}, }, @@ -184,7 +184,7 @@ func makeTestExperimentWithVariations(experimentKey string, variations []entitie return entities.Experiment{ Key: experimentKey, ID: fmt.Sprintf("test_experiment_%s", experimentKey), - VariationsIDMap: variationsMap, + Variations: variationsMap, } } diff --git a/pkg/config/datafileprojectconfig/mappers/experiment.go b/pkg/config/datafileprojectconfig/mappers/experiment.go index 1e57e2f9b..e5cbd9c00 100644 --- a/pkg/config/datafileprojectconfig/mappers/experiment.go +++ b/pkg/config/datafileprojectconfig/mappers/experiment.go @@ -84,8 +84,8 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen ID: rawExperiment.ID, LayerID: rawExperiment.LayerID, Key: rawExperiment.Key, - VariationsIDMap: make(map[string]entities.Variation), - VariationsKeyMap: make(map[string]entities.Variation), + Variations: make(map[string]entities.Variation), + VariationKeyToIDMap: make(map[string]string), TrafficAllocation: make([]entities.Range, len(rawExperiment.TrafficAllocation)), AudienceConditionTree: audienceConditionTree, Whitelist: rawExperiment.ForcedVariations, @@ -93,8 +93,8 @@ func mapExperiment(rawExperiment datafileEntities.Experiment) entities.Experimen } for _, variation := range rawExperiment.Variations { - experiment.VariationsIDMap[variation.ID] = mapVariation(variation) - experiment.VariationsKeyMap[variation.Key] = experiment.VariationsIDMap[variation.ID] + experiment.Variations[variation.ID] = mapVariation(variation) + experiment.VariationKeyToIDMap[variation.Key] = variation.ID } for i, allocation := range rawExperiment.TrafficAllocation { diff --git a/pkg/config/datafileprojectconfig/mappers/experiment_test.go b/pkg/config/datafileprojectconfig/mappers/experiment_test.go index 277559a65..9bcc9abf3 100644 --- a/pkg/config/datafileprojectconfig/mappers/experiment_test.go +++ b/pkg/config/datafileprojectconfig/mappers/experiment_test.go @@ -74,7 +74,7 @@ func TestMapExperiments(t *testing.T) { ID: "11111", GroupID: "15", Key: "test_experiment_11111", - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "21111": { ID: "21111", Variables: map[string]entities.VariationVariable{"1": entities.VariationVariable{ID: "1", Value: "1"}}, @@ -88,19 +88,9 @@ func TestMapExperiments(t *testing.T) { FeatureEnabled: false, }, }, - VariationsKeyMap: map[string]entities.Variation{ - "variation_1": { - ID: "21111", - Variables: map[string]entities.VariationVariable{"1": entities.VariationVariable{ID: "1", Value: "1"}}, - Key: "variation_1", - FeatureEnabled: true, - }, - "variation_2": { - ID: "21112", - Variables: map[string]entities.VariationVariable{"2": entities.VariationVariable{ID: "2", Value: "2"}}, - Key: "variation_2", - FeatureEnabled: false, - }, + VariationKeyToIDMap: map[string]string{ + "variation_1": "21111", + "variation_2": "21112", }, TrafficAllocation: []entities.Range{ { @@ -146,13 +136,13 @@ func TestMapExperimentsWithStringAudienceCondition(t *testing.T) { experiments, experimentKeyMap := MapExperiments(rawExperiments, experimentGroupMap) expectedExperiments := map[string]entities.Experiment{ "11111": { - AudienceIds: []string{"31111"}, - ID: "11111", - GroupID: "15", - Key: "test_experiment_11111", - VariationsIDMap: map[string]entities.Variation{}, - VariationsKeyMap: map[string]entities.Variation{}, - TrafficAllocation: []entities.Range{}, + AudienceIds: []string{"31111"}, + ID: "11111", + GroupID: "15", + Key: "test_experiment_11111", + Variations: map[string]entities.Variation{}, + VariationKeyToIDMap: map[string]string{}, + TrafficAllocation: []entities.Range{}, AudienceConditionTree: &entities.TreeNode{ Operator: "or", Nodes: []*entities.TreeNode{ diff --git a/pkg/config/datafileprojectconfig/mappers/rollout_test.go b/pkg/config/datafileprojectconfig/mappers/rollout_test.go index f9f1a9c9e..6dc9e8232 100644 --- a/pkg/config/datafileprojectconfig/mappers/rollout_test.go +++ b/pkg/config/datafileprojectconfig/mappers/rollout_test.go @@ -42,8 +42,8 @@ func TestMapRollouts(t *testing.T) { "21111": entities.Rollout{ ID: "21111", Experiments: []entities.Experiment{ - entities.Experiment{ID: "11111", Key: "exp_11111", VariationsIDMap: map[string]entities.Variation{}, VariationsKeyMap: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, - entities.Experiment{ID: "11112", Key: "exp_11112", VariationsIDMap: map[string]entities.Variation{}, VariationsKeyMap: map[string]entities.Variation{}, TrafficAllocation: []entities.Range{}}, + entities.Experiment{ID: "11111", Key: "exp_11111", Variations: map[string]entities.Variation{}, VariationKeyToIDMap: map[string]string{}, TrafficAllocation: []entities.Range{}}, + entities.Experiment{ID: "11112", Key: "exp_11112", Variations: map[string]entities.Variation{}, VariationKeyToIDMap: map[string]string{}, TrafficAllocation: []entities.Range{}}, }, }, } diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index 3ab26de57..c0fbe501e 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -57,7 +57,7 @@ func (b MurmurhashExperimentBucketer) Bucket(bucketingID string, experiment enti return nil, reasons.NotBucketedIntoVariation, nil } - if variation, ok := experiment.VariationsIDMap[bucketedVariationID]; ok { + if variation, ok := experiment.Variations[bucketedVariationID]; ok { return &variation, reasons.BucketedIntoVariation, nil } diff --git a/pkg/decision/bucketer/experiment_bucketer_test.go b/pkg/decision/bucketer/experiment_bucketer_test.go index 5fde83f94..7c486cb3a 100644 --- a/pkg/decision/bucketer/experiment_bucketer_test.go +++ b/pkg/decision/bucketer/experiment_bucketer_test.go @@ -13,7 +13,7 @@ func TestBucketExclusionGroups(t *testing.T) { experiment1 := entities.Experiment{ ID: "1886780721", Key: "experiment_1", - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "22222": entities.Variation{ID: "22222", Key: "exp_1_var_1"}, "22223": entities.Variation{ID: "22223", Key: "exp_1_var_2"}, }, @@ -26,7 +26,7 @@ func TestBucketExclusionGroups(t *testing.T) { experiment2 := entities.Experiment{ ID: "1886780723", Key: "experiment_2", - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "22224": entities.Variation{ID: "22224", Key: "exp_2_var_1"}, "22225": entities.Variation{ID: "22225", Key: "exp_2_var_2"}, }, @@ -49,7 +49,7 @@ func TestBucketExclusionGroups(t *testing.T) { bucketer := NewMurmurhashExperimentBucketer(DefaultHashSeed) // ppid2 + 1886780722 (groupId) will generate bucket value of 2434 which maps to experiment 1 bucketedVariation, reason, _ := bucketer.Bucket("ppid2", experiment1, exclusionGroup) - assert.Equal(t, experiment1.VariationsIDMap["22222"], *bucketedVariation) + assert.Equal(t, experiment1.Variations["22222"], *bucketedVariation) assert.Equal(t, reasons.BucketedIntoVariation, reason) // since the bucket value maps to experiment 1, the user will not be bucketed for experiment 2 bucketedVariation, reason, _ = bucketer.Bucket("ppid2", experiment2, exclusionGroup) diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index 97e43de21..59210b8c6 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -51,7 +51,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecision() { ID: "test_user_1", } - expectedVariation := testExp1111.VariationsIDMap["2222"] + expectedVariation := testExp1111.Variations["2222"] expectedExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } @@ -74,7 +74,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { ID: "test_user_1", } - expectedVariation := testExp1111.VariationsIDMap["2222"] + expectedVariation := testExp1111.Variations["2222"] expectedExperimentDecision := ExperimentDecision{} s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext).Return(expectedExperimentDecision, nil) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index 256d5c228..bfb4f1c93 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -98,9 +98,11 @@ func (m *MapExperimentOverridesStore) SetVariation(overrideKey ExperimentOverrid if strings.TrimSpace(variationKey) == "" { return false } - if _, ok := experiment.VariationsKeyMap[variationKey]; ok { - assignVariationKey() - return true + if variationID, ok := experiment.VariationKeyToIDMap[variationKey]; ok { + if _, ok := experiment.Variations[variationID]; ok { + assignVariationKey() + return true + } } } return false @@ -141,11 +143,13 @@ func (s ExperimentOverrideService) GetDecision(decisionContext ExperimentDecisio return decision, nil } - if variation, ok := decisionContext.Experiment.VariationsKeyMap[variationKey]; ok { - decision.Variation = &variation - decision.Reason = reasons.OverrideVariationAssignmentFound - eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) - return decision, nil + if variationID, ok := decisionContext.Experiment.VariationKeyToIDMap[variationKey]; ok { + if variation, ok := decisionContext.Experiment.Variations[variationID]; ok { + decision.Variation = &variation + decision.Reason = reasons.OverrideVariationAssignmentFound + eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) + return decision, nil + } } decision.Reason = reasons.InvalidOverrideVariationAssignment diff --git a/pkg/decision/experiment_whitelist_service.go b/pkg/decision/experiment_whitelist_service.go index cf6b08a5c..2ca62820b 100644 --- a/pkg/decision/experiment_whitelist_service.go +++ b/pkg/decision/experiment_whitelist_service.go @@ -47,10 +47,12 @@ func (s ExperimentWhitelistService) GetDecision(decisionContext ExperimentDecisi return decision, nil } - if variation, ok := decisionContext.Experiment.VariationsKeyMap[variationKey]; ok { - decision.Reason = reasons.WhitelistVariationAssignmentFound - decision.Variation = &variation - return decision, nil + if id, ok := decisionContext.Experiment.VariationKeyToIDMap[variationKey]; ok { + if variation, ok := decisionContext.Experiment.Variations[id]; ok { + decision.Reason = reasons.WhitelistVariationAssignmentFound + decision.Variation = &variation + return decision, nil + } } decision.Reason = reasons.InvalidWhitelistVariationAssignment diff --git a/pkg/decision/feature_experiment_service_test.go b/pkg/decision/feature_experiment_service_test.go index db7d7a633..5b6b7a6bb 100644 --- a/pkg/decision/feature_experiment_service_test.go +++ b/pkg/decision/feature_experiment_service_test.go @@ -45,7 +45,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecision() { ID: "test_user_1", } - expectedVariation := testExp1113.VariationsIDMap["2223"] + expectedVariation := testExp1113.Variations["2223"] returnExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } @@ -84,7 +84,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionMutex() { s.mockExperimentService.On("GetDecision", testExperimentDecisionContext1, testUserContext).Return(nilDecision, nil) // second experiment returns a valid decision to simulate user being bucketed into this experiment in the group - expectedVariation := testExp1114.VariationsIDMap["2225"] + expectedVariation := testExp1114.Variations["2225"] returnExperimentDecision := ExperimentDecision{ Variation: &expectedVariation, } diff --git a/pkg/decision/helpers_test.go b/pkg/decision/helpers_test.go index a3669e674..38798148d 100644 --- a/pkg/decision/helpers_test.go +++ b/pkg/decision/helpers_test.go @@ -101,11 +101,11 @@ var testExp1111Var2222 = entities.Variation{ID: "2222", Key: "2222"} var testExp1111 = entities.Experiment{ ID: "1111", Key: testExp1111Key, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2222": testExp1111Var2222, }, - VariationsKeyMap: map[string]entities.Variation{ - "2222": testExp1111Var2222, + VariationKeyToIDMap: map[string]string{ + "2222": "2222", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2222", EndOfRange: 10000}, @@ -140,11 +140,11 @@ var testExp1112 = entities.Experiment{ }, ID: "1112", Key: testExp1111Key, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2222": testExp1111Var2222, }, - VariationsKeyMap: map[string]entities.Variation{ - "2222": testExp1111Var2222, + VariationKeyToIDMap: map[string]string{ + "2222": "2222", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2222", EndOfRange: 10000}, @@ -174,13 +174,13 @@ var testExp1113 = entities.Experiment{ ID: "1113", Key: testExp1113Key, GroupID: "6666", - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2223": testExp1113Var2223, "2224": testExp1113Var2224, }, - VariationsKeyMap: map[string]entities.Variation{ - "2223": testExp1113Var2223, - "2224": testExp1113Var2224, + VariationKeyToIDMap: map[string]string{ + "2223": "2223", + "2224": "2224", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2223", EndOfRange: 5000}, @@ -196,13 +196,13 @@ var testExp1114 = entities.Experiment{ ID: "1114", Key: testExp1114Key, GroupID: "6666", - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2225": testExp1114Var2225, "2226": testExp1114Var2226, }, - VariationsKeyMap: map[string]entities.Variation{ - "2225": testExp1114Var2225, - "2226": testExp1114Var2226, + VariationKeyToIDMap: map[string]string{ + "2225": "2225", + "2226": "2226", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2225", EndOfRange: 5000}, @@ -225,11 +225,11 @@ var testExp1115Var2227 = entities.Variation{ID: "2227", Key: "2227", FeatureEnab var testExp1115 = entities.Experiment{ ID: "1115", Key: testExp1115Key, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2227": testExp1115Var2227, }, - VariationsKeyMap: map[string]entities.Variation{ - "2227": testExp1115Var2227, + VariationKeyToIDMap: map[string]string{ + "2227": "2227", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2227", EndOfRange: 5000}, @@ -253,11 +253,11 @@ var testTargetedExp1116 = entities.Experiment{ AudienceConditionTree: &entities.TreeNode{Operator: "or", Item: "7771"}, ID: "1116", Key: testTargetedExp1116Key, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2228": testTargetedExp1116Var2228, }, - VariationsKeyMap: map[string]entities.Variation{ - "2228": testTargetedExp1116Var2228, + VariationKeyToIDMap: map[string]string{ + "2228": "2228", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2228", EndOfRange: 10000}, @@ -271,11 +271,11 @@ var testExpWhitelistVar2229 = entities.Variation{ID: "2229", Key: "var_2229"} var testExpWhitelist = entities.Experiment{ ID: "1117", Key: testExpWhitelistKey, - VariationsIDMap: map[string]entities.Variation{ + Variations: map[string]entities.Variation{ "2229": testExpWhitelistVar2229, }, - VariationsKeyMap: map[string]entities.Variation{ - "var_2229": testExpWhitelistVar2229, + VariationKeyToIDMap: map[string]string{ + "var_2229": "2229", }, TrafficAllocation: []entities.Range{ entities.Range{EntityID: "2229", EndOfRange: 10000}, diff --git a/pkg/decision/persisting_experiment_service.go b/pkg/decision/persisting_experiment_service.go index 389fe2b1f..482b935b7 100644 --- a/pkg/decision/persisting_experiment_service.go +++ b/pkg/decision/persisting_experiment_service.go @@ -78,7 +78,7 @@ func (p PersistingExperimentService) getSavedDecision(decisionContext Experiment } if savedVariationID, ok := userProfile.ExperimentBucketMap[decisionKey]; ok { - if variation, ok := decisionContext.Experiment.VariationsIDMap[savedVariationID]; ok { + if variation, ok := decisionContext.Experiment.Variations[savedVariationID]; ok { experimentDecision.Variation = &variation pesLogger.Debug(fmt.Sprintf(`User "%s" was previously bucketed into variation "%s" of experiment "%s".`, userContext.ID, variation.Key, decisionContext.Experiment.Key)) } else { diff --git a/pkg/decision/persisting_experiment_service_test.go b/pkg/decision/persisting_experiment_service_test.go index a6966cd7a..434fafa60 100644 --- a/pkg/decision/persisting_experiment_service_test.go +++ b/pkg/decision/persisting_experiment_service_test.go @@ -47,7 +47,7 @@ func (s *PersistingExperimentServiceTestSuite) SetupTest() { ProjectConfig: s.mockProjectConfig, } - computedVariation := testExp1113.VariationsIDMap["2223"] + computedVariation := testExp1113.Variations["2223"] s.testComputedDecision = ExperimentDecision{ Variation: &computedVariation, } diff --git a/pkg/entities/experiment.go b/pkg/entities/experiment.go index eadb5452d..1ea919d4c 100644 --- a/pkg/entities/experiment.go +++ b/pkg/entities/experiment.go @@ -31,8 +31,8 @@ type Experiment struct { ID string LayerID string Key string - VariationsIDMap map[string]Variation // keyed by variation ID - VariationsKeyMap map[string]Variation // keyed by variation Key + Variations map[string]Variation // keyed by variation ID + VariationKeyToIDMap map[string]string TrafficAllocation []Range GroupID string AudienceConditionTree *TreeNode diff --git a/tests/integration/optlyplugins/userprofileservice/utils.go b/tests/integration/optlyplugins/userprofileservice/utils.go index c76030177..cf6bbb83c 100644 --- a/tests/integration/optlyplugins/userprofileservice/utils.go +++ b/tests/integration/optlyplugins/userprofileservice/utils.go @@ -54,8 +54,10 @@ func CreateUserProfileService(config pkg.ProjectConfig, apiOptions models.APIOpt for experimentKey, variationKey := range bucketMap { if experiment, err := config.GetExperimentByKey(experimentKey); err == nil { decisionKey := decision.NewUserDecisionKey(experiment.ID) - if variation, ok := experiment.VariationsKeyMap[variationKey]; ok { - profile.ExperimentBucketMap[decisionKey] = variation.ID + if variationID, ok := experiment.VariationKeyToIDMap[variationKey]; ok { + if variation, ok := experiment.Variations[variationID]; ok { + profile.ExperimentBucketMap[decisionKey] = variation.ID + } } } } diff --git a/tests/integration/support/utils.go b/tests/integration/support/utils.go index d8cf95dbc..6091b3273 100644 --- a/tests/integration/support/utils.go +++ b/tests/integration/support/utils.go @@ -101,9 +101,11 @@ func parseTemplate(s string, config pkg.ProjectConfig) string { if len(matches) > 1 { expVarKey := strings.Split(matches[1], ".") if exp, err := config.GetExperimentByKey(expVarKey[0]); err == nil { - if variation, ok := exp.VariationsKeyMap[expVarKey[1]]; ok { - parsedString = strings.Replace(parsedString, matches[0], variation.ID, -1) - replaceVariableID() + if variationID, ok := exp.VariationKeyToIDMap[expVarKey[1]]; ok { + if variation, ok := exp.Variations[variationID]; ok { + parsedString = strings.Replace(parsedString, matches[0], variation.ID, -1) + replaceVariableID() + } } } } From 4325f495704f4040034654d6535a11c0c28c9cb3 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Tue, 3 Dec 2019 00:04:55 +0500 Subject: [PATCH 05/11] nits fixed. --- pkg/client/client_test.go | 46 ++++++++++++++++++------------------- pkg/client/fixtures_test.go | 4 ++-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index ccdc816f2..f3acfce11 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -332,7 +332,7 @@ func TestGetFeatureVariableBooleanWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -379,7 +379,7 @@ func TestGetFeatureVariableBooleanReturnsDefaultValueIfFeatureNotEnabled(t *test } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -444,7 +444,7 @@ func TestGetFeatureVariableDoubleWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -490,7 +490,7 @@ func TestGetFeatureVariableDoubleWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -537,7 +537,7 @@ func TestGetFeatureVariableDoubleWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -584,7 +584,7 @@ func TestGetFeatureVariableDoubleWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -631,7 +631,7 @@ func TestGetFeatureVariableDoubleReturnsDefaultValueIfFeatureNotEnabled(t *testi } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -696,7 +696,7 @@ func TestGetFeatureVariableIntegerWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -742,7 +742,7 @@ func TestGetFeatureVariableIntegerWithInvalidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -789,7 +789,7 @@ func TestGetFeatureVariableIntegerWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -836,7 +836,7 @@ func TestGetFeatureVariableIntegerWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -883,7 +883,7 @@ func TestGetFeatureVariableIntegerReturnsDefaultValueIfFeatureNotEnabled(t *test } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -948,7 +948,7 @@ func TestGetFeatureVariableStringWithValidValue(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -994,7 +994,7 @@ func TestGetFeatureVariableStringWithInvalidValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1041,7 +1041,7 @@ func TestGetFeatureVariableStringWithEmptyValueType(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1088,7 +1088,7 @@ func TestGetFeatureVariableStringReturnsDefaultValueIfFeatureNotEnabled(t *testi } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1190,7 +1190,7 @@ func TestGetFeatureDecisionValid(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1235,7 +1235,7 @@ func TestGetFeatureDecisionErrProjectConfig(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1279,7 +1279,7 @@ func TestGetFeatureDecisionPanicProjectConfig(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1322,7 +1322,7 @@ func TestGetFeatureDecisionPanicDecisionService(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1357,7 +1357,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1403,7 +1403,7 @@ func TestGetAllFeatureVariables(t *testing.T) { testVariation := getTestVariationWithFeatureVariable(false, testVariationVariable) testVariation.FeatureEnabled = true testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) @@ -1449,7 +1449,7 @@ func TestGetAllFeatureVariablesWithError(t *testing.T) { } testVariation := getTestVariationWithFeatureVariable(true, testVariationVariable) testExperiment := entities.Experiment{ - ID: "111111", + ID: "111111", Variations: map[string]entities.Variation{"22222": testVariation}, } testFeature := getTestFeature(testFeatureKey, testExperiment) diff --git a/pkg/client/fixtures_test.go b/pkg/client/fixtures_test.go index b422065a8..143a0f9a3 100644 --- a/pkg/client/fixtures_test.go +++ b/pkg/client/fixtures_test.go @@ -182,8 +182,8 @@ func makeTestExperimentWithVariations(experimentKey string, variations []entitie variationsMap[variation.ID] = variation } return entities.Experiment{ - Key: experimentKey, - ID: fmt.Sprintf("test_experiment_%s", experimentKey), + Key: experimentKey, + ID: fmt.Sprintf("test_experiment_%s", experimentKey), Variations: variationsMap, } } From be23e7fc220f98201c1ca3f74bb788ed499e0f69 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 5 Dec 2019 13:36:24 +0500 Subject: [PATCH 06/11] Addressed review comments. --- pkg/decision/experiment_override_service.go | 49 +++---------------- .../experiment_override_service_test.go | 30 +----------- tests/integration/support/client_wrapper.go | 10 ++-- 3 files changed, 11 insertions(+), 78 deletions(-) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index bfb4f1c93..335b2445d 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -20,11 +20,8 @@ package decision import ( "errors" "fmt" - "strings" "sync" - "github.com/optimizely/go-sdk/pkg" - "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" @@ -49,27 +46,14 @@ type MEOptionFunc func(*MapExperimentOverridesStore) // MapExperimentOverridesStore is a map-based implementation of ExperimentOverridesStore that is safe to use concurrently type MapExperimentOverridesStore struct { overridesMap map[ExperimentOverrideKey]string - config pkg.ProjectConfig mutex sync.RWMutex } -// WithConfig sets the config on the MapExperimentOverridesStore -func WithConfig(config pkg.ProjectConfig) MEOptionFunc { - return func(f *MapExperimentOverridesStore) { - f.config = config - } -} - // NewMapExperimentOverridesStore returns a new MapExperimentOverridesStore -func NewMapExperimentOverridesStore(options ...MEOptionFunc) *MapExperimentOverridesStore { - overrideStore := &MapExperimentOverridesStore{ +func NewMapExperimentOverridesStore() *MapExperimentOverridesStore { + return &MapExperimentOverridesStore{ overridesMap: make(map[ExperimentOverrideKey]string), } - - for _, opts := range options { - opts(overrideStore) - } - return overrideStore } // GetVariation returns the override variation key associated with the given user+experiment key @@ -81,31 +65,10 @@ func (m *MapExperimentOverridesStore) GetVariation(overrideKey ExperimentOverrid } // SetVariation sets the given variation key as an override for the given user+experiment key -func (m *MapExperimentOverridesStore) SetVariation(overrideKey ExperimentOverrideKey, variationKey string) bool { - - assignVariationKey := func() { - m.mutex.Lock() - m.overridesMap[overrideKey] = variationKey - m.mutex.Unlock() - } - // Assign directly if no config provided - if m.config == nil { - assignVariationKey() - return true - } - // Check if experiment and variation exist - if experiment, err := m.config.GetExperimentByKey(overrideKey.ExperimentKey); err == nil { - if strings.TrimSpace(variationKey) == "" { - return false - } - if variationID, ok := experiment.VariationKeyToIDMap[variationKey]; ok { - if _, ok := experiment.Variations[variationID]; ok { - assignVariationKey() - return true - } - } - } - return false +func (m *MapExperimentOverridesStore) SetVariation(overrideKey ExperimentOverrideKey, variationKey string) { + m.mutex.Lock() + m.overridesMap[overrideKey] = variationKey + m.mutex.Unlock() } // RemoveVariation removes the override variation key associated with the argument user+experiment key. diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index 07b0aacad..d3dc78378 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -45,7 +45,7 @@ func (s *ExperimentOverrideServiceTestSuite) SetupTest() { s.overrides = NewMapExperimentOverridesStore() s.overrideService = NewExperimentOverrideService(s.overrides) - s.overridesWithConfig = NewMapExperimentOverridesStore(WithConfig(config)) + s.overridesWithConfig = NewMapExperimentOverridesStore() s.overrideServiceWithConfig = NewExperimentOverrideService(s.overridesWithConfig) } @@ -144,34 +144,6 @@ func (s *ExperimentOverrideServiceTestSuite) TestInvalidVariationInOverride() { s.Exactly(reasons.InvalidOverrideVariationAssignment, decision.Reason) } -// Test setVariation with projectconfig in overridestore -func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithInvalidExperimentKey() { - overrideKey := ExperimentOverrideKey{ExperimentKey: "", UserID: "test_user_1"} - success := s.overridesWithConfig.SetVariation(overrideKey, testExp1111Var2222.Key) - s.False(success) - variation, success := s.overridesWithConfig.GetVariation(overrideKey) - s.Exactly("", variation) - s.False(success) -} - -func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithInvalidVariationKey() { - overrideKey := ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"} - success := s.overridesWithConfig.SetVariation(overrideKey, "") - s.False(success) - variation, success := s.overridesWithConfig.GetVariation(overrideKey) - s.Exactly("", variation) - s.False(success) -} - -func (s *ExperimentOverrideServiceTestSuite) TestSetVariationWithValidVariationKeyAndExperimentKey() { - overrideKey := ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"} - success := s.overridesWithConfig.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, testExp1111Var2222.Key) - s.True(success) - variation, success := s.overridesWithConfig.GetVariation(overrideKey) - s.Exactly(testExp1111Var2222.Key, variation) - s.True(success) -} - // Test concurrent use of the MapExperimentOverrideStore // Create 3 goroutines that set and get variations, and assert that all their sets take effect at the end func (s *ExperimentOverrideServiceTestSuite) TestMapExperimentOverridesStoreConcurrent() { diff --git a/tests/integration/support/client_wrapper.go b/tests/integration/support/client_wrapper.go index 69b790526..7925a0cdf 100644 --- a/tests/integration/support/client_wrapper.go +++ b/tests/integration/support/client_wrapper.go @@ -34,7 +34,7 @@ import ( "gopkg.in/yaml.v3" ) -// Map to hold clientwrapper instances against scenarioID +// Cached instance of optly wrapper var clientInstance *ClientWrapper // ClientWrapper - wrapper around the optimizely client that keeps track of various custom components used with the client @@ -85,7 +85,7 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { log.Fatal(err) } - overrideService := decision.NewMapExperimentOverridesStore(decision.WithConfig(config)) + overrideService := decision.NewMapExperimentOverridesStore() userProfileService := userprofileservice.CreateUserProfileService(config, apiOptions) compositeExperimentService := decision.NewCompositeExperimentService( decision.WithUserProfileService(userProfileService), @@ -351,10 +351,8 @@ func (c *ClientWrapper) setForcedVariation(request models.APIOptions) (models.AP if params.VariationKey == "" { c.OverrideStore.(*decision.MapExperimentOverridesStore).RemoveVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}) } else { - result := c.OverrideStore.(*decision.MapExperimentOverridesStore).SetVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}, params.VariationKey) - if result { - response.Result = params.VariationKey - } + c.OverrideStore.(*decision.MapExperimentOverridesStore).SetVariation(decision.ExperimentOverrideKey{ExperimentKey: params.ExperimentKey, UserID: params.UserID}, params.VariationKey) + response.Result = params.VariationKey } } return response, err From c8ad960cf3bc53c647808987df55a1a0e18448f1 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 5 Dec 2019 13:44:06 +0500 Subject: [PATCH 07/11] fixes. --- pkg/decision/experiment_override_service.go | 3 --- .../experiment_override_service_test.go | 25 +++++-------------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index 335b2445d..dec0e71a3 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -40,9 +40,6 @@ type ExperimentOverrideStore interface { GetVariation(overrideKey ExperimentOverrideKey) (string, bool) } -// MEOptionFunc is used to pass custom config options into the MapExperimentOverridesStore. -type MEOptionFunc func(*MapExperimentOverridesStore) - // MapExperimentOverridesStore is a map-based implementation of ExperimentOverridesStore that is safe to use concurrently type MapExperimentOverridesStore struct { overridesMap map[ExperimentOverrideKey]string diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index d3dc78378..8f930008c 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -18,7 +18,6 @@ package decision import ( - "fmt" "sync" "testing" @@ -29,24 +28,15 @@ import ( type ExperimentOverrideServiceTestSuite struct { suite.Suite - mockConfig *mockProjectConfig - overrides *MapExperimentOverridesStore - overrideService *ExperimentOverrideService - overridesWithConfig *MapExperimentOverridesStore - overrideServiceWithConfig *ExperimentOverrideService + mockConfig *mockProjectConfig + overrides *MapExperimentOverridesStore + overrideService *ExperimentOverrideService } func (s *ExperimentOverrideServiceTestSuite) SetupTest() { - config := new(mockProjectConfig) - s.mockConfig = config - s.mockConfig.On("GetExperimentByKey", testExp1111.Key).Return(testExp1111, nil) - s.mockConfig.On("GetExperimentByKey", testExp1113.Key).Return(testExp1113, nil) - s.mockConfig.On("GetExperimentByKey", "").Return(entities.Experiment{}, fmt.Errorf("")) - + s.mockConfig = new(mockProjectConfig) s.overrides = NewMapExperimentOverridesStore() s.overrideService = NewExperimentOverrideService(s.overrides) - s.overridesWithConfig = NewMapExperimentOverridesStore() - s.overrideServiceWithConfig = NewExperimentOverrideService(s.overridesWithConfig) } func (s *ExperimentOverrideServiceTestSuite) TestOverridesIncludeVariation() { @@ -126,9 +116,6 @@ func (s *ExperimentOverrideServiceTestSuite) TestNoOverrideForUserOrExperiment() } func (s *ExperimentOverrideServiceTestSuite) TestInvalidVariationInOverride() { - overrides := NewMapExperimentOverridesStore() - overrideService := NewExperimentOverrideService(overrides) - testDecisionContext := ExperimentDecisionContext{ Experiment: &testExp1111, ProjectConfig: s.mockConfig, @@ -137,8 +124,8 @@ func (s *ExperimentOverrideServiceTestSuite) TestInvalidVariationInOverride() { ID: "test_user_1", } // This override variation key does not exist in the experiment - overrides.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, "invalid_variation_key") - decision, err := overrideService.GetDecision(testDecisionContext, testUserContext) + s.overrides.SetVariation(ExperimentOverrideKey{ExperimentKey: testExp1111.Key, UserID: "test_user_1"}, "invalid_variation_key") + decision, err := s.overrideService.GetDecision(testDecisionContext, testUserContext) s.NoError(err) s.Nil(decision.Variation) s.Exactly(reasons.InvalidOverrideVariationAssignment, decision.Reason) From d1a41ea8126fdb784fdaf8b02495e5d691063fba Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Thu, 5 Dec 2019 13:53:57 +0500 Subject: [PATCH 08/11] nits fixed. --- tests/integration/support/client_wrapper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/support/client_wrapper.go b/tests/integration/support/client_wrapper.go index 7925a0cdf..0529ff460 100644 --- a/tests/integration/support/client_wrapper.go +++ b/tests/integration/support/client_wrapper.go @@ -85,11 +85,11 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { log.Fatal(err) } - overrideService := decision.NewMapExperimentOverridesStore() + overrideStore := decision.NewMapExperimentOverridesStore() userProfileService := userprofileservice.CreateUserProfileService(config, apiOptions) compositeExperimentService := decision.NewCompositeExperimentService( decision.WithUserProfileService(userProfileService), - decision.WithOverrideStore(overrideService), + decision.WithOverrideStore(overrideStore), ) // @TODO: Add sdkKey dynamically once event-batching support is implemented @@ -110,7 +110,7 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { DecisionService: decisionService, EventDispatcher: eventProcessor.EventDispatcher, UserProfileService: userProfileService, - OverrideStore: overrideService, + OverrideStore: overrideStore, } return clientInstance } From 9f5b4cd54c2f2ab3340c884b835b92b0ceb44bbb Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Fri, 6 Dec 2019 15:40:51 +0500 Subject: [PATCH 09/11] fixes for removing flakiness while comparing dispatched events. --- tests/integration/support/client_wrapper.go | 11 +- tests/integration/support/steps.go | 132 ++++++++++++-------- tests/integration/support/utils.go | 14 +++ 3 files changed, 104 insertions(+), 53 deletions(-) diff --git a/tests/integration/support/client_wrapper.go b/tests/integration/support/client_wrapper.go index 0529ff460..5be83124c 100644 --- a/tests/integration/support/client_wrapper.go +++ b/tests/integration/support/client_wrapper.go @@ -22,6 +22,7 @@ import ( "os" "path" "path/filepath" + "strconv" "github.com/optimizely/go-sdk/pkg/client" "github.com/optimizely/go-sdk/pkg/config" @@ -37,6 +38,9 @@ import ( // Cached instance of optly wrapper var clientInstance *ClientWrapper +// Since notificationManager is mapped against sdkKey, we need a unique sdkKey for every scenario +var sdkKey int + // ClientWrapper - wrapper around the optimizely client that keeps track of various custom components used with the client type ClientWrapper struct { Client *client.OptimizelyClient @@ -58,6 +62,7 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { return clientInstance } + sdkKey++ datafileDir := os.Getenv("DATAFILES_DIR") datafile, err := ioutil.ReadFile(filepath.Clean(path.Join(datafileDir, apiOptions.DatafileName))) if err != nil { @@ -93,7 +98,8 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { ) // @TODO: Add sdkKey dynamically once event-batching support is implemented - compositeService := *decision.NewCompositeService("", decision.WithCompositeExperimentService(compositeExperimentService)) + + compositeService := *decision.NewCompositeService(strconv.Itoa(sdkKey), decision.WithCompositeExperimentService(compositeExperimentService)) decisionService := &optlyplugins.TestCompositeService{CompositeService: compositeService} client, err := optimizelyFactory.Client( @@ -112,13 +118,14 @@ func GetInstance(apiOptions models.APIOptions) *ClientWrapper { UserProfileService: userProfileService, OverrideStore: overrideStore, } + clientInstance.DecisionService.(*optlyplugins.TestCompositeService).AddListeners(apiOptions.Listeners) + return clientInstance } // InvokeAPI processes request with arguments func (c *ClientWrapper) InvokeAPI(request models.APIOptions) (models.APIResponse, error) { - c.DecisionService.(*optlyplugins.TestCompositeService).AddListeners(request.Listeners) var response models.APIResponse var err error diff --git a/tests/integration/support/steps.go b/tests/integration/support/steps.go index db0e0017a..449afc631 100644 --- a/tests/integration/support/steps.go +++ b/tests/integration/support/steps.go @@ -265,20 +265,36 @@ func (c *ScenarioCtx) InTheResponseShouldHaveEachOneOfThese(argumentType string, // TheNumberOfDispatchedEventsIs checks the count of the dispatched events to be equal to the given value. func (c *ScenarioCtx) TheNumberOfDispatchedEventsIs(count int) error { - dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() - if len(dispatchedEvents) == count { + evaluationMethod := func() (bool, string) { + dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() + result := len(dispatchedEvents) == count + if result { + return result, "" + } + return result, "dispatchedEvents count not equal" + } + result, errorMessage := EvaluateWithTimeout(evaluationMethod) + if result { return nil } - return fmt.Errorf("dispatchedEvents count not equal") + return fmt.Errorf(errorMessage) } // ThereAreNoDispatchedEvents checks the dispatched events count to be empty. func (c *ScenarioCtx) ThereAreNoDispatchedEvents() error { - dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() - if len(dispatchedEvents) == 0 { + evaluationMethod := func() (bool, string) { + dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() + result := len(dispatchedEvents) == 0 + if result { + return result, "" + } + return result, fmt.Sprintf("dispatchedEvents should be empty but received %d events", len(dispatchedEvents)) + } + result, errorMessage := EvaluateWithTimeout(evaluationMethod) + if result { return nil } - return fmt.Errorf("dispatchedEvents should be empty but received %d events", len(dispatchedEvents)) + return fmt.Errorf(errorMessage) } // DispatchedEventsPayloadsInclude checks dispatched events to contain the given events. @@ -293,70 +309,84 @@ func (c *ScenarioCtx) DispatchedEventsPayloadsInclude(value *gherkin.DocString) return fmt.Errorf("Invalid request for dispatched Events") } - eventsReceived := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() - eventsReceivedJSON, err := json.Marshal(eventsReceived) - if err != nil { - return fmt.Errorf("Invalid response for dispatched Events") - } - var actualBatchEvents []map[string]interface{} - - if err := json.Unmarshal(eventsReceivedJSON, &actualBatchEvents); err != nil { - return fmt.Errorf("Invalid response for dispatched Events") - } - - // Sort's attributes under visitors which is required for subset comparison of attributes array - sortAttributesForEvents := func(array []map[string]interface{}) []map[string]interface{} { - sortedArray := array - for mainIndex, event := range array { - if visitorsArray, ok := event["visitors"].([]interface{}); ok { - for vIndex, v := range visitorsArray { - if visitor, ok := v.(map[string]interface{}); ok { - // Only sort if all attributes were parsed successfuly - parsedSuccessfully := false - parsedAttributes := []map[string]interface{}{} - if attributesArray, ok := visitor["attributes"].([]interface{}); ok { - for _, tmpAttribute := range attributesArray { - if attribute, ok := tmpAttribute.(map[string]interface{}); ok { - parsedAttributes = append(parsedAttributes, attribute) + evaluationMethod := func() (bool, string) { + eventsReceived := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() + eventsReceivedJSON, err := json.Marshal(eventsReceived) + if err != nil { + return false, "Invalid response for dispatched Events" + } + var actualBatchEvents []map[string]interface{} + + if err := json.Unmarshal(eventsReceivedJSON, &actualBatchEvents); err != nil { + return false, "Invalid response for dispatched Events" + } + + // Sort's attributes under visitors which is required for subset comparison of attributes array + sortAttributesForEvents := func(array []map[string]interface{}) []map[string]interface{} { + sortedArray := array + for mainIndex, event := range array { + if visitorsArray, ok := event["visitors"].([]interface{}); ok { + for vIndex, v := range visitorsArray { + if visitor, ok := v.(map[string]interface{}); ok { + // Only sort if all attributes were parsed successfuly + parsedSuccessfully := false + parsedAttributes := []map[string]interface{}{} + if attributesArray, ok := visitor["attributes"].([]interface{}); ok { + for _, tmpAttribute := range attributesArray { + if attribute, ok := tmpAttribute.(map[string]interface{}); ok { + parsedAttributes = append(parsedAttributes, attribute) + } } + parsedSuccessfully = len(attributesArray) == len(parsedAttributes) + } + if parsedSuccessfully { + // Sort parsed attributes array and assign them to the original events array + sortedAttributes := sortArrayofMaps(parsedAttributes, "key") + sortedArray[mainIndex]["visitors"].([]interface{})[vIndex].(map[string]interface{})["attributes"] = sortedAttributes } - parsedSuccessfully = len(attributesArray) == len(parsedAttributes) - } - if parsedSuccessfully { - // Sort parsed attributes array and assign them to the original events array - sortedAttributes := sortArrayofMaps(parsedAttributes, "key") - sortedArray[mainIndex]["visitors"].([]interface{})[vIndex].(map[string]interface{})["attributes"] = sortedAttributes } } } } + return sortedArray } - return sortedArray - } - expectedBatchEvents = sortAttributesForEvents(expectedBatchEvents) - actualBatchEvents = sortAttributesForEvents(actualBatchEvents) + expectedBatchEvents = sortAttributesForEvents(expectedBatchEvents) + actualBatchEvents = sortAttributesForEvents(actualBatchEvents) + result := subset.Check(expectedBatchEvents, actualBatchEvents) + if result { + return result, "" + } + return result, "DispatchedEvents not equal" + } - if subset.Check(expectedBatchEvents, actualBatchEvents) { + result, errorMessage := EvaluateWithTimeout(evaluationMethod) + if result { return nil } - return fmt.Errorf("DispatchedEvents not equal") + return fmt.Errorf(errorMessage) } // PayloadsOfDispatchedEventsDontIncludeDecisions checks dispatched events to contain no decisions. func (c *ScenarioCtx) PayloadsOfDispatchedEventsDontIncludeDecisions() error { - dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() - - for _, event := range dispatchedEvents { - for _, visitor := range event.Visitors { - for _, snapshot := range visitor.Snapshots { - if len(snapshot.Decisions) > 0 { - return fmt.Errorf("dispatched events should not include decisions") + evaluationMethod := func() (bool, string) { + dispatchedEvents := c.clientWrapper.EventDispatcher.(optlyplugins.EventReceiver).GetEvents() + for _, event := range dispatchedEvents { + for _, visitor := range event.Visitors { + for _, snapshot := range visitor.Snapshots { + if len(snapshot.Decisions) > 0 { + return false, "dispatched events should not include decisions" + } } } } + return true, "" } - return nil + result, errorMessage := EvaluateWithTimeout(evaluationMethod) + if result { + return nil + } + return fmt.Errorf(errorMessage) } // TheUserProfileServiceStateShouldBe checks current state of UPS diff --git a/tests/integration/support/utils.go b/tests/integration/support/utils.go index 6091b3273..c2a2b2d32 100644 --- a/tests/integration/support/utils.go +++ b/tests/integration/support/utils.go @@ -21,6 +21,7 @@ import ( "regexp" "sort" "strings" + "time" "github.com/google/go-cmp/cmp" "github.com/optimizely/go-sdk/pkg" @@ -150,3 +151,16 @@ func compareStringSlice(x, y []string) bool { } return false } + +// EvaluateWithTimeout evaluates given function with a timeout +func EvaluateWithTimeout(evaluationMethod func() (result bool, errorMessage string)) (result bool, message string) { + result, errorMessage := evaluationMethod() + // Return immediately if evaluation was successfull + if result { + return result, errorMessage + } + // Retry after 200ms + time.Sleep(200 * time.Millisecond) + result, errorMessage = evaluationMethod() + return result, errorMessage +} From c3ae7e2e0bcf923c2407bc68092a56b41f4738d6 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Fri, 6 Dec 2019 15:52:04 +0500 Subject: [PATCH 10/11] nit fixed. --- tests/integration/support/steps.go | 8 ++++---- tests/integration/support/utils.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/support/steps.go b/tests/integration/support/steps.go index 449afc631..87c5daa80 100644 --- a/tests/integration/support/steps.go +++ b/tests/integration/support/steps.go @@ -273,7 +273,7 @@ func (c *ScenarioCtx) TheNumberOfDispatchedEventsIs(count int) error { } return result, "dispatchedEvents count not equal" } - result, errorMessage := EvaluateWithTimeout(evaluationMethod) + result, errorMessage := evaluateDispatchedEventsWithTimeout(evaluationMethod) if result { return nil } @@ -290,7 +290,7 @@ func (c *ScenarioCtx) ThereAreNoDispatchedEvents() error { } return result, fmt.Sprintf("dispatchedEvents should be empty but received %d events", len(dispatchedEvents)) } - result, errorMessage := EvaluateWithTimeout(evaluationMethod) + result, errorMessage := evaluateDispatchedEventsWithTimeout(evaluationMethod) if result { return nil } @@ -360,7 +360,7 @@ func (c *ScenarioCtx) DispatchedEventsPayloadsInclude(value *gherkin.DocString) return result, "DispatchedEvents not equal" } - result, errorMessage := EvaluateWithTimeout(evaluationMethod) + result, errorMessage := evaluateDispatchedEventsWithTimeout(evaluationMethod) if result { return nil } @@ -382,7 +382,7 @@ func (c *ScenarioCtx) PayloadsOfDispatchedEventsDontIncludeDecisions() error { } return true, "" } - result, errorMessage := EvaluateWithTimeout(evaluationMethod) + result, errorMessage := evaluateDispatchedEventsWithTimeout(evaluationMethod) if result { return nil } diff --git a/tests/integration/support/utils.go b/tests/integration/support/utils.go index c2a2b2d32..b067e9eaf 100644 --- a/tests/integration/support/utils.go +++ b/tests/integration/support/utils.go @@ -152,8 +152,8 @@ func compareStringSlice(x, y []string) bool { return false } -// EvaluateWithTimeout evaluates given function with a timeout -func EvaluateWithTimeout(evaluationMethod func() (result bool, errorMessage string)) (result bool, message string) { +// Evaluates given function with a timeout +func evaluateDispatchedEventsWithTimeout(evaluationMethod func() (result bool, errorMessage string)) (result bool, message string) { result, errorMessage := evaluationMethod() // Return immediately if evaluation was successfull if result { From 232908a4c7eb56ac44fe1fe27629aa0849703b06 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 6 Dec 2019 11:18:07 -0800 Subject: [PATCH 11/11] checking new tags --- .travis.yml | 2 +- scripts/run-fsc-tests.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 043e6552f..ecfae3696 100644 --- a/.travis.yml +++ b/.travis.yml @@ -54,7 +54,7 @@ jobs: env: GIMME_GO_VERSION=1.12.x FSC_PATH="/tmp/fsc-repo" before_script: - mkdir -p $FSC_PATH - - pushd $FSC_PATH && git init && git fetch --depth=1 https://$CI_USER_TOKEN@github.com/optimizely/fullstack-sdk-compatibility-suite ${FSC_BRANCH:-master} && git checkout FETCH_HEAD && popd + - pushd $FSC_PATH && git init && git fetch --depth=1 https://$CI_USER_TOKEN@github.com/optimizely/fullstack-sdk-compatibility-suite "yasir/go-ignore-fv-cases" && git checkout FETCH_HEAD && popd install: - eval "$(gimme)" script: diff --git a/scripts/run-fsc-tests.sh b/scripts/run-fsc-tests.sh index 6000b4759..4726b3505 100755 --- a/scripts/run-fsc-tests.sh +++ b/scripts/run-fsc-tests.sh @@ -66,5 +66,5 @@ mkdir -p $GO_FEATUREFILES_PATH cp -r $FEATURE_FILES_PATH $GO_FEATUREFILES_PATH export DATAFILES_DIR="$DATAFILES_PATH" -go test -v $(pwd)/tests/integration --godog.tags="$TAG_FILTER_EXPRESSION" --godog.f=progress +go test -v $(pwd)/tests/integration --godog.tags="~@DATAFILE_MANAGER&&~@INPUT_VALIDATION&&~@EVENT_BATCHING&&~@NO_EASY_EVENT_TRACKING&&~@OASIS-3654&&~@TARGETED_ROLLOUT&&~@EXPERIMENT_STATUS&&@~UNSUPPORTED_DATAFILE_VERSION&&@~TRACK_LISTENER&&@~ACTIVATE_LISTENER&&~@OPTIMIZELY_CONFIG&&~@LOG_EVENT_LISTENER" --godog.f=progress echo "Ready for testing."