Skip to content

Commit b946a6f

Browse files
pawels-optimizelypawels-optimizely
committed
final working version of audience condition tree parsing
1 parent 3f8dadb commit b946a6f

File tree

7 files changed

+140
-64
lines changed

7 files changed

+140
-64
lines changed

‎optimizely/decision/evaluator/audience.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
// AudienceEvaluator evaluates an audience against the given user's attributes
2424
type AudienceEvaluator interface {
25-
Evaluate(audience entities.Audience, condTreeParams *ConditionTreeParameters) bool
25+
Evaluate(audience entities.Audience, condTreeParams *entities.ConditionTreeParameters) bool
2626
}
2727

2828
// TypedAudienceEvaluator evaluates typed audiences
@@ -39,6 +39,6 @@ func NewTypedAudienceEvaluator() *TypedAudienceEvaluator {
3939
}
4040

4141
// Evaluate evaluates the typed audience against the given user's attributes
42-
func (a TypedAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *ConditionTreeParameters) bool {
42+
func (a TypedAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *entities.ConditionTreeParameters) bool {
4343
return a.conditionTreeEvaluator.Evaluate(audience.ConditionTree, condTreeParams)
4444
}

‎optimizely/decision/evaluator/condition.go

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package evaluator
1818

1919
import (
2020
"fmt"
21-
2221
"github.com/optimizely/go-sdk/optimizely/decision/evaluator/matchers"
2322
"github.com/optimizely/go-sdk/optimizely/entities"
2423
)
@@ -32,14 +31,14 @@ const (
3231

3332
// ConditionEvaluator evaluates a condition against the given user's attributes
3433
type ConditionEvaluator interface {
35-
Evaluate(entities.Condition, *ConditionTreeParameters) (bool, error)
34+
Evaluate(entities.Condition, *entities.ConditionTreeParameters) (bool, error)
3635
}
3736

3837
// CustomAttributeConditionEvaluator evaluates conditions with custom attributes
3938
type CustomAttributeConditionEvaluator struct{}
4039

4140
// Evaluate returns true if the given user's attributes match the condition
42-
func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition, condTreeParams *ConditionTreeParameters) (bool, error) {
41+
func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition, condTreeParams *entities.ConditionTreeParameters) (bool, error) {
4342
// We should only be evaluating custom attributes
4443
if condition.Type != customAttributeType {
4544
return false, fmt.Errorf(`Unable to evaluator condition of type "%s"`, condition.Type)
@@ -64,6 +63,8 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition
6463
matcher = matchers.GtMatcher{
6564
Condition: condition,
6665
}
66+
default:
67+
return false, fmt.Errorf(`Invalid Condition matcher "%s"`, condition.Match)
6768
}
6869

6970
user := *condTreeParams.User
@@ -75,34 +76,22 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition
7576
type AudienceConditionEvaluator struct{}
7677

7778
// Evaluate returns true if the given user's attributes match the condition
78-
func (c AudienceConditionEvaluator) Evaluate(condition entities.Condition, condTreeParams *ConditionTreeParameters) (bool, error) {
79+
func (c AudienceConditionEvaluator) Evaluate(condition entities.Condition, condTreeParams *entities.ConditionTreeParameters) (bool, error) {
7980
// We should only be evaluating custom attributes
80-
if condition.Type != customAttributeType {
81+
if condition.Type != audienceCondition {
8182
return false, fmt.Errorf(`Unable to evaluator condition of type "%s"`, condition.Type)
8283
}
84+
var ok bool
85+
var audienceID string
8386

84-
var matcher matchers.Matcher
85-
matchType := condition.Match
86-
switch matchType {
87-
case exactMatchType:
88-
matcher = matchers.ExactMatcher{
89-
Condition: condition,
90-
}
91-
case existsMatchType:
92-
matcher = matchers.ExistsMatcher{
93-
Condition: condition,
94-
}
95-
case ltMatchType:
96-
matcher = matchers.LtMatcher{
97-
Condition: condition,
98-
}
99-
case gtMatchType:
100-
matcher = matchers.GtMatcher{
101-
Condition: condition,
87+
if audienceID, ok = condition.Value.(string); ok {
88+
if audience, good := condTreeParams.AudienceMap[audienceID]; good {
89+
condTree := audience.ConditionTree
90+
conditionTreeEvaluator := NewConditionTreeEvaluator()
91+
retValue := conditionTreeEvaluator.Evaluate(condTree, condTreeParams)
92+
return retValue, nil
10293
}
10394
}
10495

105-
user := *condTreeParams.User
106-
result, err := matcher.Match(user)
107-
return result, err
96+
return false, fmt.Errorf(`Unable to evaluate nested tree for audience ID "%s"`, audienceID)
10897
}

‎optimizely/decision/evaluator/condition_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestCustomAttributeConditionEvaluator(t *testing.T) {
4141
},
4242
}
4343

44-
condTreeParams := NewCConditionTreeParameters(&user, map[string]entities.Audience{})
44+
condTreeParams := entities.NewConditionTreeParameters(&user, map[string]entities.Audience{})
4545
result, _ := conditionEvaluator.Evaluate(condition, condTreeParams)
4646
assert.Equal(t, result, true)
4747

‎optimizely/decision/evaluator/condition_tree.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,6 @@ const (
3232
orOperator = "or"
3333
)
3434

35-
type ConditionTreeParameters struct {
36-
User *entities.UserContext
37-
AudienceMap map[string]entities.Audience
38-
}
39-
40-
func NewCConditionTreeParameters(user *entities.UserContext, audience map[string]entities.Audience) *ConditionTreeParameters {
41-
return &ConditionTreeParameters{User: user, AudienceMap: audience}
42-
}
43-
4435
// ConditionTreeEvaluator evaluates a condition tree
4536
type ConditionTreeEvaluator struct {
4637
conditionEvaluatorMap map[string]ConditionEvaluator
@@ -59,15 +50,15 @@ func NewConditionTreeEvaluator() *ConditionTreeEvaluator {
5950

6051
// entities.UserContext
6152
// Evaluate returns true if the userAttributes satisfy the given condition tree
62-
func (c ConditionTreeEvaluator) Evaluate(node *entities.ConditionTreeNode, condTreeParams *ConditionTreeParameters) bool {
53+
func (c ConditionTreeEvaluator) Evaluate(node *entities.ConditionTreeNode, condTreeParams *entities.ConditionTreeParameters) bool {
6354
// This wrapper method converts the conditionEvalResult to a boolean
6455
result, _ := c.evaluate(node, condTreeParams)
6556
return result == true
6657
}
6758

6859
// Helper method to recursively evaluate a condition tree
6960
// Returns the result of the evaluation and whether the evaluation of the condition is valid or not (to handle null bubbling)
70-
func (c ConditionTreeEvaluator) evaluate(node *entities.ConditionTreeNode, condTreeParams *ConditionTreeParameters) (evalResult bool, isValid bool) {
61+
func (c ConditionTreeEvaluator) evaluate(node *entities.ConditionTreeNode, condTreeParams *entities.ConditionTreeParameters) (evalResult bool, isValid bool) {
7162
operator := node.Operator
7263
if operator != "" {
7364
switch operator {
@@ -96,7 +87,7 @@ func (c ConditionTreeEvaluator) evaluate(node *entities.ConditionTreeNode, condT
9687
return result, true
9788
}
9889

99-
func (c ConditionTreeEvaluator) evaluateAnd(nodes []*entities.ConditionTreeNode, condTreeParams *ConditionTreeParameters) (evalResult bool, isValid bool) {
90+
func (c ConditionTreeEvaluator) evaluateAnd(nodes []*entities.ConditionTreeNode, condTreeParams *entities.ConditionTreeParameters) (evalResult bool, isValid bool) {
10091
sawInvalid := false
10192
for _, node := range nodes {
10293
result, isValid := c.evaluate(node, condTreeParams)
@@ -115,7 +106,7 @@ func (c ConditionTreeEvaluator) evaluateAnd(nodes []*entities.ConditionTreeNode,
115106
return true, true
116107
}
117108

118-
func (c ConditionTreeEvaluator) evaluateNot(nodes []*entities.ConditionTreeNode, condTreeParams *ConditionTreeParameters) (evalResult bool, isValid bool) {
109+
func (c ConditionTreeEvaluator) evaluateNot(nodes []*entities.ConditionTreeNode, condTreeParams *entities.ConditionTreeParameters) (evalResult bool, isValid bool) {
119110
if len(nodes) > 0 {
120111
result, isValid := c.evaluate(nodes[0], condTreeParams)
121112
if !isValid {
@@ -126,7 +117,7 @@ func (c ConditionTreeEvaluator) evaluateNot(nodes []*entities.ConditionTreeNode,
126117
return false, false
127118
}
128119

129-
func (c ConditionTreeEvaluator) evaluateOr(nodes []*entities.ConditionTreeNode, condTreeParams *ConditionTreeParameters) (evalResult bool, isValid bool) {
120+
func (c ConditionTreeEvaluator) evaluateOr(nodes []*entities.ConditionTreeNode, condTreeParams *entities.ConditionTreeParameters) (evalResult bool, isValid bool) {
130121
sawInvalid := false
131122
for _, node := range nodes {
132123
result, isValid := c.evaluate(node, condTreeParams)

‎optimizely/decision/evaluator/condition_tree_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) {
4848
},
4949
},
5050
}
51-
condTreeParams := NewCConditionTreeParameters(&user, map[string]e.Audience{})
51+
condTreeParams := e.NewConditionTreeParameters(&user, map[string]e.Audience{})
5252
result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams)
5353
assert.True(t, result)
5454

@@ -87,7 +87,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) {
8787
},
8888
}
8989

90-
condTreeParams := NewCConditionTreeParameters(&user, map[string]e.Audience{})
90+
condTreeParams := e.NewConditionTreeParameters(&user, map[string]e.Audience{})
9191
result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams)
9292
assert.True(t, result)
9393

@@ -150,7 +150,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) {
150150
},
151151
}
152152

153-
condTreeParams := NewCConditionTreeParameters(&user, map[string]e.Audience{})
153+
condTreeParams := e.NewConditionTreeParameters(&user, map[string]e.Audience{})
154154
result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams)
155155
assert.False(t, result)
156156

@@ -224,7 +224,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) {
224224
},
225225
}
226226

227-
condTreeParams := NewCConditionTreeParameters(&user, map[string]e.Audience{})
227+
condTreeParams := e.NewConditionTreeParameters(&user, map[string]e.Audience{})
228228
result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams)
229229
assert.True(t, result)
230230

@@ -311,7 +311,7 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) {
311311
},
312312
}
313313

314-
condTreeParams := NewCConditionTreeParameters(&user, map[string]e.Audience{})
314+
condTreeParams := e.NewConditionTreeParameters(&user, map[string]e.Audience{})
315315
result := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams)
316316
assert.True(t, result)
317317

‎optimizely/decision/experiment_targeting_service.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,28 @@ func NewExperimentTargetingService() *ExperimentTargetingService {
3737
func (s ExperimentTargetingService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext) (ExperimentDecision, error) {
3838
experimentDecision := ExperimentDecision{}
3939
experiment := decisionContext.Experiment
40+
41+
if experiment.AudienceConditionTree != nil {
42+
43+
condTreeParams := entities.NewConditionTreeParameters(&userContext, decisionContext.AudienceMap)
44+
conditionTreeEvaluator := evaluator.NewConditionTreeEvaluator()
45+
evalResult := conditionTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams)
46+
if !evalResult {
47+
// user not targeted for experiment, return an empty variation
48+
experimentDecision.DecisionMade = true
49+
experimentDecision.Variation = entities.Variation{}
50+
}
51+
return experimentDecision, nil
52+
}
53+
4054
if len(experiment.AudienceIds) > 0 {
4155
experimentAudience := decisionContext.AudienceMap[experiment.AudienceIds[0]]
42-
condTreeParams := evaluator.NewCConditionTreeParameters(&userContext, map[string]entities.Audience{})
56+
condTreeParams := entities.NewConditionTreeParameters(&userContext, map[string]entities.Audience{})
4357
evalResult := s.audienceEvaluator.Evaluate(experimentAudience, condTreeParams)
44-
if evalResult == false {
58+
if !evalResult {
4559
// user not targeted for experiment, return an empty variation
4660
experimentDecision.DecisionMade = true
4761
experimentDecision.Variation = entities.Variation{}
48-
return experimentDecision, nil
4962
}
5063
}
5164
// user passes audience targeting, can move on to the next decision maker

‎optimizely/decision/experiment_targeting_service_test.go

Lines changed: 96 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@ type MockAudienceEvaluator struct {
2929
mock.Mock
3030
}
3131

32-
func (m *MockAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *evaluator.ConditionTreeParameters) bool {
32+
func (m *MockAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *entities.ConditionTreeParameters) bool {
3333
userContext := *condTreeParams.User
3434
args := m.Called(audience, userContext)
3535
return args.Bool(0)
3636
}
3737

38-
func TestExperimentTargetingGetDecision(t *testing.T) {
38+
// test with mocking
39+
func TestExperimentTargetingGetDecisionNoAudienceCondTree(t *testing.T) {
3940
testAudience := entities.Audience{
4041
ConditionTree: &entities.ConditionTreeNode{
4142
Operator: "or",
@@ -60,17 +61,6 @@ func TestExperimentTargetingGetDecision(t *testing.T) {
6061
"22222": testVariation,
6162
},
6263
AudienceIds: []string{"33333"},
63-
AudienceConditionTree: &entities.ConditionTreeNode{
64-
Operator: "or",
65-
Nodes: []*entities.ConditionTreeNode{
66-
&entities.ConditionTreeNode{
67-
Condition: entities.Condition{
68-
Name: "s_foo",
69-
Value: "33333",
70-
},
71-
},
72-
},
73-
},
7464
},
7565
AudienceMap: map[string]entities.Audience{
7666
"33333": testAudience,
@@ -125,3 +115,96 @@ func TestExperimentTargetingGetDecision(t *testing.T) {
125115
assert.Equal(t, expectedExperimentDecision, decision)
126116
mockAudienceEvaluator.AssertExpectations(t)
127117
}
118+
119+
// Real tests with no mocking
120+
func TestExperimentTargetingGetDecisionWithAudienceCondTree(t *testing.T) {
121+
testAudience := entities.Audience{
122+
ConditionTree: &entities.ConditionTreeNode{
123+
Operator: "or",
124+
Nodes: []*entities.ConditionTreeNode{
125+
{
126+
Condition: entities.Condition{
127+
Name: "s_foo",
128+
Type: "custom_attribute",
129+
Match: "exact",
130+
Value: "foo",
131+
},
132+
},
133+
},
134+
},
135+
}
136+
testVariation := entities.Variation{
137+
ID: "22222",
138+
Key: "22222",
139+
}
140+
testDecisionContext := ExperimentDecisionContext{
141+
Experiment: entities.Experiment{
142+
ID: "111111",
143+
Variations: map[string]entities.Variation{
144+
"22222": testVariation,
145+
},
146+
AudienceIds: []string{"33333"},
147+
AudienceConditionTree: &entities.ConditionTreeNode{
148+
Operator: "or",
149+
Nodes: []*entities.ConditionTreeNode{
150+
{
151+
Condition: entities.Condition{
152+
Name: "optimizely_generated",
153+
Type: "audience_condition",
154+
Value: "33333",
155+
},
156+
},
157+
},
158+
},
159+
},
160+
AudienceMap: map[string]entities.Audience{
161+
"33333": testAudience,
162+
},
163+
}
164+
165+
// test does not pass audience evaluation
166+
testUserContext := entities.UserContext{
167+
ID: "test_user_1",
168+
Attributes: entities.UserAttributes{
169+
Attributes: map[string]interface{}{
170+
"s_foo": "not_foo",
171+
},
172+
},
173+
}
174+
expectedExperimentDecision := ExperimentDecision{
175+
Decision: Decision{
176+
DecisionMade: true,
177+
},
178+
Variation: entities.Variation{},
179+
}
180+
181+
audienceEvaluator := evaluator.NewTypedAudienceEvaluator()
182+
experimentTargetingService := ExperimentTargetingService{
183+
audienceEvaluator: audienceEvaluator,
184+
}
185+
186+
decision, _ := experimentTargetingService.GetDecision(testDecisionContext, testUserContext)
187+
assert.Equal(t, expectedExperimentDecision, decision) //decision made but did not pass
188+
189+
/****** Perfect Match ***************/
190+
191+
testUserContext = entities.UserContext{
192+
ID: "test_user_1",
193+
Attributes: entities.UserAttributes{
194+
Attributes: map[string]interface{}{
195+
"s_foo": "foo",
196+
},
197+
},
198+
}
199+
200+
expectedExperimentDecision = ExperimentDecision{
201+
Decision: Decision{
202+
DecisionMade: false,
203+
},
204+
Variation: entities.Variation{},
205+
}
206+
207+
decision, _ = experimentTargetingService.GetDecision(testDecisionContext, testUserContext)
208+
assert.Equal(t, expectedExperimentDecision, decision) // decision not made? but it passed
209+
210+
}

0 commit comments

Comments
 (0)