diff --git a/pkg/decision/evaluator/audience.go b/pkg/decision/evaluator/audience.go index 00229efd6..c5244a149 100644 --- a/pkg/decision/evaluator/audience.go +++ b/pkg/decision/evaluator/audience.go @@ -40,6 +40,6 @@ func NewTypedAudienceEvaluator() *TypedAudienceEvaluator { } // Evaluate evaluates the typed audience against the given user's attributes -func (a TypedAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *entities.TreeParameters) bool { +func (a TypedAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { return a.conditionTreeEvaluator.Evaluate(audience.ConditionTree, condTreeParams) } diff --git a/pkg/decision/evaluator/condition.go b/pkg/decision/evaluator/condition.go index 6ec78e090..ef8da5d09 100644 --- a/pkg/decision/evaluator/condition.go +++ b/pkg/decision/evaluator/condition.go @@ -92,7 +92,10 @@ func (c AudienceConditionEvaluator) Evaluate(audienceID string, condTreeParams * if audience, ok := condTreeParams.AudienceMap[audienceID]; ok { condTree := audience.ConditionTree conditionTreeEvaluator := NewMixedTreeEvaluator() - retValue := conditionTreeEvaluator.Evaluate(condTree, condTreeParams) + retValue, isValid := conditionTreeEvaluator.Evaluate(condTree, condTreeParams) + if !isValid { + return false, fmt.Errorf(`an error occurred while evaluating nested tree for audience ID "%s"`, audienceID) + } return retValue, nil } diff --git a/pkg/decision/evaluator/condition_tree.go b/pkg/decision/evaluator/condition_tree.go index ca6df5514..6022d388c 100644 --- a/pkg/decision/evaluator/condition_tree.go +++ b/pkg/decision/evaluator/condition_tree.go @@ -36,7 +36,7 @@ const ( // TreeEvaluator evaluates a tree type TreeEvaluator interface { - Evaluate(*entities.TreeNode, *entities.TreeParameters) bool + Evaluate(*entities.TreeNode, *entities.TreeParameters) (evalResult, isValid bool) } // MixedTreeEvaluator evaluates a tree of mixed node types (condition node or audience nodes) @@ -48,16 +48,8 @@ func NewMixedTreeEvaluator() *MixedTreeEvaluator { return &MixedTreeEvaluator{} } -// Evaluate returns true if the userAttributes satisfy the given condition tree -func (c MixedTreeEvaluator) Evaluate(node *entities.TreeNode, condTreeParams *entities.TreeParameters) bool { - // This wrapper method converts the conditionEvalResult to a boolean - result, _ := c.evaluate(node, condTreeParams) - return result -} - -// Helper method to recursively evaluate a condition tree -// Returns the result of the evaluation and whether the evaluation of the condition is valid or not (to handle null bubbling) -func (c MixedTreeEvaluator) evaluate(node *entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { +// Evaluate returns whether the userAttributes satisfy the given condition tree and the evaluation of the condition is valid or not (to handle null bubbling) +func (c MixedTreeEvaluator) Evaluate(node *entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { operator := node.Operator if operator != "" { switch operator { @@ -94,7 +86,7 @@ func (c MixedTreeEvaluator) evaluate(node *entities.TreeNode, condTreeParams *en func (c MixedTreeEvaluator) evaluateAnd(nodes []*entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { sawInvalid := false for _, node := range nodes { - result, isValid := c.evaluate(node, condTreeParams) + result, isValid := c.Evaluate(node, condTreeParams) if !isValid { return false, isValid } else if !result { @@ -112,7 +104,7 @@ func (c MixedTreeEvaluator) evaluateAnd(nodes []*entities.TreeNode, condTreePara func (c MixedTreeEvaluator) evaluateNot(nodes []*entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { if len(nodes) > 0 { - result, isValid := c.evaluate(nodes[0], condTreeParams) + result, isValid := c.Evaluate(nodes[0], condTreeParams) if !isValid { return false, false } @@ -124,7 +116,7 @@ func (c MixedTreeEvaluator) evaluateNot(nodes []*entities.TreeNode, condTreePara func (c MixedTreeEvaluator) evaluateOr(nodes []*entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { sawInvalid := false for _, node := range nodes { - result, isValid := c.evaluate(node, condTreeParams) + result, isValid := c.Evaluate(node, condTreeParams) if !isValid { sawInvalid = true } else if result { diff --git a/pkg/decision/evaluator/condition_tree_test.go b/pkg/decision/evaluator/condition_tree_test.go index 6cccc96e2..dd9b74ca2 100644 --- a/pkg/decision/evaluator/condition_tree_test.go +++ b/pkg/decision/evaluator/condition_tree_test.go @@ -47,7 +47,7 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { }, } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test no match @@ -56,7 +56,7 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { "string_foo": "not foo", }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) } @@ -82,7 +82,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test match bool @@ -91,7 +91,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": true, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test match both @@ -101,7 +101,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": true, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test no match @@ -111,7 +111,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": false, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) } @@ -137,7 +137,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) // Test only bool match with NULL bubbling @@ -146,7 +146,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": true, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) // Test match both @@ -156,7 +156,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": true, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test no match @@ -166,7 +166,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": false, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) } @@ -203,7 +203,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test match bool @@ -212,7 +212,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": false, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test match both @@ -222,7 +222,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": false, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test no match @@ -232,7 +232,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": true, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) } @@ -282,7 +282,7 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test only match the NOT condition @@ -293,7 +293,7 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 43, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test only match the int condition @@ -304,7 +304,7 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 42, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.True(t, result) // Test no match @@ -315,7 +315,7 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 43, }, } - result = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) assert.False(t, result) } @@ -383,7 +383,7 @@ func TestConditionTreeEvaluateAnAudienceTreeSingleAudience(t *testing.T) { }, AudienceMap: audienceMap, } - result := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + result, _ := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) assert.True(t, result) } @@ -412,7 +412,7 @@ func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { }, AudienceMap: audienceMap, } - result := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + result, _ := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) assert.True(t, result) // Test only matches audience 11112 @@ -426,6 +426,6 @@ func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { }, AudienceMap: audienceMap, } - result = conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + result, _ = conditionTreeEvaluator.Evaluate(audienceTree, treeParams) assert.True(t, result) } diff --git a/pkg/decision/evaluator/matchers/exact.go b/pkg/decision/evaluator/matchers/exact.go index 097bc8c73..bf307c08c 100644 --- a/pkg/decision/evaluator/matchers/exact.go +++ b/pkg/decision/evaluator/matchers/exact.go @@ -34,7 +34,7 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return stringValue == attributeValue, nil } @@ -42,7 +42,7 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if boolValue, ok := m.Condition.Value.(bool); ok { attributeValue, err := user.GetBoolAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return boolValue == attributeValue, nil } @@ -50,7 +50,7 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return floatValue == attributeValue, nil } diff --git a/pkg/decision/evaluator/matchers/exact_test.go b/pkg/decision/evaluator/matchers/exact_test.go index c1622ecb7..d74649964 100644 --- a/pkg/decision/evaluator/matchers/exact_test.go +++ b/pkg/decision/evaluator/matchers/exact_test.go @@ -62,7 +62,7 @@ func TestExactMatcherString(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } func TestExactMatcherBool(t *testing.T) { @@ -103,7 +103,7 @@ func TestExactMatcherBool(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } func TestExactMatcherInt(t *testing.T) { @@ -155,7 +155,7 @@ func TestExactMatcherInt(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } func TestExactMatcherFloat(t *testing.T) { @@ -196,5 +196,5 @@ func TestExactMatcherFloat(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } diff --git a/pkg/decision/evaluator/matchers/exists.go b/pkg/decision/evaluator/matchers/exists.go index 1b9719967..50f287bd3 100644 --- a/pkg/decision/evaluator/matchers/exists.go +++ b/pkg/decision/evaluator/matchers/exists.go @@ -28,5 +28,6 @@ type ExistsMatcher struct { // Match returns true if the user's attribute is in the condition func (m ExistsMatcher) Match(user entities.UserContext) (bool, error) { + return user.CheckAttributeExists(m.Condition.Name), nil } diff --git a/pkg/decision/evaluator/matchers/gt.go b/pkg/decision/evaluator/matchers/gt.go index 0332ef9d5..04832d44b 100644 --- a/pkg/decision/evaluator/matchers/gt.go +++ b/pkg/decision/evaluator/matchers/gt.go @@ -35,7 +35,7 @@ func (m GtMatcher) Match(user entities.UserContext) (bool, error) { if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return floatValue < attributeValue, nil } diff --git a/pkg/decision/evaluator/matchers/gt_test.go b/pkg/decision/evaluator/matchers/gt_test.go index 6674e005b..a68eda2f4 100644 --- a/pkg/decision/evaluator/matchers/gt_test.go +++ b/pkg/decision/evaluator/matchers/gt_test.go @@ -84,7 +84,7 @@ func TestGtMatcherInt(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } func TestGtMatcherFloat(t *testing.T) { @@ -135,5 +135,5 @@ func TestGtMatcherFloat(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } diff --git a/pkg/decision/evaluator/matchers/lt.go b/pkg/decision/evaluator/matchers/lt.go index a0e27d8f1..ad47eddaf 100644 --- a/pkg/decision/evaluator/matchers/lt.go +++ b/pkg/decision/evaluator/matchers/lt.go @@ -35,7 +35,7 @@ func (m LtMatcher) Match(user entities.UserContext) (bool, error) { if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return floatValue > attributeValue, nil } diff --git a/pkg/decision/evaluator/matchers/lt_test.go b/pkg/decision/evaluator/matchers/lt_test.go index d4fa56593..e79c05a1c 100644 --- a/pkg/decision/evaluator/matchers/lt_test.go +++ b/pkg/decision/evaluator/matchers/lt_test.go @@ -84,7 +84,7 @@ func TestLtMatcherInt(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } func TestLtMatcherFloat(t *testing.T) { @@ -135,5 +135,5 @@ func TestLtMatcherFloat(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } diff --git a/pkg/decision/evaluator/matchers/substring.go b/pkg/decision/evaluator/matchers/substring.go index e8051700f..f25241f02 100644 --- a/pkg/decision/evaluator/matchers/substring.go +++ b/pkg/decision/evaluator/matchers/substring.go @@ -35,7 +35,7 @@ func (m SubstringMatcher) Match(user entities.UserContext) (bool, error) { if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { - return false, nil + return false, err } return strings.Contains(attributeValue, stringValue), nil } diff --git a/pkg/decision/evaluator/matchers/substring_test.go b/pkg/decision/evaluator/matchers/substring_test.go index eed9fd557..9d7db2e00 100644 --- a/pkg/decision/evaluator/matchers/substring_test.go +++ b/pkg/decision/evaluator/matchers/substring_test.go @@ -63,5 +63,5 @@ func TestSubstringMatcher(t *testing.T) { } _, err = matcher.Match(user) - assert.NoError(t, err) + assert.Error(t, err) } diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index 41fb1381e..b47e8e151 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -53,7 +53,7 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio // Determine if user can be part of the experiment if experiment.AudienceConditionTree != nil { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - evalResult := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) + evalResult, _ := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { experimentDecision.Reason = reasons.FailedAudienceTargeting return experimentDecision, nil diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index e2ce98941..2577735f4 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -86,7 +86,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { s.mockBucketer.On("Bucket", testUserContext.ID, testTargetedExp1116, entities.Group{}).Return(&testTargetedExp1116Var2228, reasons.BucketedIntoVariation, nil) mockAudienceTreeEvaluator := new(MockAudienceTreeEvaluator) - mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(true) + mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(true, true) experimentBucketerService := ExperimentBucketerService{ audienceTreeEvaluator: mockAudienceTreeEvaluator, bucketer: s.mockBucketer, @@ -113,7 +113,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { }, } mockAudienceTreeEvaluator := new(MockAudienceTreeEvaluator) - mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(false) + mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(false, true) experimentBucketerService := ExperimentBucketerService{ audienceTreeEvaluator: mockAudienceTreeEvaluator, bucketer: s.mockBucketer, diff --git a/pkg/decision/helpers_test.go b/pkg/decision/helpers_test.go index ee8c6ac30..a899afb85 100644 --- a/pkg/decision/helpers_test.go +++ b/pkg/decision/helpers_test.go @@ -89,9 +89,9 @@ func (m *MockUserProfileService) Save(userProfile UserProfile) { m.Called(userProfile) } -func (m *MockAudienceTreeEvaluator) Evaluate(node *entities.TreeNode, condTreeParams *entities.TreeParameters) bool { +func (m *MockAudienceTreeEvaluator) Evaluate(node *entities.TreeNode, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { args := m.Called(node, condTreeParams) - return args.Bool(0) + return args.Bool(0), args.Bool(1) } // Single variation experiment diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index 965d55ead..596a5172a 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -71,7 +71,7 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user // if user fails rollout targeting rule we return out of it if experiment.AudienceConditionTree != nil { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - evalResult := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) + evalResult, _ := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { featureDecision.Reason = reasons.FailedRolloutTargeting rsLogger.Debug(fmt.Sprintf(`User "%s" failed targeting for feature rollout with key "%s".`, userContext.ID, feature.Key)) diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index b9f13101f..241db7a28 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -31,13 +31,13 @@ import ( type RolloutServiceTestSuite struct { suite.Suite - mockConfig *mockProjectConfig - mockAudienceTreeEvaluator *MockAudienceTreeEvaluator - mockExperimentService *MockExperimentDecisionService + mockConfig *mockProjectConfig + mockAudienceTreeEvaluator *MockAudienceTreeEvaluator + mockExperimentService *MockExperimentDecisionService testExperimentDecisionContext ExperimentDecisionContext - testFeatureDecisionContext FeatureDecisionContext - testConditionTreeParams *entities.TreeParameters - testUserContext entities.UserContext + testFeatureDecisionContext FeatureDecisionContext + testConditionTreeParams *entities.TreeParameters + testUserContext entities.UserContext } func (s *RolloutServiceTestSuite) SetupTest() { @@ -67,9 +67,9 @@ func (s *RolloutServiceTestSuite) TestGetDecisionHappyPath() { // Test experiment passes targeting and bucketing testExperimentBucketerDecision := ExperimentDecision{ Variation: &testExp1112Var2222, - Decision: Decision{Reason: reasons.BucketedIntoVariation}, + Decision: Decision{Reason: reasons.BucketedIntoVariation}, } - s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(true) + s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(true, true) s.mockExperimentService.On("GetDecision", s.testExperimentDecisionContext, s.testUserContext).Return(testExperimentBucketerDecision, nil) testRolloutService := RolloutService{ @@ -96,7 +96,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsBucketing() { }, } - s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(true) + s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(true, true) s.mockExperimentService.On("GetDecision", s.testExperimentDecisionContext, s.testUserContext).Return(testExperimentBucketerDecision, nil) testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, @@ -116,7 +116,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsBucketing() { } func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { - s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(false) + s.mockAudienceTreeEvaluator.On("Evaluate", testExp1112.AudienceConditionTree, s.testConditionTreeParams).Return(false, true) testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, diff --git a/tests/integration/support/steps.go b/tests/integration/support/steps.go index 41612bac4..286005d4b 100644 --- a/tests/integration/support/steps.go +++ b/tests/integration/support/steps.go @@ -283,9 +283,44 @@ func (c *ScenarioCtx) DispatchedEventsPayloadsInclude(value *gherkin.DocString) 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) + } + } + 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 + } + + expectedBatchEvents = sortAttributesForEvents(expectedBatchEvents) + actualBatchEvents = sortAttributesForEvents(actualBatchEvents) + if subset.Check(expectedBatchEvents, actualBatchEvents) { return nil } diff --git a/tests/integration/support/utils.go b/tests/integration/support/utils.go index 79c2d83bc..d1ceb37e8 100644 --- a/tests/integration/support/utils.go +++ b/tests/integration/support/utils.go @@ -18,6 +18,7 @@ package support import ( "regexp" + "sort" "strings" "github.com/optimizely/go-sdk/pkg" @@ -25,6 +26,23 @@ import ( "gopkg.in/yaml.v3" ) +func sortArrayofMaps(array []map[string]interface{}, sortKey string) []map[string]interface{} { + sort.Slice(array, func(i, j int) bool { + // TODO: make it more generic but for now only expecting string comparison + if val1, ok := array[i][sortKey].(string); ok { + if val2, ok1 := array[j][sortKey].(string); ok1 { + if strings.HasPrefix(val1, "$") { + return false + } + return val1 < val2 + } + } + // if both are not string let it go. + return false + }) + return array +} + func getDispatchedEventsMapFromYaml(s string, config pkg.ProjectConfig) ([]map[string]interface{}, error) { var eventsArray []map[string]interface{} parsedString := parseTemplate(s, config)