Skip to content

Commit dffa98c

Browse files
erdirowlandserdirowlands
authored
FFM-11164 Evaluate AND rules (#149)
1 parent aa45754 commit dffa98c

File tree

2 files changed

+241
-23
lines changed

2 files changed

+241
-23
lines changed

‎evaluation/evaluator.go‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,22 @@ func (e Evaluator) evaluateRules(servingRules []rest.ServingRule, target *Target
176176
return ""
177177
}
178178

179+
// evaluateGroupRulesV2 evaluates the group rules using AND logic instead of OR.
180+
func (e Evaluator) evaluateGroupRulesV2(rules []rest.Clause, target *Target) bool {
181+
if len(rules) == 0 {
182+
e.logger.Debugf("No 'AND' rules provided, returning false")
183+
return false
184+
}
185+
for _, rule := range rules {
186+
if !e.evaluateClause(&rule, target) {
187+
e.logger.Debugf("'AND' rule did not match: %+v, returning false", rule)
188+
return false
189+
}
190+
}
191+
e.logger.Debugf("All 'AND' rules successfully matched for target: %+v", target)
192+
return true
193+
}
194+
179195
// evaluateGroupRules evaluates the groups rules. Note Group rule are represented by a rest.Clause, instead
180196
// of a rest.Rule. Unlike feature clauses which are AND'd, in a case of a group these must be OR'd.
181197
func (e Evaluator) evaluateGroupRules(rules []rest.Clause, target *Target) (bool, rest.Clause) {
@@ -262,11 +278,27 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar
262278
return true
263279
}
264280

265-
// Should Target be included via segment rules
266-
rules := segment.Rules
267-
// if rules is nil pointer or points to the empty slice
268-
if rules != nil && len(*rules) > 0 {
269-
if included, clause := e.evaluateGroupRules(*rules, target); included {
281+
// `ServingRules` replaces `Rules, so if sent by the backend then we evaluate them instead
282+
if segment.ServingRules != nil && len(*segment.ServingRules) > 0 {
283+
v2Rules := *segment.ServingRules
284+
sort.SliceStable(v2Rules, func(i, j int) bool {
285+
return v2Rules[i].Priority < v2Rules[j].Priority
286+
})
287+
for _, v2rule := range v2Rules {
288+
if e.evaluateGroupRulesV2(v2rule.Clauses, target) {
289+
e.logger.Debugf(
290+
"Target [%s] included in group [%s] via rules %+v", target.Name, segment.Name, v2Rules)
291+
return true
292+
}
293+
}
294+
return false
295+
}
296+
297+
// Fall back to legacy `Rules`
298+
if segment.Rules != nil && len(*segment.Rules) > 0 {
299+
// Should Target be included via segment rules
300+
legacyRules := *segment.Rules
301+
if included, clause := e.evaluateGroupRules(legacyRules, target); included {
270302
e.logger.Debugf(
271303
"Target [%s] included in group [%s] via rule %+v", target.Name, segment.Name, clause)
272304
return true

‎evaluation/evaluator_test.go‎

Lines changed: 204 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,26 @@ import (
1111
)
1212

1313
const (
14-
identifier = "identifier"
15-
harness = "harness"
16-
beta = "beta"
17-
alpha = "alpha"
18-
excluded = "excluded"
19-
offVariation = "false"
20-
simple = "simple"
21-
simpleWithPrereq = "simplePrereq"
22-
notValidFlag = "notValidFlag"
23-
theme = "theme"
24-
size = "size"
25-
weight = "weight"
26-
org = "org"
27-
invalidInt = "invalidInt"
28-
invalidNumber = "invalidNumber"
29-
invalidJSON = "invalidJSON"
30-
prereqNotFound = "prereqNotFound"
31-
prereqVarNotFound = "prereqVarNotFound"
14+
identifier = "identifier"
15+
harness = "harness"
16+
beta = "beta"
17+
alpha = "alpha"
18+
v2GroupRulesAllAnd = "v2GroupRulesAllAnd"
19+
v2GroupRulesANDWithOr = "v2GroupRulesAndWithOr"
20+
excluded = "excluded"
21+
offVariation = "false"
22+
simple = "simple"
23+
simpleWithPrereq = "simplePrereq"
24+
notValidFlag = "notValidFlag"
25+
theme = "theme"
26+
size = "size"
27+
weight = "weight"
28+
org = "org"
29+
invalidInt = "invalidInt"
30+
invalidNumber = "invalidNumber"
31+
invalidJSON = "invalidJSON"
32+
prereqNotFound = "prereqNotFound"
33+
prereqVarNotFound = "prereqVarNotFound"
3234
)
3335

3436
var (
@@ -272,6 +274,64 @@ var (
272274
},
273275
},
274276
},
277+
v2GroupRulesAllAnd: {
278+
Identifier: v2GroupRulesAllAnd,
279+
ServingRules: &[]rest.GroupServingRule{
280+
{
281+
Priority: 1,
282+
RuleId: "rule1",
283+
Clauses: []rest.Clause{
284+
{
285+
Attribute: "email",
286+
Op: endsWithOperator,
287+
Values: []string{"@harness.io"},
288+
},
289+
{
290+
Attribute: "role",
291+
Op: equalOperator,
292+
Values: []string{"sre"},
293+
},
294+
{
295+
Attribute: "active",
296+
Op: equalOperator,
297+
Values: []string{"true"},
298+
},
299+
},
300+
},
301+
},
302+
},
303+
v2GroupRulesANDWithOr: {
304+
Identifier: v2GroupRulesAllAnd,
305+
ServingRules: &[]rest.GroupServingRule{
306+
{
307+
Priority: 1,
308+
RuleId: "rule1",
309+
Clauses: []rest.Clause{
310+
{
311+
Attribute: "email",
312+
Op: endsWithOperator,
313+
Values: []string{"@harness.io"},
314+
},
315+
},
316+
},
317+
{
318+
Priority: 2,
319+
RuleId: "rule2",
320+
Clauses: []rest.Clause{
321+
{
322+
Attribute: "role",
323+
Op: equalOperator,
324+
Values: []string{"sre"},
325+
},
326+
{
327+
Attribute: "active",
328+
Op: equalOperator,
329+
Values: []string{"true"},
330+
},
331+
},
332+
},
333+
},
334+
},
275335
},
276336
)
277337
)
@@ -363,6 +423,96 @@ func TestNewEvaluator(t *testing.T) {
363423
}
364424
}
365425

426+
func TestEvaluateGroupRulesV2(t *testing.T) {
427+
e := &Evaluator{
428+
logger: logger.NewNoOpLogger(),
429+
}
430+
431+
// Define test cases
432+
tests := []struct {
433+
name string
434+
clauses []rest.Clause
435+
target *Target
436+
expected bool
437+
}{
438+
{
439+
name: "All conditions met",
440+
clauses: []rest.Clause{
441+
{
442+
Attribute: "email",
443+
Op: endsWithOperator,
444+
Values: []string{"@harness.io"},
445+
},
446+
{
447+
Attribute: "role",
448+
Op: equalOperator,
449+
Values: []string{"developer"},
450+
},
451+
},
452+
target: &Target{
453+
Attributes: &map[string]interface{}{
454+
"email": "[email protected]",
455+
"role": "developer",
456+
},
457+
},
458+
expected: true,
459+
},
460+
{
461+
name: "One condition not met",
462+
clauses: []rest.Clause{
463+
{
464+
Attribute: "email",
465+
Op: endsWithOperator,
466+
Values: []string{"@harness.io"},
467+
},
468+
{
469+
Attribute: "role",
470+
Op: equalOperator,
471+
Values: []string{"developer"},
472+
},
473+
},
474+
target: &Target{
475+
Attributes: &map[string]interface{}{
476+
"email": "[email protected]",
477+
"role": "manager",
478+
},
479+
},
480+
expected: false,
481+
},
482+
{
483+
name: "No conditions met",
484+
clauses: []rest.Clause{
485+
{
486+
Attribute: "email",
487+
Op: endsWithOperator,
488+
Values: []string{"@harness.io"},
489+
},
490+
{
491+
Attribute: "role",
492+
Op: equalOperator,
493+
Values: []string{"developer"},
494+
},
495+
},
496+
target: &Target{
497+
Attributes: &map[string]interface{}{
498+
"email": "[email protected]",
499+
"role": "manager",
500+
},
501+
},
502+
expected: false,
503+
},
504+
}
505+
506+
for _, tc := range tests {
507+
t.Run(tc.name, func(t *testing.T) {
508+
result := e.evaluateGroupRulesV2(tc.clauses, tc.target)
509+
if result != tc.expected {
510+
t.Errorf("TestEvaluateGroupRulesV2(%s) got %v, want %v", tc.name, result, tc.expected)
511+
}
512+
})
513+
}
514+
}
515+
366516
func TestEvaluator_evaluateClause(t *testing.T) {
367517
type fields struct {
368518
query Query
@@ -1202,6 +1352,42 @@ func TestEvaluator_isTargetIncludedOrExcludedInSegment(t *testing.T) {
12021352
},
12031353
want: false,
12041354
},
1355+
{
1356+
name: "one AND rule",
1357+
fields: fields{
1358+
query: testRepo,
1359+
},
1360+
args: args{
1361+
segmentList: []string{v2GroupRulesAllAnd},
1362+
target: &Target{
1363+
Identifier: "no_identifier",
1364+
Attributes: &map[string]interface{}{
1365+
"email": "[email protected]",
1366+
"role": "sre",
1367+
"active": true,
1368+
},
1369+
},
1370+
},
1371+
want: true,
1372+
},
1373+
{
1374+
name: "one AND with one OR",
1375+
fields: fields{
1376+
query: testRepo,
1377+
},
1378+
args: args{
1379+
segmentList: []string{v2GroupRulesANDWithOr},
1380+
target: &Target{
1381+
Identifier: "no_identifier",
1382+
Attributes: &map[string]interface{}{
1383+
"email": "[email protected]",
1384+
"role": "sre",
1385+
"active": true,
1386+
},
1387+
},
1388+
},
1389+
want: true,
1390+
},
12051391
}
12061392
for _, tt := range tests {
12071393
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)