refact(audience-evaluation): Fixed null-bubbling issues.

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)
 }

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
 
 	}

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 {

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)
 }

pkg/decision/evaluator/matchers/exact.go

@@ -34,23 +34,23 @@ 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
 	}
 
 	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
 	}
 
 	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
 	}

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)
 }

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
 }

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
 	}

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)
 }

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
 	}

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)
 }

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
 	}

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)
 }