diff --git a/evaluation/feature.go b/evaluation/feature.go index e7bbb679..a1fc0287 100644 --- a/evaluation/feature.go +++ b/evaluation/feature.go @@ -51,7 +51,16 @@ func (c *Clause) Evaluate(target *Target, segments Segments, operator types.Valu if segments == nil { return false } - return segments.Evaluate(target) + // Determine if the given target belongs to one of the segments, + // that was specified by the clause + for _, segmentName := range c.Value { + if segment, ok := segments[segmentName]; ok { + if segment.Evaluate(target) { + return true + } + } + } + return false } // Ensure operator is valid and not nil @@ -100,6 +109,7 @@ func (c Clauses) Evaluate(target *Target, segments Segments) bool { log.Warn(err) } } + if !clause.Evaluate(target, segments, op) { return false } @@ -118,9 +128,12 @@ type Distribution struct { // GetKeyName returns variation identifier based on target func (d *Distribution) GetKeyName(target *Target) string { variation := "" + + totalPercentage := 0 for _, tdVariation := range d.Variations { variation = tdVariation.Variation - if d.isEnabled(target, tdVariation.Weight) { + totalPercentage += tdVariation.Weight + if d.isEnabled(target, totalPercentage) { return variation } } diff --git a/evaluation/feature_test.go b/evaluation/feature_test.go index 8255bb9f..5258428f 100644 --- a/evaluation/feature_test.go +++ b/evaluation/feature_test.go @@ -556,34 +556,47 @@ func TestClause_Evaluate(t *testing.T) { Anonymous: &f, Attributes: &m, } - tests := []struct { + tests := map[string]struct { name string fields fields args args want bool }{ - {name: "segment match operator (include)", fields: struct { - Attribute string - ID string - Negate bool - Op string - Value []string - }{Op: segmentMatchOperator, Value: []string{"beta"}}, - args: struct { - target *Target - segments Segments - operator types.ValueType - }{target: &target, segments: map[string]*Segment{ + "segment match operator (include)": { + fields: fields{Op: segmentMatchOperator, Value: []string{"beta"}}, + args: args{target: &target, segments: map[string]*Segment{ "beta": { Identifier: "beta", Name: "Beta users", Included: []string{target.Identifier}, }, - }, operator: nil}, want: true}, + }, operator: nil}, + want: true, + }, + "evaluate returns false when clause value does not match any segment": { + fields: fields{Op: segmentMatchOperator, Value: []string{"beta"}}, + args: args{ + target: &target, + segments: Segments{ + "beta": {Identifier: "beta", Included: []string{}}, + "alpha": {Identifier: "alpha", Included: []string{target.Identifier}}, + }, operator: nil}, + want: false, + }, + "evaluate returns true when clause value matches segment that target belongs to": { + fields: fields{Op: segmentMatchOperator, Value: []string{"alpha"}}, + args: args{ + target: &target, + segments: Segments{ + "beta": {Identifier: "beta", Excluded: []string{target.Identifier}}, + "alpha": {Identifier: "alpha", Included: []string{target.Identifier}}, + }, operator: nil}, + want: true, + }, } - for _, tt := range tests { + for name, tt := range tests { val := tt - t.Run(tt.name, func(t *testing.T) { + t.Run(name, func(t *testing.T) { c := &Clause{ Attribute: val.fields.Attribute, ID: val.fields.ID, diff --git a/evaluation/segment.go b/evaluation/segment.go index e44974fe..4ee2befc 100644 --- a/evaluation/segment.go +++ b/evaluation/segment.go @@ -107,9 +107,9 @@ type Segments map[string]*Segment // Evaluate through all segments based on target input func (s Segments) Evaluate(target *Target) bool { for _, segment := range s { - if !segment.Evaluate(target) { - return false + if segment.Evaluate(target) { + return true } } - return true + return false } diff --git a/evaluation/segment_test.go b/evaluation/segment_test.go index 233396e2..aa83a250 100644 --- a/evaluation/segment_test.go +++ b/evaluation/segment_test.go @@ -51,3 +51,67 @@ func TestSegment_Evaluate(t *testing.T) { }) } } + +func TestSegments_Evaluate(t *testing.T) { + f := false + m := make(map[string]interface{}) + m["email"] = "john@doe.com" + target := Target{ + Identifier: "john", + Anonymous: &f, + Attributes: &m, + } + + tests := map[string]struct { + segments Segments + target Target + want bool + }{ + "test target included by segment alpha returns true": { + segments: Segments{"alpha": {Identifier: "alpha", Included: []string{target.Identifier}}}, + target: target, + want: true, + }, + "test target not included segment alpha, but included in beta returns true": { + segments: Segments{ + "alpha": {Identifier: "alpha", Included: []string{}}, + "beta": {Identifier: "beta", Included: []string{target.Identifier}}, + }, + target: target, + want: true, + }, + "test target not included segment alpha, and not included in beta returns false": { + segments: Segments{ + "alpha": {Identifier: "alpha", Included: []string{}}, + "beta": {Identifier: "beta", Included: []string{}}, + }, + target: target, + want: false, + }, + "test target included segment alpha, and excluded in beta returns true": { + segments: Segments{ + "alpha": {Identifier: "alpha", Included: []string{target.Identifier}}, + "beta": {Identifier: "beta", Excluded: []string{target.Identifier}}, + }, + target: target, + want: true, + }, + "test target excluded from segment alpha, and included in beta returns true": { + segments: Segments{ + "alpha": {Identifier: "alpha", Excluded: []string{target.Identifier}}, + "beta": {Identifier: "beta", Included: []string{target.Identifier}}, + }, + target: target, + want: true, + }, + } + for name, tt := range tests { + val := tt + t.Run(name, func(t *testing.T) { + s := val.segments + if got := s.Evaluate(&val.target); got != val.want { + t.Errorf("Evaluate() = %v, want %v", got, val.want) + } + }) + } +}