Skip to content

Commit

Permalink
FFM-11164 Evaluate AND rules (#149)
Browse files Browse the repository at this point in the history
  • Loading branch information
@erdirowlands
erdirowlands authored Apr 15, 2024
1 parent aa45754 commit dffa98c
Show file tree
Hide file tree
Showing 2 changed files with 241 additions and 23 deletions.
42 changes: 37 additions & 5 deletions evaluation/evaluator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,22 @@ func (e Evaluator) evaluateRules(servingRules []rest.ServingRule, target *Target
return ""
}

// evaluateGroupRulesV2 evaluates the group rules using AND logic instead of OR.
func (e Evaluator) evaluateGroupRulesV2(rules []rest.Clause, target *Target) bool {
if len(rules) == 0 {
e.logger.Debugf("No 'AND' rules provided, returning false")
return false
}
for _, rule := range rules {
if !e.evaluateClause(&rule, target) {
e.logger.Debugf("'AND' rule did not match: %+v, returning false", rule)
return false
}
}
e.logger.Debugf("All 'AND' rules successfully matched for target: %+v", target)
return true
}

// evaluateGroupRules evaluates the groups rules. Note Group rule are represented by a rest.Clause, instead
// of a rest.Rule. Unlike feature clauses which are AND'd, in a case of a group these must be OR'd.
func (e Evaluator) evaluateGroupRules(rules []rest.Clause, target *Target) (bool, rest.Clause) {
Expand Down Expand Up @@ -262,11 +278,27 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar
return true
}

// Should Target be included via segment rules
rules := segment.Rules
// if rules is nil pointer or points to the empty slice
if rules != nil && len(*rules) > 0 {
if included, clause := e.evaluateGroupRules(*rules, target); included {
// `ServingRules` replaces `Rules, so if sent by the backend then we evaluate them instead
if segment.ServingRules != nil && len(*segment.ServingRules) > 0 {
v2Rules := *segment.ServingRules
sort.SliceStable(v2Rules, func(i, j int) bool {
return v2Rules[i].Priority < v2Rules[j].Priority
})
for _, v2rule := range v2Rules {
if e.evaluateGroupRulesV2(v2rule.Clauses, target) {
e.logger.Debugf(
"Target [%s] included in group [%s] via rules %+v", target.Name, segment.Name, v2Rules)
return true
}
}
return false
}

// Fall back to legacy `Rules`
if segment.Rules != nil && len(*segment.Rules) > 0 {
// Should Target be included via segment rules
legacyRules := *segment.Rules
if included, clause := e.evaluateGroupRules(legacyRules, target); included {
e.logger.Debugf(
"Target [%s] included in group [%s] via rule %+v", target.Name, segment.Name, clause)
return true
Expand Down
222 changes: 204 additions & 18 deletions evaluation/evaluator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,26 @@ import (
)

const (
identifier = "identifier"
harness = "harness"
beta = "beta"
alpha = "alpha"
excluded = "excluded"
offVariation = "false"
simple = "simple"
simpleWithPrereq = "simplePrereq"
notValidFlag = "notValidFlag"
theme = "theme"
size = "size"
weight = "weight"
org = "org"
invalidInt = "invalidInt"
invalidNumber = "invalidNumber"
invalidJSON = "invalidJSON"
prereqNotFound = "prereqNotFound"
prereqVarNotFound = "prereqVarNotFound"
identifier = "identifier"
harness = "harness"
beta = "beta"
alpha = "alpha"
v2GroupRulesAllAnd = "v2GroupRulesAllAnd"
v2GroupRulesANDWithOr = "v2GroupRulesAndWithOr"
excluded = "excluded"
offVariation = "false"
simple = "simple"
simpleWithPrereq = "simplePrereq"
notValidFlag = "notValidFlag"
theme = "theme"
size = "size"
weight = "weight"
org = "org"
invalidInt = "invalidInt"
invalidNumber = "invalidNumber"
invalidJSON = "invalidJSON"
prereqNotFound = "prereqNotFound"
prereqVarNotFound = "prereqVarNotFound"
)

var (
Expand Down Expand Up @@ -272,6 +274,64 @@ var (
},
},
},
v2GroupRulesAllAnd: {
Identifier: v2GroupRulesAllAnd,
ServingRules: &[]rest.GroupServingRule{
{
Priority: 1,
RuleId: "rule1",
Clauses: []rest.Clause{
{
Attribute: "email",
Op: endsWithOperator,
Values: []string{"@harness.io"},
},
{
Attribute: "role",
Op: equalOperator,
Values: []string{"sre"},
},
{
Attribute: "active",
Op: equalOperator,
Values: []string{"true"},
},
},
},
},
},
v2GroupRulesANDWithOr: {
Identifier: v2GroupRulesAllAnd,
ServingRules: &[]rest.GroupServingRule{
{
Priority: 1,
RuleId: "rule1",
Clauses: []rest.Clause{
{
Attribute: "email",
Op: endsWithOperator,
Values: []string{"@harness.io"},
},
},
},
{
Priority: 2,
RuleId: "rule2",
Clauses: []rest.Clause{
{
Attribute: "role",
Op: equalOperator,
Values: []string{"sre"},
},
{
Attribute: "active",
Op: equalOperator,
Values: []string{"true"},
},
},
},
},
},
},
)
)
Expand Down Expand Up @@ -363,6 +423,96 @@ func TestNewEvaluator(t *testing.T) {
}
}

func TestEvaluateGroupRulesV2(t *testing.T) {
e := &Evaluator{
logger: logger.NewNoOpLogger(),
}

// Define test cases
tests := []struct {
name string
clauses []rest.Clause
target *Target
expected bool
}{
{
name: "All conditions met",
clauses: []rest.Clause{
{
Attribute: "email",
Op: endsWithOperator,
Values: []string{"@harness.io"},
},
{
Attribute: "role",
Op: equalOperator,
Values: []string{"developer"},
},
},
target: &Target{
Attributes: &map[string]interface{}{
"email": "user@harness.io",
"role": "developer",
},
},
expected: true,
},
{
name: "One condition not met",
clauses: []rest.Clause{
{
Attribute: "email",
Op: endsWithOperator,
Values: []string{"@harness.io"},
},
{
Attribute: "role",
Op: equalOperator,
Values: []string{"developer"},
},
},
target: &Target{
Attributes: &map[string]interface{}{
"email": "user@harness.io",
"role": "manager",
},
},
expected: false,
},
{
name: "No conditions met",
clauses: []rest.Clause{
{
Attribute: "email",
Op: endsWithOperator,
Values: []string{"@harness.io"},
},
{
Attribute: "role",
Op: equalOperator,
Values: []string{"developer"},
},
},
target: &Target{
Attributes: &map[string]interface{}{
"email": "user@otherdomain.com",
"role": "manager",
},
},
expected: false,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := e.evaluateGroupRulesV2(tc.clauses, tc.target)
if result != tc.expected {
t.Errorf("TestEvaluateGroupRulesV2(%s) got %v, want %v", tc.name, result, tc.expected)
}
})
}
}

func TestEvaluator_evaluateClause(t *testing.T) {
type fields struct {
query Query
Expand Down Expand Up @@ -1202,6 +1352,42 @@ func TestEvaluator_isTargetIncludedOrExcludedInSegment(t *testing.T) {
},
want: false,
},
{
name: "one AND rule",
fields: fields{
query: testRepo,
},
args: args{
segmentList: []string{v2GroupRulesAllAnd},
target: &Target{
Identifier: "no_identifier",
Attributes: &map[string]interface{}{
"email": "hello@harness.io",
"role": "sre",
"active": true,
},
},
},
want: true,
},
{
name: "one AND with one OR",
fields: fields{
query: testRepo,
},
args: args{
segmentList: []string{v2GroupRulesANDWithOr},
target: &Target{
Identifier: "no_identifier",
Attributes: &map[string]interface{}{
"email": "hello@contractor.io",
"role": "sre",
"active": true,
},
},
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit dffa98c

Please sign in to comment.