Skip to content

Commit 52e6d19

Browse files
author
Dave Johnston
authored
[FFM-1452]: Fix bool, int and number comparisons + implement slices
This change fixes the bool, int and number comparisons. It replaces a cast with the appropriate function from the strconv package. The slice type is implemented with support for in, contains and equals operators. Customers can store target attributes as strings, float64, bool and slices. When creating a rule the value is always a slice of strings. The code was trying to cast a string to a int, float etc which does not work. This caused rules evaluations to fail. Added unit tests to verify the type operators now work as expected.
1 parent 42cda01 commit 52e6d19

File tree

11 files changed

+437
-111
lines changed

11 files changed

+437
-111
lines changed

evaluation/target.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,10 @@ func (t Target) GetOperator(attr string) (types.ValueType, error) {
5757
case reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int8, reflect.Uint, reflect.Uint16,
5858
reflect.Uint32, reflect.Uint64, reflect.Uint8:
5959
return types.Integer(value.Int()), nil
60+
case reflect.Slice:
61+
return types.NewSlice(value.Interface()), nil
6062
case reflect.Array, reflect.Chan, reflect.Complex128, reflect.Complex64, reflect.Func, reflect.Interface,
61-
reflect.Invalid, reflect.Map, reflect.Ptr, reflect.Slice, reflect.Struct, reflect.Uintptr, reflect.UnsafePointer:
63+
reflect.Invalid, reflect.Map, reflect.Ptr, reflect.Struct, reflect.Uintptr, reflect.UnsafePointer:
6264
fallthrough
6365
default:
6466
return nil, fmt.Errorf("unexpected type: %s", value.Kind().String())

types/bool.go

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package types
22

33
import (
44
"fmt"
5+
"strconv"
6+
7+
"github.com/drone/ff-golang-server-sdk/log"
58
)
69

710
// Boolean type for clause attribute evaluation
@@ -17,61 +20,70 @@ func NewBoolean(value interface{}) (*Boolean, error) {
1720
return nil, fmt.Errorf("%v: cant cast to a booelan", ErrWrongTypeAssertion)
1821
}
1922

23+
// boolOperator iterates over value, converts it to a float64 and calls fn on each element
24+
func boolOperator(value []string, fn func(bool) bool) bool {
25+
if len(value) > 0 {
26+
if i, err := strconv.ParseBool(value[0]); err == nil {
27+
return fn(i)
28+
}
29+
log.Warnf("input contains invalid value for bool comparisons: %s\n", value)
30+
}
31+
return false
32+
}
33+
2034
// StartsWith always return false
21-
func (b Boolean) StartsWith(value interface{}) bool {
35+
func (b Boolean) StartsWith(value []string) bool {
2236
return false
2337
}
2438

2539
// EndsWith always return false
26-
func (b Boolean) EndsWith(value interface{}) bool {
40+
func (b Boolean) EndsWith(value []string) bool {
2741
return false
2842
}
2943

3044
// Match always return false
31-
func (b Boolean) Match(value interface{}) bool {
45+
func (b Boolean) Match(value []string) bool {
3246
return false
3347
}
3448

3549
// Contains always return false
36-
func (b Boolean) Contains(value interface{}) bool {
50+
func (b Boolean) Contains(value []string) bool {
3751
return false
3852
}
3953

4054
// EqualSensitive always return false
41-
func (b Boolean) EqualSensitive(value interface{}) bool {
55+
func (b Boolean) EqualSensitive(value []string) bool {
4256
return false
4357
}
4458

4559
// Equal check if the boolean and value are equal
46-
func (b Boolean) Equal(value interface{}) bool {
47-
val, ok := value.(bool)
48-
if ok {
49-
return Boolean(val) == b
50-
}
51-
return false
60+
func (b Boolean) Equal(value []string) bool {
61+
return boolOperator(value, func(f bool) bool {
62+
return bool(b) == f
63+
})
5264
}
5365

5466
// GreaterThan always return false
55-
func (b Boolean) GreaterThan(value interface{}) bool {
67+
func (b Boolean) GreaterThan(value []string) bool {
5668
return false
5769
}
5870

5971
// GreaterThanEqual always return false
60-
func (b Boolean) GreaterThanEqual(value interface{}) bool {
72+
func (b Boolean) GreaterThanEqual(value []string) bool {
6173
return false
6274
}
6375

6476
// LessThan always return false
65-
func (b Boolean) LessThan(value interface{}) bool {
77+
func (b Boolean) LessThan(value []string) bool {
6678
return false
6779
}
6880

6981
// LessThanEqual always return false
70-
func (b Boolean) LessThanEqual(value interface{}) bool {
82+
func (b Boolean) LessThanEqual(value []string) bool {
7183
return false
7284
}
7385

7486
// In always return false
75-
func (b Boolean) In(value interface{}) bool {
87+
func (b Boolean) In(value []string) bool {
7688
return false
7789
}

types/bool_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package types
2+
3+
import "testing"
4+
5+
func TestBoolean_Equal(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
b Boolean
9+
args []string
10+
want bool
11+
}{
12+
{"test equal returns true with match", Boolean(true), []string{"true"}, true},
13+
{"test equal returns false when no match", Boolean(true), []string{"false"}, false},
14+
{"test equal returns true with multiple values", Boolean(false), []string{"false", "true"}, true},
15+
{"test equal only matches first value", Boolean(false), []string{"true", "false"}, false},
16+
{"test equal returns false for invalid value", Boolean(true), []string{"on"}, false},
17+
}
18+
for _, tt := range tests {
19+
t.Run(tt.name, func(t *testing.T) {
20+
if got := tt.b.Equal(tt.args); got != tt.want {
21+
t.Errorf("Equal() = %v, want %v", got, tt.want)
22+
}
23+
})
24+
}
25+
}

types/int.go

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package types
22

33
import (
44
"fmt"
5+
"strconv"
6+
7+
"github.com/drone/ff-golang-server-sdk/log"
58
)
69

710
// Integer type for clause attribute evaluation
@@ -17,82 +20,86 @@ func NewInteger(value interface{}) (*Integer, error) {
1720
return nil, fmt.Errorf("%v: cant cast to a integer", ErrWrongTypeAssertion)
1821
}
1922

20-
func (n Integer) operator(value interface{}, fn func(int64) bool) bool {
21-
num, ok := value.([]int64)
22-
if ok {
23-
return fn(num[0])
23+
// intOperator takes the first element from the slice, converts it to a int64 and passes to fn for processing.
24+
// we ignore any additional elements if they exist.
25+
func intOperator(value []string, fn func(int64) bool) bool {
26+
if len(value) > 0 {
27+
if i, err := strconv.ParseInt(value[0], 10, 64); err == nil {
28+
if fn(i) {
29+
return true
30+
}
31+
}
32+
log.Warnf("input contains invalid value for integer comparisons: %s\n", value)
2433
}
34+
2535
return false
2636
}
2737

2838
// StartsWith always return false
29-
func (n Integer) StartsWith(interface{}) bool {
39+
func (n Integer) StartsWith([]string) bool {
3040
return false
3141
}
3242

3343
// EndsWith always return false
34-
func (n Integer) EndsWith(interface{}) bool {
44+
func (n Integer) EndsWith([]string) bool {
3545
return false
3646
}
3747

3848
// Match always return false
39-
func (n Integer) Match(interface{}) bool {
49+
func (n Integer) Match([]string) bool {
4050
return false
4151
}
4252

4353
// Contains always return false
44-
func (n Integer) Contains(interface{}) bool {
54+
func (n Integer) Contains([]string) bool {
4555
return false
4656
}
4757

4858
// EqualSensitive always return false
49-
func (n Integer) EqualSensitive(interface{}) bool {
59+
func (n Integer) EqualSensitive([]string) bool {
5060
return false
5161
}
5262

5363
// Equal check if the number and value are equal
54-
func (n Integer) Equal(value interface{}) bool {
55-
return n.operator(value, func(f int64) bool {
64+
func (n Integer) Equal(value []string) bool {
65+
return intOperator(value, func(f int64) bool {
5666
return int64(n) == f
5767
})
5868
}
5969

6070
// GreaterThan checks if the number is greater than the value
61-
func (n Integer) GreaterThan(value interface{}) bool {
62-
return n.operator(value, func(f int64) bool {
71+
func (n Integer) GreaterThan(value []string) bool {
72+
return intOperator(value, func(f int64) bool {
6373
return int64(n) > f
6474
})
6575
}
6676

6777
// GreaterThanEqual checks if the number is greater or equal than the value
68-
func (n Integer) GreaterThanEqual(value interface{}) bool {
69-
return n.operator(value, func(f int64) bool {
78+
func (n Integer) GreaterThanEqual(value []string) bool {
79+
return intOperator(value, func(f int64) bool {
7080
return int64(n) >= f
7181
})
7282
}
7383

7484
// LessThan checks if the number is less than the value
75-
func (n Integer) LessThan(value interface{}) bool {
76-
return n.operator(value, func(f int64) bool {
85+
func (n Integer) LessThan(value []string) bool {
86+
return intOperator(value, func(f int64) bool {
7787
return int64(n) < f
7888
})
7989
}
8090

8191
// LessThanEqual checks if the number is less or equal than the value
82-
func (n Integer) LessThanEqual(value interface{}) bool {
83-
return n.operator(value, func(f int64) bool {
92+
func (n Integer) LessThanEqual(value []string) bool {
93+
return intOperator(value, func(f int64) bool {
8494
return int64(n) <= f
8595
})
8696
}
8797

8898
// In checks if the number exist in slice of numbers (value)
89-
func (n Integer) In(value interface{}) bool {
90-
array, ok := value.([]interface{})
91-
if ok {
92-
for _, val := range array {
93-
if n.Equal(val) {
94-
return true
95-
}
99+
func (n Integer) In(value []string) bool {
100+
for _, x := range value {
101+
if n.Equal([]string{x}) {
102+
return true
96103
}
97104
}
98105
return false

types/int_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package types
2+
3+
import "testing"
4+
5+
func TestInteger_Equal(t *testing.T) {
6+
7+
tests := []struct {
8+
name string
9+
n Integer
10+
args []string
11+
want bool
12+
}{
13+
{"test equal returns true with match", Integer(22), []string{"22"}, true},
14+
{"test equal returns false when no match", Integer(22), []string{"25"}, false},
15+
{"test equal returns true with multiple values", Integer(22), []string{"22", "23"}, true},
16+
{"test equal only matches first value", Integer(22), []string{"23", "22"}, false},
17+
{"test equal returns false for invalid value", Integer(22), []string{"true"}, false},
18+
}
19+
for _, tt := range tests {
20+
t.Run(tt.name, func(t *testing.T) {
21+
if got := tt.n.Equal(tt.args); got != tt.want {
22+
t.Errorf("Equal() = %v, want %v", got, tt.want)
23+
}
24+
})
25+
}
26+
}

0 commit comments

Comments
 (0)